Re: [PATCH net-next v1] net: gro_cells: move backlog drop handling outside the local lock

在 2026/9/15 20:26, Eric Dumazet 写道:

On Tue, Sep 15, 2026 at 5:16 AM Xuanqiang Luo [off-list ref] wrote:

quoted

From: Xuanqiang Luo <redacted>

When the GRO cell queue length exceeds max_backlog, gro_cells_receive()
holds bh_lock while updating the RX drop counter and freeing the rejected
skb. The skb has not been queued, and the counter is updated with
this_cpu_inc(), so neither operation requires the queue lock.

Release bh_lock before accounting for and freeing the dropped skb to
shorten the critical section.

local_lock_nested_bh() is only a real lock on PREEMPT_RT. On other
kernels it is a lockdep assertion, so this patch generates the same
code. And this is the backlog overflow path: by definition we are
already dropping packets there. Shortening this "critical section"
buys nothing measurable, and there is no benchmark in the changelog.

Also, the result is harder to read than what it replaces: a label
named "unlock" that actually drops the skb, falling through into
"drop:", and a backward goto past the return. Three labels for a
20-line function.

If you want to improve gro_cells_receive(), please add drop reasons
instead: SKB_DROP_REASON_DEV_READY for the !IFF_UP case and
SKB_DROP_REASON_CPU_BACKLOG for the overflow one. That actually helps
people tracking down drops.

Thank you for the quick reply!

I see your point. I'll take another look.

Thanks,
Xuanqiang

quoted

Signed-off-by: Xuanqiang Luo <redacted>
---
  net/core/gro_cells.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c
index d8c0a28671201..f46cecd246aab 100644
--- a/net/core/gro_cells.c
+++ b/net/core/gro_cells.c

@@ -14,7 +14,6 @@ struct gro_cell {
  int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
  {
         struct net_device *dev = skb->dev;
-       bool have_bh_lock = false;
         struct gro_cell *cell;
         int res;

@@ -26,32 +25,33 @@ int gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)

         if (!gcells->cells || skb_cloned(skb) || netif_elide_gro(dev)) {
                 res = netif_rx(skb);
-               goto unlock;
+               goto out_rcu;
         }

         local_lock_nested_bh(&gcells->cells->bh_lock);
-       have_bh_lock = true;
         cell = this_cpu_ptr(gcells->cells);

-       if (skb_queue_len(&cell->napi_skbs) > READ_ONCE(net_hotdata.max_backlog)) {
-drop:
-               dev_core_stats_rx_dropped_inc(dev);
-               kfree_skb(skb);
-               res = NET_RX_DROP;
+       if (skb_queue_len(&cell->napi_skbs) > READ_ONCE(net_hotdata.max_backlog))
                 goto unlock;
-       }

         __skb_queue_tail(&cell->napi_skbs, skb);
         if (skb_queue_len(&cell->napi_skbs) == 1)
                 napi_schedule(&cell->napi);

+       local_unlock_nested_bh(&gcells->cells->bh_lock);
         res = NET_RX_SUCCESS;

-unlock:
-       if (have_bh_lock)
-               local_unlock_nested_bh(&gcells->cells->bh_lock);
+out_rcu:
         rcu_read_unlock();
         return res;
+
+unlock:
+       local_unlock_nested_bh(&gcells->cells->bh_lock);
+drop:
+       dev_core_stats_rx_dropped_inc(dev);
+       kfree_skb(skb);
+       res = NET_RX_DROP;
+       goto out_rcu;
  }
  EXPORT_SYMBOL(gro_cells_receive);

base-commit: 1142eb185b05db61a78130890fc4ed268f4cb4e6
--
2.43.0