From: "Matt Carlson" <mcarlson@broadcom.com>
To: "Ben Hutchings" <bhutchings@solarflare.com>
Cc: "Matthew Carlson" <mcarlson@broadcom.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"Michael Chan" <mchan@broadcom.com>
Subject: Re: [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin configurable
Date: Thu, 15 Dec 2011 15:37:12 -0800 [thread overview]
Message-ID: <20111215233712.GA7371@mcarlson.broadcom.com> (raw)
In-Reply-To: <1323983627.2773.18.camel@bwh-desktop>
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.
next prev parent reply other threads:[~2011-12-15 23:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-14 21:10 [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin configurable Matt Carlson
2011-12-14 21:50 ` Ben Hutchings
2011-12-15 21:03 ` Matt Carlson
2011-12-15 21:13 ` Ben Hutchings
2011-12-15 23:37 ` Matt Carlson [this message]
2011-12-15 23:43 ` Ben Hutchings
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111215233712.GA7371@mcarlson.broadcom.com \
--to=mcarlson@broadcom.com \
--cc=bhutchings@solarflare.com \
--cc=davem@davemloft.net \
--cc=mchan@broadcom.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).