Re: [PATCH 2/4] crypto: af_alg - Allow arbitrarily long algorithm names

[Herbert Xu]

On Thu, Apr 06, 2017 at 02:32:27PM +0200, Alexander Sverdlin wrote:
>
> > diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> > index 690deca..3556d8e 100644
> > --- a/crypto/af_alg.c
> > +++ b/crypto/af_alg.c
> > @@ -160,11 +160,11 @@ static int alg_bind(struct socket *sock, struct 
> > sockaddr *uaddr, int addr_len)
> > if (sock->state == SS_CONNECTED)
> > return -EINVAL;
> >  
> > -   if (addr_len != sizeof(*sa))
> > +   if (addr_len < sizeof(*sa))
> > return -EINVAL;
> >  
> > sa->salg_type[sizeof(sa->salg_type) - 1] = 0;
> > -   sa->salg_name[sizeof(sa->salg_name) - 1] = 0;
> > +   sa->salg_name[sizeof(sa->salg_name) + addr_len - sizeof(*sa) - 1] = 0;
> >  
> > type = alg_get_type(sa->salg_type);
> > if (IS_ERR(type) && PTR_ERR(type) == -ENOENT) {
> 
> Why should userspace ever extend the structure if salg_name is hardcoded to 
> 64 in if_alg.h?
> This patch doesn't change the behavior at all, or am I missing something?

We cannot change the structure in the user-space API by increasing
its length because that would break backwards compatibility.

What this patch does is relax the size check so that user-space can
pass in a name that is longer than 64 bytes.  This would fail on
older kernels with EINVAL.  Of course, user-space must be modified
to allow for such longer names.  That is beyond the scope of this
patch.

For names shorter than 64 bytes the existing API needs be respected
and user-space must pad it to exactly 64 bytes.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 12/12] ftgmac100: Remove tx descriptor accessors

[Benjamin Herrenschmidt]

Directly access the fields when needed. The accessors add clutter
not clarity and in some cases cause unnecessary read-modify-write
type access on the slow (uncached) descriptor memory.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 174 ---
 drivers/net/ethernet/faraday/ftgmac100.h |   8 +-
 2 files changed, 69 insertions(+), 113 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 1496141..bea12a6 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -466,77 +466,13 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, 
int *processed)
return true;
 }
 
-static bool ftgmac100_txdes_owned_by_dma(struct ftgmac100_txdes *txdes)
+static u32 ftgmac100_base_tx_ctlstat(struct ftgmac100 *priv,
+unsigned int index)
 {
-   return txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
-}
-
-static void ftgmac100_txdes_set_dma_own(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
-}
-
-static void ftgmac100_txdes_set_end_of_ring(const struct ftgmac100 *priv,
-   struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes0 |= cpu_to_le32(priv->txdes0_edotr_mask);
-}
-
-static void ftgmac100_txdes_set_first_segment(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_FTS);
-}
-
-static inline bool ftgmac100_txdes_get_first_segment(struct ftgmac100_txdes 
*txdes)
-{
-   return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_FTS)) != 0;
-}
-
-static void ftgmac100_txdes_set_last_segment(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_LTS);
-}
-
-static inline bool ftgmac100_txdes_get_last_segment(struct ftgmac100_txdes 
*txdes)
-{
-   return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_LTS)) != 0;
-}
-
-static void ftgmac100_txdes_set_buffer_size(struct ftgmac100_txdes *txdes,
-   unsigned int len)
-{
-   txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXBUF_SIZE(len));
-}
-
-static inline unsigned int ftgmac100_txdes_get_buffer_size(struct 
ftgmac100_txdes *txdes)
-{
-   return FTGMAC100_TXDES0_TXBUF_SIZE(cpu_to_le32(txdes->txdes0));
-}
-
-static void ftgmac100_txdes_set_tcpcs(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_TCP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_udpcs(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_UDP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_ipcs(struct ftgmac100_txdes *txdes)
-{
-   txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_IP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_dma_addr(struct ftgmac100_txdes *txdes,
-dma_addr_t addr)
-{
-   txdes->txdes3 = cpu_to_le32(addr);
-}
-
-static dma_addr_t ftgmac100_txdes_get_dma_addr(struct ftgmac100_txdes *txdes)
-{
-   return (dma_addr_t)le32_to_cpu(txdes->txdes3);
+   if (index == (TX_QUEUE_ENTRIES - 1))
+   return priv->txdes0_edotr_mask;
+   else
+   return 0;
 }
 
 static int ftgmac100_next_tx_pointer(int pointer)
@@ -564,32 +500,28 @@ static bool ftgmac100_tx_buf_cleanable(struct ftgmac100 
*priv)
 static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
 unsigned int pointer,
 struct sk_buff *skb,
-struct ftgmac100_txdes *txdes)
+struct ftgmac100_txdes *txdes,
+u32 ctl_stat)
 {
-   dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
+   dma_addr_t map = le32_to_cpu(txdes->txdes3);
+   size_t len;
 
-   if (ftgmac100_txdes_get_first_segment(txdes)) {
-   size_t len = skb_headlen(skb);
+   if (ctl_stat & FTGMAC100_TXDES0_FTS) {
+   len = skb_headlen(skb);
 
if (skb_shinfo(skb)->nr_frags == 0 && len < ETH_ZLEN)
len = ETH_ZLEN;
dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
} else {
-   dma_unmap_page(priv->dev, map,
-  ftgmac100_txdes_get_buffer_size(txdes),
-  DMA_TO_DEVICE);
+   len = FTGMAC100_TXDES0_TXBUF_SIZE(ctl_stat);
+
+   dma_unmap_page(priv->dev, map, len, DMA_TO_DEVICE);
}
 
-   if (ftgmac100_txdes_get_last_segment(txdes))
+   /* Free SKB on last segment */
+   if (ctl_stat & FTGMAC100_TXDES0_LTS)
dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
-
-   /* Clear txdes0 except end of ring bit, clear txdes1 as we
-* only "OR" into it, leave 2 and 3 alone 

Re: [Patch net] net_sched: replace yield() with cond_resched()

[Mike Galbraith]

On Thu, 2017-04-06 at 18:13 -0400, Stephen Hemminger wrote:

> Why not replace yield with msleep(1) which gets rid of the inversion
> issues?

That's fine, just not super efficient, if that matters.

-Mike

[PATCH 08/12] ftgmac100: Move the barrier out of ftgmac100_txdes_set_dma_own()

[Benjamin Herrenschmidt]

We'll use variants of this accessor without barriers when
building series of descriptors for fragmented sends

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index ccf0fcd..31fbb75 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -483,11 +483,6 @@ static bool ftgmac100_txdes_owned_by_dma(struct 
ftgmac100_txdes *txdes)
 
 static void ftgmac100_txdes_set_dma_own(struct ftgmac100_txdes *txdes)
 {
-   /*
-* Make sure dma own bit will not be set before any other
-* descriptor fields.
-*/
-   wmb();
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
 }
 
@@ -678,6 +673,10 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
 
+   /* Order the previous packet and descriptor udpates
+* before setting the OWN bit.
+*/
+   dma_wmb();
ftgmac100_txdes_set_dma_own(txdes);
 
/* Update next TX pointer */
-- 
2.9.3

[PATCH 05/12] ftgmac100: Pad small frames properly

[Benjamin Herrenschmidt]

Rather than just transmitting garbage past the end of the small
packet.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 6c71726..3f2172f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -636,6 +636,13 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
struct ftgmac100_txdes *txdes;
dma_addr_t map;
 
+   /* The HW doesn't pad small frames */
+   if (skb_padto(skb, ETH_ZLEN) < 0) {
+   netdev->stats.tx_dropped++;
+   return NETDEV_TX_OK;
+   }
+
+   /* Reject oversize packets */
if (unlikely(skb->len > MAX_PKT_SIZE)) {
if (net_ratelimit())
netdev_dbg(netdev, "tx packet too big\n");
-- 
2.9.3

[PATCH 06/12] ftgmac100: Store tx skbs in a separate array

[Benjamin Herrenschmidt]

Rather than in the descriptor. The descriptor is mapped non-cachable
and rather slow to access.

Since to do that we need to keep track of the tx "pointer" we also
have no use of all the accesors to manipulate it, just open code
it, it's as clear and will help when adding fragmented sends.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 58 +---
 1 file changed, 15 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 3f2172f..ff01dfb 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -544,63 +544,29 @@ static dma_addr_t ftgmac100_txdes_get_dma_addr(struct 
ftgmac100_txdes *txdes)
return le32_to_cpu(txdes->txdes3);
 }
 
-/*
- * txdes2 is not used by hardware. We use it to keep track of socket buffer.
- * Since hardware does not touch it, we can skip cpu_to_le32()/le32_to_cpu().
- */
-static void ftgmac100_txdes_set_skb(struct ftgmac100_txdes *txdes,
-   struct sk_buff *skb)
-{
-   txdes->txdes2 = (unsigned int)skb;
-}
-
-static struct sk_buff *ftgmac100_txdes_get_skb(struct ftgmac100_txdes *txdes)
-{
-   return (struct sk_buff *)txdes->txdes2;
-}
-
 static int ftgmac100_next_tx_pointer(int pointer)
 {
return (pointer + 1) & (TX_QUEUE_ENTRIES - 1);
 }
 
-static void ftgmac100_tx_pointer_advance(struct ftgmac100 *priv)
-{
-   priv->tx_pointer = ftgmac100_next_tx_pointer(priv->tx_pointer);
-}
-
-static void ftgmac100_tx_clean_pointer_advance(struct ftgmac100 *priv)
-{
-   priv->tx_clean_pointer = 
ftgmac100_next_tx_pointer(priv->tx_clean_pointer);
-}
-
-static struct ftgmac100_txdes *ftgmac100_current_txdes(struct ftgmac100 *priv)
-{
-   return >descs->txdes[priv->tx_pointer];
-}
-
-static struct ftgmac100_txdes *
-ftgmac100_current_clean_txdes(struct ftgmac100 *priv)
-{
-   return >descs->txdes[priv->tx_clean_pointer];
-}
-
 static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
 {
struct net_device *netdev = priv->netdev;
struct ftgmac100_txdes *txdes;
+   unsigned int pointer;
struct sk_buff *skb;
dma_addr_t map;
 
if (priv->tx_pending == 0)
return false;
 
-   txdes = ftgmac100_current_clean_txdes(priv);
+   pointer = priv->tx_clean_pointer;
+   txdes = >descs->txdes[pointer];
 
if (ftgmac100_txdes_owned_by_dma(txdes))
return false;
 
-   skb = ftgmac100_txdes_get_skb(txdes);
+   skb = priv->tx_skbs[pointer];
map = ftgmac100_txdes_get_dma_addr(txdes);
 
netdev->stats.tx_packets++;
@@ -609,10 +575,11 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
 
dev_kfree_skb(skb);
+   priv->tx_skbs[pointer] = NULL;
 
ftgmac100_txdes_reset(priv, txdes);
 
-   ftgmac100_tx_clean_pointer_advance(priv);
+   priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
 
spin_lock(>tx_lock);
priv->tx_pending--;
@@ -634,6 +601,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
struct ftgmac100 *priv = netdev_priv(netdev);
struct ftgmac100_txdes *txdes;
+   unsigned int pointer;
dma_addr_t map;
 
/* The HW doesn't pad small frames */
@@ -657,11 +625,12 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
goto drop;
}
 
-   txdes = ftgmac100_current_txdes(priv);
-   ftgmac100_tx_pointer_advance(priv);
+   /* Grab the next free tx descriptor */
+   pointer = priv->tx_pointer;
+   txdes = >descs->txdes[pointer];
 
/* setup TX descriptor */
-   ftgmac100_txdes_set_skb(txdes, skb);
+   priv->tx_skbs[pointer] = skb;
ftgmac100_txdes_set_dma_addr(txdes, map);
ftgmac100_txdes_set_buffer_size(txdes, len);
 
@@ -682,6 +651,9 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
 
+   /* Update next TX pointer */
+   priv->tx_pointer = ftgmac100_next_tx_pointer(pointer);
+
spin_lock(>tx_lock);
priv->tx_pending++;
if (priv->tx_pending == TX_QUEUE_ENTRIES)
@@ -724,7 +696,7 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
/* Free all TX buffers */
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
struct ftgmac100_txdes *txdes = >descs->txdes[i];
-   struct sk_buff *skb = ftgmac100_txdes_get_skb(txdes);
+   struct sk_buff *skb = priv->tx_skbs[i];
dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
 
if (!skb)
-- 
2.9.3

[PATCH 04/12] ftgmac100: Factor tx packet dropping path

[Benjamin Herrenschmidt]

Use a simple goto to a drop path at the tail of the function,
it will be used in a few more cases soon

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 6fa070a..6c71726 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -639,10 +639,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
if (unlikely(skb->len > MAX_PKT_SIZE)) {
if (net_ratelimit())
netdev_dbg(netdev, "tx packet too big\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
+   goto drop;
}
 
map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
@@ -650,10 +647,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
/* drop packet */
if (net_ratelimit())
netdev_err(netdev, "map socket buffer failed\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
+   goto drop;
}
 
txdes = ftgmac100_current_txdes(priv);
@@ -693,6 +687,13 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
ftgmac100_txdma_normal_prio_start_polling(priv);
 
return NETDEV_TX_OK;
+
+ drop:
+   /* Drop the packet */
+   dev_kfree_skb_any(skb);
+   netdev->stats.tx_dropped++;
+
+   return NETDEV_TX_OK;
 }
 
 static void ftgmac100_free_buffers(struct ftgmac100 *priv)
-- 
2.9.3

[PATCH 01/12] ftgmac100: Add a tx timeout handler

[Benjamin Herrenschmidt]

We have a reset task to reset our chip, use it.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 9f18e5e..f4b719214 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1248,6 +1248,17 @@ static int ftgmac100_do_ioctl(struct net_device *netdev, 
struct ifreq *ifr, int
return phy_mii_ioctl(netdev->phydev, ifr, cmd);
 }
 
+static void ftgmac100_tx_timeout(struct net_device *netdev)
+{
+   struct ftgmac100 *priv = netdev_priv(netdev);
+
+   /* Disable all interrupts */
+   iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+
+   /* Do the reset outside of interrupt context */
+   schedule_work(>reset_task);
+}
+
 static const struct net_device_ops ftgmac100_netdev_ops = {
.ndo_open   = ftgmac100_open,
.ndo_stop   = ftgmac100_stop,
@@ -1255,6 +1266,7 @@ static const struct net_device_ops ftgmac100_netdev_ops = 
{
.ndo_set_mac_address= ftgmac100_set_mac_addr,
.ndo_validate_addr  = eth_validate_addr,
.ndo_do_ioctl   = ftgmac100_do_ioctl,
+   .ndo_tx_timeout = ftgmac100_tx_timeout,
 };
 
 static int ftgmac100_setup_mdio(struct net_device *netdev)
@@ -1359,6 +1371,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
 
netdev->ethtool_ops = _ethtool_ops;
netdev->netdev_ops = _netdev_ops;
+   netdev->watchdog_timeo = 5 * HZ;
 
platform_set_drvdata(pdev, netdev);
 
-- 
2.9.3

[PATCH 09/12] ftgmac100: Split tx packet freeing from ftgmac100_tx_complete_packet()

[Benjamin Herrenschmidt]

This moves the packet freeing to a separate function
which is also used by ftgmac100_free_buffers() and will
be used more in the error path of fragmented sends.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 38 ++--
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 31fbb75..22edd01 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -561,13 +561,29 @@ static bool ftgmac100_tx_buf_cleanable(struct ftgmac100 
*priv)
return priv->tx_pointer != priv->tx_clean_pointer;
 }
 
+static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
+unsigned int pointer,
+struct sk_buff *skb,
+struct ftgmac100_txdes *txdes)
+{
+   dma_addr_t map;
+
+   map = ftgmac100_txdes_get_dma_addr(txdes);
+
+   dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
+
+   dev_kfree_skb(skb);
+   priv->tx_skbs[pointer] = NULL;
+
+   ftgmac100_txdes_reset(priv, txdes);
+}
+
 static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
 {
struct net_device *netdev = priv->netdev;
struct ftgmac100_txdes *txdes;
-   unsigned int pointer;
struct sk_buff *skb;
-   dma_addr_t map;
+   unsigned int pointer;
 
pointer = priv->tx_clean_pointer;
txdes = >descs->txdes[pointer];
@@ -576,17 +592,9 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
return false;
 
skb = priv->tx_skbs[pointer];
-   map = ftgmac100_txdes_get_dma_addr(txdes);
-
netdev->stats.tx_packets++;
netdev->stats.tx_bytes += skb->len;
-
-   dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
-
-   dev_kfree_skb(skb);
-   priv->tx_skbs[pointer] = NULL;
-
-   ftgmac100_txdes_reset(priv, txdes);
+   ftgmac100_free_tx_packet(priv, pointer, skb, txdes);
 
priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
 
@@ -728,13 +736,9 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
struct ftgmac100_txdes *txdes = >descs->txdes[i];
struct sk_buff *skb = priv->tx_skbs[i];
-   dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
-
-   if (!skb)
-   continue;
 
-   dma_unmap_single(priv->dev, map, skb_headlen(skb), 
DMA_TO_DEVICE);
-   kfree_skb(skb);
+   if (skb)
+   ftgmac100_free_tx_packet(priv, i, skb, txdes);
}
 }
 
-- 
2.9.3

[PATCH 10/12] ftgmac100: Don't clear tx desc fields unnecessarily

[Benjamin Herrenschmidt]

Those are non-cachable stores, let's avoid those we don't need. Remove
the helper, it's not particularly helpful and since it uses "priv"
I can't move it to the header file.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 22edd01..a68a7c4 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -466,16 +466,6 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, 
int *processed)
return true;
 }
 
-static void ftgmac100_txdes_reset(const struct ftgmac100 *priv,
- struct ftgmac100_txdes *txdes)
-{
-   /* clear all except end of ring bit */
-   txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
-   txdes->txdes1 = 0;
-   txdes->txdes2 = 0;
-   txdes->txdes3 = 0;
-}
-
 static bool ftgmac100_txdes_owned_by_dma(struct ftgmac100_txdes *txdes)
 {
return txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
@@ -575,7 +565,12 @@ static void ftgmac100_free_tx_packet(struct ftgmac100 
*priv,
dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
 
-   ftgmac100_txdes_reset(priv, txdes);
+   /* Clear txdes0 except end of ring bit, clear txdes1 as we
+* only "OR" into it, leave 2 and 3 alone as 2 is unused
+* and 3 will be overwritten entirely
+*/
+   txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
+   txdes->txdes1 = 0;
 }
 
 static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
-- 
2.9.3

[PATCH 03/12] ftgmac100: Merge ftgmac100_xmit() into ftgmac100_hard_start_xmit()

[Benjamin Herrenschmidt]

This will make subsequent rework of the tx path simpler

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 58 ++--
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index 3966c7a..6fa070a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -628,12 +628,33 @@ static void ftgmac100_tx_complete(struct ftgmac100 *priv)
;
 }
 
-static int ftgmac100_xmit(struct ftgmac100 *priv, struct sk_buff *skb,
- dma_addr_t map)
+static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
+struct net_device *netdev)
 {
-   struct net_device *netdev = priv->netdev;
-   struct ftgmac100_txdes *txdes;
unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
+   struct ftgmac100 *priv = netdev_priv(netdev);
+   struct ftgmac100_txdes *txdes;
+   dma_addr_t map;
+
+   if (unlikely(skb->len > MAX_PKT_SIZE)) {
+   if (net_ratelimit())
+   netdev_dbg(netdev, "tx packet too big\n");
+
+   netdev->stats.tx_dropped++;
+   kfree_skb(skb);
+   return NETDEV_TX_OK;
+   }
+
+   map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
+   if (unlikely(dma_mapping_error(priv->dev, map))) {
+   /* drop packet */
+   if (net_ratelimit())
+   netdev_err(netdev, "map socket buffer failed\n");
+
+   netdev->stats.tx_dropped++;
+   kfree_skb(skb);
+   return NETDEV_TX_OK;
+   }
 
txdes = ftgmac100_current_txdes(priv);
ftgmac100_tx_pointer_advance(priv);
@@ -674,35 +695,6 @@ static int ftgmac100_xmit(struct ftgmac100 *priv, struct 
sk_buff *skb,
return NETDEV_TX_OK;
 }
 
-static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
-struct net_device *netdev)
-{
-   struct ftgmac100 *priv = netdev_priv(netdev);
-   dma_addr_t map;
-
-   if (unlikely(skb->len > MAX_PKT_SIZE)) {
-   if (net_ratelimit())
-   netdev_dbg(netdev, "tx packet too big\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
-   }
-
-   map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(priv->dev, map))) {
-   /* drop packet */
-   if (net_ratelimit())
-   netdev_err(netdev, "map socket buffer failed\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
-   }
-
-   return ftgmac100_xmit(priv, skb, map);
-}
-
 static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 {
int i;
-- 
2.9.3

[PATCH 02/12] ftgmac100: Move ftgmac100_hard_start_xmit() around

[Benjamin Herrenschmidt]

Move it below ftgmac100_xmit() and the rest of the tx path

No code change.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 59 
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index f4b719214..3966c7a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -63,6 +63,7 @@ struct ftgmac100 {
u32 rxdes0_edorr_mask;
 
/* Tx ring */
+   struct sk_buff *tx_skbs[TX_QUEUE_ENTRIES];
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
unsigned int tx_pending;
@@ -673,6 +674,35 @@ static int ftgmac100_xmit(struct ftgmac100 *priv, struct 
sk_buff *skb,
return NETDEV_TX_OK;
 }
 
+static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
+struct net_device *netdev)
+{
+   struct ftgmac100 *priv = netdev_priv(netdev);
+   dma_addr_t map;
+
+   if (unlikely(skb->len > MAX_PKT_SIZE)) {
+   if (net_ratelimit())
+   netdev_dbg(netdev, "tx packet too big\n");
+
+   netdev->stats.tx_dropped++;
+   kfree_skb(skb);
+   return NETDEV_TX_OK;
+   }
+
+   map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
+   if (unlikely(dma_mapping_error(priv->dev, map))) {
+   /* drop packet */
+   if (net_ratelimit())
+   netdev_err(netdev, "map socket buffer failed\n");
+
+   netdev->stats.tx_dropped++;
+   kfree_skb(skb);
+   return NETDEV_TX_OK;
+   }
+
+   return ftgmac100_xmit(priv, skb, map);
+}
+
 static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 {
int i;
@@ -1210,35 +1240,6 @@ static int ftgmac100_stop(struct net_device *netdev)
return 0;
 }
 
-static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
-struct net_device *netdev)
-{
-   struct ftgmac100 *priv = netdev_priv(netdev);
-   dma_addr_t map;
-
-   if (unlikely(skb->len > MAX_PKT_SIZE)) {
-   if (net_ratelimit())
-   netdev_dbg(netdev, "tx packet too big\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
-   }
-
-   map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(priv->dev, map))) {
-   /* drop packet */
-   if (net_ratelimit())
-   netdev_err(netdev, "map socket buffer failed\n");
-
-   netdev->stats.tx_dropped++;
-   kfree_skb(skb);
-   return NETDEV_TX_OK;
-   }
-
-   return ftgmac100_xmit(priv, skb, map);
-}
-
 /* optional */
 static int ftgmac100_do_ioctl(struct net_device *netdev, struct ifreq *ifr, 
int cmd)
 {
-- 
2.9.3

[PATCH 07/12] ftgmac100: Cleanup tx queue handling

[Benjamin Herrenschmidt]

We have a private lock which isn't terribly useful, and we maintain
a "tx_pending" counter for information that's already available
via a trivial arithmetic operation. Then we unconditionaly wake
the queue even when not stopped. Finally our code in tx isn't
really safe vs. a concurrent reclaim. The aspeed chips aren't SMP
today but I prefer the code being right and future proof.

So rip that out and replace it with more "standard" queue handling,
currently with a threshold of 1 queue element, which will be
increased when we implement fragmented sends.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/net/ethernet/faraday/ftgmac100.c | 103 ---
 1 file changed, 68 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index ff01dfb..ccf0fcd 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -44,6 +44,9 @@
 #define MAX_PKT_SIZE   1536
 #define RX_BUF_SIZEMAX_PKT_SIZE/* must be smaller than 0x3fff 
*/
 
+/* Min number of tx ring entries before stopping queue */
+#define TX_THRESHOLD   (1)
+
 struct ftgmac100_descs {
struct ftgmac100_rxdes rxdes[RX_QUEUE_ENTRIES];
struct ftgmac100_txdes txdes[TX_QUEUE_ENTRIES];
@@ -66,9 +69,7 @@ struct ftgmac100 {
struct sk_buff *tx_skbs[TX_QUEUE_ENTRIES];
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
-   unsigned int tx_pending;
u32 txdes0_edotr_mask;
-   spinlock_t tx_lock;
 
/* Scratch page to use when rx skb alloc fails */
void *rx_scratch;
@@ -163,7 +164,6 @@ static int ftgmac100_reset_and_config_mac(struct ftgmac100 
*priv)
priv->rx_pointer = 0;
priv->tx_clean_pointer = 0;
priv->tx_pointer = 0;
-   priv->tx_pending = 0;
 
/* The doc says reset twice with 10us interval */
if (ftgmac100_reset_mac(priv, maccr))
@@ -549,6 +549,23 @@ static int ftgmac100_next_tx_pointer(int pointer)
return (pointer + 1) & (TX_QUEUE_ENTRIES - 1);
 }
 
+static u32 ftgmac100_tx_buf_avail(struct ftgmac100 *priv)
+{
+   /* Returns the number of available slots in the TX queue
+*
+* This always leaves one free slot so we don't have to
+* worry about empty vs. full, and this simplifies the
+* test for ftgmac100_tx_buf_cleanable() below
+*/
+   return (priv->tx_clean_pointer - priv->tx_pointer - 1) &
+   (TX_QUEUE_ENTRIES - 1);
+}
+
+static bool ftgmac100_tx_buf_cleanable(struct ftgmac100 *priv)
+{
+   return priv->tx_pointer != priv->tx_clean_pointer;
+}
+
 static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
 {
struct net_device *netdev = priv->netdev;
@@ -557,9 +574,6 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
struct sk_buff *skb;
dma_addr_t map;
 
-   if (priv->tx_pending == 0)
-   return false;
-
pointer = priv->tx_clean_pointer;
txdes = >descs->txdes[pointer];
 
@@ -581,18 +595,31 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100 
*priv)
 
priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
 
-   spin_lock(>tx_lock);
-   priv->tx_pending--;
-   spin_unlock(>tx_lock);
-   netif_wake_queue(netdev);
-
return true;
 }
 
 static void ftgmac100_tx_complete(struct ftgmac100 *priv)
 {
-   while (ftgmac100_tx_complete_packet(priv))
+   struct net_device *netdev = priv->netdev;
+
+   /* Process all completed packets */
+   while (ftgmac100_tx_buf_cleanable(priv) &&
+  ftgmac100_tx_complete_packet(priv))
;
+
+   /* Restart queue if needed */
+   smp_mb();
+   if (unlikely(netif_queue_stopped(netdev) &&
+ftgmac100_tx_buf_avail(priv) >= TX_THRESHOLD)) {
+   struct netdev_queue *txq;
+
+   txq = netdev_get_tx_queue(netdev, 0);
+   __netif_tx_lock(txq, smp_processor_id());
+   if (netif_queue_stopped(netdev) &&
+   ftgmac100_tx_buf_avail(priv) >= TX_THRESHOLD)
+   netif_wake_queue(netdev);
+   __netif_tx_unlock(txq);
+   }
 }
 
 static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
@@ -651,17 +678,22 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
 
+   ftgmac100_txdes_set_dma_own(txdes);
+
/* Update next TX pointer */
priv->tx_pointer = ftgmac100_next_tx_pointer(pointer);
 
-   spin_lock(>tx_lock);
-   priv->tx_pending++;
-   if (priv->tx_pending == TX_QUEUE_ENTRIES)
+   /* If there isn't enough room for all the fragments of a new packet
+* in the TX ring, stop the queue. The sequence below is race free
+* vs. a concurrent restart in ftgmac100_poll()
+*/
+   if 

[PATCH 11/12] ftgmac100: Add support for fragmented tx

[Benjamin Herrenschmidt]

Add NETIF_F_SG and create multiple TX ring entries for skb fragments.

On reclaim, the skb is only freed on the segment marked as "last".

Signed-off-by: Benjamin Herrenschmidt 

# Conflicts:
#   drivers/net/ethernet/faraday/ftgmac100.c
---
 drivers/net/ethernet/faraday/ftgmac100.c | 121 +--
 1 file changed, 97 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c 
b/drivers/net/ethernet/faraday/ftgmac100.c
index a68a7c4..1496141 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -45,7 +45,7 @@
 #define RX_BUF_SIZEMAX_PKT_SIZE/* must be smaller than 0x3fff 
*/
 
 /* Min number of tx ring entries before stopping queue */
-#define TX_THRESHOLD   (1)
+#define TX_THRESHOLD   (MAX_SKB_FRAGS + 1)
 
 struct ftgmac100_descs {
struct ftgmac100_rxdes rxdes[RX_QUEUE_ENTRIES];
@@ -487,20 +487,30 @@ static void ftgmac100_txdes_set_first_segment(struct 
ftgmac100_txdes *txdes)
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_FTS);
 }
 
+static inline bool ftgmac100_txdes_get_first_segment(struct ftgmac100_txdes 
*txdes)
+{
+   return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_FTS)) != 0;
+}
+
 static void ftgmac100_txdes_set_last_segment(struct ftgmac100_txdes *txdes)
 {
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_LTS);
 }
 
+static inline bool ftgmac100_txdes_get_last_segment(struct ftgmac100_txdes 
*txdes)
+{
+   return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_LTS)) != 0;
+}
+
 static void ftgmac100_txdes_set_buffer_size(struct ftgmac100_txdes *txdes,
unsigned int len)
 {
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXBUF_SIZE(len));
 }
 
-static void ftgmac100_txdes_set_txint(struct ftgmac100_txdes *txdes)
+static inline unsigned int ftgmac100_txdes_get_buffer_size(struct 
ftgmac100_txdes *txdes)
 {
-   txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_TXIC);
+   return FTGMAC100_TXDES0_TXBUF_SIZE(cpu_to_le32(txdes->txdes0));
 }
 
 static void ftgmac100_txdes_set_tcpcs(struct ftgmac100_txdes *txdes)
@@ -526,7 +536,7 @@ static void ftgmac100_txdes_set_dma_addr(struct 
ftgmac100_txdes *txdes,
 
 static dma_addr_t ftgmac100_txdes_get_dma_addr(struct ftgmac100_txdes *txdes)
 {
-   return le32_to_cpu(txdes->txdes3);
+   return (dma_addr_t)le32_to_cpu(txdes->txdes3);
 }
 
 static int ftgmac100_next_tx_pointer(int pointer)
@@ -556,13 +566,22 @@ static void ftgmac100_free_tx_packet(struct ftgmac100 
*priv,
 struct sk_buff *skb,
 struct ftgmac100_txdes *txdes)
 {
-   dma_addr_t map;
+   dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
 
-   map = ftgmac100_txdes_get_dma_addr(txdes);
+   if (ftgmac100_txdes_get_first_segment(txdes)) {
+   size_t len = skb_headlen(skb);
 
-   dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
+   if (skb_shinfo(skb)->nr_frags == 0 && len < ETH_ZLEN)
+   len = ETH_ZLEN;
+   dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
+   } else {
+   dma_unmap_page(priv->dev, map,
+  ftgmac100_txdes_get_buffer_size(txdes),
+  DMA_TO_DEVICE);
+   }
 
-   dev_kfree_skb(skb);
+   if (ftgmac100_txdes_get_last_segment(txdes))
+   dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
 
/* Clear txdes0 except end of ring bit, clear txdes1 as we
@@ -623,10 +642,9 @@ static void ftgmac100_tx_complete(struct ftgmac100 *priv)
 static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
 struct net_device *netdev)
 {
-   unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
struct ftgmac100 *priv = netdev_priv(netdev);
-   struct ftgmac100_txdes *txdes;
-   unsigned int pointer;
+   struct ftgmac100_txdes *txdes, *first;
+   unsigned int pointer, nfrags, len, i, j;
dma_addr_t map;
 
/* The HW doesn't pad small frames */
@@ -642,26 +660,35 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
goto drop;
}
 
-   map = dma_map_single(priv->dev, skb->data, skb_headlen(skb), 
DMA_TO_DEVICE);
-   if (unlikely(dma_mapping_error(priv->dev, map))) {
-   /* drop packet */
+   /* Do we have a limit on #fragments ? I yet have to get a reply
+* from Aspeed. If there's one I haven't hit it.
+*/
+   nfrags = skb_shinfo(skb)->nr_frags;
+
+   /* Get header len and pad for non-fragmented packets */
+   len = skb_headlen(skb);
+   if (nfrags == 0 && len < ETH_ZLEN)
+   len = ETH_ZLEN;
+
+   /* Map the packet head */
+   map = dma_map_single(priv->dev, skb->data, len, DMA_TO_DEVICE);
+   if 

[PATCH 00/12] ftgmac100: Rework batch 3 - TX path

[Benjamin Herrenschmidt]

This is the third batch of updates to the ftgmac100 driver.

This one tackles the TX path of the driver. This provides the
bulk of the performance improvements by adding support for
fragmented sends along with a bunch of cleanups.

Subsequent batches will add various features (ethtool functions,
vlan offlan, ...) and cleanups.

[net-next 0/4][pull request] 40GbE Intel Wired LAN Driver Updates 2017-04-06

[Jeff Kirsher]

This series contains updates to i40e and i40evf.

Preethi adds support for the outer checksum and TSO offloads for
encapsulated packets for the VF.

Mitch fixes a possible memory leak, where we need to remove the client
instance when the driver unloads.  Also we need to check to see if the
client (i40iw) is already present during probe, and add a client instance
if necessary.  Lastly, make sure we close any attached clients when the
driver is removed or shut down to prevent a kernel panic.

The following are changes since commit dc423b6be10c56d0dd957caf307f12d16f728f07:
  Merge branch 'ftgmac-rework-batch2-rx-path'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE

Mitch Williams (3):
  i40e: remove client instance on driver unload
  i40e: register existing client on probe
  i40e: close client on remove and shutdown

Preethi Banala (1):
  i40e/i40evf: Add capability exchange for outer checksum

 drivers/net/ethernet/intel/i40e/i40e_client.c  |  9 
 drivers/net/ethernet/intel/i40e/i40e_main.c| 62 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl.h|  3 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  7 +++
 drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h  |  3 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c| 61 +++--
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c|  4 +-
 7 files changed, 93 insertions(+), 56 deletions(-)

-- 
2.12.2

[net-next 1/4] i40e/i40evf: Add capability exchange for outer checksum

[Jeff Kirsher]

From: Preethi Banala 

This patch adds a capability negotiation between VF and PF using ENCAP/
ENCAP_CSUM offload flags in order for the VF to support outer checksum
and TSO offloads for encapsulated packets. These capabilities were assumed
by default and enabled in current hardware. Going forward, these features
needs to be negotiated with PF before advertising to the stack.
Additionally, strip out the mac.type checks for X722 since outer checksums
are enabled based on the ENCAP_CSUM offload negotiation flag and maintain
consistency between drivers in how the features are configured.

Change-ID: Ie380a6f57eca557a2bb575b66b12fae36d308920
Signed-off-by: Preethi Banala 
Signed-off-by: Alan Brady 
Signed-off-by: Jesse Brandeburg 
Signed-off-by: Jacob Keller 
Tested-by: Andrew Bowers 
Signed-off-by: Jeff Kirsher 
---
 drivers/net/ethernet/intel/i40e/i40e_main.c| 52 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl.h|  3 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  7 +++
 drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h  |  3 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c| 61 --
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c|  4 +-
 6 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 703444e92964..7147c67a939d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9253,6 +9253,8 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
u8 broadcast[ETH_ALEN];
u8 mac_addr[ETH_ALEN];
int etherdev_size;
+   netdev_features_t hw_enc_features;
+   netdev_features_t hw_features;
 
etherdev_size = sizeof(struct i40e_netdev_priv);
netdev = alloc_etherdev_mq(etherdev_size, vsi->alloc_queue_pairs);
@@ -9263,43 +9265,43 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
np = netdev_priv(netdev);
np->vsi = vsi;
 
-   netdev->hw_enc_features |= NETIF_F_SG   |
-  NETIF_F_IP_CSUM  |
-  NETIF_F_IPV6_CSUM|
-  NETIF_F_HIGHDMA  |
-  NETIF_F_SOFT_FEATURES|
-  NETIF_F_TSO  |
-  NETIF_F_TSO_ECN  |
-  NETIF_F_TSO6 |
-  NETIF_F_GSO_GRE  |
-  NETIF_F_GSO_GRE_CSUM |
-  NETIF_F_GSO_IPXIP4   |
-  NETIF_F_GSO_IPXIP6   |
-  NETIF_F_GSO_UDP_TUNNEL   |
-  NETIF_F_GSO_UDP_TUNNEL_CSUM  |
-  NETIF_F_GSO_PARTIAL  |
-  NETIF_F_SCTP_CRC |
-  NETIF_F_RXHASH   |
-  NETIF_F_RXCSUM   |
-  0;
+   hw_enc_features = NETIF_F_SG|
+ NETIF_F_IP_CSUM   |
+ NETIF_F_IPV6_CSUM |
+ NETIF_F_HIGHDMA   |
+ NETIF_F_SOFT_FEATURES |
+ NETIF_F_TSO   |
+ NETIF_F_TSO_ECN   |
+ NETIF_F_TSO6  |
+ NETIF_F_GSO_GRE   |
+ NETIF_F_GSO_GRE_CSUM  |
+ NETIF_F_GSO_PARTIAL   |
+ NETIF_F_GSO_UDP_TUNNEL|
+ NETIF_F_GSO_UDP_TUNNEL_CSUM   |
+ NETIF_F_SCTP_CRC  |
+ NETIF_F_RXHASH|
+ NETIF_F_RXCSUM|
+ 0;
 
if (!(pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE))
netdev->gso_partial_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
 
netdev->gso_partial_features |= NETIF_F_GSO_GRE_CSUM;
 
+   netdev->hw_enc_features |= hw_enc_features;
+
/* record features VLANs can make use of */
-   netdev->vlan_features |= netdev->hw_enc_features |
-NETIF_F_TSO_MANGLEID;
+   netdev->vlan_features |= hw_enc_features | NETIF_F_TSO_MANGLEID;
 
if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
netdev->hw_features |= 

[net-next 4/4] i40e: close client on remove and shutdown

[Jeff Kirsher]

From: Mitch Williams 

When the driver is removed or shut down, close any attached clients
(i.e. i40iw). This prevents a panic seen sometimes on forced driver
removal or system shutdown when iWarp is running.

Change-ID: I4f6161e5a73ffbb2fd5883567b007310302bfcb5
Signed-off-by: Mitch Williams 
Tested-by: Andrew Bowers 
Signed-off-by: Jeff Kirsher 
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7147c67a939d..d83430faaa41 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11395,6 +11395,11 @@ static void i40e_remove(struct pci_dev *pdev)
if (pf->service_task.func)
cancel_work_sync(>service_task);
 
+   /* Client close must be called explicitly here because the timer
+* has been stopped.
+*/
+   i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi], false);
+
if (pf->flags & I40E_FLAG_SRIOV_ENABLED) {
i40e_free_vfs(pf);
pf->flags &= ~I40E_FLAG_SRIOV_ENABLED;
@@ -11635,6 +11640,11 @@ static void i40e_shutdown(struct pci_dev *pdev)
cancel_work_sync(>service_task);
i40e_fdir_teardown(pf);
 
+   /* Client close must be called explicitly here because the timer
+* has been stopped.
+*/
+   i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi], false);
+
if (pf->wol_en && (pf->flags & I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE))
i40e_enable_mc_magic_wake(pf);
 
-- 
2.12.2

[net-next 2/4] i40e: remove client instance on driver unload

[Jeff Kirsher]

From: Mitch Williams 

When the driver is unloaded, we need to remove the client instance,
otherwise we leak memory.

Change-ID: If1e7882ac1f6ce15d004722fafbe31afbe0adc9a
Signed-off-by: Mitch Williams 
Tested-by: Andrew Bowers 
Signed-off-by: Jeff Kirsher 
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c 
b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 191028b1489b..d05296a7078e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -459,6 +459,9 @@ int i40e_lan_del_device(struct i40e_pf *pf)
struct i40e_device *ldev, *tmp;
int ret = -ENODEV;
 
+   /* First, remove any client instance. */
+   i40e_client_del_instance(pf);
+
mutex_lock(_device_mutex);
list_for_each_entry_safe(ldev, tmp, _devices, list) {
if (ldev->pf == pf) {
-- 
2.12.2

[net-next 3/4] i40e: register existing client on probe

[Jeff Kirsher]

From: Mitch Williams 

In some cases, a client (i40iw) may already be present when probe is
called. Check for this, and add a client instance if necessary.

Change-ID: I2009312694b7ad81f1023919e4c6c86181f21689
Signed-off-by: Mitch Williams 
Tested-by: Andrew Bowers 
Signed-off-by: Jeff Kirsher 
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c 
b/drivers/net/ethernet/intel/i40e/i40e_client.c
index d05296a7078e..eb2896fd52a6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -436,6 +436,12 @@ int i40e_lan_add_device(struct i40e_pf *pf)
 pf->hw.pf_id, pf->hw.bus.bus_id,
 pf->hw.bus.device, pf->hw.bus.func);
 
+   /* If a client has already been registered, we need to add an instance
+* of it to our new LAN device.
+*/
+   if (registered_client)
+   i40e_client_add_instance(pf);
+
/* Since in some cases register may have happened before a device gets
 * added, we can schedule a subtask to go initiate the clients if
 * they can be launched at probe time.
-- 
2.12.2

Re: [PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro

[Joe Perches]

On Thu, 2017-04-06 at 16:54 -0700, Matthias Kaehlcke wrote:
> Hi Joe,
> 
> El Thu, Apr 06, 2017 at 02:29:20PM -0700 Joe Perches ha dit:
> 
> > On Thu, 2017-04-06 at 14:21 -0700, Matthias Kaehlcke wrote:
> > > The macro results are assigned to u8 variables/fields. Adding the cast
> > > fixes plenty of clang warnings about "implicit conversion from 'int' to
> > > 'u8'".
> > > 
> > > Signed-off-by: Matthias Kaehlcke 
> > > ---
> > >  drivers/net/wireless/ath/ath9k/eeprom.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h 
> > > b/drivers/net/wireless/ath/ath9k/eeprom.h
> > > index 30bf722e33ed..31390af6c33e 100644
> > > --- a/drivers/net/wireless/ath/ath9k/eeprom.h
> > > +++ b/drivers/net/wireless/ath/ath9k/eeprom.h
> > > @@ -106,7 +106,7 @@
> > >  #define AR9285_RDEXT_DEFAULT0x1F
> > >  
> > >  #define ATH9K_POW_SM(_r, _s) (((_r) & 0x3f) << (_s))
> > > -#define FREQ2FBIN(x, y)  ((y) ? ((x) - 2300) : (((x) - 4800) / 
> > > 5))
> > > +#define FREQ2FBIN(x, y)  (u8)((y) ? ((x) - 2300) : (((x) - 4800) 
> > > / 5))
> > 
> > Maybe better to use:
> > 
> > static inline u8 FREQ2FBIN(int x, int y)
> > {
> > if (y)
> > return x - 2300;
> > return (x - 4800) / 5;
> > }
> 
> Thanks for your suggestion! Unfortunately in this case an inline
> function is not suitable since FREQ2FBIN() is mostly used for
> structure initialization:
> 
> static const struct ar9300_eeprom ar9300_default = {
> ...
> .calFreqPier2G = {
> FREQ2FBIN(2412, 1),
> FREQ2FBIN(2437, 1),
> FREQ2FBIN(2472, 1)
> },
> ...

Maybe it's better to remove the second argument and write
something like:

#define FREQ2FBIN(x) \
(u8)(((x) >= 2300 && (x) <= 2555) ? (x) - 2300 : \
 ((x) >= 4800 && (x) <= 4800 + (256 * 5) ? ((x) - 4800) / 5) : \
 __builtin_const_p(x) ? BUILD_BUG_ON(1) : 0)

[PATCH net-next] liquidio: fix VF incorrectly indicating that it successfully set its VLAN

[Felix Manlunas]

For security reasons, NIC firmware does not allow VF to set its VLAN if PF
set it already.  Firmware allows VF to set its VLAN if PF did not set it.
After the VF instructs the firmware to set the VLAN, VF always indicates
(via return 0) that the operation is successful--even for the times when it
isn't.

Put in a mechanism for the VF's set VLAN function to receive the firmware
response code, then make that function return -EPERM if the firmware
forbids the operation.

Make that mechanism available for other functions that may, in the future,
be interested in receiving the response code from the firmware.  That
mechanism involves adding new fields to struct octnic_ctrl_pkt, so make all
users of struct octnic_ctrl_pkt initialize the struct to zero before using
it; otherwise, the mechanism might act on uninitialized garbage.

Signed-off-by: Felix Manlunas 
Signed-off-by: Derek Chickles 
---
 drivers/net/ethernet/cavium/liquidio/lio_core.c| 11 +++
 drivers/net/ethernet/cavium/liquidio/lio_main.c|  4 
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 19 ++-
 drivers/net/ethernet/cavium/liquidio/octeon_nic.c  | 10 ++
 drivers/net/ethernet/cavium/liquidio/octeon_nic.h  |  4 
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_core.c 
b/drivers/net/ethernet/cavium/liquidio/lio_core.c
index 08676df..796c2cb 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_core.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_core.c
@@ -127,6 +127,17 @@ void liquidio_link_ctrl_cmd_completion(void *nctrl_ptr)
struct octeon_device *oct = lio->oct_dev;
u8 *mac;
 
+   if (nctrl->completion && nctrl->response_code) {
+   /* Signal whoever is interested that the response code from the
+* firmware has arrived.
+*/
+   WRITE_ONCE(*nctrl->response_code, nctrl->status);
+   complete(nctrl->completion);
+   }
+
+   if (nctrl->status)
+   return;
+
switch (nctrl->ncmd.s.cmd) {
case OCTNET_CMD_CHANGE_DEVFLAGS:
case OCTNET_CMD_SET_MULTI_LIST:
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c 
b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index a8426d3..afa816e 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3472,6 +3472,8 @@ static int liquidio_set_rxcsum_command(struct net_device 
*netdev, int command,
struct octnic_ctrl_pkt nctrl;
int ret = 0;
 
+   memset(, 0, sizeof(struct octnic_ctrl_pkt));
+
nctrl.ncmd.u64 = 0;
nctrl.ncmd.s.cmd = command;
nctrl.ncmd.s.param1 = rx_cmd;
@@ -3505,6 +3507,8 @@ static int liquidio_vxlan_port_command(struct net_device 
*netdev, int command,
struct octnic_ctrl_pkt nctrl;
int ret = 0;
 
+   memset(, 0, sizeof(struct octnic_ctrl_pkt));
+
nctrl.ncmd.u64 = 0;
nctrl.ncmd.s.cmd = command;
nctrl.ncmd.s.more = vxlan_cmd_bit;
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c 
b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 174d748..34c7782 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -2484,6 +2484,8 @@ liquidio_vlan_rx_add_vid(struct net_device *netdev,
struct lio *lio = GET_LIO(netdev);
struct octeon_device *oct = lio->oct_dev;
struct octnic_ctrl_pkt nctrl;
+   struct completion compl;
+   u16 response_code;
int ret = 0;
 
memset(, 0, sizeof(struct octnic_ctrl_pkt));
@@ -2495,14 +2497,25 @@ liquidio_vlan_rx_add_vid(struct net_device *netdev,
nctrl.wait_time = 100;
nctrl.netpndev = (u64)netdev;
nctrl.cb_fn = liquidio_link_ctrl_cmd_completion;
+   init_completion();
+   nctrl.completion = 
+   nctrl.response_code = _code;
 
ret = octnet_send_nic_ctrl_pkt(lio->oct_dev, );
if (ret < 0) {
dev_err(>pci_dev->dev, "Add VLAN filter failed in core 
(ret: 0x%x)\n",
ret);
+   return -EIO;
}
 
-   return ret;
+   if (!wait_for_completion_timeout(,
+msecs_to_jiffies(nctrl.wait_time)))
+   return -EPERM;
+
+   if (READ_ONCE(response_code))
+   return -EPERM;
+
+   return 0;
 }
 
 static int
@@ -2547,6 +2560,8 @@ static int liquidio_set_rxcsum_command(struct net_device 
*netdev, int command,
struct octnic_ctrl_pkt nctrl;
int ret = 0;
 
+   memset(, 0, sizeof(struct octnic_ctrl_pkt));
+
nctrl.ncmd.u64 = 0;
nctrl.ncmd.s.cmd = command;
nctrl.ncmd.s.param1 = rx_cmd;
@@ -2579,6 +2594,8 @@ static int liquidio_vxlan_port_command(struct net_device 
*netdev, int command,
struct octnic_ctrl_pkt 

Documentation error for checksum offload check ?

[Benjamin Herrenschmidt]

I noticed in both Documentation/networking/checksum-offload.txt
and include/linux/skbuff.h reference to helpers

skb_csum_off_chk*

Now, I can't find anything like that with grep ... :-)

Am I missing something ?

Cheers,
Ben.

linux-next: manual merge of the net-next tree with the vfs tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the net-next tree got a conflict in:

  drivers/net/ethernet/ibm/ibmvnic.c

between commit:

  98998345a579 ("ibmvnic: fix kstrtoul, copy_from_user and copy_to_user misuse")

from the vfs tree and commit:

  e704f0434ea6 ("ibmvnic: Remove debugfs support")

from the net-next tree.

I fixed it up (the latter removed all the code modified by the former,
so I just did that) and can carry the fix as necessary. This is now fixed
as far as linux-next is concerned, but any non trivial conflicts should
be mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the net-next tree with the net tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the net-next tree got a conflict in:

  net/sched/sch_generic.c

between commit:

  92f9170621a1 ("net_sched: check noop_qdisc before qdisc_hash_add()")

from the net tree and commit:

  49b499718fa1 ("net: sched: make default fifo qdiscs appear in the dump")

from the net-next tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc net/sched/sch_generic.c
index 1a2f9e964330,3e64d23e098c..
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@@ -794,8 -794,8 +794,8 @@@ static void attach_default_qdiscs(struc
}
}
  #ifdef CONFIG_NET_SCHED
 -  if (dev->qdisc)
 +  if (dev->qdisc != _qdisc)
-   qdisc_hash_add(dev->qdisc);
+   qdisc_hash_add(dev->qdisc, false);
  #endif
  }
  

Re: [PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro

[Matthias Kaehlcke]

Hi Joe,

El Thu, Apr 06, 2017 at 02:29:20PM -0700 Joe Perches ha dit:

> On Thu, 2017-04-06 at 14:21 -0700, Matthias Kaehlcke wrote:
> > The macro results are assigned to u8 variables/fields. Adding the cast
> > fixes plenty of clang warnings about "implicit conversion from 'int' to
> > 'u8'".
> > 
> > Signed-off-by: Matthias Kaehlcke 
> > ---
> >  drivers/net/wireless/ath/ath9k/eeprom.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h 
> > b/drivers/net/wireless/ath/ath9k/eeprom.h
> > index 30bf722e33ed..31390af6c33e 100644
> > --- a/drivers/net/wireless/ath/ath9k/eeprom.h
> > +++ b/drivers/net/wireless/ath/ath9k/eeprom.h
> > @@ -106,7 +106,7 @@
> >  #define AR9285_RDEXT_DEFAULT0x1F
> >  
> >  #define ATH9K_POW_SM(_r, _s)   (((_r) & 0x3f) << (_s))
> > -#define FREQ2FBIN(x, y)((y) ? ((x) - 2300) : (((x) - 4800) / 
> > 5))
> > +#define FREQ2FBIN(x, y)(u8)((y) ? ((x) - 2300) : (((x) - 4800) 
> > / 5))
> 
> Maybe better to use:
> 
> static inline u8 FREQ2FBIN(int x, int y)
> {
>   if (y)
>   return x - 2300;
>   return (x - 4800) / 5;
> }

Thanks for your suggestion! Unfortunately in this case an inline
function is not suitable since FREQ2FBIN() is mostly used for
structure initialization:

static const struct ar9300_eeprom ar9300_default = {
...
.calFreqPier2G = {
FREQ2FBIN(2412, 1),
FREQ2FBIN(2437, 1),
FREQ2FBIN(2472, 1),
},
...

Cheers

Matthias

[PATCH v2] mac80211: Fix clang warning about constant operand in logical operation

[Matthias Kaehlcke]

When clang detects a non-boolean constant in a logical operation it
generates a 'constant-logical-operand' warning. In
ieee80211_try_rate_control_ops_get() the result of strlen()
is used in a logical operation, clang resolves the expression to an
(integer) constant at compile time when clang's builtin strlen function
is used.

Change the condition to check for strlen() > 0 to make the constant
operand boolean and thus avoid the warning.

Signed-off-by: Matthias Kaehlcke 
---
Changes in v2:
- Changed expression to strlen() > 0 to make constant operand boolean
  instead of splitting up condition
- Updated commit message

 net/mac80211/rate.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 206698bc93f4..694faf6ab574 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -173,9 +173,11 @@ ieee80211_rate_control_ops_get(const char *name)
/* try default if specific alg requested but not found */
ops = 
ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
 
-   /* try built-in one if specific alg requested but not found */
-   if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT))
+   /* Note: check for > 0 is intentional to avoid clang warning */
+   if (!ops && (strlen(CONFIG_MAC80211_RC_DEFAULT) > 0))
+   /* try built-in one if specific alg requested but not found */
ops = 
ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
+
kernel_param_unlock(THIS_MODULE);
 
return ops;
-- 
2.12.2.715.g7642488e1d-goog

Re: [PATCH][-next] nfp: don't dereference a null nn->eth_port to print a warning

[Jakub Kicinski]

On Thu,  6 Apr 2017 13:54:35 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> On the case where nn->eth_port is null the warning message
> is printing the port by dereferencing this null pointer.
> Remove the deference to avoid a crash when printing the
> warning message.
> 
> Detected by CoverityScan, CID#1426198 ("Dereference after null check")
> 
> Fixes: ce22f5a2cbe3c627 ("nfp: separate high level and low level NSP headers")
> Signed-off-by: Colin Ian King 

Acked-by: Jakub Kicinski 

Thanks!

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Matthias Kaehlcke]

El Fri, Apr 07, 2017 at 12:51:57AM +0200 Johannes Berg ha dit:

> On Thu, 2017-04-06 at 15:42 -0700, Matthias Kaehlcke wrote:
> > 
> > Thanks, it would also require to move the initialization of
> > ieee80211_default_rc_algo into an ifdef. If you can live with such a
> > solution I'm happy to change it.
> 
> I think that'd be something I can live with, yeah.
> 
> > >   git grep 'IS_ENABLED(' | grep '&&'
> > 
> > Indeed the warning is not triggered by these constructs. It seems
> > clang only emits the warning when the constant operand is not
> > boolean.
> 
> That points to just adding "> 0" to the condition here as another
> alternative solution, I guess? With a comment to make sure it's not
> removed again, that'd seem like the best thing to do.

Good point, that's more digestible. I'll send an updated change soon.

Matthias

Re: [PATCH] net: ipv4: netfilter: Remove unused function nf_nat_need_gre()

[Pablo Neira Ayuso]

On Sat, Apr 01, 2017 at 07:06:33PM +0530, simran singhal wrote:
> The function nf_nat_need_gre() on being called, simply returns
> back. The function doesn't have FIXME code around.
> Hence, nf_nat_need_gre() and its calls have been removed.
> 
> Signed-off-by: simran singhal 
> ---
>  net/ipv4/netfilter/nf_nat_pptp.c  | 2 --
>  net/ipv4/netfilter/nf_nat_proto_gre.c | 6 --
>  2 files changed, 8 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/nf_nat_pptp.c 
> b/net/ipv4/netfilter/nf_nat_pptp.c
> index b3ca21b..747e737 100644
> --- a/net/ipv4/netfilter/nf_nat_pptp.c
> +++ b/net/ipv4/netfilter/nf_nat_pptp.c
> @@ -282,8 +282,6 @@ pptp_inbound_pkt(struct sk_buff *skb,
>  
>  static int __init nf_nat_helper_pptp_init(void)
>  {
> - nf_nat_need_gre();
> -
>   BUG_ON(nf_nat_pptp_hook_outbound != NULL);
>   RCU_INIT_POINTER(nf_nat_pptp_hook_outbound, pptp_outbound_pkt);
>  
> diff --git a/net/ipv4/netfilter/nf_nat_proto_gre.c 
> b/net/ipv4/netfilter/nf_nat_proto_gre.c
> index edf0500..c020a4d 100644
> --- a/net/ipv4/netfilter/nf_nat_proto_gre.c
> +++ b/net/ipv4/netfilter/nf_nat_proto_gre.c
> @@ -142,9 +142,3 @@ static void __exit nf_nat_proto_gre_fini(void)
>  
>  module_init(nf_nat_proto_gre_init);
>  module_exit(nf_nat_proto_gre_fini);
> -
> -void nf_nat_need_gre(void)
> -{
> - return;
> -}
> -EXPORT_SYMBOL_GPL(nf_nat_need_gre);

There is a good reason why we have this :)

By digging out and doing a bit of software archeology work via the
'git annotate' you know this triggers an explicit module dependency.

Re: [Outreachy kernel] [PATCH] net: netfilter: Remove typedef from "typedef struct bitstr_t".

[Pablo Neira Ayuso]

On Tue, Mar 28, 2017 at 11:54:13PM +0530, Arushi Singhal wrote:
> This patch removes typedefs from struct and renames it from "typedef struct
> bitstr_t" to "struct bitstr" as per kernel coding standards."
> 
> Signed-off-by: Arushi Singhal 
> ---
>  net/netfilter/nf_conntrack_h323_asn1.c | 80 
> +-
>  1 file changed, 40 insertions(+), 40 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c 
> b/net/netfilter/nf_conntrack_h323_asn1.c
> index fb8cf238a76f..4502c0d6071d 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -91,41 +91,41 @@ struct field {
>  };
>  
>  /* Bit Stream */
> -typedef struct {
> +struct bitstr {

Same thing here, I'd suggest you use 'struct h323_bitstr' instead.

Thanks!

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Johannes Berg]

On Thu, 2017-04-06 at 15:42 -0700, Matthias Kaehlcke wrote:
> 
> Thanks, it would also require to move the initialization of
> ieee80211_default_rc_algo into an ifdef. If you can live with such a
> solution I'm happy to change it.

I think that'd be something I can live with, yeah.

> > git grep 'IS_ENABLED(' | grep '&&'
> 
> Indeed the warning is not triggered by these constructs. It seems
> clang only emits the warning when the constant operand is not
> boolean.

That points to just adding "> 0" to the condition here as another
alternative solution, I guess? With a comment to make sure it's not
removed again, that'd seem like the best thing to do.

johannes

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Matthias Kaehlcke]

El Thu, Apr 06, 2017 at 11:12:25PM +0200 Johannes Berg ha dit:

> On Thu, 2017-04-06 at 12:24 -0700, Matthias Kaehlcke wrote:
> 
> > I agree that the code looks worse :( I hoped to find a fix using a
> > preprocessor condition but wasn't successful.
> 
> It's actually easy - just remove the 'default ""' from Kconfig, and
> then the symbol won't be defined at all if it doesn't get a proper
> value. Then you can ifdef the whole thing.

Thanks, it would also require to move the initialization of
ieee80211_default_rc_algo into an ifdef. If you can live with such a
solution I'm happy to change it.

> > Some projects (like Chrome OS) build their kernel with all warnings
> > being treated as errors. Besides changing the 'offending' code the
> > alternatives are to disable the warning completely or to tell clang
> > not to use the builtin(s). IMO changing the code is the preferable
> > solution, especially since this is so far the only occurrence of the
> > warning that I have encountered.
> > 
> > I used goto instead of nested ifs since other functions in this file
> > use the same pattern. If nested ifs are preferred I can change that.
> 
> I don't really buy either argument. The warning is simply bogus - I'm
> very surprised you don't hit it with more similar macros or cases, like
> for example CONFIG_ENABLED(). Try
> 
>   git grep 'IS_ENABLED(' | grep '&&'
> 
> and you'll find lots of places that seem like they should trigger this
> warning.

Indeed the warning is not triggered by these constructs. It seems
clang only emits the warning when the constant operand is not boolean.

> You're advocating to make the code worse - not very significantly in
> this case, but still - just to quiet a compiler warning.

For Chrome OS we need to quiet the warning in one way or another,
otherwise our builds will fail due to warnings being treated as
errors. I'm reluctant to disable the warning completely since it can
be useful to detect certain errors, e.g. the use of a logical operator
when bitwise was intended.

And yes, in this case I would favor the slight deterioration of a
small section of code over losing a potentially useful check on the
entire kernel code.

I agree that this is a bit of a corner case, the code is definitely
not buggy by itself, ideally clang would detect that the constant is
a result of its own optimization and skip the warning.

Obviously we can always apply a patch locally, but ideally we try to
upstream changes that seem of general use so that others and our
future selves can benefit from it.

I have no intention to insist on this, we can live with a local patch
if you don't think it is useful.

Cheers

Matthias

Re: [PATCH 00/10] ftgmac: Rework batch 2 - RX path

[David Miller]

From: Benjamin Herrenschmidt 
Date: Thu,  6 Apr 2017 11:02:42 +1000

> This is the second batch of updates to the ftgmac100 driver.
> 
> This one tackles the RX path of the driver, simplifying
> it greatly to match common practice while significantly
> increasing the performance.
> 
> (The bulk of the performance gains of my series will be
> provided by the TX path improvements, notably fragmented
> sends, these will be in the next batch).

Looks good, series applied, thanks.

Re: tcpprobe display format for snd_nxt and snd_una

[hiren panchasara]

On 04/06/17 at 05:45P, Stephen Hemminger wrote:
> On Wed, 5 Apr 2017 12:40:09 -0700
> hiren panchasara  wrote:
> 
> > (New to linux and first-time poster so please guide me if needed.)
> >  
> > Upon using tcpprobe I realized that it prints snd_nxt and snd_una as hex
> > which makes it harder to read and compare with tcpdump for example.
> >  
> > Not sure if that is intentional. If not, a simple patch like this would
> > print them as decimals.
> >  
> > [PATCH] Display snd_nxt and snd_una as decimals for better
> >  readability.
> >  
> > ---
> >  net/ipv4/tcp_probe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >  
> > diff --git a/net/ipv4/tcp_probe.c b/net/ipv4/tcp_probe.c
> > index f6c50af..a8e66c1 100644
> > --- a/net/ipv4/tcp_probe.c
> > +++ b/net/ipv4/tcp_probe.c
> > @@ -191,7 +191,7 @@ static int tcpprobe_sprint(char *tbuf, int n)
> > = ktime_to_timespec64(ktime_sub(p->tstamp, 
> > tcp_probe.start));
> >  
> > return scnprintf(tbuf, n,
> > -   "%lu.%09lu %pISpc %pISpc %d %#x %#x %u %u %u %u 
> > %u\n",
> > +   "%lu.%09lu %pISpc %pISpc %d %u %u %u %u %u %u %u\n",
> > (unsigned long)ts.tv_sec,
> > (unsigned long)ts.tv_nsec,
> > >src, >dst, p->length, p->snd_nxt, p->snd_una,
> > --
> >  
> > Let me know if I am missing something obvious.
> 
> The output of tcpprobe is intended for consumption by programs.
> Changing the output format would be considered a kernel ABI breakage which
> is something Linux tries not to do. Therefore I would prefer it
> not be changed.
Ah, I see.

> Sorry if this is inconvenient for you but breaking other
> users scripts would be a bigger problem.
No, I don't have any scripts depending on this. I just want to see this
o/p with naked eye and compare it with a pcap to see any possible
abnormalities of a "bad" connection. I guess I'd have to no write a
wrapper on top of this to achieve whhat

Can you point me to the programs/tools that consume this data? I am new
to Linux and want to learn tools that can help in troubleshooting. (In
FreeBSD land I can trivially dump this data with dtrace and I assume
something like that exists here too.)

I can always have a wrapper on top of this to do the conversion. Trivial
but inconvenient. :-)

> 
> Also, your patch email is not formatted with subject [RFC] or [PATCH]
> and is missing signed-off-by.

Apologies for that. I'll make sure I do the right thing next time as
this looks like a non-starter because of ABI concerns.

Thanks for taking time and responding.
Cheers,
Hiren


pgpTnglCqKjg8.pgp
Description: PGP signature

Re: [Patch net] net_sched: replace yield() with cond_resched()

[Stephen Hemminger]

On Thu, 06 Apr 2017 03:54:19 +0200
Mike Galbraith  wrote:

> On Wed, 2017-04-05 at 16:42 -0700, Cong Wang wrote:
> > On Tue, Apr 4, 2017 at 10:56 PM, Mike Galbraith  wrote:  
> > > On Tue, 2017-04-04 at 22:19 -0700, Cong Wang wrote:  
> > > > On Tue, Apr 4, 2017 at 8:55 PM, Mike Galbraith  wrote:  
> > >   
> > > > > That won't help, cond_resched() has the same impact upon a lone
> > > > > SCHED_FIFO task as yield() does.. none.  
> > > > 
> > > > Hmm? In the comment you quote:
> > > > 
> > > >  * If you want to use yield() to wait for something, use wait_event().
> > > >  * If you want to use yield() to be 'nice' for others, use 
> > > > cond_resched().
> > > > 
> > > > So if cond_resched() doesn't help, why this misleading comment?  
> > > 
> > > This is not an oh let's be nice guys thing, it's a perfect match of...
> > > 
> > > 
> > >  * while (!event)
> > >  *  yield();  
> > > (/copy/paste>  
> > > 
> > > ..get off the CPU until this happens thing.  With nobody to yield the C
> > > PU to, some_qdisc_is_busy() will remain true forever more.  
> > 
> > 
> > This is exactly the misleading part, a while-loop waiting for an event
> > can always be a be-nice-for-others thing, because if not we can just
> > spin as a spinlock.  
> 
> Ah, but the kworker _is_ spinning on a 'lock' or sorts, starving the
> 'owner', ergo this polling loop fails the 'may be nice' litmus test. 
>  No polling loop is safe without a guarantee that the polling thread
> cannot block the loop breaking event.
> 
>   -Mike

Why not replace yield with msleep(1) which gets rid of the inversion
issues?

Re: [Outreachy kernel] [PATCH v3] net: netfilter: Add nfnl_msg_type() helper function

[Pablo Neira Ayuso]

Hi,

On Tue, Mar 28, 2017 at 10:27:32PM +0530, Arushi Singhal wrote:
> To remove complexity of code the function is added in nfnetlink.h
> to make code more clear and readable.
> This is opencoded in a way that makes it error prone for future
> netfilter netlink subsystems.
> 
> Signed-off-by: Arushi Singhal 
> ---
> changes in v3
>  -make the subject more clear.
> 
>  include/linux/netfilter/nfnetlink.h  |  6 ++
>  net/netfilter/nf_conntrack_netlink.c | 12 +++-
>  net/netfilter/nfnetlink_acct.c   |  2 +-
>  net/netfilter/nfnetlink_cthelper.c   |  2 +-
>  net/netfilter/nfnetlink_cttimeout.c  |  4 ++--
>  5 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/netfilter/nfnetlink.h 
> b/include/linux/netfilter/nfnetlink.h
> index 1b49209dd5c7..9a36a7c3145d 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -50,6 +50,12 @@ static inline bool lockdep_nfnl_is_held(__u8 subsys_id)
>  {
>   return true;
>  }
> +
> +static inline u16 nfnl_msg_type(u8 subsys, u8 msg_type)
> +{
> + return subsys << 8 | msg_type;
> +}

This is not right. You have placed this new function definition inside
the CONFIG_PROVE_LOCKING.

So this is only defined iff CONFIG_PROVE_LOCKING is set on.

>  #endif /* CONFIG_PROVE_LOCKING */
>  
>  /*
> diff --git a/net/netfilter/nf_conntrack_netlink.c 
> b/net/netfilter/nf_conntrack_netlink.c
> index aa344c5868c5..67f6f88a3e92 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -467,7 +467,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32 
> seq, u32 type,
>   struct nlattr *nest_parms;
>   unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>  
> - event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW;

I can find many more spots to be replaced via:

git grep NFNL_SUBSYS_ net/netfilter/

Patch attached.
>From 1f03a770eb030480968c9cb29be85e3d1cbadf3e Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso 
Date: Tue, 28 Mar 2017 22:27:32 +0530
Subject: [PATCH] netfilter: Add nfnl_msg_type() helper function

Add and use nfnl_msg_type() function to replace opencoded nfnetlink
message type. I suggested this change, Arushi Singhal made an initial
patch to address this but was missing several spots.

Signed-off-by: Pablo Neira Ayuso 
---
 include/linux/netfilter/nfnetlink.h  |  5 +
 net/netfilter/ipset/ip_set_core.c|  2 +-
 net/netfilter/nf_conntrack_netlink.c | 16 +---
 net/netfilter/nf_tables_api.c| 17 -
 net/netfilter/nf_tables_trace.c  |  3 ++-
 net/netfilter/nfnetlink_acct.c   |  2 +-
 net/netfilter/nfnetlink_cthelper.c   |  2 +-
 net/netfilter/nfnetlink_cttimeout.c  |  4 ++--
 net/netfilter/nfnetlink_log.c|  2 +-
 net/netfilter/nfnetlink_queue.c  |  2 +-
 net/netfilter/nft_compat.c   |  2 +-
 11 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 1b49209dd5c7..996711d8a7b4 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -41,6 +41,11 @@ int nfnetlink_set_err(struct net *net, u32 portid, u32 group, int error);
 int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u32 portid,
 		  int flags);
 
+static inline u16 nfnl_msg_type(u8 subsys, u8 msg_type)
+{
+	return subsys << 8 | msg_type;
+}
+
 void nfnl_lock(__u8 subsys_id);
 void nfnl_unlock(__u8 subsys_id);
 #ifdef CONFIG_PROVE_LOCKING
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index c296f9b606d4..731ba9c0cf9b 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -769,7 +769,7 @@ start_msg(struct sk_buff *skb, u32 portid, u32 seq, unsigned int flags,
 	struct nlmsghdr *nlh;
 	struct nfgenmsg *nfmsg;
 
-	nlh = nlmsg_put(skb, portid, seq, cmd | (NFNL_SUBSYS_IPSET << 8),
+	nlh = nlmsg_put(skb, portid, seq, nfnl_msg_type(NFNL_SUBSYS_IPSET, cmd),
 			sizeof(*nfmsg), flags);
 	if (!nlh)
 		return NULL;
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index cd0a6d270ebe..773d2187a5ea 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -467,7 +467,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	struct nlattr *nest_parms;
 	unsigned int flags = portid ? NLM_F_MULTI : 0, event;
 
-	event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW);
+	event = nfnl_msg_type(NFNL_SUBSYS_CTNETLINK, IPCTNL_MSG_CT_NEW);
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
 	if (nlh == NULL)
 		goto nlmsg_failure;
@@ -652,7 +652,7 @@ ctnetlink_conntrack_event(unsigned int events, struct nf_ct_event *item)
 	if (skb == NULL)
 		goto errout;
 
-	type |= NFNL_SUBSYS_CTNETLINK << 8;
+	type = 

[PATCH net-next 1/1] netvsc: Initialize all channel related state prior to opening the channel

[kys]

From: K. Y. Srinivasan 

Prior to opening the channel we should have all the state setup to handle
interrupts. The current code does not do that; fix the bug. This bug
can result in faults in the interrupt path.
 
Signed-off-by: K. Y. Srinivasan 
---
 drivers/net/hyperv/netvsc.c   |   27 +++
 drivers/net/hyperv/rndis_filter.c |5 +++--
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index e998e2f..7ab06b3 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1289,6 +1289,21 @@ int netvsc_device_add(struct hv_device *device,
 */
set_channel_read_mode(device->channel, HV_CALL_ISR);
 
+   /* If we're reopening the device we may have multiple queues, fill the
+* chn_table with the default channel to use it before subchannels are
+* opened.
+* Initialize the channel state before we open;
+* we can be interrupted as soon as we open the channel.
+*/
+
+   for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
+   struct netvsc_channel *nvchan = _device->chan_table[i];
+
+   nvchan->channel = device->channel;
+   netif_napi_add(ndev, >napi,
+  netvsc_poll, NAPI_POLL_WEIGHT);
+   }
+
/* Open the channel */
ret = vmbus_open(device->channel, ring_size * PAGE_SIZE,
 ring_size * PAGE_SIZE, NULL, 0,
@@ -1303,18 +1318,6 @@ int netvsc_device_add(struct hv_device *device,
/* Channel is opened */
netdev_dbg(ndev, "hv_netvsc channel opened successfully\n");
 
-   /* If we're reopening the device we may have multiple queues, fill the
-* chn_table with the default channel to use it before subchannels are
-* opened.
-*/
-   for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
-   struct netvsc_channel *nvchan = _device->chan_table[i];
-
-   nvchan->channel = device->channel;
-   netif_napi_add(ndev, >napi,
-  netvsc_poll, NAPI_POLL_WEIGHT);
-   }
-
/* Enable NAPI handler for init callbacks */
napi_enable(_device->chan_table[0].napi);
 
diff --git a/drivers/net/hyperv/rndis_filter.c 
b/drivers/net/hyperv/rndis_filter.c
index 9835825..1e9445b 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1007,12 +1007,13 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
 */
set_channel_read_mode(new_sc, HV_CALL_ISR);
 
+   /* Set the channel before opening.*/
+   nvchan->channel = new_sc;
+
ret = vmbus_open(new_sc, nvscdev->ring_size * PAGE_SIZE,
 nvscdev->ring_size * PAGE_SIZE, NULL, 0,
 netvsc_channel_cb, nvchan);
 
-   if (ret == 0)
-   nvchan->channel = new_sc;
 
napi_enable(>napi);
 
-- 
1.7.1

Re: [PATCH 00/19] pull request for net-next: batman-adv 2017-04-06

[David Miller]

From: Simon Wunderlich 
Date: Thu,  6 Apr 2017 16:07:22 +0200

> here is our feature/cleanup pull request of batman-adv to go into net-next.
> 
> Please pull or let me know of any problem!

Pulled, thanks Simon.

Re: [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts

[Benjamin Herrenschmidt]

On Thu, 2017-04-06 at 12:46 -0700, Florian Fainelli wrote:
> > I thought a while ago we could add some dev flag to prevent the link
> > watch from doing that, but never got to look into it myself and
> > apparently neither did Gavin.
> 
> It sounds like a similar situation to e.g: 802.1x, you want to let some
> frames make it through and you need a working link for that, but you
> can't quite flag the device as UP entirely. Maybe you can find a way to
> re-use IFF_LOWER_UP to that purpose and only set that flag based on the
> NC-SI frames indicating link state?

Possibly yes, I wasn't familiar with the difference between 
"carrier" and IFF_LOWER_UP so far. I'll have a look or have Gavin do
so.

It would be nice to reflect that properly so things like fallover or
bonding do the right thing. On the other hand I yet have to see a
system where the BMC has more than one NC-SI link to a NIC (what they
usually have is that NC-SI is connected to multiple NICs which is a
different matter and should be handled within the NC-SI stack).

Anyway, I'll look into it further down the track along with other
improvements I'd like to do to the NC-SI stack such as properly setting
up multicast and vlan filters in the peer NIC and various cleanups.

Cheers,
Ben.

Re: tcpprobe display format for snd_nxt and snd_una

[Stephen Hemminger]

On Wed, 5 Apr 2017 12:40:09 -0700
hiren panchasara  wrote:

> (New to linux and first-time poster so please guide me if needed.)
>  
> Upon using tcpprobe I realized that it prints snd_nxt and snd_una as hex
> which makes it harder to read and compare with tcpdump for example.
>  
> Not sure if that is intentional. If not, a simple patch like this would
> print them as decimals.
>  
> [PATCH] Display snd_nxt and snd_una as decimals for better
>  readability.
>  
> ---
>  net/ipv4/tcp_probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  
> diff --git a/net/ipv4/tcp_probe.c b/net/ipv4/tcp_probe.c
> index f6c50af..a8e66c1 100644
> --- a/net/ipv4/tcp_probe.c
> +++ b/net/ipv4/tcp_probe.c
> @@ -191,7 +191,7 @@ static int tcpprobe_sprint(char *tbuf, int n)
> = ktime_to_timespec64(ktime_sub(p->tstamp, tcp_probe.start));
>  
> return scnprintf(tbuf, n,
> -   "%lu.%09lu %pISpc %pISpc %d %#x %#x %u %u %u %u %u\n",
> +   "%lu.%09lu %pISpc %pISpc %d %u %u %u %u %u %u %u\n",
> (unsigned long)ts.tv_sec,
> (unsigned long)ts.tv_nsec,
> >src, >dst, p->length, p->snd_nxt, p->snd_una,
> --
>  
> Let me know if I am missing something obvious.

The output of tcpprobe is intended for consumption by programs.
Changing the output format would be considered a kernel ABI breakage which
is something Linux tries not to do. Therefore I would prefer it
not be changed. Sorry if this is inconvenient for you but breaking other
users scripts would be a bigger problem.

Also, your patch email is not formatted with subject [RFC] or [PATCH]
and is missing signed-off-by.

Re: [PATCH 4/4] PCI: remove pci_enable_msix

[Bjorn Helgaas]

On Thu, Apr 06, 2017 at 02:24:48PM +0200, Christoph Hellwig wrote:
> Unused now that all callers switched to pci_alloc_irq_vectors.
> 
> Signed-off-by: Christoph Hellwig 

I already acked this, but I can do it again :)
(https://lkml.kernel.org/r/20170330230913.ga3...@bhelgaas-glaptop.roam.corp.google.com)

Acked-by: Bjorn Helgaas 

> ---
>  drivers/pci/msi.c   | 21 -
>  include/linux/pci.h |  4 
>  2 files changed, 25 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d571bc330686..0042c365b29b 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -973,27 +973,6 @@ static int __pci_enable_msix(struct pci_dev *dev, struct 
> msix_entry *entries,
>   return msix_capability_init(dev, entries, nvec, affd);
>  }
>  
> -/**
> - * pci_enable_msix - configure device's MSI-X capability structure
> - * @dev: pointer to the pci_dev data structure of MSI-X device function
> - * @entries: pointer to an array of MSI-X entries (optional)
> - * @nvec: number of MSI-X irqs requested for allocation by device driver
> - *
> - * Setup the MSI-X capability structure of device function with the number
> - * of requested irqs upon its software driver call to request for
> - * MSI-X mode enabled on its hardware device function. A return of zero
> - * indicates the successful configuration of MSI-X capability structure
> - * with new allocated MSI-X irqs. A return of < 0 indicates a failure.
> - * Or a return of > 0 indicates that driver request is exceeding the number
> - * of irqs or MSI-X vectors available. Driver should use the returned value 
> to
> - * re-send its request.
> - **/
> -int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int 
> nvec)
> -{
> - return __pci_enable_msix(dev, entries, nvec, NULL);
> -}
> -EXPORT_SYMBOL(pci_enable_msix);
> -
>  void pci_msix_shutdown(struct pci_dev *dev)
>  {
>   struct msi_desc *entry;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index eb3da1a04e6c..82dec36845e6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1300,7 +1300,6 @@ int pci_msi_vec_count(struct pci_dev *dev);
>  void pci_msi_shutdown(struct pci_dev *dev);
>  void pci_disable_msi(struct pci_dev *dev);
>  int pci_msix_vec_count(struct pci_dev *dev);
> -int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int 
> nvec);
>  void pci_msix_shutdown(struct pci_dev *dev);
>  void pci_disable_msix(struct pci_dev *dev);
>  void pci_restore_msi_state(struct pci_dev *dev);
> @@ -1330,9 +1329,6 @@ static inline int pci_msi_vec_count(struct pci_dev 
> *dev) { return -ENOSYS; }
>  static inline void pci_msi_shutdown(struct pci_dev *dev) { }
>  static inline void pci_disable_msi(struct pci_dev *dev) { }
>  static inline int pci_msix_vec_count(struct pci_dev *dev) { return -ENOSYS; }
> -static inline int pci_enable_msix(struct pci_dev *dev,
> -   struct msix_entry *entries, int nvec)
> -{ return -ENOSYS; }
>  static inline void pci_msix_shutdown(struct pci_dev *dev) { }
>  static inline void pci_disable_msix(struct pci_dev *dev) { }
>  static inline void pci_restore_msi_state(struct pci_dev *dev) { }
> -- 
> 2.11.0
> 

Re: [PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro

[Joe Perches]

On Thu, 2017-04-06 at 14:21 -0700, Matthias Kaehlcke wrote:
> The macro results are assigned to u8 variables/fields. Adding the cast
> fixes plenty of clang warnings about "implicit conversion from 'int' to
> 'u8'".
> 
> Signed-off-by: Matthias Kaehlcke 
> ---
>  drivers/net/wireless/ath/ath9k/eeprom.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h 
> b/drivers/net/wireless/ath/ath9k/eeprom.h
> index 30bf722e33ed..31390af6c33e 100644
> --- a/drivers/net/wireless/ath/ath9k/eeprom.h
> +++ b/drivers/net/wireless/ath/ath9k/eeprom.h
> @@ -106,7 +106,7 @@
>  #define AR9285_RDEXT_DEFAULT0x1F
>  
>  #define ATH9K_POW_SM(_r, _s) (((_r) & 0x3f) << (_s))
> -#define FREQ2FBIN(x, y)  ((y) ? ((x) - 2300) : (((x) - 4800) / 
> 5))
> +#define FREQ2FBIN(x, y)  (u8)((y) ? ((x) - 2300) : (((x) - 4800) 
> / 5))

Maybe better to use:

static inline u8 FREQ2FBIN(int x, int y)
{
if (y)
return x - 2300;
return (x - 4800) / 5;
}

Re: [PATCH net-next 0/8] qed: Misc cleanups and fixes

[David Miller]

From: Yuval Mintz 
Date: Thu, 6 Apr 2017 15:58:27 +0300

> Patches #1 and #2 revolve around register access performed by driver;
> The first merely adds some debug, while the second does some fixing
> of incorrect PTT usage as well as preventing issues similar to those
> fixed by 6f437d431930 ("qed: Don't use attention PTT for configuring BW").
> 
> Patch #3 better configures HW for architecture where cacheline isn't 64B.
> 
> Patches #4-#8 all affect iSCSI related functionaility - 
> adding statistics information [both to driver & management firmware],
> passing information on number of resources to qedi, and simplifying
> the Out-of-order implementation in SW.

Series applied, thanks Yuval.

Re: [PATCH net-next 0/7] rxrpc: Miscellany

[David Miller]

From: David Howells <dhowe...@redhat.com>
Date: Thu, 06 Apr 2017 11:22:14 +0100

> Here's a set of patches that make some minor changes to AF_RXRPC:
 ...
> Tagged thusly:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
>   rxrpc-rewrite-20170406

Pulled, thanks David.

[PATCH] ath9k: Add cast to u8 to FREQ2FBIN macro

[Matthias Kaehlcke]

The macro results are assigned to u8 variables/fields. Adding the cast
fixes plenty of clang warnings about "implicit conversion from 'int' to
'u8'".

Signed-off-by: Matthias Kaehlcke 
---
 drivers/net/wireless/ath/ath9k/eeprom.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h 
b/drivers/net/wireless/ath/ath9k/eeprom.h
index 30bf722e33ed..31390af6c33e 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/eeprom.h
@@ -106,7 +106,7 @@
 #define AR9285_RDEXT_DEFAULT0x1F
 
 #define ATH9K_POW_SM(_r, _s)   (((_r) & 0x3f) << (_s))
-#define FREQ2FBIN(x, y)((y) ? ((x) - 2300) : (((x) - 4800) / 
5))
+#define FREQ2FBIN(x, y)(u8)((y) ? ((x) - 2300) : (((x) - 4800) 
/ 5))
 #define FBIN2FREQ(x, y)((y) ? (2300 + x) : (4800 + 5 * x))
 #define ath9k_hw_use_flash(_ah)(!(_ah->ah_flags & AH_USE_EEPROM))
 
-- 
2.12.2.715.g7642488e1d-goog

Re: [PATCH 3/4] xfrm: Prepare for CRYPTO_MAX_ALG_NAME expansion

[Steffen Klassert]

On Thu, Apr 06, 2017 at 04:16:10PM +0800, Herbert Xu wrote:
> This patch fixes the xfrm_user code to use the actual array size
> rather than the hard-coded CRYPTO_MAX_ALG_NAME length.  This is
> because the array size is fixed at 64 bytes while we want to increase
> the in-kernel CRYPTO_MAX_ALG_NAME value.
> 
> Signed-off-by: Herbert Xu 

Acked-by: Steffen Klassert 

Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[Steffen Klassert]

On Thu, Apr 06, 2017 at 01:58:32PM -0700, David Miller wrote:
> From: Herbert Xu 
> Date: Thu, 6 Apr 2017 16:15:09 +0800
> 
> > As the final patch depends on all three it would be easiest if
> > we pushed the xfrm patch through the crypto tree.  Steffen/David?
> 
> No objections from me for this going through the crypto tree.

I have no objections too.

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Johannes Berg]

On Thu, 2017-04-06 at 12:24 -0700, Matthias Kaehlcke wrote:

> I agree that the code looks worse :( I hoped to find a fix using a
> preprocessor condition but wasn't successful.

It's actually easy - just remove the 'default ""' from Kconfig, and
then the symbol won't be defined at all if it doesn't get a proper
value. Then you can ifdef the whole thing.

> Some projects (like Chrome OS) build their kernel with all warnings
> being treated as errors. Besides changing the 'offending' code the
> alternatives are to disable the warning completely or to tell clang
> not to use the builtin(s). IMO changing the code is the preferable
> solution, especially since this is so far the only occurrence of the
> warning that I have encountered.
> 
> I used goto instead of nested ifs since other functions in this file
> use the same pattern. If nested ifs are preferred I can change that.

I don't really buy either argument. The warning is simply bogus - I'm
very surprised you don't hit it with more similar macros or cases, like
for example CONFIG_ENABLED(). Try

git grep 'IS_ENABLED(' | grep '&&'

and you'll find lots of places that seem like they should trigger this
warning.

You're advocating to make the code worse - not very significantly in
this case, but still - just to quiet a compiler warning.

johannes

Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[David Miller]

From: Herbert Xu 
Date: Thu, 6 Apr 2017 16:15:09 +0800

> As the final patch depends on all three it would be easiest if
> we pushed the xfrm patch through the crypto tree.  Steffen/David?

No objections from me for this going through the crypto tree.

Re: [PATCH net] sctp: listen on the sock only when it's state is listening or closed

[David Miller]

From: Xin Long 
Date: Thu,  6 Apr 2017 13:10:52 +0800

> Now sctp doesn't check sock's state before listening on it. It could
> even cause changing a sock with any state to become a listening sock
> when doing sctp_listen.
> 
> This patch is to fix it by checking sock's state in sctp_listen, so
> that it will listen on the sock with right state.
> 
> Reported-by: Andrey Konovalov 
> Tested-by: Andrey Konovalov 
> Signed-off-by: Xin Long 

Applied and queued up for -stable.

Re: [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05

[David Miller]

From: Jeff Kirsher 
Date: Thu,  6 Apr 2017 00:54:49 -0700

> This series contains updates to fm10k only.
> 
> Phil Turnbull from Oracle fixes an issue where the argument provided to
> FM10K_REMOVED macro was not what was expecting.
> 
> Jake modifies the driver to replace the bitwise operators and defines with
> a BITMAP and enumeration values to avoid race conditions.  Also future
> proof the driver so that developers do not have to remember to re-size the
> bitmaps when adding new values.  Fixed the wording of a code comment to
> avoid stating that we return a value for a void function.
> 
> Ngai-Mint makes sure that when configuring the receive ring, we make sure
> the receive queue is disabled.  Fixed an issue where interfaces were
> resetting because the transmit mailbox FIFO was becoming full since the
> host was not ready, so ensure the host is ready before queueing up
> mailbox messages.

Pulled, thanks Jeff.

Re: [BUG] kernel oops during bridge creation

[Ido Schimmel]

On Thu, Apr 06, 2017 at 11:26:06PM +0300, Nikolay Aleksandrov wrote:
> Actually making br_vlan_init() idempotent might work, keep the code as-is 
> just init the
> the vlans before the changelink() in newlink(), then the second vlan_init() 
> inside would
> be a no-op, but it will work for the old ioctl and wouldn't require too much 
> change.
> On the downside it will be ugly.

Yep, that can work. I'll try that tomorrow morning and let you know.

Thanks Nik!

Re: [PATCH net-next v5 0/2] L2TP:Adjust intf MTU, add underlay L3, L2 hdrs.

[David Miller]

From: "R. Parameswaran" 
Date: Wed, 5 Apr 2017 17:05:49 -0700 (PDT)

> 
> Existing L2TP kernel code does not derive the optimal MTU for Ethernet
> pseudowires and instead leaves this to a userspace L2TP daemon or
> operator. If an MTU is not specified, the existing kernel code chooses
> an MTU that does not take account of all tunnel header overheads, which
> can lead to unwanted IP fragmentation. When L2TP is used without a
> control plane (userspace daemon), we would prefer that the kernel does a
> better job of choosing a default pseudowire MTU, taking account of all
> tunnel header overheads, including IP header options, if any. This patch
> addresses this.
> 
> Change-set is organized as a two part patch series, with one patch
> introducing a new kernel function to compute the IP overhead on a
> socket, and the other patch using this new kernel function to compute
> the default L2TP MTU for an Ethernet pseudowire.
> 
> Existing code also seems to assume an Ethernet (non-jumbo) underlay. The
> change proposed here uses the PMTU mechanism and the dst entry in the
> L2TP tunnel socket to directly pull up the underlay MTU (as the baseline
> number on top of which the encapsulation headers are factored in).
> An default MTU value of 1500 bytes is assumed as a fallback only if
> this fails. 
> 
> Fixed the kbuild test robot error in the previous posting.

Series applied, thanks.

Re: [PATCH] net: ethernet: wiznet: avoid format string exposure

[David Miller]

From: Kees Cook 
Date: Wed, 5 Apr 2017 14:39:35 -0700

> While unlikely, this makes sure any format strings in the device name
> can't exposure information via the resulting workqueue name.
> 
> Signed-off-by: Kees Cook 

Applied.

Re: [PATCH] qlge: avoid format string exposure in workqueue

[David Miller]

From: Kees Cook 
Date: Wed, 5 Apr 2017 14:39:03 -0700

> While unlikely, this makes sure the workqueue name won't be processed
> as a format string.
> 
> Signed-off-by: Kees Cook 

Applied.

Re: [PATCH net] qed: Correct MSI-x for storage

[David Miller]

From: Yuval Mintz 
Date: Wed, 5 Apr 2017 21:20:11 +0300

> When qedr is enabled, qed would try dividing the msi-x vectors between
> L2 and RoCE, starting with L2 and providing it with sufficient vectors
> for its queues.
> 
> Problem is qed would also do that for storage partitions, and as those
> don't need queues it would lead qed to award those partitions with 0
> msi-x vectors, causing them to believe theye're using INTa and
> preventing them from operating.
> 
> Fixes: 51ff17251c9c ("qed: Add support for RoCE hw init")
> Signed-off-by: Yuval Mintz 

Applied, thanks.

Re: [PATCH net-next 0/2] net: dsa: Mock-up driver couple fixes

[David Miller]

From: Florian Fainelli 
Date: Wed,  5 Apr 2017 11:19:29 -0700

> Thanks to Dan's static checker, a bunch of small issues were found in the 
> code.

Series applied, thanks Florian.

Re: [PATCH net-next] net/sched: Removed unused vlan actions definition

[David Miller]

From: Or Gerlitz 
Date: Wed,  5 Apr 2017 19:09:25 +0300

> Commit c7e2b9689ef "sched: introduce vlan action" added both the
> UAPI values for the vlan actions (TCA_VLAN_ACT_) and these two
> in-kernel ones which are not used, remove them.
> 
> Signed-off-by: Or Gerlitz 
> Acked-by: Jiri Pirko 

Applied.

Re: [PATCH net-next] mlx4: trust shinfo->gso_segs

[David Miller]

From: Eric Dumazet 
Date: Wed, 05 Apr 2017 08:49:02 -0700

> From: Eric Dumazet 
> 
> mlx4 is the only driver in the tree making a point to recompute
> shinfo->gso_segs.
> 
> Lets remove superfluous code.
> 
> Signed-off-by: Eric Dumazet 

Applied, thanks Eric.

Re: [BUG] kernel oops during bridge creation

[Nikolay Aleksandrov]

On 06/04/17 21:58, Ido Schimmel wrote:
> +Nik
> 

Thanks!

> On Thu, Apr 06, 2017 at 08:19:35PM +0200, Peter V. Saveliev wrote:
>> Operation:
>>
>> # ip link add name test type bridge vlan_default_pvid 1
>>
>> Result:
>>
>> Kernel oops. Minimal required netlink packet structure:
>>
>> ifinfmsg:
>>  - IFLA_IFNAME: "test"
>>  - IFLA_LINKINFO:
>>  = IFLA_INFO_KIND: "bridge"
>>  = IFLA_INFO_DATA:
>>  - IFLA_BR_VLAN_DEFAULT_PVID: 1
>>
>> Reproducible: always
>>
>> Versions affected: all the latest kernels, including net-next
>>
>> Possibly related to 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b6677449dff674cf5b81429b11d5c7f358852ef9
> 
> I believe this is correct.
> 

Yep.
+CC Ivan

>>
>> Trace:
>>
>> [13634.939408] BUG: unable to handle kernel NULL pointer dereference at
>> 0190
>> [13634.939436] IP: __vlan_add+0x73/0x5f0
>> [13634.939445] PGD 3dfe0067
>> [13634.939445] PUD 35807067
>> [13634.939451] PMD 0
>>
>> [13634.939467] Oops: 0002 [#1] SMP
>> [13634.939474] Modules linked in: crct10dif_pclmul crc32_pclmul
>> ghash_clmulni_intel pcbc qxl ttm drm_kms_helper drm aesni_intel aes_x86_64
>> crypto_simd glue_helper cryptd virtio_balloon evdev input_leds pcspkr
>> led_class serio_raw virtio_console i2c_piix4 acpi_cpufreq tpm_tis
>> tpm_tis_core tpm button ext4 crc16 jbd2 mbcache virtio_net virtio_blk
>> crc32c_intel psmouse 8139cp mii virtio_pci virtio_ring virtio
>> [13634.939561] CPU: 0 PID: 1535 Comm: ip Not tainted 4.11.0-rc3+ #6
>> [13634.939574] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> 1.9.3-1.fc25 04/01/2014
>> [13634.939593] task: 9b4838f5a000 task.stack: c1b74046c000
>> [13634.939607] RIP: 0010:__vlan_add+0x73/0x5f0
>> [13634.939617] RSP: 0018:c1b74046f668 EFLAGS: 00010246
>> [13634.939629] RAX:  RBX: 9b483522d680 RCX:
>> 
>> [13634.939644] RDX: be8f2978 RSI: 0200 RDI:
>> be2c120f
>> [13634.939659] RBP:  R08:  R09:
>> 
>> [13634.939674] R10:  R11: c609 R12:
>> 
>> [13634.939689] R13: 9b48350ae8c0 R14: 9b48350ae000 R15:
>> 
>> [13634.939705] FS:  7fe5e4f13700() GS:9b483fc0()
>> knlGS:
>> [13634.939722] CS:  0010 DS:  ES:  CR0: 80050033
>> [13634.939734] CR2: 0190 CR3: 35805000 CR4:
>> 000406f0
>> [13634.939752] DR0:  DR1:  DR2:
>> 
>> [13634.939767] DR3:  DR6: fffe0ff0 DR7:
>> 0400
>> [13634.939783] Call Trace:
>> [13634.939791]  ? pcpu_next_unpop+0x3b/0x50
>> [13634.939801]  ? pcpu_alloc+0x3d2/0x680
>> [13634.939810]  ? br_vlan_add+0x135/0x1b0
> 
> Nik, I looked at this earlier today since Peter mentioned this over IRC.
> I think the problem is that the bridge's vlan group is accessed before
> it's allocated in br_vlan_init().
> 

Yes, that's the problem.

> Not sure what's the correct way to solve this though, maybe moving
> br_vlan_init() to the bridge's newlink()? Do you see any problem with
> that?
> 

I think we also have to support the old way of adding bridges via the ioctl, 
and moving
the init to newlink() would break that, making br_vlan_init() idempotent 
wouldn't help
either as we need to destroy the vg if it's allocated upon failure. So we'll 
have to
consider all error paths in both newlink() and the old ioctl and handle the 
vlan if it
was created before, ugh..

Right now I don't see a better option that would keep the current state though, 
so if you'd
like go ahead and cook a patch, I won't be able to do it until tomorrow my time.

Actually making br_vlan_init() idempotent might work, keep the code as-is just 
init the
the vlans before the changelink() in newlink(), then the second vlan_init() 
inside would
be a no-op, but it will work for the old ioctl and wouldn't require too much 
change.
On the downside it will be ugly.

Cheers,
 Nik

Re: [PATCH net] l2tp: define SOL_PPPOL2TP in uapi

[David Miller]

From: Guillaume Nault 
Date: Wed, 5 Apr 2017 16:52:35 +0200

> Userspace needs SOL_PPPOL2TP to be defined for using PPPOL2TP_SO_*
> socket options.
> 
> Signed-off-by: Guillaume Nault 

We really can't do this, there is no precedence for defining the SOL_*
values in the kernel headers, it needs to come from the libc headers just
like the other SOL_* values do for applications.

Thank you.

Re: [PATCH] qed: fix missing break in OOO_LB_TC case

[David Miller]

From: Colin King 
Date: Wed,  5 Apr 2017 13:35:44 +0100

> From: Colin Ian King 
> 
> There seems to be a missing break on the OOO_LB_TC case, pq_id
> is being assigned and then re-assigned on the fall through default
> case and that seems suspect.
> 
> Detected by CoverityScan, CID#1424402 ("Missing break in switch")
> 
> Fixes: b5a9ee7cf3be1 ("qed: Revise QM cofiguration")
> 
> Signed-off-by: Colin Ian King 

Applied, thanks.

Re: [PATCH] usbnet: make sure no NULL pointer is passed through

[David Miller]

From: Oliver Neukum 
Date: Wed,  5 Apr 2017 14:14:39 +0200

> Coverity reports:
 ...
> It is valid to offer commands without a buffer, but then you need a size
> of zero. This should actually be checked.
> 
> Signed-off-by: Oliver Neukum 

Applied, thanks Oliver.

I had to apply this by hand via my inbox rather than using patchwork
because those coverity report delimiters cause patchwork to chop off
most of your commit message.

Just FYI...

Re: [PATCH net-next] net/mlx5e: fix build error without CONFIG_SYSFS

[David Miller]

From: Tobias Regnery 
Date: Wed,  5 Apr 2017 11:11:10 +0200

> Commit 9008ae074885 ("net/mlx5e: Minimize mlx5e_{open/close}_locked")
> copied the calls to netif_set_real_num_{tx,rx}_queues from
> mlx5e_open_locked to mlx5e_activate_priv_channels and wraps them in an
> if condition to test for netdev->real_num_{tx,rx}_queues.
> 
> But netdev->real_num_rx_queues is conditionally compiled in if CONFIG_SYSFS
> is set. Without CONFIG_SYSFS the build fails:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c: In function 
> 'mlx5e_activate_priv_channels':
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c:2515:12: error: 'struct 
> net_device' has no member named 'real_num_rx_queues'; did you mean 
> 'real_num_tx_queues'?
> 
> Fix this by unconditionally call netif_set_real_num{tx,rx}_queues like before
> commit 9008ae074885.
> 
> Fixes: 9008ae074885 ("net/mlx5e: Minimize mlx5e_{open/close}_locked")
> Signed-off-by: Tobias Regnery 

Applied, thanks.

Re: [PATCH net-next 0/7] s390 patches for net-next

[David Miller]

From: Ursula Braun 
Date: Wed,  5 Apr 2017 10:40:08 +0200

> here are some cleanup patches for drivers/s390/net.

This doesn't apply cleanly to net-next, please respin.

Re: [PATCH net-next v2] net: dsa: mv88e6xxx: Make SMI c22/c45 read/write functions static

[Vivien Didelot]

Hi Florian,

Florian Fainelli  writes:

> The SMI clause 22 & 45 read/write operations are local to the global2.c file,
> so make them static. This eliminates the following warning:
>
> drivers/net/dsa/mv88e6xxx/global2.c:571:5: warning: no previous prototype for 
> 'mv88e6xxx_g2_smi_phy_read_c45' [-Wmissing-prototypes]
>  int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip, int addr,
>  ^
> drivers/net/dsa/mv88e6xxx/global2.c:602:5: warning: no previous prototype for 
> 'mv88e6xxx_g2_smi_phy_read_c22' [-Wmissing-prototypes]
>  int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip, int addr,
>  ^
> drivers/net/dsa/mv88e6xxx/global2.c:635:5: warning: no previous prototype for 
> 'mv88e6xxx_g2_smi_phy_write_c45' [-Wmissing-prototypes]
>  int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip, int addr,
>  ^~
> drivers/net/dsa/mv88e6xxx/global2.c:664:5: warning: no previous prototype for 
> 'mv88e6xxx_g2_smi_phy_write_c22' [-Wmissing-prototypes]
>  int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip, int addr,
>  ^~
>
> Suggested-by: Andrew Lunn 
> Signed-off-by: Florian Fainelli 

Reviewed-by: Vivien Didelot 

Thanks,

Vivien

Re: [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts

[Florian Fainelli]

On 04/04/2017 11:31 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 23:02 -0700, Florian Fainelli wrote:
> 
>> We don't necessarily have a phydev attached when using NC-SI, so it was
>>> easier to have the core code path not have to go fishing for those
>>> settings in different places based on whether we're using NC-SI or not.
>>
>> Oh right, I missed that part. Is there a reason why NC-SI does not have
>> a PHY device attached? If not, could you somehow model the link using a
>> fixed PHY (which appears to Linux as a normal phy_device) just to keep
>> things simple.
> 
> Hrm ... maybe another day if you don't mind ;-)
> 
> First NC-SI isn't really a PHY  it's a cross-over RMII connection
> to another NIC.
> 
> Now we could make it a phydev using a "fixed" PHY I suppose, that just
> "represents" the other end. That would be a way to do it. It would need
> to have the link permanently up however (see below).
> 
> That said I do want to tackle making it some kind of pseudo-PHY that
> actually reflects the state of the remote end (especially the link
> state, ie. up/down).
> 
> However there are a couple of issues to tackle if we do that. Well
> mostly one annoying one:
> 
> NC-SI needs to talk to the remote NIC via specific ethernet frames.
> 
> With the current link watch code however, if we reflect the remote link
> to the local NIC link via netif_carrier_on/off, we end up deactivating
> the device on link off and thus preventing the NC-SI stack from talking
> to the peer NIC at all.
> 
> I thought a while ago we could add some dev flag to prevent the link
> watch from doing that, but never got to look into it myself and
> apparently neither did Gavin.

It sounds like a similar situation to e.g: 802.1x, you want to let some
frames make it through and you need a working link for that, but you
can't quite flag the device as UP entirely. Maybe you can find a way to
re-use IFF_LOWER_UP to that purpose and only set that flag based on the
NC-SI frames indicating link state?
-- 
Florian

Re: [PATCH] af_unix: Use designated initializers

[David Miller]

From: Kees Cook 
Date: Tue, 4 Apr 2017 22:12:09 -0700

> Prepare to mark sensitive kernel structures for randomization by making
> sure they're using designated initializers. These were identified during
> allyesconfig builds of x86, arm, and arm64, and the initializer fixes
> were extracted from grsecurity. In this case, NULL initialize with { }
> instead of undesignated NULLs.
> 
> Signed-off-by: Kees Cook 

Applied to net-next.

[PATCH net-next v2] net: dsa: mv88e6xxx: Make SMI c22/c45 read/write functions static

[Florian Fainelli]

The SMI clause 22 & 45 read/write operations are local to the global2.c file,
so make them static. This eliminates the following warning:

drivers/net/dsa/mv88e6xxx/global2.c:571:5: warning: no previous prototype for 
'mv88e6xxx_g2_smi_phy_read_c45' [-Wmissing-prototypes]
 int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip, int addr,
 ^
drivers/net/dsa/mv88e6xxx/global2.c:602:5: warning: no previous prototype for 
'mv88e6xxx_g2_smi_phy_read_c22' [-Wmissing-prototypes]
 int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip, int addr,
 ^
drivers/net/dsa/mv88e6xxx/global2.c:635:5: warning: no previous prototype for 
'mv88e6xxx_g2_smi_phy_write_c45' [-Wmissing-prototypes]
 int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip, int addr,
 ^~
drivers/net/dsa/mv88e6xxx/global2.c:664:5: warning: no previous prototype for 
'mv88e6xxx_g2_smi_phy_write_c22' [-Wmissing-prototypes]
 int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip, int addr,
 ^~

Suggested-by: Andrew Lunn 
Signed-off-by: Florian Fainelli 
---
 drivers/net/dsa/mv88e6xxx/global2.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global2.c 
b/drivers/net/dsa/mv88e6xxx/global2.c
index 7c6bc33a9516..b3fea55071e3 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -568,8 +568,9 @@ static int mv88e6xxx_g2_smi_phy_write_addr(struct 
mv88e6xxx_chip *chip,
return mv88e6xxx_g2_smi_phy_cmd(chip, cmd);
 }
 
-int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip, int addr,
- int reg_c45, u16 *val, bool external)
+static int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip,
+int addr, int reg_c45, u16 *val,
+bool external)
 {
int device = (reg_c45 >> 16) & 0x1f;
int reg = reg_c45 & 0x;
@@ -599,8 +600,9 @@ int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip 
*chip, int addr,
return 0;
 }
 
-int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 *val, bool external)
+static int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip,
+int addr, int reg, u16 *val,
+bool external)
 {
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
int err;
@@ -632,8 +634,9 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
return mv88e6xxx_g2_smi_phy_read_c22(chip, addr, reg, val, external);
 }
 
-int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip, int addr,
-  int reg_c45, u16 val, bool external)
+static int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip,
+ int addr, int reg_c45, u16 val,
+ bool external)
 {
int device = (reg_c45 >> 16) & 0x1f;
int reg = reg_c45 & 0x;
@@ -661,8 +664,9 @@ int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip 
*chip, int addr,
return 0;
 }
 
-int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip, int addr,
-  int reg, u16 val, bool external)
+static int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip,
+ int addr, int reg, u16 val,
+ bool external)
 {
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
int err;
-- 
2.9.3

Re: [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts

[David Miller]

From: Benjamin Herrenschmidt 
Date: Wed,  5 Apr 2017 12:28:40 +1000

> This is version 2 of the first batch of updates to the
> ftgmac100 driver.
> 
> Essentially:
> 
>  - A few misc cleanups
>  - Fixing link speed & duplex handling (including dealing with
>an Aspeed requirement to double reset the controller when
>the speed changes)
>  - And addition of a reset task workqueue which will be used
>for delaying the re-initialization of the controller
>  - Fixing a number of issues with how interrupts and NAPI
>are dealt with.
> 
> Subsequent batches will rework and improve the rx path, the
> tx path, and add a bunch of features and fixes.
> 
> Version 2 addresses some review comments to patches 5 and 10
> (see version history in the respective emails).

Series applied.

Re: [PATCH v2] net: netfilter: Remove multiple assignment.

[Pablo Neira Ayuso]

Hi Arushi,

On Tue, Mar 28, 2017 at 04:03:27AM +0530, Arushi Singhal wrote:
> This patch removes multiple assignments to follow the kernel coding
> style as also reported by checkpatch.pl.
> Done using coccinelle.
> @@
> identifier i1,i2;
> constant c;
> @@
> - i1=i2=c;
> + i1=c;
> + i2=i1;

I see more multiple assignments like this under:

net/netfilter/
net/ipv4/netfilter/
net/ipv6/netfilter/
net/bridge/netfilter/

So I would prefer whether fix them all or none.

Thanks!

Re: [PATCH net-next] liquidio: fix Octeon core watchdog timeout false alarm

[David Miller]

From: Felix Manlunas 
Date: Tue, 4 Apr 2017 19:26:57 -0700

> Detection of watchdog timeout of Octeon cores is flawed and susceptible to
> false alarms.  Refactor by removing the detection code, and in its place,
> leverage existing code that monitors for an indication from the NIC
> firmware that an Octeon core crashed; expand the meaning of the indication
> to "an Octeon core crashed or its watchdog timer expired".  Detection of
> watchdog timeout is now delegated to an exception handler in the NIC
> firmware; this is free of false alarms.
> 
> Also if there's an Octeon core crash or watchdog timeout:
> (1) Disable VF Ethernet links.
> (2) Decrement the module refcount by an amount equal to the number of
> active VFs of the NIC whose Octeon core crashed or had a watchdog
> timeout.  The refcount will continue to reflect the active VFs of
> other liquidio NIC(s) (if present) whose Octeon cores are faultless.
> 
> Item (2) is needed to avoid the case of not being able to unload the driver
> because the module refcount is stuck at some non-zero number.  There is
> code that, in normal cases, decrements the refcount upon receiving a
> message from the firmware that a VF driver was unloaded.  But in
> exceptional cases like an Octeon core crash or watchdog timeout, arrival of
> that particular message from the firmware might be unreliable.  That normal
> case code is changed to not touch the refcount in the exceptional case to
> avoid contention (over the refcount) with the liquidio_watchdog kernel
> thread who will carry out item (2).
> 
> Signed-off-by: Felix Manlunas 
> Signed-off-by: Derek Chickles 

Applied, thanks.

Re: [Patch net] net_sched: check noop_qdisc before qdisc_hash_add()

[David Miller]

From: Cong Wang 
Date: Tue,  4 Apr 2017 18:51:30 -0700

> Dmitry reported a crash when injecting faults in
> attach_one_default_qdisc() and dev->qdisc is still
> a noop_disc, the check before qdisc_hash_add() fails
> to catch it because it tests NULL. We should test
> against noop_qdisc since it is the default qdisc
> at this point.
> 
> Fixes: 59cc1f61f09c ("net: sched: convert qdisc linked list to hashtable")
> Reported-by: Dmitry Vyukov 
> Cc: Jiri Kosina 
> Signed-off-by: Cong Wang 

Applied and queued up for -stable.

Re: [PATCH net-next] net: usbnet: Remove unused driver_name variable

[David Miller]

From: Florian Fainelli 
Date: Tue,  4 Apr 2017 18:16:57 -0700

> With GCC 6.3, we can get the following warning:
> 
> drivers/net/usb/usbnet.c:85:19: warning: 'driver_name' defined but not
> used [-Wunused-const-variable=]
>  static const char driver_name [] = "usbnet";
>^~~
> 
> Signed-off-by: Florian Fainelli 

Applied, thanks.

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Matthias Kaehlcke]

Hi Johannes,

thanks for your comments

El Thu, Apr 06, 2017 at 09:11:18PM +0200 Johannes Berg ha dit:

> On Thu, 2017-04-06 at 11:56 -0700, Matthias Kaehlcke wrote:
> > Clang raises a warning about the expression 'strlen(CONFIG_XXX)'
> > being
> > used in a logical operation. Clangs' builtin strlen function resolves
> > the
> > expression to a constant at compile time, which causes clang to
> > generate
> > a 'constant-logical-operand' warning.
> > 
> > Split the if statement in two to avoid using the const expression in
> > a logical operation.
> > 
> I don't really see all much point in doing this for the warning's
> sake... hopefully it doesn't actually generate worse code, but I think
> the code ends up looking worse and people will forever wonder what the
> goto is really doing there.

I agree that the code looks worse :( I hoped to find a fix using a
preprocessor condition but wasn't successful.

Some projects (like Chrome OS) build their kernel with all warnings
being treated as errors. Besides changing the 'offending' code the
alternatives are to disable the warning completely or to tell clang
not to use the builtin(s). IMO changing the code is the preferable
solution, especially since this is so far the only occurrence of the
warning that I have encountered.

I used goto instead of nested ifs since other functions in this file
use the same pattern. If nested ifs are preferred I can change that.

Cheers

Matthias

Re: [PATCH net-next] selftests/bpf: fix merge conflict

[David Miller]

From: Alexei Starovoitov 
Date: Thu, 6 Apr 2017 12:20:26 -0700

> fix artifact of merge resolution
> 
> Signed-off-by: Alexei Starovoitov 
> Acked-by: Daniel Borkmann 

Sorry about that, applied, thanks!

[PATCH net-next] selftests/bpf: fix merge conflict

[Alexei Starovoitov]

fix artifact of merge resolution

Signed-off-by: Alexei Starovoitov 
Acked-by: Daniel Borkmann 
---
 tools/testing/selftests/bpf/test_verifier.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c 
b/tools/testing/selftests/bpf/test_verifier.c
index 0963f8ffd25c..6178b65fee59 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4904,12 +4904,7 @@ static void do_test_single(struct bpf_test *test, bool 
unpriv,
struct bpf_insn *prog = test->insns;
int prog_len = probe_filter_length(prog);
int prog_type = test->prog_type;
-<<< HEAD
int map_fds[MAX_NR_MAPS];
-   int fd_prog, expected_ret;
-===
-   int fd_f1 = -1, fd_f2 = -1, fd_f3 = -1;
->>> ea6b1720ce25f92f7a17b2e0c2b653d20773d10a
const char *expected_err;
int i;
 
-- 
2.9.3

Re: [PATCH] net: ipv4: fix multipath RTM_GETROUTE behavior when iif is given

[David Miller]

From: Florian Larysch 
Date: Mon,  3 Apr 2017 16:46:09 +0200

> inet_rtm_getroute synthesizes a skeletal ICMP skb, which is passed to
> ip_route_input when iif is given. If a multipath route is present for
> the designated destination, ip_multipath_icmp_hash ends up being called,
> which uses the source/destination addresses within the skb to calculate
> a hash. However, those are not set in the synthetic skb, causing it to
> return an arbitrary and incorrect result.
> 
> Instead, use UDP, which gets no such special treatment.
> 
> Signed-off-by: Florian Larysch 

Applied and queued up for -stable.

Please submit the net-next variant you mentioned, thank you.

Re: [PATCH] netpoll: Check for skb->queue_mapping

[Eric Dumazet]

On Thu, 2017-04-06 at 12:07 -0700, tndave wrote:

> > +   q_index = q_index % dev->real_num_tx_queues;
> cpu interrupted here and dev->real_num_tx_queues has reduced!
> > +   skb_set_queue_mapping(skb, q_index);
> > +   }
> > +   txq = netdev_get_tx_queue(dev, q_index);
> or cpu interrupted here and dev->real_num_tx_queues has reduced!

If dev->real_num_tx_queues can be changed while this code is running we
are in deep deep trouble.

Better make sure that when control path does this change, device (and/pr
netpoll) is frozen and no packet can be sent.

Re: [PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Johannes Berg]

On Thu, 2017-04-06 at 11:56 -0700, Matthias Kaehlcke wrote:
> Clang raises a warning about the expression 'strlen(CONFIG_XXX)'
> being
> used in a logical operation. Clangs' builtin strlen function resolves
> the
> expression to a constant at compile time, which causes clang to
> generate
> a 'constant-logical-operand' warning.
> 
> Split the if statement in two to avoid using the const expression in
> a logical operation.
> 
I don't really see all much point in doing this for the warning's
sake... hopefully it doesn't actually generate worse code, but I think
the code ends up looking worse and people will forever wonder what the
goto is really doing there.

johannes

Re: [PATCH] netpoll: Check for skb->queue_mapping

[tndave]

On 04/06/2017 03:26 AM, Eric Dumazet wrote:

On Wed, 2017-04-05 at 19:06 -0700, Tushar Dave wrote:

Reducing real_num_tx_queues needs to be in sync with skb queue_mapping
otherwise skbs with queue_mapping greater than real_num_tx_queues
can be sent to the underlying driver and can result in kernel panic.

One such event is running netconsole and enabling VF on the same
device. Or running netconsole and changing number of tx queues via
ethtool on same device.

e.g.




Signed-off-by: Tushar Dave 
---
 net/core/netpoll.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 9424673..c572e49 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -101,6 +101,7 @@ static void queue_process(struct work_struct *work)
container_of(work, struct netpoll_info, tx_work.work);
struct sk_buff *skb;
unsigned long flags;
+   u16 q_index;

while ((skb = skb_dequeue(>txq))) {
struct net_device *dev = skb->dev;
@@ -117,6 +118,12 @@ static void queue_process(struct work_struct *work)
HARD_TX_LOCK(dev, txq, smp_processor_id());
if (netif_xmit_frozen_or_stopped(txq) ||
netpoll_start_xmit(skb, dev, txq) != NETDEV_TX_OK) {
+   /* check if skb->queue_mapping has changed */
+   q_index = skb_get_queue_mapping(skb);
+   if (unlikely(q_index >= dev->real_num_tx_queues)) {
+   q_index = q_index % dev->real_num_tx_queues;
+   skb_set_queue_mapping(skb, q_index);
+   }
skb_queue_head(>txq, skb);
HARD_TX_UNLOCK(dev, txq);
local_irq_restore(flags);


Hi Tushar, thank you for working on this issue.

Eric,

Thank you for reviewing my change and your valuable comments.


Where and when skb->queue_mapping has changed ?

Well, I should amend the comment,
"/* check if skb->queue_mapping has changed */" is misleading.
I should say, /* check if dev->real_num_tx_queues has changed */


It looks that the real problem is that dev->real_num_tx_queues has been
changed instead of skb->queue_mapping

Yes.


So maybe the more correct change would be to cap skb->queue_mapping even
before getting skb_get_tx_queue() ?

Otherwise, even after your patch, we might still access an invalid queue
on the device ?

This is the case of direct xmit (netdev_start_xmit). Most of underlying
mq device drivers retrieve tx queue index from skb->queue_mapping.
One possibility I see where device driver still get invalid queue index
(skb->queue_mapping) with my patch is when following occurs:
- after executing "txq = skb_get_tx_queue(dev, skb)" cpu is interrupted';
- some other cpu reduced dev->real_num_tx_queues;
- Interrupted cpu resume (queue_process()).
With above sequence, at the underlying driver's xmit function we can
have skb->queue_mapping > dev->real_num_tx_queues [1].

I like your suggested patch more than mine though because it checks for
change in dev->real_num_tx_queues before even we retrieve 'netdev_queue
txq'. Less chance to have wrong txq.

However even your patch has same issue like [1] ? (see my comment below).



Something like the following :

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 
9424673009c14e0fb288b8e4041dba596b37ee8d..16702d95f83ab884e605e3868cfef94615dcbc72
 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -105,13 +105,20 @@ static void queue_process(struct work_struct *work)
while ((skb = skb_dequeue(>txq))) {
struct net_device *dev = skb->dev;
struct netdev_queue *txq;
+   unsigned int q_index;

if (!netif_device_present(dev) || !netif_running(dev)) {
kfree_skb(skb);
continue;
}

-   txq = skb_get_tx_queue(dev, skb);
+   /* check if skb->queue_mapping is still valid */
+   q_index = skb_get_queue_mapping(skb);
+   if (unlikely(q_index >= dev->real_num_tx_queues)) {
+   q_index = q_index % dev->real_num_tx_queues;

cpu interrupted here and dev->real_num_tx_queues has reduced!

+   skb_set_queue_mapping(skb, q_index);
+   }
+   txq = netdev_get_tx_queue(dev, q_index);

or cpu interrupted here and dev->real_num_tx_queues has reduced!

In any case we hit the bug or am I missing something?

The other concern is concurrent execution of netpoll's direct xmit path
and updates to dev->real_num_tx_queues (by other cpu)?


Thanks.

-Tushar


local_irq_save(flags);
HARD_TX_LOCK(dev, txq, smp_processor_id());

Re: [BUG] kernel oops during bridge creation

[Ido Schimmel]

+Nik

On Thu, Apr 06, 2017 at 08:19:35PM +0200, Peter V. Saveliev wrote:
> Operation:
> 
> # ip link add name test type bridge vlan_default_pvid 1
> 
> Result:
> 
> Kernel oops. Minimal required netlink packet structure:
> 
> ifinfmsg:
>   - IFLA_IFNAME: "test"
>   - IFLA_LINKINFO:
>   = IFLA_INFO_KIND: "bridge"
>   = IFLA_INFO_DATA:
>   - IFLA_BR_VLAN_DEFAULT_PVID: 1
> 
> Reproducible: always
> 
> Versions affected: all the latest kernels, including net-next
> 
> Possibly related to 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b6677449dff674cf5b81429b11d5c7f358852ef9

I believe this is correct.

> 
> Trace:
> 
> [13634.939408] BUG: unable to handle kernel NULL pointer dereference at
> 0190
> [13634.939436] IP: __vlan_add+0x73/0x5f0
> [13634.939445] PGD 3dfe0067
> [13634.939445] PUD 35807067
> [13634.939451] PMD 0
> 
> [13634.939467] Oops: 0002 [#1] SMP
> [13634.939474] Modules linked in: crct10dif_pclmul crc32_pclmul
> ghash_clmulni_intel pcbc qxl ttm drm_kms_helper drm aesni_intel aes_x86_64
> crypto_simd glue_helper cryptd virtio_balloon evdev input_leds pcspkr
> led_class serio_raw virtio_console i2c_piix4 acpi_cpufreq tpm_tis
> tpm_tis_core tpm button ext4 crc16 jbd2 mbcache virtio_net virtio_blk
> crc32c_intel psmouse 8139cp mii virtio_pci virtio_ring virtio
> [13634.939561] CPU: 0 PID: 1535 Comm: ip Not tainted 4.11.0-rc3+ #6
> [13634.939574] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.9.3-1.fc25 04/01/2014
> [13634.939593] task: 9b4838f5a000 task.stack: c1b74046c000
> [13634.939607] RIP: 0010:__vlan_add+0x73/0x5f0
> [13634.939617] RSP: 0018:c1b74046f668 EFLAGS: 00010246
> [13634.939629] RAX:  RBX: 9b483522d680 RCX:
> 
> [13634.939644] RDX: be8f2978 RSI: 0200 RDI:
> be2c120f
> [13634.939659] RBP:  R08:  R09:
> 
> [13634.939674] R10:  R11: c609 R12:
> 
> [13634.939689] R13: 9b48350ae8c0 R14: 9b48350ae000 R15:
> 
> [13634.939705] FS:  7fe5e4f13700() GS:9b483fc0()
> knlGS:
> [13634.939722] CS:  0010 DS:  ES:  CR0: 80050033
> [13634.939734] CR2: 0190 CR3: 35805000 CR4:
> 000406f0
> [13634.939752] DR0:  DR1:  DR2:
> 
> [13634.939767] DR3:  DR6: fffe0ff0 DR7:
> 0400
> [13634.939783] Call Trace:
> [13634.939791]  ? pcpu_next_unpop+0x3b/0x50
> [13634.939801]  ? pcpu_alloc+0x3d2/0x680
> [13634.939810]  ? br_vlan_add+0x135/0x1b0

Nik, I looked at this earlier today since Peter mentioned this over IRC.
I think the problem is that the bridge's vlan group is accessed before
it's allocated in br_vlan_init().

Not sure what's the correct way to solve this though, maybe moving
br_vlan_init() to the bridge's newlink()? Do you see any problem with
that?

> [13634.939820]  ? __br_vlan_set_default_pvid.part.28+0x204/0x2b0
> [13634.939834]  ? br_changelink+0x120/0x4e0
> [13634.939844]  ? br_dev_newlink+0x50/0x70
> [13634.939854]  ? rtnl_newlink+0x5f5/0x8a0
> [13634.939864]  ? rtnl_newlink+0x176/0x8a0
> [13634.939874]  ? mem_cgroup_commit_charge+0x7c/0x4e0
> [13634.939886]  ? rtnetlink_rcv_msg+0xe1/0x220
> [13634.939896]  ? lookup_fast+0x52/0x370
> [13634.939905]  ? rtnl_newlink+0x8a0/0x8a0
> [13634.939915]  ? netlink_rcv_skb+0xa1/0xc0
> [13634.939925]  ? rtnetlink_rcv+0x24/0x30
> [13634.939934]  ? netlink_unicast+0x177/0x220
> [13634.939944]  ? netlink_sendmsg+0x2fe/0x3b0
> [13634.939954]  ? _copy_from_user+0x39/0x40
> [13634.939964]  ? sock_sendmsg+0x30/0x40
> [13634.940159]  ? ___sys_sendmsg+0x29d/0x2b0
> [13634.940326]  ? __alloc_pages_nodemask+0xdf/0x230
> [13634.940478]  ? mem_cgroup_commit_charge+0x7c/0x4e0
> [13634.940592]  ? mem_cgroup_try_charge+0x76/0x1a0
> [13634.940701]  ? __handle_mm_fault+0xdb9/0x10b0
> [13634.940809]  ? __sys_sendmsg+0x51/0x90
> [13634.940917]  ? entry_SYSCALL_64_fastpath+0x1e/0xad
> [13634.941027] Code: 75 18 49 8b 6d 30 a8 20 74 29 0f b7 4b 10 49 8b 96 28
> 03 00 00 4c 89 e6 4c 89 ef e8 88 eb fe ff 85 c0 41 89 c7 0f 85 58 05 00 00
> <66> 83 85 90 01 00 00 01 48 8b 45 30 48 89 c2 48 f7 da 48 83 7d
> [13634.941301] RIP: __vlan_add+0x73/0x5f0 RSP: c1b74046f668
> [13634.941423] CR2: 0190
> [13634.943641] ---[ end trace bb793e957e3e263c ]---
> 
> …
> 
> Thanks.

Re: [git pull] skb_copy_{,and_csum_}datagram_msg() fixes

[David Miller]

From: Al Viro 
Date: Sun, 2 Apr 2017 17:23:28 +0100

>   Fixes rsync et.al. regression since 3.19...

Pulled, thanks Al.

[PATCH] mac80211: Fix clang warning about constant operand in logical operation

[Matthias Kaehlcke]

Clang raises a warning about the expression 'strlen(CONFIG_XXX)' being
used in a logical operation. Clangs' builtin strlen function resolves the
expression to a constant at compile time, which causes clang to generate
a 'constant-logical-operand' warning.

Split the if statement in two to avoid using the const expression in a
logical operation.

Signed-off-by: Matthias Kaehlcke 
---
 net/mac80211/rate.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 206698bc93f4..68ff202d6380 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -173,9 +173,14 @@ ieee80211_rate_control_ops_get(const char *name)
/* try default if specific alg requested but not found */
ops = 
ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
 
+   if (ops)
+   goto unlock;
+
/* try built-in one if specific alg requested but not found */
-   if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT))
+   if (strlen(CONFIG_MAC80211_RC_DEFAULT))
ops = 
ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
+
+unlock:
kernel_param_unlock(THIS_MODULE);
 
return ops;
-- 
2.12.2.715.g7642488e1d-goog

[BUG] kernel oops during bridge creation

[Peter V. Saveliev]

Operation:

# ip link add name test type bridge vlan_default_pvid 1

Result:

Kernel oops. Minimal required netlink packet structure:

ifinfmsg:
- IFLA_IFNAME: "test"
- IFLA_LINKINFO:
= IFLA_INFO_KIND: "bridge"
= IFLA_INFO_DATA:
- IFLA_BR_VLAN_DEFAULT_PVID: 1

Reproducible: always

Versions affected: all the latest kernels, including net-next

Possibly related to 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b6677449dff674cf5b81429b11d5c7f358852ef9


Trace:

[13634.939408] BUG: unable to handle kernel NULL pointer dereference at 
0190

[13634.939436] IP: __vlan_add+0x73/0x5f0
[13634.939445] PGD 3dfe0067
[13634.939445] PUD 35807067
[13634.939451] PMD 0

[13634.939467] Oops: 0002 [#1] SMP
[13634.939474] Modules linked in: crct10dif_pclmul crc32_pclmul 
ghash_clmulni_intel pcbc qxl ttm drm_kms_helper drm aesni_intel 
aes_x86_64 crypto_simd glue_helper cryptd virtio_balloon evdev 
input_leds pcspkr led_class serio_raw virtio_console i2c_piix4 
acpi_cpufreq tpm_tis tpm_tis_core tpm button ext4 crc16 jbd2 mbcache 
virtio_net virtio_blk crc32c_intel psmouse 8139cp mii virtio_pci 
virtio_ring virtio

[13634.939561] CPU: 0 PID: 1535 Comm: ip Not tainted 4.11.0-rc3+ #6
[13634.939574] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS 1.9.3-1.fc25 04/01/2014

[13634.939593] task: 9b4838f5a000 task.stack: c1b74046c000
[13634.939607] RIP: 0010:__vlan_add+0x73/0x5f0
[13634.939617] RSP: 0018:c1b74046f668 EFLAGS: 00010246
[13634.939629] RAX:  RBX: 9b483522d680 RCX: 

[13634.939644] RDX: be8f2978 RSI: 0200 RDI: 
be2c120f
[13634.939659] RBP:  R08:  R09: 

[13634.939674] R10:  R11: c609 R12: 

[13634.939689] R13: 9b48350ae8c0 R14: 9b48350ae000 R15: 

[13634.939705] FS:  7fe5e4f13700() GS:9b483fc0() 
knlGS:

[13634.939722] CS:  0010 DS:  ES:  CR0: 80050033
[13634.939734] CR2: 0190 CR3: 35805000 CR4: 
000406f0
[13634.939752] DR0:  DR1:  DR2: 

[13634.939767] DR3:  DR6: fffe0ff0 DR7: 
0400

[13634.939783] Call Trace:
[13634.939791]  ? pcpu_next_unpop+0x3b/0x50
[13634.939801]  ? pcpu_alloc+0x3d2/0x680
[13634.939810]  ? br_vlan_add+0x135/0x1b0
[13634.939820]  ? __br_vlan_set_default_pvid.part.28+0x204/0x2b0
[13634.939834]  ? br_changelink+0x120/0x4e0
[13634.939844]  ? br_dev_newlink+0x50/0x70
[13634.939854]  ? rtnl_newlink+0x5f5/0x8a0
[13634.939864]  ? rtnl_newlink+0x176/0x8a0
[13634.939874]  ? mem_cgroup_commit_charge+0x7c/0x4e0
[13634.939886]  ? rtnetlink_rcv_msg+0xe1/0x220
[13634.939896]  ? lookup_fast+0x52/0x370
[13634.939905]  ? rtnl_newlink+0x8a0/0x8a0
[13634.939915]  ? netlink_rcv_skb+0xa1/0xc0
[13634.939925]  ? rtnetlink_rcv+0x24/0x30
[13634.939934]  ? netlink_unicast+0x177/0x220
[13634.939944]  ? netlink_sendmsg+0x2fe/0x3b0
[13634.939954]  ? _copy_from_user+0x39/0x40
[13634.939964]  ? sock_sendmsg+0x30/0x40
[13634.940159]  ? ___sys_sendmsg+0x29d/0x2b0
[13634.940326]  ? __alloc_pages_nodemask+0xdf/0x230
[13634.940478]  ? mem_cgroup_commit_charge+0x7c/0x4e0
[13634.940592]  ? mem_cgroup_try_charge+0x76/0x1a0
[13634.940701]  ? __handle_mm_fault+0xdb9/0x10b0
[13634.940809]  ? __sys_sendmsg+0x51/0x90
[13634.940917]  ? entry_SYSCALL_64_fastpath+0x1e/0xad
[13634.941027] Code: 75 18 49 8b 6d 30 a8 20 74 29 0f b7 4b 10 49 8b 96 
28 03 00 00 4c 89 e6 4c 89 ef e8 88 eb fe ff 85 c0 41 89 c7 0f 85 58 05 
00 00 <66> 83 85 90 01 00 00 01 48 8b 45 30 48 89 c2 48 f7 da 48 83 7d

[13634.941301] RIP: __vlan_add+0x73/0x5f0 RSP: c1b74046f668
[13634.941423] CR2: 0190
[13634.943641] ---[ end trace bb793e957e3e263c ]---

…

Thanks.

Re: [PATCH net-next] bonding: attempt to better support longer hw addresses

[Jarod Wilson]

On 2017-04-05 9:45 PM, David Miller wrote:

From: Jarod Wilson 
Date: Tue,  4 Apr 2017 17:32:42 -0400

...

Applied, but:


+static inline void bond_hw_addr_copy(u8 *dst, const u8 *src, unsigned int len)
+{
+   if (len == ETH_ALEN) {
+   ether_addr_copy(dst, src);
+   return;
+   }
+
+   memcpy(dst, src, len);
+}


I wonder how much value there is in trying to conditionally use
ether_addr_copy().  Unless some of these calls are in the data
plane, just a straight memcpy() all the time is fine and much
simpler.


Yeah, I wasn't sure how much gain the bonding driver actually got from 
using the super-optimized ether_addr_copy(), and thought about just 
doing a memcpy all the time, but wanted to go for minimal impact to 
traditional ethernet bonding. Looks like bond_handle_frame() might 
benefit from sticking to ether_addr_copy() when it can, but the majority 
of other callers, at least in bond_main.c, are all in setup, teardown 
and failover paths, which ought to be tolerant of some overhead, though 
optimized failover isn't a bad thing for connection uptime. I do see 
bond_alb.c has one caller in the arp transmit path as well. I think I'm 
inclined to just leave well enough alone for the moment.


--
Jarod Wilson
ja...@redhat.com

Re: [PATCH] net: davinci_mdio: add GPIO reset logic

[Roger Quadros]

On 06/04/17 15:05, Andrew Lunn wrote:
>>> Do you really need more than one GPIO? A single gpio would make all
>>> this code a lot simpler.
>>>
>>
>> Yes we need. Some of our boards have separate GPIO RESET lines for
>> different PHYs on the same MDIO bus.
> 
> If you have a one-to-one mapping of GPIO and PHY, you should really be
> modelling that differently. You want to be able to reset just a single
> PHY, i.e. make it part of the PHY driver, or maybe the PHY core.
> 
>  Andrew
> 


I'm not sure how it would be modelled. We have never had the need to reset
just one PHY on the MDIO bus.
Some boards have a single RESET line to multiple PHYs whereas others have 
individual
RESET lines. In all cases we just want to do the RESET once per boot.

cheers,
-roger

Re: [PATCH] mwifiex: MAC randomization should not be persistent

[Brian Norris]

On Thu, Apr 06, 2017 at 07:02:15AM +0300, Kalle Valo wrote:
> Brian Norris  writes:
> 
> > nl80211 provides the NL80211_SCAN_FLAG_RANDOM_ADDR for every scan
> > request that should be randomized; the absence of such a flag means we
> > should not randomize. However, mwifiex was stashing the latest
> > randomization request and *always* using it for future scans, even those
> > that didn't set the flag.
> >
> > Let's zero out the randomization info whenever we get a scan request
> > without NL80211_SCAN_FLAG_RANDOM_ADDR. I'd prefer to remove
> > priv->random_mac entirely (and plumb the randomization MAC properly
> > through the call sequence), but the spaghetti is a little difficult to
> > unravel here for me.
> >
> > Fixes: c2a8f0ff9c6c ("mwifiex: support random MAC address for scanning")
> 
> So the first release with this was v4.9.
> 
> > Signed-off-by: Brian Norris 
> > ---
> > Should this be tagged for -stable?
> 
> IMHO yes.

Sounds fine to me. I suppose you'll do this when applying? Or I can
resend...

Brian

[PATCH net 1/2] l2tp: don't mask errors in pppol2tp_setsockopt()

[Guillaume Nault]

pppol2tp_setsockopt() unconditionally overwrites the error value
returned by pppol2tp_tunnel_setsockopt() or
pppol2tp_session_setsockopt(), thus hiding errors from userspace.

Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp 
parts")
Signed-off-by: Guillaume Nault 
---
 net/l2tp/l2tp_ppp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 861b255a2d51..973a9185b276 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1383,8 +1383,6 @@ static int pppol2tp_setsockopt(struct socket *sock, int 
level, int optname,
} else
err = pppol2tp_session_setsockopt(sk, session, optname, val);
 
-   err = 0;
-
 end_put_sess:
sock_put(sk);
 end:
-- 
2.11.0

[PATCH net 2/2] l2tp: don't mask errors in pppol2tp_getsockopt()

[Guillaume Nault]

pppol2tp_getsockopt() doesn't take into account the error code returned
by pppol2tp_tunnel_getsockopt() or pppol2tp_session_getsockopt(). If
error occurs there, pppol2tp_getsockopt() continues unconditionally and
reports erroneous values.

Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp 
parts")
Signed-off-by: Guillaume Nault 
---
 net/l2tp/l2tp_ppp.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 973a9185b276..32ea0f3d868c 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1505,8 +1505,13 @@ static int pppol2tp_getsockopt(struct socket *sock, int 
level, int optname,
 
err = pppol2tp_tunnel_getsockopt(sk, tunnel, optname, );
sock_put(ps->tunnel_sock);
-   } else
+   if (err)
+   goto end_put_sess;
+   } else {
err = pppol2tp_session_getsockopt(sk, session, optname, );
+   if (err)
+   goto end_put_sess;
+   }
 
err = -EFAULT;
if (put_user(len, optlen))
-- 
2.11.0

[PATCH net 0/2] l2tp: fix error handling of PPPoL2TP socket options

[Guillaume Nault]

Fix pppol2tp_[gs]etsockopt() so that they don't ignore errors returned
by their helper functions.

Guillaume Nault (2):
  l2tp: don't mask errors in pppol2tp_setsockopt()
  l2tp: don't mask errors in pppol2tp_getsockopt()

 net/l2tp/l2tp_ppp.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.11.0

Re: [PATCH 1/2] net: netfilter: Remove typedef from "typedef struct field_t"

[Pablo Neira Ayuso]

On Sat, Mar 25, 2017 at 05:57:55PM +0530, Arushi Singhal wrote:
> This patch removes typedefs from struct and renames it from "typedef struct
> field_t" to "struct field" as per kernel coding standards."
> 
> Signed-off-by: Arushi Singhal 
> ---
>  net/netfilter/nf_conntrack_h323_asn1.c | 68 
> +-
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c 
> b/net/netfilter/nf_conntrack_h323_asn1.c
> index 89b2e46925c4..fb8cf238a76f 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -77,7 +77,7 @@
>  
>  
>  /* ASN.1 Field Structure */
> -typedef struct field_t {
> +struct field {

Probably better if you rename this to 'struct h323_field' to make sure
compilation doesn't break due to structure name pollution. And we also
got a report from kbuild robot that would be good to investigate.

Re: [PATCH net-next] net/sched: Removed unused vlan actions definition

[Simon Horman]

On Wed, Apr 05, 2017 at 07:09:25PM +0300, Or Gerlitz wrote:
> Commit c7e2b9689ef "sched: introduce vlan action" added both the
> UAPI values for the vlan actions (TCA_VLAN_ACT_) and these two
> in-kernel ones which are not used, remove them.
> 
> Signed-off-by: Or Gerlitz 
> Acked-by: Jiri Pirko 

Reviewed-by: Simon Horman 

Re: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST

[Eric Dumazet]

On Thu, 2017-04-06 at 10:08 -0400, Neal Cardwell wrote:
> On Thu, Apr 6, 2017 at 10:05 AM, Gao Feng  wrote:
> > If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
> > When machine is overloaded and mem is exhausted, it may fail to alloc skb.
> 
> Moving the increment of TCP_MIB_OUTRSTS to the top of
> tcp_send_active_reset() sounds fine to me.


Yes.

Keep in mind that whatever hard we try to send the packet, something
might drop it later without TCP stack knowing it.

So it is not really useful to test the immediate actions that are under
our control.

Re: [PATCH 3/4] xfrm: Prepare for CRYPTO_MAX_ALG_NAME expansion

[Alexander Sverdlin]

On 06/04/17 10:16, Herbert Xu wrote:
> This patch fixes the xfrm_user code to use the actual array size
> rather than the hard-coded CRYPTO_MAX_ALG_NAME length.  This is
> because the array size is fixed at 64 bytes while we want to increase
> the in-kernel CRYPTO_MAX_ALG_NAME value.
> 
> Signed-off-by: Herbert Xu 

Acked-by: Alexander Sverdlin 
Tested-by: Alexander Sverdlin 

> ---
> 
>  net/xfrm/xfrm_user.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 9705c27..96557cf 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -55,7 +55,7 @@ static int verify_one_alg(struct nlattr **attrs, enum 
> xfrm_attr_type_t type)
>   return -EINVAL;
>   }
>  
> - algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
> + algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
>   return 0;
>  }
>  
> @@ -71,7 +71,7 @@ static int verify_auth_trunc(struct nlattr **attrs)
>   if (nla_len(rt) < xfrm_alg_auth_len(algp))
>   return -EINVAL;
>  
> - algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
> + algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
>   return 0;
>  }
>  
> @@ -87,7 +87,7 @@ static int verify_aead(struct nlattr **attrs)
>   if (nla_len(rt) < aead_len(algp))
>   return -EINVAL;
>  
> - algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
> + algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
>   return 0;
>  }
>  
> 

-- 
Best regards,
Alexander Sverdlin.

Re: [PATCH 4/4] crypto: api - Extend algorithm name limit to 128 bytes

[Alexander Sverdlin]

On 06/04/17 10:16, Herbert Xu wrote:
> With the new explicit IV generators, we may now exceed the 64-byte
> length limit on the algorithm name, e.g., with
> 
>   echainiv(authencesn(hmac(sha256-generic),cbc(des3_ede-generic)))
> 
> This patch extends the length limit to 128 bytes.
> 
> Reported-by: Alexander Sverdlin 
> Signed-off-by: Herbert Xu 

Acked-by: Alexander Sverdlin 
Tested-by: Alexander Sverdlin 

> ---
> 
>  include/linux/crypto.h |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index c0b0cf3..84da997 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -123,7 +123,7 @@
>  /*
>   * Miscellaneous stuff.
>   */
> -#define CRYPTO_MAX_ALG_NAME  64
> +#define CRYPTO_MAX_ALG_NAME  128
>  
>  /*
>   * The macro CRYPTO_MINALIGN_ATTR (along with the void * type in the actual
> 

-- 
Best regards,
Alexander Sverdlin.

Re: [PATCH 1/4] crypto: user - Prepare for CRYPTO_MAX_ALG_NAME expansion

[Alexander Sverdlin]

On 06/04/17 10:16, Herbert Xu wrote:
> This patch hard-codes CRYPTO_MAX_NAME in the user-space API to
> 64, which is the current value of CRYPTO_MAX_ALG_NAME.  This patch
> also replaces all remaining occurences of CRYPTO_MAX_ALG_NAME
> in the user-space API with CRYPTO_MAX_NAME.
> 
> This way the user-space API will not be modified when we raise
> the value of CRYPTO_MAX_ALG_NAME.
> 
> Furthermore, the code has been updated to handle names longer than
> the user-space API.  They will be truncated.
> 
> Signed-off-by: Herbert Xu 

Acked-by: Alexander Sverdlin 
Tested-by: Alexander Sverdlin 

> ---
> 
>  crypto/crypto_user.c|   18 +-
>  include/uapi/linux/cryptouser.h |   10 +-
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
> index a90404a..89acaab 100644
> --- a/crypto/crypto_user.c
> +++ b/crypto/crypto_user.c
> @@ -83,7 +83,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct 
> crypto_alg *alg)
>  {
>   struct crypto_report_cipher rcipher;
>  
> - strncpy(rcipher.type, "cipher", sizeof(rcipher.type));
> + strlcpy(rcipher.type, "cipher", sizeof(rcipher.type));
>  
>   rcipher.blocksize = alg->cra_blocksize;
>   rcipher.min_keysize = alg->cra_cipher.cia_min_keysize;
> @@ -102,7 +102,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct 
> crypto_alg *alg)
>  {
>   struct crypto_report_comp rcomp;
>  
> - strncpy(rcomp.type, "compression", sizeof(rcomp.type));
> + strlcpy(rcomp.type, "compression", sizeof(rcomp.type));
>   if (nla_put(skb, CRYPTOCFGA_REPORT_COMPRESS,
>   sizeof(struct crypto_report_comp), ))
>   goto nla_put_failure;
> @@ -116,7 +116,7 @@ static int crypto_report_acomp(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  {
>   struct crypto_report_acomp racomp;
>  
> - strncpy(racomp.type, "acomp", sizeof(racomp.type));
> + strlcpy(racomp.type, "acomp", sizeof(racomp.type));
>  
>   if (nla_put(skb, CRYPTOCFGA_REPORT_ACOMP,
>   sizeof(struct crypto_report_acomp), ))
> @@ -131,7 +131,7 @@ static int crypto_report_akcipher(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  {
>   struct crypto_report_akcipher rakcipher;
>  
> - strncpy(rakcipher.type, "akcipher", sizeof(rakcipher.type));
> + strlcpy(rakcipher.type, "akcipher", sizeof(rakcipher.type));
>  
>   if (nla_put(skb, CRYPTOCFGA_REPORT_AKCIPHER,
>   sizeof(struct crypto_report_akcipher), ))
> @@ -146,7 +146,7 @@ static int crypto_report_kpp(struct sk_buff *skb, struct 
> crypto_alg *alg)
>  {
>   struct crypto_report_kpp rkpp;
>  
> - strncpy(rkpp.type, "kpp", sizeof(rkpp.type));
> + strlcpy(rkpp.type, "kpp", sizeof(rkpp.type));
>  
>   if (nla_put(skb, CRYPTOCFGA_REPORT_KPP,
>   sizeof(struct crypto_report_kpp), ))
> @@ -160,10 +160,10 @@ static int crypto_report_kpp(struct sk_buff *skb, 
> struct crypto_alg *alg)
>  static int crypto_report_one(struct crypto_alg *alg,
>struct crypto_user_alg *ualg, struct sk_buff *skb)
>  {
> - strncpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
> - strncpy(ualg->cru_driver_name, alg->cra_driver_name,
> + strlcpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
> + strlcpy(ualg->cru_driver_name, alg->cra_driver_name,
>   sizeof(ualg->cru_driver_name));
> - strncpy(ualg->cru_module_name, module_name(alg->cra_module),
> + strlcpy(ualg->cru_module_name, module_name(alg->cra_module),
>   sizeof(ualg->cru_module_name));
>  
>   ualg->cru_type = 0;
> @@ -176,7 +176,7 @@ static int crypto_report_one(struct crypto_alg *alg,
>   if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
>   struct crypto_report_larval rl;
>  
> - strncpy(rl.type, "larval", sizeof(rl.type));
> + strlcpy(rl.type, "larval", sizeof(rl.type));
>   if (nla_put(skb, CRYPTOCFGA_REPORT_LARVAL,
>   sizeof(struct crypto_report_larval), ))
>   goto nla_put_failure;
> diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
> index 11d21fc..b4def5c 100644
> --- a/include/uapi/linux/cryptouser.h
> +++ b/include/uapi/linux/cryptouser.h
> @@ -31,7 +31,7 @@ enum {
>  #define CRYPTO_MSG_MAX (__CRYPTO_MSG_MAX - 1)
>  #define CRYPTO_NR_MSGTYPES (CRYPTO_MSG_MAX + 1 - CRYPTO_MSG_BASE)
>  
> -#define CRYPTO_MAX_NAME CRYPTO_MAX_ALG_NAME
> +#define CRYPTO_MAX_NAME 64
>  
>  /* Netlink message attributes.  */
>  enum crypto_attr_type_t {
> @@ -53,9 +53,9 @@ enum crypto_attr_type_t {
>  };
>  
>  struct crypto_user_alg {
> - char cru_name[CRYPTO_MAX_ALG_NAME];
> - char cru_driver_name[CRYPTO_MAX_ALG_NAME];
> - char cru_module_name[CRYPTO_MAX_ALG_NAME];
> + char 

RE: [PATCH net 1/1] net: tcp: Don't increase the TCP_MIB_OUTRSTS when fail to transmit RST

[Gao Feng]

Hi Neal

> -Original Message-
> 
> On Thu, Apr 6, 2017 at 10:05 AM, Gao Feng  wrote:
> > If so, we should increase the TCP_MIB_OUTRSTS too when fail to alloc skb.
> > When machine is overloaded and mem is exhausted, it may fail to alloc skb.
> 
> Moving the increment of TCP_MIB_OUTRSTS to the top of
> tcp_send_active_reset() sounds fine to me.
> 
> neal
I sent the v2 patch, and didn't change the tcp_v4_send_reset and 
tcp_v6_send_response.
Because I think they are only used during connecting. The rst count is not as 
important as 
tcp_send_active_reset.

Regards
Feng



.

Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[Alexander Sverdlin]

Hi!

On 06/04/17 10:15, Herbert Xu wrote:
> On Thu, Mar 16, 2017 at 03:16:29PM +0100, Alexander Sverdlin wrote:
>> This is a regression caused by 856e3f4092
>> ("crypto: seqiv - Add support for new AEAD interface")
>>
>> As I've said above, I can offer one of the two solutions, which patch should 
>> I send?
>> Or do you see any better alternatives?
> Here is a series of patches which should fix the problem.
> 
> The first three patches prepare the user-space interfaces to deal
> with longer names.  The final patch extends it.
> 
> Note that with crypto_user I haven't actually extended it to
> configure longer names.  It'll only be able to configure names
> less than 64 bytes.  However, it should be able to dump/read
> algorithms with longer names, albeit the name will be truncated
> to 64 bytes length.
> 
> Steffen, when convenient could you look into extending the crypto
> user interface to handle longer names (preferably arbitraty length
> since netlink should be able to deal with that)?
> 
> Likewise xfrm is still fixed to 64 bytes long.  But this should
> be OK as the problematic case only arises with IV generators for
> now and we do not allow IV generators to be specified through xfrm.
> 
> af_alg on the other hand now allows arbitrarily long names.

I'm not sure about patch 2 (as I've replied separately), but I've applied
and tested the whole series and it at least solves the original problem
with long algorithm name.

> As the final patch depends on all three it would be easiest if
> we pushed the xfrm patch through the crypto tree.  Steffen/David?

-- 
Best regards,
Alexander Sverdlin.