| Message ID | 20241014205300.193519-10-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/mlx5: Refactor esw QoS to support generalized operations | expand |
On Mon, Oct 14, 2024 at 11:52:54PM +0300, Tariq Toukan wrote: > From: Carolina Jubran <cjubran@nvidia.com> > > Remove the `enabled` flag from the `vport->qos` struct, as QoS now > relies solely on the `sched_node` pointer to determine whether QoS > features are in use. > > Currently, the vport `qos` struct consists only of the `sched_node`, > introducing an unnecessary two-level reference. However, the qos struct > is retained as it will be extended in future patches to support new QoS > features. > > Signed-off-by: Carolina Jubran <cjubran@nvidia.com> > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c | 13 ++++++------- > drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 2 -- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c ... > @@ -933,7 +932,7 @@ int mlx5_esw_qos_modify_vport_rate(struct mlx5_eswitch *esw, u16 vport_num, u32 > } > > esw_qos_lock(esw); > - if (!vport->qos.enabled) { > + if (!vport->qos.sched_node) { > /* Eswitch QoS wasn't enabled yet. Enable it and vport QoS. */ > err = esw_qos_vport_enable(vport, rate_mbps, vport->qos.sched_node->bw_share, NULL); Sorry, another nit from my side: If we get here then vport->qos.sched_node is NULL, but it is dereferenced on the line above. Flagged by Smatch. > } else { ...
On 15/10/2024 14:04, Simon Horman wrote: > On Mon, Oct 14, 2024 at 11:52:54PM +0300, Tariq Toukan wrote: >> From: Carolina Jubran <cjubran@nvidia.com> >> >> Remove the `enabled` flag from the `vport->qos` struct, as QoS now >> relies solely on the `sched_node` pointer to determine whether QoS >> features are in use. >> >> Currently, the vport `qos` struct consists only of the `sched_node`, >> introducing an unnecessary two-level reference. However, the qos struct >> is retained as it will be extended in future patches to support new QoS >> features. >> >> Signed-off-by: Carolina Jubran <cjubran@nvidia.com> >> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> >> Signed-off-by: Tariq Toukan <tariqt@nvidia.com> >> --- >> drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c | 13 ++++++------- >> drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 2 -- >> 2 files changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c > > ... > >> @@ -933,7 +932,7 @@ int mlx5_esw_qos_modify_vport_rate(struct mlx5_eswitch *esw, u16 vport_num, u32 >> } >> >> esw_qos_lock(esw); >> - if (!vport->qos.enabled) { >> + if (!vport->qos.sched_node) { >> /* Eswitch QoS wasn't enabled yet. Enable it and vport QoS. */ >> err = esw_qos_vport_enable(vport, rate_mbps, vport->qos.sched_node->bw_share, NULL); > > Sorry, another nit from my side: > > If we get here then vport->qos.sched_node is NULL, > but it is dereferenced on the line above. > > Flagged by Smatch. > I should definitely check why I'm not getting these ones flagged. Will fix. Thanks. >> } else { > > ... >
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c index a665b8990dda..b36fbaf8ead0 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c @@ -742,7 +742,7 @@ static int esw_qos_vport_enable(struct mlx5_vport *vport, int err; esw_assert_qos_lock_held(esw); - if (vport->qos.enabled) + if (vport->qos.sched_node) return 0; err = esw_qos_get(esw, extack); @@ -761,7 +761,6 @@ static int esw_qos_vport_enable(struct mlx5_vport *vport, goto err_alloc; } - vport->qos.enabled = true; vport->qos.sched_node->vport = vport; trace_mlx5_esw_vport_qos_create(vport->dev, vport, bw_share, max_rate); @@ -787,9 +786,9 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport) lockdep_assert_held(&esw->state_lock); esw_qos_lock(esw); - if (!vport->qos.enabled) - goto unlock; vport_node = vport->qos.sched_node; + if (!vport_node) + goto unlock; WARN(vport_node->parent != esw->qos.node0, "Disabling QoS on port before detaching it from node"); @@ -836,7 +835,7 @@ bool mlx5_esw_qos_get_vport_rate(struct mlx5_vport *vport, u32 *max_rate, u32 *m bool enabled; esw_qos_lock(esw); - enabled = vport->qos.enabled; + enabled = !!vport->qos.sched_node; if (enabled) { *max_rate = vport->qos.sched_node->max_rate; *min_rate = vport->qos.sched_node->min_rate; @@ -933,7 +932,7 @@ int mlx5_esw_qos_modify_vport_rate(struct mlx5_eswitch *esw, u16 vport_num, u32 } esw_qos_lock(esw); - if (!vport->qos.enabled) { + if (!vport->qos.sched_node) { /* Eswitch QoS wasn't enabled yet. Enable it and vport QoS. */ err = esw_qos_vport_enable(vport, rate_mbps, vport->qos.sched_node->bw_share, NULL); } else { @@ -1142,7 +1141,7 @@ int mlx5_esw_qos_vport_update_node(struct mlx5_vport *vport, } esw_qos_lock(esw); - if (!vport->qos.enabled && !node) + if (!vport->qos.sched_node && !node) goto unlock; err = esw_qos_vport_enable(vport, 0, 0, extack); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h index e77ec82787de..14dd42d44e6f 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h @@ -214,8 +214,6 @@ struct mlx5_vport { /* Protected with the E-Switch qos domain lock. */ struct { - /* Initially false, set to true whenever any QoS features are used. */ - bool enabled; /* Vport scheduling element node. */ struct mlx5_esw_sched_node *sched_node; } qos;