On Thu, May 22, 2025 at 06:52:18PM +0200, Pablo Neira Ayuso wrote:
> From: Florian Westphal <fw@xxxxxxxxx>
>
> fib has two modes:
> 1. Obtain output device according to source or destination address
> 2. Obtain the type of the address, e.g. local, unicast, multicast.
>
> 'fib daddr type' should return 'local' if the address is configured
> in this netns or unicast otherwise.
>
> 'fib daddr . iif type' should return 'local' if the address is configured
> on the input interface or unicast otherwise, i.e. more restrictive.
>
> However, if the interface is part of a VRF, then 'fib daddr type'
> returns unicast even if the address is configured on the incoming
> interface.
>
> This is broken for both ipv4 and ipv6.
>
> In the ipv4 case, inet_dev_addr_type must only be used if the
> 'iif' or 'oif' (strict mode) was requested.
>
> Else inet_addr_type_dev_table() needs to be used and the correct
> dev argument must be passed as well so the correct fib (vrf) table
> is used.
>
> In the ipv6 case, the bug is similar, without strict mode, dev is NULL
> so .flowi6_l3mdev will be set to 0.
>
> Add a new 'nft_fib_l3mdev_master_ifindex_rcu()' helper and use that
> to init the .l3mdev structure member.
>
> For ipv6, use it from nft_fib6_flowi_init() which gets called from
> both the 'type' and the 'route' mode eval functions.
>
> This provides consistent behaviour for all modes for both ipv4 and ipv6:
> If strict matching is requested, the input respectively output device
> of the netfilter hooks is used.
>
> Otherwise, use skb->dev to obtain the l3mdev ifindex.
>
> Without this, most type checks in updated nft_fib.sh selftest fail:
>
> FAIL: did not find veth0 . 10.9.9.1 . local in fibtype4
> FAIL: did not find veth0 . dead:1::1 . local in fibtype6
> FAIL: did not find veth0 . dead:9::1 . local in fibtype6
> FAIL: did not find tvrf . 10.0.1.1 . local in fibtype4
> FAIL: did not find tvrf . 10.9.9.1 . local in fibtype4
> FAIL: did not find tvrf . dead:1::1 . local in fibtype6
> FAIL: did not find tvrf . dead:9::1 . local in fibtype6
> FAIL: fib expression address types match (iif in vrf)
>
> (fib errounously returns 'unicast' for all of them, even
> though all of these addresses are local to the vrf).
>
> Fixes: f6d0cbcf09c5 ("netfilter: nf_tables: add fib expression")
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> include/net/netfilter/nft_fib.h | 16 ++++++++++++++++
> net/ipv4/netfilter/nft_fib_ipv4.c | 11 +++++++++--
> net/ipv6/netfilter/nft_fib_ipv6.c | 4 +---
> 3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/netfilter/nft_fib.h b/include/net/netfilter/nft_fib.h
> index 6e202ed5e63f..2eea102c6609 100644
> --- a/include/net/netfilter/nft_fib.h
> +++ b/include/net/netfilter/nft_fib.h
> @@ -2,6 +2,7 @@
> #ifndef _NFT_FIB_H_
> #define _NFT_FIB_H_
>
> +#include <net/l3mdev.h>
> #include <net/netfilter/nf_tables.h>
>
> struct nft_fib {
> @@ -39,6 +40,21 @@ static inline bool nft_fib_can_skip(const struct nft_pktinfo *pkt)
> return nft_fib_is_loopback(pkt->skb, indev);
> }
>
> +/**
> + * nft_fib_l3mdev_get_rcu - return ifindex of l3 master device
Hi Pablo,
I don't mean to hold up progress of this pull request. But it would be nice
if at some point the above could be changed to
nft_fib_l3mdev_master_ifindex_rcu so it matches the name of the function
below that it documents.
Flagged by ./scripts/kernel-doc -none
> + * @pkt: pktinfo container passed to nft_fib_eval function
> + * @iif: input interface, can be NULL
> + *
> + * Return: interface index or 0.
> + */
> +static inline int nft_fib_l3mdev_master_ifindex_rcu(const struct nft_pktinfo *pkt,
> + const struct net_device *iif)
> +{
> + const struct net_device *dev = iif ? iif : pkt->skb->dev;
> +
> + return l3mdev_master_ifindex_rcu(dev);
> +}
> +
> int nft_fib_dump(struct sk_buff *skb, const struct nft_expr *expr, bool reset);
> int nft_fib_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
> const struct nlattr * const tb[]);
...
