[PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

[PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

[Daniel Borkmann]

From: Bernardo Soares <bersoare@isovalent.com>

mlx5 registers the bridge offload switchdev notifiers once per eswitch
instance, i.e. once per PF, but the switchdev notifier chains are global.
Every registered instance is called for every switchdev event and has to
filter out the events that are not its own.

Since commit c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch
connectivity") that filter is mlx5_esw_bridge_dev_same_hw(), which only
tells that the event netdevice and the eswitch of the instance sit on the
same HCA. This is intentional: with merged eswitch a bridge can span the
representors of several eswitches of one HCA, and each instance keeps the
ports of its sibling eswitches as peer ports in order to offload FDB
entries pointing at them.

However, the instance may not have the port at all. Peer ports are only
created from NETDEV_CHANGEUPPER events observed while the instance is
already registered, and only when merged_eswitch is supported. So when
more than one PF of the same HCA runs bridge offload and the PF that is
put in switchdev mode last has the uplink of an earlier configured PF
already enslaved to a bridge, that instance has no port for it.

The switchdev port object and attribute handlers do not check for this.
They claim the event via port_obj_info->handled and pass the
vport_num/esw_owner_vhca_id pair down, where the port lookup fails and
-EINVAL is returned. call_switchdev_blocking_notifiers() stops the chain
on the error and it is reported to user space, even though the instance
owning the port has already offloaded the request:

  # bridge vlan add dev ens1f2np2 vid 999 master
  RTNETLINK answers: Invalid argument

The same applies to bridge vlan del, mdb add/del and to the bridge
attributes (ageing time, vlan filtering, vlan protocol, mcast). Thus
fix by filter on the port instead. We tested that this fixes the issue
on ConnectX-7.

Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity")
Signed-off-by: Bernardo Soares <bersoare@isovalent.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Vlad Buslov <vladbu@nvidia.com>
Cc: Saeed Mahameed <saeedm@nvidia.com>
---
 .../mellanox/mlx5/core/en/rep/bridge.c        | 32 +++++++++++++++----
 .../ethernet/mellanox/mlx5/core/esw/bridge.c  |  6 ++++
 .../ethernet/mellanox/mlx5/core/esw/bridge.h  |  2 ++
 3 files changed, 34 insertions(+), 6 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..56592e2e6b9b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
@@ -104,6 +104,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 +240,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 +272,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 +303,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

Re: [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

[Mark Bloch]

On 08/09/2026 19:19, Daniel Borkmann wrote:
> From: Bernardo Soares <bersoare@isovalent.com>
> 
> mlx5 registers the bridge offload switchdev notifiers once per eswitch
> instance, i.e. once per PF, but the switchdev notifier chains are global.
> Every registered instance is called for every switchdev event and has to
> filter out the events that are not its own.
> 
> Since commit c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch
> connectivity") that filter is mlx5_esw_bridge_dev_same_hw(), which only
> tells that the event netdevice and the eswitch of the instance sit on the
> same HCA. This is intentional: with merged eswitch a bridge can span the
> representors of several eswitches of one HCA, and each instance keeps the
> ports of its sibling eswitches as peer ports in order to offload FDB
> entries pointing at them.
> 
> However, the instance may not have the port at all. Peer ports are only
> created from NETDEV_CHANGEUPPER events observed while the instance is
> already registered, and only when merged_eswitch is supported. So when
> more than one PF of the same HCA runs bridge offload and the PF that is
> put in switchdev mode last has the uplink of an earlier configured PF
> already enslaved to a bridge, that instance has no port for it.
> 
> The switchdev port object and attribute handlers do not check for this.
> They claim the event via port_obj_info->handled and pass the
> vport_num/esw_owner_vhca_id pair down, where the port lookup fails and
> -EINVAL is returned. call_switchdev_blocking_notifiers() stops the chain
> on the error and it is reported to user space, even though the instance
> owning the port has already offloaded the request:
> 
>   # bridge vlan add dev ens1f2np2 vid 999 master
>   RTNETLINK answers: Invalid argument
> 
> The same applies to bridge vlan del, mdb add/del and to the bridge
> attributes (ageing time, vlan filtering, vlan protocol, mcast). Thus
> fix by filter on the port instead. We tested that this fixes the issue
> on ConnectX-7.
> 
> Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity")
> Signed-off-by: Bernardo Soares <bersoare@isovalent.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Vlad Buslov <vladbu@nvidia.com>
> Cc: Saeed Mahameed <saeedm@nvidia.com>
> ---
>  .../mellanox/mlx5/core/en/rep/bridge.c        | 32 +++++++++++++++----
>  .../ethernet/mellanox/mlx5/core/esw/bridge.c  |  6 ++++
>  .../ethernet/mellanox/mlx5/core/esw/bridge.h  |  2 ++
>  3 files changed, 34 insertions(+), 6 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..56592e2e6b9b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
> @@ -104,6 +104,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;

mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() returns on the first rep it finds, shouldn't
you verify it's tracked by br_offloads?


> +
> +	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 +240,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 +272,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 +303,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);
>  }

I wonder if changes are required to the peer unlink function as well.

Mark

>  
> +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);

[PATCH net v2 0/3] net/mlx5: Bridge, fix remaining switchdev ownership gaps on merged eswitch

[Bernardo Soares]

v2 of "net/mlx5: Bridge, don't fail switchdev events of sibling
eswitch ports" (5f324c5d1b12), addressing the two follow-up issues
raised in review plus one additional issue found while auditing the
same code for the same bug class:

 - Patch 1 is the original fix, unchanged.
 - Patch 2 addresses the reviewer's comment that
   mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() picks the first
   structurally-eligible rep found while walking a LAG bond's lower
   devices, without verifying that rep is actually tracked by the
   calling instance's br_offloads. 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.
 - Patch 3 addresses the reviewer's comment asking whether the peer
   unlink path also needed changes, plus a related idempotency gap
   found in follow-up: 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 replayed or duplicate NETDEV_CHANGEUPPER unlink.

Tested Patch 1 on a ConnectX-7 NIC (MT2910) on my single NIC system.
Patches 2 and 3 require a multiple eswitch instance setup, so I wasn't
able to exercise their code paths.

Bernardo Soares (3):
  net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
  net/mlx5: LAG, check if vport exists on br_offloads
  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.50.1

[PATCH net v2 1/3] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

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

Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch connectivity")
Signed-off-by: Bernardo Soares <bersoare@isovalent.com>
Cc: Vlad Buslov <vladbu@nvidia.com>
Cc: Saeed Mahameed <saeedm@nvidia.com>
---
v2: unchanged, resending as part of the series alongside the two
    follow-up fixes raised in review (patches 2 and 3).
---

 .../mellanox/mlx5/core/en/rep/bridge.c        | 32 +++++++++++++++----
 .../ethernet/mellanox/mlx5/core/esw/bridge.c  |  6 ++++
 .../ethernet/mellanox/mlx5/core/esw/bridge.h  |  2 ++
 3 files changed, 34 insertions(+), 6 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..56592e2e6b9b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
@@ -104,6 +104,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 +240,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 +272,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 +303,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

Re: [PATCH net v2 1/3] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] In this commit, mlx5_esw_bridge_lower_rep_port_lookup()
  (drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c:116-126)…

--

net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports

The mlx5 bridge offload registers switchdev notifiers per eswitch instance
while the notifier chains are global, so each instance must filter events
that aren't its own. This patch replaces the same-HW filter in the port
object and attribute handlers with a check that the instance actually
tracks the port, so sibling eswitch events no longer return -EINVAL to
user space.

> 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 baac38bece14a..56592e2e6b9b8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c
> @@ -104,6 +104,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);
> +}
> +

[Severity: Low]
Should the tracked-port test happen inside the lower device walk rather
than after it?

mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() stops at the first
same-HCA representor it finds while walking the bridge's lower devices:

	netdev_for_each_lower_dev(dev, lower_dev, iter) {
		struct net_device *rep;

		if (netif_is_bridge_master(lower_dev))
			continue;

		rep = mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(lower_dev, esw, vport_num,
								      esw_owner_vhca_id);
		if (rep)
			return rep;
	}

Bridge-level attributes are notified with dev == the bridge netdevice, so
on a merged-eswitch HCA whose bridge holds representors of two eswitch
instances, the walk can return the sibling's representor first. If the
calling instance has no peer port for that representor - the late-start
case described in the commit message - mlx5_esw_bridge_port_exists()
fails and the whole lookup returns false, even though a later lower
device is a port this instance does track.

> @@ -283,8 +303,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;

Can this turn the previous -EINVAL into a silently dropped attribute
update for that instance?  mlx5_esw_bridge_port_obj_attr_set() returns 0
with port_attr_info->handled left false, so the ageing time, VLAN
filtering, VLAN protocol and MC-disabled updates never reach that
instance's own bridge object.  Each instance resolves its bridge through
the port, for example:

drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c:mlx5_esw_bridge_vlan_filtering_set() {
	bridge = mlx5_esw_bridge_from_port_lookup(vport_num, esw_owner_vhca_id, br_offloads);
	if (!bridge)
		return -EINVAL;
	...
}

so after e.g. "ip link set br0 type bridge vlan_filtering 1" the software
bridge would have filtering enabled while that instance's hardware bridge
keeps MLX5_ESW_BRIDGE_VLAN_FILTERING_FLAG clear, with no error reported.

The follow-up patch in this series, "net/mlx5: LAG, check if vport exists
on br_offloads", moves the mlx5_esw_bridge_port_exists() test into the
leaf of the recursive walk so an untracked representor returns NULL and
netdev_for_each_lower_dev() continues to the next lower device, which
removes this behaviour at the end of the series.  Would it make sense to
fold that change into this patch, or to note that this patch alone is not
suitable for backporting without it?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914085430.18694-1-bersoare%40isovalent.com

[PATCH net v2 2/3] net/mlx5: LAG, check if vport exists on br_offloads

[Bernardo Soares]

mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() walks a LAG 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. On a merged-eswitch HCA where a bond spans
reps of more than one eswitch instance, this can return a sibling's
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.
Fix by checking mlx5_esw_bridge_port_exists() at the point each rep is
picked, same as the previous commit did for the single-rep case.

Signed-off-by: Bernardo Soares <bersoare@isovalent.com>
---
v2: new patch, addresses review comment on v1 asking whether
    mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() should verify
    the rep it picks while walking a LAG bond's lower devices is
    actually tracked by the calling instance's br_offloads, since it
    otherwise returns on the first structurally-eligible rep found,
    which is not necessarily the one owned by this eswitch instance.
---

 .../net/ethernet/mellanox/mlx5/core/en/rep/bridge.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 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 56592e2e6b9b..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;
-- 
2.43.0

Re: [PATCH net v2 2/3] net/mlx5: LAG, check if vport exists on br_offloads

[Mark Bloch]

On 14/09/2026 11:54, Bernardo Soares wrote:
> mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() walks a LAG 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. On a merged-eswitch HCA where a bond spans
> reps of more than one eswitch instance, this can return a sibling's
> 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.
> Fix by checking mlx5_esw_bridge_port_exists() at the point each rep is
> picked, same as the previous commit did for the single-rep case.
> 
> Signed-off-by: Bernardo Soares <bersoare@isovalent.com>

Please squash this into patch 1.

Mark> ---
> v2: new patch, addresses review comment on v1 asking whether
>     mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() should verify
>     the rep it picks while walking a LAG bond's lower devices is
>     actually tracked by the calling instance's br_offloads, since it
>     otherwise returns on the first structurally-eligible rep found,
>     which is not necessarily the one owned by this eswitch instance.
> ---
> 
>  .../net/ethernet/mellanox/mlx5/core/en/rep/bridge.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 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 56592e2e6b9b..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;

Re: [PATCH net v2 2/3] net/mlx5: LAG, check if vport exists on br_offloads

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 1

- [High] The commit message describes a bug that the code cannot exhibit,
  and the patch is a functional no-op for the scenario it names.
- [Low] The commit asserts a user-visible functional defect ('a bridge
  attribute change on a bonded uplink can silently no-op on the right…

--

net/mlx5: LAG, check if vport exists on br_offloads

This adds a mlx5_esw_bridge_port_exists() check to the LAG-master/rep
branch of mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(), so that a
rep which is not tracked by the calling instance's br_offloads is
rejected instead of returned.

> 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 56592e2e6b9b8..4b7b0a0fc2b2f 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;
> +	}

[Severity: High]
Does this hunk change anything for the bonded-uplink case the changelog
describes?

The changelog says:

    mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() walks a LAG 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. On a merged-eswitch HCA where a bond spans
    reps of more than one eswitch instance, this can return a sibling's
    rep instead of continuing to the one this instance actually owns

For dev being the LAG master, the bond walk happens in
mlx5_esw_bridge_lag_rep_get(), reached via
mlx5_esw_bridge_rep_vport_num_vhca_id_get():

	if (netif_is_lag_master(dev))
		dev = mlx5_esw_bridge_lag_rep_get(dev, esw);

and that walk already filters on the eswitch instance:

	netdev_for_each_lower_dev(dev, lower, iter) {
		if (!mlx5e_eswitch_rep(lower))
			continue;

		if (mlx5_esw_bridge_dev_same_esw(lower, esw))
			return lower;
	}

So can it actually return a sibling instance's rep here?  It looks like
only lowers satisfying mlx5_esw_bridge_dev_same_esw(lower, esw) are
accepted, which would make the "same HCA, is a rep" description apply to
mlx5_esw_bridge_rep_vport_num_vhca_id_get()'s non-LAG path rather than to
the bond walk.

The new branch is also terminal - on rejection it returns NULL and no
other bond member is examined - so what is "continued to" in the LAG case?

There is a second question about the result being identical before and
after.  The only caller is mlx5_esw_bridge_lower_rep_port_lookup(), which
repeats the same test on the same br_offloads:

	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);

With esw->br_offloads and br_offloads being the same object, a dev that is
a LAG master ends up with the same verdict either way, and
mlx5_esw_bridge_port_obj_attr_set() still returns 0 without applying the
attribute:

	if (!mlx5_esw_bridge_lower_rep_port_lookup(dev, br_offloads, &vport_num,
						   &esw_owner_vhca_id))
		return 0;

The behaviour that does change is the recursive lower-device walk further
down, for a dev that is neither a LAG master nor a rep (the bridge
netdevice itself, for bridge-wide attributes such as ageing time, VLAN
filtering, VLAN protocol and MC-disabled): an untracked sibling lower rep
no longer terminates the walk, so the walk can reach the lower rep this
instance does track.  Would it be clearer to describe that path in the
subject and changelog instead of the LAG bond?

[Severity: Low]
Should this patch carry a Fixes: tag?

The changelog claims a user-visible effect ("a bridge attribute change on
a bonded uplink can silently no-op on the right instance"), but there is
no Fixes: line, while the two sibling patches in the series both tag
Fixes: c358ea1741bc ("net/mlx5: Bridge, allow merged eswitch
connectivity").

The behaviour actually changed here (the recursive lower-device walk in
mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() stopping on an untracked
sibling rep) is a merged-eswitch bug too, so either adding the matching
Fixes: tag or rewording the message to say this is follow-up hardening of
the preceding patch with no independent user-visible effect would remove
the ambiguity.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914085430.18694-1-bersoare%40isovalent.com

[PATCH net v2 3/3] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports

[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 replayed/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 skip the global chain
entirely - so the old -EINVAL silently dropped the event for every
other listener on the system, including a sibling PF's own
bridge-offload instance, 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 <bersoare@isovalent.com>
---
v2: new patch, addresses review comment on v1 asking whether the peer
    unlink path also needed changes. mlx5_esw_bridge_vport_peer_unlink()
    now mirrors mlx5_esw_bridge_vport_peer_link()'s merged_eswitch
    capability guard, and mlx5_esw_bridge_vport_unlink() treats an
    already-absent/untracked port as a no-op (return 0) instead of
    -EINVAL, which is reachable on a replayed or duplicate
    NETDEV_CHANGEUPPER unlink and was found while auditing the same
    code for the ownership-check gap raised on v1.
---

 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

Re: [PATCH net v2 3/3] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports

[Mark Bloch]

On 14/09/2026 11:54, 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 replayed/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 skip the global chain
> entirely - so the old -EINVAL silently dropped the event for every
> other listener on the system, including a sibling PF's own
> bridge-offload instance, 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 <bersoare@isovalent.com>

Commit message nit: NETDEV_CHANGEUPPER is not replayed, and
only listeners later in the chain are skipped. The code looks
good.

checkpatch warns the From address is gmail while the signed-off-by
uses the Isovalent address. Please make them match.
v2 was sent as a reply to v1. Please send v3 as a new thread.


Mark

> ---
> v2: new patch, addresses review comment on v1 asking whether the peer
>     unlink path also needed changes. mlx5_esw_bridge_vport_peer_unlink()
>     now mirrors mlx5_esw_bridge_vport_peer_link()'s merged_eswitch
>     capability guard, and mlx5_esw_bridge_vport_unlink() treats an
>     already-absent/untracked port as a no-op (return 0) instead of
>     -EINVAL, which is reachable on a replayed or duplicate
>     NETDEV_CHANGEUPPER unlink and was found while auditing the same
>     code for the ownership-check gap raised on v1.
> ---
> 
>  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);
>  }

Re: [PATCH net v2 3/3] net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Commit message overstates the notifier-chain impact.

--

net/mlx5: Bridge, don't fail unlink of untracked/unsupported peer ports

The patch makes mlx5_esw_bridge_vport_unlink() return 0 instead of -EINVAL
when the vport isn't present in this instance's br_offloads->ports xarray,
and adds a merged_eswitch capability guard to
mlx5_esw_bridge_vport_peer_unlink() mirroring the one in
mlx5_esw_bridge_vport_peer_link().

(No Link: tags in the commit header.)

Only the commit message is being commented on here, so the diff hunks are
omitted.

> 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 skip the global chain
> entirely - so the old -EINVAL silently dropped the event for every
> other listener on the system, including a sibling PF's own
> bridge-offload instance, even though none of it was visible to user
> space since __netdev_upper_dev_unlink() discards the return value.

[Severity: Low]
Is "dropped the event for every other listener on the system" accurate here?

notifier_call_chain() breaks at the callback that returned the stop bit, so
listeners ordered before mlx5's netdev_nb have already been invoked:

kernel/notifier.c:notifier_call_chain() {
	...
		ret = nb->notifier_call(nb, val, v);
	...
		if (ret & NOTIFY_STOP_MASK)
			break;
		nb = next_nb;
	...
}

The part about the global chain does hold:

net/core/dev.c:call_netdevice_notifiers_info() {
	ret = raw_notifier_call_chain(&net->netdev_chain, val, info);
	if (ret & NOTIFY_STOP_MASK)
		return ret;
	return raw_notifier_call_chain(&netdev_chain, val, info);
}

Regarding the sibling PF claim, mlx5 registers on the per-netns chain:

drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c:mlx5e_rep_bridge_init() {
	br_offloads->netdev_nb.notifier_call = mlx5_esw_bridge_switchdev_port_event;
	err = register_netdevice_notifier_net(&init_net, &br_offloads->netdev_nb);
	...
}

A sibling PF's br_offloads instance registers on that same per-netns chain,
so whether it misses the event depends on registration order rather than
being unconditional.

Would wording like "suppresses the remaining per-netns listeners
(potentially including a sibling PF's instance) and all global-chain
listeners" describe the behaviour more precisely?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914085430.18694-1-bersoare%40isovalent.com