Message ID | 20200219191632.253526-1-willemdebruijn.kernel@gmail.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net] udp: rehash on disconnect | expand |
On 2/19/20 11:16 AM, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > As of the below commit, udp sockets bound to a specific address can > coexist with one bound to the any addr for the same port. > > The commit also phased out the use of socket hashing based only on > port (hslot), in favor of always hashing on {addr, port} (hslot2). > > The change broke the following behavior with disconnect (AF_UNSPEC): > > server binds to 0.0.0.0:1337 > server connects to 127.0.0.1:80 > server disconnects > client connects to 127.0.0.1:1337 > client sends "hello" > server reads "hello" // times out, packet did not find sk > > On connect the server acquires a specific source addr suitable for > routing to its destination. On disconnect it reverts to the any addr. > > The connect call triggers a rehash to a different hslot2. On > disconnect, add the same to return to the original hslot2. > > Skip this step if the socket is going to be unhashed completely. > > Fixes: 4cdeeee9252a ("net: udp: prefer listeners bound to an address") > Reported-by: Pavel Roskin <plroskin@gmail.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> > --- > net/ipv4/udp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index db76b96092991..08a41f1e1cd22 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -1857,8 +1857,12 @@ int __udp_disconnect(struct sock *sk, int flags) > inet->inet_dport = 0; > sock_rps_reset_rxhash(sk); > sk->sk_bound_dev_if = 0; > - if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK)) > + if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK)) { > inet_reset_saddr(sk); > + if (sk->sk_prot->rehash && > + (sk->sk_userlocks & SOCK_BINDPORT_LOCK)) > + sk->sk_prot->rehash(sk); > + } > > if (!(sk->sk_userlocks & SOCK_BINDPORT_LOCK)) { > sk->sk_prot->unhash(sk); > Reviewed-by: Eric Dumazet <edumazet@google.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: Wed, 19 Feb 2020 14:16:32 -0500 > From: Willem de Bruijn <willemb@google.com> > > As of the below commit, udp sockets bound to a specific address can > coexist with one bound to the any addr for the same port. > > The commit also phased out the use of socket hashing based only on > port (hslot), in favor of always hashing on {addr, port} (hslot2). > > The change broke the following behavior with disconnect (AF_UNSPEC): > > server binds to 0.0.0.0:1337 > server connects to 127.0.0.1:80 > server disconnects > client connects to 127.0.0.1:1337 > client sends "hello" > server reads "hello" // times out, packet did not find sk > > On connect the server acquires a specific source addr suitable for > routing to its destination. On disconnect it reverts to the any addr. > > The connect call triggers a rehash to a different hslot2. On > disconnect, add the same to return to the original hslot2. > > Skip this step if the socket is going to be unhashed completely. > > Fixes: 4cdeeee9252a ("net: udp: prefer listeners bound to an address") > Reported-by: Pavel Roskin <plroskin@gmail.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Applied and queued up for -stable, thanks Willem.
On Wed, Feb 19, 2020 at 4:35 PM David Miller <davem@davemloft.net> wrote: > > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > Date: Wed, 19 Feb 2020 14:16:32 -0500 > > > From: Willem de Bruijn <willemb@google.com> > > > > As of the below commit, udp sockets bound to a specific address can > > coexist with one bound to the any addr for the same port. > > > > The commit also phased out the use of socket hashing based only on > > port (hslot), in favor of always hashing on {addr, port} (hslot2). > > > > The change broke the following behavior with disconnect (AF_UNSPEC): > > > > server binds to 0.0.0.0:1337 > > server connects to 127.0.0.1:80 > > server disconnects > > client connects to 127.0.0.1:1337 > > client sends "hello" > > server reads "hello" // times out, packet did not find sk > > > > On connect the server acquires a specific source addr suitable for > > routing to its destination. On disconnect it reverts to the any addr. > > > > The connect call triggers a rehash to a different hslot2. On > > disconnect, add the same to return to the original hslot2. > > > > Skip this step if the socket is going to be unhashed completely. > > > > Fixes: 4cdeeee9252a ("net: udp: prefer listeners bound to an address") > > Reported-by: Pavel Roskin <plroskin@gmail.com> > > Signed-off-by: Willem de Bruijn <willemb@google.com> > > Applied and queued up for -stable, thanks Willem. Successfully tested with the original server and the demo programs I posted. Tested-by: Pavel Roskin <plroskin@gmail.com>
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index db76b96092991..08a41f1e1cd22 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1857,8 +1857,12 @@ int __udp_disconnect(struct sock *sk, int flags) inet->inet_dport = 0; sock_rps_reset_rxhash(sk); sk->sk_bound_dev_if = 0; - if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK)) + if (!(sk->sk_userlocks & SOCK_BINDADDR_LOCK)) { inet_reset_saddr(sk); + if (sk->sk_prot->rehash && + (sk->sk_userlocks & SOCK_BINDPORT_LOCK)) + sk->sk_prot->rehash(sk); + } if (!(sk->sk_userlocks & SOCK_BINDPORT_LOCK)) { sk->sk_prot->unhash(sk);