* [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
@ 2026-09-08 16:19 Daniel Borkmann
2026-09-10 12:47 ` Mark Bloch
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2026-09-08 16:19 UTC (permalink / raw)
To: netdev; +Cc: saeedm, Bernardo Soares, Vlad Buslov
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
2026-09-08 16:19 [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Daniel Borkmann
@ 2026-09-10 12:47 ` Mark Bloch
0 siblings, 0 replies; 3+ messages in thread
From: Mark Bloch @ 2026-09-10 12:47 UTC (permalink / raw)
To: Daniel Borkmann, netdev; +Cc: saeedm, Bernardo Soares, Vlad Buslov
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);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
@ 2026-09-11 7:21 netdev-bot+sashiko
0 siblings, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-11 7:21 UTC (permalink / raw)
To: daniel; +Cc: netdev, saeedm, bersoare, vladbu, kuba
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 1 (1) · Medium: 0 · Low: 0
- [High] Incomplete fix: the new port-presence filter is applied only to
the *first* representor returned by…
Pre-existing issues:
- [High] The NETDEV_CHANGEUPPER path was not converted to a port-presence
filter, so the same "instance does not own the port" condition still…
--
net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports
The mlx5 bridge offload registers its switchdev notifiers once per eswitch
instance while the switchdev chains are global, so every instance sees every
event and filters it with mlx5_esw_bridge_dev_same_hw(). This patch adds
mlx5_esw_bridge_port_exists() and makes the port object and port attribute
handlers filter on the presence of the port in the instance instead, so an
instance that does not track the port no longer claims the event and returns
-EINVAL.
No Link: tags in the commit header.
> 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);
> +}
> +
[Severity: High]
In mlx5_esw_bridge_lower_rep_port_lookup(), the port presence test runs only
on the single representor that the traversal already picked, so should the
check instead be folded into the walk over the bridge's lower devices?
mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get() returns the first same-HCA
representor it finds:
netdev_for_each_lower_dev(dev, lower_dev, iter) {
...
rep = mlx5_esw_bridge_lower_rep_vport_num_vhca_id_get(lower_dev, esw, vport_num,
esw_owner_vhca_id);
if (rep)
return rep;
}
Lower adjacencies are appended in enslavement order by
__netdev_adjacent_dev_insert() (list_add_tail_rcu() for the down list), so in
the very scenario from the commit message - PF0's uplink rep enslaved to br0
first, then PF1 put in switchdev mode and its rep enslaved to the same br0 -
the first lower rep is PF0's uplink. PF1's br_offloads instance has no port
for it, so mlx5_esw_bridge_port_exists() is false and the new helper returns
false, even though PF1's instance does own the local port for its own rep on
that same bridge.
Since the remaining lower devices are never examined, does
mlx5_esw_bridge_port_obj_attr_set() then skip the attribute for PF1's own
bridge object?
> @@ -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;
[Severity: High]
The bridge-global attributes are emitted against the bridge master netdev,
for example br_vlan_filter_toggle() calls switchdev_port_attr_set(br->dev,
&attr, extack), and __set_ageing_time() and the MC_DISABLED path do the same.
That is why the driver resolves a vport through the lower-device walk here.
With this change, returning 0 leaves handled false, so the sibling instance
applies nothing to its own struct mlx5_esw_bridge, which
mlx5_esw_bridge_create() initialises with hard-coded defaults:
bridge->ageing_time = clock_t_to_jiffies(BR_DEFAULT_AGEING_TIME);
bridge->vlan_proto = ETH_P_8021Q;
and a zeroed flags field, i.e. no MLX5_ESW_BRIDGE_VLAN_FILTERING_FLAG and
mcast off.
mlx5 never calls switchdev_bridge_port_offload() (no occurrences under
drivers/net/ethernet/mellanox/mlx5/), so br_switchdev_vlan_attr_replay() does
not run for it and the skipped attribute is never recovered.
Before this patch the same case returned -EINVAL and br_vlan_filter_toggle()
/ __br_vlan_set_proto() rolled the software state back, so software and
hardware stayed in agreement. Afterwards, does
ip link set br0 type bridge vlan_filtering 1
succeed in software and on the first instance while the sibling instance keeps
offloading its own ports with vlan filtering disabled, the default ageing time
and ETH_P_8021Q, i.e. a divergence that depends on enslavement order?
Note that all four setters resolve the per-instance bridge object through a
port owned by that instance, e.g. 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 each registered instance needs to receive the attribute for its own
offload state to match the software bridge.
[ ... ]
> 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);
> +}
> +
[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but should
the new port-presence filter also be applied to the NETDEV_CHANGEUPPER path,
which hits the same "instance does not own the port" condition?
mlx5_esw_bridge_port_changeupper() still filters only with the same-HCA test:
rep = mlx5_esw_bridge_rep_vport_num_vhca_id_get(dev, esw, &vport_num, &esw_owner_vhca_id);
if (!rep)
return 0;
...
else if (mlx5_esw_bridge_dev_same_hw(rep, esw))
err = info->linking ?
mlx5_esw_bridge_vport_peer_link(...) :
mlx5_esw_bridge_vport_peer_unlink(upper, vport_num, esw_owner_vhca_id,
br_offloads, extack);
Unlike mlx5_esw_bridge_vport_peer_link(), mlx5_esw_bridge_vport_peer_unlink()
has no merged_eswitch / port-absent early return and goes straight to
mlx5_esw_bridge_vport_unlink(), which returns -EINVAL:
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;
}
mlx5_esw_bridge_switchdev_port_event() turns that into
notifier_from_errno(-EINVAL), which sets NOTIFY_STOP_MASK. The notifier is
registered with register_netdevice_notifier_net(&init_net,
&br_offloads->netdev_nb), so the stop verdict on the per-netns chain also
suppresses the global chain in 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);
Does one un-enslave of a sibling PF's representor then drop
NETDEV_CHANGEUPPER for every global listener, including rtnetlink_event(),
fib_netdev_event(), addrconf_notify() and dsa_user_netdevice_event(), plus any
per-netns block registered after this one (a third PF's bridge-offload
instance would keep a stale peer port and FDB entries pointing at a vport that
has left the bridge)?
__netdev_upper_dev_unlink() discards the verdict:
call_netdevice_notifiers_info(NETDEV_CHANGEUPPER,
&changeupper_info.info);
so nothing is reported to user space.
The trigger is the same one the commit message describes: the last-registered
instance never saw the CHANGEUPPER that would have created the peer port, then
ip link set <earlier PF's rep> nomaster hits the -EINVAL. It also looks
reachable on HCAs without merged_eswitch, where
mlx5_esw_bridge_vport_peer_link() returns 0 without creating a port while the
unlink side still fails.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/f26aa63abf085a087f7ed1df9080f9f0982bb311.1788884253.git.daniel%40iogearbox.net
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-11 7:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08 16:19 [PATCH net] net/mlx5: Bridge, don't fail switchdev events of sibling eswitch ports Daniel Borkmann
2026-09-10 12:47 ` Mark Bloch
-- strict thread matches above, loose matches on Subject: below --
2026-09-11 7:21 netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox