linux-kernel - Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201209114845.61839f46@kicinski-fedora-pc1c0hjn.DHCP.thefacebook.com>
Date:   Wed, 9 Dec 2020 11:48:45 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Saeed Mahameed <saeed@...nel.org>
Cc:     Geetha sowjanya <gakula@...vell.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, sgoutham@...vell.com,
        davem@...emloft.net, sbhatta@...vell.com
Subject: Re: [PATCHv3 net-next] octeontx2-pf: Add RSS multi group support

On Wed, 09 Dec 2020 11:08:24 -0800 Saeed Mahameed wrote:
> > -/* 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

Plus looks like this version introduces a W=1 C=1 warning:

drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:768:28: warning: Using plain integer as NULL pointer