Subject: Re: [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin configurable

From: "Matt Carlson" <[email-masked]>
Date: Thu, 15 Dec 2011 15:37:12 -0800

On Thu, Dec 15, 2011 at 01:13:47PM -0800, Ben Hutchings wrote:
> On Thu, 2011-12-15 at 13:03 -0800, Matt Carlson wrote:
> > On Wed, Dec 14, 2011 at 01:50:59PM -0800, Ben Hutchings wrote:
> > > On Wed, 2011-12-14 at 13:10 -0800, Matt Carlson wrote:
> [...]
> > > > +	if (!indir->size) {
> > > > +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> > > > +		return -EINVAL;
> > > 
> > > This is enough to make the ethtool command work, but you're really
> > > supposed to copy min(indir->size, TG3_RSS_INDIR_TBL_SIZE) entries.
> > 
> > Could you elaborate on this?  I'm confused because I can't figure out
> > how returning half of an indirection table could be useful.
> 
> It's a generalisation of the zero-length and full-length cases.  But no,
> it isn't very useful, nor did I actually specify that anywhere!
> 
> Maybe there should be a driver operation to get the table size, and then
> the core can make sure that drivers only ever deal with full-table
> buffers.  Though that wouldn't cover your reset-to-default case.

That's how your current implementation of ethtool works, doesn't it?  It
first makes a call to get_rxfh_indir() with a zero size parameter.  The
driver interprets this as a query for the size of the driver's
indirection table.  Subsequent calls to get_rxfh_indir() with a non-zero
size are interpreted as a request for the indirection table data.

Right now, I coded get_rxfh_indir() and set_rxfh_indir() to be strict in
what they accept.  The restrictions can be relaxed though if we can
outline what the driver should do with partial tables.  (I wonder if
these types of interpretations are better placed above the driver
though.  Then again, maybe not.  Tg3 devices can tune the size of the
indirection table by powers of two if needed at the cost of a reset.)

But IMO all these ideas make the reset-to-default case more desirable.