diff mbox series

[net-next,V4,1/4] sch_htb: Allow HTB priority parameter in offload mode

Message ID 20230210111051.13654-2-hkelam@marvell.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series octeontx2-pf: HTB offload support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 59 this patch: 59
netdev/cc_maintainers warning 3 maintainers not CCed: leon@kernel.org moshet@nvidia.com maximmi@nvidia.com
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 77 this patch: 77
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 53 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Hariprasad Kelam Feb. 10, 2023, 11:10 a.m. UTC 
From: Naveen Mamindlapalli <naveenm@marvell.com>

The current implementation of HTB offload returns the EINVAL error
for unsupported parameters like prio and quantum. This patch removes
the error returning checks for 'prio' parameter and populates its
value to tc_htb_qopt_offload structure such that driver can use the
same.

Add prio parameter check in mlx5 driver, as mlx5 devices are not capable
of supporting the prio parameter when htb offload is used. Report error
if prio parameter is set to a non-default value.

Signed-off-by: Naveen Mamindlapalli <naveenm@marvell.com>
Co-developed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/qos.c | 7 ++++++-
 include/net/pkt_cls.h                            | 1 +
 net/sched/sch_htb.c                              | 7 +++----
 3 files changed, 10 insertions(+), 5 deletions(-)

Comments

Simon Horman Feb. 10, 2023, 1:25 p.m. UTC | #1 
On Fri, Feb 10, 2023 at 04:40:48PM +0530, Hariprasad Kelam wrote:
> From: Naveen Mamindlapalli <naveenm@marvell.com>
> 
> The current implementation of HTB offload returns the EINVAL error
> for unsupported parameters like prio and quantum. This patch removes
> the error returning checks for 'prio' parameter and populates its
> value to tc_htb_qopt_offload structure such that driver can use the
> same.
> 
> Add prio parameter check in mlx5 driver, as mlx5 devices are not capable
> of supporting the prio parameter when htb offload is used. Report error
> if prio parameter is set to a non-default value.
> 
> Signed-off-by: Naveen Mamindlapalli <naveenm@marvell.com>
> Co-developed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/qos.c | 7 ++++++-
>  include/net/pkt_cls.h                            | 1 +
>  net/sched/sch_htb.c                              | 7 +++----
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> index 2842195ee548..b683dc787827 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> @@ -379,6 +379,12 @@ int mlx5e_htb_setup_tc(struct mlx5e_priv *priv, struct tc_htb_qopt_offload *htb_
>  	if (!htb && htb_qopt->command != TC_HTB_CREATE)
>  		return -EINVAL;
>  
> +	if (htb_qopt->prio) {
> +		NL_SET_ERR_MSG_MOD(htb_qopt->extack,
> +				   "prio parameter is not supported by device with HTB offload enabled.");
> +		return -EINVAL;

I think returning -EOPNOTSUPP would be more appropriate here.

> +	}
> +
>  	switch (htb_qopt->command) {
>  	case TC_HTB_CREATE:
>  		if (!mlx5_qos_is_supported(priv->mdev)) {
> @@ -515,4 +521,3 @@ int mlx5e_mqprio_rl_get_node_hw_id(struct mlx5e_mqprio_rl *rl, int tc, u32 *hw_i
>  	*hw_id = rl->leaves_id[tc];
>  	return 0;
>  }
> -

nit: This is a good cleanup. But not strictly related to this patch.

...
Hariprasad Kelam Feb. 12, 2023, 4:19 p.m. UTC | #2 
> On Fri, Feb 10, 2023 at 04:40:48PM +0530, Hariprasad Kelam wrote:
> > From: Naveen Mamindlapalli <naveenm@marvell.com>
> >
> > The current implementation of HTB offload returns the EINVAL error for
> > unsupported parameters like prio and quantum. This patch removes the
> > error returning checks for 'prio' parameter and populates its value to
> > tc_htb_qopt_offload structure such that driver can use the same.
> >
> > Add prio parameter check in mlx5 driver, as mlx5 devices are not
> > capable of supporting the prio parameter when htb offload is used.
> > Report error if prio parameter is set to a non-default value.
> >
> > Signed-off-by: Naveen Mamindlapalli <naveenm@marvell.com>
> > Co-developed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> > Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> > Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/en/qos.c | 7 ++++++-
> >  include/net/pkt_cls.h                            | 1 +
> >  net/sched/sch_htb.c                              | 7 +++----
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> > index 2842195ee548..b683dc787827 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
> > @@ -379,6 +379,12 @@ int mlx5e_htb_setup_tc(struct mlx5e_priv *priv,
> struct tc_htb_qopt_offload *htb_
> >  	if (!htb && htb_qopt->command != TC_HTB_CREATE)
> >  		return -EINVAL;
> >
> > +	if (htb_qopt->prio) {
> > +		NL_SET_ERR_MSG_MOD(htb_qopt->extack,
> > +				   "prio parameter is not supported by device
> with HTB offload enabled.");
> > +		return -EINVAL;
> 
> I think returning -EOPNOTSUPP would be more appropriate here.

ACK, will fix in next version.

Thanks,
Hariprasad k
> 
> > +	}
> > +
> >  	switch (htb_qopt->command) {
> >  	case TC_HTB_CREATE:
> >  		if (!mlx5_qos_is_supported(priv->mdev)) { @@ -515,4
> +521,3 @@ int
> > mlx5e_mqprio_rl_get_node_hw_id(struct mlx5e_mqprio_rl *rl, int tc, u32
> *hw_i
> >  	*hw_id = rl->leaves_id[tc];
> >  	return 0;
> >  }
> > -
> 
> nit: This is a good cleanup. But not strictly related to this patch.
> 
> ...
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
index 2842195ee548..b683dc787827 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/qos.c
@@ -379,6 +379,12 @@  int mlx5e_htb_setup_tc(struct mlx5e_priv *priv, struct tc_htb_qopt_offload *htb_
 	if (!htb && htb_qopt->command != TC_HTB_CREATE)
 		return -EINVAL;
 
+	if (htb_qopt->prio) {
+		NL_SET_ERR_MSG_MOD(htb_qopt->extack,
+				   "prio parameter is not supported by device with HTB offload enabled.");
+		return -EINVAL;
+	}
+
 	switch (htb_qopt->command) {
 	case TC_HTB_CREATE:
 		if (!mlx5_qos_is_supported(priv->mdev)) {
@@ -515,4 +521,3 @@  int mlx5e_mqprio_rl_get_node_hw_id(struct mlx5e_mqprio_rl *rl, int tc, u32 *hw_i
 	*hw_id = rl->leaves_id[tc];
 	return 0;
 }
-
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index cd410a87517b..65d50ed60b41 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -854,6 +854,7 @@  struct tc_htb_qopt_offload {
 	u16 qid;
 	u64 rate;
 	u64 ceil;
+	u8 prio;
 };
 
 #define TC_HTB_CLASSID_ROOT U32_MAX
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 92f2975b6a82..1cd9b48c96cd 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1814,10 +1814,6 @@  static int htb_change_class(struct Qdisc *sch, u32 classid,
 			NL_SET_ERR_MSG(extack, "HTB offload doesn't support the quantum parameter");
 			goto failure;
 		}
-		if (hopt->prio) {
-			NL_SET_ERR_MSG(extack, "HTB offload doesn't support the prio parameter");
-			goto failure;
-		}
 	}
 
 	/* Keeping backward compatible with rate_table based iproute2 tc */
@@ -1913,6 +1909,7 @@  static int htb_change_class(struct Qdisc *sch, u32 classid,
 					TC_HTB_CLASSID_ROOT,
 				.rate = max_t(u64, hopt->rate.rate, rate64),
 				.ceil = max_t(u64, hopt->ceil.rate, ceil64),
+				.prio = hopt->prio,
 				.extack = extack,
 			};
 			err = htb_offload(dev, &offload_opt);
@@ -1933,6 +1930,7 @@  static int htb_change_class(struct Qdisc *sch, u32 classid,
 					TC_H_MIN(parent->common.classid),
 				.rate = max_t(u64, hopt->rate.rate, rate64),
 				.ceil = max_t(u64, hopt->ceil.rate, ceil64),
+				.prio = hopt->prio,
 				.extack = extack,
 			};
 			err = htb_offload(dev, &offload_opt);
@@ -2018,6 +2016,7 @@  static int htb_change_class(struct Qdisc *sch, u32 classid,
 				.classid = cl->common.classid,
 				.rate = max_t(u64, hopt->rate.rate, rate64),
 				.ceil = max_t(u64, hopt->ceil.rate, ceil64),
+				.prio = hopt->prio,
 				.extack = extack,
 			};
 			err = htb_offload(dev, &offload_opt);