From: Pablo Neira Ayuso A direct xmit flow that reaches a bridge port keeps sending to that port when the host moves to another port of the bridge, and packets from the other side keep the flow from timing out. The gc now checks the fdb entry next to the dst check, through nf_br_utils_hook, and tears the flow down when the entry points to another port. An entry that aged out does not tear it down, because offloaded packets do not refresh it. One possibility to address this issue is to subscribe to SWITCHDEV_FDB_DEL_TO_DEVICE event notifications, however, this requires iterating over the whole flowtable whenever a fdb entry is removed. The bridge port is found from out.ifidx through the vlan devices that the forward path has below the port. The vid is the bridge vlan in the encap stack, or the pvid of the port for an untagged port of a vlan_filtering bridge. Reported-by: Eric Woudstra Link: https://lore.kernel.org/netfilter-devel/20250925182623.114045-1-ericwouds@gmail.com/ Signed-off-by: Pablo Neira Ayuso [julius@bairaktaris.de: find the bridge port below the vlan devices of the path, take the vid from a bridge vlan or the port pvid, keep a flow whose entry aged out, check the bridge the flow transmits on, follow struct net_bridge_dst, build br_nf_utils.o only with the flowtable] Link: https://lore.kernel.org/netfilter-devel/20261014210256.1573072-1-pablo@netfilter.org/ Signed-off-by: Julius Bairaktaris --- include/linux/netfilter.h | 7 +++++ include/net/netfilter/nf_flow_table.h | 8 ++++- net/bridge/Makefile | 2 ++ net/bridge/br.c | 2 ++ net/bridge/br_nf_utils.c | 38 ++++++++++++++++++++++ net/bridge/br_private.h | 13 ++++++++ net/netfilter/core.c | 3 ++ net/netfilter/nf_flow_table_core.c | 45 +++++++++++++++++++++++++-- net/netfilter/nf_flow_table_path.c | 20 ++++++++++-- 9 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 net/bridge/br_nf_utils.c diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h index e99afc1414cd3..be85d8f68b452 100644 --- a/include/linux/netfilter.h +++ b/include/linux/netfilter.h @@ -67,6 +67,13 @@ static inline void nf_inet_addr_mask(const union nf_inet_addr *a1, #endif } +struct nf_br_utils { + bool (*fdb_valid_dst)(const struct net_device *dev, + 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 5b22d31cb1407..220ece9490e8e 100644 --- a/include/net/netfilter/nf_flow_table.h +++ b/include/net/netfilter/nf_flow_table.h @@ -153,6 +153,9 @@ struct flow_offload_tuple { xmit_type:3, encap_num:2, needs_gso_segment:1, + bridge_path:1, + bridge_vlan:1, + port_vlans:2, tun_num:2, in_vlan_ingress:2; u16 mtu; @@ -235,7 +238,10 @@ struct nf_flow_route { u32 hw_ifindex; u8 h_source[ETH_ALEN]; u8 h_dest[ETH_ALEN]; - u8 needs_gso_segment:1; + u8 needs_gso_segment:1, + bridge_path:1, + bridge_vlan:1, + port_vlans:2; } out; enum flow_offload_xmit_type xmit_type; } tuple[FLOW_OFFLOAD_DIR_MAX]; 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..300bce6c1b0b8 --- /dev/null +++ b/net/bridge/br_nf_utils.c @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include +#include + +#include "br_private.h" + +static bool nf_br_fdb_valid_dst(const struct net_device *dev, + const unsigned char *addr, u16 vid) +{ + const struct net_bridge_fdb_entry *f; + const struct net_bridge_port *p; + + p = br_port_get_check_rcu(dev); + if (!p) + return false; + + if (!vid && br_opt_get(p->br, BROPT_VLAN_ENABLED)) + vid = br_get_pvid(nbp_vlan_group_rcu(p)); + + /* Offloaded packets do not refresh the entry, it can age out. */ + 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_valid_dst = nf_br_fdb_valid_dst, +}; + +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 014d0b8db8c73..3b68f8d85672b 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -2165,6 +2165,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 44c856c1e621c..92e9852c80c0c 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..113580adf10d3 100644 --- a/net/netfilter/nf_flow_table_core.c +++ b/net/netfilter/nf_flow_table_core.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -139,6 +140,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->bridge_path = route->tuple[dir].out.bridge_path; + flow_tuple->bridge_vlan = route->tuple[dir].out.bridge_vlan; + flow_tuple->port_vlans = route->tuple[dir].out.port_vlans; break; case FLOW_OFFLOAD_XMIT_XFRM: case FLOW_OFFLOAD_XMIT_NEIGH: @@ -572,6 +576,43 @@ static void nf_flow_table_extend_ct_timeout(struct nf_conn *ct) nf_ct_put(ct); } +static bool nf_flow_dst_valid(struct nf_flowtable *flow_table, + struct flow_offload *flow, int dir) +{ + struct flow_offload_tuple *other = &flow->tuplehash[!dir].tuple; + struct flow_offload_tuple *tuple = &flow->tuplehash[dir].tuple; + struct net *net = read_pnet(&flow_table->net); + const struct nf_br_utils *br_utils; + struct net_device *dev; + bool valid = true; + u16 vid = 0; + int i; + + if (!nf_flow_dst_check(tuple)) + return false; + + if (!tuple->bridge_path) + return true; + + rcu_read_lock(); + br_utils = rcu_dereference(nf_br_utils_hook); + if (!br_utils) + goto out; + + dev = dev_get_by_index_rcu(net, tuple->out.ifidx); + for (i = 0; dev && i < tuple->port_vlans; i++) + dev = __vlan_find_dev_deep_rcu(dev, other->encap[i].proto, + other->encap[i].id); + if (tuple->bridge_vlan) + vid = other->encap[i].id; + + valid = dev && br_utils->fdb_valid_dst(dev, tuple->out.h_dest, vid); +out: + rcu_read_unlock(); + + return valid; +} + static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table, struct flow_offload *flow, void *data) { @@ -584,8 +625,8 @@ static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table, if (nf_flow_has_expired(flow) || 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_dst_valid(flow_table, flow, FLOW_OFFLOAD_DIR_ORIGINAL) || + !nf_flow_dst_valid(flow_table, flow, FLOW_OFFLOAD_DIR_REPLY) || 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 841f099434d98..cd3098c1d0de6 100644 --- a/net/netfilter/nf_flow_table_path.c +++ b/net/netfilter/nf_flow_table_path.c @@ -95,7 +95,10 @@ struct nft_forward_info { u8 ingress_vlans; u8 h_source[ETH_ALEN]; u8 h_dest[ETH_ALEN]; - bool needs_gso_segment; + u8 needs_gso_segment:1, + bridge_path:1, + bridge_vlan:1, + port_vlans:2; enum flow_offload_xmit_type xmit_type; }; @@ -107,7 +110,7 @@ static int nft_dev_path_info(struct net_device_path_stack *stack, unsigned char *ha, struct nft_flowtable *ft) { const struct net_device_path *path; - int i; + int i, n; memcpy(info->h_dest, ha, ETH_ALEN); @@ -149,6 +152,8 @@ static int nft_dev_path_info(struct net_device_path_stack *stack, info->encap[info->num_encaps].proto = path->encap.proto; info->num_encaps++; + if (info->bridge_path) + info->port_vlans++; } if (path->type == DEV_PATH_PPPOE) { memcpy(info->h_dest, path->encap.h_dest, ETH_ALEN); @@ -184,6 +189,14 @@ static int nft_dev_path_info(struct net_device_path_stack *stack, case DEV_PATH_BR_VLAN_KEEP: break; } + info->bridge_path = 1; + info->port_vlans = 0; + n = info->num_encaps; + info->bridge_vlan = path->bridge.vlan_id && n && + info->encap[n - 1].id == + path->bridge.vlan_id && + info->encap[n - 1].proto == + path->bridge.vlan_proto; info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT; break; default: @@ -262,6 +275,9 @@ 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_path = info.bridge_path; + route->tuple[dir].out.bridge_vlan = info.bridge_vlan; + route->tuple[dir].out.port_vlans = info.port_vlans; route->tuple[dir].xmit_type = info.xmit_type; } route->tuple[dir].out.needs_gso_segment = info.needs_gso_segment; -- 2.53.0