| Message ID | 20210312162127.239795-5-alobakin@pm.me |
|---|---|
| State | New |
| Headers | show |
| Series | gro: micro-optimize dev_gro_receive() | expand |
On Fri, Mar 12, 2021 at 5:22 PM Alexander Lobakin <alobakin@pm.me> wrote: > > Most of the functions that "convert" hash value into an index > (when RPS is configured / XPS is not configured / etc.) set > reciprocal_scale() on it. Its logics is simple, but fair enough and > accounts the entire input value. > On the opposite side, 'hash & (GRO_HASH_BUCKETS - 1)' expression uses > only 3 least significant bits of the value, which is far from > optimal (especially for XOR RSS hashers, where the hashes of two > different flows may differ only by 1 bit somewhere in the middle). > > Use reciprocal_scale() here too to take the entire hash value into > account and improve flow dispersion between GRO hash buckets. > > Signed-off-by: Alexander Lobakin <alobakin@pm.me> > --- > net/core/dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 65d9e7d9d1e8..bd7c9ba54623 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -5952,7 +5952,7 @@ static void gro_flush_oldest(struct napi_struct *napi, struct list_head *head) > > static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb) > { > - u32 bucket = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1); > + u32 bucket = reciprocal_scale(skb_get_hash_raw(skb), GRO_HASH_BUCKETS); This is going to use 3 high order bits instead of 3 low-order bits. Now, had you use hash_32(skb_get_hash_raw(skb), 3), you could have claimed to use "more bits" Toeplitz already shuffles stuff. Adding a multiply here seems not needed. Please provide experimental results, because this looks unnecessary to me.
diff --git a/net/core/dev.c b/net/core/dev.c index 65d9e7d9d1e8..bd7c9ba54623 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5952,7 +5952,7 @@ static void gro_flush_oldest(struct napi_struct *napi, struct list_head *head) static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb) { - u32 bucket = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1); + u32 bucket = reciprocal_scale(skb_get_hash_raw(skb), GRO_HASH_BUCKETS); struct gro_list *gro_list = &napi->gro_hash[bucket]; struct list_head *gro_head = &gro_list->list; struct list_head *head = &offload_base;
Most of the functions that "convert" hash value into an index (when RPS is configured / XPS is not configured / etc.) set reciprocal_scale() on it. Its logics is simple, but fair enough and accounts the entire input value. On the opposite side, 'hash & (GRO_HASH_BUCKETS - 1)' expression uses only 3 least significant bits of the value, which is far from optimal (especially for XOR RSS hashers, where the hashes of two different flows may differ only by 1 bit somewhere in the middle). Use reciprocal_scale() here too to take the entire hash value into account and improve flow dispersion between GRO hash buckets. Signed-off-by: Alexander Lobakin <alobakin@pm.me> --- net/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.30.2