Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Geethasowjanya Akula <gakula@marvell.com>
To:     Saeed Mahameed <saeed@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "Subbaraya Sundeep Bhatta" <sbhatta@marvell.com>
Subject: Re: [EXT] Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group
 support
Thread-Topic: [EXT] Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group
 support
Thread-Index: AQHWzk4d9K8Ye5uitUy6hna3D0lwvKnvILkAgAFgzQI=
Date:   Thu, 10 Dec 2020 16:12:12 +0000
Message-ID: <DM6PR18MB26028B727E26DAFBA2D711E5CDCB0@DM6PR18MB2602.namprd18.prod.outlook.com>
References: <20201209170937.19548-1-gakula@marvell.com>,<f47444311bc7661c6482de11d570fb815f8e7941.camel@kernel.org>
In-Reply-To: <f47444311bc7661c6482de11d570fb815f8e7941.camel@kernel.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 97ebdbce-3467-40cb-1a30-08d89d265a9d
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Dec 2020 16:12:12.6589
 (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: TmxtcvkrV55QOvGewCZ6IEyshBWMPs8l3mxtxY216RF+ew26bWaP8RZk+1+L/bG8FKFzrwZKSLjGyep0Hbeh9g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR18MB3585
X-OriginatorOrg: marvell.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737
 definitions=2020-12-10_06:2020-12-09,2020-12-10 signatures=0
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Thanks Saeed for the feedback. Will address your comments in next version.


________________________________________
From: Saeed Mahameed <saeed@kernel.org>
Sent: Thursday, December 10, 2020 12:38 AM
To: Geethasowjanya Akula; netdev@vger.kernel.org; linux-kernel@vger.kernel.=
org
Cc: Sunil Kovvuri Goutham; davem@davemloft.net; kuba@kernel.org; Subbaraya =
Sundeep Bhatta
Subject: [EXT] Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group sup=
port

External Email

----------------------------------------------------------------------
On Wed, 2020-12-09 at 22:39 +0530, Geetha sowjanya wrote:
> Hardware supports 8 RSS groups per interface. Currently we are using
> only group '0'. This patch allows user to create new RSS
> groups/contexts
> and use the same as destination for flow steering rules.
>
> usage:
> To steer the traffic to RQ 2,3
>
> ethtool -X eth0 weight 0 0 1 1 context new
> (It will print the allocated context id number)
> New RSS context is 1
>
> ethtool -N eth0 flow-type tcp4 dst-port 80 context 1 loc 1
>
> To delete the context
> ethtool -X eth0 context 1 delete
>
> When an RSS context is removed, the active classification
> rules using this context are also removed.
>
> Change-log:
> v2
> - Removed unrelated whitespace
> - Coverted otx2_get_rxfh() to use new function.
>
> v3
> - Coverted otx2_set_rxfh() to use new function.
>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> ---

...

> -/* Configure RSS table and hash key */
> -static int otx2_set_rxfh(struct net_device *dev, const u32 *indir,
> -                      const u8 *hkey, const u8 hfunc)
> +static int otx2_get_rxfh_context(struct net_device *dev, u32 *indir,
> +                              u8 *hkey, u8 *hfunc, u32 rss_context)
>  {
>       struct otx2_nic *pfvf =3D netdev_priv(dev);
> +     struct otx2_rss_ctx *rss_ctx;
>       struct otx2_rss_info *rss;
>       int idx;
>
> -     if (hfunc !=3D ETH_RSS_HASH_NO_CHANGE && hfunc !=3D
> ETH_RSS_HASH_TOP)
> -             return -EOPNOTSUPP;
> -
>       rss =3D &pfvf->hw.rss_info;
>
>       if (!rss->enable) {
> -             netdev_err(dev, "RSS is disabled, cannot change
> settings\n");
> +             netdev_err(dev, "RSS is disabled\n");
>               return -EIO;
>       }

I see that you init/enable rss on open, is this is your way to block
getting rss info if device is not open ? why do you need to report an
error anyway, why not just report whatever default config you will be
setting up on next open ?

to me reporting errors to ethtool queries when device is down is a bad
user experience.

> +     if (rss_context >=3D MAX_RSS_GROUPS)
> +             return -EINVAL;
> +

-ENOENT
> +     rss_ctx =3D rss->rss_ctx[rss_context];
> +     if (!rss_ctx)
> +             return -EINVAL;
>

-ENOENT