[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z9bNYPX165yxdoId@shredder>
Date: Sun, 16 Mar 2025 15:08:48 +0200
From: Ido Schimmel <idosch@...sch.org>
To: Stanislav Fomichev <stfomichev@...il.com>
Cc: Guillaume Nault <gnault@...hat.com>, David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
Simon Horman <horms@...nel.org>, David Ahern <dsahern@...nel.org>,
Antonio Quartulli <antonio@...delbit.com>,
Petr Machata <petrm@...dia.com>
Subject: Re: [PATCH net v4 1/2] gre: Fix IPv6 link-local address generation.
On Fri, Mar 14, 2025 at 01:18:21PM -0700, Stanislav Fomichev wrote:
> On 03/14, Guillaume Nault wrote:
> > On Fri, Mar 14, 2025 at 08:18:32AM -0700, Stanislav Fomichev wrote:
> > > On 03/07, Guillaume Nault wrote:
> > > > Use addrconf_addr_gen() to generate IPv6 link-local addresses on GRE
> > > > devices in most cases and fall back to using add_v4_addrs() only in
> > > > case the GRE configuration is incompatible with addrconf_addr_gen().
> > > >
> > > > GRE used to use addrconf_addr_gen() until commit e5dd729460ca
> > > > ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL
> > > > address") restricted this use to gretap and ip6gretap devices, and
> > > > created add_v4_addrs() (borrowed from SIT) for non-Ethernet GRE ones.
> > > >
> > > > The original problem came when commit 9af28511be10 ("addrconf: refuse
> > > > isatap eui64 for INADDR_ANY") made __ipv6_isatap_ifid() fail when its
> > > > addr parameter was 0. The commit says that this would create an invalid
> > > > address, however, I couldn't find any RFC saying that the generated
> > > > interface identifier would be wrong. Anyway, since gre over IPv4
> > > > devices pass their local tunnel address to __ipv6_isatap_ifid(), that
> > > > commit broke their IPv6 link-local address generation when the local
> > > > address was unspecified.
> > > >
> > > > Then commit e5dd729460ca ("ip/ip6_gre: use the same logic as SIT
> > > > interfaces when computing v6LL address") tried to fix that case by
> > > > defining add_v4_addrs() and calling it to generate the IPv6 link-local
> > > > address instead of using addrconf_addr_gen() (apart for gretap and
> > > > ip6gretap devices, which would still use the regular
> > > > addrconf_addr_gen(), since they have a MAC address).
> > > >
> > > > That broke several use cases because add_v4_addrs() isn't properly
> > > > integrated into the rest of IPv6 Neighbor Discovery code. Several of
> > > > these shortcomings have been fixed over time, but add_v4_addrs()
> > > > remains broken on several aspects. In particular, it doesn't send any
> > > > Router Sollicitations, so the SLAAC process doesn't start until the
> > > > interface receives a Router Advertisement. Also, add_v4_addrs() mostly
> > > > ignores the address generation mode of the interface
> > > > (/proc/sys/net/ipv6/conf/*/addr_gen_mode), thus breaking the
> > > > IN6_ADDR_GEN_MODE_RANDOM and IN6_ADDR_GEN_MODE_STABLE_PRIVACY cases.
> > > >
> > > > Fix the situation by using add_v4_addrs() only in the specific scenario
> > > > where the normal method would fail. That is, for interfaces that have
> > > > all of the following characteristics:
> > > >
> > > > * run over IPv4,
> > > > * transport IP packets directly, not Ethernet (that is, not gretap
> > > > interfaces),
> > > > * tunnel endpoint is INADDR_ANY (that is, 0),
> > > > * device address generation mode is EUI64.
> > >
> > > Could you please double check net/forwarding/ip6gre_custom_multipath_hash.sh ?
> > > It seems like it started falling after this series has been pulled:
> > > https://netdev-3.bots.linux.dev/vmksft-forwarding-dbg/results/31301/2-ip6gre-custom-multipath-hash-sh/stdout
> >
> > Hum, net/forwarding/ip6gre_custom_multipath_hash.sh works for me on the
> > current net tree (I'm at commit 4003c9e78778). I have only one failure,
> > but it already happened before 183185a18ff9 ("gre: Fix IPv6 link-local
> > address generation.") was applied.
>
> On my side I see the following (ignore ping6 FAILs):
>
> bfc6c67ec2d6 - (net-next/main, net-next/HEAD) net/smc: use the correct ndev to find pnetid by pnetid table (7 hours ago) <Guangguan Wang>
>
> TAP version 13
> 1..1
> # timeout set to 0
> # selftests: net/forwarding: ip6gre_custom_multipath_hash.sh
> [ 9.275735][ T167] ip (167) used greatest stack depth: 23536 bytes left
> [ 13.769300][ T255] gre: GRE over IPv4 demultiplexor driver
> [ 13.838185][ T255] ip6_gre: GRE over IPv6 tunneling driver
> [ 13.951780][ T12] ip6_tunnel: g1 xmit: Local address not yet configured!
> [ 14.038101][ T12] ip6_tunnel: g1 xmit: Local address not yet configured!
> [ 15.148469][ T281] 8021q: 802.1Q VLAN Support v1.8
> [ 17.559477][ T321] GACT probability NOT on
> [ 18.551876][ T12] ip6_tunnel: g2 xmit: Local address not yet configured!
> [ 18.633656][ T12] ip6_tunnel: g2 xmit: Local address not yet configured!
> # TEST: ping [ OK ]
> # TEST: ping6 [FAIL]
> # INFO: Running IPv4 overlay custom multipath hash tests
> # TEST: Multipath hash field: Inner source IP (balanced) [FAIL]
> # Expected traffic to be balanced, but it is not
> # INFO: Packets sent on path1 / path2: 1 / 12602
> # TEST: Multipath hash field: Inner source IP (unbalanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 0 / 12601
> # TEST: Multipath hash field: Inner destination IP (balanced) [FAIL]
> # Expected traffic to be balanced, but it is not
> # INFO: Packets sent on path1 / path2: 1 / 12600
> # TEST: Multipath hash field: Inner destination IP (unbalanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 0 / 12600
> ...
>
> 8ecea691e844 - (HEAD -> upstream/net-next/main) Revert "gre: Fix IPv6 link-local address generation." (2 minutes ago) <Stanislav Fomichev>
>
> TAP version 13
> 1..1
> # timeout set to 0
> # selftests: net/forwarding: ip6gre_custom_multipath_hash.sh
> [ 13.863060][ T252] gre: GRE over IPv4 demultiplexor driver
> [ 13.911551][ T252] ip6_gre: GRE over IPv6 tunneling driver
> [ 15.226124][ T277] 8021q: 802.1Q VLAN Support v1.8
> [ 17.629460][ T317] GACT probability NOT on
> [ 17.645781][ T315] tc (315) used greatest stack depth: 23040 bytes left
> # TEST: ping [ OK ]
> # TEST: ping6 [FAIL]
> # INFO: Running IPv4 overlay custom multipath hash tests
> # TEST: Multipath hash field: Inner source IP (balanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 5552 / 7052
> # TEST: Multipath hash field: Inner source IP (unbalanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 12600 / 2
> [ 36.278056][ C2] clocksource: Long readout interval, skipping watchdog check: cs_nsec: 1078005296 wd_nsec: 1078004682
> # TEST: Multipath hash field: Inner destination IP (balanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 6650 / 5950
> # TEST: Multipath hash field: Inner destination IP (unbalanced) [ OK ]
> # INFO: Packets sent on path1 / path2: 0 / 12600
> ...
>
> And I also see the failures on 4003c9e78778. Not sure why we see
> different results. And the NIPAs fails as well:
>
> https://netdev-3.bots.linux.dev/vmksft-forwarding-dbg/results/32922/1-ip6gre-custom-multipath-hash-sh/stdout
I can reproduce this locally and I'm getting the exact same result as
the CI. All the balanced tests fail because the traffic is forwarded via
a single nexthop. No failures after reverting 183185a18ff9.
I'm still not sure what happens, but for some reason a neighbour is not
created on one of the nexthop devices which causes rt6_check_neigh() to
skip over this path (returning RT6_NUD_FAIL_DO_RR). Enabling
CONFIG_IPV6_ROUTER_PREF fixes the issue because then RT6_NUD_SUCCEED is
returned.
I can continue looking into this on Tuesday (mostly AFK tomorrow).
Powered by blists - more mailing lists