Messages in this thread |  | | Date | Sat, 11 Jun 2022 10:44:50 -0600 | Subject | Re: [REGRESSION] connection timeout with routes to VRF | From | David Ahern <> |
| |
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.
...
> > 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.
|  |