Re: [PATCH nf] netfilter: flowtable: fix IP6IP6 tunnel offset double-count with vlan/pppoe encap

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Jun 05, 2026 at 03:53:50PM +0200, Lorenzo Bianconi wrote:
> > nf_flow_ip6_tunnel_proto() stores the return value of ipv6_skip_exthdr()
> > directly into ctx->tun.hdr_size and then does ctx->offset +=
> > ctx->tun.hdr_size.
> > 
> > ipv6_skip_exthdr() returns an offset measured from skb->data, i.e. its
> > result already includes the "sizeof(*ip6h) + ctx->offset" start argument.
> > So hdr_size ends up containing ctx->offset, and the subsequent
> > "ctx->offset += ctx->tun.hdr_size" counts the encap offset twice.
> > 
> > This is harmless for a bare IP6IP6 packet, where ctx->offset is 0 on
> > entry, which is why it has gone unnoticed. But nf_flow_skb_encap_protocol()
> > advances ctx->offset by VLAN_HLEN / PPPOE_SES_HLEN before the tunnel
> > parser runs, so for an IP6IP6 flow carried over vlan or pppoe both
> > ctx->offset and ctx->tun.hdr_size are off by the encap length:
> > 
> >   - nf_flow_tuple_ipv6() then reads the inner header at the wrong offset,
> >     the computed tuple no longer matches the flowtable entry, and the
> >     packet silently falls back to the slow path (IP6IP6 rx acceleration
> >     stops working);
> >   - on the forward path nf_flow_ip_tunnel_pop() would skb_pull() past the
> >     inner header.
> > 
> > The IPv4 sibling nf_flow_ip4_tunnel_proto() does this correctly: it stores
> > a relative header length (iph->ihl << 2) and adds that to ctx->offset.
> > Make the IPv6 path symmetric by storing the relative size.
> > 
> > Fixes: d98103575dcd ("netfilter: flowtable: Add IP6IP6 rx sw acceleration")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> 
> Hi David,
> 
> thx for fixing it. I developed the IP6IP6 vlan support using the veth as
> underlying device. veth enables vlan rx/tx offload by default so I was
> not able to spot the issue.

One question when looking at this code:

In nf_flow_ip6_tunnel_proto():

        if (nexthdr == IPPROTO_IPV6) {
                ctx->tun.hdr_size = hdrlen - ctx->offset;
                ctx->tun.proto = IPPROTO_IPV6;
        }
        ctx->offset += ctx->tun.hdr_size;

ctx->offset is bumped out of the branch.

and nf_flow_ip4_tunnel_proto():

        if (iph->protocol == IPPROTO_IPIP) {
                ctx->tun.proto = IPPROTO_IPIP;
                ctx->tun.hdr_size = size;
                ctx->offset += size;
        }

I think these checks are superfluous at this stage:

if (nexthdr == IPPROTO_IPV6) {

if (iph->protocol == IPPROTO_IPIP) {

because only ipip and ip6ip6 is supported.