Messages in this thread |  | | Subject | Re: [REGRESSION] connection timeout with routes to VRF | From | Jan Luebbe <> | Date | Wed, 15 Jun 2022 19:47:44 +0200 |
| |
On Sat, 2022-06-11 at 10:44 -0600, David Ahern wrote: > On 6/11/22 5:14 AM, Jan Luebbe wrote: > > Hi, > > > > TL;DR: We think we have found a regression in the handling of VRF route > > leaking > > caused by "net: allow binding socket in a VRF when there's an unbound > > socket" > > (3c82a21f4320). > > This is the 3rd report in the past few months about this commit. > > ...
Hmm, I've not been able to find other reports. Could you point me to them?
> > > > Our minimized test case looks like this: > > ip rule add pref 32765 from all lookup local > > ip rule del pref 0 from all lookup local > > ip link add red type vrf table 1000 > > ip link set red up > > ip route add vrf red unreachable default metric 8192 > > ip addr add dev red 172.16.0.1/24 > > ip route add 172.16.0.0/24 dev red > > ip vrf exec red socat -dd TCP-LISTEN:1234,reuseaddr,fork SYSTEM:"echo > > connected" & > > sleep 1 > > nc 172.16.0.1 1234 < /dev/null > > > > ... > Thanks for the detailed analysis and reproducer. > > > > > The partial revert > > diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h > > index 98e1ec1a14f0..41e7f20d7e51 100644 > > --- a/include/net/inet_hashtables.h > > +++ b/include/net/inet_hashtables.h > > @@ -310,8 +310,9 @@ static inline struct sock *inet_lookup_listener(struct > > net *net, > > #define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif, > > __sdif) \ > > (((__sk)->sk_portpair == (__ports)) && \ > > ((__sk)->sk_addrpair == (__cookie)) && \ > > - (((__sk)->sk_bound_dev_if == (__dif)) || \ > > - ((__sk)->sk_bound_dev_if == (__sdif))) && \ > > + (!(__sk)->sk_bound_dev_if || \ > > + ((__sk)->sk_bound_dev_if == (__dif)) || \ > > + ((__sk)->sk_bound_dev_if == (__sdif))) && \ > > net_eq(sock_net(__sk), (__net))) > > #else /* 32-bit arch */ > > #define INET_ADDR_COOKIE(__name, __saddr, __daddr) \ > > @@ -321,8 +322,9 @@ static inline struct sock *inet_lookup_listener(struct > > net *net, > > (((__sk)->sk_portpair == (__ports)) && \ > > ((__sk)->sk_daddr == (__saddr)) && \ > > ((__sk)->sk_rcv_saddr == (__daddr)) && \ > > - (((__sk)->sk_bound_dev_if == (__dif)) || \ > > - ((__sk)->sk_bound_dev_if == (__sdif))) && \ > > + (!(__sk)->sk_bound_dev_if || \ > > + ((__sk)->sk_bound_dev_if == (__dif)) || \ > > + ((__sk)->sk_bound_dev_if == (__sdif))) && \ > > net_eq(sock_net(__sk), (__net))) > > #endif /* 64-bit arch */ > > > > restores the original behavior when applied on v5.18. This doesn't apply > > directly on master, as the macro was replaced by an inline function in > > "inet: > > add READ_ONCE(sk->sk_bound_dev_if) in INET_MATCH()" (4915d50e300e). > > > > I have to admit I don't quite understand 3c82a21f4320, so I'm not sure how > > to proceed. What would be broken by the partial revert above? Are there > > better ways to configure routing into the VRF than simply "ip route add > > 172.16.0.0/24 dev red" that still work? > > > > Thanks, > > Jan > > > > #regzbot introduced: 3c82a21f4320 > > > > > > > > Andy Roulin suggested the same fix to the same problem a few weeks back. > Let's do it along with a test case in fcnl-test.sh which covers all of > these vrf permutations.
Thanks! I'd be happy to test any patch in our real setup.
Regards, Jan
|  |