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

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]

Message-ID: <BF3270C86E8B1349A26C34E4EC1C44CB2C84D71E@CMEXMB1.ad.emulex.com>
Date:	Thu, 23 Jan 2014 13:47:52 +0000
From:	Venkata Duvvuru <VenkatKumar.Duvvuru@...lex.Com>
To:	Ben Hutchings <ben@...adent.org.uk>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash
 key.

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@...adent.org.uk]
> Sent: Thursday, January 23, 2014 11:09 AM
> To: Venkata Duvvuru
> Cc: netdev@...r.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.

Current structure:
struct ethtool_rxfh_indir {
        __u32   cmd;
        __u32   size;
        __u32   ring_index[0];
};

For example, 
1. You mentioned about having key as zero byte array, there is already a zero byte array “ring_index”. We can probably have a single zero byte array and allocate both indir + key memory. This  may not work as the old ethtool would not allocate memory for key and the newer kernel will try to interpret key which could be relatively junk or invalid addresses.
2. Even if we assume that key will be static array. We have to have the key field before the zero byte array “ring_index”. This also will not be compatible as we are introducing a field in the middle.

> 
> Ben.
> 
> --
> Ben Hutchings
> compatible: Gracefully accepts erroneous data from any source