From:   Hariprasad Kelam <hkelam@marvell.com>
To:     Simon Horman <simon.horman@corigine.com>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "edumazet@google.com" <edumazet@google.com>,
        Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        Linu Cherian <lcherian@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        "jhs@mojatatu.com" <jhs@mojatatu.com>,
        "xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>,
        "jiri@resnulli.us" <jiri@resnulli.us>,
        "saeedm@nvidia.com" <saeedm@nvidia.com>,
        "richardcochran@gmail.com" <richardcochran@gmail.com>,
        "tariqt@nvidia.com" <tariqt@nvidia.com>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "maxtram95@gmail.com" <maxtram95@gmail.com>,
        Naveen Mamindlapalli <naveenm@marvell.com>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "hariprasad.netdev@gmail.com" <hariprasad.netdev@gmail.com>
Subject: Re: [net-next Patch V4 1/4] sch_htb: Allow HTB priority parameter in
 offload mode
Thread-Topic: [net-next Patch V4 1/4] sch_htb: Allow HTB priority parameter in
 offload mode
Thread-Index: AQHZPv3G3JptYvJTH06gQoPeAlW8cw==
Date:   Sun, 12 Feb 2023 16:19:26 +0000
Message-ID: <PH0PR18MB4474ACB4E8103A05AE160B17DEDC9@PH0PR18MB4474.namprd18.prod.outlook.com>
References: <20230210111051.13654-1-hkelam@marvell.com>
 <20230210111051.13654-2-hkelam@marvell.com> <Y+ZF4k8+CidOG75r@corigine.com>
In-Reply-To: <Y+ZF4k8+CidOG75r@corigine.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?03mECWzUi6FnPzbbDHynl/Aazu1S4dxY8WWbR8qgKpfeQiDd43hQpAw0Nurf?=
 =?us-ascii?Q?dLCgZ9fwe8WTN8rjjK3D8zG6QleVxbdKzd54AMa0ZQHNhTCp6dJ4u0UDS1P8?=
 =?us-ascii?Q?9s+heQ7w6XLKEf7RhTueFg9PdmkNeNpZUaUsHWf/jl9lV/JmICxTHM9foc/W?=
 =?us-ascii?Q?QCFyCvduHVtbaJkLxr+twam0c01B2DcYvksom16gIkixAK2TcNYLOVQc1szo?=
 =?us-ascii?Q?Wdj9sqDj0yiYvB5m1fDrVxafHiqRLSp6gAL6ucLMxys30XD5IHRGTtX0wvyy?=
 =?us-ascii?Q?NeSJAKP33EABrpWVzwYTC+grs/p+tx9kFbUpsveMuTRF94bOx8babTn+kl4N?=
 =?us-ascii?Q?puuuEJL+YaDQglSP7zhOkUr2uhh6racPWwJVBRgWL/uBF8tA6umpi49BjRqf?=
 =?us-ascii?Q?Sqctdq3Pzt5Erp92TH/WLBKDD2bUUh1rUQPfjT4H56Hz3dkDMjlL1sUr6Yyg?=
 =?us-ascii?Q?wrQ1sR6U/gJXOyi/st5ToMkySzjvgumeI3Rck0JIV1hx+sAuOkXEVTG49XhI?=
 =?us-ascii?Q?molcCFUPBpOppVpiihob15DT2odIfGdo5VP5tqQEOEO+DFYsShRTl01x1OXZ?=
 =?us-ascii?Q?8Vjs5iObfp+ZrdkE8cFOruV6sQ8SqTeQgoRvlVj61+CrJyq/rvb/1mwQuhNC?=
 =?us-ascii?Q?Q4gqnpALha8AL9iBIi72JsRMgDP6PdBSMXAX7EKPAubMZVw4ax3lRzRqC+er?=
 =?us-ascii?Q?jom07sN2INyQiZOBURJDEIDqeoU44Aal7P7uD3vR6MJZBTX76/lqBa5N+jml?=
 =?us-ascii?Q?nNJdqLP6ky2iIgDRLt/mMv08Xucdg2oeIpImDmT7xz/MQOrMDMR0eJhAZ8UW?=
 =?us-ascii?Q?isnSxnjSP1hpOiSP3UwfveBoVVO96vIMRRpAkPcHtPmwwkm+Uhbsi6YBy7Zi?=
 =?us-ascii?Q?ihu7B5nfgF/lvV33F3K1Tpt0U/ZEnFPqKl4NVa9tnPYL0u/P/IB/EemvM5VP?=
 =?us-ascii?Q?fzXcX4tMcswIz7pD2wBPKaA5/sYg2/oAdIi9W6ZFK9ITr85TdJffUbmgp5pj?=
 =?us-ascii?Q?KypUYJ7gYdFg4CqzTWQY7X5DbQle7bXdGuvG4PupMltUP0ISGYab3GRymtR5?=
 =?us-ascii?Q?8zF50jd250u2BRTjeVwZE8fEjXVs6gODuGZMt6LxCxtYWJ/EElA6vyLwdk1d?=
 =?us-ascii?Q?BCzSimIiVFsyxHyBvYO9Jx2iQZcIeuTD+2Iylp4n+4B5m/pasFQ6/SgcTfyJ?=
 =?us-ascii?Q?Vp4+X1b5rYc6MprDitq92hOxywu5uXTE8ft9kUSFxSVtNVav+kMpwfuxSmF5?=
 =?us-ascii?Q?sNTAmxlIzb2fzs3Yew7gf/qH8Vjsfyxf4DuMCdnNLxi8qN/xhjTv0DxKhAaF?=
 =?us-ascii?Q?hjsJbSXlWcLy4e1yOhv7v05ERddMbVzHqU4FY1EHa7pGyFFJwsjOCLJr7UDa?=
 =?us-ascii?Q?26T5nFuKy8S8rTLyX9HrCQMYzcctzVydo1zzp8vsehZ+4iC9WdCx4R9U1rWB?=
 =?us-ascii?Q?puoaUFjLUmmMgS3QoQOQ7uAVl7ZBFcjqgxVNkSzeXUQb2KrSaUd/c1sHA4yq?=
 =?us-ascii?Q?m0WUfKQYntO3PD6voIT+2j5pT2iVZgWLHGL9bW+b5L1MEe7Xvgp/I+tGz1zH?=
 =?us-ascii?Q?v/WcaSF3SrwPKd1N5PFRqZMQeBj8pFs11bgq3CiK?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB4474.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c67ce555-5063-466a-72dc-08db0d14e928
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Feb 2023 16:19:26.5150
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: K2G1QZXEZCohEsziO9gq6Cz6AG5bJg7ninGHEtEbOtrgDRI3VEgl6LI3Z8qlWZH9mTQQ4jQrSMPa0Cxi7dY5hQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR18MB5477
X-Proofpoint-GUID: s4BTVeZUj3Z0JU8LZs1yEeUe6hxmE-Em
X-Proofpoint-ORIG-GUID: s4BTVeZUj3Z0JU8LZs1yEeUe6hxmE-Em
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.170.22
 definitions=2023-02-12_06,2023-02-09_03,2023-02-09_01
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> 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 !=3D 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;
>=20
> I think returning -EOPNOTSUPP would be more appropriate here.

ACK, will fix in next version.

Thanks,
Hariprasad k
>=20
> > +	}
> > +
> >  	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 =3D rl->leaves_id[tc];
> >  	return 0;
> >  }
> > -
>=20
> nit: This is a good cleanup. But not strictly related to this patch.
>=20
> ...