* [PATCH] net: Small bug on rxhash calculation
@ 2012-09-06 6:35 Chema Gonzalez
2012-09-06 6:42 ` Eric Dumazet
2012-09-07 16:57 ` David Miller
0 siblings, 2 replies; 9+ messages in thread
From: Chema Gonzalez @ 2012-09-06 6:35 UTC (permalink / raw)
To: netdev; +Cc: edumazet, Chema Gonzalez
>From 5c02179069826bfba9360383b88601b31ff05517 Mon Sep 17 00:00:00 2001
From: Chema Gonzalez <chema@google.com>
Date: Wed, 5 Sep 2012 17:05:48 -0700
Subject: [PATCH] net: Small bug on rxhash calculation
While the (current) sorting of the ports/addresses is coherent
(you get the same rxhash for packets sharing the same (unsorted)
4-tuple), ports and addresses should not be sorted independently,
which currently are: For traffic between A=S:l and B=L:s, packets
in both directions would get their rxhash from hashing
{L, S, {s, l}}. The same rxhash is obtained from packets between
C=S:s and D=L:l.
This patch ensures traffic between A and B gets their rxhash
from {S, L, {s, l}}, and traffic between C and D from {S, L,
{l, s}}
The patch is co-written with Eric Dumazet <edumazet@google.com>
Signed-off-by: Chema Gonzalez <chema@google.com>
---
net/core/dev.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b1e6d63..dcc673d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2655,15 +2655,16 @@ void __skb_get_rxhash(struct sk_buff *skb)
if (!skb_flow_dissect(skb, &keys))
return;
- if (keys.ports) {
- if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
- swap(keys.port16[0], keys.port16[1]);
+ if (keys.ports)
skb->l4_rxhash = 1;
- }
/* get a consistent hash (same value on both flow directions) */
- if ((__force u32)keys.dst < (__force u32)keys.src)
+ if (((__force u32)keys.dst < (__force u32)keys.src) ||
+ (((__force u32)keys.dst == (__force u32)keys.src) &&
+ ((__force u16)keys.port16[1] < (__force u16)keys.port16[0]))) {
swap(keys.dst, keys.src);
+ swap(keys.port16[0], keys.port16[1]);
+ }
hash = jhash_3words((__force u32)keys.dst,
(__force u32)keys.src,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] net: Small bug on rxhash calculation
2012-09-06 6:35 [PATCH] net: Small bug on rxhash calculation Chema Gonzalez
@ 2012-09-06 6:42 ` Eric Dumazet
2012-09-07 16:57 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2012-09-06 6:42 UTC (permalink / raw)
To: Chema Gonzalez; +Cc: netdev, edumazet, Chema Gonzalez
On Wed, 2012-09-05 at 23:35 -0700, Chema Gonzalez wrote:
> From 5c02179069826bfba9360383b88601b31ff05517 Mon Sep 17 00:00:00 2001
> From: Chema Gonzalez <chema@google.com>
> Date: Wed, 5 Sep 2012 17:05:48 -0700
> Subject: [PATCH] net: Small bug on rxhash calculation
>
> While the (current) sorting of the ports/addresses is coherent
> (you get the same rxhash for packets sharing the same (unsorted)
> 4-tuple), ports and addresses should not be sorted independently,
> which currently are: For traffic between A=S:l and B=L:s, packets
> in both directions would get their rxhash from hashing
> {L, S, {s, l}}. The same rxhash is obtained from packets between
> C=S:s and D=L:l.
>
> This patch ensures traffic between A and B gets their rxhash
> from {S, L, {s, l}}, and traffic between C and D from {S, L,
> {l, s}}
>
> The patch is co-written with Eric Dumazet <edumazet@google.com>
>
> Signed-off-by: Chema Gonzalez <chema@google.com>
> ---
Signed-off-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] net: Small bug on rxhash calculation
2012-09-06 6:35 [PATCH] net: Small bug on rxhash calculation Chema Gonzalez
2012-09-06 6:42 ` Eric Dumazet
@ 2012-09-07 16:57 ` David Miller
2012-09-07 23:40 ` [PATCH] net: small " Chema Gonzalez
1 sibling, 1 reply; 9+ messages in thread
From: David Miller @ 2012-09-07 16:57 UTC (permalink / raw)
To: chema; +Cc: netdev, edumazet, chema
From: Chema Gonzalez <chema@berkeley.edu>
Date: Wed, 5 Sep 2012 23:35:40 -0700
> - if (keys.ports) {
> - if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
> - swap(keys.port16[0], keys.port16[1]);
> + if (keys.ports)
This patch was corrupted by your email client, it transformed TAB characters
into spaces, making your patch completely useless to us.
Please fix this, send a test patch to yourself, and make sure that the patch
you receive in that test email will apply cleanly and properly.
Only after you are able to get such an email test patch to work should
you submit it here again.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] net: small bug on rxhash calculation
2012-09-07 16:57 ` David Miller
@ 2012-09-07 23:40 ` Chema Gonzalez
2012-09-07 23:40 ` Chema Gonzalez
2012-09-08 8:18 ` Eric Dumazet
0 siblings, 2 replies; 9+ messages in thread
From: Chema Gonzalez @ 2012-09-07 23:40 UTC (permalink / raw)
To: David Miller, edumazet; +Cc: netdev, Chema Gonzalez, Chema Gonzalez
In the current rxhash calculation function, while the
sorting of the ports/addrs is coherent (you get the
same rxhash for packets sharing the same 4-tuple, in
both directions), ports and addrs are sorted
independently. This implies packets from a connection
between the same addresses but crossed ports hash to
the same rxhash.
For example, traffic between A=S:l and B=L:s is hashed
(in both directions) from {L, S, {s, l}}. The same
rxhash is obtained for packets between C=S:s and D=L:l.
This patch ensures that you either swap both addrs and ports,
or you swap none. Traffic between A and B, and traffic
between C and D, get their rxhash from different sources
({L, S, {l, s}} for A<->B, and {L, S, {s, l}} for C<->D)
The patch is co-written with Eric Dumazet <edumazet@google.com>
Signed-off-by: Chema Gonzalez <chema@google.com>
---
net/core/dev.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b1e6d63..dcc673d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2655,15 +2655,16 @@ void __skb_get_rxhash(struct sk_buff *skb)
if (!skb_flow_dissect(skb, &keys))
return;
- if (keys.ports) {
- if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
- swap(keys.port16[0], keys.port16[1]);
+ if (keys.ports)
skb->l4_rxhash = 1;
- }
/* get a consistent hash (same value on both flow directions) */
- if ((__force u32)keys.dst < (__force u32)keys.src)
+ if (((__force u32)keys.dst < (__force u32)keys.src) ||
+ (((__force u32)keys.dst == (__force u32)keys.src) &&
+ ((__force u16)keys.port16[1] < (__force u16)keys.port16[0]))) {
swap(keys.dst, keys.src);
+ swap(keys.port16[0], keys.port16[1]);
+ }
hash = jhash_3words((__force u32)keys.dst,
(__force u32)keys.src,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] net: small bug on rxhash calculation
2012-09-07 23:40 ` [PATCH] net: small " Chema Gonzalez
@ 2012-09-07 23:40 ` Chema Gonzalez
2012-09-07 23:50 ` David Miller
2012-09-08 8:18 ` Eric Dumazet
1 sibling, 1 reply; 9+ messages in thread
From: Chema Gonzalez @ 2012-09-07 23:40 UTC (permalink / raw)
To: David Miller, edumazet; +Cc: netdev, Chema Gonzalez, Chema Gonzalez
In the current rxhash calculation function, while the
sorting of the ports/addrs is coherent (you get the
same rxhash for packets sharing the same 4-tuple, in
both directions), ports and addrs are sorted
independently. This implies packets from a connection
between the same addresses but crossed ports hash to
the same rxhash.
For example, traffic between A=S:l and B=L:s is hashed
(in both directions) from {L, S, {s, l}}. The same
rxhash is obtained for packets between C=S:s and D=L:l.
This patch ensures that you either swap both addrs and ports,
or you swap none. Traffic between A and B, and traffic
between C and D, get their rxhash from different sources
({L, S, {l, s}} for A<->B, and {L, S, {s, l}} for C<->D)
The patch is co-written with Eric Dumazet <edumazet@google.com>
Signed-off-by: Chema Gonzalez <chema@google.com>
---
net/core/dev.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b1e6d63..dcc673d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2655,15 +2655,16 @@ void __skb_get_rxhash(struct sk_buff *skb)
if (!skb_flow_dissect(skb, &keys))
return;
- if (keys.ports) {
- if ((__force u16)keys.port16[1] < (__force u16)keys.port16[0])
- swap(keys.port16[0], keys.port16[1]);
+ if (keys.ports)
skb->l4_rxhash = 1;
- }
/* get a consistent hash (same value on both flow directions) */
- if ((__force u32)keys.dst < (__force u32)keys.src)
+ if (((__force u32)keys.dst < (__force u32)keys.src) ||
+ (((__force u32)keys.dst == (__force u32)keys.src) &&
+ ((__force u16)keys.port16[1] < (__force u16)keys.port16[0]))) {
swap(keys.dst, keys.src);
+ swap(keys.port16[0], keys.port16[1]);
+ }
hash = jhash_3words((__force u32)keys.dst,
(__force u32)keys.src,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] net: small bug on rxhash calculation
2012-09-07 23:40 ` [PATCH] net: small " Chema Gonzalez
2012-09-07 23:40 ` Chema Gonzalez
@ 2012-09-08 8:18 ` Eric Dumazet
2012-09-08 22:43 ` David Miller
1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2012-09-08 8:18 UTC (permalink / raw)
To: Chema Gonzalez; +Cc: David Miller, edumazet, netdev, Chema Gonzalez
On Fri, 2012-09-07 at 16:40 -0700, Chema Gonzalez wrote:
> In the current rxhash calculation function, while the
> sorting of the ports/addrs is coherent (you get the
> same rxhash for packets sharing the same 4-tuple, in
> both directions), ports and addrs are sorted
> independently. This implies packets from a connection
> between the same addresses but crossed ports hash to
> the same rxhash.
>
> For example, traffic between A=S:l and B=L:s is hashed
> (in both directions) from {L, S, {s, l}}. The same
> rxhash is obtained for packets between C=S:s and D=L:l.
>
> This patch ensures that you either swap both addrs and ports,
> or you swap none. Traffic between A and B, and traffic
> between C and D, get their rxhash from different sources
> ({L, S, {l, s}} for A<->B, and {L, S, {s, l}} for C<->D)
>
> The patch is co-written with Eric Dumazet <edumazet@google.com>
>
> Signed-off-by: Chema Gonzalez <chema@google.com>
> ---
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] net: small bug on rxhash calculation
2012-09-08 8:18 ` Eric Dumazet
@ 2012-09-08 22:43 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2012-09-08 22:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: chema, edumazet, netdev, chema
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 08 Sep 2012 10:18:40 +0200
> On Fri, 2012-09-07 at 16:40 -0700, Chema Gonzalez wrote:
>> In the current rxhash calculation function, while the
>> sorting of the ports/addrs is coherent (you get the
>> same rxhash for packets sharing the same 4-tuple, in
>> both directions), ports and addrs are sorted
>> independently. This implies packets from a connection
>> between the same addresses but crossed ports hash to
>> the same rxhash.
>>
>> For example, traffic between A=S:l and B=L:s is hashed
>> (in both directions) from {L, S, {s, l}}. The same
>> rxhash is obtained for packets between C=S:s and D=L:l.
>>
>> This patch ensures that you either swap both addrs and ports,
>> or you swap none. Traffic between A and B, and traffic
>> between C and D, get their rxhash from different sources
>> ({L, S, {l, s}} for A<->B, and {L, S, {s, l}} for C<->D)
>>
>> The patch is co-written with Eric Dumazet <edumazet@google.com>
>>
>> Signed-off-by: Chema Gonzalez <chema@google.com>
>> ---
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-09-08 22:43 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-06 6:35 [PATCH] net: Small bug on rxhash calculation Chema Gonzalez
2012-09-06 6:42 ` Eric Dumazet
2012-09-07 16:57 ` David Miller
2012-09-07 23:40 ` [PATCH] net: small " Chema Gonzalez
2012-09-07 23:40 ` Chema Gonzalez
2012-09-07 23:50 ` David Miller
2012-09-08 0:24 ` Chema Gonzalez
2012-09-08 8:18 ` Eric Dumazet
2012-09-08 22:43 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).