Re: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash key.

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 3364 bytes --]

On Fri, 2014-01-24 at 12:00 +0000, Venkata Duvvuru wrote:
> 
> > -----Original Message-----
> > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > Sent: Thursday, January 23, 2014 8:39 PM
> > To: Venkata Duvvuru
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash
> > key.
> > 
> > On Thu, 2014-01-23 at 13:47 +0000, Venkata Duvvuru wrote:
> > > > -----Original Message-----
> > > > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > > > Sent: Thursday, January 23, 2014 11:09 AM
> > > > To: Venkata Duvvuru
> > > > Cc: netdev@vger.kernel.org
> > > > Subject: Re: [PATCH net-next 3/4] ethtool: Support for configurable
> > > > RSS hash key.
> > > >
> > > > On Wed, 2014-01-22 at 12:12 +0000, Venkata Duvvuru wrote:
> > > > [...]
> > > > > > No, what I mean is:
> > > > > >
> > > > > > 1. An RX flow steering filter can specify use of RSS, in which
> > > > > > case the value looked up in the indirection is added to the
> > > > > > queue number specified in the filter.  This is not yet
> > > > > > controllable through RX NFC though there is room for extension
> > there.
> > > > > >
> > > > > > 2. Multi-function controllers need multiple RSS contexts (key +
> > > > > > indirection
> > > > > > table) to support independent use of RSS on each function.
> > > > > > But it may also be possible to allocate multiple contexts to a
> > > > > > single
> > > > function.
> > > > > > This could be useful in conjunction with 1.  But there would
> > > > > > need to be a way to allocate and configure extra contexts first.
> > > > > The proposed changes will be incremental so I think this can be
> > > > > done in a separate patch. Thoughts?
> > > >
> > > > The ethtool ABI (to userland) has to remain backward-compatible, and
> > > > it is preferable if we don't add lots of different structures for this.
> > > >
> > > > So please define the new command structure to include both the key
> > > > and indirection table, and some reserved space (documented as
> > > > 'userland must set to 0') for future extensions.
> > >
> > > I think it’s better to keep key and indirection table settings as
> > > different ethtool commands. We can probably add rss contexts (reserved
> > > space) to both the command structures.
> > > If we mix key and indirection table into one command structure then it
> > > will hamper the compatibility.
> > [...]
> > 
> > Right, there is no compatible way to extend struct ethtool_rxfh_indir.
> > I should have thought ahead when defining it!  But the new structure doesn't
> > need to have that problem.
> 
> If any one of the operations (key or indirection table) is not
> supported by the driver, should we silently ignore that operation and
> process the other supported operation or should we fail the command.

It should fail completely.

>  If we fail the command then we are mandating the drivers to implement
> both the operations.

So far as I know, all the multiqueue NICs that include a flow hash
indirection table do so as part of the Microsoft RSS specification,
which requires the hash key to be configurable as well.  Do you know of
any cases where only one of the two is configurable?

Ben.

-- 
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]