netdev - Re: [PATCH net-next v2 1/3] ipv4: prefer multipath nexthop that matches source address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89i+ve7zU1+BceKCH4Sp2ycMcRsO=_i7Vn1oo9PGFWUg1Mw@mail.gmail.com>
Date: Thu, 24 Apr 2025 09:47:03 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org, 
	pabeni@...hat.com, dsahern@...nel.org, horms@...nel.org, idosch@...dia.com, 
	kuniyu@...zon.com, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next v2 1/3] ipv4: prefer multipath nexthop that
 matches source address

On Thu, Apr 24, 2025 at 7:35 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> From: Willem de Bruijn <willemb@...gle.com>
>
> With multipath routes, try to ensure that packets leave on the device
> that is associated with the source address.
>
> Avoid the following tcpdump example:
>
>     veth0 Out IP 10.1.0.2.38640 > 10.2.0.3.8000: Flags [S]
>     veth1 Out IP 10.1.0.2.38648 > 10.2.0.3.8000: Flags [S]
>
> Which can happen easily with the most straightforward setup:
>
>     ip addr add 10.0.0.1/24 dev veth0
>     ip addr add 10.1.0.1/24 dev veth1
>
>     ip route add 10.2.0.3 nexthop via 10.0.0.2 dev veth0 \
>                           nexthop via 10.1.0.2 dev veth1
>
> This is apparently considered WAI, based on the comment in
> ip_route_output_key_hash_rcu:
>
>     * 2. Moreover, we are allowed to send packets with saddr
>     *    of another iface. --ANK
>
> It may be ok for some uses of multipath, but not all. For instance,
> when using two ISPs, a router may drop packets with unknown source.
>
> The behavior occurs because tcp_v4_connect makes three route
> lookups when establishing a connection:
>
> 1. ip_route_connect calls to select a source address, with saddr zero.
> 2. ip_route_connect calls again now that saddr and daddr are known.
> 3. ip_route_newports calls again after a source port is also chosen.
>
> With a route with multiple nexthops, each lookup may make a different
> choice depending on available entropy to fib_select_multipath. So it
> is possible for 1 to select the saddr from the first entry, but 3 to
> select the second entry. Leading to the above situation.
>
> Address this by preferring a match that matches the flowi4 saddr. This
> will make 2 and 3 make the same choice as 1. Continue to update the
> backup choice until a choice that matches saddr is found.
>
> Do this in fib_select_multipath itself, rather than passing an fl4_oif
> constraint, to avoid changing non-multipath route selection. Commit
> e6b45241c57a ("ipv4: reset flowi parameters on route connect") shows
> how that may cause regressions.
>
> Also read ipv4.sysctl_fib_multipath_use_neigh only once. No need to
> refresh in the loop.
>
> This does not happen in IPv6, which performs only one lookup.
>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> Reviewed-by: David Ahern <dsahern@...nel.org>

Reviewed-by: Eric Dumazet <edumazet@...gle.com>