Re: [PATCH net-next 1/7] net: ethtool: prevent flow steering to RSS contexts which don't exist

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 9 Feb 2025 10:17:30 +0200 Gal Pressman wrote:
> >  	/* Nonzero ring with RSS only makes sense if NIC adds them together */  
> 
> This comment should be moved inside the if statement.

Will follow up, sorry.

> > -	if (cmd == ETHTOOL_SRXCLSRLINS && info.fs.flow_type & FLOW_RSS &&
> > -	    !ops->cap_rss_rxnfc_adds &&
> > -	    ethtool_get_flow_spec_ring(info.fs.ring_cookie))
> > -		return -EINVAL;
> > +	if (cmd == ETHTOOL_SRXCLSRLINS && info.fs.flow_type & FLOW_RSS) {
> > +		if (!ops->cap_rss_rxnfc_adds &&
> > +		    ethtool_get_flow_spec_ring(info.fs.ring_cookie))
> > +			return -EINVAL;
> > +
> > +		if (!xa_load(&dev->ethtool->rss_ctx, info.rss_context))  
> 
> Accessing rss_ctx without rss_lock?

Yes, same as ethtool_get_max_rxnfc_channel(). Since we'd have to drop
the lock instantly after the check the whole rule addition wouldn't 
be atomic under that lock, anyway. IOW the xa_load() access is safe
in itself, and I couldn't think of practical use for taking the rss
lock.