Date:   Fri, 10 Feb 2023 14:25:54 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Hariprasad Kelam <hkelam@marvell.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        kuba@kernel.org, davem@davemloft.net, pabeni@redhat.com,
        edumazet@google.com, sgoutham@marvell.com, lcherian@marvell.com,
        gakula@marvell.com, jerinj@marvell.com, sbhatta@marvell.com,
        jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
        saeedm@nvidia.com, richardcochran@gmail.com, tariqt@nvidia.com,
        linux-rdma@vger.kernel.org, maxtram95@gmail.com,
        naveenm@marvell.com, bpf@vger.kernel.org,
        hariprasad.netdev@gmail.com
Subject: Re: [net-next Patch V4 1/4] sch_htb: Allow HTB priority parameter in
 offload mode
Message-ID: <Y+ZF4k8+CidOG75r@corigine.com>
References: <20230210111051.13654-1-hkelam@marvell.com>
 <20230210111051.13654-2-hkelam@marvell.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230210111051.13654-2-hkelam@marvell.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?iUkhd0lKzruM9MZOTRjXqmTtN59L8oM5yD5tVjJ09erYPmmWnySJYxBundog?=
 =?us-ascii?Q?/jByiXpUMLG/noOHP2obyeXf1SjzsaJ/9jd9m77LWMA8Mbl8/Wb/bc27O5fx?=
 =?us-ascii?Q?nhMtDnlJYL1NESI+fUUSGxKIWmkLK1mEOgbPBa12y2LPM0oRrVWzodfGJPL+?=
 =?us-ascii?Q?jthNV011kbv/2OZvfTyL9f+dF9X5yUFkXDnex0s4YtAWrewyC/ulHx6Mb8Fq?=
 =?us-ascii?Q?mOsYm6Qi8r8Nb4fMUVd4j8DTlhKv6D9fMPVrsgpzXFoKzdm2AGCnE1JkfLle?=
 =?us-ascii?Q?rqqt8b3atFDQgqELjpR7E2a/dMumKfh9s/NJWw/xoMDDjfutGgJwNLIjHRDD?=
 =?us-ascii?Q?kiCppe0osdT52tWRwH5/Q8VnFWvZ/oTQnlewgaYVY9hFdqU+oIJS/ZhCHkrT?=
 =?us-ascii?Q?bLamRjIn4SrfOqMl/Qwl185bSQNOpH2+Yqxsdv1J/t/ELvChmLQtq5fLHfRS?=
 =?us-ascii?Q?fo/HxTY0FX7fzx0n7p8aqD01xvMQhqGVtvAp1kxPlJM6lm2Bdzhd7NSwZo1V?=
 =?us-ascii?Q?y/7RnF3lwdDDGMYplt92mmUJkAujhYxBK9Y8Y1L0t9vGzIrlsV4f6ZPVhpBq?=
 =?us-ascii?Q?UV7QvCjvfQrWs9jeaWtFjiPIxQtym6dfvHVc80uBKCvip+1s9F7hS4griQ1j?=
 =?us-ascii?Q?e0E9B5lFXMCvdgN2A6MxdSdakjdvuRvrigojs8Rjy4mPG0Rvvregnt2Y1fBa?=
 =?us-ascii?Q?5YMdXmWuWon9f2b5XCNqglFD4mH7P8XujWDEzybLoORsFh82vAOuYyW46GmY?=
 =?us-ascii?Q?twSP/QiQpmQkpfZ3UXbbhtQjS67XdKuIxr4tl9+74EV6csjsmFE+InvnFvvy?=
 =?us-ascii?Q?ehExT5WSC+N3k0Osa4iJqjvItVKvrZavM0CylsaFyMcoZzG9mU56zsQma19x?=
 =?us-ascii?Q?grNI3ucolrXoP0SUD3cSe6rpX4oaiIexkoh8AIUJJrHnD8V1cqYbMGD5vqrr?=
 =?us-ascii?Q?SP+6R+6ZrVAp8C0xrCnifAYOsaLgjIhcB/uxeK6RCOsjmNAxq5J9ayfnqOuA?=
 =?us-ascii?Q?Ml9XbTkCTVXPGVQrKtxG1a0+xvZgss80HFKjI4zohEFEhQ3w/4LZVHsp7n1c?=
 =?us-ascii?Q?uCFEq8HJsOf0iI0UeF4XUzjawCdgjAIe0SXcHh8awcHz6+w05qoxf/DjzaYo?=
 =?us-ascii?Q?dvlfqFLwnCcT4WSn0AyyZ16b5o5LX+W+hOcPQdL9jaTLsZ+ZEaKaA+zDcgEL?=
 =?us-ascii?Q?aJnEpL4arOrMh7ERica/5oId63KQPOpqftJgg9qh8SMpbvmHRqd/ss3PGkrn?=
 =?us-ascii?Q?mwlYgOfmdz81kX8A+jz0s2hIg7CXry55wwL0ee8kum5Hm9iOFnjdgAHseTZa?=
 =?us-ascii?Q?oSjzz+riXNFGjKZY8vQoaWFiWvsqTEJkUGOVVGwZBcBBoHk07TsK00S0cBYb?=
 =?us-ascii?Q?C3+FZaHw4jcT8BE5l4RLh2DcKo0tpvO0tLowl+ST8iWTF0cSR1vNZR/SNDyo?=
 =?us-ascii?Q?KTr1/lDraW604I8eTEwxRZ9XJRIdOSgFSgd1Mtf8ukkZhCqdUXROccEtoCIU?=
 =?us-ascii?Q?pCJ7hkRYofwfkUeb1iH4Dxvt+b95vzJBk8hboLwsLJYGfEW+8Dx33CA2t5Xz?=
 =?us-ascii?Q?dsL2+vsKcXXXH1BeTrh5FKLEVJdf/8X9m+RbzFTcH+mby+Vld4M83q43EZZG?=
 =?us-ascii?Q?OZDg+/3Le4ekt6NpQe0ewPd7GyJU6ISl2nxUmzqDXuLLrvGfi91vE/KTkhvc?=
 =?us-ascii?Q?GU7R8A=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9f5c85bb-23e1-43ca-2a0f-08db0b6a5a69
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2023 13:26:01.7462
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: EMQUR7YCfNgH24mGZnWh6ANd7Rsm4mplFNZfhTqwCafZTOrdByE4RwSvELofcepetla9LtAs3+kieWktaQx0r+yQmCyCw/lmmtXmoI7zx8U=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR13MB5594
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 != 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.

...