* [PATCH net-next 01/12] net: bridge: introduce a bridge destination type
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-01 7:24 ` Nikolay Aleksandrov
2026-10-02 19:45 ` Jakub Kicinski
2026-09-30 7:14 ` [PATCH net-next 02/12] net: bridge: use net_bridge_dst for fdb destinations Nikolay Aleksandrov
` (11 subsequent siblings)
12 siblings, 2 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Add an opaque destination type and helpers for representing bridge port
destinations. Using a separate structure makes raw pointer assignments and
comparisons fail at build time while marking its value member __private
makes sparse warn about accesses that bypass the helpers.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_private.h | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 67117fb3dc88..1146187aa2ba 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -306,6 +306,10 @@ struct net_bridge_fdb_key {
u16 vlan_id;
};
+struct net_bridge_dst {
+ unsigned long __private value;
+};
+
struct net_bridge_fdb_entry {
struct rhash_head rhnode;
struct net_bridge_port *dst;
@@ -670,6 +674,40 @@ struct br_input_skb_cb {
#define br_debug(br, format, args...) \
pr_debug("%s: " format, (br)->dev->name, ##args)
+static inline struct net_bridge_dst
+br_dst_read(const struct net_bridge_dst *src)
+{
+ struct net_bridge_dst dst;
+
+ ACCESS_PRIVATE(&dst, value) =
+ READ_ONCE(ACCESS_PRIVATE(src, value));
+
+ return dst;
+}
+
+static inline void br_dst_write(struct net_bridge_dst *dst,
+ struct net_bridge_dst src)
+{
+ WRITE_ONCE(ACCESS_PRIVATE(dst, value),
+ ACCESS_PRIVATE(&src, value));
+}
+
+static inline struct net_bridge_dst
+br_port_to_dst(const struct net_bridge_port *p)
+{
+ struct net_bridge_dst dst;
+
+ ACCESS_PRIVATE(&dst, value) = (unsigned long)p;
+
+ return dst;
+}
+
+static inline struct net_bridge_port *
+br_dst_port(struct net_bridge_dst dst)
+{
+ return (struct net_bridge_port *)ACCESS_PRIVATE(&dst, value);
+}
+
/* called under bridge lock */
static inline int br_is_root_bridge(const struct net_bridge *br)
{
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 01/12] net: bridge: introduce a bridge destination type
2026-09-30 7:14 ` [PATCH net-next 01/12] net: bridge: introduce a bridge destination type Nikolay Aleksandrov
@ 2026-10-01 7:24 ` Nikolay Aleksandrov
2026-10-02 19:45 ` Jakub Kicinski
1 sibling, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 7:24 UTC (permalink / raw)
To: netdev; +Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 30/09/2026 10:14, Nikolay Aleksandrov wrote:
> Add an opaque destination type and helpers for representing bridge port
> destinations. Using a separate structure makes raw pointer assignments and
> comparisons fail at build time while marking its value member __private
> makes sparse warn about accesses that bypass the helpers.
>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
> net/bridge/br_private.h | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 67117fb3dc88..1146187aa2ba 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -306,6 +306,10 @@ struct net_bridge_fdb_key {
> u16 vlan_id;
> };
>
> +struct net_bridge_dst {
> + unsigned long __private value;
> +};
> +
> struct net_bridge_fdb_entry {
> struct rhash_head rhnode;
> struct net_bridge_port *dst;
> @@ -670,6 +674,40 @@ struct br_input_skb_cb {
> #define br_debug(br, format, args...) \
> pr_debug("%s: " format, (br)->dev->name, ##args)
>
> +static inline struct net_bridge_dst
> +br_dst_read(const struct net_bridge_dst *src)
> +{
> + struct net_bridge_dst dst;
> +
> + ACCESS_PRIVATE(&dst, value) =
> + READ_ONCE(ACCESS_PRIVATE(src, value));
> +
> + return dst;
> +}
> +
> +static inline void br_dst_write(struct net_bridge_dst *dst,
> + struct net_bridge_dst src)
> +{
> + WRITE_ONCE(ACCESS_PRIVATE(dst, value),
> + ACCESS_PRIVATE(&src, value));
> +}
> +
> +static inline struct net_bridge_dst
> +br_port_to_dst(const struct net_bridge_port *p)
> +{
> + struct net_bridge_dst dst;
> +
> + ACCESS_PRIVATE(&dst, value) = (unsigned long)p;
> +
Sashiko says:
Does casting this const-qualified struct net_bridge_port pointer to an
unsigned long drop the const restriction?
- Yes, it does but that is expected.
> + return dst;
> +}
> +
> +static inline struct net_bridge_port *
> +br_dst_port(struct net_bridge_dst dst)
> +{
> + return (struct net_bridge_port *)ACCESS_PRIVATE(&dst, value);
> +}
Sashiko says:
When the opaque token from br_port_to_dst() is resolved here in
br_dst_port(), is it unconditionally cast back to a mutable
struct net_bridge_port pointer?
Could this provide a silent mechanism to cast away const, bypassing C type
safety constraints in the API design?
- That is again expected and wanted behaviour, we're extracting the dst.
> +
> /* called under bridge lock */
> static inline int br_is_root_bridge(const struct net_bridge *br)
> {
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 01/12] net: bridge: introduce a bridge destination type
2026-09-30 7:14 ` [PATCH net-next 01/12] net: bridge: introduce a bridge destination type Nikolay Aleksandrov
2026-10-01 7:24 ` Nikolay Aleksandrov
@ 2026-10-02 19:45 ` Jakub Kicinski
2026-10-02 20:09 ` Nikolay Aleksandrov
1 sibling, 1 reply; 28+ messages in thread
From: Jakub Kicinski @ 2026-10-02 19:45 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, idosch, davem, edumazet, pabeni, horms, bridge
On Wed, 30 Sep 2026 10:14:00 +0300 Nikolay Aleksandrov wrote:
> Add an opaque destination type and helpers for representing bridge port
> destinations. Using a separate structure makes raw pointer assignments and
> comparisons fail at build time while marking its value member __private
> makes sparse warn about accesses that bypass the helpers.
First time I'm seeing the __private thing. FTR feels quite foreign to C.
__bitwise works very nicely for the same thing 🤷️
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH net-next 01/12] net: bridge: introduce a bridge destination type
2026-10-02 19:45 ` Jakub Kicinski
@ 2026-10-02 20:09 ` Nikolay Aleksandrov
2026-10-02 20:18 ` Jakub Kicinski
0 siblings, 1 reply; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-02 20:09 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, idosch, davem, edumazet, pabeni, horms, bridge
On 02/10/2026 22:45, Jakub Kicinski wrote:
> On Wed, 30 Sep 2026 10:14:00 +0300 Nikolay Aleksandrov wrote:
>> Add an opaque destination type and helpers for representing bridge port
>> destinations. Using a separate structure makes raw pointer assignments and
>> comparisons fail at build time while marking its value member __private
>> makes sparse warn about accesses that bypass the helpers.
>
> First time I'm seeing the __private thing. FTR feels quite foreign to C.
> __bitwise works very nicely for the same thing 🤷️
I don't mind switching to __bitwise, no preference here. I saw __private initially
in the console code. It seems to be more restrictive (__bitwise allows direct member
copies and comparisons, __private warns about those), but if __bitwise is preferred
I can send a follow-up.
Cheers,
Nik
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH net-next 01/12] net: bridge: introduce a bridge destination type
2026-10-02 20:09 ` Nikolay Aleksandrov
@ 2026-10-02 20:18 ` Jakub Kicinski
0 siblings, 0 replies; 28+ messages in thread
From: Jakub Kicinski @ 2026-10-02 20:18 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, idosch, davem, edumazet, pabeni, horms, bridge
On Fri, 2 Oct 2026 23:09:56 +0300 Nikolay Aleksandrov wrote:
> On 02/10/2026 22:45, Jakub Kicinski wrote:
> > On Wed, 30 Sep 2026 10:14:00 +0300 Nikolay Aleksandrov wrote:
> >> Add an opaque destination type and helpers for representing bridge port
> >> destinations. Using a separate structure makes raw pointer assignments and
> >> comparisons fail at build time while marking its value member __private
> >> makes sparse warn about accesses that bypass the helpers.
> >
> > First time I'm seeing the __private thing. FTR feels quite foreign to C.
> > __bitwise works very nicely for the same thing 🤷️
>
> I don't mind switching to __bitwise, no preference here. I saw __private initially
> in the console code. It seems to be more restrictive (__bitwise allows direct member
> copies and comparisons, __private warns about those), but if __bitwise is preferred
> I can send a follow-up.
It's alright, mostly flagging for future reference.
And I was curious how you chose it.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net-next 02/12] net: bridge: use net_bridge_dst for fdb destinations
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 01/12] net: bridge: introduce a bridge destination type Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations Nikolay Aleksandrov
` (10 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers
Store fdb destinations in struct net_bridge_dst and access them through
the destination helpers. This doesn't change their representation or
behavior, fdb destinations still contain only bridge port pointers.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Masami Hiramatsu <mhiramat@kernel.org>
CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
include/trace/events/bridge.h | 7 +++--
net/bridge/br_arp_nd_proxy.c | 4 +--
net/bridge/br_device.c | 4 +--
net/bridge/br_fdb.c | 53 ++++++++++++++++++-----------------
net/bridge/br_input.c | 4 +--
net/bridge/br_private.h | 20 ++++++++++++-
net/bridge/br_switchdev.c | 2 +-
7 files changed, 58 insertions(+), 36 deletions(-)
diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
index 3fe4725c83ff..f676516b33fa 100644
--- a/include/trace/events/bridge.h
+++ b/include/trace/events/bridge.h
@@ -68,13 +68,14 @@ TRACE_EVENT(br_fdb_external_learn_add,
TRACE_EVENT(fdb_delete,
- TP_PROTO(struct net_bridge *br, struct net_bridge_fdb_entry *f),
+ TP_PROTO(struct net_bridge *br, struct net_bridge_fdb_entry *f,
+ const struct net_bridge_port *dst),
- TP_ARGS(br, f),
+ TP_ARGS(br, f, dst),
TP_STRUCT__entry(
__string(br_dev, br->dev->name)
- __string(dev, f->dst ? f->dst->dev->name : "null")
+ __string(dev, dst ? dst->dev->name : "null")
__array(unsigned char, addr, ETH_ALEN)
__field(u16, vid)
),
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index da15f4d7c1ae..ba4a63840b21 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -223,7 +223,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
neigh_ha_snapshot(ha, n, n->dev);
f = br_fdb_find_rcu(br, ha, vid);
if (f) {
- const struct net_bridge_port *dst = READ_ONCE(f->dst);
+ const struct net_bridge_port *dst = br_fdb_dst_port(f);
bool replied = false;
if ((p && test_bit(BR_PROXYARP_BIT, &p->flags)) ||
@@ -500,7 +500,7 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
neigh_ha_snapshot(ha, n, n->dev);
f = br_fdb_find_rcu(br, ha, vid);
if (f) {
- const struct net_bridge_port *dst = READ_ONCE(f->dst);
+ const struct net_bridge_port *dst = br_fdb_dst_port(f);
bool replied = false;
if (br_is_neigh_suppress_enabled_vid(dst, vid)) {
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index ce9ea9ac3d0a..c6804409b60b 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -107,7 +107,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
else
br_flood(br, vlan, skb, BR_PKT_MULTICAST, false, true);
} else if ((dst = br_fdb_find_rcu(br, dest, vid)) != NULL) {
- br_forward(READ_ONCE(dst->dst), skb, false, true);
+ br_forward(br_fdb_dst_port(dst), skb, false, true);
} else {
br_flood(br, vlan, skb, BR_PKT_UNICAST, false, true);
}
@@ -400,7 +400,7 @@ static int br_fill_forward_path(struct net_device_path_ctx *ctx,
if (!f)
return -1;
- dst = READ_ONCE(f->dst);
+ dst = br_fdb_dst_port(f);
if (!dst)
return -1;
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index e4570bbed854..0f5cf5615b56 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -90,7 +90,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
const struct net_bridge_fdb_entry *fdb,
u32 portid, u32 seq, int type, unsigned int flags)
{
- const struct net_bridge_port *dst = READ_ONCE(fdb->dst);
+ const struct net_bridge_port *dst = br_fdb_dst_port(fdb);
unsigned long now = jiffies;
struct nda_cacheinfo ci;
struct nlmsghdr *nlh;
@@ -250,7 +250,7 @@ struct net_device *br_fdb_find_port(const struct net_device *br_dev,
rcu_read_lock();
f = br_fdb_find_rcu(br, addr, vid);
if (f) {
- dst = READ_ONCE(f->dst);
+ dst = br_fdb_dst_port(f);
if (dst)
dev = dst->dev;
}
@@ -315,7 +315,7 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
bool swdev_notify)
{
- trace_fdb_delete(br, f);
+ trace_fdb_delete(br, f, br_fdb_dst_port(f));
if (test_bit(BR_FDB_STATIC, &f->flags))
fdb_del_hw_addr(br, f->key.addr.addr);
@@ -350,7 +350,7 @@ static void fdb_delete_local(struct net_bridge *br,
vg = nbp_vlan_group(op);
if (op != p && ether_addr_equal(op->dev->dev_addr, addr) &&
(!vid || br_vlan_find(vg, vid))) {
- WRITE_ONCE(f->dst, op);
+ br_fdb_dst_write(f, br_port_to_dst(op));
clear_bit(BR_FDB_ADDED_BY_USER, &f->flags);
return;
}
@@ -361,7 +361,7 @@ static void fdb_delete_local(struct net_bridge *br,
/* Maybe bridge device has same hw addr? */
if (p && ether_addr_equal(br->dev->dev_addr, addr) &&
(!vid || (v && br_vlan_should_use(v)))) {
- WRITE_ONCE(f->dst, NULL);
+ br_fdb_dst_write(f, br_port_to_dst(NULL));
clear_bit(BR_FDB_ADDED_BY_USER, &f->flags);
return;
}
@@ -378,7 +378,8 @@ void br_fdb_find_delete_local(struct net_bridge *br,
spin_lock_bh(&br->hash_lock);
f = br_fdb_find(br, addr, vid);
if (f && test_bit(BR_FDB_LOCAL, &f->flags) &&
- !test_bit(BR_FDB_ADDED_BY_USER, &f->flags) && f->dst == p)
+ !test_bit(BR_FDB_ADDED_BY_USER, &f->flags) &&
+ br_fdb_dst_port(f) == p)
fdb_delete_local(br, p, f);
spin_unlock_bh(&br->hash_lock);
}
@@ -408,7 +409,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br,
return NULL;
memcpy(fdb->key.addr.addr, addr, ETH_ALEN);
- WRITE_ONCE(fdb->dst, source);
+ br_fdb_dst_write(fdb, br_port_to_dst(source));
fdb->key.vlan_id = vid;
fdb->flags = flags;
fdb->updated = fdb->used = jiffies;
@@ -470,7 +471,7 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
spin_lock_bh(&br->hash_lock);
vg = nbp_vlan_group(p);
hlist_for_each_entry(f, &br->fdb_list, fdb_node) {
- if (READ_ONCE(f->dst) == p &&
+ if (br_fdb_dst_port(f) == p &&
test_bit(BR_FDB_LOCAL, &f->flags) &&
!test_bit(BR_FDB_ADDED_BY_USER, &f->flags)) {
/* delete old one */
@@ -517,7 +518,8 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
/* If old entry was unassociated with any port, then delete it. */
f = br_fdb_find(br, br->dev->dev_addr, 0);
if (f && test_bit(BR_FDB_LOCAL, &f->flags) &&
- !f->dst && !test_bit(BR_FDB_ADDED_BY_USER, &f->flags))
+ !br_fdb_dst_port(f) &&
+ !test_bit(BR_FDB_ADDED_BY_USER, &f->flags))
fdb_delete_local(br, NULL, f);
fdb_add_local(br, NULL, newaddr, 0);
@@ -533,7 +535,8 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
continue;
f = br_fdb_find(br, br->dev->dev_addr, v->vid);
if (f && test_bit(BR_FDB_LOCAL, &f->flags) &&
- !f->dst && !test_bit(BR_FDB_ADDED_BY_USER, &f->flags))
+ !br_fdb_dst_port(f) &&
+ !test_bit(BR_FDB_ADDED_BY_USER, &f->flags))
fdb_delete_local(br, NULL, f);
fdb_add_local(br, NULL, newaddr, v->vid);
}
@@ -693,7 +696,7 @@ static bool __fdb_flush_matches(const struct net_bridge *br,
const struct net_bridge_fdb_entry *f,
const struct net_bridge_fdb_flush_desc *desc)
{
- const struct net_bridge_port *dst = READ_ONCE(f->dst);
+ const struct net_bridge_port *dst = br_fdb_dst_port(f);
int port_ifidx = dst ? dst->dev->ifindex : br->dev->ifindex;
if (desc->vlan_id && desc->vlan_id != f->key.vlan_id)
@@ -879,7 +882,7 @@ void br_fdb_delete_by_port(struct net_bridge *br,
spin_lock_bh(&br->hash_lock);
hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
- if (READ_ONCE(f->dst) != p)
+ if (br_fdb_dst_port(f) != p)
continue;
if (!do_all)
@@ -921,7 +924,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
continue;
/* ignore pseudo entry for local MAC address */
- dst = READ_ONCE(f->dst);
+ dst = br_fdb_dst_port(f);
if (!dst)
continue;
@@ -996,10 +999,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
}
/* fastpath: update of existing entry */
- if (unlikely(source != READ_ONCE(fdb->dst) &&
+ if (unlikely(source != br_fdb_dst_port(fdb) &&
!test_bit(BR_FDB_STICKY, &fdb->flags))) {
br_switchdev_fdb_notify(br, fdb, RTM_DELNEIGH);
- WRITE_ONCE(fdb->dst, source);
+ br_fdb_dst_write(fdb, br_port_to_dst(source));
fdb_modified = true;
/* Take over HW learned entry */
if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN,
@@ -1061,17 +1064,17 @@ int br_fdb_dump(struct sk_buff *skb,
rcu_read_lock();
hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
- const struct net_bridge_port *dst = READ_ONCE(f->dst);
+ const struct net_bridge_port *dst = br_fdb_dst_port(f);
if (*idx < ctx->fdb_idx)
goto skip;
if (filter_dev && (!dst || dst->dev != filter_dev)) {
if (filter_dev != dev)
goto skip;
- /* !f->dst is a special case for bridge
+ /* A NULL destination is a special case for bridge
* It means the MAC belongs to the bridge
* Therefore need a little more filtering
- * we only want to dump the !f->dst case
+ * we only want to dump the NULL destination case
*/
if (dst)
goto skip;
@@ -1193,8 +1196,8 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
if (flags & NLM_F_EXCL)
return -EEXIST;
- if (READ_ONCE(fdb->dst) != source) {
- WRITE_ONCE(fdb->dst, source);
+ if (br_fdb_dst_port(fdb) != source) {
+ br_fdb_dst_write(fdb, br_port_to_dst(source));
modified = true;
}
@@ -1385,7 +1388,7 @@ static int fdb_delete_by_addr_and_port(struct net_bridge *br,
struct net_bridge_fdb_entry *fdb;
fdb = br_fdb_find(br, addr, vlan);
- if (!fdb || READ_ONCE(fdb->dst) != p)
+ if (!fdb || br_fdb_dst_port(fdb) != p)
return -ENOENT;
fdb_delete(br, fdb, true);
@@ -1541,15 +1544,15 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
} else {
if (locked &&
(!test_bit(BR_FDB_LOCKED, &fdb->flags) ||
- READ_ONCE(fdb->dst) != p)) {
+ br_fdb_dst_port(fdb) != p)) {
err = -EINVAL;
goto err_unlock;
}
WRITE_ONCE(fdb->updated, jiffies);
- if (READ_ONCE(fdb->dst) != p) {
- WRITE_ONCE(fdb->dst, p);
+ if (br_fdb_dst_port(fdb) != p) {
+ br_fdb_dst_write(fdb, br_port_to_dst(p));
modified = true;
}
@@ -1632,7 +1635,7 @@ void br_fdb_clear_offload(const struct net_device *dev, u16 vid)
spin_lock_bh(&p->br->hash_lock);
hlist_for_each_entry(f, &p->br->fdb_list, fdb_node) {
- if (READ_ONCE(f->dst) == p && f->key.vlan_id == vid)
+ if (br_fdb_dst_port(f) == p && f->key.vlan_id == vid)
clear_bit(BR_FDB_OFFLOADED, &f->flags);
}
spin_unlock_bh(&p->br->hash_lock);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 4357d78524a6..68aa8fa52eba 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -123,7 +123,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
br_fdb_update(br, p, eth_hdr(skb)->h_source,
vid, BIT(BR_FDB_LOCKED));
goto drop;
- } else if (READ_ONCE(fdb_src->dst) != p ||
+ } else if (br_fdb_dst_port(fdb_src) != p ||
test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
/* FDB mismatch. Drop the packet without roaming. */
goto drop;
@@ -223,7 +223,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
if (now != READ_ONCE(dst->used))
WRITE_ONCE(dst->used, now);
- br_forward(READ_ONCE(dst->dst), skb, local_rcv, false);
+ br_forward(br_fdb_dst_port(dst), skb, local_rcv, false);
} else {
if (!mcast_hit)
br_flood(br, vlan, skb, pkt_type, local_rcv, false);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1146187aa2ba..bbb59b53e745 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -312,7 +312,7 @@ struct net_bridge_dst {
struct net_bridge_fdb_entry {
struct rhash_head rhnode;
- struct net_bridge_port *dst;
+ struct net_bridge_dst dst;
struct net_bridge_fdb_key key;
struct hlist_node fdb_node;
@@ -708,6 +708,24 @@ br_dst_port(struct net_bridge_dst dst)
return (struct net_bridge_port *)ACCESS_PRIVATE(&dst, value);
}
+static inline struct net_bridge_dst
+br_fdb_dst_read(const struct net_bridge_fdb_entry *fdb)
+{
+ return br_dst_read(&fdb->dst);
+}
+
+static inline void br_fdb_dst_write(struct net_bridge_fdb_entry *fdb,
+ struct net_bridge_dst dst)
+{
+ br_dst_write(&fdb->dst, dst);
+}
+
+static inline struct net_bridge_port *
+br_fdb_dst_port(const struct net_bridge_fdb_entry *fdb)
+{
+ return br_dst_port(br_fdb_dst_read(fdb));
+}
+
/* called under bridge lock */
static inline int br_is_root_bridge(const struct net_bridge *br)
{
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 990c6b38fd39..10f3aea6a1f8 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -129,7 +129,7 @@ static void br_switchdev_fdb_populate(struct net_bridge *br,
const struct net_bridge_fdb_entry *fdb,
const void *ctx)
{
- const struct net_bridge_port *p = READ_ONCE(fdb->dst);
+ const struct net_bridge_port *p = br_fdb_dst_port(fdb);
item->addr = fdb->key.addr.addr;
item->vid = fdb->key.vlan_id;
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 01/12] net: bridge: introduce a bridge destination type Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 02/12] net: bridge: use net_bridge_dst for fdb destinations Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-01 11:59 ` netdev-bot+sashiko
2026-09-30 7:14 ` [PATCH net-next 04/12] net: bridge: vlan: return VLAN entries from ingress helpers Nikolay Aleksandrov
` (9 subsequent siblings)
12 siblings, 1 reply; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Use bit 0 to distinguish port-VLAN pointers from bridge port pointers in
struct net_bridge_dst. Add helpers for constructing and decoding VLAN
destinations. No caller creates one yet, so behavior remains unchanged.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_private.h | 50 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index bbb59b53e745..2bf7f4429a3b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -306,6 +306,8 @@ struct net_bridge_fdb_key {
u16 vlan_id;
};
+#define BR_DST_VLAN_TAG BIT(0)
+
struct net_bridge_dst {
unsigned long __private value;
};
@@ -702,10 +704,56 @@ br_port_to_dst(const struct net_bridge_port *p)
return dst;
}
+static inline struct net_bridge_dst
+br_vlan_to_dst(const struct net_bridge_vlan *v)
+{
+ struct net_bridge_dst dst;
+
+ ACCESS_PRIVATE(&dst, value) = (unsigned long)v | BR_DST_VLAN_TAG;
+
+ return dst;
+}
+
+static inline void br_dst_decode(struct net_bridge_dst dst,
+ struct net_bridge_port **port,
+ struct net_bridge_vlan **vlan)
+{
+ struct net_bridge_vlan *v;
+ unsigned long value;
+
+ value = ACCESS_PRIVATE(&dst, value);
+ if (!(value & BR_DST_VLAN_TAG)) {
+ *port = (struct net_bridge_port *)value;
+ *vlan = NULL;
+ return;
+ }
+
+ v = (struct net_bridge_vlan *)(value & ~BR_DST_VLAN_TAG);
+ *port = v->port;
+ *vlan = v;
+}
+
static inline struct net_bridge_port *
br_dst_port(struct net_bridge_dst dst)
{
- return (struct net_bridge_port *)ACCESS_PRIVATE(&dst, value);
+ struct net_bridge_port *p;
+ struct net_bridge_vlan *v;
+
+ br_dst_decode(dst, &p, &v);
+
+ return p;
+}
+
+static inline struct net_bridge_vlan *
+br_dst_vlan(struct net_bridge_dst dst)
+{
+ unsigned long value;
+
+ value = ACCESS_PRIVATE(&dst, value);
+ if (!(value & BR_DST_VLAN_TAG))
+ return NULL;
+
+ return (struct net_bridge_vlan *)(value & ~BR_DST_VLAN_TAG);
}
static inline struct net_bridge_dst
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations
2026-09-30 7:14 ` [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations Nikolay Aleksandrov
@ 2026-10-01 11:59 ` netdev-bot+sashiko
2026-10-01 12:03 ` Nikolay Aleksandrov
0 siblings, 1 reply; 28+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 11:59 UTC (permalink / raw)
To: razor; +Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
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] Baseline decision: false positive.
--
net: bridge: add VLAN support to bridge destinations
This commit sets bit 0 of struct net_bridge_dst to mark a port-VLAN
pointer, so it can be told apart from a bridge port pointer. It adds
helpers that build VLAN destinations and decode them back into a port and
a VLAN. Nothing creates a VLAN destination yet.
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index bbb59b53e7452..2bf7f4429a3b2 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
[ ... ]
> @@ -702,10 +704,56 @@ br_port_to_dst(const struct net_bridge_port *p)
> return dst;
> }
>
> +static inline struct net_bridge_dst
> +br_vlan_to_dst(const struct net_bridge_vlan *v)
> +{
> + struct net_bridge_dst dst;
> +
> + ACCESS_PRIVATE(&dst, value) = (unsigned long)v | BR_DST_VLAN_TAG;
> +
> + return dst;
> +}
> +
> +static inline void br_dst_decode(struct net_bridge_dst dst,
> + struct net_bridge_port **port,
> + struct net_bridge_vlan **vlan)
> +{
> + struct net_bridge_vlan *v;
> + unsigned long value;
> +
> + value = ACCESS_PRIVATE(&dst, value);
> + if (!(value & BR_DST_VLAN_TAG)) {
> + *port = (struct net_bridge_port *)value;
> + *vlan = NULL;
> + return;
> + }
> +
> + v = (struct net_bridge_vlan *)(value & ~BR_DST_VLAN_TAG);
> + *port = v->port;
[Severity: Low]
This isn't a bug, but br_vlan_to_dst() takes any struct net_bridge_vlan
pointer, and br_dst_decode() reads v->port without checking which kind of
VLAN entry it has.
In struct net_bridge_vlan, port is in a union with br:
union {
struct net_bridge *br;
struct net_bridge_port *port;
};
For a bridge master VLAN, br is the active member. Decoding one would hand
back a struct net_bridge pointer typed as a struct net_bridge_port.
Passing NULL would encode to 0x1, and decoding that would dereference a
near-NULL address.
As far as I can tell, no caller can hit this. Later in the series, all of
br_fdb_update(), __br_fdb_add(), br_fdb_resolve_dst() and
nbp_vlan_delete() pass a non-NULL VLAN taken from a port VLAN group.
__br_fdb_add() also excludes the bridge master case on purpose:
dst = p && vlan ? br_vlan_to_dst(vlan) : br_port_to_dst(p);
Would a comment on br_vlan_to_dst() be worth adding, stating that it only
takes non-NULL port VLANs? Something like
WARN_ON_ONCE(br_vlan_is_master(v)) would also enforce that.
> + *vlan = v;
> +}
> +
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071411.2786201-1-razor%40blackwall.org
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations
2026-10-01 11:59 ` netdev-bot+sashiko
@ 2026-10-01 12:03 ` Nikolay Aleksandrov
0 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 12:03 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 01/10/2026 14:59, netdev-bot+sashiko@kernel.org wrote:
> 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] Baseline decision: false positive.
>
> --
>
> net: bridge: add VLAN support to bridge destinations
>
> This commit sets bit 0 of struct net_bridge_dst to mark a port-VLAN
> pointer, so it can be told apart from a bridge port pointer. It adds
> helpers that build VLAN destinations and decode them back into a port and
> a VLAN. Nothing creates a VLAN destination yet.
>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index bbb59b53e7452..2bf7f4429a3b2 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>
> [ ... ]
>
>> @@ -702,10 +704,56 @@ br_port_to_dst(const struct net_bridge_port *p)
>> return dst;
>> }
>>
>> +static inline struct net_bridge_dst
>> +br_vlan_to_dst(const struct net_bridge_vlan *v)
>> +{
>> + struct net_bridge_dst dst;
>> +
>> + ACCESS_PRIVATE(&dst, value) = (unsigned long)v | BR_DST_VLAN_TAG;
>> +
>> + return dst;
>> +}
>> +
>> +static inline void br_dst_decode(struct net_bridge_dst dst,
>> + struct net_bridge_port **port,
>> + struct net_bridge_vlan **vlan)
>> +{
>> + struct net_bridge_vlan *v;
>> + unsigned long value;
>> +
>> + value = ACCESS_PRIVATE(&dst, value);
>> + if (!(value & BR_DST_VLAN_TAG)) {
>> + *port = (struct net_bridge_port *)value;
>> + *vlan = NULL;
>> + return;
>> + }
>> +
>> + v = (struct net_bridge_vlan *)(value & ~BR_DST_VLAN_TAG);
>> + *port = v->port;
>
> [Severity: Low]
> This isn't a bug, but br_vlan_to_dst() takes any struct net_bridge_vlan
> pointer, and br_dst_decode() reads v->port without checking which kind of
> VLAN entry it has.
>
> In struct net_bridge_vlan, port is in a union with br:
>
> union {
> struct net_bridge *br;
> struct net_bridge_port *port;
> };
>
> For a bridge master VLAN, br is the active member. Decoding one would hand
> back a struct net_bridge pointer typed as a struct net_bridge_port.
> Passing NULL would encode to 0x1, and decoding that would dereference a
> near-NULL address.
>
> As far as I can tell, no caller can hit this. Later in the series, all of
> br_fdb_update(), __br_fdb_add(), br_fdb_resolve_dst() and
> nbp_vlan_delete() pass a non-NULL VLAN taken from a port VLAN group.
> __br_fdb_add() also excludes the bridge master case on purpose:
>
> dst = p && vlan ? br_vlan_to_dst(vlan) : br_port_to_dst(p);
>
> Would a comment on br_vlan_to_dst() be worth adding, stating that it only
> takes non-NULL port VLANs? Something like
> WARN_ON_ONCE(br_vlan_is_master(v)) would also enforce that.
>
- No, that is clear from the code and its usage. It would be redundant.
>> + *vlan = v;
>> +}
>> +
>
> [ ... ]
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net-next 04/12] net: bridge: vlan: return VLAN entries from ingress helpers
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (2 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 03/12] net: bridge: add VLAN support to bridge destinations Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 05/12] net: bridge: fdb: pass VLAN entries to learning updates Nikolay Aleksandrov
` (8 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Have the ingress helpers return the resolved vlan entry directly and let
callers derive the vid from it. This keeps the vlan available for later fdb
operations without carrying parallel vlan pointer and vid.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_device.c | 5 +++--
net/bridge/br_input.c | 14 ++++++++------
net/bridge/br_private.h | 12 +++++++-----
net/bridge/br_vlan.c | 38 +++++++++++++++++++++-----------------
4 files changed, 39 insertions(+), 30 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index c6804409b60b..2423d69f2498 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -65,9 +65,10 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
skb_reset_mac_header(skb);
skb_pull(skb, ETH_HLEN);
- if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &vid,
- &state, &vlan))
+ if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &state,
+ &vlan))
goto out;
+ vid = vlan ? vlan->vid : 0;
if (IS_ENABLED(CONFIG_INET) &&
(eth_hdr(skb)->h_proto == htons(ETH_P_ARP) ||
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 68aa8fa52eba..d9a0e1f65ec9 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -86,8 +86,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
struct net_bridge_vlan *vlan;
struct net_bridge *br;
bool promisc;
- u16 vid = 0;
u8 state;
+ u16 vid;
if (!p)
goto drop;
@@ -107,9 +107,10 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
brmctx = &p->br->multicast_ctx;
pmctx = &p->multicast_ctx;
- if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &vid,
- &state, &vlan))
+ if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &state,
+ &vlan))
goto out;
+ vid = vlan ? vlan->vid : 0;
if (test_bit(BR_PORT_LOCKED_BIT, &p->flags)) {
struct net_bridge_fdb_entry *fdb_src =
@@ -245,14 +246,15 @@ EXPORT_SYMBOL_GPL(br_handle_frame_finish);
static void __br_handle_local_finish(struct sk_buff *skb)
{
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
- u16 vid = 0;
+ struct net_bridge_vlan *vlan;
/* check if vlan is allowed, to avoid spoofing */
if (test_bit(BR_LEARNING_BIT, &p->flags) &&
nbp_state_should_learn(p) &&
!br_opt_get(p->br, BROPT_NO_LL_LEARN) &&
- br_should_learn(p, skb, &vid))
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, 0);
+ br_should_learn(p, skb, &vlan))
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source,
+ vlan ? vlan->vid : 0, 0);
}
/* note: already called with rcu_read_lock */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2bf7f4429a3b..34e95741b940 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1702,11 +1702,11 @@ br_multicast_ctx_options_equal(const struct net_bridge_mcast *brmctx1,
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
bool br_allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg, struct sk_buff *skb,
- u16 *vid, u8 *state,
- struct net_bridge_vlan **vlan);
+ u8 *state, struct net_bridge_vlan **vlan);
bool br_allowed_egress(struct net_bridge_vlan_group *vg,
const struct sk_buff *skb);
-bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid);
+bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb,
+ struct net_bridge_vlan **vlan);
struct sk_buff *br_handle_vlan(struct net_bridge *br,
const struct net_bridge_port *port,
struct net_bridge_vlan_group *vg,
@@ -1825,7 +1825,7 @@ static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
static inline bool br_allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg,
struct sk_buff *skb,
- u16 *vid, u8 *state,
+ u8 *state,
struct net_bridge_vlan **vlan)
{
@@ -1840,8 +1840,10 @@ static inline bool br_allowed_egress(struct net_bridge_vlan_group *vg,
}
static inline bool br_should_learn(struct net_bridge_port *p,
- struct sk_buff *skb, u16 *vid)
+ struct sk_buff *skb,
+ struct net_bridge_vlan **vlan)
{
+ *vlan = NULL;
return true;
}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index ce5aa15c4540..471b4e7f0571 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -583,13 +583,13 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
/* Called under RCU */
static bool __allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg,
- struct sk_buff *skb, u16 *vid,
- u8 *state,
+ struct sk_buff *skb, u8 *state,
struct net_bridge_vlan **vlan)
{
struct pcpu_sw_netstats *stats;
struct net_bridge_vlan *v;
bool tagged;
+ u16 vid;
BR_INPUT_SKB_CB(skb)->vlan_filtered = true;
/* If vlan tx offload is disabled on bridge device and frame was
@@ -603,7 +603,7 @@ static bool __allowed_ingress(const struct net_bridge *br,
return false;
}
- if (!br_vlan_get_tag(skb, vid)) {
+ if (!br_vlan_get_tag(skb, &vid)) {
/* Tagged frame */
if (skb->vlan_proto != br->vlan_proto) {
/* Protocol-mismatch, empty out vlan_tci for new tag */
@@ -615,7 +615,7 @@ static bool __allowed_ingress(const struct net_bridge *br,
skb_pull(skb, ETH_HLEN);
skb_reset_mac_len(skb);
- *vid = 0;
+ vid = 0;
tagged = false;
} else {
tagged = true;
@@ -625,7 +625,7 @@ static bool __allowed_ingress(const struct net_bridge *br,
tagged = false;
}
- if (!*vid) {
+ if (!vid) {
v = vg ? rcu_dereference(vg->pvid) : NULL;
/* Frame had a tag with VID 0 or did not have a tag.
* See if pvid is set on this port. That tells us which
@@ -637,7 +637,6 @@ static bool __allowed_ingress(const struct net_bridge *br,
/* PVID is set on this port. Any untagged or priority-tagged
* ingress frame is considered to belong to this vlan.
*/
- *vid = v->vid;
if (likely(!tagged))
/* Untagged Frame. */
__vlan_hwaccel_put_tag(skb, br->vlan_proto, v->vid);
@@ -649,7 +648,7 @@ static bool __allowed_ingress(const struct net_bridge *br,
*/
skb->vlan_tci |= v->vid;
} else {
- v = br_vlan_find(vg, *vid);
+ v = br_vlan_find(vg, vid);
}
if (!v || !br_vlan_should_use(v))
@@ -680,8 +679,7 @@ static bool __allowed_ingress(const struct net_bridge *br,
bool br_allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg, struct sk_buff *skb,
- u16 *vid, u8 *state,
- struct net_bridge_vlan **vlan)
+ u8 *state, struct net_bridge_vlan **vlan)
{
/* If VLAN filtering is disabled on the bridge, all packets are
* permitted.
@@ -692,7 +690,7 @@ bool br_allowed_ingress(const struct net_bridge *br,
return true;
}
- return __allowed_ingress(br, vg, skb, vid, state, vlan);
+ return __allowed_ingress(br, vg, skb, state, vlan);
}
/* Called under RCU. */
@@ -716,11 +714,15 @@ bool br_allowed_egress(struct net_bridge_vlan_group *vg,
}
/* Called under RCU */
-bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
+bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb,
+ struct net_bridge_vlan **vlan)
{
struct net_bridge_vlan_group *vg;
struct net_bridge *br = p->br;
struct net_bridge_vlan *v;
+ u16 vid;
+
+ *vlan = NULL;
/* If filtering was disabled at input, let it pass. */
if (!br_opt_get(br, BROPT_VLAN_ENABLED))
@@ -730,20 +732,22 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
if (!vg || !READ_ONCE(vg->num_vlans))
return false;
- if (!br_vlan_get_tag(skb, vid) && skb->vlan_proto != br->vlan_proto)
- *vid = 0;
+ if (!br_vlan_get_tag(skb, &vid) && skb->vlan_proto != br->vlan_proto)
+ vid = 0;
- if (!*vid) {
+ if (!vid) {
v = rcu_dereference(vg->pvid);
if (!v || !br_vlan_state_allowed(br_vlan_get_state(v), true))
return false;
- *vid = v->vid;
+ *vlan = v;
return true;
}
- v = br_vlan_find(vg, *vid);
- if (v && br_vlan_state_allowed(br_vlan_get_state(v), true))
+ v = br_vlan_find(vg, vid);
+ if (v && br_vlan_state_allowed(br_vlan_get_state(v), true)) {
+ *vlan = v;
return true;
+ }
return false;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 05/12] net: bridge: fdb: pass VLAN entries to learning updates
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (3 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 04/12] net: bridge: vlan: return VLAN entries from ingress helpers Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup Nikolay Aleksandrov
` (7 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Pass the resolved VLAN entry through fdb learning paths and derive the vid
inside br_fdb_update(). The fdb destination remains a port for now, this is
a preparation for caching the VLAN.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_fdb.c | 16 ++++++++++------
net/bridge/br_input.c | 12 ++++++------
net/bridge/br_private.h | 3 ++-
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 0f5cf5615b56..9cd6deae8635 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -974,9 +974,11 @@ static bool __fdb_mark_active(struct net_bridge_fdb_entry *fdb)
}
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr, u16 vid, unsigned long flags)
+ struct net_bridge_vlan *vlan, const unsigned char *addr,
+ unsigned long flags)
{
struct net_bridge_fdb_entry *fdb;
+ u16 vid = vlan ? vlan->vid : 0;
/* some users want to always flood. */
if (hold_time(br) == 0)
@@ -1247,9 +1249,11 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
struct net_bridge_port *p, const unsigned char *addr,
- u16 nlh_flags, u16 vid, struct nlattr *nfea_tb[],
+ u16 nlh_flags, struct net_bridge_vlan *vlan,
+ struct nlattr *nfea_tb[],
bool *notified, struct netlink_ext_ack *extack)
{
+ u16 vid = vlan ? vlan->vid : 0;
int err = 0;
if (ndm->ndm_flags & NTF_USE) {
@@ -1263,7 +1267,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
local_bh_disable();
rcu_read_lock();
- br_fdb_update(br, p, addr, vid, BIT(BR_FDB_ADDED_BY_USER));
+ br_fdb_update(br, p, vlan, addr, BIT(BR_FDB_ADDED_BY_USER));
rcu_read_unlock();
local_bh_enable();
} else if (ndm->ndm_flags & NTF_EXT_LEARNED) {
@@ -1355,10 +1359,10 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
}
/* VID was specified, so use it. */
- err = __br_fdb_add(ndm, br, p, addr, nlh_flags, vid, nfea_tb,
+ err = __br_fdb_add(ndm, br, p, addr, nlh_flags, v, nfea_tb,
notified, extack);
} else {
- err = __br_fdb_add(ndm, br, p, addr, nlh_flags, 0, nfea_tb,
+ err = __br_fdb_add(ndm, br, p, addr, nlh_flags, NULL, nfea_tb,
notified, extack);
if (err || !vg || !vg->num_vlans)
goto out;
@@ -1370,7 +1374,7 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
list_for_each_entry(v, &vg->vlan_list, vlist) {
if (!br_vlan_should_use(v))
continue;
- err = __br_fdb_add(ndm, br, p, addr, nlh_flags, v->vid,
+ err = __br_fdb_add(ndm, br, p, addr, nlh_flags, v,
nfea_tb, notified, extack);
if (err)
goto out;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index d9a0e1f65ec9..44a217d65e7f 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -121,8 +121,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
* and drop the packet.
*/
if (test_bit(BR_PORT_MAB_BIT, &p->flags))
- br_fdb_update(br, p, eth_hdr(skb)->h_source,
- vid, BIT(BR_FDB_LOCKED));
+ br_fdb_update(br, p, vlan,
+ eth_hdr(skb)->h_source,
+ BIT(BR_FDB_LOCKED));
goto drop;
} else if (br_fdb_dst_port(fdb_src) != p ||
test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
@@ -132,7 +133,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
/* FDB match, but entry is locked. Refresh it and drop
* the packet.
*/
- br_fdb_update(br, p, eth_hdr(skb)->h_source, vid,
+ br_fdb_update(br, p, vlan, eth_hdr(skb)->h_source,
BIT(BR_FDB_LOCKED));
goto drop;
}
@@ -142,7 +143,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
/* insert into forwarding database after filtering to avoid spoofing */
if (test_bit(BR_LEARNING_BIT, &p->flags))
- br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
+ br_fdb_update(br, p, vlan, eth_hdr(skb)->h_source, 0);
promisc = !!(br->dev->flags & IFF_PROMISC);
local_rcv = promisc;
@@ -253,8 +254,7 @@ static void __br_handle_local_finish(struct sk_buff *skb)
nbp_state_should_learn(p) &&
!br_opt_get(p->br, BROPT_NO_LL_LEARN) &&
br_should_learn(p, skb, &vlan))
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source,
- vlan ? vlan->vid : 0, 0);
+ br_fdb_update(p->br, p, vlan, eth_hdr(skb)->h_source, 0);
}
/* note: already called with rcu_read_lock */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 34e95741b940..058cc082311a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -993,7 +993,8 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf, unsigned long count,
int br_fdb_add_local(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid);
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr, u16 vid, unsigned long flags);
+ struct net_bridge_vlan *vlan, const unsigned char *addr,
+ unsigned long flags);
int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev, const unsigned char *addr, u16 vid,
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (4 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 05/12] net: bridge: fdb: pass VLAN entries to learning updates Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-01 11:59 ` netdev-bot+sashiko
2026-09-30 7:14 ` [PATCH net-next 07/12] net: bridge: vlan: split unpublishing from deletion Nikolay Aleksandrov
` (6 subsequent siblings)
12 siblings, 1 reply; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Deleting a port-VLAN currently takes the hash_lock twice and performs two
fdb ops. The first removes its automatically generated local entry and the
second deletes dynamic entries associated with the port and vid. Rename
br_fdb_delete_by_port() to br_fdb_cleanup_by_dst() and pass a bridge dst so
it can distinguish a port-VLAN from a raw port. This allows both operations
to be done during the same locked fdb walk.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br.c | 2 +-
net/bridge/br_fdb.c | 17 +++++++++++++----
net/bridge/br_if.c | 6 +++---
net/bridge/br_netlink.c | 2 +-
net/bridge/br_private.h | 4 ++--
net/bridge/br_stp_if.c | 2 +-
net/bridge/br_sysfs_if.c | 3 ++-
net/bridge/br_vlan.c | 5 ++---
8 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/net/bridge/br.c b/net/bridge/br.c
index 09a120b3b76c..b1abe1c5bbdd 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -202,7 +202,7 @@ static int br_switchdev_event(struct notifier_block *unused,
case SWITCHDEV_FDB_FLUSH_TO_BRIDGE:
fdb_info = ptr;
/* Don't delete static entries */
- br_fdb_delete_by_port(br, p, fdb_info->vid, 0);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(p), fdb_info->vid, 0);
break;
}
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 9cd6deae8635..7c68b540b358 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -868,15 +868,16 @@ int br_fdb_delete_bulk(struct nlmsghdr *nlh, struct net_device *dev,
return 0;
}
-/* Flush all entries referring to a specific port.
+/* Clean up all entries referring to a specific destination.
* if do_all is set also flush static entries
* if vid is set delete all entries that match the vlan_id
*/
-void br_fdb_delete_by_port(struct net_bridge *br,
- const struct net_bridge_port *p,
- u16 vid,
+void br_fdb_cleanup_by_dst(struct net_bridge *br,
+ struct net_bridge_dst cleanup_dst, u16 vid,
int do_all)
{
+ const struct net_bridge_vlan *vlan = br_dst_vlan(cleanup_dst);
+ const struct net_bridge_port *p = br_dst_port(cleanup_dst);
struct net_bridge_fdb_entry *f;
struct hlist_node *tmp;
@@ -885,6 +886,14 @@ void br_fdb_delete_by_port(struct net_bridge *br,
if (br_fdb_dst_port(f) != p)
continue;
+ if (vlan && f->key.vlan_id == vlan->vid &&
+ test_bit(BR_FDB_LOCAL, &f->flags) &&
+ !test_bit(BR_FDB_ADDED_BY_USER, &f->flags) &&
+ ether_addr_equal(f->key.addr.addr, p->dev->dev_addr)) {
+ fdb_delete_local(br, p, f);
+ continue;
+ }
+
if (!do_all)
if (test_bit(BR_FDB_STATIC, &f->flags) ||
(test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f88..d94558a5e3e9 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -355,7 +355,7 @@ static void del_nbp(struct net_bridge_port *p)
netdev_reset_rx_headroom(dev);
nbp_vlan_flush(p);
- br_fdb_delete_by_port(br, p, 0, 1);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1);
switchdev_deferred_process();
nbp_backup_clear(p);
@@ -390,7 +390,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
br_mst_uninit(br);
br_recalculate_neigh_suppress_enabled(br);
- br_fdb_delete_by_port(br, NULL, 0, 1);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(NULL), 0, 1);
timer_shutdown_sync(&br->hello_timer);
timer_shutdown_sync(&br->topology_change_timer);
@@ -696,7 +696,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev,
if (fdb_synced)
br_fdb_unsync_static(br, p);
list_del_rcu(&p->list);
- br_fdb_delete_by_port(br, p, 0, 1);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1);
nbp_update_port_count(br);
netdev_upper_dev_unlink(dev, br->dev);
err5:
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index ae76df0de05a..f65b8b6ca97c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1044,7 +1044,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[],
}
if (tb[IFLA_BRPORT_FLUSH])
- br_fdb_delete_by_port(p->br, p, 0, 0);
+ br_fdb_cleanup_by_dst(p->br, br_port_to_dst(p), 0, 0);
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
if (tb[IFLA_BRPORT_MULTICAST_ROUTER]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 058cc082311a..a790368b69e9 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -983,8 +983,8 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr);
void br_fdb_cleanup(struct work_struct *work);
int br_fdb_toggle_local_vlan_0(struct net_bridge *br, bool on,
struct netlink_ext_ack *extack);
-void br_fdb_delete_by_port(struct net_bridge *br,
- const struct net_bridge_port *p, u16 vid, int do_all);
+void br_fdb_cleanup_by_dst(struct net_bridge *br,
+ struct net_bridge_dst dst, u16 vid, int do_all);
struct net_bridge_fdb_entry *br_fdb_find_rcu(struct net_bridge *br,
const unsigned char *addr,
__u16 vid);
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index a7e5422eb5d1..2e5190278c90 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -114,7 +114,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
timer_delete(&p->hold_timer);
if (!rcu_access_pointer(p->backup_port))
- br_fdb_delete_by_port(br, p, 0, 0);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 0);
br_multicast_disable_port(p);
br_configuration_update(br);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index b9bcafa7f034..621d62a063d9 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -191,7 +191,8 @@ static BRPORT_ATTR(hold_timer, 0444, show_hold_timer, NULL);
static int store_flush(struct net_bridge_port *p, unsigned long v)
{
- br_fdb_delete_by_port(p->br, p, 0, 0); // Don't delete local entry
+ /* Don't delete local entry */
+ br_fdb_cleanup_by_dst(p->br, br_port_to_dst(p), 0, 0);
return 0;
}
static BRPORT_ATTR(flush, 0200, NULL, store_flush);
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 471b4e7f0571..7ba6e57b18fa 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -870,7 +870,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid)
return -ENOENT;
br_fdb_find_delete_local(br, NULL, br->dev->dev_addr, vid);
- br_fdb_delete_by_port(br, NULL, vid, 0);
+ br_fdb_cleanup_by_dst(br, br_port_to_dst(NULL), vid, 0);
vlan_tunnel_info_del(vg, v);
__vlan_del(v);
@@ -1396,8 +1396,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
v = br_vlan_find(nbp_vlan_group(port), vid);
if (!v)
return -ENOENT;
- br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid);
- br_fdb_delete_by_port(port->br, port, vid, 0);
+ br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
__vlan_del(v);
return 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup
2026-09-30 7:14 ` [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup Nikolay Aleksandrov
@ 2026-10-01 11:59 ` netdev-bot+sashiko
2026-10-01 12:05 ` Nikolay Aleksandrov
0 siblings, 1 reply; 28+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 11:59 UTC (permalink / raw)
To: razor; +Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
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 patch rewrote the header comment of br_fdb_cleanup_by_dst()
in net/bridge/br_fdb.c, but the comment does not describe what the…
--
net: bridge: fdb: consolidate port-VLAN cleanup
This renames br_fdb_delete_by_port() to br_fdb_cleanup_by_dst() and passes
a bridge dst, so the function can tell a port-VLAN from a raw port. For a
port-VLAN dst, the port's automatically generated local entry for that VLAN
is now removed in the same locked fdb walk that flushes dynamic entries.
As a result, nbp_vlan_delete() no longer calls br_fdb_find_delete_local()
separately.
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 9cd6deae86359..7c68b540b3580 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -868,15 +868,16 @@ int br_fdb_delete_bulk(struct nlmsghdr *nlh, struct net_device *dev,
> return 0;
> }
>
> -/* Flush all entries referring to a specific port.
> +/* Clean up all entries referring to a specific destination.
> * if do_all is set also flush static entries
> * if vid is set delete all entries that match the vlan_id
> */
> -void br_fdb_delete_by_port(struct net_bridge *br,
> - const struct net_bridge_port *p,
> - u16 vid,
> +void br_fdb_cleanup_by_dst(struct net_bridge *br,
> + struct net_bridge_dst cleanup_dst, u16 vid,
> int do_all)
> {
> + const struct net_bridge_vlan *vlan = br_dst_vlan(cleanup_dst);
> + const struct net_bridge_port *p = br_dst_port(cleanup_dst);
> struct net_bridge_fdb_entry *f;
> struct hlist_node *tmp;
>
> @@ -885,6 +886,14 @@ void br_fdb_delete_by_port(struct net_bridge *br,
> if (br_fdb_dst_port(f) != p)
> continue;
>
> + if (vlan && f->key.vlan_id == vlan->vid &&
> + test_bit(BR_FDB_LOCAL, &f->flags) &&
> + !test_bit(BR_FDB_ADDED_BY_USER, &f->flags) &&
> + ether_addr_equal(f->key.addr.addr, p->dev->dev_addr)) {
> + fdb_delete_local(br, p, f);
> + continue;
> + }
> +
> if (!do_all)
> if (test_bit(BR_FDB_STATIC, &f->flags) ||
> (test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
[Severity: Low]
This isn't a bug, but does the rewritten header comment above
br_fdb_cleanup_by_dst() still describe what the function does?
The comment says static entries are only flushed when do_all is set. With a
port-VLAN dst, the new branch runs before the do_all check. It removes the
port's auto-generated local entry even when do_all == 0. fdb_add_local()
creates that entry as static:
net/bridge/br_fdb.c:fdb_add_local() {
...
fdb = fdb_create(br, source, addr, vid,
BIT(BR_FDB_LOCAL) | BIT(BR_FDB_STATIC));
...
}
The comment also says the function cleans up "all entries referring to a
specific destination". The walk only compares the port part of the dst:
if (br_fdb_dst_port(f) != p)
continue;
The VLAN part is used only for the local-entry case.
The local-entry match uses vlan->vid, but the dynamic-entry filter uses the
separate vid argument, and nothing ties the two together. nbp_vlan_delete()
is fine today because it gets v from br_vlan_find(vg, vid).
A later caller might pass br_vlan_to_dst(v) with vid == 0 or with a
different vid. That call would remove v's local entry. It would then flush
dynamic entries across all of the port's VLANs, or in a different VLAN.
Could the comment say that vid is expected to match the VLAN's vid when a
VLAN dst is passed?
The line "if vid is set delete all entries that match the vlan_id" was
already inaccurate before this patch, because vid is ignored under do_all
and static or ext-learned entries are kept. That part is pre-existing, but
the patch kept the line when it rewrote the comment.
Later in the series, in "net: bridge: fdb: avoid VLAN lookups in unicast
forwarding", the comment is still the same. By then the function does more
VLAN-specific work, including falling back from the VLAN dst to the port
dst.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071411.2786201-1-razor%40blackwall.org
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup
2026-10-01 11:59 ` netdev-bot+sashiko
@ 2026-10-01 12:05 ` Nikolay Aleksandrov
0 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 12:05 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 01/10/2026 14:59, netdev-bot+sashiko@kernel.org wrote:
> 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 patch rewrote the header comment of br_fdb_cleanup_by_dst()
> in net/bridge/br_fdb.c, but the comment does not describe what the…
>
> --
>
> net: bridge: fdb: consolidate port-VLAN cleanup
>
> This renames br_fdb_delete_by_port() to br_fdb_cleanup_by_dst() and passes
> a bridge dst, so the function can tell a port-VLAN from a raw port. For a
> port-VLAN dst, the port's automatically generated local entry for that VLAN
> is now removed in the same locked fdb walk that flushes dynamic entries.
> As a result, nbp_vlan_delete() no longer calls br_fdb_find_delete_local()
> separately.
>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 9cd6deae86359..7c68b540b3580 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -868,15 +868,16 @@ int br_fdb_delete_bulk(struct nlmsghdr *nlh, struct net_device *dev,
>> return 0;
>> }
>>
>> -/* Flush all entries referring to a specific port.
>> +/* Clean up all entries referring to a specific destination.
>> * if do_all is set also flush static entries
>> * if vid is set delete all entries that match the vlan_id
>> */
>> -void br_fdb_delete_by_port(struct net_bridge *br,
>> - const struct net_bridge_port *p,
>> - u16 vid,
>> +void br_fdb_cleanup_by_dst(struct net_bridge *br,
>> + struct net_bridge_dst cleanup_dst, u16 vid,
>> int do_all)
>> {
>> + const struct net_bridge_vlan *vlan = br_dst_vlan(cleanup_dst);
>> + const struct net_bridge_port *p = br_dst_port(cleanup_dst);
>> struct net_bridge_fdb_entry *f;
>> struct hlist_node *tmp;
>>
>> @@ -885,6 +886,14 @@ void br_fdb_delete_by_port(struct net_bridge *br,
>> if (br_fdb_dst_port(f) != p)
>> continue;
>>
>> + if (vlan && f->key.vlan_id == vlan->vid &&
>> + test_bit(BR_FDB_LOCAL, &f->flags) &&
>> + !test_bit(BR_FDB_ADDED_BY_USER, &f->flags) &&
>> + ether_addr_equal(f->key.addr.addr, p->dev->dev_addr)) {
>> + fdb_delete_local(br, p, f);
>> + continue;
>> + }
>> +
>> if (!do_all)
>> if (test_bit(BR_FDB_STATIC, &f->flags) ||
>> (test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
>
> [Severity: Low]
> This isn't a bug, but does the rewritten header comment above
> br_fdb_cleanup_by_dst() still describe what the function does?
>
> The comment says static entries are only flushed when do_all is set. With a
> port-VLAN dst, the new branch runs before the do_all check. It removes the
> port's auto-generated local entry even when do_all == 0. fdb_add_local()
> creates that entry as static:
>
> net/bridge/br_fdb.c:fdb_add_local() {
> ...
> fdb = fdb_create(br, source, addr, vid,
> BIT(BR_FDB_LOCAL) | BIT(BR_FDB_STATIC));
> ...
> }
>
> The comment also says the function cleans up "all entries referring to a
> specific destination". The walk only compares the port part of the dst:
>
> if (br_fdb_dst_port(f) != p)
> continue;
>
> The VLAN part is used only for the local-entry case.
>
> The local-entry match uses vlan->vid, but the dynamic-entry filter uses the
> separate vid argument, and nothing ties the two together. nbp_vlan_delete()
> is fine today because it gets v from br_vlan_find(vg, vid).
>
> A later caller might pass br_vlan_to_dst(v) with vid == 0 or with a
> different vid. That call would remove v's local entry. It would then flush
> dynamic entries across all of the port's VLANs, or in a different VLAN.
>
> Could the comment say that vid is expected to match the VLAN's vid when a
> VLAN dst is passed?
>
> The line "if vid is set delete all entries that match the vlan_id" was
> already inaccurate before this patch, because vid is ignored under do_all
> and static or ext-learned entries are kept. That part is pre-existing, but
> the patch kept the line when it rewrote the comment.
>
> Later in the series, in "net: bridge: fdb: avoid VLAN lookups in unicast
> forwarding", the comment is still the same. By then the function does more
> VLAN-specific work, including falling back from the VLAN dst to the port
> dst.
>
Hmm ok, I'd prefer to take care of the comment in a follow up as I have a few
minor cleanups gathered from the previous set review and can add this on top.
I'll take care of the pre-existing inaccuracy as well.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net-next 07/12] net: bridge: vlan: split unpublishing from deletion
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (5 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 06/12] net: bridge: fdb: consolidate port-VLAN cleanup Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs Nikolay Aleksandrov
` (5 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Split removal of a VLAN from its lookup and flood structures from object
destruction. Pass the VLAN group explicitly to __vlan_del() and call both
operations consecutively for now. This prepares deletion paths to quiesce
readers after making an entry unreachable and before freeing it.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_vlan.c | 45 +++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 7ba6e57b18fa..8a914e249767 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -429,22 +429,34 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
goto out;
}
-static void __vlan_del(struct net_bridge_vlan *v)
+static void __vlan_unpublish(struct net_bridge_vlan_group *vg,
+ struct net_bridge_vlan *v)
+{
+ __vlan_delete_pvid(vg, v);
+ if (!br_vlan_is_master(v)) {
+ struct net_bridge_vlan *masterv = v->brvlan;
+
+ rhashtable_remove_fast(&vg->vlan_hash, &v->vnode,
+ br_vlan_rht_params);
+ __vlan_del_list(v);
+ /* -1 because br_vlan_put_master() is called later */
+ br_vlan_rebuild_port_array(masterv,
+ br_vlan_num_ports(masterv) - 1);
+ }
+}
+
+static void __vlan_del(struct net_bridge_vlan_group *vg,
+ struct net_bridge_vlan *v)
{
struct net_bridge_vlan *masterv = v;
- struct net_bridge_vlan_group *vg;
struct net_bridge_port *p = NULL;
int err;
- if (br_vlan_is_master(v)) {
- vg = br_vlan_group(v->br);
- } else {
+ if (!br_vlan_is_master(v)) {
p = v->port;
- vg = nbp_vlan_group(v->port);
masterv = v->brvlan;
}
- __vlan_delete_pvid(vg, v);
if (p) {
err = __vlan_vid_del(p->dev, p->br, v);
if (err)
@@ -467,12 +479,6 @@ static void __vlan_del(struct net_bridge_vlan *v)
if (masterv != v) {
vlan_tunnel_info_del(vg, v);
- rhashtable_remove_fast(&vg->vlan_hash, &v->vnode,
- br_vlan_rht_params);
- __vlan_del_list(v);
- /* -1 because br_vlan_put_master() is called later */
- br_vlan_rebuild_port_array(masterv,
- br_vlan_num_ports(masterv) - 1);
nbp_vlan_set_vlan_dev_state(p, v->vid);
br_multicast_toggle_one_vlan(v, false);
br_multicast_port_ctx_deinit(&v->port_mcast_ctx);
@@ -509,7 +515,8 @@ static void __vlan_flush(const struct net_bridge *br,
}
v_end = vlan->vid;
- __vlan_del(vlan);
+ __vlan_unpublish(vg, vlan);
+ __vlan_del(vg, vlan);
}
/* notify about the last/whole vlan range */
@@ -873,7 +880,8 @@ int br_vlan_delete(struct net_bridge *br, u16 vid)
br_fdb_cleanup_by_dst(br, br_port_to_dst(NULL), vid, 0);
vlan_tunnel_info_del(vg, v);
- __vlan_del(v);
+ __vlan_unpublish(vg, v);
+ __vlan_del(vg, v);
return 0;
}
@@ -1389,15 +1397,18 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
*/
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
{
+ struct net_bridge_vlan_group *vg;
struct net_bridge_vlan *v;
ASSERT_RTNL();
- v = br_vlan_find(nbp_vlan_group(port), vid);
+ vg = nbp_vlan_group(port);
+ v = br_vlan_find(vg, vid);
if (!v)
return -ENOENT;
br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
- __vlan_del(v);
+ __vlan_unpublish(vg, v);
+ __vlan_del(vg, v);
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (6 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 07/12] net: bridge: vlan: split unpublishing from deletion Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-01 7:28 ` Nikolay Aleksandrov
2026-10-01 11:59 ` netdev-bot+sashiko
2026-09-30 7:14 ` [PATCH net-next 09/12] net: bridge: fdb: factor out existing entry updates Nikolay Aleksandrov
` (4 subsequent siblings)
12 siblings, 2 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Later fdb entries will cache port-VLAN pointers so unpublish a VLAN, wait
for a grace period (existing readers) and then purge or rewrite fdb
references before releasing it. Cached destinations can continue forwarding
until they are cleaned, that is acceptable so add a comment to document it.
During port teardown unpublish the complete VLAN group first, clean the
port fdbs and then release the VLANs. This lets all VLANs share one grace
period.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_fdb.c | 20 ++++++++++++++++----
net/bridge/br_if.c | 7 +++++--
net/bridge/br_private.h | 12 ++++++++++--
net/bridge/br_vlan.c | 21 +++++++++++++++------
4 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 7c68b540b358..307f9c12914e 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -883,7 +883,9 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
spin_lock_bh(&br->hash_lock);
hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
- if (br_fdb_dst_port(f) != p)
+ struct net_bridge_dst dst = br_fdb_dst_read(f);
+
+ if (br_dst_port(dst) != p)
continue;
if (vlan && f->key.vlan_id == vlan->vid &&
@@ -894,12 +896,22 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
continue;
}
- if (!do_all)
+ if (!do_all) {
+ if (vid && f->key.vlan_id != vid)
+ continue;
+
if (test_bit(BR_FDB_STATIC, &f->flags) ||
(test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
- !test_bit(BR_FDB_OFFLOADED, &f->flags)) ||
- (vid && f->key.vlan_id != vid))
+ !test_bit(BR_FDB_OFFLOADED, &f->flags))) {
+ /* The entry outlives the VLAN, so it must fall
+ * back to the raw port destination
+ */
+ if (vlan && br_dst_vlan(dst) == vlan)
+ br_fdb_dst_write(f,
+ br_port_to_dst(p));
continue;
+ }
+ }
if (test_bit(BR_FDB_LOCAL, &f->flags))
fdb_delete_local(br, p, f);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index d94558a5e3e9..2c05ebc1299d 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -333,8 +333,9 @@ static void update_headroom(struct net_bridge *br, int new_hr)
*/
static void del_nbp(struct net_bridge_port *p)
{
- struct net_bridge *br = p->br;
+ struct net_bridge_vlan_group *vg;
struct net_device *dev = p->dev;
+ struct net_bridge *br = p->br;
sysfs_remove_link(br->ifobj, p->dev->name);
@@ -354,8 +355,10 @@ static void del_nbp(struct net_bridge_port *p)
update_headroom(br, get_max_headroom(br));
netdev_reset_rx_headroom(dev);
- nbp_vlan_flush(p);
+ vg = nbp_vlan_group(p);
+ nbp_vlan_group_unpublish(p);
br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1);
+ nbp_vlan_flush(p, vg);
switchdev_deferred_process();
nbp_backup_clear(p);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a790368b69e9..951b6ac5f484 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1735,7 +1735,9 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid,
int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
bool *changed, struct netlink_ext_ack *extack);
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
-void nbp_vlan_flush(struct net_bridge_port *port);
+void nbp_vlan_group_unpublish(struct net_bridge_port *port);
+void nbp_vlan_flush(struct net_bridge_port *port,
+ struct net_bridge_vlan_group *vg);
int nbp_vlan_init(struct net_bridge_port *port, struct netlink_ext_ack *extack);
int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
void br_vlan_get_stats(const struct net_bridge_vlan *v,
@@ -1894,7 +1896,13 @@ static inline int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
return -EOPNOTSUPP;
}
-static inline void nbp_vlan_flush(struct net_bridge_port *port)
+static inline void nbp_vlan_group_unpublish(struct net_bridge_port *port)
+{
+}
+
+static inline void
+nbp_vlan_flush(struct net_bridge_port *port,
+ struct net_bridge_vlan_group *vg)
{
}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8a914e249767..0675f74cf1b9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1406,23 +1406,32 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
v = br_vlan_find(vg, vid);
if (!v)
return -ENOENT;
- br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
__vlan_unpublish(vg, v);
+ synchronize_net();
+ /* Traffic may still use v through cached fdb dsts until they are
+ * cleaned below. This is acceptable during vlan deletion. Above we
+ * drain the readers that could republish the dst before cleaning it
+ */
+ br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
__vlan_del(vg, v);
return 0;
}
-void nbp_vlan_flush(struct net_bridge_port *port)
+void nbp_vlan_group_unpublish(struct net_bridge_port *port)
{
- struct net_bridge_vlan_group *vg;
-
ASSERT_RTNL();
- vg = nbp_vlan_group(port);
- __vlan_flush(port->br, port, vg);
RCU_INIT_POINTER(port->vlgrp, NULL);
synchronize_net();
+}
+
+void nbp_vlan_flush(struct net_bridge_port *port,
+ struct net_bridge_vlan_group *vg)
+{
+ ASSERT_RTNL();
+
+ __vlan_flush(port->br, port, vg);
__vlan_group_free(vg);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-09-30 7:14 ` [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs Nikolay Aleksandrov
@ 2026-10-01 7:28 ` Nikolay Aleksandrov
2026-10-01 11:59 ` netdev-bot+sashiko
1 sibling, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 7:28 UTC (permalink / raw)
To: netdev; +Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 30/09/2026 10:14, Nikolay Aleksandrov wrote:
> Later fdb entries will cache port-VLAN pointers so unpublish a VLAN, wait
> for a grace period (existing readers) and then purge or rewrite fdb
> references before releasing it. Cached destinations can continue forwarding
> until they are cleaned, that is acceptable so add a comment to document it.
> During port teardown unpublish the complete VLAN group first, clean the
> port fdbs and then release the VLANs. This lets all VLANs share one grace
> period.
>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
> net/bridge/br_fdb.c | 20 ++++++++++++++++----
> net/bridge/br_if.c | 7 +++++--
> net/bridge/br_private.h | 12 ++++++++++--
> net/bridge/br_vlan.c | 21 +++++++++++++++------
> 4 files changed, 46 insertions(+), 14 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 7c68b540b358..307f9c12914e 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -883,7 +883,9 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
>
> spin_lock_bh(&br->hash_lock);
> hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
> - if (br_fdb_dst_port(f) != p)
> + struct net_bridge_dst dst = br_fdb_dst_read(f);
> +
> + if (br_dst_port(dst) != p)
> continue;
>
> if (vlan && f->key.vlan_id == vlan->vid &&
> @@ -894,12 +896,22 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
> continue;
> }
>
> - if (!do_all)
> + if (!do_all) {
> + if (vid && f->key.vlan_id != vid)
> + continue;
> +
> if (test_bit(BR_FDB_STATIC, &f->flags) ||
> (test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
> - !test_bit(BR_FDB_OFFLOADED, &f->flags)) ||
> - (vid && f->key.vlan_id != vid))
> + !test_bit(BR_FDB_OFFLOADED, &f->flags))) {
> + /* The entry outlives the VLAN, so it must fall
> + * back to the raw port destination
> + */
> + if (vlan && br_dst_vlan(dst) == vlan)
> + br_fdb_dst_write(f,
> + br_port_to_dst(p));
> continue;
> + }
> + }
>
> if (test_bit(BR_FDB_LOCAL, &f->flags))
> fdb_delete_local(br, p, f);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index d94558a5e3e9..2c05ebc1299d 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -333,8 +333,9 @@ static void update_headroom(struct net_bridge *br, int new_hr)
> */
> static void del_nbp(struct net_bridge_port *p)
> {
> - struct net_bridge *br = p->br;
> + struct net_bridge_vlan_group *vg;
> struct net_device *dev = p->dev;
> + struct net_bridge *br = p->br;
>
> sysfs_remove_link(br->ifobj, p->dev->name);
>
> @@ -354,8 +355,10 @@ static void del_nbp(struct net_bridge_port *p)
> update_headroom(br, get_max_headroom(br));
> netdev_reset_rx_headroom(dev);
>
> - nbp_vlan_flush(p);
> + vg = nbp_vlan_group(p);
> + nbp_vlan_group_unpublish(p);
> br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1);
> + nbp_vlan_flush(p, vg);
> switchdev_deferred_process();
> nbp_backup_clear(p);
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index a790368b69e9..951b6ac5f484 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1735,7 +1735,9 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid,
> int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags,
> bool *changed, struct netlink_ext_ack *extack);
> int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
> -void nbp_vlan_flush(struct net_bridge_port *port);
> +void nbp_vlan_group_unpublish(struct net_bridge_port *port);
> +void nbp_vlan_flush(struct net_bridge_port *port,
> + struct net_bridge_vlan_group *vg);
> int nbp_vlan_init(struct net_bridge_port *port, struct netlink_ext_ack *extack);
> int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
> void br_vlan_get_stats(const struct net_bridge_vlan *v,
> @@ -1894,7 +1896,13 @@ static inline int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
> return -EOPNOTSUPP;
> }
>
> -static inline void nbp_vlan_flush(struct net_bridge_port *port)
> +static inline void nbp_vlan_group_unpublish(struct net_bridge_port *port)
> +{
> +}
> +
> +static inline void
> +nbp_vlan_flush(struct net_bridge_port *port,
> + struct net_bridge_vlan_group *vg)
> {
> }
>
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 8a914e249767..0675f74cf1b9 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -1406,23 +1406,32 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
> v = br_vlan_find(vg, vid);
> if (!v)
> return -ENOENT;
> - br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
> __vlan_unpublish(vg, v);
> + synchronize_net();
Sashiko says:
Does this synchronize_net() call introduce an RTNL stall and IPI storm when
processing a large VLAN range?
When a user issues an RTM_DELLINK command specifying a large range of VLANs
(e.g., vid 1-4094), br_process_vlan_info() iterates over the range sequentially:
net/bridge/br_netlink.c:br_process_vlan_info() {
...
for (v = (*vinfo_last)->vid; v <= vinfo_curr->vid; v++) {
bool curr_change = false;
tmp_vinfo.vid = v;
err = br_vlan_info(br, p, cmd, &tmp_vinfo, &curr_change,
extack);
...
}
This loop calls into br_vlan_info(), which then calls nbp_vlan_delete() for
each VLAN. Because the global RTNL lock is held during this process,
synchronize_net() translates to synchronize_rcu_expedited().
Calling this iteratively up to 4094 times blocks the RTNL lock for several
seconds and spams all CPUs with IPIs, causing a severe system-wide network
configuration Denial of Service. This is reachable by any user with
CAP_NET_ADMIN in the network namespace, which can often be obtained by
unprivileged local users via user namespaces.
Could these grace periods be batched or deferred when deleting multiple VLANs?
- Yes, they can be batched and I had them batched initially but when I ran my tests I didn't
notice any problems deleting a full VLAN range. It took 79ms on my host to delete 4k VLANs.
As I've stated in my cover letter - the complexity is not worth it, so I dropped that patch.
> + /* Traffic may still use v through cached fdb dsts until they are
> + * cleaned below. This is acceptable during vlan deletion. Above we
> + * drain the readers that could republish the dst before cleaning it
> + */
> + br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
> __vlan_del(vg, v);
>
> return 0;
> }
>
> -void nbp_vlan_flush(struct net_bridge_port *port)
> +void nbp_vlan_group_unpublish(struct net_bridge_port *port)
> {
> - struct net_bridge_vlan_group *vg;
> -
> ASSERT_RTNL();
>
> - vg = nbp_vlan_group(port);
> - __vlan_flush(port->br, port, vg);
> RCU_INIT_POINTER(port->vlgrp, NULL);
> synchronize_net();
> +}
> +
> +void nbp_vlan_flush(struct net_bridge_port *port,
> + struct net_bridge_vlan_group *vg)
> +{
> + ASSERT_RTNL();
> +
> + __vlan_flush(port->br, port, vg);
> __vlan_group_free(vg);
> }
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-09-30 7:14 ` [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs Nikolay Aleksandrov
2026-10-01 7:28 ` Nikolay Aleksandrov
@ 2026-10-01 11:59 ` netdev-bot+sashiko
2026-10-01 12:07 ` Nikolay Aleksandrov
1 sibling, 1 reply; 28+ messages in thread
From: netdev-bot+sashiko @ 2026-10-01 11:59 UTC (permalink / raw)
To: razor; +Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] nbp_vlan_delete() in net/bridge/br_vlan.c now calls
synchronize_net() for every port-VLAN it deletes.
- [Low] br_fdb_cleanup_by_dst() in net/bridge/br_fdb.c reads f->dst once
into `dst`.
--
net: bridge: vlan: quiesce readers before freeing port VLANs
This patch prepares for fdb entries caching port-VLAN pointers. A port
VLAN is now unpublished, a grace period passes, and only then are the fdb
references purged or rewritten and the VLAN freed. During port teardown
the whole VLAN group is unpublished first, so all VLANs share one grace
period.
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 7c68b540b3580..307f9c12914ef 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -883,7 +883,9 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
>
> spin_lock_bh(&br->hash_lock);
> hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
> - if (br_fdb_dst_port(f) != p)
> + struct net_bridge_dst dst = br_fdb_dst_read(f);
> +
> + if (br_dst_port(dst) != p)
> continue;
>
[ ... ]
> + !test_bit(BR_FDB_OFFLOADED, &f->flags))) {
> + /* The entry outlives the VLAN, so it must fall
> + * back to the raw port destination
> + */
> + if (vlan && br_dst_vlan(dst) == vlan)
> + br_fdb_dst_write(f,
> + br_port_to_dst(p));
[Severity: Low]
Can this unconditional write lose a concurrent roam?
dst is read once at the top of the loop. br_fdb_update() roams entries by
writing fdb->dst under RCU only, without br->hash_lock:
br_fdb_update()
br_fdb_dst_write(fdb, br_port_to_dst(source));
If a roam lands between br_fdb_dst_read() and this br_fdb_dst_write(), the
new port would be overwritten with p.
This branch can't be reached at this commit. Every br_fdb_dst_write()
caller stores br_port_to_dst(), so br_dst_vlan(dst) is always NULL here.
Later in the series, "net: bridge: fdb: cache port VLANs in learned
entries" replaces this with br_fdb_dst_replace(f, dst, br_port_to_dst(p)).
That is a cmpxchg against the snapshot, so a concurrent roam is kept.
Would it make sense to use the conditional replace here from the start?
> continue;
> + }
> + }
[ ... ]
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 8a914e249767a..0675f74cf1b99 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -1406,23 +1406,32 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
> v = br_vlan_find(vg, vid);
> if (!v)
> return -ENOENT;
> - br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
> __vlan_unpublish(vg, v);
> + synchronize_net();
[Severity: Medium]
Does this add one expedited grace period for each port VLAN deleted, all
while RTNL is held?
Every caller of nbp_vlan_delete() holds RTNL, so synchronize_net() always
goes through synchronize_rcu_expedited(). Range requests are processed one
VID at a time under a single RTNL hold:
br_process_vlan_info()
for (v = (*vinfo_last)->vid; v <= vinfo_curr->vid; v++) {
br_vlan_info()
nbp_vlan_delete(p, vinfo->vid)
synchronize_net()
So a request like "bridge vlan del dev swp1 vid 1-4094" could run up to
4094 expedited grace periods back to back with RTNL held. The
cond_resched() in that loop doesn't drop RTNL.
The per-port loops in br_vlan_disable_default_pvid() and
__br_vlan_set_default_pvid() also now take one grace period per bridge
port. That includes the error unwind loop in __br_vlan_set_default_pvid().
Before this patch, nbp_vlan_delete() waited for no grace period at all,
because __vlan_del() only queues call_rcu().
The commit message only describes batching for port teardown:
"This lets all VLANs share one grace period."
Could range deletion be batched the same way? For example, unpublish the
whole range, wait once, then clean the fdbs and free the VLANs.
At this commit, no fdb writer stores a VLAN dst yet. That starts with "net:
bridge: fdb: cache port VLANs in learned entries". The per-VLAN
synchronize_net() is still unchanged at the end of the series, in "net:
bridge: fdb: avoid VLAN lookups in unicast forwarding".
> + /* Traffic may still use v through cached fdb dsts until they are
> + * cleaned below. This is acceptable during vlan deletion. Above we
> + * drain the readers that could republish the dst before cleaning it
> + */
> + br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
> __vlan_del(vg, v);
>
> return 0;
> }
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071411.2786201-1-razor%40blackwall.org
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-10-01 11:59 ` netdev-bot+sashiko
@ 2026-10-01 12:07 ` Nikolay Aleksandrov
2026-10-02 19:50 ` Jakub Kicinski
0 siblings, 1 reply; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 12:07 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 01/10/2026 14:59, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] nbp_vlan_delete() in net/bridge/br_vlan.c now calls
> synchronize_net() for every port-VLAN it deletes.
> - [Low] br_fdb_cleanup_by_dst() in net/bridge/br_fdb.c reads f->dst once
> into `dst`.
>
> --
>
I already answered, but here it is again:
> net: bridge: vlan: quiesce readers before freeing port VLANs
>
> This patch prepares for fdb entries caching port-VLAN pointers. A port
> VLAN is now unpublished, a grace period passes, and only then are the fdb
> references purged or rewritten and the VLAN freed. During port teardown
> the whole VLAN group is unpublished first, so all VLANs share one grace
> period.
>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 7c68b540b3580..307f9c12914ef 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -883,7 +883,9 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
>>
>> spin_lock_bh(&br->hash_lock);
>> hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
>> - if (br_fdb_dst_port(f) != p)
>> + struct net_bridge_dst dst = br_fdb_dst_read(f);
>> +
>> + if (br_dst_port(dst) != p)
>> continue;
>>
>
> [ ... ]
>
>> + !test_bit(BR_FDB_OFFLOADED, &f->flags))) {
>> + /* The entry outlives the VLAN, so it must fall
>> + * back to the raw port destination
>> + */
>> + if (vlan && br_dst_vlan(dst) == vlan)
>> + br_fdb_dst_write(f,
>> + br_port_to_dst(p));
>
> [Severity: Low]
> Can this unconditional write lose a concurrent roam?
>
> dst is read once at the top of the loop. br_fdb_update() roams entries by
> writing fdb->dst under RCU only, without br->hash_lock:
>
> br_fdb_update()
> br_fdb_dst_write(fdb, br_port_to_dst(source));
>
> If a roam lands between br_fdb_dst_read() and this br_fdb_dst_write(), the
> new port would be overwritten with p.
>
> This branch can't be reached at this commit. Every br_fdb_dst_write()
> caller stores br_port_to_dst(), so br_dst_vlan(dst) is always NULL here.
>
> Later in the series, "net: bridge: fdb: cache port VLANs in learned
> entries" replaces this with br_fdb_dst_replace(f, dst, br_port_to_dst(p)).
> That is a cmpxchg against the snapshot, so a concurrent roam is kept.
>
> Would it make sense to use the conditional replace here from the start?
>
- No, we're fine with the roam race, it is normal. Trying to change these races
requires much more work and isn't worth it at the moment.
>> continue;
>> + }
>> + }
>
> [ ... ]
>
>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>> index 8a914e249767a..0675f74cf1b99 100644
>> --- a/net/bridge/br_vlan.c
>> +++ b/net/bridge/br_vlan.c
>> @@ -1406,23 +1406,32 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid)
>> v = br_vlan_find(vg, vid);
>> if (!v)
>> return -ENOENT;
>> - br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
>> __vlan_unpublish(vg, v);
>> + synchronize_net();
>
> [Severity: Medium]
> Does this add one expedited grace period for each port VLAN deleted, all
> while RTNL is held?
>
> Every caller of nbp_vlan_delete() holds RTNL, so synchronize_net() always
> goes through synchronize_rcu_expedited(). Range requests are processed one
> VID at a time under a single RTNL hold:
>
> br_process_vlan_info()
> for (v = (*vinfo_last)->vid; v <= vinfo_curr->vid; v++) {
> br_vlan_info()
> nbp_vlan_delete(p, vinfo->vid)
> synchronize_net()
>
> So a request like "bridge vlan del dev swp1 vid 1-4094" could run up to
> 4094 expedited grace periods back to back with RTNL held. The
> cond_resched() in that loop doesn't drop RTNL.
>
> The per-port loops in br_vlan_disable_default_pvid() and
> __br_vlan_set_default_pvid() also now take one grace period per bridge
> port. That includes the error unwind loop in __br_vlan_set_default_pvid().
>
> Before this patch, nbp_vlan_delete() waited for no grace period at all,
> because __vlan_del() only queues call_rcu().
>
> The commit message only describes batching for port teardown:
>
> "This lets all VLANs share one grace period."
>
> Could range deletion be batched the same way? For example, unpublish the
> whole range, wait once, then clean the fdbs and free the VLANs.
>
> At this commit, no fdb writer stores a VLAN dst yet. That starts with "net:
> bridge: fdb: cache port VLANs in learned entries". The per-VLAN
> synchronize_net() is still unchanged at the end of the series, in "net:
> bridge: fdb: avoid VLAN lookups in unicast forwarding".
>
- Yes, they can be batched and I had them batched initially but when I ran my tests I didn't
notice any problems deleting a full VLAN range. It took 79ms on my host to delete 4k VLANs.
As I've stated in my cover letter - the complexity is not worth it, so I dropped that patch.
(from my previous reply)
>> + /* Traffic may still use v through cached fdb dsts until they are
>> + * cleaned below. This is acceptable during vlan deletion. Above we
>> + * drain the readers that could republish the dst before cleaning it
>> + */
>> + br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0);
>> __vlan_del(vg, v);
>>
>> return 0;
>> }
>
> [ ... ]
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-10-01 12:07 ` Nikolay Aleksandrov
@ 2026-10-02 19:50 ` Jakub Kicinski
2026-10-02 19:59 ` Nikolay Aleksandrov
0 siblings, 1 reply; 28+ messages in thread
From: Jakub Kicinski @ 2026-10-02 19:50 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev-bot+sashiko, netdev, idosch, davem, edumazet, pabeni,
horms, bridge
On Thu, 1 Oct 2026 15:07:21 +0300 Nikolay Aleksandrov wrote:
> - Yes, they can be batched and I had them batched initially but when
> I ran my tests I didn't notice any problems deleting a full VLAN
> range. It took 79ms on my host to delete 4k VLANs. As I've stated in
> my cover letter - the complexity is not worth it, so I dropped that
> patch.
For the 79ms number - how big was the system and was it busy?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
2026-10-02 19:50 ` Jakub Kicinski
@ 2026-10-02 19:59 ` Nikolay Aleksandrov
0 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-02 19:59 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev-bot+sashiko, netdev, idosch, davem, edumazet, pabeni,
horms, bridge
On 02/10/2026 22:50, Jakub Kicinski wrote:
> On Thu, 1 Oct 2026 15:07:21 +0300 Nikolay Aleksandrov wrote:
>> - Yes, they can be batched and I had them batched initially but when
>> I ran my tests I didn't notice any problems deleting a full VLAN
>> range. It took 79ms on my host to delete 4k VLANs. As I've stated in
>> my cover letter - the complexity is not worth it, so I dropped that
>> patch.
>
> For the 79ms number - how big was the system and was it busy?
It was a VM with 3 vCPUs, not load. The baseline (without patches) was ~25% faster.
This is still a slow path and deleting 4k VLANs is not a common thing. :)
Note that flushing all VLANs usually happens when a port is being deleted and
that is still fast as before (it doesn't wait a gp for every VLAN).
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net-next 09/12] net: bridge: fdb: factor out existing entry updates
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (7 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 10/12] net: bridge: fdb: cache port VLANs in learned entries Nikolay Aleksandrov
` (3 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Move the existing fdb entry update path out of br_fdb_update() into
__fdb_update(). This reduces nesting in br_fdb_update() and keeps the
lookup and creation flow separate from updates to an entry that was found.
No functional changes intended.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_fdb.c | 96 +++++++++++++++++++++++----------------------
1 file changed, 50 insertions(+), 46 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 307f9c12914e..b87c6f8875da 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -994,6 +994,55 @@ static bool __fdb_mark_active(struct net_bridge_fdb_entry *fdb)
test_and_clear_bit(BR_FDB_NOTIFY_INACTIVE, &fdb->flags));
}
+static void __fdb_update(struct net_bridge *br,
+ struct net_bridge_fdb_entry *fdb,
+ struct net_bridge_port *source,
+ const unsigned char *addr, u16 vid,
+ unsigned long flags)
+{
+ bool fdb_modified = false;
+ unsigned long now;
+
+ /* attempt to update an entry for a local interface */
+ if (unlikely(test_bit(BR_FDB_LOCAL, &fdb->flags))) {
+ if (net_ratelimit())
+ br_warn(br, "received packet on %s with own address as source address (addr:%pM, vlan:%u)\n",
+ source->dev->name, addr, vid);
+ return;
+ }
+
+ now = jiffies;
+ if (now != READ_ONCE(fdb->updated)) {
+ WRITE_ONCE(fdb->updated, now);
+ fdb_modified = __fdb_mark_active(fdb);
+ }
+
+ /* fastpath: update of existing entry */
+ if (unlikely(source != br_fdb_dst_port(fdb) &&
+ !test_bit(BR_FDB_STICKY, &fdb->flags))) {
+ br_switchdev_fdb_notify(br, fdb, RTM_DELNEIGH);
+ br_fdb_dst_write(fdb, br_port_to_dst(source));
+ fdb_modified = true;
+ /* Take over HW learned entry */
+ if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags)))
+ clear_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags);
+ /* Clear locked flag when roaming to an unlocked port */
+ if (unlikely(test_bit(BR_FDB_LOCKED, &fdb->flags)))
+ clear_bit(BR_FDB_LOCKED, &fdb->flags);
+ }
+
+ if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) {
+ set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
+ if (test_and_clear_bit(BR_FDB_DYNAMIC_LEARNED, &fdb->flags))
+ atomic_dec(&br->fdb_n_learned);
+ }
+
+ if (unlikely(fdb_modified)) {
+ trace_br_fdb_update(br, source, addr, vid, flags);
+ fdb_notify(br, fdb, RTM_NEWNEIGH, true);
+ }
+}
+
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
struct net_bridge_vlan *vlan, const unsigned char *addr,
unsigned long flags)
@@ -1007,49 +1056,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
fdb = fdb_find_rcu(&br->fdb_hash_tbl, addr, vid);
if (likely(fdb)) {
- /* attempt to update an entry for a local interface */
- if (unlikely(test_bit(BR_FDB_LOCAL, &fdb->flags))) {
- if (net_ratelimit())
- br_warn(br, "received packet on %s with own address as source address (addr:%pM, vlan:%u)\n",
- source->dev->name, addr, vid);
- } else {
- unsigned long now = jiffies;
- bool fdb_modified = false;
-
- if (now != READ_ONCE(fdb->updated)) {
- WRITE_ONCE(fdb->updated, now);
- fdb_modified = __fdb_mark_active(fdb);
- }
-
- /* fastpath: update of existing entry */
- if (unlikely(source != br_fdb_dst_port(fdb) &&
- !test_bit(BR_FDB_STICKY, &fdb->flags))) {
- br_switchdev_fdb_notify(br, fdb, RTM_DELNEIGH);
- br_fdb_dst_write(fdb, br_port_to_dst(source));
- fdb_modified = true;
- /* Take over HW learned entry */
- if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN,
- &fdb->flags)))
- clear_bit(BR_FDB_ADDED_BY_EXT_LEARN,
- &fdb->flags);
- /* Clear locked flag when roaming to an
- * unlocked port.
- */
- if (unlikely(test_bit(BR_FDB_LOCKED, &fdb->flags)))
- clear_bit(BR_FDB_LOCKED, &fdb->flags);
- }
-
- if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) {
- set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
- if (test_and_clear_bit(BR_FDB_DYNAMIC_LEARNED,
- &fdb->flags))
- atomic_dec(&br->fdb_n_learned);
- }
- if (unlikely(fdb_modified)) {
- trace_br_fdb_update(br, source, addr, vid, flags);
- fdb_notify(br, fdb, RTM_NEWNEIGH, true);
- }
- }
+ __fdb_update(br, fdb, source, addr, vid, flags);
} else {
spin_lock(&br->hash_lock);
fdb = fdb_create(br, source, addr, vid, flags);
@@ -1057,9 +1064,6 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
trace_br_fdb_update(br, source, addr, vid, flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, true);
}
- /* else we lose race and someone else inserts
- * it first, don't bother updating
- */
spin_unlock(&br->hash_lock);
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 10/12] net: bridge: fdb: cache port VLANs in learned entries
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (8 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 09/12] net: bridge: fdb: factor out existing entry updates Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 11/12] net: bridge: fdb: cache VLAN destinations in configured entries Nikolay Aleksandrov
` (2 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Store the resolved port-VLAN as the destination of dynamically learned FDB
entries. Roaming updates the destination directly, while an existing raw
port destination is conditionally upgraded without generating a spurious
notification. Conditional replacements prevent both this upgrade and a VLAN
deletion fallback from overwriting a concurrent roam.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_fdb.c | 57 ++++++++++++++++++++++++++---------------
net/bridge/br_private.h | 18 +++++++++++++
2 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index b87c6f8875da..5664f3d649fa 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -385,7 +385,7 @@ void br_fdb_find_delete_local(struct net_bridge *br,
}
static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br,
- struct net_bridge_port *source,
+ struct net_bridge_dst dst,
const unsigned char *addr,
__u16 vid,
unsigned long flags)
@@ -409,7 +409,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br,
return NULL;
memcpy(fdb->key.addr.addr, addr, ETH_ALEN);
- br_fdb_dst_write(fdb, br_port_to_dst(source));
+ br_fdb_dst_write(fdb, dst);
fdb->key.vlan_id = vid;
fdb->flags = flags;
fdb->updated = fdb->used = jiffies;
@@ -448,7 +448,7 @@ static int fdb_add_local(struct net_bridge *br, struct net_bridge_port *source,
fdb_delete(br, fdb, true);
}
- fdb = fdb_create(br, source, addr, vid,
+ fdb = fdb_create(br, br_port_to_dst(source), addr, vid,
BIT(BR_FDB_LOCAL) | BIT(BR_FDB_STATIC));
if (!fdb)
return -ENOMEM;
@@ -907,8 +907,8 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br,
* back to the raw port destination
*/
if (vlan && br_dst_vlan(dst) == vlan)
- br_fdb_dst_write(f,
- br_port_to_dst(p));
+ br_fdb_dst_replace(f, dst,
+ br_port_to_dst(p));
continue;
}
}
@@ -997,9 +997,12 @@ static bool __fdb_mark_active(struct net_bridge_fdb_entry *fdb)
static void __fdb_update(struct net_bridge *br,
struct net_bridge_fdb_entry *fdb,
struct net_bridge_port *source,
+ struct net_bridge_dst dst,
const unsigned char *addr, u16 vid,
unsigned long flags)
{
+ struct net_bridge_port *old_port;
+ struct net_bridge_dst old_dst;
bool fdb_modified = false;
unsigned long now;
@@ -1017,18 +1020,30 @@ static void __fdb_update(struct net_bridge *br,
fdb_modified = __fdb_mark_active(fdb);
}
+ old_dst = br_fdb_dst_read(fdb);
+ old_port = br_dst_port(old_dst);
/* fastpath: update of existing entry */
- if (unlikely(source != br_fdb_dst_port(fdb) &&
- !test_bit(BR_FDB_STICKY, &fdb->flags))) {
- br_switchdev_fdb_notify(br, fdb, RTM_DELNEIGH);
- br_fdb_dst_write(fdb, br_port_to_dst(source));
- fdb_modified = true;
- /* Take over HW learned entry */
- if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags)))
- clear_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags);
- /* Clear locked flag when roaming to an unlocked port */
- if (unlikely(test_bit(BR_FDB_LOCKED, &fdb->flags)))
- clear_bit(BR_FDB_LOCKED, &fdb->flags);
+ if (unlikely(!br_dst_equal(dst, old_dst) &&
+ (source == old_port ||
+ !test_bit(BR_FDB_STICKY, &fdb->flags)))) {
+ if (source != old_port) {
+ br_switchdev_fdb_notify(br, fdb, RTM_DELNEIGH);
+ br_fdb_dst_write(fdb, dst);
+ fdb_modified = true;
+ /* take over HW learned entry */
+ if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN,
+ &fdb->flags)))
+ clear_bit(BR_FDB_ADDED_BY_EXT_LEARN,
+ &fdb->flags);
+ /* clear locked flag when roaming to an unlocked port */
+ if (unlikely(test_bit(BR_FDB_LOCKED, &fdb->flags)))
+ clear_bit(BR_FDB_LOCKED, &fdb->flags);
+ } else {
+ /* raw port-to-VLAN promotion shouldn't overwrite a
+ * concurrent roam to another port
+ */
+ br_fdb_dst_replace(fdb, old_dst, dst);
+ }
}
if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) {
@@ -1047,6 +1062,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
struct net_bridge_vlan *vlan, const unsigned char *addr,
unsigned long flags)
{
+ struct net_bridge_dst dst = vlan ? br_vlan_to_dst(vlan) :
+ br_port_to_dst(source);
struct net_bridge_fdb_entry *fdb;
u16 vid = vlan ? vlan->vid : 0;
@@ -1056,10 +1073,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
fdb = fdb_find_rcu(&br->fdb_hash_tbl, addr, vid);
if (likely(fdb)) {
- __fdb_update(br, fdb, source, addr, vid, flags);
+ __fdb_update(br, fdb, source, dst, addr, vid, flags);
} else {
spin_lock(&br->hash_lock);
- fdb = fdb_create(br, source, addr, vid, flags);
+ fdb = fdb_create(br, dst, addr, vid, flags);
if (fdb) {
trace_br_fdb_update(br, source, addr, vid, flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, true);
@@ -1213,7 +1230,7 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
if (!(flags & NLM_F_CREATE))
return -ENOENT;
- fdb = fdb_create(br, source, addr, vid,
+ fdb = fdb_create(br, br_port_to_dst(source), addr, vid,
BIT(BR_FDB_ADDED_BY_USER));
if (!fdb)
return -ENOMEM;
@@ -1564,7 +1581,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
if (locked)
flags |= BIT(BR_FDB_LOCKED);
- fdb = fdb_create(br, p, addr, vid, flags);
+ fdb = fdb_create(br, br_port_to_dst(p), addr, vid, flags);
if (!fdb) {
err = -ENOMEM;
goto err_unlock;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 951b6ac5f484..a1c5b1abc1ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -694,6 +694,13 @@ static inline void br_dst_write(struct net_bridge_dst *dst,
ACCESS_PRIVATE(&src, value));
}
+static inline bool br_dst_equal(struct net_bridge_dst dst1,
+ struct net_bridge_dst dst2)
+{
+ return ACCESS_PRIVATE(&dst1, value) ==
+ ACCESS_PRIVATE(&dst2, value);
+}
+
static inline struct net_bridge_dst
br_port_to_dst(const struct net_bridge_port *p)
{
@@ -768,6 +775,17 @@ static inline void br_fdb_dst_write(struct net_bridge_fdb_entry *fdb,
br_dst_write(&fdb->dst, dst);
}
+static inline bool
+br_fdb_dst_replace(struct net_bridge_fdb_entry *fdb,
+ struct net_bridge_dst old,
+ struct net_bridge_dst new)
+{
+ unsigned long old_value = ACCESS_PRIVATE(&old, value);
+
+ return cmpxchg(&ACCESS_PRIVATE(&fdb->dst, value), old_value,
+ ACCESS_PRIVATE(&new, value)) == old_value;
+}
+
static inline struct net_bridge_port *
br_fdb_dst_port(const struct net_bridge_fdb_entry *fdb)
{
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH net-next 11/12] net: bridge: fdb: cache VLAN destinations in configured entries
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (9 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 10/12] net: bridge: fdb: cache port VLANs in learned entries Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-01 7:36 ` Nikolay Aleksandrov
2026-09-30 7:14 ` [PATCH net-next 12/12] net: bridge: fdb: avoid VLAN lookups in unicast forwarding Nikolay Aleksandrov
2026-10-02 20:00 ` [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path patchwork-bot+netdevbpf
12 siblings, 1 reply; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov
Use the known port-VLAN destination for user-configured fdb entries and
resolve switchdev vids with an fdb helper. VLAN 0, bridge entries and
unconfigured switchdev vids retain raw port destinations. Keep the vid
alongside the destination because it remains part of the fdb key.
Use conditional replacement for same port destination upgrades so they
don't overwrite a concurrent packet-learned roam. Port changing updates
remain direct and report a forwarding change.
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
net/bridge/br_fdb.c | 61 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 5664f3d649fa..730b178cf6c4 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1192,10 +1192,11 @@ static bool fdb_handle_notify(struct net_bridge_fdb_entry *fdb, u8 notify)
}
/* Update (create or replace) forwarding database entry */
-static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
+static int fdb_add_entry(struct net_bridge *br, struct net_bridge_dst dst,
const u8 *addr, struct ndmsg *ndm, u16 flags, u16 vid,
struct nlattr *nfea_tb[])
{
+ struct net_bridge_port *source = br_dst_port(dst);
bool is_sticky = !!(ndm->ndm_flags & NTF_STICKY);
bool refresh = !nfea_tb[NFEA_DONT_REFRESH];
struct net_bridge_fdb_entry *fdb;
@@ -1230,19 +1231,26 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
if (!(flags & NLM_F_CREATE))
return -ENOENT;
- fdb = fdb_create(br, br_port_to_dst(source), addr, vid,
+ fdb = fdb_create(br, dst, addr, vid,
BIT(BR_FDB_ADDED_BY_USER));
if (!fdb)
return -ENOMEM;
modified = true;
} else {
+ struct net_bridge_dst old_dst;
+
if (flags & NLM_F_EXCL)
return -EEXIST;
- if (br_fdb_dst_port(fdb) != source) {
- br_fdb_dst_write(fdb, br_port_to_dst(source));
- modified = true;
+ old_dst = br_fdb_dst_read(fdb);
+ if (!br_dst_equal(old_dst, dst)) {
+ if (br_dst_port(old_dst) != source) {
+ modified = true;
+ br_fdb_dst_write(fdb, dst);
+ } else {
+ br_fdb_dst_replace(fdb, old_dst, dst);
+ }
}
set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
@@ -1296,6 +1304,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
bool *notified, struct netlink_ext_ack *extack)
{
u16 vid = vlan ? vlan->vid : 0;
+ struct net_bridge_dst dst;
int err = 0;
if (ndm->ndm_flags & NTF_USE) {
@@ -1320,8 +1329,10 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
}
err = br_fdb_external_learn_add(br, p, addr, vid, false, true);
} else {
+ dst = p && vlan ? br_vlan_to_dst(vlan) : br_port_to_dst(p);
spin_lock_bh(&br->hash_lock);
- err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
+ err = fdb_add_entry(br, dst, addr, ndm, nlh_flags, vid,
+ nfea_tb);
spin_unlock_bh(&br->hash_lock);
}
@@ -1553,10 +1564,31 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
rcu_read_unlock();
}
+static struct net_bridge_dst br_fdb_resolve_dst(const struct net_bridge_port *p,
+ u16 vid)
+{
+ struct net_bridge_dst dst = br_port_to_dst(p);
+ struct net_bridge_vlan *v;
+
+ lockdep_assert(rcu_read_lock_held() || lockdep_rtnl_is_held());
+
+ if (!p || !vid)
+ return dst;
+
+ rcu_read_lock();
+ v = br_vlan_find(nbp_vlan_group_rcu(p), vid);
+ if (v)
+ dst = br_vlan_to_dst(v);
+ rcu_read_unlock();
+
+ return dst;
+}
+
int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid, bool locked,
bool swdev_notify)
{
+ struct net_bridge_dst dst = br_fdb_resolve_dst(p, vid);
struct net_bridge_fdb_entry *fdb;
bool modified = false;
int err = 0;
@@ -1581,25 +1613,32 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
if (locked)
flags |= BIT(BR_FDB_LOCKED);
- fdb = fdb_create(br, br_port_to_dst(p), addr, vid, flags);
+ fdb = fdb_create(br, dst, addr, vid, flags);
if (!fdb) {
err = -ENOMEM;
goto err_unlock;
}
fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
} else {
+ struct net_bridge_dst old_dst;
+
+ old_dst = br_fdb_dst_read(fdb);
if (locked &&
(!test_bit(BR_FDB_LOCKED, &fdb->flags) ||
- br_fdb_dst_port(fdb) != p)) {
+ br_dst_port(old_dst) != p)) {
err = -EINVAL;
goto err_unlock;
}
WRITE_ONCE(fdb->updated, jiffies);
- if (br_fdb_dst_port(fdb) != p) {
- br_fdb_dst_write(fdb, br_port_to_dst(p));
- modified = true;
+ if (!br_dst_equal(old_dst, dst)) {
+ if (br_dst_port(old_dst) != p) {
+ modified = true;
+ br_fdb_dst_write(fdb, dst);
+ } else {
+ br_fdb_dst_replace(fdb, old_dst, dst);
+ }
}
if (test_and_set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags)) {
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 11/12] net: bridge: fdb: cache VLAN destinations in configured entries
2026-09-30 7:14 ` [PATCH net-next 11/12] net: bridge: fdb: cache VLAN destinations in configured entries Nikolay Aleksandrov
@ 2026-10-01 7:36 ` Nikolay Aleksandrov
0 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-10-01 7:36 UTC (permalink / raw)
To: netdev; +Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge
On 30/09/2026 10:14, Nikolay Aleksandrov wrote:
> Use the known port-VLAN destination for user-configured fdb entries and
> resolve switchdev vids with an fdb helper. VLAN 0, bridge entries and
> unconfigured switchdev vids retain raw port destinations. Keep the vid
> alongside the destination because it remains part of the fdb key.
> Use conditional replacement for same port destination upgrades so they
> don't overwrite a concurrent packet-learned roam. Port changing updates
> remain direct and report a forwarding change.
>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
> net/bridge/br_fdb.c | 61 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 5664f3d649fa..730b178cf6c4 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -1192,10 +1192,11 @@ static bool fdb_handle_notify(struct net_bridge_fdb_entry *fdb, u8 notify)
> }
>
> /* Update (create or replace) forwarding database entry */
> -static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> +static int fdb_add_entry(struct net_bridge *br, struct net_bridge_dst dst,
> const u8 *addr, struct ndmsg *ndm, u16 flags, u16 vid,
> struct nlattr *nfea_tb[])
> {
> + struct net_bridge_port *source = br_dst_port(dst);
> bool is_sticky = !!(ndm->ndm_flags & NTF_STICKY);
> bool refresh = !nfea_tb[NFEA_DONT_REFRESH];
> struct net_bridge_fdb_entry *fdb;
> @@ -1230,19 +1231,26 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> if (!(flags & NLM_F_CREATE))
> return -ENOENT;
>
> - fdb = fdb_create(br, br_port_to_dst(source), addr, vid,
> + fdb = fdb_create(br, dst, addr, vid,
> BIT(BR_FDB_ADDED_BY_USER));
> if (!fdb)
> return -ENOMEM;
>
> modified = true;
> } else {
> + struct net_bridge_dst old_dst;
> +
> if (flags & NLM_F_EXCL)
> return -EEXIST;
>
> - if (br_fdb_dst_port(fdb) != source) {
> - br_fdb_dst_write(fdb, br_port_to_dst(source));
> - modified = true;
> + old_dst = br_fdb_dst_read(fdb);
> + if (!br_dst_equal(old_dst, dst)) {
> + if (br_dst_port(old_dst) != source) {
> + modified = true;
> + br_fdb_dst_write(fdb, dst);
> + } else {
> + br_fdb_dst_replace(fdb, old_dst, dst);
> + }
> }
>
> set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
Sashiko says:
If a concurrent packet learning roam occurs via __fdb_update() updating the
destination locklessly, could br_fdb_dst_replace() fail here?
If it fails, is it safe to ignore the failure and proceed to set
BR_FDB_ADDED_BY_USER and potentially other authoritative flags like
BR_FDB_STATIC below? It appears this might apply the flags to the concurrently
roamed destination rather than the requested one, potentially corrupting
hardware offload state and locking traffic to the wrong port.
- Yes, it is safe and intentional behaviour, it must fail if a roam to a different port
happened. The flags races are well-known and pre-existing behaviour.
> @@ -1296,6 +1304,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
> bool *notified, struct netlink_ext_ack *extack)
> {
> u16 vid = vlan ? vlan->vid : 0;
> + struct net_bridge_dst dst;
> int err = 0;
>
> if (ndm->ndm_flags & NTF_USE) {
> @@ -1320,8 +1329,10 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
> }
> err = br_fdb_external_learn_add(br, p, addr, vid, false, true);
> } else {
> + dst = p && vlan ? br_vlan_to_dst(vlan) : br_port_to_dst(p);
> spin_lock_bh(&br->hash_lock);
> - err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
> + err = fdb_add_entry(br, dst, addr, ndm, nlh_flags, vid,
> + nfea_tb);
> spin_unlock_bh(&br->hash_lock);
> }
>
> @@ -1553,10 +1564,31 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
> rcu_read_unlock();
> }
>
> +static struct net_bridge_dst br_fdb_resolve_dst(const struct net_bridge_port *p,
> + u16 vid)
> +{
> + struct net_bridge_dst dst = br_port_to_dst(p);
> + struct net_bridge_vlan *v;
> +
> + lockdep_assert(rcu_read_lock_held() || lockdep_rtnl_is_held());
> +
> + if (!p || !vid)
> + return dst;
> +
> + rcu_read_lock();
> + v = br_vlan_find(nbp_vlan_group_rcu(p), vid);
> + if (v)
> + dst = br_vlan_to_dst(v);
> + rcu_read_unlock();
> +
> + return dst;
> +}
> +
> int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid, bool locked,
> bool swdev_notify)
> {
> + struct net_bridge_dst dst = br_fdb_resolve_dst(p, vid);
> struct net_bridge_fdb_entry *fdb;
> bool modified = false;
> int err = 0;
> @@ -1581,25 +1613,32 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> if (locked)
> flags |= BIT(BR_FDB_LOCKED);
>
> - fdb = fdb_create(br, br_port_to_dst(p), addr, vid, flags);
> + fdb = fdb_create(br, dst, addr, vid, flags);
> if (!fdb) {
> err = -ENOMEM;
> goto err_unlock;
> }
> fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
> } else {
> + struct net_bridge_dst old_dst;
> +
> + old_dst = br_fdb_dst_read(fdb);
> if (locked &&
> (!test_bit(BR_FDB_LOCKED, &fdb->flags) ||
> - br_fdb_dst_port(fdb) != p)) {
> + br_dst_port(old_dst) != p)) {
> err = -EINVAL;
> goto err_unlock;
> }
>
> WRITE_ONCE(fdb->updated, jiffies);
>
> - if (br_fdb_dst_port(fdb) != p) {
> - br_fdb_dst_write(fdb, br_port_to_dst(p));
> - modified = true;
> + if (!br_dst_equal(old_dst, dst)) {
> + if (br_dst_port(old_dst) != p) {
> + modified = true;
> + br_fdb_dst_write(fdb, dst);
> + } else {
> + br_fdb_dst_replace(fdb, old_dst, dst);
> + }
> }
>
> if (test_and_set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags)) {
Sashiko says:
Similarly, if br_fdb_dst_replace() fails here due to a concurrent destination
update in __fdb_update(), could hardware learning flags like
BR_FDB_ADDED_BY_EXT_LEARN be misapplied to the new roamed port?
- Yes, again pre-existing and well-known behaviour. See my reply above.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH net-next 12/12] net: bridge: fdb: avoid VLAN lookups in unicast forwarding
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (10 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 11/12] net: bridge: fdb: cache VLAN destinations in configured entries Nikolay Aleksandrov
@ 2026-09-30 7:14 ` Nikolay Aleksandrov
2026-10-02 20:00 ` [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path patchwork-bot+netdevbpf
12 siblings, 0 replies; 28+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 7:14 UTC (permalink / raw)
To: netdev
Cc: idosch, davem, edumazet, kuba, pabeni, horms, bridge,
Nikolay Aleksandrov, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, netfilter-devel, coreteam
Pass the complete fdb destination to br_forward(). The tagged value is
decoded once into br_fwd_dst on entry so all subsequent forwarding checks
use the decoded port and VLAN. A cached port-VLAN is then used to avoid two
VLAN hash lookups in the standard unicast forwarding path. A backup-port
redirect also uses a raw destination because the cached VLAN belongs to the
original port.
[1] 64 byte vlan packets, 4k randomized fdb hits with 4k fdbs, 5 million
packets passed 5 times for every case
VIDs Ports Mpps Gbps CPU1 cycles/input
before after gain before after before after reduction
64 8/2 2.375486 2.635811 11.0% 1.216 1.350 1724.6 1553.7 9.9%
64 8/4 2.376731 2.636459 10.9% 1.217 1.350 1714.8 1554.7 9.3%
64 8/8 2.376621 2.635783 10.9% 1.217 1.350 1722.6 1553.2 9.8%
64 32/2 2.375876 2.637897 11.0% 1.216 1.351 1726.7 1552.3 10.1%
64 32/16 2.376442 2.636426 10.9% 1.217 1.350 1721.3 1554.2 9.7%
64 32/32 2.376763 2.636268 10.9% 1.217 1.350 1726.8 1553.5 10.0%
64 64/2 2.375678 2.638707 11.1% 1.216 1.351 1725.0 1552.1 10.0%
64 64/32 2.376681 2.637359 11.0% 1.217 1.350 1725.1 1555.9 9.8%
64 64/64 2.376787 2.638396 11.0% 1.217 1.351 1722.8 1550.9 10.0%
1024 8/2 2.263768 2.498881 10.4% 1.159 1.279 1808.5 1637.1 9.5%
1024 8/4 2.264519 2.499075 10.4% 1.159 1.280 1807.6 1638.8 9.3%
1024 8/8 2.263551 2.499231 10.4% 1.159 1.280 1812.0 1640.2 9.5%
1024 32/2 2.263841 2.499120 10.4% 1.159 1.280 1806.8 1639.6 9.3%
1024 32/16 2.263990 2.499562 10.4% 1.159 1.280 1806.1 1639.2 9.2%
1024 32/32 2.263719 2.498905 10.4% 1.159 1.279 1807.7 1639.3 9.3%
1024 64/2 2.263635 2.501889 10.5% 1.159 1.281 1809.4 1635.6 9.6%
1024 64/32 2.264467 2.500359 10.4% 1.159 1.280 1806.5 1633.0 9.6%
1024 64/64 2.240494 2.470246 10.3% 1.147 1.265 1810.0 1637.1 9.6%
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
CC: Pablo Neira Ayuso <pablo@netfilter.org>
CC: Florian Westphal <fw@strlen.de>
CC: Phil Sutter <phil@nwl.cc>
CC: netfilter-devel@vger.kernel.org
CC: coreteam@netfilter.org
net/bridge/br_device.c | 2 +-
net/bridge/br_forward.c | 24 +++++++++++++-----------
net/bridge/br_input.c | 2 +-
net/bridge/br_mrp.c | 6 +++---
net/bridge/br_private.h | 2 +-
net/bridge/netfilter/nft_reject_bridge.c | 8 ++++----
6 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 2423d69f2498..0a1ce9e59a4d 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -108,7 +108,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
else
br_flood(br, vlan, skb, BR_PKT_MULTICAST, false, true);
} else if ((dst = br_fdb_find_rcu(br, dest, vid)) != NULL) {
- br_forward(br_fdb_dst_port(dst), skb, false, true);
+ br_forward(br_fdb_dst_read(dst), skb, false, true);
} else {
br_flood(br, vlan, skb, BR_PKT_UNICAST, false, true);
}
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index b5eece1ff9e8..82645db32efb 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -17,7 +17,7 @@
#include "br_private.h"
struct br_fwd_dst {
- const struct net_bridge_port *port;
+ struct net_bridge_port *port;
struct net_bridge_vlan *vlan;
};
@@ -149,35 +149,37 @@ static int deliver_clone(const struct br_fwd_dst *fwd,
/**
* br_forward - forward a packet to a specific port
- * @to: destination port
+ * @dst: bridge destination
* @skb: packet being forwarded
* @local_rcv: packet will be received locally after forwarding
* @local_orig: packet is locally originated
*
* Should be called with rcu_read_lock.
*/
-void br_forward(const struct net_bridge_port *to,
+void br_forward(struct net_bridge_dst dst,
struct sk_buff *skb, bool local_rcv, bool local_orig)
{
struct br_fwd_dst fwd;
- if (unlikely(!to))
+ br_dst_decode(dst, &fwd.port, &fwd.vlan);
+
+ if (unlikely(!fwd.port))
goto out;
/* redirect to backup link if the destination port is down */
- if (rcu_access_pointer(to->backup_port) &&
- (!netif_carrier_ok(to->dev) || !netif_running(to->dev))) {
+ if (rcu_access_pointer(fwd.port->backup_port) &&
+ (!netif_carrier_ok(fwd.port->dev) ||
+ !netif_running(fwd.port->dev))) {
struct net_bridge_port *backup_port;
- backup_port = rcu_dereference(to->backup_port);
+ backup_port = rcu_dereference(fwd.port->backup_port);
if (unlikely(!backup_port))
goto out;
- BR_INPUT_SKB_CB(skb)->backup_nhid = READ_ONCE(to->backup_nhid);
- to = backup_port;
+ BR_INPUT_SKB_CB(skb)->backup_nhid = READ_ONCE(fwd.port->backup_nhid);
+ fwd.port = backup_port;
+ fwd.vlan = NULL;
}
- fwd.port = to;
- fwd.vlan = NULL;
if (should_deliver(&fwd, skb)) {
if (local_rcv)
deliver_clone(&fwd, skb, local_orig);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 44a217d65e7f..fc2e381c4fe0 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -225,7 +225,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
if (now != READ_ONCE(dst->used))
WRITE_ONCE(dst->used, now);
- br_forward(br_fdb_dst_port(dst), skb, local_rcv, false);
+ br_forward(br_fdb_dst_read(dst), skb, local_rcv, false);
} else {
if (!mcast_hit)
br_flood(br, vlan, skb, pkt_type, local_rcv, false);
diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index dce6efa96c4c..c8cd5b4f3af8 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -1219,11 +1219,11 @@ static int br_mrp_rcv(struct net_bridge_port *p,
forward:
if (p_dst)
- br_forward(p_dst, skb, true, false);
+ br_forward(br_port_to_dst(p_dst), skb, true, false);
if (s_dst)
- br_forward(s_dst, skb, true, false);
+ br_forward(br_port_to_dst(s_dst), skb, true, false);
if (i_dst)
- br_forward(i_dst, skb, true, false);
+ br_forward(br_port_to_dst(i_dst), skb, true, false);
no_forward:
return 1;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a1c5b1abc1ef..05921192ebda 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1045,7 +1045,7 @@ enum br_pkt_type {
BR_PKT_BROADCAST
};
int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct sk_buff *skb);
-void br_forward(const struct net_bridge_port *to, struct sk_buff *skb,
+void br_forward(struct net_bridge_dst dst, struct sk_buff *skb,
bool local_rcv, bool local_orig);
int br_forward_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index cd2b04236a99..92b488793914 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -55,7 +55,7 @@ static void nft_reject_br_send_v4_tcp_reset(struct net *net,
nft_reject_br_push_etherhdr(oldskb, nskb);
- br_forward(br_port_get_rcu(dev), nskb, false, true);
+ br_forward(br_port_to_dst(br_port_get_rcu(dev)), nskb, false, true);
}
static void nft_reject_br_send_v4_unreach(struct net *net,
@@ -71,7 +71,7 @@ static void nft_reject_br_send_v4_unreach(struct net *net,
nft_reject_br_push_etherhdr(oldskb, nskb);
- br_forward(br_port_get_rcu(dev), nskb, false, true);
+ br_forward(br_port_to_dst(br_port_get_rcu(dev)), nskb, false, true);
}
static void nft_reject_br_send_v6_tcp_reset(struct net *net,
@@ -87,7 +87,7 @@ static void nft_reject_br_send_v6_tcp_reset(struct net *net,
nft_reject_br_push_etherhdr(oldskb, nskb);
- br_forward(br_port_get_rcu(dev), nskb, false, true);
+ br_forward(br_port_to_dst(br_port_get_rcu(dev)), nskb, false, true);
}
@@ -104,7 +104,7 @@ static void nft_reject_br_send_v6_unreach(struct net *net,
nft_reject_br_push_etherhdr(oldskb, nskb);
- br_forward(br_port_get_rcu(dev), nskb, false, true);
+ br_forward(br_port_to_dst(br_port_get_rcu(dev)), nskb, false, true);
}
static void nft_reject_bridge_eval(const struct nft_expr *expr,
--
2.47.3
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path
2026-09-30 7:13 [PATCH net-next 00/12] net: bridge: vlan: optimize standard fdb fwding path Nikolay Aleksandrov
` (11 preceding siblings ...)
2026-09-30 7:14 ` [PATCH net-next 12/12] net: bridge: fdb: avoid VLAN lookups in unicast forwarding Nikolay Aleksandrov
@ 2026-10-02 20:00 ` patchwork-bot+netdevbpf
12 siblings, 0 replies; 28+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-02 20:00 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, idosch, davem, edumazet, kuba, pabeni, horms, bridge
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 30 Sep 2026 10:13:59 +0300 you wrote:
> Hi,
> This patch-set is a follow-up after the bridge flood fwding path
> optimizations and applies the same idea to the standard fdb fwding path.
> We are able to remove 2 vlan hash lookups in the standard fdb fwding
> path by caching the port-VLAN pointer in the fdb, to do that we switch the
> fdb dst to an opaque type that can be either a port pointer or vlan pointer
> differentiated by a bit. The field is a struct and also has a __private tag
> so we can catch direct users, it should be used only via the helpers.
> Sharing a field makes it easier to pass it around and also allows us to
> save struct space for the fast-path. Interesting case is when an fdb is
> promoted from a raw port to port-VLAN on the same port, it needs to be
> handled carefully so we use cmpxchg to make sure we don't generate
> deletion/replace notifications because it doesn't change the port.
> I tested VLAN deletion after these changes (we now wait a grace period for
> every delete) and the hit was ~20% reduction in deleted VLANs / sec
> deleting 4k VLANs took 14ms more and on my VM the VLANs deleted / sec went
> from 63k to 52k / sec. The complexity to batch them is not worth it.
>
> [...]
Here is the summary with links:
- [net-next,01/12] net: bridge: introduce a bridge destination type
https://git.kernel.org/netdev/net-next/c/13bb47965e02
- [net-next,02/12] net: bridge: use net_bridge_dst for fdb destinations
https://git.kernel.org/netdev/net-next/c/fd7a8eb03c32
- [net-next,03/12] net: bridge: add VLAN support to bridge destinations
https://git.kernel.org/netdev/net-next/c/a39c339fd6c5
- [net-next,04/12] net: bridge: vlan: return VLAN entries from ingress helpers
https://git.kernel.org/netdev/net-next/c/cebd6130e6c2
- [net-next,05/12] net: bridge: fdb: pass VLAN entries to learning updates
https://git.kernel.org/netdev/net-next/c/8247ef76032b
- [net-next,06/12] net: bridge: fdb: consolidate port-VLAN cleanup
https://git.kernel.org/netdev/net-next/c/f0a349b92120
- [net-next,07/12] net: bridge: vlan: split unpublishing from deletion
https://git.kernel.org/netdev/net-next/c/e5c71bf91447
- [net-next,08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
https://git.kernel.org/netdev/net-next/c/83edc87a9172
- [net-next,09/12] net: bridge: fdb: factor out existing entry updates
https://git.kernel.org/netdev/net-next/c/941056f91907
- [net-next,10/12] net: bridge: fdb: cache port VLANs in learned entries
https://git.kernel.org/netdev/net-next/c/8009eb405fc6
- [net-next,11/12] net: bridge: fdb: cache VLAN destinations in configured entries
https://git.kernel.org/netdev/net-next/c/8533e9b85bd2
- [net-next,12/12] net: bridge: fdb: avoid VLAN lookups in unicast forwarding
https://git.kernel.org/netdev/net-next/c/f1829618e0ad
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 28+ messages in thread