Re: [nf-next,v3,4/5] netfilter: flowtable: Add IP6IP6 tx sw acceleration

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netfilter: flowtable: Add IP6IP6 tx sw acceleration

This patch adds software acceleration for the TX path of IP6IP6 tunnels
using the netfilter flowtable infrastructure.

> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index 73ec8cb88285..f72fcb85c318 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c

[ ... ]

> +static int nf_flow_tunnel_ip6ip6_push(struct net *net, struct sk_buff *skb,
> +				      struct flow_offload_tuple *tuple,
> +				      struct in6_addr **ip6_daddr)
> +{
> +	struct ipv6hdr *ip6h = (struct ipv6hdr *)skb_network_header(skb);
> +	int err, mtu, encap_limit = IPV6_DEFAULT_TNL_ENCAP_LIMIT;

[ ... ]

> +	if (encap_limit > 0) {
> +		struct ipv6_tel_txoption opt = {

[ ... ]

> +		hopt = skb_push(skb, ipv6_optlen(opt.ops.dst1opt));

This pushes 8 bytes for the destination options header.

> +		memcpy(hopt, opt.ops.dst1opt, ipv6_optlen(opt.ops.dst1opt));
> +		hopt->nexthdr = IPPROTO_IPV6;
> +		proto = NEXTHDR_DEST;
> +	}
> +
> +	skb_push(skb, sizeof(*ip6h));

And this pushes another 40 bytes for the IPv6 header.

So total encapsulation overhead is 48 bytes when encap_limit is enabled
(which is the default, since IPV6_DEFAULT_TNL_ENCAP_LIMIT is non-zero).

[ ... ]

> @@ -923,6 +1012,9 @@ static int nf_flow_offload_ipv6_forward(struct nf_flowtable_ctx *ctx,
>  	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
>
>  	mtu = flow->tuplehash[dir].tuple.mtu + ctx->offset;
> +	if (flow->tuplehash[!dir].tuple.tun_num)
> +		mtu -= sizeof(*ip6h);
                        ^^^^^^^^^^^^

Should this also subtract 8 bytes for the destination options header when
encap_limit is enabled? The MTU calculation here only accounts for the
40-byte IPv6 header, but nf_flow_tunnel_ip6ip6_push() adds 48 bytes total
(40 for the IPv6 header plus 8 for the encap_limit destination options).

The reference implementation in ip6_tnl_xmit() subtracts both:

    mtu = dst_mtu(dst) - eth_hlen - psh_hlen - t->tun_hlen;
    if (encap_limit >= 0) {
        max_headroom += 8;
        mtu -= 8;
    }

Without accounting for the extra 8 bytes here, packets in the size range
(mtu-48, mtu-40] could pass the MTU check but produce oversized packets
after encapsulation.

> +
>  	if (unlikely(nf_flow_exceeds_mtu(skb, mtu)))
>  		return 0;

[ ... ]