[PATCH net-next V5 11/15] net/mlx5: Expose a function to clear a vport's parent

Tariq Toukan posted 15 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH net-next V5 11/15] net/mlx5: Expose a function to clear a vport's parent
Posted by Tariq Toukan 2 weeks, 5 days ago
From: Cosmin Ratiu <cratiu@nvidia.com>

Currently, clearing a vport's parent happens with a call that looks like
this:
	mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);

Change that to something nicer that looks like this:
	mlx5_esw_qos_vport_clear_parent(vport);

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/esw/devlink_port.c    |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c     | 11 +++++++++--
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.h     |  3 +--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 89a58dee50b3..31704ea9cdb4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -202,7 +202,7 @@ void mlx5_esw_offloads_devlink_port_unregister(struct mlx5_vport *vport)
 		return;
 	dl_port = vport->dl_port;
 
-	mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+	mlx5_esw_qos_vport_clear_parent(vport);
 	devl_rate_leaf_destroy(&dl_port->dl_port);
 
 	devl_port_unregister(&dl_port->dl_port);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
index 4278bcb04c72..8c3a026b8db4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
@@ -1896,8 +1896,10 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
 	return 0;
 }
 
-int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_sched_node *parent,
-				     struct netlink_ext_ack *extack)
+static int
+mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
+				 struct mlx5_esw_sched_node *parent,
+				 struct netlink_ext_ack *extack)
 {
 	struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
 	int err = 0;
@@ -1922,6 +1924,11 @@ int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_s
 	return err;
 }
 
+void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport)
+{
+	mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
+}
+
 int mlx5_esw_devlink_rate_leaf_parent_set(struct devlink_rate *devlink_rate,
 					  struct devlink_rate *parent,
 					  void *priv, void *parent_priv,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index ad1073f7b79f..20cf9dd542a1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -452,8 +452,7 @@ int mlx5_eswitch_set_vport_trust(struct mlx5_eswitch *esw,
 				 u16 vport_num, bool setting);
 int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, u16 vport,
 				u32 max_rate, u32 min_rate);
-int mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport, struct mlx5_esw_sched_node *node,
-				     struct netlink_ext_ack *extack);
+void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport);
 int mlx5_eswitch_set_vepa(struct mlx5_eswitch *esw, u8 setting);
 int mlx5_eswitch_get_vepa(struct mlx5_eswitch *esw, u8 *setting);
 int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw,
-- 
2.44.0
Re: [net-next,V5,11/15] net/mlx5: Expose a function to clear a vport's parent
Posted by Jakub Kicinski 2 weeks, 4 days ago
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/mlx5e: Skip ESN replay window setup for IPsec crypto offload

This patch fixes a WARN_ON being triggered for crypto offload by adding
a check to skip the replay window setup when the offload type is not
XFRM_DEV_OFFLOAD_PACKET.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> index a8fb4bec369c..63aa23f5c49e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> @@ -430,7 +430,8 @@ void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
>  		attrs->replay_esn.esn = sa_entry->esn_state.esn;
>  		attrs->replay_esn.esn_msb = sa_entry->esn_state.esn_msb;
>  		attrs->replay_esn.overlap = sa_entry->esn_state.overlap;
> -		if (attrs->dir == XFRM_DEV_OFFLOAD_OUT)
> +		if (attrs->dir == XFRM_DEV_OFFLOAD_OUT ||
> +		    attrs->type != XFRM_DEV_OFFLOAD_PACKET)
>  			goto skip_replay_window;

At this point in mlx5e_ipsec_build_accel_xfrm_attrs(), attrs->type has not
been initialized yet. The function starts with memset(attrs, 0, ...) which
sets attrs->type to 0 (XFRM_DEV_OFFLOAD_UNSPECIFIED).

The assignment attrs->type = x->xso.type happens later after the
skip_replay_window label. This means the condition attrs->type !=
XFRM_DEV_OFFLOAD_PACKET evaluates as 0 != 2 which is always true.

As a result, the replay window setup will be skipped for all inbound
configurations, not just crypto offload. Should this check use x->xso.type
instead of attrs->type, or should attrs->type be assigned earlier in the
function before this check?
Re: [net-next,V5,11/15] net/mlx5: Expose a function to clear a vport's parent
Posted by Jakub Kicinski 2 weeks, 4 days ago
On Wed, 21 Jan 2026 19:40:01 -0800 Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net/mlx5e: Skip ESN replay window setup for IPsec crypto offload
> 
> This patch fixes a WARN_ON being triggered for crypto offload by adding
> a check to skip the replay window setup when the offload type is not
> XFRM_DEV_OFFLOAD_PACKET.
> 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > index a8fb4bec369c..63aa23f5c49e 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> > @@ -430,7 +430,8 @@ void mlx5e_ipsec_build_accel_xfrm_attrs(struct mlx5e_ipsec_sa_entry *sa_entry,
> >  		attrs->replay_esn.esn = sa_entry->esn_state.esn;
> >  		attrs->replay_esn.esn_msb = sa_entry->esn_state.esn_msb;
> >  		attrs->replay_esn.overlap = sa_entry->esn_state.overlap;
> > -		if (attrs->dir == XFRM_DEV_OFFLOAD_OUT)
> > +		if (attrs->dir == XFRM_DEV_OFFLOAD_OUT ||
> > +		    attrs->type != XFRM_DEV_OFFLOAD_PACKET)
> >  			goto skip_replay_window;  
> 
> At this point in mlx5e_ipsec_build_accel_xfrm_attrs(), attrs->type has not
> been initialized yet. The function starts with memset(attrs, 0, ...) which
> sets attrs->type to 0 (XFRM_DEV_OFFLOAD_UNSPECIFIED).
> 
> The assignment attrs->type = x->xso.type happens later after the
> skip_replay_window label. This means the condition attrs->type !=
> XFRM_DEV_OFFLOAD_PACKET evaluates as 0 != 2 which is always true.
> 
> As a result, the replay window setup will be skipped for all inbound
> configurations, not just crypto offload. Should this check use x->xso.type
> instead of attrs->type, or should attrs->type be assigned earlier in the
> function before this check?

Herm, something misfired here, this is a review for the series of fixes
posted on the same day, not for the rate scheduling.