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 <[email protected]>
Signed-off-by: Andrea Mayer <[email protected]>
---
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;
--
2.20.1
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 <[email protected]>
> Signed-off-by: Andrea Mayer <[email protected]>
> ---
> 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 <[email protected]>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html