[PATCH nf-next v7 0/2] netfilter: flowtable: tear down bridged flows on layer 2 roaming

[PATCH nf-next v7 0/2] netfilter: flowtable: tear down bridged flows on layer 2 roaming

[Julius Bairaktaris]

A flowtable flow that transmits straight to a bridge port keeps sending
to the old port when the host moves to another port, for example a
Wi-Fi station roaming between access points.

Patch 1 makes the gc tear such a flow down, using the bridge fdb lookup
from Pablo's patch. Patch 2 adds selftest cases.

Tested on nf-next a5e7d8e446af in an x86 guest: nft_flowtable.sh passes,
including all seven roaming cases, and ftrace shows no
flow_offload_teardown during 10 s of steady traffic in any of them.
flow_offload grows by 16 bytes (296 to 312 on x86_64).

Changes in v7:
- look up the fdb entry through a bridge hook instead of walking the
  forward path again (from Pablo's patch)
- save the vid of the fdb lookup; the encap stack does not have it for
  untagged ports of a vlan_filtering bridge
- build br_nf_utils.o only with CONFIG_NF_FLOW_TABLE
- selftest: vlan_filtering case

v6: https://lore.kernel.org/netfilter-devel/20261004171605.3544792-1-julius@bairaktaris.de/
Pablo's patch: https://lore.kernel.org/netfilter-devel/20261014210256.1573072-1-pablo@netfilter.org/

Julius Bairaktaris (2):
  netfilter: flowtable: tear down direct xmit flows when the fdb entry
    moves
  selftests: netfilter: nft_flowtable.sh: roam a host between two bridge
    ports

 include/linux/netfilter.h                     |   8 +
 include/net/netfilter/nf_flow_table.h         |   4 +
 net/bridge/Makefile                           |   2 +
 net/bridge/br.c                               |   2 +
 net/bridge/br_nf_utils.c                      |  40 ++++
 net/bridge/br_private.h                       |  13 ++
 net/netfilter/core.c                          |   3 +
 net/netfilter/nf_flow_table_core.c            |  27 +++
 net/netfilter/nf_flow_table_path.c            |   9 +
 .../selftests/net/netfilter/nft_flowtable.sh  | 204 ++++++++++++++++++
 10 files changed, 312 insertions(+)
 create mode 100644 net/bridge/br_nf_utils.c


base-commit: a5e7d8e446af9803e37a3b6a4d416fb41178348f
-- 
2.53.0

[PATCH nf-next v7 1/2] netfilter: flowtable: tear down direct xmit flows when the fdb entry moves

[Julius Bairaktaris]

A direct xmit flow stores the bridge port the fdb resolved when the flow
was created. When the host moves to another port of the bridge, the flow
keeps sending to the old port, and packets from the other side keep it
from timing out. A hardware offloaded flow behaves the same.

The gc now asks the bridge, through nf_br_utils_hook, which port the fdb
entry points to, and tears the flow down when it is another port. The
hook keeps the flowtable independent of the bridge module. An entry that
aged out leaves the flow alone, because offloaded packets do not refresh
it.

The bridge port and the vid of the fdb lookup are saved when the flow is
created. The port is not out.ifidx when it is a vlan device, and the vid
is not in the encap stack for an untagged port of a vlan_filtering
bridge.

Co-developed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 include/linux/netfilter.h             |  8 ++++++
 include/net/netfilter/nf_flow_table.h |  4 +++
 net/bridge/Makefile                   |  2 ++
 net/bridge/br.c                       |  2 ++
 net/bridge/br_nf_utils.c              | 40 +++++++++++++++++++++++++++
 net/bridge/br_private.h               | 13 +++++++++
 net/netfilter/core.c                  |  3 ++
 net/netfilter/nf_flow_table_core.c    | 27 ++++++++++++++++++
 net/netfilter/nf_flow_table_path.c    |  9 ++++++
 9 files changed, 108 insertions(+)
 create mode 100644 net/bridge/br_nf_utils.c

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index e99afc1414cd3..b79626137579f 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -67,6 +67,14 @@ static inline void nf_inet_addr_mask(const union nf_inet_addr *a1,
 #endif
 }
 
+/* The bridge fills this in for the flowtable, which must not depend on it. */
+struct nf_br_utils {
+	bool (*fdb_moved)(struct net *net, int ifindex,
+			  const unsigned char *addr, u16 vid);
+};
+
+extern const struct nf_br_utils __rcu *nf_br_utils_hook;
+
 int netfilter_init(void);
 
 struct sk_buff;
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 5b611efaa3cd1..fb3786220a652 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -164,6 +164,8 @@ struct flow_offload_tuple {
 		};
 		struct {
 			u32		ifidx;
+			u32		bridge_ifidx;
+			u16		bridge_vid;
 			u8		h_source[ETH_ALEN];
 			u8		h_dest[ETH_ALEN];
 		} out;
@@ -232,6 +234,8 @@ struct nf_flow_route {
 		struct {
 			u32			ifindex;
 			u32			hw_ifindex;
+			u32			bridge_ifindex;
+			u16			bridge_vid;
 			u8			h_source[ETH_ALEN];
 			u8			h_dest[ETH_ALEN];
 			u8			needs_gso_segment:1;
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 1203dc19e15c9..fc53c332246de 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -14,6 +14,8 @@ bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
 
 bridge-$(subst m,y,$(CONFIG_BRIDGE_NETFILTER)) += br_nf_core.o
 
+bridge-$(subst m,y,$(CONFIG_NF_FLOW_TABLE)) += br_nf_utils.o
+
 br_netfilter-y := br_netfilter_hooks.o
 br_netfilter-$(subst m,y,$(CONFIG_IPV6)) += br_netfilter_ipv6.o
 obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
diff --git a/net/bridge/br.c b/net/bridge/br.c
index d9a39ff98868d..c0618ef6dac5c 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -458,6 +458,7 @@ static int __init br_init(void)
 		goto err_out6;
 
 	brioctl_set(br_ioctl_stub);
+	br_nf_utils_init();
 
 #if IS_MODULE(CONFIG_BRIDGE_NETFILTER)
 	pr_info("bridge: filtering via arp/ip/ip6tables is no longer available "
@@ -486,6 +487,7 @@ static int __init br_init(void)
 
 static void __exit br_deinit(void)
 {
+	br_nf_utils_fini();
 	stp_proto_unregister(&br_stp_proto);
 	br_netlink_fini();
 	unregister_switchdev_blocking_notifier(&br_switchdev_blocking_notifier);
diff --git a/net/bridge/br_nf_utils.c b/net/bridge/br_nf_utils.c
new file mode 100644
index 0000000000000..dac08ff6346d6
--- /dev/null
+++ b/net/bridge/br_nf_utils.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/netdevice.h>
+#include <linux/netfilter.h>
+
+#include "br_private.h"
+
+/* True if the bridge now sends to @addr through another port than @ifindex.
+ * An entry that aged out is not a move: packets the flowtable forwards do
+ * not pass the bridge, so they do not refresh it. Called under RCU.
+ */
+static bool nf_br_fdb_moved(struct net *net, int ifindex,
+			    const unsigned char *addr, u16 vid)
+{
+	const struct net_bridge_fdb_entry *f;
+	const struct net_bridge_port *p;
+	struct net_device *dev;
+
+	dev = dev_get_by_index_rcu(net, ifindex);
+	p = dev ? br_port_get_check_rcu(dev) : NULL;
+	if (!p)
+		return true;
+
+	f = br_fdb_find_rcu(p->br, addr, vid);
+
+	return f && br_fdb_dst_port(f) != p;
+}
+
+static const struct nf_br_utils nf_br_utils = {
+	.fdb_moved	= nf_br_fdb_moved,
+};
+
+void br_nf_utils_init(void)
+{
+	rcu_assign_pointer(nf_br_utils_hook, &nf_br_utils);
+}
+
+void br_nf_utils_fini(void)
+{
+	RCU_INIT_POINTER(nf_br_utils_hook, NULL);
+}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 05921192ebda1..22c9e17d76175 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -2164,6 +2164,19 @@ struct nf_br_ops {
 };
 extern const struct nf_br_ops __rcu *nf_br_ops;
 
+#if IS_ENABLED(CONFIG_NF_FLOW_TABLE)
+void br_nf_utils_init(void);
+void br_nf_utils_fini(void);
+#else
+static inline void br_nf_utils_init(void)
+{
+}
+
+static inline void br_nf_utils_fini(void)
+{
+}
+#endif
+
 /* br_netfilter.c */
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
 int br_nf_core_init(void);
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 940dea2663e95..914f0934f6da3 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -814,6 +814,9 @@ static struct pernet_operations netfilter_net_ops = {
 	.exit = netfilter_net_exit,
 };
 
+const struct nf_br_utils __rcu *nf_br_utils_hook;
+EXPORT_SYMBOL_GPL(nf_br_utils_hook);
+
 int __init netfilter_init(void)
 {
 	int ret;
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 0d72dc8ae47a1..5f49e1f8e7c80 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -139,6 +139,9 @@ static int flow_offload_fill_route(struct flow_offload *flow,
 		memcpy(flow_tuple->out.h_source, route->tuple[dir].out.h_source,
 		       ETH_ALEN);
 		flow_tuple->out.ifidx = route->tuple[dir].out.ifindex;
+		flow_tuple->out.bridge_ifidx =
+			route->tuple[dir].out.bridge_ifindex;
+		flow_tuple->out.bridge_vid = route->tuple[dir].out.bridge_vid;
 		break;
 	case FLOW_OFFLOAD_XMIT_XFRM:
 	case FLOW_OFFLOAD_XMIT_NEIGH:
@@ -572,9 +575,31 @@ static void nf_flow_table_extend_ct_timeout(struct nf_conn *ct)
 	nf_ct_put(ct);
 }
 
+static bool nf_flow_bridge_port_moved(struct net *net,
+				      const struct flow_offload_tuple *tuple)
+{
+	const struct nf_br_utils *br_utils;
+	bool moved = false;
+
+	if (tuple->xmit_type != FLOW_OFFLOAD_XMIT_DIRECT ||
+	    !tuple->out.bridge_ifidx)
+		return false;
+
+	rcu_read_lock();
+	br_utils = rcu_dereference(nf_br_utils_hook);
+	if (br_utils)
+		moved = br_utils->fdb_moved(net, tuple->out.bridge_ifidx,
+					    tuple->out.h_dest,
+					    tuple->out.bridge_vid);
+	rcu_read_unlock();
+
+	return moved;
+}
+
 static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
 				    struct flow_offload *flow, void *data)
 {
+	struct net *net = read_pnet(&flow_table->net);
 	bool teardown;
 
 	if (test_bit(NF_FLOW_PENDING, &flow->flags))
@@ -586,6 +611,8 @@ static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
 	    nf_ct_is_dying(flow->ct) ||
 	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple) ||
 	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple) ||
+	    nf_flow_bridge_port_moved(net, &flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple) ||
+	    nf_flow_bridge_port_moved(net, &flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple) ||
 	    nf_flow_custom_gc(flow_table, flow)) {
 		flow_offload_teardown(flow);
 		teardown = true;
diff --git a/net/netfilter/nf_flow_table_path.c b/net/netfilter/nf_flow_table_path.c
index d90013685bf12..f1efe5095f07b 100644
--- a/net/netfilter/nf_flow_table_path.c
+++ b/net/netfilter/nf_flow_table_path.c
@@ -89,6 +89,8 @@ struct nft_forward_info {
 		__be16	proto;
 	} encap[NF_FLOW_TABLE_ENCAP_MAX];
 	u8 num_encaps;
+	u32 bridge_ifidx;
+	u16 bridge_vid;
 	struct flow_offload_tunnel tun;
 	struct dst_entry *tun_dst;
 	u8 num_tuns;
@@ -183,6 +185,11 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
 			case DEV_PATH_BR_VLAN_KEEP:
 				break;
 			}
+			/* dev_fill_forward_path() adds the bridge port after
+			 * the bridge.
+			 */
+			info->bridge_ifidx = stack->path[i + 1].dev->ifindex;
+			info->bridge_vid = path->bridge.vlan_id;
 			info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
 			break;
 		default:
@@ -260,6 +267,8 @@ static int nft_dev_forward_path(const struct nft_pktinfo *pkt,
 	if (info.xmit_type == FLOW_OFFLOAD_XMIT_DIRECT) {
 		memcpy(route->tuple[dir].out.h_source, info.h_source, ETH_ALEN);
 		memcpy(route->tuple[dir].out.h_dest, info.h_dest, ETH_ALEN);
+		route->tuple[dir].out.bridge_ifindex = info.bridge_ifidx;
+		route->tuple[dir].out.bridge_vid = info.bridge_vid;
 		route->tuple[dir].xmit_type = info.xmit_type;
 	}
 	route->tuple[dir].out.needs_gso_segment = info.needs_gso_segment;
-- 
2.53.0

[PATCH nf-next v7 2/2] selftests: netfilter: nft_flowtable.sh: roam a host between two bridge ports

[Julius Bairaktaris]

Move a host to another port of a bridge on the router while its flow is
offloaded, and check that the transfer continues and is offloaded again.
The cases are plain and vlan bridge ports, "bridge fdb replace", a
pinned and then deleted fdb entry, an aged out entry (the flow must
stay), the plain case over IPv6 and on a vlan_filtering bridge.

Without the gc check the transfer stalls when the host moves.

Assisted-by: Claude:claude-fable-5-1
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 .../selftests/net/netfilter/nft_flowtable.sh  | 204 ++++++++++++++++++
 1 file changed, 204 insertions(+)

diff --git a/tools/testing/selftests/net/netfilter/nft_flowtable.sh b/tools/testing/selftests/net/netfilter/nft_flowtable.sh
index 449c518bd947d..e6b345ff45b5d 100755
--- a/tools/testing/selftests/net/netfilter/nft_flowtable.sh
+++ b/tools/testing/selftests/net/netfilter/nft_flowtable.sh
@@ -851,6 +851,210 @@ test_ipip
 
 test_bridge
 
+# Roam a host between two ports of a bridge on the router while its flow is
+# offloaded, as a station moving between two access points attached to ports
+# of one bridge. The host keeps its address and mac on a bridge of its own
+# and roams by moving that bridge's port, so only the fdb changes. With a vid
+# the bridge ports are vlan devices, whose ifindex differs from the device
+# the flow transmits on. With "replace" the router's fdb entry is sticky and
+# the move is a "bridge fdb replace". With "del" the host stays, its entry is
+# pinned to the other port and then deleted, and a ping relearns it. With
+# "age" the host stays and its entry ages out, which must not tear the flow
+# down. A flow that stays on the fast path leaves no more than a few packets
+# in the forward chain, the counter tells offloaded from slow path. The
+# plain roam also runs over IPv6, and on a vlan_filtering bridge, whose fdb
+# entries are keyed by the pvid of the untagged ports.
+test_bridge_roam()
+{
+	local vid=$1 how=$2 fam=${3:-4} vf=$4
+	local p0=sr0 p1=sr1 r0=rr0 r1=rr1
+	local mac=02:00:00:09:01:99
+	local what="roaming on bridge${vf:+ with vlan_filtering}"
+	what="$what${vid:+ with vlan ports}${how:+ by fdb $how}"
+	local hn=10.9.1 wn=10.9.2 dot=. len=24 nodad= ip6= srv=10.9.2.99
+	local min=10000000
+	local dev old new slow lpid cpid max=100 win=2
+
+	if [ "$fam" = 6 ]; then
+		hn=dead:9:1 wn=dead:9:2 dot=:: len=64 nodad=nodad ip6=-6
+		srv="[dead:9:2::99]"
+		what="$what over IPv6"
+	fi
+
+	setup_ns rns1 rnsr rns2
+
+	if ! ip -net "$rnsr" link add br0 type bridge ${vf:+vlan_filtering 1} 2>/dev/null; then
+		echo "SKIP: could not add bridge for $what"
+		[ "$ret" -eq 0 ] && ret=$ksft_skip
+		return
+	fi
+
+	ip link add sr0 netns "$rns1" type veth peer name rr0 netns "$rnsr"
+	ip link add sr1 netns "$rns1" type veth peer name rr1 netns "$rnsr"
+	ip link add rw0 netns "$rnsr" type veth peer name w0 netns "$rns2"
+	for dev in sr0 sr1; do
+		ip -net "$rns1" link set "$dev" up
+	done
+	for dev in rr0 rr1; do
+		ip -net "$rnsr" link set "$dev" up
+	done
+
+	if [ -n "$vid" ]; then
+		for dev in sr0 sr1; do
+			ip -net "$rns1" link add link "$dev" name "$dev.$vid" type vlan id "$vid"
+			ip -net "$rns1" link set "$dev.$vid" up
+		done
+		for dev in rr0 rr1; do
+			ip -net "$rnsr" link add link "$dev" name "$dev.$vid" type vlan id "$vid"
+			ip -net "$rnsr" link set "$dev.$vid" up
+		done
+		p0=sr0.$vid; p1=sr1.$vid; r0=rr0.$vid; r1=rr1.$vid
+	fi
+
+	ip -net "$rnsr" link set "$r0" master br0
+	ip -net "$rnsr" link set "$r1" master br0
+	ip -net "$rnsr" addr add $hn${dot}1/$len dev br0 $nodad
+	ip -net "$rnsr" link set br0 up
+	ip -net "$rnsr" addr add $wn${dot}1/$len dev rw0 $nodad
+	ip -net "$rnsr" link set rw0 up
+	ip netns exec "$rnsr" sysctl -q net.ipv4.ip_forward=1
+	ip netns exec "$rnsr" sysctl -q net.ipv6.conf.all.forwarding=1
+
+	ip -net "$rns1" link add stbr type bridge
+	ip -net "$rns1" link set stbr address $mac
+	ip -net "$rns1" link set "$p0" master stbr
+	ip -net "$rns1" link set stbr up
+	ip -net "$rns1" addr add $hn${dot}99/$len dev stbr $nodad
+	ip $ip6 -net "$rns1" route add default via $hn${dot}1
+
+	ip -net "$rns2" addr add $wn${dot}99/$len dev w0 $nodad
+	ip -net "$rns2" link set w0 up
+	ip $ip6 -net "$rns2" route add default via $wn${dot}1
+
+	# no neighbour probes, so the host only sends when it receives
+	ip -net "$rnsr" neigh replace $hn${dot}99 lladdr $mac nud permanent dev br0
+	ip -net "$rns1" neigh replace $hn${dot}1 nud permanent dev stbr \
+		lladdr "$(ip -net "$rnsr" -br link show br0 | awk '{print $3}')"
+
+	if ! ip netns exec "$rnsr" nft -f - <<EOF
+table inet filter {
+  counter roam_repl { }
+  flowtable f1 { hook ingress priority 0; devices = { rr0, rr1, rw0 }; }
+  chain forward {
+    type filter hook forward priority 0; policy accept;
+    meta oif "rw0" tcp dport 12345 flow add @f1
+    meta iif "rw0" tcp sport 12345 counter name roam_repl flow add @f1
+  }
+}
+EOF
+	then
+		echo "SKIP: could not load ruleset for $what"
+		[ "$ret" -eq 0 ] && ret=$ksft_skip
+		return
+	fi
+
+	# the host discards what it receives, its socket byte counter is read
+	rx_bytes() { ip netns exec "$rns1" ss -tin "dst $srv" 2>/dev/null |
+		grep -o 'bytes_received:[0-9]*' | head -1 | cut -d: -f2; }
+	rx_started() { local n; n=$(rx_bytes); [ "${n:-0}" -gt 0 ]; }
+	# forward chain packets of the reply direction since the last call
+	slow_pkts() { ip netns exec "$rnsr" nft reset counter inet filter roam_repl |
+		grep -o 'packets [0-9]*' | cut -d' ' -f2; }
+	roam_listener_ready() { ss -N "$rns2" -lnt -o "sport = :12345" | grep -q 12345; }
+
+	roam_stop() { kill "$cpid" "$lpid" 2>/dev/null; wait "$cpid" "$lpid" 2>/dev/null; }
+
+	timeout "$SOCAT_TIMEOUT" ip netns exec "$rns2" socat -u \
+		OPEN:/dev/zero TCP$fam-LISTEN:12345,reuseaddr &
+	lpid=$!
+	if ! busywait "$BUSYWAIT_TIMEOUT" roam_listener_ready; then
+		echo "FAIL: $what: listener did not start" 1>&2
+		ret=1
+		kill "$lpid" 2>/dev/null; wait "$lpid" 2>/dev/null
+		return
+	fi
+	timeout "$SOCAT_TIMEOUT" ip netns exec "$rns1" socat -u \
+		TCP$fam:$srv:12345 OPEN:/dev/null &
+	cpid=$!
+
+	if ! busywait "$BUSYWAIT_TIMEOUT" rx_started; then
+		echo "FAIL: $what: transfer did not start" 1>&2
+		ret=1
+		roam_stop
+		return
+	fi
+	sleep 2
+	slow_pkts > /dev/null
+	old=$(rx_bytes)
+	sleep 2
+	new=$(rx_bytes)
+	slow=$(slow_pkts)
+	if [ -z "$slow" ] || [ $((${new:-0} - ${old:-0})) -lt $min ] ||
+	   [ "$slow" -gt 100 ]; then
+		echo "FAIL: $what: not offloaded before the roam:" \
+		     "$((${new:-0} - ${old:-0})) bytes, ${slow:-no} forward chain packets" 1>&2
+		ret=1
+		roam_stop
+		return
+	fi
+
+	if [ "$how" = age ]; then
+		# ageing_time is in centiseconds
+		ip -net "$rnsr" link set br0 type bridge ageing_time 200
+		max=0 win=10
+	elif [ "$how" = del ]; then
+		bridge -n "$rnsr" fdb replace $mac dev "$r1" master static sticky
+		sleep 2
+		bridge -n "$rnsr" fdb del $mac dev "$r1" master static
+		ip netns exec "$rns1" ping $ip6 -c 2 -W 1 -q $hn${dot}1 >/dev/null 2>&1
+	else
+		# roam: move the host bridge to the other port and send on it
+		if [ "$how" = replace ]; then
+			bridge -n "$rnsr" fdb replace $mac dev "$r0" master static sticky
+		fi
+		ip -net "$rns1" link set "$p0" nomaster
+		ip -net "$rns1" link set "$p1" master stbr
+		ip netns exec "$rns1" ping $ip6 -c 2 -W 1 -q $hn${dot}1 >/dev/null 2>&1
+		if [ "$how" = replace ]; then
+			bridge -n "$rnsr" fdb replace $mac dev "$r1" master static sticky
+		fi
+	fi
+
+	# the bytes in flight at the roam drain within the first seconds; a
+	# stale flow delivers nothing after that, a torn down one resumes and
+	# is offloaded again.
+	sleep 4
+	old=$(rx_bytes)
+	sleep 4
+	new=$(rx_bytes)
+	slow_pkts > /dev/null
+	sleep $win
+	slow=$(slow_pkts)
+	roam_stop
+
+	if [ "$how" = age ] &&
+	   bridge -n "$rnsr" fdb show br br0 | grep -q "$mac.*master"; then
+		echo "FAIL: $what: the host's fdb entry did not age out" 1>&2
+		ret=1
+	elif [ $((${new:-0} - ${old:-0})) -lt $min ]; then
+		echo "FAIL: $what: transfer stalled after the roam" 1>&2
+		ret=1
+	elif [ -z "$slow" ] || [ "$slow" -gt $max ]; then
+		echo "FAIL: $what: $slow forward chain packets after the roam" 1>&2
+		ret=1
+	else
+		echo "PASS: flow offload for $what"
+	fi
+}
+
+test_bridge_roam ""
+test_bridge_roam 100
+test_bridge_roam "" replace
+test_bridge_roam "" del
+test_bridge_roam "" age
+test_bridge_roam "" "" 6
+test_bridge_roam "" "" 4 vf
+
 KEY_SHA="0x"$(ps -af | sha1sum | cut -d " " -f 1)
 KEY_AES="0x"$(ps -af | md5sum | cut -d " " -f 1)
 SPI1=$RANDOM
-- 
2.53.0

Re: [PATCH nf-next v7 1/2] netfilter: flowtable: tear down direct xmit flows when the fdb entry moves

[Pablo Neira Ayuso]

Hi,

On Thu, Oct 15, 2026 at 01:56:26AM +0200, Julius Bairaktaris wrote:
> A direct xmit flow stores the bridge port the fdb resolved when the flow
> was created. When the host moves to another port of the bridge, the flow
> keeps sending to the old port, and packets from the other side keep it
> from timing out. A hardware offloaded flow behaves the same.
> 
> The gc now asks the bridge, through nf_br_utils_hook, which port the fdb
> entry points to, and tears the flow down when it is another port. The
> hook keeps the flowtable independent of the bridge module. An entry that
> aged out leaves the flow alone, because offloaded packets do not refresh
> it.
> 
> The bridge port and the vid of the fdb lookup are saved when the flow is
> created. The port is not out.ifidx when it is a vlan device, and the vid
> is not in the encap stack for an untagged port of a vlan_filtering
> bridge.
> 
> Co-developed-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Assisted-by: Claude:claude-opus-5-5

I did not use this LLM assistance for my patch and I show as
co-developer here ;-)

> Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
> ---
>  include/linux/netfilter.h             |  8 ++++++
>  include/net/netfilter/nf_flow_table.h |  4 +++
>  net/bridge/Makefile                   |  2 ++
>  net/bridge/br.c                       |  2 ++
>  net/bridge/br_nf_utils.c              | 40 +++++++++++++++++++++++++++
>  net/bridge/br_private.h               | 13 +++++++++
>  net/netfilter/core.c                  |  3 ++
>  net/netfilter/nf_flow_table_core.c    | 27 ++++++++++++++++++
>  net/netfilter/nf_flow_table_path.c    |  9 ++++++
>  9 files changed, 108 insertions(+)
>  create mode 100644 net/bridge/br_nf_utils.c
> 
> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> index e99afc1414cd3..b79626137579f 100644
> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -67,6 +67,14 @@ static inline void nf_inet_addr_mask(const union nf_inet_addr *a1,
>  #endif
>  }
>  
> +/* The bridge fills this in for the flowtable, which must not depend on it. */

This comment is not needed? LLM likes to bloat code with unnecessary comments.

Comments are last resort for something you cannot get via git
annotate, to highlight something is not really obvious in the code.

> +struct nf_br_utils {
> +	bool (*fdb_moved)(struct net *net, int ifindex,
> +			  const unsigned char *addr, u16 vid);

Why LLM decided to rename this function? :-)

> +};
> +
> +extern const struct nf_br_utils __rcu *nf_br_utils_hook;
> +
>  int netfilter_init(void);
>  
>  struct sk_buff;
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index 5b611efaa3cd1..fb3786220a652 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -164,6 +164,8 @@ struct flow_offload_tuple {
>  		};
>  		struct {
>  			u32		ifidx;
> +			u32		bridge_ifidx;

I think bridge_ifidx is not needed here and I am not sure we can cache
this value? We might not track of this br device on events, we could
end up with a stale index here? I think my approach searching for
upper is safer?

Plus, we don't need the extra memory in bridge_ifidx.

> +			u16		bridge_vid;

Why do we need bridge_vid? See below to explain an alternative.

>  			u8		h_source[ETH_ALEN];
>  			u8		h_dest[ETH_ALEN];
>  		} out;
> @@ -232,6 +234,8 @@ struct nf_flow_route {
>  		struct {
>  			u32			ifindex;
>  			u32			hw_ifindex;
> +			u32			bridge_ifindex;
> +			u16			bridge_vid;
>  			u8			h_source[ETH_ALEN];
>  			u8			h_dest[ETH_ALEN];
>  			u8			needs_gso_segment:1;
> diff --git a/net/bridge/Makefile b/net/bridge/Makefile
> index 1203dc19e15c9..fc53c332246de 100644
> --- a/net/bridge/Makefile
> +++ b/net/bridge/Makefile
> @@ -14,6 +14,8 @@ bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
>  
>  bridge-$(subst m,y,$(CONFIG_BRIDGE_NETFILTER)) += br_nf_core.o
>  
> +bridge-$(subst m,y,$(CONFIG_NF_FLOW_TABLE)) += br_nf_utils.o

Yes, this can be compiled conditionally.

> +
>  br_netfilter-y := br_netfilter_hooks.o
>  br_netfilter-$(subst m,y,$(CONFIG_IPV6)) += br_netfilter_ipv6.o
>  obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index d9a39ff98868d..c0618ef6dac5c 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -458,6 +458,7 @@ static int __init br_init(void)
>  		goto err_out6;
>  
>  	brioctl_set(br_ioctl_stub);
> +	br_nf_utils_init();
>  
>  #if IS_MODULE(CONFIG_BRIDGE_NETFILTER)
>  	pr_info("bridge: filtering via arp/ip/ip6tables is no longer available "
> @@ -486,6 +487,7 @@ static int __init br_init(void)
>  
>  static void __exit br_deinit(void)
>  {
> +	br_nf_utils_fini();
>  	stp_proto_unregister(&br_stp_proto);
>  	br_netlink_fini();
>  	unregister_switchdev_blocking_notifier(&br_switchdev_blocking_notifier);
> diff --git a/net/bridge/br_nf_utils.c b/net/bridge/br_nf_utils.c
> new file mode 100644
> index 0000000000000..dac08ff6346d6
> --- /dev/null
> +++ b/net/bridge/br_nf_utils.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/netdevice.h>
> +#include <linux/netfilter.h>
> +
> +#include "br_private.h"
> +
> +/* True if the bridge now sends to @addr through another port than @ifindex.
> + * An entry that aged out is not a move: packets the flowtable forwards do
> + * not pass the bridge, so they do not refresh it. Called under RCU.
> + */
> +static bool nf_br_fdb_moved(struct net *net, int ifindex,
> +			    const unsigned char *addr, u16 vid)
> +{
> +	const struct net_bridge_fdb_entry *f;
> +	const struct net_bridge_port *p;
> +	struct net_device *dev;
> +
> +	dev = dev_get_by_index_rcu(net, ifindex);
> +	p = dev ? br_port_get_check_rcu(dev) : NULL;
> +	if (!p)
> +		return true;
> +
> +	f = br_fdb_find_rcu(p->br, addr, vid);
> +
> +	return f && br_fdb_dst_port(f) != p;
> +}
> +
> +static const struct nf_br_utils nf_br_utils = {
> +	.fdb_moved	= nf_br_fdb_moved,
> +};
> +
> +void br_nf_utils_init(void)
> +{
> +	rcu_assign_pointer(nf_br_utils_hook, &nf_br_utils);
> +}
> +
> +void br_nf_utils_fini(void)
> +{
> +	RCU_INIT_POINTER(nf_br_utils_hook, NULL);
> +}
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 05921192ebda1..22c9e17d76175 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -2164,6 +2164,19 @@ struct nf_br_ops {
>  };
>  extern const struct nf_br_ops __rcu *nf_br_ops;
>  
> +#if IS_ENABLED(CONFIG_NF_FLOW_TABLE)
> +void br_nf_utils_init(void);
> +void br_nf_utils_fini(void);
> +#else
> +static inline void br_nf_utils_init(void)
> +{
> +}
> +
> +static inline void br_nf_utils_fini(void)
> +{
> +}
> +#endif
> +
>  /* br_netfilter.c */
>  #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
>  int br_nf_core_init(void);
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 940dea2663e95..914f0934f6da3 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -814,6 +814,9 @@ static struct pernet_operations netfilter_net_ops = {
>  	.exit = netfilter_net_exit,
>  };
>  
> +const struct nf_br_utils __rcu *nf_br_utils_hook;
> +EXPORT_SYMBOL_GPL(nf_br_utils_hook);
> +
>  int __init netfilter_init(void)
>  {
>  	int ret;
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index 0d72dc8ae47a1..5f49e1f8e7c80 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -139,6 +139,9 @@ static int flow_offload_fill_route(struct flow_offload *flow,
>  		memcpy(flow_tuple->out.h_source, route->tuple[dir].out.h_source,
>  		       ETH_ALEN);
>  		flow_tuple->out.ifidx = route->tuple[dir].out.ifindex;
> +		flow_tuple->out.bridge_ifidx =
> +			route->tuple[dir].out.bridge_ifindex;
> +		flow_tuple->out.bridge_vid = route->tuple[dir].out.bridge_vid;
>  		break;
>  	case FLOW_OFFLOAD_XMIT_XFRM:
>  	case FLOW_OFFLOAD_XMIT_NEIGH:
> @@ -572,9 +575,31 @@ static void nf_flow_table_extend_ct_timeout(struct nf_conn *ct)
>  	nf_ct_put(ct);
>  }
>  
> +static bool nf_flow_bridge_port_moved(struct net *net,
> +				      const struct flow_offload_tuple *tuple)
> +{
> +	const struct nf_br_utils *br_utils;
> +	bool moved = false;
> +
> +	if (tuple->xmit_type != FLOW_OFFLOAD_XMIT_DIRECT ||
> +	    !tuple->out.bridge_ifidx)
> +		return false;
> +
> +	rcu_read_lock();

Yes, my rcu_read_lock() was in the wrong spot indeed, this is fine.

> +	br_utils = rcu_dereference(nf_br_utils_hook);
> +	if (br_utils)
> +		moved = br_utils->fdb_moved(net, tuple->out.bridge_ifidx,
> +					    tuple->out.h_dest,
> +					    tuple->out.bridge_vid);
> +	rcu_read_unlock();
> +
> +	return moved;
> +}
> +
>  static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
>  				    struct flow_offload *flow, void *data)
>  {
> +	struct net *net = read_pnet(&flow_table->net);
>  	bool teardown;
>  
>  	if (test_bit(NF_FLOW_PENDING, &flow->flags))
> @@ -586,6 +611,8 @@ static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
>  	    nf_ct_is_dying(flow->ct) ||
>  	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple) ||
>  	    !nf_flow_dst_check(&flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple) ||
> +	    nf_flow_bridge_port_moved(net, &flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple) ||
> +	    nf_flow_bridge_port_moved(net, &flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple) ||

Why LLM moved this away from nf_flow_dst_check()?

My intention was to consolidate this in a single dst_check function,
because this is similar to what we do with dst_check(). While fdb does
not provide similar API, from a semantic POV is doing the same:
Validate that this destination is still valid.

>  	    nf_flow_custom_gc(flow_table, flow)) {
>  		flow_offload_teardown(flow);
>  		teardown = true;
> diff --git a/net/netfilter/nf_flow_table_path.c b/net/netfilter/nf_flow_table_path.c
> index d90013685bf12..f1efe5095f07b 100644
> --- a/net/netfilter/nf_flow_table_path.c
> +++ b/net/netfilter/nf_flow_table_path.c
> @@ -89,6 +89,8 @@ struct nft_forward_info {
>  		__be16	proto;
>  	} encap[NF_FLOW_TABLE_ENCAP_MAX];
>  	u8 num_encaps;
> +	u32 bridge_ifidx;
> +	u16 bridge_vid;
>  	struct flow_offload_tunnel tun;
>  	struct dst_entry *tun_dst;
>  	u8 num_tuns;
> @@ -183,6 +185,11 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
>  			case DEV_PATH_BR_VLAN_KEEP:
>  				break;
>  			}
> +			/* dev_fill_forward_path() adds the bridge port after
> +			 * the bridge.
> +			 */
> +			info->bridge_ifidx = stack->path[i + 1].dev->ifindex;
> +			info->bridge_vid = path->bridge.vlan_id;

Why do we need this new bridge_vid field? This is stored in info->encaps.

                case DEV_PATH_BR_VLAN_TAG:
                                if (info->num_encaps >= NF_FLOW_TABLE_ENCAP_MAX)
                                        goto err_out;
 
                                info->encap[info->num_encaps].id = path->bridge.vlan_id;

My understanding is that this is need to know if this vlan coming from
bridge vlan filtering (not vlan device), but it is possible to track
this in a local variable while walking over the path stack?

>  			info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
>  			break;
>  		default:
> @@ -260,6 +267,8 @@ static int nft_dev_forward_path(const struct nft_pktinfo *pkt,
>  	if (info.xmit_type == FLOW_OFFLOAD_XMIT_DIRECT) {
>  		memcpy(route->tuple[dir].out.h_source, info.h_source, ETH_ALEN);
>  		memcpy(route->tuple[dir].out.h_dest, info.h_dest, ETH_ALEN);
> +		route->tuple[dir].out.bridge_ifindex = info.bridge_ifidx;
> +		route->tuple[dir].out.bridge_vid = info.bridge_vid;
>  		route->tuple[dir].xmit_type = info.xmit_type;
>  	}
>  	route->tuple[dir].out.needs_gso_segment = info.needs_gso_segment;
> -- 
> 2.53.0
> 

Re: [PATCH nf-next v7 1/2] netfilter: flowtable: tear down direct xmit flows when the fdb entry moves

[Pablo Neira Ayuso]

On Thu, Oct 15, 2026 at 12:34:06PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Oct 15, 2026 at 01:56:26AM +0200, Julius Bairaktaris wrote:
[...]
> > @@ -183,6 +185,11 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
> >  			case DEV_PATH_BR_VLAN_KEEP:
> >  				break;
> >  			}
> > +			/* dev_fill_forward_path() adds the bridge port after
> > +			 * the bridge.
> > +			 */
> > +			info->bridge_ifidx = stack->path[i + 1].dev->ifindex;
> > +			info->bridge_vid = path->bridge.vlan_id;
> 
> Why do we need this new bridge_vid field? This is stored in info->encaps.
> 
>                 case DEV_PATH_BR_VLAN_TAG:
>                                 if (info->num_encaps >= NF_FLOW_TABLE_ENCAP_MAX)
>                                         goto err_out;
>  
>                                 info->encap[info->num_encaps].id = path->bridge.vlan_id;
> 
> My understanding is that this is need to know if this vlan coming from
> bridge vlan filtering (not vlan device), but it is possible to track
> this in a local variable while walking over the path stack?

A new flag might be need, but there is room for new flags to annotate
this VLAN header came from bridge filter filtering, I understand
info->encap is not enough.

> >  			info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT;
> >  			break;
> >  		default:

Re: [PATCH nf-next v7 1/2] netfilter: flowtable: tear down direct xmit flows when the fdb entry moves

[Julius Bairaktaris]

Hi Pablo,

Thanks for looking over this.

I will incorporate your feedback and send out v8 + explanations for
the questions Saturday.

Julius