Re: [PATCH net] net: gro: do not keep too many GRO packets in napi->rx_list
From: Saeed Mahameed <saeed@kernel.org>
Date: 2021-02-04 22:15:35
On Thu, 2021-02-04 at 13:31 -0800, Eric Dumazet wrote:
From: Eric Dumazet <edumazet@google.com>
Commit c80794323e82 ("net: Fix packet reordering caused by GRO and
listified RX cooperation") had the unfortunate effect of adding
latencies in common workloads.
Before the patch, GRO packets were immediately passed to
upper stacks.
After the patch, we can accumulate quite a lot of GRO
packets (depdending on NAPI budget).Why napi budget ? looking at the code it seems to be more related to MAX_GRO_SKBS * gro_normal_batch, since we are counting GRO SKBs as 1 but maybe i am missing some information about the actual issue you are hitting.
quoted hunk ↗ jump to hunk
My fix is counting in napi->rx_count number of segments instead of number of logical packets. Fixes: c80794323e82 ("net: Fix packet reordering caused by GRO and listified RX cooperation") Signed-off-by: Eric Dumazet <edumazet@google.com> Bisected-by: John Sperbeck [off-list ref] Tested-by: Jian Yang <redacted> Cc: Maxim Mikityanskiy <redacted> Cc: Alexander Lobakin <redacted> Cc: Saeed Mahameed <redacted> Cc: Edward Cree <redacted> --- net/core/dev.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)diff --git a/net/core/dev.c b/net/core/dev.cindex a979b86dbacda9dfe31dd8b269024f7f0f5a8ef1..449b45b843d40ece7dd1e2ed6a5 996ee1db9f591 100644--- a/net/core/dev.c +++ b/net/core/dev.c@@ -5735,10 +5735,11 @@ static void gro_normal_list(structnapi_struct *napi) /* Queue one GRO_NORMAL SKB up for list processing. If batch size exceeded, * pass the whole batch up to the stack. */ -static void gro_normal_one(struct napi_struct *napi, struct sk_buff *skb) +static void gro_normal_one(struct napi_struct *napi, struct sk_buff *skb, int segs) { list_add_tail(&skb->list, &napi->rx_list); - if (++napi->rx_count >= gro_normal_batch) + napi->rx_count += segs; + if (napi->rx_count >= gro_normal_batch) gro_normal_list(napi); }@@ -5777,7 +5778,7 @@ static int napi_gro_complete(struct napi_struct*napi, struct sk_buff *skb) } out: - gro_normal_one(napi, skb); + gro_normal_one(napi, skb, NAPI_GRO_CB(skb)->count);
Seems correct to me, Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>