Message-ID: <5667bed7-3b22-484e-8e31-9abb8029caee@kernel.org>
Date: Tue, 15 Apr 2025 08:40:11 -0700
From: David Ahern <dsahern@...nel.org>
To: Ido Schimmel <idosch@...dia.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
 edumazet@...gle.com, horms@...nel.org, hanhuihui5@...wei.com
Subject: Re: [PATCH net 1/2] net: fib_rules: Fix iif / oif matching on L3
 master device

On 4/14/25 11:20 AM, Ido Schimmel wrote:
> Before commit 40867d74c374 ("net: Add l3mdev index to flow struct and
> avoid oif reset for port devices") it was possible to use FIB rules to
> match on a L3 domain. This was done by having a FIB rule match on iif /
> oif being a L3 master device. It worked because prior to the FIB rule
> lookup the iif / oif fields in the flow structure were reset to the
> index of the L3 master device to which the input / output device was
> enslaved to.
> 
> The above scheme made it impossible to match on the original input /
> output device. Therefore, cited commit stopped overwriting the iif / oif
> fields in the flow structure and instead stored the index of the
> enslaving L3 master device in a new field ('flowi_l3mdev') in the flow
> structure.
> 
> While the change enabled new use cases, it broke the original use case
> of matching on a L3 domain. Fix this by interpreting the iif / oif
> matching on a L3 master device as a match against the L3 domain. In
> other words, if the iif / oif in the FIB rule points to a L3 master
> device, compare the provided index against 'flowi_l3mdev' rather than
> 'flowi_{i,o}if'.
> 
> Before cited commit, a FIB rule that matched on 'iif vrf1' would only
> match incoming traffic from devices enslaved to 'vrf1'. With the
> proposed change (i.e., comparing against 'flowi_l3mdev'), the rule would
> also match traffic originating from a socket bound to 'vrf1'. Avoid that
> by adding a new flow flag ('FLOWI_FLAG_L3MDEV_OIF') that indicates if
> the L3 domain was derived from the output interface or the input
> interface (when not set) and take this flag into account when evaluating
> the FIB rule against the flow structure.
> 
> Avoid unnecessary checks in the data path by detecting that a rule
> matches on a L3 master device when the rule is installed and marking it
> as such.
> 
> Tested using the following script [1].
> 
> Output before 40867d74c374 (v5.4.291):
> 
> default dev dummy1 table 100 scope link
> default dev dummy1 table 200 scope link
> 
> Output after 40867d74c374:
> 
> default dev dummy1 table 300 scope link
> default dev dummy1 table 300 scope link
> 
> Output with this patch:
> 
> default dev dummy1 table 100 scope link
> default dev dummy1 table 200 scope link
> 
> [1]
>  #!/bin/bash
> 
>  ip link add name vrf1 up type vrf table 10
>  ip link add name dummy1 up master vrf1 type dummy
> 
>  sysctl -wq net.ipv4.conf.all.forwarding=1
>  sysctl -wq net.ipv4.conf.all.rp_filter=0
> 
>  ip route add table 100 default dev dummy1
>  ip route add table 200 default dev dummy1
>  ip route add table 300 default dev dummy1
> 
>  ip rule add prio 0 oif vrf1 table 100
>  ip rule add prio 1 iif vrf1 table 200
>  ip rule add prio 2 table 300
> 
>  ip route get 192.0.2.1 oif dummy1 fibmatch
>  ip route get 192.0.2.1 iif dummy1 from 198.51.100.1 fibmatch
> 
> Fixes: 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif reset for port devices")
> Reported-by: hanhuihui <hanhuihui5@...wei.com>
> Closes: https://lore.kernel.org/netdev/ec671c4f821a4d63904d0da15d604b75@huawei.com/
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> ---
>  include/net/fib_rules.h |  2 ++
>  include/net/flow.h      |  1 +
>  include/net/l3mdev.h    | 27 +++++++++++++++++++++++
>  net/core/fib_rules.c    | 48 ++++++++++++++++++++++++++++++++++-------
>  net/l3mdev/l3mdev.c     |  4 +++-
>  5 files changed, 73 insertions(+), 9 deletions(-)
> 

Acked-by: David Ahern <dsahern@...nel.org>

Powered by blists - more mailing lists