Thanks Saeed for the feedback. Will address your comments in next version.
________________________________________
From: Saeed Mahameed <sa...@kernel.org>
Sent: Thursday, December 10, 2020 12:38 AM
To: Geethasowjanya Akula; netdev@vger.kernel.org; linux-ker...@vger.kernel.org
Cc: Sunil Kovvuri Goutham; da...@davemloft.net; k...@kernel.org; Subbaraya
Sundeep Bhatta
Subject: [EXT] Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group support
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 <sgout...@marvell.com>
> Signed-off-by: Geetha sowjanya <gak...@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 = netdev_priv(dev);
> + struct otx2_rss_ctx *rss_ctx;
> struct otx2_rss_info *rss;
> int idx;
>
> - if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc !=
> ETH_RSS_HASH_TOP)
> - return -EOPNOTSUPP;
> -
> rss = &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 >= MAX_RSS_GROUPS)
> + return -EINVAL;
> +
-ENOENT
> + rss_ctx = rss->rss_ctx[rss_context];
> + if (!rss_ctx)
> + return -EINVAL;
>
-ENOENT
