On Thu, Feb 20, 2025 at 02:07:01PM +0100, Florian Westphal wrote:
> In case the fib match is used from the input hook we can avoid the fib
> lookup if early demux assigned a socket for us: check that the input
> interface matches sk-cached one.
>
> Rework the existing 'lo bypass' logic to first check sk, then
> for loopback interface type to elide the fib lookup.
>
> This speeds up fib matching a little, before:
> 93.08 GBit/s (no rules at all)
> 75.1 GBit/s ("fib saddr . iif oif missing drop" in prerouting)
> 75.62 GBit/s ("fib saddr . iif oif missing drop" in input)
>
> After:
> 92.48 GBit/s (no rules at all)
> 75.62 GBit/s (fib rule in prerouting)
> 90.37 GBit/s (fib rule in input).
>
> Numbers for the 'no rules' and 'prerouting' are expected to
> closely match in-between runs, the 3rd/input test case exercises the
> the 'avoid lookup if cached ifindex in sk matches' case.
>
> Test used iperf3 via veth interface, lo can't be used due to existing
> loopback test.
A few questions below.
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> include/net/netfilter/nft_fib.h | 21 +++++++++++++++++++++
> net/ipv4/netfilter/nft_fib_ipv4.c | 11 +++++------
> net/ipv6/netfilter/nft_fib_ipv6.c | 19 ++++++++++---------
> 3 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/netfilter/nft_fib.h b/include/net/netfilter/nft_fib.h
> index 38cae7113de4..6e202ed5e63f 100644
> --- a/include/net/netfilter/nft_fib.h
> +++ b/include/net/netfilter/nft_fib.h
> @@ -18,6 +18,27 @@ nft_fib_is_loopback(const struct sk_buff *skb, const struct net_device *in)
> return skb->pkt_type == PACKET_LOOPBACK || in->flags & IFF_LOOPBACK;
> }
>
> +static inline bool nft_fib_can_skip(const struct nft_pktinfo *pkt)
> +{
> + const struct net_device *indev = nft_in(pkt);
> + const struct sock *sk;
> +
> + switch (nft_hook(pkt)) {
> + case NF_INET_PRE_ROUTING:
> + case NF_INET_INGRESS:
Not an issue in your patch itself, it seems nft_fib_validate() was
never updated to support NF_INET_INGRESS.
> + case NF_INET_LOCAL_IN:
> + break;
> + default:
> + return false;
> + }
> +
> + sk = pkt->skb->sk;
> + if (sk && sk_fullsock(sk))
> + return sk->sk_rx_dst_ifindex == indev->ifindex;
> +
> + return nft_fib_is_loopback(pkt->skb, indev);
> +}
> +
> 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[]);
> diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
> index 625adbc42037..9082ca17e845 100644
> --- a/net/ipv4/netfilter/nft_fib_ipv4.c
> +++ b/net/ipv4/netfilter/nft_fib_ipv4.c
> @@ -71,6 +71,11 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
> const struct net_device *oif;
> const struct net_device *found;
>
> + if (nft_fib_can_skip(pkt)) {
> + nft_fib_store_result(dest, priv, nft_in(pkt));
> + return;
> + }
Silly question: Does this optimization work for all cases?
NFTA_FIB_F_MARK and NFTA_FIB_F_DADDR.
> /*
> * Do not set flowi4_oif, it restricts results (for example, asking
> * for oif 3 will get RTN_UNICAST result even if the daddr exits
> @@ -85,12 +90,6 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
> else
> oif = NULL;
>
> - if (nft_hook(pkt) == NF_INET_PRE_ROUTING &&
> - nft_fib_is_loopback(pkt->skb, nft_in(pkt))) {
> - nft_fib_store_result(dest, priv, nft_in(pkt));
> - return;
> - }
> -
> iph = skb_header_pointer(pkt->skb, noff, sizeof(_iph), &_iph);
> if (!iph) {
> regs->verdict.code = NFT_BREAK;
> diff --git a/net/ipv6/netfilter/nft_fib_ipv6.c b/net/ipv6/netfilter/nft_fib_ipv6.c
> index c9f1634b3838..7fd9d7b21cd4 100644
> --- a/net/ipv6/netfilter/nft_fib_ipv6.c
> +++ b/net/ipv6/netfilter/nft_fib_ipv6.c
> @@ -170,6 +170,11 @@ void nft_fib6_eval(const struct nft_expr *expr, struct nft_regs *regs,
> struct rt6_info *rt;
> int lookup_flags;
>
> + if (nft_fib_can_skip(pkt)) {
> + nft_fib_store_result(dest, priv, nft_in(pkt));
> + return;
> + }
> +
> if (priv->flags & NFTA_FIB_F_IIF)
> oif = nft_in(pkt);
> else if (priv->flags & NFTA_FIB_F_OIF)
> @@ -181,17 +186,13 @@ void nft_fib6_eval(const struct nft_expr *expr, struct nft_regs *regs,
> return;
> }
>
> - lookup_flags = nft_fib6_flowi_init(&fl6, priv, pkt, oif, iph);
> -
> - if (nft_hook(pkt) == NF_INET_PRE_ROUTING ||
> - nft_hook(pkt) == NF_INET_INGRESS) {
> - if (nft_fib_is_loopback(pkt->skb, nft_in(pkt)) ||
> - nft_fib_v6_skip_icmpv6(pkt->skb, pkt->tprot, iph)) {
> - nft_fib_store_result(dest, priv, nft_in(pkt));
> - return;
> - }
> + if (nft_fib_v6_skip_icmpv6(pkt->skb, pkt->tprot, iph)) {
> + nft_fib_store_result(dest, priv, nft_in(pkt));
> + return;
> }
>
> + lookup_flags = nft_fib6_flowi_init(&fl6, priv, pkt, oif, iph);
> +
> *dest = 0;
> rt = (void *)ip6_route_lookup(nft_net(pkt), &fl6, pkt->skb,
> lookup_flags);
> --
> 2.45.3
>
>
