* [PATCH net v3 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch
@ 2026-09-16 10:46 Bernardo Soares
2026-09-16 10:46 ` [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares
2026-09-16 10:46 ` [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares
0 siblings, 2 replies; 5+ messages in thread
From: Bernardo Soares @ 2026-09-16 10:46 UTC (permalink / raw)
To: Mark Bloch; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov, Bernardo Soares
v3 of "net/mlx5: Bridge, don't fail switchdev events of sibling
eswitch ports" (5f324c5d1b12), addressing review comments on v1/v2:
- Patch 1 fixes the switchdev port-object/attribute notifier filter
(the original fix), and now also covers the LAG bond lower-device
walk used by attribute changes on a bonded uplink
(mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get()), which had the
same ownership gap: it returned the first structurally-eligible
rep found while walking the bond's lower devices, without checking
it's tracked by the calling instance's br_offloads. Since lower
devices are appended in enslavement order, on a merged-eswitch HCA
with a bond spanning reps of more than one eswitch instance, this
could cause a bridge attribute change (ageing time, vlan filtering/
protocol, mcast) to silently no-op on the correct instance. These
two were squashed into one commit per review.
- Patch 2 addresses the reviewer's comment asking whether the peer
unlink path also needed changes: mlx5_esw_bridge_vport_peer_unlink()
lacked the merged_eswitch capability guard that peer_link() already
has, and mlx5_esw_bridge_vport_unlink() itself returned -EINVAL
rather than treating an already-absent/untracked port as a no-op,
which is reachable on a duplicate NETDEV_CHANGEUPPER unlink.
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
is 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] 5+ messages in thread* [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports 2026-09-16 10:46 [PATCH net v3 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares @ 2026-09-16 10:46 ` Bernardo Soares 2026-09-16 11:57 ` Mark Bloch 2026-09-16 10:46 ` [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares 1 sibling, 1 reply; 5+ messages in thread From: Bernardo Soares @ 2026-09-16 10:46 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 LAG bond lookup used by attribute changes on a bonded uplink: mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() walks the bond's lower devices and returns the first rep that's structurally eligible (same HCA, is a rep), without checking it's tracked by the calling instance's br_offloads. Lower devices are appended in enslavement order (__netdev_adjacent_dev_insert() uses list_add_tail_rcu()), so on a merged-eswitch HCA where a bond spans reps of more than one eswitch instance, whichever rep was enslaved first wins the walk regardless of which instance's br_offloads is doing the lookup. If a sibling's rep was enslaved first, this returns that rep instead of continuing to the one this instance actually owns - reached via mlx5_esw_bridge_port_obj_attr_set(), so a bridge attribute change on a bonded uplink can silently no-op on the right instance depending on enslavement order. Fix the same way, by checking mlx5_esw_bridge_port_exists() at the point each rep is picked. 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] 5+ messages in thread
* Re: [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports 2026-09-16 10:46 ` [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares @ 2026-09-16 11:57 ` Mark Bloch 0 siblings, 0 replies; 5+ messages in thread From: Mark Bloch @ 2026-09-16 11:57 UTC (permalink / raw) To: Bernardo Soares; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov On 16/09/2026 13:46, 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 LAG bond lookup used by attribute changes > on a bonded uplink: mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() > walks the bond's lower devices and returns the first rep that's > structurally eligible (same HCA, is a rep), without checking it's > tracked by the calling instance's br_offloads. Lower devices are > appended in enslavement order (__netdev_adjacent_dev_insert() uses > list_add_tail_rcu()), so on a merged-eswitch HCA where a bond spans > reps of more than one eswitch instance, whichever rep was enslaved > first wins the walk regardless of which instance's br_offloads is > doing the lookup. If a sibling's rep was enslaved first, this returns > that rep instead of continuing to the one this instance actually owns This doesn't match the code. mlx5_esw_bridge_lag_rep_get() requires same_esw, so it cannot select a sibling's rep based on enslavement order. AFAICS this fixes the generic recursive lower-device walk instead. code looks okay, lets just nail the commit messages. > - reached via mlx5_esw_bridge_port_obj_attr_set(), so a bridge > attribute change on a bonded uplink can silently no-op on the right > instance depending on enslavement order. Fix the same way, by checking > mlx5_esw_bridge_port_exists() at the point each rep is picked. > > 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); ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports 2026-09-16 10:46 [PATCH net v3 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares 2026-09-16 10:46 ` [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares @ 2026-09-16 10:46 ` Bernardo Soares 2026-09-16 11:58 ` Mark Bloch 1 sibling, 1 reply; 5+ messages in thread From: Bernardo Soares @ 2026-09-16 10:46 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, which is reachable in two legitimate cases: a duplicate NETDEV_CHANGEUPPER unlink for a port already cleaned up, or a peer-port unlink for a vport whose link was never created because peer_link() skipped it. Return 0 instead in both cases, and give mlx5_esw_bridge_vport_peer_unlink() the same merged_eswitch capability guard peer_link() already has. 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] 5+ messages in thread
* Re: [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports 2026-09-16 10:46 ` [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares @ 2026-09-16 11:58 ` Mark Bloch 0 siblings, 0 replies; 5+ messages in thread From: Mark Bloch @ 2026-09-16 11:58 UTC (permalink / raw) To: Bernardo Soares; +Cc: Daniel Borkmann, netdev, saeedm, Vlad Buslov On 16/09/2026 13:46, Bernardo Soares wrote: > mlx5_esw_bridge_vport_unlink() returns -EINVAL when the port isn't > tracked by this instance's br_offloads, which is reachable in two > legitimate cases: a duplicate NETDEV_CHANGEUPPER unlink for a port > already cleaned up, or a peer-port unlink for a vport whose link was > never created because peer_link() skipped it. Return 0 instead in Where does the duplicate NETDEV_CHANGEUPPER unlink come from? AFAICS the concrete case is a late registered sibling which never saw the link event. Mark > both cases, and give mlx5_esw_bridge_vport_peer_unlink() the same > merged_eswitch capability guard peer_link() already has. > > 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); > } ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-16 11:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-16 10:46 [PATCH net v3 0/2] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch Bernardo Soares 2026-09-16 10:46 ` [PATCH net v3 1/2] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Bernardo Soares 2026-09-16 11:57 ` Mark Bloch 2026-09-16 10:46 ` [PATCH net v3 2/2] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports Bernardo Soares 2026-09-16 11:58 ` Mark Bloch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox