| Message ID | 20220608091917.20345-1-andrea.mayer@uniroma2.it (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a3bd2102e464202b58d57390a538d96f57ffc361 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: seg6: fix seg6_lookup_any_nexthop() to handle VRFs using flowi_l3mdev | expand |
On 6/8/22 3:19 AM, Andrea Mayer wrote: > Commit 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif > reset for port devices") adds a new entry (flowi_l3mdev) in the common > flow struct used for indicating the l3mdev index for later rule and > table matching. > The l3mdev_update_flow() has been adapted to properly set the > flowi_l3mdev based on the flowi_oif/flowi_iif. In fact, when a valid > flowi_iif is supplied to the l3mdev_update_flow(), this function can > update the flowi_l3mdev entry only if it has not yet been set (i.e., the > flowi_l3mdev entry is equal to 0). > > The SRv6 End.DT6 behavior in VRF mode leverages a VRF device in order to > force the routing lookup into the associated routing table. This routing > operation is performed by seg6_lookup_any_nextop() preparing a flowi6 > data structure used by ip6_route_input_lookup() which, in turn, > (indirectly) invokes l3mdev_update_flow(). > > However, seg6_lookup_any_nexthop() does not initialize the new > flowi_l3mdev entry which is filled with random garbage data. This > prevents l3mdev_update_flow() from properly updating the flowi_l3mdev > with the VRF index, and thus SRv6 End.DT6 (VRF mode)/DT46 behaviors are > broken. > > This patch correctly initializes the flowi6 instance allocated and used > by seg6_lookup_any_nexhtop(). Specifically, the entire flowi6 instance > is wiped out: in case new entries are added to flowi/flowi6 (as happened > with the flowi_l3mdev entry), we should no longer have incorrectly > initialized values. As a result of this operation, the value of > flowi_l3mdev is also set to 0. > > The proposed fix can be tested easily. Starting from the commit > referenced in the Fixes, selftests [1],[2] indicate that the SRv6 > End.DT6 (VRF mode)/DT46 behaviors no longer work correctly. By applying > this patch, those behaviors are back to work properly again. > > [1] - tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh > [2] - tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh > > Fixes: 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif reset for port devices") > Reported-by: Anton Makarov <am@3a-alliance.com> > Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it> > --- > net/ipv6/seg6_local.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c > index 9fbe243a0e81..98a34287439c 100644 > --- a/net/ipv6/seg6_local.c > +++ b/net/ipv6/seg6_local.c > @@ -218,6 +218,7 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, > struct flowi6 fl6; > int dev_flags = 0; > > + memset(&fl6, 0, sizeof(fl6)); > fl6.flowi6_iif = skb->dev->ifindex; > fl6.daddr = nhaddr ? *nhaddr : hdr->daddr; > fl6.saddr = hdr->saddr; Missed the open initialization of this flow struct. Thanks for the fix: Reviewed-by: David Ahern <dsahern@kernel.org>
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Wed, 8 Jun 2022 11:19:17 +0200 you wrote: > Commit 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif > reset for port devices") adds a new entry (flowi_l3mdev) in the common > flow struct used for indicating the l3mdev index for later rule and > table matching. > The l3mdev_update_flow() has been adapted to properly set the > flowi_l3mdev based on the flowi_oif/flowi_iif. In fact, when a valid > flowi_iif is supplied to the l3mdev_update_flow(), this function can > update the flowi_l3mdev entry only if it has not yet been set (i.e., the > flowi_l3mdev entry is equal to 0). > > [...] Here is the summary with links: - [net] net: seg6: fix seg6_lookup_any_nexthop() to handle VRFs using flowi_l3mdev https://git.kernel.org/netdev/net/c/a3bd2102e464 You are awesome, thank you!
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c index 9fbe243a0e81..98a34287439c 100644 --- a/net/ipv6/seg6_local.c +++ b/net/ipv6/seg6_local.c @@ -218,6 +218,7 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, struct flowi6 fl6; int dev_flags = 0; + memset(&fl6, 0, sizeof(fl6)); fl6.flowi6_iif = skb->dev->ifindex; fl6.daddr = nhaddr ? *nhaddr : hdr->daddr; fl6.saddr = hdr->saddr;
Commit 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif reset for port devices") adds a new entry (flowi_l3mdev) in the common flow struct used for indicating the l3mdev index for later rule and table matching. The l3mdev_update_flow() has been adapted to properly set the flowi_l3mdev based on the flowi_oif/flowi_iif. In fact, when a valid flowi_iif is supplied to the l3mdev_update_flow(), this function can update the flowi_l3mdev entry only if it has not yet been set (i.e., the flowi_l3mdev entry is equal to 0). The SRv6 End.DT6 behavior in VRF mode leverages a VRF device in order to force the routing lookup into the associated routing table. This routing operation is performed by seg6_lookup_any_nextop() preparing a flowi6 data structure used by ip6_route_input_lookup() which, in turn, (indirectly) invokes l3mdev_update_flow(). However, seg6_lookup_any_nexthop() does not initialize the new flowi_l3mdev entry which is filled with random garbage data. This prevents l3mdev_update_flow() from properly updating the flowi_l3mdev with the VRF index, and thus SRv6 End.DT6 (VRF mode)/DT46 behaviors are broken. This patch correctly initializes the flowi6 instance allocated and used by seg6_lookup_any_nexhtop(). Specifically, the entire flowi6 instance is wiped out: in case new entries are added to flowi/flowi6 (as happened with the flowi_l3mdev entry), we should no longer have incorrectly initialized values. As a result of this operation, the value of flowi_l3mdev is also set to 0. The proposed fix can be tested easily. Starting from the commit referenced in the Fixes, selftests [1],[2] indicate that the SRv6 End.DT6 (VRF mode)/DT46 behaviors no longer work correctly. By applying this patch, those behaviors are back to work properly again. [1] - tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh [2] - tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh Fixes: 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif reset for port devices") Reported-by: Anton Makarov <am@3a-alliance.com> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it> --- net/ipv6/seg6_local.c | 1 + 1 file changed, 1 insertion(+)