* [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch
@ 2026-09-18 9:59 Bernardo Soares
2026-09-18 9:59 ` [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Bernardo Soares @ 2026-09-18 9:59 UTC (permalink / raw)
To: Mark Bloch; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov, Bernardo Soares
v4 of "net/mlx5: Bridge, don't fail switchdev events of sibling
eswitch ports" (5f324c5d1b12), addressing reviewer feedback on v3:
- The v3 commit message for patch 1 wrongly attributed the recursive
lower-device walk fix to LAG bond enslavement order. As pointed
out in review, mlx5_esw_bridge_lag_rep_get() already filters on
mlx5_esw_bridge_dev_same_esw() per candidate and cannot select a
sibling's rep. The actual bug is in the generic recursive walk
used when attribute changes are emitted against the bridge master
netdevice directly (a bridge with representors of more than one
eswitch instance enslaved, no LAG involved): the walk returns as
soon as any lower device yields a rep, and the underlying base
case only checks same-HW, not ownership. Commit message rewritten
to describe this correctly; no functional change from v3.
- The v3 commit message for patch 2 claimed a "replayed/duplicate"
NETDEV_CHANGEUPPER unlink as one of the reachable cases. As
pointed out in review, netdevice notifiers are not replayed, so
there is no such duplicate delivery. The actual (and only)
reachable case is a sibling instance whose bridge offload notifier
registers after a peer port was already enslaved, so it misses
the link event and never tracks the port, then genuinely receives
the later unlink event. Commit message rewritten accordingly; no
functional change from v3.
Tested Patch 1 on a ConnectX-7 NIC (MT2910) on my single NIC system.
Patch 2 requires a multiple eswitch instance setup, so I wasn't able
to exercise its code paths.
Note: v1/v2 were sent From/Signed-off-by bersoare@isovalent.com; v3
and v4 are sent from my personal address (bsoares.it@gmail.com)
instead, for unrelated mail delivery reasons. Same author, same
person.
Bernardo Soares (2):
net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer
ports
.../mellanox/mlx5/core/en/rep/bridge.c | 45 +++++++++++++++----
.../ethernet/mellanox/mlx5/core/esw/bridge.c | 15 +++++--
.../ethernet/mellanox/mlx5/core/esw/bridge.h | 2 +
3 files changed, 49 insertions(+), 13 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports 2026-09-18 9:59 [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares @ 2026-09-18 9:59 ` Bernardo Soares 2026-09-22 6:52 ` Mark Bloch 2026-09-18 9:59 ` [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares ` (2 subsequent siblings) 3 siblings, 1 reply; 7+ messages in thread From: Bernardo Soares @ 2026-09-18 9:59 UTC (permalink / raw) To: Mark Bloch; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov, Bernardo Soares mlx5 registers the bridge offload switchdev notifiers once per eswitch instance, but the notifier chains are global, so every instance sees every event and must filter out the ones that aren't its own. The existing filter, mlx5_esw_bridge_dev_same_hw(), only checks that the event netdevice sits on the same HCA - intentional for merged eswitch, where one bridge can span representors of several eswitches on one HCA - but same-HCA doesn't mean the instance actually has that port: peer ports are only created reactively from NETDEV_CHANGEUPPER, so an instance brought up after a sibling PF's port was already enslaved has none. The port object and attribute handlers claim the event anyway once same-HW passes, then fail the port lookup and return -EINVAL, which gets reported to user space even though the owning instance already handled it (e.g. "bridge vlan add ... RTNETLINK answers: Invalid argument"). Fix by filtering on the tracked port instead. The same gap exists in the generic recursive lower-device walk used by attribute changes on a bridge with more than one representor enslaved directly: mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() is entered with the bridge master netdevice, falls through to its generic netdev_for_each_lower_dev() loop, and returns as soon as the recursion into any one lower device yields a non-NULL rep - the underlying base case, mlx5_esw_bridge_rep_vport_num_vhca_id_get(), only checks mlx5_esw_bridge_dev_same_hw(), not ownership by the calling instance's br_offloads. mlx5_esw_bridge_lag_rep_get(), used for the LAG-master case, already filters on mlx5_esw_bridge_dev_same_esw() per candidate and so cannot select a sibling's rep; it is not the source of this bug. On a merged-eswitch HCA with a bridge spanning representors of more than one eswitch instance directly, the walk can return a sibling's rep instead of continuing to the one the calling instance actually owns, so the attribute change fails the same way as above. Fix by checking mlx5_esw_bridge_port_exists() at the point each rep is picked, same as the previous fix did for the notifier filter. Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity") Signed-off-by: Bernardo Soares <bsoares.it@gmail.com> Cc: Vlad Buslov <vladbu@nvidia.com> Cc: Saeed Mahameed <saeedm@nvidia.com> --- .../mellanox/mlx5/core/en/rep/bridge.c | 45 +++++++++++++++---- .../ethernet/mellanox/mlx5/core/esw/bridge.c | 6 +++ .../ethernet/mellanox/mlx5/core/esw/bridge.h | 2 + 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c index baac38bece14..4b7b0a0fc2b2 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c @@ -85,9 +85,16 @@ mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(struct net_device *dev, struct m struct net_device *lower_dev; struct list_head *iter; - if (netif_is_lag_master(dev) || mlx5e_eswitch_rep(dev)) - return mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, esw, vport_num, - esw_owner_vhca_id); + if (netif_is_lag_master(dev) || mlx5e_eswitch_rep(dev)) { + struct net_device *rep; + + rep = mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, esw, vport_num, + esw_owner_vhca_id); + if (rep && !mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, + esw->br_offloads)) + return NULL; + return rep; + } netdev_for_each_lower_dev(dev, lower_dev, iter) { struct net_device *rep; @@ -104,6 +111,28 @@ mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(struct net_device *dev, struct m return NULL; } +static bool mlx5_esw_bridge_rep_port_lookup(struct net_device *dev, + struct mlx5_esw_bridge_offloads *br_offloads, + u16 *vport_num, u16 *esw_owner_vhca_id) +{ + if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, vport_num, + esw_owner_vhca_id)) + return false; + + return mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, br_offloads); +} + +static bool mlx5_esw_bridge_lower_rep_port_lookup(struct net_device *dev, + struct mlx5_esw_bridge_offloads *br_offloads, + u16 *vport_num, u16 *esw_owner_vhca_id) +{ + if (!mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(dev, br_offloads->esw, vport_num, + esw_owner_vhca_id)) + return false; + + return mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, br_offloads); +} + static bool mlx5_esw_bridge_is_local(struct net_device *dev, struct net_device *rep, struct mlx5_eswitch *esw) { @@ -218,8 +247,7 @@ mlx5_esw_bridge_port_obj_add(struct net_device *dev, u16 vport_num, esw_owner_vhca_id; int err; - if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, - &esw_owner_vhca_id)) + if (!mlx5_esw_bridge_rep_port_lookup(dev, br_offloads, &vport_num, &esw_owner_vhca_id)) return 0; port_obj_info->handled = true; @@ -251,8 +279,7 @@ mlx5_esw_bridge_port_obj_del(struct net_device *dev, const struct switchdev_obj_port_mdb *mdb; u16 vport_num, esw_owner_vhca_id; - if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, - &esw_owner_vhca_id)) + if (!mlx5_esw_bridge_rep_port_lookup(dev, br_offloads, &vport_num, &esw_owner_vhca_id)) return 0; port_obj_info->handled = true; @@ -283,8 +310,8 @@ mlx5_esw_bridge_port_obj_attr_set(struct net_device *dev, u16 vport_num, esw_owner_vhca_id; int err = 0; - if (!mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, - &esw_owner_vhca_id)) + if (!mlx5_esw_bridge_lower_rep_port_lookup(dev, br_offloads, &vport_num, + &esw_owner_vhca_id)) return 0; port_attr_info->handled = true; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c index 87b5fd349594..ac90ccda1272 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c @@ -1686,6 +1686,12 @@ int mlx5_esw_bridge_vport_peer_unlink(struct net_device *br_netdev, u16 vport_nu extack); } +bool mlx5_esw_bridge_port_exists(u16 vport_num, u16 esw_owner_vhca_id, + struct mlx5_esw_bridge_offloads *br_offloads) +{ + return mlx5_esw_bridge_port_lookup(vport_num, esw_owner_vhca_id, br_offloads); +} + int mlx5_esw_bridge_port_vlan_add(u16 vport_num, u16 esw_owner_vhca_id, u16 vid, u16 flags, struct mlx5_esw_bridge_offloads *br_offloads, struct netlink_ext_ack *extack) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h index d6f539161993..a4e59cc21089 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h @@ -80,6 +80,8 @@ int mlx5_esw_bridge_vlan_proto_set(u16 vport_num, u16 esw_owner_vhca_id, u16 pro struct mlx5_esw_bridge_offloads *br_offloads); int mlx5_esw_bridge_mcast_set(u16 vport_num, u16 esw_owner_vhca_id, bool enable, struct mlx5_esw_bridge_offloads *br_offloads); +bool mlx5_esw_bridge_port_exists(u16 vport_num, u16 esw_owner_vhca_id, + struct mlx5_esw_bridge_offloads *br_offloads); int mlx5_esw_bridge_port_vlan_add(u16 vport_num, u16 esw_owner_vhca_id, u16 vid, u16 flags, struct mlx5_esw_bridge_offloads *br_offloads, struct netlink_ext_ack *extack); -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports 2026-09-18 9:59 ` [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares @ 2026-09-22 6:52 ` Mark Bloch 0 siblings, 0 replies; 7+ messages in thread From: Mark Bloch @ 2026-09-22 6:52 UTC (permalink / raw) To: Bernardo Soares; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov On 18/09/2026 12:59, Bernardo Soares wrote: > mlx5 registers the bridge offload switchdev notifiers once per eswitch > instance, but the notifier chains are global, so every instance sees > every event and must filter out the ones that aren't its own. The > existing filter, mlx5_esw_bridge_dev_same_hw(), only checks that the > event netdevice sits on the same HCA - intentional for merged eswitch, > where one bridge can span representors of several eswitches on one > HCA - but same-HCA doesn't mean the instance actually has that port: > peer ports are only created reactively from NETDEV_CHANGEUPPER, so an > instance brought up after a sibling PF's port was already enslaved has > none. The port object and attribute handlers claim the event anyway > once same-HW passes, then fail the port lookup and return -EINVAL, > which gets reported to user space even though the owning instance > already handled it (e.g. "bridge vlan add ... RTNETLINK answers: > Invalid argument"). Fix by filtering on the tracked port instead. > > The same gap exists in the generic recursive lower-device walk used by > attribute changes on a bridge with more than one representor enslaved > directly: mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() is entered > with the bridge master netdevice, falls through to its generic > netdev_for_each_lower_dev() loop, and returns as soon as the recursion > into any one lower device yields a non-NULL rep - the underlying base > case, mlx5_esw_bridge_rep_vport_num_vhca_id_get(), only checks > mlx5_esw_bridge_dev_same_hw(), not ownership by the calling instance's > br_offloads. mlx5_esw_bridge_lag_rep_get(), used for the LAG-master > case, already filters on mlx5_esw_bridge_dev_same_esw() per candidate > and so cannot select a sibling's rep; it is not the source of this bug. > On a merged-eswitch HCA with a bridge spanning representors of more > than one eswitch instance directly, the walk can return a sibling's rep > instead of continuing to the one the calling instance actually owns, so > the attribute change fails the same way as above. Fix by checking > mlx5_esw_bridge_port_exists() at the point each rep is picked, same as > the previous fix did for the notifier filter. > > Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity") > Signed-off-by: Bernardo Soares <bsoares.it@gmail.com> > Cc: Vlad Buslov <vladbu@nvidia.com> > Cc: Saeed Mahameed <saeedm@nvidia.com> Thanks, Reviewed-by: Mark Bloch <mbloch@nvidia.com> Mark > --- > .../mellanox/mlx5/core/en/rep/bridge.c | 45 +++++++++++++++---- > .../ethernet/mellanox/mlx5/core/esw/bridge.c | 6 +++ > .../ethernet/mellanox/mlx5/core/esw/bridge.h | 2 + > 3 files changed, 44 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c > index baac38bece14..4b7b0a0fc2b2 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c > @@ -85,9 +85,16 @@ mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(struct net_device *dev, struct m > struct net_device *lower_dev; > struct list_head *iter; > > - if (netif_is_lag_master(dev) || mlx5e_eswitch_rep(dev)) > - return mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, esw, vport_num, > - esw_owner_vhca_id); > + if (netif_is_lag_master(dev) || mlx5e_eswitch_rep(dev)) { > + struct net_device *rep; > + > + rep = mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, esw, vport_num, > + esw_owner_vhca_id); > + if (rep && !mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, > + esw->br_offloads)) > + return NULL; > + return rep; > + } > > netdev_for_each_lower_dev(dev, lower_dev, iter) { > struct net_device *rep; > @@ -104,6 +111,28 @@ mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(struct net_device *dev, struct m > return NULL; > } > > +static bool mlx5_esw_bridge_rep_port_lookup(struct net_device *dev, > + struct mlx5_esw_bridge_offloads *br_offloads, > + u16 *vport_num, u16 *esw_owner_vhca_id) > +{ > + if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, vport_num, > + esw_owner_vhca_id)) > + return false; > + > + return mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, br_offloads); > +} > + > +static bool mlx5_esw_bridge_lower_rep_port_lookup(struct net_device *dev, > + struct mlx5_esw_bridge_offloads *br_offloads, > + u16 *vport_num, u16 *esw_owner_vhca_id) > +{ > + if (!mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(dev, br_offloads->esw, vport_num, > + esw_owner_vhca_id)) > + return false; > + > + return mlx5_esw_bridge_port_exists(*vport_num, *esw_owner_vhca_id, br_offloads); > +} > + > static bool mlx5_esw_bridge_is_local(struct net_device *dev, struct net_device *rep, > struct mlx5_eswitch *esw) > { > @@ -218,8 +247,7 @@ mlx5_esw_bridge_port_obj_add(struct net_device *dev, > u16 vport_num, esw_owner_vhca_id; > int err; > > - if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, > - &esw_owner_vhca_id)) > + if (!mlx5_esw_bridge_rep_port_lookup(dev, br_offloads, &vport_num, &esw_owner_vhca_id)) > return 0; > > port_obj_info->handled = true; > @@ -251,8 +279,7 @@ mlx5_esw_bridge_port_obj_del(struct net_device *dev, > const struct switchdev_obj_port_mdb *mdb; > u16 vport_num, esw_owner_vhca_id; > > - if (!mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, > - &esw_owner_vhca_id)) > + if (!mlx5_esw_bridge_rep_port_lookup(dev, br_offloads, &vport_num, &esw_owner_vhca_id)) > return 0; > > port_obj_info->handled = true; > @@ -283,8 +310,8 @@ mlx5_esw_bridge_port_obj_attr_set(struct net_device *dev, > u16 vport_num, esw_owner_vhca_id; > int err = 0; > > - if (!mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(dev, br_offloads->esw, &vport_num, > - &esw_owner_vhca_id)) > + if (!mlx5_esw_bridge_lower_rep_port_lookup(dev, br_offloads, &vport_num, > + &esw_owner_vhca_id)) > return 0; > > port_attr_info->handled = true; > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > index 87b5fd349594..ac90ccda1272 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > @@ -1686,6 +1686,12 @@ int mlx5_esw_bridge_vport_peer_unlink(struct net_device *br_netdev, u16 vport_nu > extack); > } > > +bool mlx5_esw_bridge_port_exists(u16 vport_num, u16 esw_owner_vhca_id, > + struct mlx5_esw_bridge_offloads *br_offloads) > +{ > + return mlx5_esw_bridge_port_lookup(vport_num, esw_owner_vhca_id, br_offloads); > +} > + > int mlx5_esw_bridge_port_vlan_add(u16 vport_num, u16 esw_owner_vhca_id, u16 vid, u16 flags, > struct mlx5_esw_bridge_offloads *br_offloads, > struct netlink_ext_ack *extack) > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h > index d6f539161993..a4e59cc21089 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h > +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.h > @@ -80,6 +80,8 @@ int mlx5_esw_bridge_vlan_proto_set(u16 vport_num, u16 esw_owner_vhca_id, u16 pro > struct mlx5_esw_bridge_offloads *br_offloads); > int mlx5_esw_bridge_mcast_set(u16 vport_num, u16 esw_owner_vhca_id, bool enable, > struct mlx5_esw_bridge_offloads *br_offloads); > +bool mlx5_esw_bridge_port_exists(u16 vport_num, u16 esw_owner_vhca_id, > + struct mlx5_esw_bridge_offloads *br_offloads); > int mlx5_esw_bridge_port_vlan_add(u16 vport_num, u16 esw_owner_vhca_id, u16 vid, u16 flags, > struct mlx5_esw_bridge_offloads *br_offloads, > struct netlink_ext_ack *extack); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports 2026-09-18 9:59 [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares 2026-09-18 9:59 ` [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares @ 2026-09-18 9:59 ` Bernardo Soares 2026-09-22 6:52 ` Mark Bloch 2026-09-22 6:42 ` [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Mark Bloch 2026-09-23 1:20 ` patchwork-bot+netdevbpf 3 siblings, 1 reply; 7+ messages in thread From: Bernardo Soares @ 2026-09-18 9:59 UTC (permalink / raw) To: Mark Bloch; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov, Bernardo Soares mlx5_esw_bridge_vport_unlink() returns -EINVAL when the port isn't tracked by this instance's br_offloads. This is reachable on a sibling instance that registered its notifier after the port was already enslaved: it never saw the NETDEV_CHANGEUPPER link event, so peer_link() never created a peer port for it, but it does see the later unlink event and fails. Return 0 instead, and give mlx5_esw_bridge_vport_peer_unlink() the same merged_eswitch capability guard peer_link() already has, since without it peer_link() likewise never creates a port to unlink. This also matters beyond the -EINVAL itself: mlx5_esw_bridge_switchdev_port_event() runs on the per-netns netdev_chain, and notifier_from_errno(-EINVAL) sets NOTIFY_STOP_MASK, which call_netdevice_notifiers_info() checks to stop calling further listeners on that chain - so the old -EINVAL silently dropped the event for any listener registered later on the same chain, even though none of it was visible to user space since __netdev_upper_dev_unlink() discards the return value. Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity") Signed-off-by: Bernardo Soares <bsoares.it@gmail.com> --- drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c index ac90ccda1272..b4cf3c5ac0dd 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c @@ -1649,10 +1649,8 @@ int mlx5_esw_bridge_vport_unlink(struct net_device *br_netdev, u16 vport_num, int err; port = mlx5_esw_bridge_port_lookup(vport_num, esw_owner_vhca_id, br_offloads); - if (!port) { - NL_SET_ERR_MSG_MOD(extack, "Port is not attached to any bridge"); - return -EINVAL; - } + if (!port) + return 0; if (port->bridge->ifindex != br_netdev->ifindex) { NL_SET_ERR_MSG_MOD(extack, "Port is attached to another bridge"); return -EINVAL; @@ -1682,6 +1680,9 @@ int mlx5_esw_bridge_vport_peer_unlink(struct net_device *br_netdev, u16 vport_nu struct mlx5_esw_bridge_offloads *br_offloads, struct netlink_ext_ack *extack) { + if (!MLX5_CAP_ESW(br_offloads->esw->dev, merged_eswitch)) + return 0; + return mlx5_esw_bridge_vport_unlink(br_netdev, vport_num, esw_owner_vhca_id, br_offloads, extack); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports 2026-09-18 9:59 ` [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares @ 2026-09-22 6:52 ` Mark Bloch 0 siblings, 0 replies; 7+ messages in thread From: Mark Bloch @ 2026-09-22 6:52 UTC (permalink / raw) To: Bernardo Soares; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov On 18/09/2026 12:59, Bernardo Soares wrote: > mlx5_esw_bridge_vport_unlink() returns -EINVAL when the port isn't > tracked by this instance's br_offloads. This is reachable on a sibling > instance that registered its notifier after the port was already > enslaved: it never saw the NETDEV_CHANGEUPPER link event, so > peer_link() never created a peer port for it, but it does see the > later unlink event and fails. Return 0 instead, and give > mlx5_esw_bridge_vport_peer_unlink() the same merged_eswitch capability > guard peer_link() already has, since without it peer_link() likewise > never creates a port to unlink. > > This also matters beyond the -EINVAL itself: > mlx5_esw_bridge_switchdev_port_event() runs on the per-netns > netdev_chain, and notifier_from_errno(-EINVAL) sets NOTIFY_STOP_MASK, > which call_netdevice_notifiers_info() checks to stop calling further > listeners on that chain - so the old -EINVAL silently dropped the > event for any listener registered later on the same chain, even > though none of it was visible to user space since > __netdev_upper_dev_unlink() discards the return value. > > Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity") > Signed-off-by: Bernardo Soares <bsoares.it@gmail.com> Thanks, Reviewed-by: Mark Bloch <mbloch@nvidia.com> Mark > --- > drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > index ac90ccda1272..b4cf3c5ac0dd 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c > @@ -1649,10 +1649,8 @@ int mlx5_esw_bridge_vport_unlink(struct net_device *br_netdev, u16 vport_num, > int err; > > port = mlx5_esw_bridge_port_lookup(vport_num, esw_owner_vhca_id, br_offloads); > - if (!port) { > - NL_SET_ERR_MSG_MOD(extack, "Port is not attached to any bridge"); > - return -EINVAL; > - } > + if (!port) > + return 0; > if (port->bridge->ifindex != br_netdev->ifindex) { > NL_SET_ERR_MSG_MOD(extack, "Port is attached to another bridge"); > return -EINVAL; > @@ -1682,6 +1680,9 @@ int mlx5_esw_bridge_vport_peer_unlink(struct net_device *br_netdev, u16 vport_nu > struct mlx5_esw_bridge_offloads *br_offloads, > struct netlink_ext_ack *extack) > { > + if (!MLX5_CAP_ESW(br_offloads->esw->dev, merged_eswitch)) > + return 0; > + > return mlx5_esw_bridge_vport_unlink(br_netdev, vport_num, esw_owner_vhca_id, br_offloads, > extack); > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch 2026-09-18 9:59 [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares 2026-09-18 9:59 ` [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares 2026-09-18 9:59 ` [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares @ 2026-09-22 6:42 ` Mark Bloch 2026-09-23 1:20 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 7+ messages in thread From: Mark Bloch @ 2026-09-22 6:42 UTC (permalink / raw) To: Bernardo Soares; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov On 18/09/2026 12:59, Bernardo Soares wrote: > v4 of "net/mlx5: Bridge, don't fail switchdev events of sibling > eswitch ports" (5f324c5d1b12), addressing reviewer feedback on v3: > > - The v3 commit message for patch 1 wrongly attributed the recursive > lower-device walk fix to LAG bond enslavement order. As pointed > out in review, mlx5_esw_bridge_lag_rep_get() already filters on > mlx5_esw_bridge_dev_same_esw() per candidate and cannot select a > sibling's rep. The actual bug is in the generic recursive walk > used when attribute changes are emitted against the bridge master > netdevice directly (a bridge with representors of more than one > eswitch instance enslaved, no LAG involved): the walk returns as > soon as any lower device yields a rep, and the underlying base > case only checks same-HW, not ownership. Commit message rewritten > to describe this correctly; no functional change from v3. > - The v3 commit message for patch 2 claimed a "replayed/duplicate" > NETDEV_CHANGEUPPER unlink as one of the reachable cases. As > pointed out in review, netdevice notifiers are not replayed, so > there is no such duplicate delivery. The actual (and only) > reachable case is a sibling instance whose bridge offload notifier > registers after a peer port was already enslaved, so it misses > the link event and never tracks the port, then genuinely receives > the later unlink event. Commit message rewritten accordingly; no > functional change from v3. > > Tested Patch 1 on a ConnectX-7 NIC (MT2910) on my single NIC system. > Patch 2 requires a multiple eswitch instance setup, so I wasn't able > to exercise its code paths. > > Note: v1/v2 were sent From/Signed-off-by bersoare@isovalent.com; v3 > and v4 are sent from my personal address (bsoares.it@gmail.com) > instead, for unrelated mail delivery reasons. Same author, same > person. > For the series, Reviewed-by: Mark Bloch <mbloch@nvidia.com> Mark > Bernardo Soares (2): > net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports > net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer > ports > > .../mellanox/mlx5/core/en/rep/bridge.c | 45 +++++++++++++++---- > .../ethernet/mellanox/mlx5/core/esw/bridge.c | 15 +++++-- > .../ethernet/mellanox/mlx5/core/esw/bridge.h | 2 + > 3 files changed, 49 insertions(+), 13 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch 2026-09-18 9:59 [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares ` (2 preceding siblings ...) 2026-09-22 6:42 ` [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Mark Bloch @ 2026-09-23 1:20 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 7+ messages in thread From: patchwork-bot+netdevbpf @ 2026-09-23 1:20 UTC (permalink / raw) To: Bernardo Soares; +Cc: mbloch, daniel, netdev, saeedm, vladbu Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 18 Sep 2026 10:59:29 +0100 you wrote: > v4 of "net/mlx5: Bridge, don't fail switchdev events of sibling > eswitch ports" (5f324c5d1b12), addressing reviewer feedback on v3: > > - The v3 commit message for patch 1 wrongly attributed the recursive > lower-device walk fix to LAG bond enslavement order. As pointed > out in review, mlx5_esw_bridge_lag_rep_get() already filters on > mlx5_esw_bridge_dev_same_esw() per candidate and cannot select a > sibling's rep. The actual bug is in the generic recursive walk > used when attribute changes are emitted against the bridge master > netdevice directly (a bridge with representors of more than one > eswitch instance enslaved, no LAG involved): the walk returns as > soon as any lower device yields a rep, and the underlying base > case only checks same-HW, not ownership. Commit message rewritten > to describe this correctly; no functional change from v3. > - The v3 commit message for patch 2 claimed a "replayed/duplicate" > NETDEV_CHANGEUPPER unlink as one of the reachable cases. As > pointed out in review, netdevice notifiers are not replayed, so > there is no such duplicate delivery. The actual (and only) > reachable case is a sibling instance whose bridge offload notifier > registers after a peer port was already enslaved, so it misses > the link event and never tracks the port, then genuinely receives > the later unlink event. Commit message rewritten accordingly; no > functional change from v3. > > [...] Here is the summary with links: - [net,v4,1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports https://git.kernel.org/netdev/net/c/35e6f970f553 - [net,v4,2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports https://git.kernel.org/netdev/net/c/2e51097c982b 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] 7+ messages in thread
end of thread, other threads:[~2026-09-23 1:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-18 9:59 [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares 2026-09-18 9:59 ` [PATCH net v4 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares 2026-09-22 6:52 ` Mark Bloch 2026-09-18 9:59 ` [PATCH net v4 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares 2026-09-22 6:52 ` Mark Bloch 2026-09-22 6:42 ` [PATCH net v4 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Mark Bloch 2026-09-23 1:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox