| Message ID | 1530531703-11368-1-git-send-email-lirongqing@baidu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] net: increase MAX_GRO_SKBS to 64 | expand |
From: Li RongQing <lirongqing@baidu.com> Date: Mon, 2 Jul 2018 19:41:43 +0800 > After 07d78363dcffd [net: Convert NAPI gro list into a small hash table] > there is 8 hash buckets, which allow more flows to be held for merging. > > keep each as original list length, so increase MAX_GRO_SKBS to 64 > > Signed-off-by: Li RongQing <lirongqing@baidu.com> I would like to hear some feedback from Eric, 64 might be too big.
On 7/2/18, David Miller <davem@davemloft.net> wrote: > From: Li RongQing <lirongqing@baidu.com> > Date: Mon, 2 Jul 2018 19:41:43 +0800 > >> After 07d78363dcffd [net: Convert NAPI gro list into a small hash table] >> there is 8 hash buckets, which allow more flows to be held for merging. >> >> keep each as original list length, so increase MAX_GRO_SKBS to 64 >> >> Signed-off-by: Li RongQing <lirongqing@baidu.com> > > I would like to hear some feedback from Eric, 64 might be too big. > How about the below change? commit 6270b973a973b2944fedb4b5f9926ed3e379d0c2 (HEAD -> master) Author: Li RongQing <lirongqing@baidu.com> Date: Mon Jul 2 19:08:37 2018 +0800 net: limit each hash list length to MAX_GRO_SKBS After 07d78363dcffd [net: Convert NAPI gro list into a small hash table] there is 8 hash buckets, which allows more flows to be held for merging. but MAX_GRO_SKBS, the total held skb for merging, is 8 skb still, limit the hash table performance. keep MAX_GRO_SKBS as 8 skb, but limit each hash list length to 8 skb, not the total 8 skb Signed-off-by: Li RongQing <lirongqing@baidu.com> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 8bf8d6149f79..09d7764a8917 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -324,6 +324,7 @@ struct napi_struct { #endif struct net_device *dev; struct list_head gro_hash[GRO_HASH_BUCKETS]; + int list_len[GRO_HASH_BUCKETS]; struct sk_buff *skb; struct hrtimer timer; struct list_head dev_list; diff --git a/net/core/dev.c b/net/core/dev.c index 08d58e0debe5..3cf3c6676cb3 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -149,7 +149,6 @@ #include "net-sysfs.h" -/* Instead of increasing this, you should create a hash table. */ #define MAX_GRO_SKBS 8 /* This should be increased if a protocol with a bigger head is added. */ @@ -4989,10 +4988,11 @@ static int napi_gro_complete(struct sk_buff *skb) return netif_receive_skb_internal(skb); } -static void __napi_gro_flush_chain(struct napi_struct *napi, struct list_head *head, +static void __napi_gro_flush_chain(struct napi_struct *napi, int index, bool flush_old) { struct sk_buff *skb, *p; + struct list_head *head = &napi->gro_hash[index]; list_for_each_entry_safe_reverse(skb, p, head, list) { if (flush_old && NAPI_GRO_CB(skb)->age == jiffies) @@ -5000,6 +5000,7 @@ static void __napi_gro_flush_chain(struct napi_struct *napi, struct list_head *h list_del_init(&skb->list); napi_gro_complete(skb); napi->gro_count--; + napi->list_len[index]--; } } @@ -5011,11 +5012,8 @@ void napi_gro_flush(struct napi_struct *napi, bool flush_old) { int i; - for (i = 0; i < GRO_HASH_BUCKETS; i++) { - struct list_head *head = &napi->gro_hash[i]; - - __napi_gro_flush_chain(napi, head, flush_old); - } + for (i = 0; i < GRO_HASH_BUCKETS; i++) + __napi_gro_flush_chain(napi, i, flush_old); } EXPORT_SYMBOL(napi_gro_flush); @@ -5095,27 +5093,13 @@ static void gro_pull_from_frag0(struct sk_buff *skb, int grow) } } -static void gro_flush_oldest(struct napi_struct *napi) +static void gro_flush_oldest(struct list_head *head) { struct sk_buff *oldest = NULL; - unsigned long age = jiffies; - int i; - for (i = 0; i < GRO_HASH_BUCKETS; i++) { - struct list_head *head = &napi->gro_hash[i]; - struct sk_buff *skb; + oldest = list_last_entry(head, struct sk_buff, list); - if (list_empty(head)) - continue; - - skb = list_last_entry(head, struct sk_buff, list); - if (!oldest || time_before(NAPI_GRO_CB(skb)->age, age)) { - oldest = skb; - age = NAPI_GRO_CB(skb)->age; - } - } - - /* We are called with napi->gro_count >= MAX_GRO_SKBS, so this is + /* We are called with head length >= MAX_GRO_SKBS, so this is * impossible. */ if (WARN_ON_ONCE(!oldest)) @@ -5138,6 +5122,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff enum gro_result ret; int same_flow; int grow; + u32 hash = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1); if (netif_elide_gro(skb->dev)) goto normal; @@ -5196,6 +5181,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff list_del_init(&pp->list); napi_gro_complete(pp); napi->gro_count--; + napi->list_len[hash]--; } if (same_flow) @@ -5204,10 +5190,11 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff if (NAPI_GRO_CB(skb)->flush) goto normal; - if (unlikely(napi->gro_count >= MAX_GRO_SKBS)) { - gro_flush_oldest(napi); + if (unlikely(napi->list_len[hash] >= MAX_GRO_SKBS)) { + gro_flush_oldest(gro_head); } else { napi->gro_count++; + napi->list_len[hash]++; } NAPI_GRO_CB(skb)->count = 1; NAPI_GRO_CB(skb)->age = jiffies; @@ -5844,8 +5831,10 @@ void netif_napi_add(struct net_device *dev, struct napi_struct *napi, hrtimer_init(&napi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED); napi->timer.function = napi_watchdog; napi->gro_count = 0; - for (i = 0; i < GRO_HASH_BUCKETS; i++) + for (i = 0; i < GRO_HASH_BUCKETS; i++) { INIT_LIST_HEAD(&napi->gro_hash[i]); + napi->list_len[i] = 0; + } napi->skb = NULL; napi->poll = poll; if (weight > NAPI_POLL_WEIGHT) @@ -5887,6 +5876,7 @@ static void flush_gro_hash(struct napi_struct *napi) list_for_each_entry_safe(skb, n, &napi->gro_hash[i], list) kfree_skb(skb); + napi->list_len[i] = 0; } } -R
From: Li RongQing <roy.qing.li@gmail.com> Date: Tue, 3 Jul 2018 14:21:48 +0800 > On 7/2/18, David Miller <davem@davemloft.net> wrote: >> From: Li RongQing <lirongqing@baidu.com> >> Date: Mon, 2 Jul 2018 19:41:43 +0800 >> >>> After 07d78363dcffd [net: Convert NAPI gro list into a small hash table] >>> there is 8 hash buckets, which allow more flows to be held for merging. >>> >>> keep each as original list length, so increase MAX_GRO_SKBS to 64 >>> >>> Signed-off-by: Li RongQing <lirongqing@baidu.com> >> >> I would like to hear some feedback from Eric, 64 might be too big. >> > How about the below change? > > commit 6270b973a973b2944fedb4b5f9926ed3e379d0c2 (HEAD -> master) > Author: Li RongQing <lirongqing@baidu.com> > Date: Mon Jul 2 19:08:37 2018 +0800 > > net: limit each hash list length to MAX_GRO_SKBS Yes, this is much better. > @@ -324,6 +324,7 @@ struct napi_struct { > #endif > struct net_device *dev; > struct list_head gro_hash[GRO_HASH_BUCKETS]; > + int list_len[GRO_HASH_BUCKETS]; For cache locality, it is probably best to make gro_hash an array of structures whose members are: struct list_head hash_chain; int list_len;
diff --git a/net/core/dev.c b/net/core/dev.c index 08d58e0debe5..ac315e41d5e7 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -149,8 +149,7 @@ #include "net-sysfs.h" -/* Instead of increasing this, you should create a hash table. */ -#define MAX_GRO_SKBS 8 +#define MAX_GRO_SKBS 64 /* This should be increased if a protocol with a bigger head is added. */ #define GRO_MAX_HEAD (MAX_HEADER + 128)
After 07d78363dcffd [net: Convert NAPI gro list into a small hash table] there is 8 hash buckets, which allow more flows to be held for merging. keep each as original list length, so increase MAX_GRO_SKBS to 64 Signed-off-by: Li RongQing <lirongqing@baidu.com> --- net/core/dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)