[net-next PATCH 0/7] net: bulk alloc side and more bulk free drivers

[net-next PATCH 0/7] net: bulk alloc side and more bulk free drivers

[Jesper Dangaard Brouer]

This patchset use the bulk ALLOC side of the kmem_cache bulk APIs, for
SKB allocations.  The bulk free side got enabled in merge commit
3134b9f019f2 ("net: mitigating kmem_cache free slowpath").

The first two patches is a followup on the free-side, which enables
bulk-free in the drivers mlx4 and mlx5 (dev_kfree_skb -> napi_consume_skb).

Rest of patchset is focused on bulk alloc-side.  We start with a
conservative bulk alloc of 8 SKB, which all drivers using the
napi_alloc_skb() call will benefit from.  Then the API is extended to,
allow driver hinting on needed SKBs (only some drivers know this
size), and mlx5 driver is the first user of hinting.

Small hint for people wanting to tune their systems. Default number of
SKB objects per slab-page is 32 objects.  This limits the bulking
sizes for the SLUB allocator.  SLUB can be tuned via kernel cmdline
boot option slub_min_objects=128.  Increasing this gives a significant
performance boost, but at the cost of more memory "waste" inside
kmem_cache/slab allocator.

Patchset based on net-next at commit 3ebeac1d0295

---

Jesper Dangaard Brouer (7):
      mlx5: use napi_consume_skb API to get bulk free operations
      mlx4: use napi_consume_skb API to get bulk free operations
      net: bulk alloc and reuse of SKBs in NAPI context
      mlx5: use napi_alloc_skb API to get SKB bulk allocations
      mlx4: use napi_alloc_skb API to get SKB bulk allocations
      net: introduce napi_alloc_skb_hint() for more use-cases
      mlx5: hint the NAPI alloc skb API about the expected bulk size


 drivers/net/ethernet/mellanox/mlx4/en_rx.c        |    7 +-
 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   19 ++++-
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    5 +
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c   |   11 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 +
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    4 +
 include/linux/skbuff.h                            |   19 ++++-
 net/core/skbuff.c                                 |   75 +++++++++++++--------
 8 files changed, 92 insertions(+), 52 deletions(-)

[net-next PATCH 1/7] mlx5: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is needed by napi_consume_skb() to detect
if called from netpoll.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 9c0e80e64b43..a0708782eb78 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -619,7 +619,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 void mlx5e_completion_event(struct mlx5_core_cq *mcq);
 void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq);
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
 bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index c34f4f3e9537..996b13a5042f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -336,7 +336,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
 	return mlx5e_sq_xmit(sq, skb);
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq *sq;
 	u32 dma_fifo_cc;
@@ -412,7 +412,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
 			npkts++;
 			nbytes += wi->num_bytes;
 			sqcc += wi->num_wqebbs;
-			dev_kfree_skb(skb);
+			napi_consume_skb(skb, napi_budget);
 		} while (!last_wqe);
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 4ac8d716dbdd..d244acee63a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -60,7 +60,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	clear_bit(MLX5E_CHANNEL_NAPI_SCHED, &c->flags);
 
 	for (i = 0; i < c->num_tc; i++)
-		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq);
+		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq, budget);
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;

[net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is needed by napi_consume_skb() to detect
if called from netpoll.

For mlx4 driver, the mlx4_en_stop_port() call cleanup the entire
TX ring via mlx4_en_free_tx_buf().  To handle this situation,
napi budget value -1 is used for indicating this call happens
outside NAPI context.  To reflect this, variable is called napi_mode
for the function call that needed this distinction.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index e0946ab22010..b94ed84646b0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
 
 static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring,
-				int index, u8 owner, u64 timestamp)
+				int index, u8 owner, u64 timestamp,
+				int napi_mode)
 {
 	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
 	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
@@ -347,7 +348,11 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 			}
 		}
 	}
-	dev_consume_skb_any(skb);
+	if (unlikely(napi_mode < 0))
+		dev_consume_skb_any(skb); /* none-NAPI via mlx4_en_stop_port */
+	else
+		napi_consume_skb(skb, napi_mode);
+
 	return tx_info->nr_txbb;
 }
 
@@ -371,7 +376,9 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 	while (ring->cons != ring->prod) {
 		ring->last_nr_txbb = mlx4_en_free_tx_desc(priv, ring,
 						ring->cons & ring->size_mask,
-						!!(ring->cons & ring->size), 0);
+						!!(ring->cons & ring->size), 0,
+						-1 /* none-NAPI caller */
+			);
 		ring->cons += ring->last_nr_txbb;
 		cnt++;
 	}
@@ -385,7 +392,7 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 }
 
 static bool mlx4_en_process_tx_cq(struct net_device *dev,
-				 struct mlx4_en_cq *cq)
+				  struct mlx4_en_cq *cq, int napi_budget)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_cq *mcq = &cq->mcq;
@@ -451,7 +458,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
 			last_nr_txbb = mlx4_en_free_tx_desc(
 					priv, ring, ring_index,
 					!!((ring_cons + txbbs_skipped) &
-					ring->size), timestamp);
+					ring->size), timestamp, napi_budget);
 
 			mlx4_en_stamp_wqe(priv, ring, stamp_index,
 					  !!((ring_cons + txbbs_stamp) &
@@ -511,7 +518,7 @@ int mlx4_en_poll_tx_cq(struct napi_struct *napi, int budget)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int clean_complete;
 
-	clean_complete = mlx4_en_process_tx_cq(dev, cq);
+	clean_complete = mlx4_en_process_tx_cq(dev, cq, budget);
 	if (!clean_complete)
 		return budget;
 

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[David Miller]

From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Fri, 04 Mar 2016 14:01:33 +0100

> @@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
>  
>  static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
>  				struct mlx4_en_tx_ring *ring,
> -				int index, u8 owner, u64 timestamp)
> +				int index, u8 owner, u64 timestamp,
> +				int napi_mode)
>  {
>  	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
>  	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
> @@ -347,7 +348,11 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
>  			}
>  		}
>  	}
> -	dev_consume_skb_any(skb);
> +	if (unlikely(napi_mode < 0))
> +		dev_consume_skb_any(skb); /* none-NAPI via mlx4_en_stop_port */
> +	else
> +		napi_consume_skb(skb, napi_mode);
> +
>  	return tx_info->nr_txbb;
>  }

If '0' is the signal that napi_consume_skb() uses to detect the case where we
can't bulk, just pass that instead of having a special test here on yet another
special value "-1".

If it makes any nicer, you can define a NAPI_BUDGET_FROM_NETPOLL macro
or similar.

I also wonder if passing the budget around all the way down to
napi_consume_skb() is the cleanest thing to do, as we just want to
know if bulk freeing is possible or not.

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

On Tue, 08 Mar 2016 14:24:22 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Fri, 04 Mar 2016 14:01:33 +0100
> 
> > @@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
> >  
> >  static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
> >  				struct mlx4_en_tx_ring *ring,
> > -				int index, u8 owner, u64 timestamp)
> > +				int index, u8 owner, u64 timestamp,
> > +				int napi_mode)
> >  {
> >  	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
> >  	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
> > @@ -347,7 +348,11 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
> >  			}
> >  		}
> >  	}
> > -	dev_consume_skb_any(skb);
> > +	if (unlikely(napi_mode < 0))
> > +		dev_consume_skb_any(skb); /* none-NAPI via mlx4_en_stop_port */
> > +	else
> > +		napi_consume_skb(skb, napi_mode);
> > +
> >  	return tx_info->nr_txbb;
> >  }  
> 
> If '0' is the signal that napi_consume_skb() uses to detect the case where we
> can't bulk, just pass that instead of having a special test here on yet another
> special value "-1".

Cannot use '0' to signal this, because napi_consume_skb() invoke
dev_consume_skb_irq(), and here we need dev_consume_skb_any(), as
mlx4_en_stop_port() assume memory is released immediately.

I guess, we can (in napi_consume_skb) just replace the
dev_consume_skb_irq() with dev_consume_skb_any(), and then use '0' to
signal both situations?


> If it makes any nicer, you can define a NAPI_BUDGET_FROM_NETPOLL macro
> or similar.
> 
> I also wonder if passing the budget around all the way down to
> napi_consume_skb() is the cleanest thing to do, as we just want to
> know if bulk freeing is possible or not.

Passing the budget down was Alex'es design.  Axel any thoughts?

Perhaps we can use another way to detect if bulk freeing is possible?
E.g. using test in_serving_softirq() ?

   if (!in_serving_softirq())
       dev_consume_skb_any(skb); /* cannot bulk free */

Or maybe in_softirq() is enough? (to also allows callers having bh disabled).

I do wonder how expensive this check is... as it goes into a code
hotpath, which is very unlikely.  The good thing would be, that we
handle if buggy drivers call this function from a none softirq context
(as these bugs could be hard to catch).

Can netpoll ever be called from softirq or with BH disabled? (It
disables IRQs, which would break calling kmem_cache_free_bulk).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Alexander Duyck]

On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 08 Mar 2016 14:24:22 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Jesper Dangaard Brouer <brouer@redhat.com>
>> Date: Fri, 04 Mar 2016 14:01:33 +0100
>>
>> > @@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
>> >
>> >  static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
>> >                             struct mlx4_en_tx_ring *ring,
>> > -                           int index, u8 owner, u64 timestamp)
>> > +                           int index, u8 owner, u64 timestamp,
>> > +                           int napi_mode)
>> >  {
>> >     struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
>> >     struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
>> > @@ -347,7 +348,11 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
>> >                     }
>> >             }
>> >     }
>> > -   dev_consume_skb_any(skb);
>> > +   if (unlikely(napi_mode < 0))
>> > +           dev_consume_skb_any(skb); /* none-NAPI via mlx4_en_stop_port */
>> > +   else
>> > +           napi_consume_skb(skb, napi_mode);
>> > +
>> >     return tx_info->nr_txbb;
>> >  }
>>
>> If '0' is the signal that napi_consume_skb() uses to detect the case where we
>> can't bulk, just pass that instead of having a special test here on yet another
>> special value "-1".
>
> Cannot use '0' to signal this, because napi_consume_skb() invoke
> dev_consume_skb_irq(), and here we need dev_consume_skb_any(), as
> mlx4_en_stop_port() assume memory is released immediately.
>
> I guess, we can (in napi_consume_skb) just replace the
> dev_consume_skb_irq() with dev_consume_skb_any(), and then use '0' to
> signal both situations?
>
>
>> If it makes any nicer, you can define a NAPI_BUDGET_FROM_NETPOLL macro
>> or similar.
>>
>> I also wonder if passing the budget around all the way down to
>> napi_consume_skb() is the cleanest thing to do, as we just want to
>> know if bulk freeing is possible or not.
>
> Passing the budget down was Alex'es design.  Axel any thoughts?

I'd say just use dev_consume_skb_any in the bulk free instead of
dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
come up often.

> Perhaps we can use another way to detect if bulk freeing is possible?
> E.g. using test in_serving_softirq() ?
>
>    if (!in_serving_softirq())
>        dev_consume_skb_any(skb); /* cannot bulk free */
>
> Or maybe in_softirq() is enough? (to also allows callers having bh disabled).

I wasn't so much concerned about the check as us getting it mixed up.
For example the Tx path has soft IRQ disabled if I recall correctly.
Is this called from there?  At least with the "any" approach you can
guarantee you don't leave stale buffers sitting in the lists until
someone wakes up NAPI.

> I do wonder how expensive this check is... as it goes into a code
> hotpath, which is very unlikely.  The good thing would be, that we
> handle if buggy drivers call this function from a none softirq context
> (as these bugs could be hard to catch).
>
> Can netpoll ever be called from softirq or with BH disabled? (It
> disables IRQs, which would break calling kmem_cache_free_bulk).

It is better for us to switch things out so that the napi_consume_skb
is the fast path with dev_consume_skb_any as the slow.  There are too
many scenarios where we could be invoking something that makes use of
this within the Tx path so it is probably easiest to just solve it
that way so we don't have to deal with it again in the future.

- Alex

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[David Miller]

From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Wed, 9 Mar 2016 08:47:58 -0800

> On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
>> Passing the budget down was Alex'es design.  Axel any thoughts?
> 
> I'd say just use dev_consume_skb_any in the bulk free instead of
> dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
> come up often.

Agreed.

>> I do wonder how expensive this check is... as it goes into a code
>> hotpath, which is very unlikely.  The good thing would be, that we
>> handle if buggy drivers call this function from a none softirq context
>> (as these bugs could be hard to catch).
>>
>> Can netpoll ever be called from softirq or with BH disabled? (It
>> disables IRQs, which would break calling kmem_cache_free_bulk).
> 
> It is better for us to switch things out so that the napi_consume_skb
> is the fast path with dev_consume_skb_any as the slow.  There are too
> many scenarios where we could be invoking something that makes use of
> this within the Tx path so it is probably easiest to just solve it
> that way so we don't have to deal with it again in the future.

Indeed.

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

On Wed, 09 Mar 2016 16:03:20 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Alexander Duyck <alexander.duyck@gmail.com>
> Date: Wed, 9 Mar 2016 08:47:58 -0800
> 
> > On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:  
> >> Passing the budget down was Alex'es design.  Axel any thoughts?  
> > 
> > I'd say just use dev_consume_skb_any in the bulk free instead of
> > dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
> > come up often.  
> 
> Agreed.
> 
> >> I do wonder how expensive this check is... as it goes into a code
> >> hotpath, which is very unlikely.  The good thing would be, that we
> >> handle if buggy drivers call this function from a none softirq context
> >> (as these bugs could be hard to catch).
> >>
> >> Can netpoll ever be called from softirq or with BH disabled? (It
> >> disables IRQs, which would break calling kmem_cache_free_bulk).  
> > 
> > It is better for us to switch things out so that the napi_consume_skb
> > is the fast path with dev_consume_skb_any as the slow.  There are too
> > many scenarios where we could be invoking something that makes use of
> > this within the Tx path so it is probably easiest to just solve it
> > that way so we don't have to deal with it again in the future.  
> 
> Indeed.

So, if I understand you correctly, then we drop the budget parameter
and check for in_softirq(), like:

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7af7ec635d90..a3c61a9b65d2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -796,14 +796,14 @@ void __kfree_skb_defer(struct sk_buff *skb)
        _kfree_skb_defer(skb);
 }
 
-void napi_consume_skb(struct sk_buff *skb, int budget)
+void napi_consume_skb(struct sk_buff *skb)
 {
        if (unlikely(!skb))
                return;
 
-       /* if budget is 0 assume netpoll w/ IRQs disabled */
-       if (unlikely(!budget)) {
-               dev_consume_skb_irq(skb);
+       /* Handle if not called from NAPI context, and netpoll invocation */
+       if (unlikely(!in_softirq())) {
+               dev_consume_skb_any(skb);
                return;
        }


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Alexander Duyck]

On Wed, Mar 9, 2016 at 1:36 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Wed, 09 Mar 2016 16:03:20 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Alexander Duyck <alexander.duyck@gmail.com>
>> Date: Wed, 9 Mar 2016 08:47:58 -0800
>>
>> > On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
>> > <brouer@redhat.com> wrote:
>> >> Passing the budget down was Alex'es design.  Axel any thoughts?
>> >
>> > I'd say just use dev_consume_skb_any in the bulk free instead of
>> > dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
>> > come up often.
>>
>> Agreed.
>>
>> >> I do wonder how expensive this check is... as it goes into a code
>> >> hotpath, which is very unlikely.  The good thing would be, that we
>> >> handle if buggy drivers call this function from a none softirq context
>> >> (as these bugs could be hard to catch).
>> >>
>> >> Can netpoll ever be called from softirq or with BH disabled? (It
>> >> disables IRQs, which would break calling kmem_cache_free_bulk).
>> >
>> > It is better for us to switch things out so that the napi_consume_skb
>> > is the fast path with dev_consume_skb_any as the slow.  There are too
>> > many scenarios where we could be invoking something that makes use of
>> > this within the Tx path so it is probably easiest to just solve it
>> > that way so we don't have to deal with it again in the future.
>>
>> Indeed.
>
> So, if I understand you correctly, then we drop the budget parameter
> and check for in_softirq(), like:
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7af7ec635d90..a3c61a9b65d2 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -796,14 +796,14 @@ void __kfree_skb_defer(struct sk_buff *skb)
>         _kfree_skb_defer(skb);
>  }
>
> -void napi_consume_skb(struct sk_buff *skb, int budget)
> +void napi_consume_skb(struct sk_buff *skb)
>  {
>         if (unlikely(!skb))
>                 return;
>
> -       /* if budget is 0 assume netpoll w/ IRQs disabled */
> -       if (unlikely(!budget)) {
> -               dev_consume_skb_irq(skb);
> +       /* Handle if not called from NAPI context, and netpoll invocation */
> +       if (unlikely(!in_softirq())) {
> +               dev_consume_skb_any(skb);
>                 return;
>         }
>

No.  We still need to have the budget value.  What we do though is
have that feed into dev_consume_skb_any.

The problem with using in_softirq is that it will trigger if softirqs
are just disabled so there are more possible paths where it is
possible.  For example the transmit path has bottom halves disabled so
I am pretty sure it might trigger this as well.  We want this to only
execute when we are running from a NAPI polling routine with a
non-zero budget.

- Alex

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

On Wed, 9 Mar 2016 13:43:59 -0800
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> On Wed, Mar 9, 2016 at 1:36 PM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > On Wed, 09 Mar 2016 16:03:20 -0500 (EST)
> > David Miller <davem@davemloft.net> wrote:
> >  
> >> From: Alexander Duyck <alexander.duyck@gmail.com>
> >> Date: Wed, 9 Mar 2016 08:47:58 -0800
> >>  
> >> > On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
> >> > <brouer@redhat.com> wrote:  
> >> >> Passing the budget down was Alex'es design.  Axel any thoughts?  
> >> >
> >> > I'd say just use dev_consume_skb_any in the bulk free instead of
> >> > dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
> >> > come up often.  
> >>
> >> Agreed.
> >>  
> >> >> I do wonder how expensive this check is... as it goes into a code
> >> >> hotpath, which is very unlikely.  The good thing would be, that we
> >> >> handle if buggy drivers call this function from a none softirq context
> >> >> (as these bugs could be hard to catch).
> >> >>
> >> >> Can netpoll ever be called from softirq or with BH disabled? (It
> >> >> disables IRQs, which would break calling kmem_cache_free_bulk).  
> >> >
> >> > It is better for us to switch things out so that the napi_consume_skb
> >> > is the fast path with dev_consume_skb_any as the slow.  There are too
> >> > many scenarios where we could be invoking something that makes use of
> >> > this within the Tx path so it is probably easiest to just solve it
> >> > that way so we don't have to deal with it again in the future.  
> >>
> >> Indeed.  
> >
> > So, if I understand you correctly, then we drop the budget parameter
> > and check for in_softirq(), like:
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 7af7ec635d90..a3c61a9b65d2 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -796,14 +796,14 @@ void __kfree_skb_defer(struct sk_buff *skb)
> >         _kfree_skb_defer(skb);
> >  }
> >
> > -void napi_consume_skb(struct sk_buff *skb, int budget)
> > +void napi_consume_skb(struct sk_buff *skb)
> >  {
> >         if (unlikely(!skb))
> >                 return;
> >
> > -       /* if budget is 0 assume netpoll w/ IRQs disabled */
> > -       if (unlikely(!budget)) {
> > -               dev_consume_skb_irq(skb);
> > +       /* Handle if not called from NAPI context, and netpoll invocation */
> > +       if (unlikely(!in_softirq())) {
> > +               dev_consume_skb_any(skb);
> >                 return;
> >         }
> >  
> 
> No.  We still need to have the budget value.  What we do though is
> have that feed into dev_consume_skb_any.
> 
> The problem with using in_softirq is that it will trigger if softirqs
> are just disabled so there are more possible paths where it is
> possible.  For example the transmit path has bottom halves disabled so
> I am pretty sure it might trigger this as well.  We want this to only
> execute when we are running from a NAPI polling routine with a
> non-zero budget.

What about using in_serving_softirq() instead of in_softirq() ?
(would that allow us to drop the budget parameter?)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [net-next PATCH 2/7] mlx4: use napi_consume_skb API to get bulk free operations

[Alexander Duyck]

On Wed, Mar 9, 2016 at 1:47 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Wed, 9 Mar 2016 13:43:59 -0800
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
>> On Wed, Mar 9, 2016 at 1:36 PM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>> > On Wed, 09 Mar 2016 16:03:20 -0500 (EST)
>> > David Miller <davem@davemloft.net> wrote:
>> >
>> >> From: Alexander Duyck <alexander.duyck@gmail.com>
>> >> Date: Wed, 9 Mar 2016 08:47:58 -0800
>> >>
>> >> > On Wed, Mar 9, 2016 at 3:00 AM, Jesper Dangaard Brouer
>> >> > <brouer@redhat.com> wrote:
>> >> >> Passing the budget down was Alex'es design.  Axel any thoughts?
>> >> >
>> >> > I'd say just use dev_consume_skb_any in the bulk free instead of
>> >> > dev_consume_skb_irq.  This is slow path, as you said, so it shouldn't
>> >> > come up often.
>> >>
>> >> Agreed.
>> >>
>> >> >> I do wonder how expensive this check is... as it goes into a code
>> >> >> hotpath, which is very unlikely.  The good thing would be, that we
>> >> >> handle if buggy drivers call this function from a none softirq context
>> >> >> (as these bugs could be hard to catch).
>> >> >>
>> >> >> Can netpoll ever be called from softirq or with BH disabled? (It
>> >> >> disables IRQs, which would break calling kmem_cache_free_bulk).
>> >> >
>> >> > It is better for us to switch things out so that the napi_consume_skb
>> >> > is the fast path with dev_consume_skb_any as the slow.  There are too
>> >> > many scenarios where we could be invoking something that makes use of
>> >> > this within the Tx path so it is probably easiest to just solve it
>> >> > that way so we don't have to deal with it again in the future.
>> >>
>> >> Indeed.
>> >
>> > So, if I understand you correctly, then we drop the budget parameter
>> > and check for in_softirq(), like:
>> >
>> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> > index 7af7ec635d90..a3c61a9b65d2 100644
>> > --- a/net/core/skbuff.c
>> > +++ b/net/core/skbuff.c
>> > @@ -796,14 +796,14 @@ void __kfree_skb_defer(struct sk_buff *skb)
>> >         _kfree_skb_defer(skb);
>> >  }
>> >
>> > -void napi_consume_skb(struct sk_buff *skb, int budget)
>> > +void napi_consume_skb(struct sk_buff *skb)
>> >  {
>> >         if (unlikely(!skb))
>> >                 return;
>> >
>> > -       /* if budget is 0 assume netpoll w/ IRQs disabled */
>> > -       if (unlikely(!budget)) {
>> > -               dev_consume_skb_irq(skb);
>> > +       /* Handle if not called from NAPI context, and netpoll invocation */
>> > +       if (unlikely(!in_softirq())) {
>> > +               dev_consume_skb_any(skb);
>> >                 return;
>> >         }
>> >
>>
>> No.  We still need to have the budget value.  What we do though is
>> have that feed into dev_consume_skb_any.
>>
>> The problem with using in_softirq is that it will trigger if softirqs
>> are just disabled so there are more possible paths where it is
>> possible.  For example the transmit path has bottom halves disabled so
>> I am pretty sure it might trigger this as well.  We want this to only
>> execute when we are running from a NAPI polling routine with a
>> non-zero budget.
>
> What about using in_serving_softirq() instead of in_softirq() ?
> (would that allow us to drop the budget parameter?)

The problem is there are multiple softirq handlers you could be
referring to.  NAPI is just one.  What if for example someone sets up
a tasklet that has to perform some sort of reset with RTNL held.  I am
pretty sure we don't want the tasklet using the NAPI free context
since there will be nothing to actually free the buffers at the end of
it.  We want to avoid that.  That is why I was using the budget value.

- Alex

[net-next PATCH V2 0/3] net: bulk free adjustment and two driver use-cases

[Jesper Dangaard Brouer]

I've split out the bulk free adjustments, from the bulk alloc patches,
as I want the adjustment to napi_consume_skb to be in same kernel cycle
the API was introduced.

Adjustments based on discussion:
 Subj: "mlx4: use napi_consume_skb API to get bulk free operations"
 http://thread.gmane.org/gmane.linux.network/402503/focus=403386

Patchset based on net-next at commit 3ebeac1d0295

---

Jesper Dangaard Brouer (3):
      net: adjust napi_consume_skb to handle none-NAPI callers
      mlx4: use napi_consume_skb API to get bulk free operations
      mlx5: use napi_consume_skb API to get bulk free operations


 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   16 ++++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 net/core/skbuff.c                                 |    4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

[net-next PATCH V2 1/3] net: adjust napi_consume_skb to handle none-NAPI callers

[Jesper Dangaard Brouer]

Some drivers reuse/share code paths that free SKBs between NAPI
and none-NAPI calls. Adjust napi_consume_skb to handle this
use-case.

Before, calls from netpoll (w/ IRQs disabled) was handled and
indicated with a budget zero indication.  Use the same zero
indication to handle calls not originating from NAPI/softirq.
Simply handled by using dev_consume_skb_any().

This adds an extra branch+call for the netpoll case (checking
in_irq() + irqs_disabled()), but that is okay as this is a slowpath.

Suggested-by: Alexander Duyck <aduyck@mirantis.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 net/core/skbuff.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7af7ec635d90..bc62baa54ceb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -801,9 +801,9 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 	if (unlikely(!skb))
 		return;
 
-	/* if budget is 0 assume netpoll w/ IRQs disabled */
+	/* Zero budget indicate none-NAPI context called us, like netpoll */
 	if (unlikely(!budget)) {
-		dev_consume_skb_irq(skb);
+		dev_consume_skb_any(skb);
 		return;
 	}
 

[net-next PATCH V2 2/3] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is usually needed by napi_consume_skb()
to detect if called from netpoll.  In this patch it have an extra meaning.

For mlx4 driver, the mlx4_en_stop_port() call is done outside
NAPI/softirq context, and cleanup the entire TX ring via
mlx4_en_free_tx_buf().  The code mlx4_en_free_tx_desc() for
freeing SKBs are shared with NAPI calls.

To handle this shared use the zero budget indication is reused,
and handled appropiately in napi_consume_skb(). To reflect this,
variable is called napi_mode for the function call that needed
this distinction.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index e0946ab22010..1b41feafce9e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
 
 static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring,
-				int index, u8 owner, u64 timestamp)
+				int index, u8 owner, u64 timestamp,
+				int napi_mode)
 {
 	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
 	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
@@ -347,7 +348,8 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 			}
 		}
 	}
-	dev_consume_skb_any(skb);
+	napi_consume_skb(skb, napi_mode);
+
 	return tx_info->nr_txbb;
 }
 
@@ -371,7 +373,9 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 	while (ring->cons != ring->prod) {
 		ring->last_nr_txbb = mlx4_en_free_tx_desc(priv, ring,
 						ring->cons & ring->size_mask,
-						!!(ring->cons & ring->size), 0);
+						!!(ring->cons & ring->size), 0,
+						0 /* none-NAPI caller */
+			);
 		ring->cons += ring->last_nr_txbb;
 		cnt++;
 	}
@@ -385,7 +389,7 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 }
 
 static bool mlx4_en_process_tx_cq(struct net_device *dev,
-				 struct mlx4_en_cq *cq)
+				  struct mlx4_en_cq *cq, int napi_budget)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_cq *mcq = &cq->mcq;
@@ -451,7 +455,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
 			last_nr_txbb = mlx4_en_free_tx_desc(
 					priv, ring, ring_index,
 					!!((ring_cons + txbbs_skipped) &
-					ring->size), timestamp);
+					ring->size), timestamp, napi_budget);
 
 			mlx4_en_stamp_wqe(priv, ring, stamp_index,
 					  !!((ring_cons + txbbs_stamp) &
@@ -511,7 +515,7 @@ int mlx4_en_poll_tx_cq(struct napi_struct *napi, int budget)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int clean_complete;
 
-	clean_complete = mlx4_en_process_tx_cq(dev, cq);
+	clean_complete = mlx4_en_process_tx_cq(dev, cq, budget);
 	if (!clean_complete)
 		return budget;
 

Re: [net-next PATCH V2 2/3] mlx4: use napi_consume_skb API to get bulk free operations

[Sergei Shtylyov]

Hello.

On 3/10/2016 3:15 PM, Jesper Dangaard Brouer wrote:

> Bulk free of SKBs happen transparently by the API call napi_consume_skb().
> The napi budget parameter is usually needed by napi_consume_skb()
> to detect if called from netpoll.  In this patch it have an extra meaning.

    It has.

> For mlx4 driver, the mlx4_en_stop_port() call is done outside
> NAPI/softirq context, and cleanup the entire TX ring via
> mlx4_en_free_tx_buf().  The code mlx4_en_free_tx_desc() for
> freeing SKBs are shared with NAPI calls.
>
> To handle this shared use the zero budget indication is reused,
> and handled appropiately in napi_consume_skb(). To reflect this,

    Appropriately.

> variable is called napi_mode for the function call that needed
> this distinction.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_tx.c |   16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index e0946ab22010..1b41feafce9e 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
[...]
> @@ -371,7 +373,9 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
>   	while (ring->cons != ring->prod) {
>   		ring->last_nr_txbb = mlx4_en_free_tx_desc(priv, ring,
>   						ring->cons & ring->size_mask,
> -						!!(ring->cons & ring->size), 0);
> +						!!(ring->cons & ring->size), 0,
> +						0 /* none-NAPI caller */

    Non-NAPI, perhaps?

[...]

MBR, Sergei

[net-next PATCH V3 0/3] net: bulk free adjustment and two driver use-cases

[Jesper Dangaard Brouer]

I've split out the bulk free adjustments, from the bulk alloc patches,
as I want the adjustment to napi_consume_skb be in same kernel cycle
the API was introduced.

Adjustments based on discussion:
 Subj: "mlx4: use napi_consume_skb API to get bulk free operations"
 http://thread.gmane.org/gmane.linux.network/402503/focus=403386

Patchset based on net-next at commit 3ebeac1d0295

V3: spelling fixes from Sergei
---

Jesper Dangaard Brouer (3):
      net: adjust napi_consume_skb to handle none-NAPI callers
      mlx4: use napi_consume_skb API to get bulk free operations
      mlx5: use napi_consume_skb API to get bulk free operations


 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   16 ++++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 net/core/skbuff.c                                 |    4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

[net-next PATCH V3 1/3] net: adjust napi_consume_skb to handle none-NAPI callers

[Jesper Dangaard Brouer]

Some drivers reuse/share code paths that free SKBs between NAPI
and none-NAPI calls. Adjust napi_consume_skb to handle this
use-case.

Before, calls from netpoll (w/ IRQs disabled) was handled and
indicated with a budget zero indication.  Use the same zero
indication to handle calls not originating from NAPI/softirq.
Simply handled by using dev_consume_skb_any().

This adds an extra branch+call for the netpoll case (checking
in_irq() + irqs_disabled()), but that is okay as this is a slowpath.

Suggested-by: Alexander Duyck <aduyck@mirantis.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 net/core/skbuff.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7af7ec635d90..bc62baa54ceb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -801,9 +801,9 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 	if (unlikely(!skb))
 		return;
 
-	/* if budget is 0 assume netpoll w/ IRQs disabled */
+	/* Zero budget indicate none-NAPI context called us, like netpoll */
 	if (unlikely(!budget)) {
-		dev_consume_skb_irq(skb);
+		dev_consume_skb_any(skb);
 		return;
 	}
 

Re: [net-next PATCH V3 1/3] net: adjust napi_consume_skb to handle none-NAPI callers

[Sergei Shtylyov]

Hello.

On 03/10/2016 05:59 PM, Jesper Dangaard Brouer wrote:

> Some drivers reuse/share code paths that free SKBs between NAPI
> and none-NAPI calls. Adjust napi_consume_skb to handle this
> use-case.
>
> Before, calls from netpoll (w/ IRQs disabled) was handled and
> indicated with a budget zero indication.  Use the same zero
> indication to handle calls not originating from NAPI/softirq.
> Simply handled by using dev_consume_skb_any().
>
> This adds an extra branch+call for the netpoll case (checking
> in_irq() + irqs_disabled()), but that is okay as this is a slowpath.
>
> Suggested-by: Alexander Duyck <aduyck@mirantis.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>   net/core/skbuff.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7af7ec635d90..bc62baa54ceb 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -801,9 +801,9 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
>   	if (unlikely(!skb))
>   		return;
>
> -	/* if budget is 0 assume netpoll w/ IRQs disabled */
> +	/* Zero budget indicate none-NAPI context called us, like netpoll */

    Non-NAPI?

[...]

MBR, Sergei

Re: [net-next PATCH V3 1/3] net: adjust napi_consume_skb to handle none-NAPI callers

[Jesper Dangaard Brouer]

On Thu, 10 Mar 2016 20:21:55 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -801,9 +801,9 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
> >   	if (unlikely(!skb))
> >   		return;
> >
> > -	/* if budget is 0 assume netpoll w/ IRQs disabled */
> > +	/* Zero budget indicate none-NAPI context called us, like netpoll */  
> 
>     Non-NAPI?

Okay, I'll send a V4.  Hope there are no more nitpicking changes...
I'll also adjust the subj none-NAPI -> non-NAPI, and hope that does not
disturb patchwork.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

[net-next PATCH V4 0/3] net: bulk free adjustment and two driver use-cases

[Jesper Dangaard Brouer]

I've split out the bulk free adjustments, from the bulk alloc patches,
as I want the adjustment to napi_consume_skb be in same kernel cycle
the API was introduced.

Adjustments based on discussion:
 Subj: "mlx4: use napi_consume_skb API to get bulk free operations"
 http://thread.gmane.org/gmane.linux.network/402503/focus=403386

Patchset based on net-next at commit 3ebeac1d0295

V4: more nitpicks from Sergei
V3: spelling fixes from Sergei

---

Jesper Dangaard Brouer (3):
      net: adjust napi_consume_skb to handle non-NAPI callers
      mlx4: use napi_consume_skb API to get bulk free operations
      mlx5: use napi_consume_skb API to get bulk free operations


 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   15 +++++++++------
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 net/core/skbuff.c                                 |    4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)

[net-next PATCH V4 1/3] net: adjust napi_consume_skb to handle non-NAPI callers

[Jesper Dangaard Brouer]

Some drivers reuse/share code paths that free SKBs between NAPI
and non-NAPI calls. Adjust napi_consume_skb to handle this
use-case.

Before, calls from netpoll (w/ IRQs disabled) was handled and
indicated with a budget zero indication.  Use the same zero
indication to handle calls not originating from NAPI/softirq.
Simply handled by using dev_consume_skb_any().

This adds an extra branch+call for the netpoll case (checking
in_irq() + irqs_disabled()), but that is okay as this is a slowpath.

Suggested-by: Alexander Duyck <aduyck@mirantis.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 net/core/skbuff.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7af7ec635d90..cf63a6000405 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -801,9 +801,9 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 	if (unlikely(!skb))
 		return;
 
-	/* if budget is 0 assume netpoll w/ IRQs disabled */
+	/* Zero budget indicate non-NAPI context called us, like netpoll */
 	if (unlikely(!budget)) {
-		dev_consume_skb_irq(skb);
+		dev_consume_skb_any(skb);
 		return;
 	}
 

[net-next PATCH V4 2/3] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is usually needed by napi_consume_skb()
to detect if called from netpoll.  In this patch it has an extra meaning.

For mlx4 driver, the mlx4_en_stop_port() call is done outside
NAPI/softirq context, and cleanup the entire TX ring via
mlx4_en_free_tx_buf().  The code mlx4_en_free_tx_desc() for
freeing SKBs are shared with NAPI calls.

To handle this shared use the zero budget indication is reused,
and handled appropriately in napi_consume_skb(). To reflect this,
variable is called napi_mode for the function call that needed
this distinction.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index e0946ab22010..c0d7b7296236 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
 
 static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring,
-				int index, u8 owner, u64 timestamp)
+				int index, u8 owner, u64 timestamp,
+				int napi_mode)
 {
 	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
 	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
@@ -347,7 +348,8 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 			}
 		}
 	}
-	dev_consume_skb_any(skb);
+	napi_consume_skb(skb, napi_mode);
+
 	return tx_info->nr_txbb;
 }
 
@@ -371,7 +373,8 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 	while (ring->cons != ring->prod) {
 		ring->last_nr_txbb = mlx4_en_free_tx_desc(priv, ring,
 						ring->cons & ring->size_mask,
-						!!(ring->cons & ring->size), 0);
+						!!(ring->cons & ring->size), 0,
+						0 /* Non-NAPI caller */);
 		ring->cons += ring->last_nr_txbb;
 		cnt++;
 	}
@@ -385,7 +388,7 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 }
 
 static bool mlx4_en_process_tx_cq(struct net_device *dev,
-				 struct mlx4_en_cq *cq)
+				  struct mlx4_en_cq *cq, int napi_budget)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_cq *mcq = &cq->mcq;
@@ -451,7 +454,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
 			last_nr_txbb = mlx4_en_free_tx_desc(
 					priv, ring, ring_index,
 					!!((ring_cons + txbbs_skipped) &
-					ring->size), timestamp);
+					ring->size), timestamp, napi_budget);
 
 			mlx4_en_stamp_wqe(priv, ring, stamp_index,
 					  !!((ring_cons + txbbs_stamp) &
@@ -511,7 +514,7 @@ int mlx4_en_poll_tx_cq(struct napi_struct *napi, int budget)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int clean_complete;
 
-	clean_complete = mlx4_en_process_tx_cq(dev, cq);
+	clean_complete = mlx4_en_process_tx_cq(dev, cq, budget);
 	if (!clean_complete)
 		return budget;
 

[net-next PATCH V4 3/3] mlx5: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is needed by napi_consume_skb() to detect
if called from netpoll.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 9c0e80e64b43..a0708782eb78 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -619,7 +619,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 void mlx5e_completion_event(struct mlx5_core_cq *mcq);
 void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq);
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
 bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index c34f4f3e9537..996b13a5042f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -336,7 +336,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
 	return mlx5e_sq_xmit(sq, skb);
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq *sq;
 	u32 dma_fifo_cc;
@@ -412,7 +412,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
 			npkts++;
 			nbytes += wi->num_bytes;
 			sqcc += wi->num_wqebbs;
-			dev_kfree_skb(skb);
+			napi_consume_skb(skb, napi_budget);
 		} while (!last_wqe);
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 4ac8d716dbdd..d244acee63a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -60,7 +60,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	clear_bit(MLX5E_CHANNEL_NAPI_SCHED, &c->flags);
 
 	for (i = 0; i < c->num_tc; i++)
-		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq);
+		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq, budget);
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;

Re: [net-next PATCH V4 0/3] net: bulk free adjustment and two driver use-cases

[David Miller]

From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Fri, 11 Mar 2016 09:43:48 +0100

> I've split out the bulk free adjustments, from the bulk alloc patches,
> as I want the adjustment to napi_consume_skb be in same kernel cycle
> the API was introduced.
> 
> Adjustments based on discussion:
>  Subj: "mlx4: use napi_consume_skb API to get bulk free operations"
>  http://thread.gmane.org/gmane.linux.network/402503/focus=403386
> 
> Patchset based on net-next at commit 3ebeac1d0295
> 
> V4: more nitpicks from Sergei
> V3: spelling fixes from Sergei

Series applied, thanks Jesper.

[net-next PATCH V3 2/3] mlx4: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is usually needed by napi_consume_skb()
to detect if called from netpoll.  In this patch it has an extra meaning.

For mlx4 driver, the mlx4_en_stop_port() call is done outside
NAPI/softirq context, and cleanup the entire TX ring via
mlx4_en_free_tx_buf().  The code mlx4_en_free_tx_desc() for
freeing SKBs are shared with NAPI calls.

To handle this shared use the zero budget indication is reused,
and handled appropriately in napi_consume_skb(). To reflect this,
variable is called napi_mode for the function call that needed
this distinction.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index e0946ab22010..001a3d78168e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -276,7 +276,8 @@ static void mlx4_en_stamp_wqe(struct mlx4_en_priv *priv,
 
 static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring,
-				int index, u8 owner, u64 timestamp)
+				int index, u8 owner, u64 timestamp,
+				int napi_mode)
 {
 	struct mlx4_en_tx_info *tx_info = &ring->tx_info[index];
 	struct mlx4_en_tx_desc *tx_desc = ring->buf + index * TXBB_SIZE;
@@ -347,7 +348,8 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 			}
 		}
 	}
-	dev_consume_skb_any(skb);
+	napi_consume_skb(skb, napi_mode);
+
 	return tx_info->nr_txbb;
 }
 
@@ -371,7 +373,9 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 	while (ring->cons != ring->prod) {
 		ring->last_nr_txbb = mlx4_en_free_tx_desc(priv, ring,
 						ring->cons & ring->size_mask,
-						!!(ring->cons & ring->size), 0);
+						!!(ring->cons & ring->size), 0,
+						0 /* Non-NAPI caller */
+			);
 		ring->cons += ring->last_nr_txbb;
 		cnt++;
 	}
@@ -385,7 +389,7 @@ int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring)
 }
 
 static bool mlx4_en_process_tx_cq(struct net_device *dev,
-				 struct mlx4_en_cq *cq)
+				  struct mlx4_en_cq *cq, int napi_budget)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_cq *mcq = &cq->mcq;
@@ -451,7 +455,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
 			last_nr_txbb = mlx4_en_free_tx_desc(
 					priv, ring, ring_index,
 					!!((ring_cons + txbbs_skipped) &
-					ring->size), timestamp);
+					ring->size), timestamp, napi_budget);
 
 			mlx4_en_stamp_wqe(priv, ring, stamp_index,
 					  !!((ring_cons + txbbs_stamp) &
@@ -511,7 +515,7 @@ int mlx4_en_poll_tx_cq(struct napi_struct *napi, int budget)
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int clean_complete;
 
-	clean_complete = mlx4_en_process_tx_cq(dev, cq);
+	clean_complete = mlx4_en_process_tx_cq(dev, cq, budget);
 	if (!clean_complete)
 		return budget;
 

[net-next PATCH V3 3/3] mlx5: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is needed by napi_consume_skb() to detect
if called from netpoll.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 9c0e80e64b43..a0708782eb78 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -619,7 +619,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 void mlx5e_completion_event(struct mlx5_core_cq *mcq);
 void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq);
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
 bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index c34f4f3e9537..996b13a5042f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -336,7 +336,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
 	return mlx5e_sq_xmit(sq, skb);
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq *sq;
 	u32 dma_fifo_cc;
@@ -412,7 +412,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
 			npkts++;
 			nbytes += wi->num_bytes;
 			sqcc += wi->num_wqebbs;
-			dev_kfree_skb(skb);
+			napi_consume_skb(skb, napi_budget);
 		} while (!last_wqe);
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 4ac8d716dbdd..d244acee63a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -60,7 +60,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	clear_bit(MLX5E_CHANNEL_NAPI_SCHED, &c->flags);
 
 	for (i = 0; i < c->num_tc; i++)
-		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq);
+		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq, budget);
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;

[net-next PATCH V2 3/3] mlx5: use napi_consume_skb API to get bulk free operations

[Jesper Dangaard Brouer]

Bulk free of SKBs happen transparently by the API call napi_consume_skb().
The napi budget parameter is needed by napi_consume_skb() to detect
if called from netpoll.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 9c0e80e64b43..a0708782eb78 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -619,7 +619,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 void mlx5e_completion_event(struct mlx5_core_cq *mcq);
 void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq);
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
 bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index c34f4f3e9537..996b13a5042f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -336,7 +336,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
 	return mlx5e_sq_xmit(sq, skb);
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq *sq;
 	u32 dma_fifo_cc;
@@ -412,7 +412,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
 			npkts++;
 			nbytes += wi->num_bytes;
 			sqcc += wi->num_wqebbs;
-			dev_kfree_skb(skb);
+			napi_consume_skb(skb, napi_budget);
 		} while (!last_wqe);
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index 4ac8d716dbdd..d244acee63a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -60,7 +60,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	clear_bit(MLX5E_CHANNEL_NAPI_SCHED, &c->flags);
 
 	for (i = 0; i < c->num_tc; i++)
-		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq);
+		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq, budget);
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;