Re: [PATCH net-next 12/15] netfilter: flowtable: tunnel device poisons h_source of fast-path frames

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message's only reproduction cannot happen on the tree
  this patch targets.

--

netfilter: flowtable: tunnel device poisons h_source of fast-path frames

This commit stops nft_dev_path_info() from copying the dev_addr of a
DEV_PATH_TUN device into info->h_source. On a tunnel device, dev_addr is
the local tunnel IP address and not an Ethernet address. Before this
change, those bytes became the source MAC of fast-path frames.

> Seen on a DS-Lite setup, LAN -> ip6tnl (IPv4 inside IPv6) -> PPPoE ->
> 802.1Q VLAN -> Ethernet, with conntrack flows offloaded through an
> nftables flowtable ('flow add @ft'). The first packets of a flow, still
> on the slow path, and the fast-path packets of the same flow differ only
> in the Ethernet source address:
>
>   slow path:  d8:58:d7:00:2e:bd > 80:ac:ac:39:ef:c2, 802.1Q vlan 35,
>               PPPoE [ses 0x454] IP6 2a01:110f:4328:c400::1 >
>               2a01:1000:0:1::2216 next-header IPIP, 192.0.0.2.40010 >
>               1.1.1.1.53
>   fast path:  2a:01:11:0f:43:28 > 80:ac:ac:39:ef:c2, same frame otherwise

[Severity: Low]
Can this DS-Lite setup reach the flowtable fast path on this tree?

ip6_tnl_fill_forward_path() starts by rejecting any inner flow that is not
IPv6:

net/ipv6/ip6_tunnel.c:ip6_tnl_fill_forward_path() {
    ...
	if (ctx->ether_type != cpu_to_be16(ETH_P_IPV6))
		return -EOPNOTSUPP;
    ...
	path->tun.inner_proto = IPPROTO_IPV6;
    ...
}

For a forwarded IPv4 flow, ctx->ether_type comes from pkt->ethertype.
nft_set_pktinfo() sets that to skb->protocol, which is ETH_P_IP here:

include/net/netfilter/nf_tables.h:nft_set_pktinfo() {
    ...
	pkt->ethertype = pkt->skb->protocol;
    ...
}

So dev_fill_forward_path() fails and nft_flow_route() returns -ENOENT. No
flow entry is created, and nft_dev_path_info() never sees a DEV_PATH_TUN
entry for this topology.

inner_proto is also hard-coded to IPPROTO_IPV6. Given that, could the
offload path on this tree produce the "next-header IPIP" fast-path frame
quoted above?

The check came from commit 5592223f3d4e45 ("net: netfilter: add
ether_type to net_device_path_ctx and use it"). That commit says IPv4
over IPv6 is "currently not support[ed]". Was the capture taken on an
older kernel, or on one with local changes?

> The PPPoE endpoint binds the session to the
> subscriber MAC and silently drops the frames, the effect is that every
> TCP/UDP flow dies the moment it is offloaded, while ICMP, which is never
> offloaded, keeps working.

The code change itself looks correct. On this tree, the h_source
poisoning still seems reachable in two ways:

  - ipip_fill_forward_path(), which accepts ETH_P_IP inner flows and
    emits DEV_PATH_TUN with dev_addr set to the local IPv4 address

  - ip6tnl carrying an IPv6 inner flow

Either one needs a PPPoE entry later on the path, or a hw-offload
flowtable on an Ethernet device. Then xmit_type is
FLOW_OFFLOAD_XMIT_DIRECT, and info.h_source is copied into
route->tuple[dir].out.h_source.

Could the commit message describe one of those reproducible cases
instead? Alternatively, it could note that the DS-Lite observation came
from a kernel without commit 5592223f3d4e45.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261113031251.2437074-2-pablo%40netfilter.org