Re: [PATCH v2] udp reuseport: fix packet of same flow hashed to different socket

[Eric Dumazet]

On Wed, 2016-06-08 at 11:40 +0800, Su Xuemin wrote:
> From: "Su, Xuemin" <suxm@xxxxxxxxxxxxxxxxxx>
> 
> There is a corner case in which udp packets belonging to a same
> flow are hashed to different socket when hslot->count changes from 10
> to 11:

> It's the same case for IPv6, and this patch also fixes that.
> 
> Signed-off-by: Su, Xuemin <suxm@xxxxxxxxxxxxxxxxxx>
> ---
> The patch v1 does not fix the code in IPv6. Thank Eric Dumazet for
> pointing that.
> And I use this tree to generate this patch, hope it's correct:
>   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 

I am not convinced this is the right way to fix the issue.

Fact that you did not change udp4_lib_lookup2() is telling me something.


1) If host has 100 different IP, and 10 sockets bound to 0.0.0.0:53 
2) If we receive datagrams from SRCIP:srcport send to IP{1..100}:53
will all be hashed on same slot.

See the hash used in soreuseport logic as an equivalent of a 4-tuple
hash really, not a 3-tuple one.

I would try instead :

(Looks like we missed this when commit 1223c67c093 ("udp: fix for
unicast RX path optimization") was reviewed/merged.

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d56c0559b477cb96ce78c9b9b7dacc3109594f3a..5847fe48f17d64bee32551e9dd42024a8e65fb10 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -563,7 +563,7 @@ struct sock *__udp4_lib_lookup(struct net *net, __be32 saddr,
 				goto begin;
 
 			result = udp4_lib_lookup2(net, saddr, sport,
-						  htonl(INADDR_ANY), hnum, dif,
+						  daddr, hnum, dif,
 						  hslot2, slot2, skb);
 		}
 		return result;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 2da1896af93496cc58762c67b4cd4b4f42924901..501b5563234d0921eff9f1e50155db76f27b3837 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -277,7 +277,7 @@ struct sock *__udp6_lib_lookup(struct net *net,
 				goto begin;
 
 			result = udp6_lib_lookup2(net, saddr, sport,
-						  &in6addr_any, hnum, dif,
+						  daddr, hnum, dif,
 						  hslot2, slot2, skb);
 		}
 		return result;