Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Francois Romieu 
Måns Rullgård  :
> Francois Romieu  writes:
[...]
> > It looks like it receives, then tries to allocate new resources. If so
> > it may deplete the ring and you should instead consider allocating new
> > resources first, then receive data if alloc + map suceeded.
> 
> The hardware receives a frame and stores it in the provided DMA buffer,
> then raises an interrupt and moves on to the next buffer.  When a buffer
> is handed over to the network stack, a new one has to take its place in
> the DMA queue.  I'm not sure how you're suggesting this be done
> differently.

- The hardware raises an interrupt and moves on to the next packet
- The driver allocates a new buffer - call it Bob - and maps it.
  - If it succeeds
- The driver unmaps the received packet
- The driver hands the received packet to the network stack
- The driver hands Bob to the hardware
  - It it fails
- The driver updates the dropped packet stats
- The driver recycles the received - yet non-unmapped - packet to the
  hardware

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT] Networking

2015-10-31 Thread David Miller 
From: David Miller 
Date: Thu, 29 Oct 2015 08:19:41 -0700 (PDT)

> This is the same as the previous pull request, with the ipv6 overflow
> fix redone.  The merge conflict is therefore gone too.
 ...
> Please pull, thanks a lot.
> 
> The following changes since commit 1099f86044111e9a7807f09523e42d4c9d0fb781:
> 
>   Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-10-19 
> 09:55:40 -0700)

Ping?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Måns Rullgård 
Andy Shevchenko  writes:

>> +static int nb8800_mdio_read(struct mii_bus *bus, int phy_id, int reg)
>> +{
>> +   struct nb8800_priv *priv = bus->priv;
>> +   int val;
>> +
>> +   if (!nb8800_mdio_wait(bus))
>> +   return -ETIMEDOUT;
>> +
>> +   val = MIIAR_ADDR(phy_id) | MIIAR_REG(reg);
>> +
>> +   nb8800_writel(priv, NB8800_MDIO_CMD, val);
>> +   udelay(10);
>
> Why 10? Perhaps add a comment line.

Because it works.  The documentation doesn't mention a delay, but it
works unreliably without one.

>> +   nb8800_writel(priv, NB8800_MDIO_CMD, val | MDIO_CMD_GO);
>> +
>> +   if (!nb8800_mdio_wait(bus))
>> +   return -ETIMEDOUT;
>> +
>> +   val = nb8800_readl(priv, NB8800_MDIO_STS);
>> +   if (val & MDIO_STS_ERR)
>> +   return 0x;
>
> Can we return an error here?

That breaks the bus scanning in phylib.

>> +
>> +   return val & 0x;
>> +}

[...]

>> +static int nb8800_poll(struct napi_struct *napi, int budget)
>> +{
>> +   struct net_device *dev = napi->dev;
>> +   struct nb8800_priv *priv = netdev_priv(dev);
>> +   struct nb8800_dma_desc *rx;
>> +   int work = 0;
>> +   int last = priv->rx_eoc;
>> +   int next;
>> +
>> +   while (work < budget) {
>> +   struct rx_buf *rx_buf;
>> +   u32 report;
>> +   int len;
>> +
>> +   next = (last + 1) & (RX_DESC_COUNT - 1);
>
> Maybe (last + 1) % RX_DESC_COUNT ? It will not prevent to use
> non-power-of-two numbers.

We don't want to be doing divisions anyway, but I can certainly change
it to % if that's preferred.

>> +
>> +   rx_buf = >rx_bufs[next];
>> +   rx = >rx_descs[next];
>
>> +   report = rx->report;
>
> Maybe you can use rx->report directly below.

It's in uncached memory, so didn't want to have gcc accidentally doing
more reads than necessary.

>> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +   struct nb8800_priv *priv = netdev_priv(dev);
>> +   struct tx_skb_data *skb_data;
>> +   struct tx_buf *tx_buf;
>> +   dma_addr_t dma_addr;
>> +   unsigned int dma_len;
>> +   int cpsz, next;
>> +   int frags;
>> +
>> +   if (atomic_read(>tx_free) <= NB8800_DESC_LOW) {
>> +   netif_stop_queue(dev);
>> +   return NETDEV_TX_BUSY;
>> +   }
>> +
>> +   cpsz = (8 - (uintptr_t)skb->data) & 7;
>
> So, cast to uintptr_t looks strange in this driver, since used only
> twice in such expression, why not to use plain unsigned int * ?

Because it's not the same thing.  uintptr_t is an integer type the same
size as a pointer.  I need to check if the data pointer is 8-byte
aligned as required by the DMA.

>> +static void nb8800_set_rx_mode(struct net_device *dev)
>> +{
>> +   struct nb8800_priv *priv = netdev_priv(dev);
>> +   struct netdev_hw_addr *ha;
>> +   bool af_en;
>> +   int i;
>> +
>> +   if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI))
>> +   af_en = false;
>> +   else
>> +   af_en = true;
>> +
>> +   nb8800_mac_af(dev, af_en);
>> +
>> +   if (!af_en)
>> +   return;
>
> Would it be
>
> if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) {
>   nb8800_mac_af(dev, false);
>   return;
> }
>
> nb8800_mac_af(dev, true);
>
> ?

Looks equivalent.  Maybe that's clearer.

>> +static int nb8800_hw_init(struct net_device *dev)
>> +{
>> +   struct nb8800_priv *priv = netdev_priv(dev);
>
>> +   unsigned int val = 0;
>
> Useless assignment.

Indeed.  Why didn't gcc warn about that?

>> +
>> +   nb8800_writeb(priv, NB8800_RANDOM_SEED, 0x08);
>> +
>> +   /* TX single deferral params */
>> +   nb8800_writeb(priv, NB8800_TX_SDP, 0xc);
>> +
>> +   /* Threshold for partial full */
>> +   nb8800_writeb(priv, NB8800_PF_THRESHOLD, 0xff);
>> +
>> +   /* Pause Quanta */
>> +   nb8800_writeb(priv, NB8800_PQ1, 0xff);
>> +   nb8800_writeb(priv, NB8800_PQ2, 0xff);
>
> Lot of magic numbers above and below.

Those are from the original driver.  Some of them disagree with the
documentation, and the "correct" values don't work.

>> +
>> +   /* configure TX DMA Channels */
>> +   val = nb8800_readl(priv, NB8800_TXC_CR);
>> +   val &= TCR_LE;
>> +   val |= TCR_DM | TCR_RS | TCR_TFI(1) | TCR_BTS(2);
>> +   nb8800_writel(priv, NB8800_TXC_CR, val);
>> +
>> +   /* TX Interrupt Time Register */
>> +   nb8800_writel(priv, NB8800_TX_ITR, 1);
>> +
>> +   /* configure RX DMA Channels */
>> +   val = nb8800_readl(priv, NB8800_RXC_CR);
>> +   val &= RCR_LE;
>> +   val |= RCR_DM | RCR_RS | RCR_RFI(7) | RCR_BTS(2) | RCR_FL;
>> +   nb8800_writel(priv, NB8800_RXC_CR, val);
>> +
>> +   /* RX Interrupt Time Register */
>> +   nb8800_writel(priv, NB8800_RX_ITR, 1);
>> +
>> +   val = TX_RETRY_EN | TX_PAD_EN | TX_APPEND_FCS;
>> +

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Måns Rullgård 
Francois Romieu  writes:

> Måns Rullgård  :
>> Francois Romieu  writes:
> [...]
>> > It looks like it receives, then tries to allocate new resources. If so
>> > it may deplete the ring and you should instead consider allocating new
>> > resources first, then receive data if alloc + map suceeded.
>> 
>> The hardware receives a frame and stores it in the provided DMA buffer,
>> then raises an interrupt and moves on to the next buffer.  When a buffer
>> is handed over to the network stack, a new one has to take its place in
>> the DMA queue.  I'm not sure how you're suggesting this be done
>> differently.
>
> - The hardware raises an interrupt and moves on to the next packet
> - The driver allocates a new buffer - call it Bob - and maps it.
>   - If it succeeds
> - The driver unmaps the received packet
> - The driver hands the received packet to the network stack
> - The driver hands Bob to the hardware
>   - It it fails
> - The driver updates the dropped packet stats
> - The driver recycles the received - yet non-unmapped - packet to the
>   hardware

Oh, I see.  That's better.

-- 
Måns Rullgård
m...@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Andy Shevchenko 
On Thu, Oct 29, 2015 at 6:48 PM, Mans Rullgard  wrote:
> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.
>
> Signed-off-by: Mans Rullgard 


> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -0,0 +1,1146 @@

> +#define nb8800_set_bits(sz, priv, reg, bits) do {  \
> +   u32 __o = nb8800_read##sz(priv, reg);   \
> +   u32 __n = __o | (bits); \
> +   if (__n != __o) \
> +   nb8800_write##sz(priv, reg, __n);   \
> +   } while (0)
> +
> +#define nb8800_clear_bits(sz, priv, reg, bits) do {\
> +   u32 __o = nb8800_read##sz(priv, reg);   \
> +   u32 __n = __o & ~(bits);\
> +   if (__n != __o) \
> +   nb8800_write##sz(priv, reg, __n);   \
> +   } while (0)
> +
> +#define MDIO_TIMEOUT   1000
> +
> +static int nb8800_mdio_wait(struct mii_bus *bus)
> +{
> +   struct nb8800_priv *priv = bus->priv;
> +   int tmo = MDIO_TIMEOUT;
> +
> +   while (--tmo) {
> +   if (!(nb8800_readl(priv, NB8800_MDIO_CMD) & MDIO_CMD_GO))
> +   break;
> +   udelay(1);

Can you use cpu_relax() or readx_poll_timeout() ?

> +   }
> +
> +   return tmo;
> +}
> +
> +static int nb8800_mdio_read(struct mii_bus *bus, int phy_id, int reg)
> +{
> +   struct nb8800_priv *priv = bus->priv;
> +   int val;
> +
> +   if (!nb8800_mdio_wait(bus))
> +   return -ETIMEDOUT;
> +
> +   val = MIIAR_ADDR(phy_id) | MIIAR_REG(reg);
> +
> +   nb8800_writel(priv, NB8800_MDIO_CMD, val);
> +   udelay(10);

Why 10? Perhaps add a comment line.

> +   nb8800_writel(priv, NB8800_MDIO_CMD, val | MDIO_CMD_GO);
> +
> +   if (!nb8800_mdio_wait(bus))
> +   return -ETIMEDOUT;
> +
> +   val = nb8800_readl(priv, NB8800_MDIO_STS);
> +   if (val & MDIO_STS_ERR)
> +   return 0x;

Can we return an error here?

> +
> +   return val & 0x;
> +}
> +
> +static int nb8800_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 
> val)
> +{
> +   struct nb8800_priv *priv = bus->priv;
> +   int tmp;
> +
> +   if (!nb8800_mdio_wait(bus))
> +   return -ETIMEDOUT;
> +
> +   tmp = MIIAR_DATA(val) | MIIAR_ADDR(phy_id) | MIIAR_REG(reg) |
> +   MDIO_CMD_WR;
> +
> +   nb8800_writel(priv, NB8800_MDIO_CMD, tmp);
> +   udelay(10);
> +   nb8800_writel(priv, NB8800_MDIO_CMD, tmp | MDIO_CMD_GO);
> +
> +   if (!nb8800_mdio_wait(bus))
> +   return -ETIMEDOUT;
> +
> +   return 0;
> +}
> +
> +static void nb8800_mac_tx(struct net_device *dev, bool enable)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +
> +   while (nb8800_readl(priv, NB8800_TXC_CR) & TCR_EN)
> +   cpu_relax();
> +
> +   if (enable)
> +   nb8800_set_bits(b, priv, NB8800_TX_CTL1, TX_EN);
> +   else
> +   nb8800_clear_bits(b, priv, NB8800_TX_CTL1, TX_EN);
> +}
> +
> +static void nb8800_mac_rx(struct net_device *dev, bool enable)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +
> +   if (enable)
> +   nb8800_set_bits(b, priv, NB8800_RX_CTL, RX_EN);
> +   else
> +   nb8800_clear_bits(b, priv, NB8800_RX_CTL, RX_EN);
> +}
> +
> +static void nb8800_mac_af(struct net_device *dev, bool enable)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +
> +   if (enable)
> +   nb8800_set_bits(b, priv, NB8800_RX_CTL, RX_AF_EN);
> +   else
> +   nb8800_clear_bits(b, priv, NB8800_RX_CTL, RX_AF_EN);
> +}
> +
> +static void nb8800_stop_rx(struct net_device *dev)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +   int i;
> +
> +   for (i = 0; i < RX_DESC_COUNT; i++)
> +   priv->rx_descs[i].config |= DESC_EOC;
> +
> +   while (nb8800_readl(priv, NB8800_RXC_CR) & RCR_EN)
> +   usleep_range(1000, 1);

Here is a busy loop without timeout limitation.

> +}
> +
> +static void nb8800_start_rx(struct net_device *dev)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +
> +   nb8800_set_bits(l, priv, NB8800_RXC_CR, RCR_EN);
> +}
> +
> +static int nb8800_alloc_rx(struct net_device *dev, int i, bool napi)
> +{
> +   struct nb8800_priv *priv = netdev_priv(dev);
> +   struct nb8800_dma_desc *rx = >rx_descs[i];
> +   struct rx_buf *buf = >rx_bufs[i];
> +   int size = L1_CACHE_ALIGN(RX_BUF_SIZE);
> +   void *data;
> +
> +   data = napi ? napi_alloc_frag(size) : netdev_alloc_frag(size);
> +   if (!data) {
> +

Re: [PATCH net-next V19 0/3] openvswitch: Add support for 802.1ad

2015-10-31 Thread Pravin Shelar 
On Fri, Oct 30, 2015 at 10:08 AM, Thomas F Herbert
 wrote:
> V19: Rebased to latest net-next 4.3.0-rc6+
> commit 6d08f617872cc048173d59f1ce4660b030bab5a6
> plus a few reviewer's suggested changes.
>
> Note: changes to netdev-vport.c are not included. dev_queue_xmit() is
> now called directly as the send vport ops function and length
> adjustments for vlans don't apply. I have concerns whether this is
> correct but haven't review all the vport code. Reviewer's comments
> are welcome.
>
dev_queue_xmit is effectively called from ovs_vport_send. vlan len
check can be done in this ovs_vport_send() function.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Al Viro 
On Fri, Oct 30, 2015 at 04:52:41PM -0700, Linus Torvalds wrote:
> I really suspect this patch is "good enough" in reality, and I would
> *much* rather do something like this than add a new non-POSIX flag
> that people have to update their binaries for. I agree with Eric that
> *some* people will do so, but it's still the wrong thing to do. Let's
> just make performance with the normal semantics be good enough that we
> don't need to play odd special games.
> 
> Eric?

... and here's the current variant of mine.  FWIW, it seems to survive
LTP and xfstests + obvious "let's torture the allocator".  On the
"million dups" test it seems to be about 25% faster than the one Linus
had posted, at ten millions - about 80%.  On opensock results seem to be
about 20% better than with the variant Linus has posted, but I'm not sure
if the testbox is anywhere near the expected, so I'd appreciate if you'd
given it a spin on your setups.

It obviously needs saner comments, tuning, etc.  BTW, another obvious
low-hanging fruit with this data structure is count_open_files() (and
that goes for 1:64 bitmap Linus uses) - dup2(0, 1000); close(1000);
fork(); and count_open_files() is chewing through the damn bitmap from
about 16M down to low tens.  While holding ->files_lock, at that...
I'm not saying it's critical, and it's definitely a followup patch fodder
in either approach, but it's easy enough to do.

diff --git a/fs/file.c b/fs/file.c
index 6c672ad..fa43cbe 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -30,6 +30,8 @@ int sysctl_nr_open_min = BITS_PER_LONG;
 int sysctl_nr_open_max = __const_max(INT_MAX, ~(size_t)0/sizeof(void *)) &
 -BITS_PER_LONG;
 
+#define BITS_PER_CHUNK 512
+
 static void *alloc_fdmem(size_t size)
 {
/*
@@ -46,8 +48,10 @@ static void *alloc_fdmem(size_t size)
 
 static void __free_fdtable(struct fdtable *fdt)
 {
+   int i;
kvfree(fdt->fd);
-   kvfree(fdt->open_fds);
+   for (i = 0; i <= 3; i++)
+   kvfree(fdt->bitmaps[i]);
kfree(fdt);
 }
 
@@ -62,7 +66,7 @@ static void free_fdtable_rcu(struct rcu_head *rcu)
  */
 static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt)
 {
-   unsigned int cpy, set;
+   unsigned int cpy, set, to, from, level, n;
 
BUG_ON(nfdt->max_fds < ofdt->max_fds);
 
@@ -71,18 +75,53 @@ static void copy_fdtable(struct fdtable *nfdt, struct 
fdtable *ofdt)
memcpy(nfdt->fd, ofdt->fd, cpy);
memset((char *)(nfdt->fd) + cpy, 0, set);
 
-   cpy = ofdt->max_fds / BITS_PER_BYTE;
-   set = (nfdt->max_fds - ofdt->max_fds) / BITS_PER_BYTE;
-   memcpy(nfdt->open_fds, ofdt->open_fds, cpy);
-   memset((char *)(nfdt->open_fds) + cpy, 0, set);
+   cpy = ofdt->max_fds / 8;
+   set = (nfdt->max_fds - ofdt->max_fds) / 8;
memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy);
memset((char *)(nfdt->close_on_exec) + cpy, 0, set);
+   if (likely(!nfdt->bitmaps[1])) {
+   // flat to flat
+   memcpy(nfdt->bitmaps[0], ofdt->bitmaps[0], cpy);
+   memset((char *)(nfdt->bitmaps[0]) + cpy, 0, set);
+   return;
+   }
+   to = round_up(nfdt->max_fds, BITS_PER_CHUNK);
+   set = (to - ofdt->max_fds) / 8;
+   // copy and pad the primary
+   memcpy(nfdt->bitmaps[0], ofdt->bitmaps[0], ofdt->max_fds / 8);
+   memset((char *)nfdt->bitmaps[0] + ofdt->max_fds / 8, 0, set);
+   // copy and pad the old secondaries
+   from = round_up(ofdt->max_fds, BITS_PER_CHUNK);
+   for (level = 1; level <= 3; level++) {
+   if (!ofdt->bitmaps[level])
+   break;
+   to = round_up(to / BITS_PER_CHUNK, BITS_PER_CHUNK);
+   from = round_up(from / BITS_PER_CHUNK, BITS_PER_CHUNK);
+   memcpy(nfdt->bitmaps[level], ofdt->bitmaps[level], from / 8);
+   memset((char *)nfdt->bitmaps[level] + from / 8, 0, (to - from) 
/ 8);
+   }
+   // zero the new ones (if any)
+   for (n = level; n <= 3; n++) {
+   if (!nfdt->bitmaps[n])
+   break;
+   to = round_up(to / BITS_PER_CHUNK, BITS_PER_CHUNK);
+   memset(nfdt->bitmaps[n], 0, to / 8);
+   }
+   // and maybe adjust bit 0 in the first new one.
+   if (unlikely(n != level)) {
+   unsigned long *p = nfdt->bitmaps[level - 1];
+   for (n = 0; n < BITS_PER_CHUNK / BITS_PER_LONG; n++)
+   if (~p[n])
+   return;
+   __set_bit(0, nfdt->bitmaps[level]);
+   }
 }
 
 static struct fdtable * alloc_fdtable(unsigned int nr)
 {
struct fdtable *fdt;
void *data;
+   int level = 0;
 
/*
 * Figure out how many fds we actually want to support in this fdtable.
@@ -114,16 +153,28 @@ static struct fdtable * alloc_fdtable(unsigned int nr)
goto out_fdt;
fdt->fd = data;
 
+   if (nr >

[PATCH next 2/3] bonding: unify all places where actor-oper key needs to be updated.

2015-10-31 Thread Mahesh Bandewar 
actor_admin, and actor_oper key is changed at multiple locations in
the code. This patch brings all those updates into one location in
an attempt to avoid possible inconsistent updates causing LACP state
machine to go in weird state.

The unified place is ad_update_actor_key() with simple state-machine
logic -
  (a) If port is "duplex" then only it can participate in LACP
  (b) Speed change reinitializes the LACP state-machine.

Signed-off-by: Mahesh Bandewar 
---
 drivers/net/bonding/bond_3ad.c | 87 +-
 1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 3a17fd207ec6..b9816b7f319f 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -127,6 +127,7 @@ static void ad_marker_info_received(struct bond_marker 
*marker_info,
struct port *port);
 static void ad_marker_response_received(struct bond_marker *marker,
struct port *port);
+static void ad_update_actor_keys(struct port *port, bool reset);
 
 
 /* = api to bonding and kernel code == */
@@ -1951,14 +1952,7 @@ void bond_3ad_bind_slave(struct slave *slave)
 * user key
 */
port->actor_admin_port_key = bond->params.ad_user_port_key << 6;
-   port->actor_admin_port_key |= __get_duplex(port);
-   port->actor_admin_port_key |= (__get_link_speed(port) << 1);
-   port->actor_oper_port_key = port->actor_admin_port_key;
-   /* if the port is not full duplex, then the port should be not
-* lacp Enabled
-*/
-   if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS))
-   port->sm_vars &= ~AD_PORT_LACP_ENABLED;
+   ad_update_actor_keys(port, false);
/* actor system is the bond's system */
port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
port->actor_system_priority =
@@ -2308,6 +2302,52 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, 
struct slave *slave,
 }
 
 /**
+ * ad_update_actor_keys - Update the oper / admin keys for a port based on
+ * its current speed and duplex settings.
+ *
+ * @port: the port we'are looking at
+ * @reset: Boolean to just reset the speed and the duplex part of the key
+ *
+ * The logic to change the oper / admin keys is:
+ * (a) A full duplex port can participate in LACP with partner.
+ * (b) When the speed is changed, LACP need to be reinitiated.
+ */
+static void ad_update_actor_keys(struct port *port, bool reset)
+{
+   u8 duplex = 0;
+   u16 ospeed = 0, speed = 0;
+   u16 old_oper_key = port->actor_oper_port_key;
+
+   port->actor_admin_port_key &= ~(AD_SPEED_KEY_MASKS|AD_DUPLEX_KEY_MASKS);
+   if (!reset) {
+   speed = __get_link_speed(port);
+   ospeed = (old_oper_key & AD_SPEED_KEY_MASKS) >> 1;
+   duplex = __get_duplex(port);
+   port->actor_admin_port_key |= (speed << 1) | duplex;
+   }
+   port->actor_oper_port_key = port->actor_admin_port_key;
+
+   if (old_oper_key != port->actor_oper_port_key) {
+   /* Only 'duplex' port participates in LACP */
+   if (duplex)
+   port->sm_vars |= AD_PORT_LACP_ENABLED;
+   else
+   port->sm_vars &= ~AD_PORT_LACP_ENABLED;
+
+   if (!reset) {
+   if (!speed) {
+   netdev_err(port->slave->dev,
+  "speed changed to 0 for port %s",
+  port->slave->dev->name);
+   } else if (duplex && ospeed != speed) {
+   /* Speed change restarts LACP state-machine */
+   port->sm_vars |= AD_PORT_BEGIN;
+   }
+   }
+   }
+}
+
+/**
  * bond_3ad_adapter_speed_changed - handle a slave's speed change indication
  * @slave: slave struct to work on
  *
@@ -2328,14 +2368,8 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 
spin_lock_bh(>bond->mode_lock);
 
-   port->actor_admin_port_key &= ~AD_SPEED_KEY_MASKS;
-   port->actor_admin_port_key |= __get_link_speed(port) << 1;
-   port->actor_oper_port_key = port->actor_admin_port_key;
+   ad_update_actor_keys(port, false);
netdev_dbg(slave->bond->dev, "Port %d changed speed\n", 
port->actor_port_number);
-   /* there is no need to reselect a new aggregator, just signal the
-* state machines to reinitialize
-*/
-   port->sm_vars |= AD_PORT_BEGIN;
 
spin_unlock_bh(>bond->mode_lock);
 }
@@ -2361,17 +2395,9 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)

[PATCH next 3/3] bonding: simplify / unify event handling code for 3ad mode.

2015-10-31 Thread Mahesh Bandewar 
Old logic of updating state-machine is not required since
ad_update_actor_keys() does it implicitly. The only loss is
the notification differentiation between speed vs. duplex
change. Now only one unified notification is printed.

Signed-off-by: Mahesh Bandewar 
---
 drivers/net/bonding/bond_3ad.c  | 38 ++
 drivers/net/bonding/bond_main.c | 14 ++
 include/net/bond_3ad.h  |  3 +--
 3 files changed, 9 insertions(+), 46 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index b9816b7f319f..940e2ebbdea8 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2348,39 +2348,14 @@ static void ad_update_actor_keys(struct port *port, 
bool reset)
 }
 
 /**
- * bond_3ad_adapter_speed_changed - handle a slave's speed change indication
- * @slave: slave struct to work on
+ * bond_3ad_adapter_speed_duplex_changed - handle a slave's speed / duplex
+ * change indication
  *
- * Handle reselection of aggregator (if needed) for this port.
- */
-void bond_3ad_adapter_speed_changed(struct slave *slave)
-{
-   struct port *port;
-
-   port = &(SLAVE_AD_INFO(slave)->port);
-
-   /* if slave is null, the whole port is not initialized */
-   if (!port->slave) {
-   netdev_warn(slave->bond->dev, "speed changed for uninitialized 
port on %s\n",
-   slave->dev->name);
-   return;
-   }
-
-   spin_lock_bh(>bond->mode_lock);
-
-   ad_update_actor_keys(port, false);
-   netdev_dbg(slave->bond->dev, "Port %d changed speed\n", 
port->actor_port_number);
-
-   spin_unlock_bh(>bond->mode_lock);
-}
-
-/**
- * bond_3ad_adapter_duplex_changed - handle a slave's duplex change indication
  * @slave: slave struct to work on
  *
  * Handle reselection of aggregator (if needed) for this port.
  */
-void bond_3ad_adapter_duplex_changed(struct slave *slave)
+void bond_3ad_adapter_speed_duplex_changed(struct slave *slave)
 {
struct port *port;
 
@@ -2388,17 +2363,16 @@ void bond_3ad_adapter_duplex_changed(struct slave 
*slave)
 
/* if slave is null, the whole port is not initialized */
if (!port->slave) {
-   netdev_warn(slave->bond->dev, "duplex changed for uninitialized 
port on %s\n",
+   netdev_warn(slave->bond->dev,
+   "speed/duplex changed for uninitialized port %s\n",
slave->dev->name);
return;
}
 
spin_lock_bh(>bond->mode_lock);
-
ad_update_actor_keys(port, false);
-   netdev_dbg(slave->bond->dev, "Port %d slave %s changed duplex\n",
+   netdev_dbg(slave->bond->dev, "Port %d slave %s changed speed/duplex\n",
   port->actor_port_number, slave->dev->name);
-
spin_unlock_bh(>bond->mode_lock);
 }
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d0f23cd6e236..b4351caf8e01 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2943,8 +2943,6 @@ static int bond_slave_netdev_event(unsigned long event,
struct slave *slave = bond_slave_get_rtnl(slave_dev), *primary;
struct bonding *bond;
struct net_device *bond_dev;
-   u32 old_speed;
-   u8 old_duplex;
 
/* A netdev event can be generated while enslaving a device
 * before netdev_rx_handler_register is called in which case
@@ -2965,17 +2963,9 @@ static int bond_slave_netdev_event(unsigned long event,
break;
case NETDEV_UP:
case NETDEV_CHANGE:
-   old_speed = slave->speed;
-   old_duplex = slave->duplex;
-
bond_update_speed_duplex(slave);
-
-   if (BOND_MODE(bond) == BOND_MODE_8023AD) {
-   if (old_speed != slave->speed)
-   bond_3ad_adapter_speed_changed(slave);
-   if (old_duplex != slave->duplex)
-   bond_3ad_adapter_duplex_changed(slave);
-   }
+   if (BOND_MODE(bond) == BOND_MODE_8023AD)
+   bond_3ad_adapter_speed_duplex_changed(slave);
/* Fallthrough */
case NETDEV_DOWN:
/* Refresh slave-array if applicable!
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index c2a40a172fcd..f1fbc3b11962 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -297,8 +297,7 @@ void bond_3ad_bind_slave(struct slave *slave);
 void bond_3ad_unbind_slave(struct slave *slave);
 void bond_3ad_state_machine_handler(struct work_struct *);
 void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout);
-void bond_3ad_adapter_speed_changed(struct slave *slave);
-void bond_3ad_adapter_duplex_changed(struct slave *slave);
+void bond_3ad_adapter_speed_duplex_changed(struct slave *slave);
 void

[PATCH next 0/3] re-org actor admin/oper key updates

2015-10-31 Thread Mahesh Bandewar 
I was observing machines entering into weird LACP state when the
partner is in passive mode. This issue is not because of the partners
in passive state but probably because of some operational key update
which is pushing the state-machine is that weird state. This was
happening randomly on about 1% of the machine (when the sample size
is a large set of machines with a variety of NICs/ports bonded).

In this patch-series I'm attempting to unify the logic of actor-key
/ operational-key changes to one place to avoid possible errors in
update. Also this eliminates the need for the event-handler to decide
if the key needs update.

After this patch-set none of the machines (from same sample set) were
exhibiting LACP-weirdness that was observed earlier.

Mahesh Bandewar (3):
  bonding: Simplify __get_duplex function.
  bonding: unify all places where actor-oper key needs to be updated.
  bonding: simplify / unify event handling code for 3ad mode.

 drivers/net/bonding/bond_3ad.c  | 113 ++--
 drivers/net/bonding/bond_main.c |  14 +
 include/net/bond_3ad.h  |   3 +-
 3 files changed, 54 insertions(+), 76 deletions(-)

-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH next 1/3] bonding: Simplify __get_duplex function.

2015-10-31 Thread Mahesh Bandewar 
Eliminate 'else' clause by simply initializing variable

Signed-off-by: Mahesh Bandewar 
---
 drivers/net/bonding/bond_3ad.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 3c45358844eb..3a17fd207ec6 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -327,14 +327,12 @@ static u16 __get_link_speed(struct port *port)
 static u8 __get_duplex(struct port *port)
 {
struct slave *slave = port->slave;
-   u8 retval;
+   u8 retval = 0x0;
 
/* handling a special case: when the configuration starts with
 * link down, it sets the duplex to 0.
 */
-   if (slave->link != BOND_LINK_UP) {
-   retval = 0x0;
-   } else {
+   if (slave->link == BOND_LINK_UP) {
switch (slave->duplex) {
case DUPLEX_FULL:
retval = 0x1;
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Al Viro 
On Sat, Oct 31, 2015 at 12:54:50PM -0700, Linus Torvalds wrote:
> On Sat, Oct 31, 2015 at 12:34 PM, Al Viro  wrote:
> >
> > ... and here's the current variant of mine.
> 
> Ugh. I really liked how simple mine ended up being. Yours is definitely not.

Note that it's not the final variant - just something that should be
testable.  There are all kinds of things that still need cleaning/simplifying
in there - e.g. scan() is definitely more complex than needed (if nothing
else, the "small bitmap" case is simply find_next_zero_bit(), and the
rest all have size equal to full cacheline; moreover, I'd overdone the
"... and check if there are other zero bits left" thing - its callers
used to use that a lot, and with the execption of two of them it was
absolutely worthless.  So it ended up more generic than necessary and
I'm going to trim that crap down.

It's still going to end up more complex than yours, obviously, but not as
badly as it is now.  I'm not sure if the speedup will be worth the
extra complexity, thus asking for testing...  On _some_ loads it is
considerably faster (at least by factor of 5), but whether those loads
resemble anything that occurs on real systems...

BTW, considerable amount of unpleasantness is due to ragged-right-end kind
of problems - take /proc/sys/fs/nr-open to something other than a power of
two and a whole lot of fun issues start happening.  I went for "if there
are secondary bitmaps at all, pad all bitmaps to a multiple of cacheline",
which at least somewhat mitigates that mess; hell knows, there might be
a clever way to sidestep it entirely...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Måns Rullgård 
Andy Shevchenko  writes:

 +   nb8800_writel(priv, NB8800_MDIO_CMD, val | MDIO_CMD_GO);
 +
 +   if (!nb8800_mdio_wait(bus))
 +   return -ETIMEDOUT;
 +
 +   val = nb8800_readl(priv, NB8800_MDIO_STS);
 +   if (val & MDIO_STS_ERR)
 +   return 0x;
>>>
>>> Can we return an error here?
>>
>> That breaks the bus scanning in phylib.
>
> You mean this is non-fatal error?

It's what you get if nothing responded to the address.

 +
 +   rx_buf = >rx_bufs[next];
 +   rx = >rx_descs[next];
>>>
 +   report = rx->report;
>>>
>>> Maybe you can use rx->report directly below.
>>
>> It's in uncached memory, so didn't want to have gcc accidentally doing
>> more reads than necessary.
>
> How it would not be possible without ACCESS_ONCE() or similar?

True, it probably doesn't make a difference.  Sometimes copying a value
to a local variable prevents re-reads due to potential pointer aliasing,
but I don't think that's an issue here.

 +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
 +{
 +   struct nb8800_priv *priv = netdev_priv(dev);
 +   struct tx_skb_data *skb_data;
 +   struct tx_buf *tx_buf;
 +   dma_addr_t dma_addr;
 +   unsigned int dma_len;
 +   int cpsz, next;
 +   int frags;
 +
 +   if (atomic_read(>tx_free) <= NB8800_DESC_LOW) {
 +   netif_stop_queue(dev);
 +   return NETDEV_TX_BUSY;
 +   }
 +
 +   cpsz = (8 - (uintptr_t)skb->data) & 7;
>>>
>>> So, cast to uintptr_t looks strange in this driver, since used only
>>> twice in such expression, why not to use plain unsigned int * ?
>>
>> Because it's not the same thing.  uintptr_t is an integer type the same
>> size as a pointer.  I need to check if the data pointer is 8-byte
>> aligned as required by the DMA.
>
> Yes, I understand why you're doing this. But since you use lowest
> bits, I think result will be the same even without casting.

You can't do that kind of arithmetic on pointer values.  They have to be
cast to an integer type first.

 +   nb8800_writeb(priv, NB8800_RANDOM_SEED, 0x08);
 +
 +   /* TX single deferral params */
 +   nb8800_writeb(priv, NB8800_TX_SDP, 0xc);
 +
 +   /* Threshold for partial full */
 +   nb8800_writeb(priv, NB8800_PF_THRESHOLD, 0xff);
 +
 +   /* Pause Quanta */
 +   nb8800_writeb(priv, NB8800_PQ1, 0xff);
 +   nb8800_writeb(priv, NB8800_PQ2, 0xff);
>>>
>>> Lot of magic numbers above and below.
>>
>> Those are from the original driver.  Some of them disagree with the
>> documentation, and the "correct" values don't work.
>
> It would be nice to somehow describe them if possible.

I'll see what I can do.

-- 
Måns Rullgård
m...@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Andy Shevchenko 
On Sat, Oct 31, 2015 at 8:41 PM, Måns Rullgård  wrote:
> Andy Shevchenko  writes:
>
>>> +static int nb8800_mdio_read(struct mii_bus *bus, int phy_id, int reg)
>>> +{
>>> +   struct nb8800_priv *priv = bus->priv;
>>> +   int val;
>>> +
>>> +   if (!nb8800_mdio_wait(bus))
>>> +   return -ETIMEDOUT;
>>> +
>>> +   val = MIIAR_ADDR(phy_id) | MIIAR_REG(reg);
>>> +
>>> +   nb8800_writel(priv, NB8800_MDIO_CMD, val);
>>> +   udelay(10);
>>
>> Why 10? Perhaps add a comment line.
>
> Because it works.  The documentation doesn't mention a delay, but it
> works unreliably without one.

/* Put delay here, otherwise it works unreliably */

>
>>> +   nb8800_writel(priv, NB8800_MDIO_CMD, val | MDIO_CMD_GO);
>>> +
>>> +   if (!nb8800_mdio_wait(bus))
>>> +   return -ETIMEDOUT;
>>> +
>>> +   val = nb8800_readl(priv, NB8800_MDIO_STS);
>>> +   if (val & MDIO_STS_ERR)
>>> +   return 0x;
>>
>> Can we return an error here?
>
> That breaks the bus scanning in phylib.

You mean this is non-fatal error?

>>> +static int nb8800_poll(struct napi_struct *napi, int budget)
>>> +{
>>> +   struct net_device *dev = napi->dev;
>>> +   struct nb8800_priv *priv = netdev_priv(dev);
>>> +   struct nb8800_dma_desc *rx;
>>> +   int work = 0;
>>> +   int last = priv->rx_eoc;
>>> +   int next;
>>> +
>>> +   while (work < budget) {
>>> +   struct rx_buf *rx_buf;
>>> +   u32 report;
>>> +   int len;
>>> +
>>> +   next = (last + 1) & (RX_DESC_COUNT - 1);
>>
>> Maybe (last + 1) % RX_DESC_COUNT ? It will not prevent to use
>> non-power-of-two numbers.
>
> We don't want to be doing divisions anyway, but I can certainly change
> it to % if that's preferred.

I'm pretty sure the result for power-of-two numbers will be the
similar (right shift).

>
>>> +
>>> +   rx_buf = >rx_bufs[next];
>>> +   rx = >rx_descs[next];
>>
>>> +   report = rx->report;
>>
>> Maybe you can use rx->report directly below.
>
> It's in uncached memory, so didn't want to have gcc accidentally doing
> more reads than necessary.

How it would not be possible without ACCESS_ONCE() or similar?

>
>>> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
>>> +{
>>> +   struct nb8800_priv *priv = netdev_priv(dev);
>>> +   struct tx_skb_data *skb_data;
>>> +   struct tx_buf *tx_buf;
>>> +   dma_addr_t dma_addr;
>>> +   unsigned int dma_len;
>>> +   int cpsz, next;
>>> +   int frags;
>>> +
>>> +   if (atomic_read(>tx_free) <= NB8800_DESC_LOW) {
>>> +   netif_stop_queue(dev);
>>> +   return NETDEV_TX_BUSY;
>>> +   }
>>> +
>>> +   cpsz = (8 - (uintptr_t)skb->data) & 7;
>>
>> So, cast to uintptr_t looks strange in this driver, since used only
>> twice in such expression, why not to use plain unsigned int * ?
>
> Because it's not the same thing.  uintptr_t is an integer type the same
> size as a pointer.  I need to check if the data pointer is 8-byte
> aligned as required by the DMA.

Yes, I understand why you're doing this. But since you use lowest
bits, I think result will be the same even without casting.

>>> +   nb8800_writeb(priv, NB8800_RANDOM_SEED, 0x08);
>>> +
>>> +   /* TX single deferral params */
>>> +   nb8800_writeb(priv, NB8800_TX_SDP, 0xc);
>>> +
>>> +   /* Threshold for partial full */
>>> +   nb8800_writeb(priv, NB8800_PF_THRESHOLD, 0xff);
>>> +
>>> +   /* Pause Quanta */
>>> +   nb8800_writeb(priv, NB8800_PQ1, 0xff);
>>> +   nb8800_writeb(priv, NB8800_PQ2, 0xff);
>>
>> Lot of magic numbers above and below.
>
> Those are from the original driver.  Some of them disagree with the
> documentation, and the "correct" values don't work.

It would be nice to somehow describe them if possible.

>>> +static int nb8800_probe(struct platform_device *pdev)
>>> +{
>>> +   const struct of_device_id *match;
>>> +   const struct nb8800_ops *ops = NULL;
>>> +   struct nb8800_priv *priv;
>>> +   struct resource *res;
>>> +   struct net_device *dev;
>>> +   struct mii_bus *bus;
>>> +   const unsigned char *mac;
>>> +   void __iomem *base;
>>> +   int irq;
>>> +   int ret;
>>> +
>>> +   match = of_match_device(nb8800_dt_ids, >dev);
>>> +   if (match)
>>> +   ops = match->data;
>>> +
>>> +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +   if (!res) {
>>> +   dev_err(>dev, "No MMIO base\n");
>>> +   return -EINVAL;
>>> +   }
>>
>> Move platform_get_resource() just before devm_ioremap_resource() and
>> remove redundant condition with the message.
>>
>>> +   bus = devm_mdiobus_alloc(>dev);
>>> +   if (!bus) {
>>> +   ret = -ENOMEM;
>>> +   goto err_disable_clk;
>>> +   }
>>> +
>>> +   bus->name = "nb8800-mii";
>>> +

Re: [PATCH] bpf: convert hashtab lock to raw lock

2015-10-31 Thread Daniel Borkmann 

On 10/31/2015 02:47 PM, Steven Rostedt wrote:

On Fri, 30 Oct 2015 17:03:58 -0700
Alexei Starovoitov  wrote:

On Fri, Oct 30, 2015 at 03:16:26PM -0700, Yang Shi wrote:

When running bpf samples on rt kernel, it reports the below warning:

BUG: sleeping function called from invalid context at 
kernel/locking/rtmutex.c:917
in_atomic(): 1, irqs_disabled(): 128, pid: 477, name: ping
Preemption disabled at:[] kprobe_perf_func+0x30/0x228

...

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 83c209d..972b76b 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -17,7 +17,7 @@
  struct bpf_htab {
struct bpf_map map;
struct hlist_head *buckets;
-   spinlock_t lock;
+   raw_spinlock_t lock;


How do we address such things in general?
I bet there are tons of places around the kernel that
call spin_lock from atomic.
I'd hate to lose the benefits of lockdep of non-raw spin_lock
just to make rt happy.


You wont lose any benefits of lockdep. Lockdep still checks
raw_spin_lock(). The only difference between raw_spin_lock and
spin_lock is that in -rt spin_lock turns into an rt_mutex() and
raw_spin_lock stays a spin lock.


( Btw, Yang, would have been nice if your commit description would have
  already included such info, not only that you convert it, but also why
  it's okay to do so. )


The error is that in -rt, you called a mutex and not a spin lock while
atomic.


You are right, I think this happens due to the preempt_disable() in the
trace_call_bpf() handler. So, I think the patch seems okay. The dep_map
is btw union'ed in the struct spinlock case to the same offset of the
dep_map from raw_spinlock.

It's a bit inconvenient, though, when we add other library code as maps
in future, f.e. things like rhashtable as they would first need to be
converted to raw_spinlock_t as well, but judging from the git log, it
looks like common practice.

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Linus Torvalds 
On Sat, Oct 31, 2015 at 12:34 PM, Al Viro  wrote:
>
> ... and here's the current variant of mine.

Ugh. I really liked how simple mine ended up being. Yours is definitely not.

And based on the profiles from Eric, finding the fd is no longer the
problem even with my simpler patch. The problem ends up being the
contention on the file_lock spinlock.

Eric, I assume that's not "expand_fdtable", since your test-program
seems to expand the fd array at the beginning. So it's presumably all
from the __alloc_fd() use, but we should double-check.. Eric, can you
do a callgraph profile and see which caller is the hottest?

 Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

We have a parcel right here in our office to be delivered to you.

2015-10-31 Thread ''COURIER'' 
Kindly Reply Back For More Details.
FedEx Express Courier.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Eric Dumazet 
On Sat, 2015-10-31 at 12:54 -0700, Linus Torvalds wrote:
> On Sat, Oct 31, 2015 at 12:34 PM, Al Viro  wrote:
> >
> > ... and here's the current variant of mine.
> 
> Ugh. I really liked how simple mine ended up being. Yours is definitely not.
> 
> And based on the profiles from Eric, finding the fd is no longer the
> problem even with my simpler patch. The problem ends up being the
> contention on the file_lock spinlock.
> 
> Eric, I assume that's not "expand_fdtable", since your test-program
> seems to expand the fd array at the beginning. So it's presumably all
> from the __alloc_fd() use, but we should double-check.. Eric, can you
> do a callgraph profile and see which caller is the hottest?

Sure : profile taken while test runs using 16 threads (Since this is
probably not a too biased micro benchmark...)

# hostname : lpaa24
# os release : 4.3.0-smp-DEV
# perf version : 3.12.0-6-GOOGLE
# arch : x86_64
# nrcpus online : 48
# nrcpus avail : 48
# cpudesc : Intel(R) Xeon(R) CPU E5-2696 v2 @ 2.50GHz
# cpuid : GenuineIntel,6,62,4
# total memory : 264126320 kB
# cmdline : /usr/bin/perf record -a -g sleep 4 
# event : name = cycles, type = 0, config = 0x0, config1 = 0x0, config2 = 0x0, 
excl_usr = 0, excl_kern = 0, excl_host = 0, excl_guest = 1, precise_ip = 0, att
# CPU_TOPOLOGY info available, use -I to display
# NUMA_TOPOLOGY info available, use -I to display
# pmu mappings: cpu = 4, msr = 38, uncore_cbox_10 = 35, uncore_cbox_11 = 36, 
software = 1, power = 7, uncore_irp = 8, uncore_pcu = 37, tracepoint = 2, 
uncore_
# Samples: 260K of event 'cycles'
# Event count (approx.): 196742182232
#
# OverheadCommandShared Object  
  
#   .  ...  
..
#
67.15%   opensock  opensock [.] memset  
  
 |
 --- memset

13.84%   opensock  [kernel.kallsyms][k] queued_spin_lock_slowpath   
  
 |
 --- queued_spin_lock_slowpath
|  
|--99.97%-- _raw_spin_lock
|  |  
|  |--53.03%-- __close_fd
|  |  sys_close
|  |  entry_SYSCALL_64_fastpath
|  |  __libc_close
|  |  |  
|  |   --100.00%-- 0x0
|  |  
|  |--46.83%-- __alloc_fd
|  |  get_unused_fd_flags
|  |  sock_map_fd
|  |  sys_socket
|  |  entry_SYSCALL_64_fastpath
|  |  __socket
|  |  |  
|  |   --100.00%-- 0x0
|   --0.13%-- [...]
 --0.03%-- [...]

 1.84%   opensock  [kernel.kallsyms][k] _find_next_bit.part.0   
  
 |
 --- _find_next_bit.part.0
|  
|--65.97%-- find_next_zero_bit
|  __alloc_fd
|  get_unused_fd_flags
|  sock_map_fd
|  sys_socket
|  entry_SYSCALL_64_fastpath
|  __socket
|  
|--34.01%-- __alloc_fd
|  get_unused_fd_flags
|  sock_map_fd
|  sys_socket
|  entry_SYSCALL_64_fastpath
|  __socket
|  |  
|   --100.00%-- 0x0
 --0.02%-- [...]

 1.59%   opensock  [kernel.kallsyms][k] _raw_spin_lock  
  
 |
 --- _raw_spin_lock
|  
|--28.78%-- get_unused_fd_flags
|  sock_map_fd
|  sys_socket
|  entry_SYSCALL_64_fastpath
|  __socket
|

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Linus Torvalds 
On Sat, Oct 31, 2015 at 1:45 PM, Eric Dumazet  wrote:
> 13.84%   opensock  [kernel.kallsyms][k] queued_spin_lock_slowpath
>  |
>  --- queued_spin_lock_slowpath
> |
> |--99.97%-- _raw_spin_lock
> |  |
> |  |--53.03%-- __close_fd
> |  |
> |  |--46.83%-- __alloc_fd

Interesting. "__close_fd" actually looks more expensive than
allocation. They presumably get called equally often, so it's probably
some cache effect.

__close_fd() doesn't do anything even remotely interesting as far as I
can tell, but it strikes me that we probably take a *lot* of cache
misses on the stupid "close-on-exec" flags, which are probably always
zero anyway.

Mind testing something really stupid, and making the __clear_bit() in
__clear_close_on_exec() conditiona, something like this:

 static inline void __clear_close_on_exec(int fd, struct fdtable *fdt)
 {
-   __clear_bit(fd, fdt->close_on_exec);
+   if (test_bit(fd, fdt->close_on_exec)
+   __clear_bit(fd, fdt->close_on_exec);
 }

and see if it makes a difference.

This is the kind of thing that a single-threaded (or even
single-socket) test will never actually show, because it caches well
enough. But for two sockets, I could imagine the unnecessary dirtying
of cachelines and ping-pong being noticeable.

The other stuff we probably can't do all that much about. Unless we
decide to go for some complicated lockless optimistic file descriptor
allocation scheme with retry-on-failure instead of locks. Which I'm
sure is possible, but I'm equally sure is painful.

Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Al Viro 
On Sat, Oct 31, 2015 at 02:23:31PM -0700, Linus Torvalds wrote:

> The other stuff we probably can't do all that much about. Unless we
> decide to go for some complicated lockless optimistic file descriptor
> allocation scheme with retry-on-failure instead of locks. Which I'm
> sure is possible, but I'm equally sure is painful.

The interesting part is dup2() - we'd have to do something like
serialize against other dup2
was_claimed = atomically set and test bit in bitmap
if was_claimed
tofree = fdt->fd[fd];
if (!tofree)
fail with EBUSY
install into ->fd[...]
end of critical area
in there; __alloc_fd() could be made retry-on-failure, but I don't see
how to cope with dup2 vs. dup2 without an explicit exclusion.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bpf: convert hashtab lock to raw lock

2015-10-31 Thread Steven Rostedt 
On Fri, 30 Oct 2015 17:03:58 -0700
Alexei Starovoitov  wrote:

> On Fri, Oct 30, 2015 at 03:16:26PM -0700, Yang Shi wrote:
> > When running bpf samples on rt kernel, it reports the below warning:
> > 
> > BUG: sleeping function called from invalid context at 
> > kernel/locking/rtmutex.c:917
> > in_atomic(): 1, irqs_disabled(): 128, pid: 477, name: ping
> > Preemption disabled at:[] kprobe_perf_func+0x30/0x228  
> ...
> > diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> > index 83c209d..972b76b 100644
> > --- a/kernel/bpf/hashtab.c
> > +++ b/kernel/bpf/hashtab.c
> > @@ -17,7 +17,7 @@
> >  struct bpf_htab {
> > struct bpf_map map;
> > struct hlist_head *buckets;
> > -   spinlock_t lock;
> > +   raw_spinlock_t lock;  
> 
> How do we address such things in general?
> I bet there are tons of places around the kernel that
> call spin_lock from atomic.
> I'd hate to lose the benefits of lockdep of non-raw spin_lock
> just to make rt happy.

You wont lose any benefits of lockdep. Lockdep still checks
raw_spin_lock(). The only difference between raw_spin_lock and
spin_lock is that in -rt spin_lock turns into an rt_mutex() and
raw_spin_lock stays a spin lock.

The error is that in -rt, you called a mutex and not a spin lock while
atomic.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next V19 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes

2015-10-31 Thread Pravin Shelar 
On Fri, Oct 30, 2015 at 10:08 AM, Thomas F Herbert
 wrote:
> Add support for 802.1ad including the ability to push and pop double
> tagged vlans. Add support for 802.1ad to netlink parsing and flow
> conversion. Uses double nested encap attributes to represent double
> tagged vlan. Inner TPID encoded along with ctci in nested attributes.
>
> Signed-off-by: Thomas F Herbert 
> ---
>  net/openvswitch/actions.c  |   6 +-
>  net/openvswitch/flow.c |  76 +
>  net/openvswitch/flow.h |   8 +-
>  net/openvswitch/flow_netlink.c | 235 
> +++--
>  4 files changed, 251 insertions(+), 74 deletions(-)
>
...
...
> +
> +static int __parse_vlan_from_nlattrs(const struct nlattr **nla,
> +struct sw_flow_match *match,
> +u64 *key_attrs, bool inner,
> +const struct nlattr **a, bool is_mask,
> +bool log)
> +{
**nla is unused argument in this function.

> +   int err;
> +   u64 v_attrs = *key_attrs;
> +
> +   err = encode_vlan_from_nlattrs(match, a, is_mask, inner, log);
> +   if (err)
> +   return err;
> +
> +   v_attrs &= ~(1 << OVS_KEY_ATTR_ENCAP);
> +
> +   /* Insure that tci key attribute isn't
> +* overwritten by encapsulated customer tci.
> +* Ethertype is cleared because it is c_tpid.
> +*/
> +   v_attrs &= ~(1 << OVS_KEY_ATTR_VLAN);
> +   v_attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE);
> +
> +   *key_attrs = v_attrs;
> +
> +   return 0;
> +}
> +
> +static int parse_vlan_from_nlattrs(const struct nlattr **nla,
> +  struct sw_flow_match *match,
> +  u64 *key_attrs, bool *ie_valid,
> +  const struct nlattr **a, bool is_mask,
> +  bool log)
> +{
encap passed by reference here, but caller does not make use of this,
So we can just pass pointer to encap.
> +   int err;
> +   const struct nlattr *encap;
> +   u64 v_attrs = 0;
> +
> +   if (!is_mask) {
> +   err = __parse_vlan_from_nlattrs(nla, match, key_attrs,
> +   false, a, is_mask, log);
> +   if (err)
> +   return err;
> +
> +   /* Another encap attribute here indicates
> +* the presence of a double tagged vlan.
> +*/
> +   encap = a[OVS_KEY_ATTR_ENCAP];
> +
> +   err = parse_flow_nlattrs(encap, a, _attrs, log);
> +   if (err)
> +   return err;
> +
> +   if ((v_attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) &&
> +   eth_type_vlan(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]))) {
> +   if (!((v_attrs & (1 << OVS_KEY_ATTR_VLAN)) &&
> + (v_attrs & (1 << OVS_KEY_ATTR_ENCAP {
> +   OVS_NLERR(log, "Invalid Inner VLAN frame");
> +   return -EINVAL;
> +   }
> +   *ie_valid = true;
> +   err = __parse_vlan_from_nlattrs(, match, 
> _attrs,
> +   true, a, is_mask, 
> log);

__parse_vlan_from_nlattrs() is not parsing inner encap nlattr in this
case. If we move above call to parse_flow_nlattrs() to
__parse_vlan_from_nlattrs() we can fix this issue.

> +   if (err)
> +   return err;
> +   *key_attrs |= v_attrs;
> +   }
> +   } else {
> +   err = __parse_vlan_from_nlattrs(nla, match, key_attrs,
> +   false, a, is_mask, log);
> +   if (err)
> +   return err;
> +
> +   encap = a[OVS_KEY_ATTR_ENCAP];
> +
> +   err = parse_flow_nlattrs(encap, a, _attrs, log);
> +   if (err)
> +   return err;
> +
> +   if (v_attrs & (1 << OVS_KEY_ATTR_ENCAP)) {
> +   if (!*ie_valid) {
> +   OVS_NLERR(log, "Encap mask attribute is set 
> for non-CVLAN frame.");
> +   return -EINVAL;
> +   }
> +   err = __parse_vlan_from_nlattrs(nla, match,
> +   _attrs, true, a,
> +   is_mask,
> +   log);
> +   if (err)
> +   return err;
> +   *key_attrs |= v_attrs;
> +   }
> +   }
> +   return 0;
> +}
> +
--
To unsubscribe from this list: send the line

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Eric Dumazet 
On Fri, 2015-10-30 at 16:52 -0700, Linus Torvalds wrote: sequential
allocations...
> 
> I don't think it would matter in real life, since I don't really think
> you have lots of fd's with strictly sequential behavior.
> 
> That said, the trivial "open lots of fds" benchmark would show it, so
> I guess we can just keep it. The next_fd logic is not expensive or
> complex, after all.

+1


> Attached is an updated patch that just uses the regular bitmap
> allocator and extends it to also have the bitmap of bitmaps. It
> actually simplifies the patch, so I guess it's better this way.
> 
> Anyway, I've tested it all a bit more, and for a trivial worst-case
> stress program that explicitly kills the next_fd logic by doing
> 
> for (i = 0; i < 100; i++) {
> close(3);
> dup2(0,3);
> if (dup(0))
> break;
> }
> 
> it takes it down from roughly 10s to 0.2s. So the patch is quite
> noticeable on that kind of pattern.
> 
> NOTE! You'll obviously need to increase your limits to actually be
> able to do the above with lots of file descriptors.
> 
> I ran Eric's test-program too, and find_next_zero_bit() dropped to a
> fraction of a percent. It's not entirely gone, but it's down in the
> noise.
> 
> I really suspect this patch is "good enough" in reality, and I would
> *much* rather do something like this than add a new non-POSIX flag
> that people have to update their binaries for. I agree with Eric that
> *some* people will do so, but it's still the wrong thing to do. Let's
> just make performance with the normal semantics be good enough that we
> don't need to play odd special games.
> 
> Eric?

I absolutely agree a generic solution is far better, especially when
its performance is in par.

Tested-by: Eric Dumazet 
Acked-by: Eric Dumazet 

Note that a non-POSIX flag (or a thread personality hints)
would still allow the kernel to do proper NUMA affinity placement : Say
the fd_array and bitmaps are split on the 2 nodes (or more, but most
servers nowadays have 2 sockets really).

Then at fd allocation time, we can prefer to pick an fd for which memory
holding various bits and the file pointer are in the local node

This speeds up subsequent fd system call on programs that constantly
blow away cpu caches, saving QPI transactions.

Thanks a lot Linus.

lpaa24:~# taskset ff0ff ./opensock -t 16 -n 1000 -l 10
count=1000 (check/increase ulimit -n)
total = 3992764

lpaa24:~# ./opensock -t 48 -n 1000 -l 10
count=1000 (check/increase ulimit -n)
total = 3545249

Profile with 16 threads :

69.55%  opensock  [.] memset   
11.83%  [kernel]  [k] queued_spin_lock_slowpath
 1.91%  [kernel]  [k] _find_next_bit.part.0
 1.68%  [kernel]  [k] _raw_spin_lock   
 0.99%  [kernel]  [k] kmem_cache_alloc 
 0.99%  [kernel]  [k] memset_erms  
 0.95%  [kernel]  [k] get_empty_filp   
 0.82%  [kernel]  [k] __close_fd   
 0.73%  [kernel]  [k] __alloc_fd   
 0.65%  [kernel]  [k] sk_alloc 
 0.63%  opensock  [.] child_function   
 0.56%  [kernel]  [k] fput 
 0.35%  [kernel]  [k] sock_alloc   
 0.31%  [kernel]  [k] kmem_cache_free  
 0.31%  [kernel]  [k] inode_init_always
 0.28%  [kernel]  [k] d_set_d_op   
 0.27%  [kernel]  [k] entry_SYSCALL_64_after_swapgs

Profile with 48 threads :

57.92%  [kernel]  [k] queued_spin_lock_slowpath
32.14%  opensock  [.] memset   
 0.81%  [kernel]  [k] _find_next_bit.part.0
 0.51%  [kernel]  [k] _raw_spin_lock   
 0.45%  [kernel]  [k] kmem_cache_alloc 
 0.38%  [kernel]  [k] kmem_cache_free  
 0.34%  [kernel]  [k] __close_fd   
 0.32%  [kernel]  [k] memset_erms  
 0.25%  [kernel]  [k] __alloc_fd   
 0.24%  [kernel]  [k] get_empty_filp   
 0.23%  opensock  [.] child_function   
 0.18%  [kernel]  [k] __d_alloc
 0.17%  [kernel]  [k] inode_init_always
 0.16%  [kernel]  [k] sock_alloc   
 0.16%  [kernel]  [k] del_timer
 0.15%  [kernel]  [k] entry_SYSCALL_64_after_swapgs
 0.15%  perf  [.] 0x0004d924   
 0.15%  [kernel]  [k] tcp_close




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to

Re: [PATCH v4] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

2015-10-31 Thread Måns Rullgård 
Francois Romieu  writes:

>> +static int nb8800_poll(struct napi_struct *napi, int budget)
>> +{
>> +struct net_device *dev = napi->dev;
>> +struct nb8800_priv *priv = netdev_priv(dev);
>> +struct nb8800_dma_desc *rx;
>> +int work = 0;
>> +int last = priv->rx_eoc;
>> +int next;
>> +
>> +while (work < budget) {
>> +struct rx_buf *rx_buf;
>> +u32 report;
>> +int len;
>> +
>> +next = (last + 1) & (RX_DESC_COUNT - 1);
>> +
>> +rx_buf = >rx_bufs[next];
>> +rx = >rx_descs[next];
>> +report = rx->report;
>> +
>> +if (!report)
>> +break;
>> +
>> +if (IS_RX_ERROR(report)) {
>> +nb8800_rx_error(dev, report);
>> +} else if (likely(rx_buf->page)) {
>> +len = RX_BYTES_TRANSFERRED(report);
>> +nb8800_receive(dev, next, len);
>> +}
>> +
>> +rx->report = 0;
>> +if (!rx_buf->page)
>> +nb8800_alloc_rx(dev, next, true);
>
> It looks like it receives, then tries to allocate new resources. If so
> it may deplete the ring and you should instead consider allocating new
> resources first, then receive data if alloc + map suceeded.

The hardware receives a frame and stores it in the provided DMA buffer,
then raises an interrupt and moves on to the next buffer.  When a buffer
is handed over to the network stack, a new one has to take its place in
the DMA queue.  I'm not sure how you're suggesting this be done
differently.

-- 
Måns Rullgård
m...@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 106241] New: shutdown(3)/close(3) behaviour is incorrect for sockets in accept(3)

2015-10-31 Thread Eric Dumazet 
On Sat, 2015-10-31 at 14:23 -0700, Linus Torvalds wrote:

> Mind testing something really stupid, and making the __clear_bit() in
> __clear_close_on_exec() conditiona, something like this:
> 
>  static inline void __clear_close_on_exec(int fd, struct fdtable *fdt)
>  {
> -   __clear_bit(fd, fdt->close_on_exec);
> +   if (test_bit(fd, fdt->close_on_exec)
> +   __clear_bit(fd, fdt->close_on_exec);
>  }
> 
> and see if it makes a difference.

It does ;)

About 4 % qps increase

3 runs : 
lpaa24:~# taskset ff0ff ./opensock -t 16 -n 1000 -l 10
total = 4176651
total = 4178012
total = 4105226

instead of :
total = 3910620
total = 3874567
total = 3971028

Perf profile :

69.12%   opensock  opensock [.] memset  
 
 |
 --- memset

12.37%   opensock  [kernel.kallsyms][k] queued_spin_lock_slowpath   
 
 |
 --- queued_spin_lock_slowpath
|  
|--99.99%-- _raw_spin_lock
|  |  
|  |--51.99%-- __close_fd
|  |  sys_close
|  |  entry_SYSCALL_64_fastpath
|  |  __libc_close
|  |  |  
|  |   --100.00%-- 0x0
|  |  
|  |--47.79%-- __alloc_fd
|  |  get_unused_fd_flags
|  |  sock_map_fd
|  |  sys_socket
|  |  entry_SYSCALL_64_fastpath
|  |  __socket
|  |  |  
|  |   --100.00%-- 0x0
|   --0.21%-- [...]
 --0.01%-- [...]

 1.92%   opensock  [kernel.kallsyms][k] _find_next_bit.part.0   
 
 |
 --- _find_next_bit.part.0
|  
|--66.93%-- find_next_zero_bit
|  __alloc_fd
|  get_unused_fd_flags
|  sock_map_fd
|  sys_socket
|  entry_SYSCALL_64_fastpath
|  __socket
|  
 --33.07%-- __alloc_fd
   get_unused_fd_flags
   sock_map_fd
   sys_socket
   entry_SYSCALL_64_fastpath
   __socket
   |  
--100.00%-- 0x0

 1.63%   opensock  [kernel.kallsyms][k] _raw_spin_lock  
 
 |
 --- _raw_spin_lock
|  
|--28.66%-- get_unused_fd_flags
|  sock_map_fd


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net,stable] qmi_wwan: fix entry for HP lt4112 LTE/HSPA+ Gobi 4G Module

2015-10-31 Thread Bjørn Mork 
The lt4112 is a HP branded Huawei me906e modem. Like other Huawei
modems, it does not have a fixed interface to function mapping.
Instead it uses a Huawei specific scheme: functions are mapped by
subclass and protocol.

However, the HP vendor ID is used for modems from many different
manufacturers using different schemes, so we cannot apply a generic
vendor rule like we do for the Huawei vendor ID.

Replace the previous lt4112 entry pointing to an arbitrary interface
number with a device specific subclass + protocol match.

Reported-and-tested-by: Muri Nicanor 
Tested-by: Martin Hauke 
Fixes: bb2bdeb83fb1 ("qmi_wwan: Add support for HP lt4112 LTE/HSPA+ Gobi 4G 
Modem")
Signed-off-by: Bjørn Mork 
---
 drivers/net/usb/qmi_wwan.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index c2e7222f2556..434f147e67ac 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -487,6 +487,10 @@ static const struct usb_device_id products[] = {
  USB_CDC_PROTO_NONE),
.driver_info= (unsigned long)_wwan_info,
},
+   {   /* HP lt4112 LTE/HSPA+ Gobi 4G Module (Huawei me906e) */
+   USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x581d, 
USB_CLASS_VENDOR_SPEC, 1, 7),
+   .driver_info = (unsigned long)_wwan_info,
+   },
 
/* 3. Combined interface devices matching on interface number */
{QMI_FIXED_INTF(0x0408, 0xea42, 4)},/* Yota / Megafon M100-1 */
@@ -739,7 +743,6 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x413c, 0x81a9, 8)},/* Dell Wireless 5808e Gobi(TM) 
4G LTE Mobile Broadband Card */
{QMI_FIXED_INTF(0x413c, 0x81b1, 8)},/* Dell Wireless 5809e Gobi(TM) 
4G LTE Mobile Broadband Card */
{QMI_FIXED_INTF(0x03f0, 0x4e1d, 8)},/* HP lt4111 LTE/EV-DO/HSPA+ 
Gobi 4G Module */
-   {QMI_FIXED_INTF(0x03f0, 0x581d, 4)},/* HP lt4112 LTE/HSPA+ Gobi 4G 
Module (Huawei me906e) */
 
/* 4. Gobi 1000 devices */
{QMI_GOBI1K_DEVICE(0x05c6, 0x9212)},/* Acer Gobi Modem Device */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html