[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AAEA33E297BCAC4B9BB20A7C2DF0AB8D65578A60@FMSMSX113.amr.corp.intel.com>
Date: Wed, 26 Mar 2014 19:21:51 +0000
From: "Williams, Mitch A" <mitch.a.williams@...el.com>
To: Ben Hutchings <ben@...adent.org.uk>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"gospo@...hat.com" <gospo@...hat.com>,
"sassmann@...hat.com" <sassmann@...hat.com>,
"Sullivan, Catherine" <catherine.sullivan@...el.com>
Subject: RE: [net-next 03/16] i40evf: Support RSS option in ethtool
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@...adent.org.uk]
> Sent: Monday, March 17, 2014 11:54 AM
> To: Kirsher, Jeffrey T
> Cc: davem@...emloft.net; Williams, Mitch A; netdev@...r.kernel.org;
> gospo@...hat.com; sassmann@...hat.com; Sullivan, Catherine
> Subject: Re: [net-next 03/16] i40evf: Support RSS option in ethtool
>
> On Mon, 2014-03-17 at 05:45 -0700, Jeff Kirsher wrote:
> > From: Mitch Williams <mitch.a.williams@...el.com>
> >
> > Add support for viewing and modifying RSS hash options and RSS hash
> > look-up table programming through ethtool. Because the lookup table is
> > so small for the VFs (only 16 registers), we don't bother to maintain a
> > shadow table in memory, we just read and write the registers directly.
> [...]
> > +static int i40evf_get_rss_hash_opts(struct i40evf_adapter *adapter,
> > + struct ethtool_rxnfc *cmd)
> > +{
> > + cmd->data = 0;
> > +
> > + /* Report default options for RSS on i40e */
>
> But you're allowing them to be changed from the defaults, so this is
> wrong.
Ben, I've got a question on this. Should the ETHTOOL_GRXFH option report what the driver and device are capable of, or should it report the current configuration?
If this option is supposed to report device capabilities, how can we find out the current configuration? And if it's supposed to report the current configuration, then how do we find out the device capabilities?
I've been wading through the kernel source, and I see both. So I'm confused.
Currently, I have the i40evf driver just reporting capabilities, because I mostly swiped the code from i40e, which (I think) was swiped from ixgbe. (Note to any laywers reading this: "swiped" is a euphemism for "legally copied and/or adapted in compliance with all appropriate software licenses.)
What's your interpretation of how this should work? I'm respinning this for submittal upstream, and now's the time to change it if it needs to be changed.
Thanks,
Mitch
Powered by blists - more mailing lists