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

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]

Date:	Thu, 15 Dec 2011 15:37:12 -0800
From:	"Matt Carlson" <mcarlson@...adcom.com>
To:	"Ben Hutchings" <bhutchings@...arflare.com>
cc:	"Matthew Carlson" <mcarlson@...adcom.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Michael Chan" <mchan@...adcom.com>
Subject: Re: [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin
 configurable

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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html