Netdev List
 help / color / mirror / Atom feed
* [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

* [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 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 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

* 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
                   ` (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