netdev - RE: [PATCH v5 net-next 0/2] patchset: 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: <BF3270C86E8B1349A26C34E4EC1C44CB2C8968A5@CMEXMB1.ad.emulex.com>
Date:	Thu, 20 Mar 2014 12:02:11 +0000
From:	Venkata Duvvuru <VenkatKumar.Duvvuru@...lex.Com>
To:	David Miller <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Ben Hutchings (ben@...adent.org.uk)" <ben@...adent.org.uk>
Subject: RE: [PATCH v5 net-next 0/2] patchset: Support for configurable RSS
 hash key

> > NIC drivers that support RSS use either a hard-coded value or a random
> > value for the RSS hash key. Irrespective of the type of the key used,
> > the user would want to change the hash key if he/she is not satisfied
> > with the effectiveness of the default hash-key in spreading the incoming flows
> evenly across the RSS queues.
> >
> > This patch set adds support for configuring the RSS hash-key via the
> > ethtool interface using -X option.
> 
> I apologize, but I really dislike this.  For several reasons.
> 
> First, why aren't we adding _just_ a RSS hash changing interface?
> 
> We already have an interface for changing the indirection table, there is
> absolutely not need to add a second interface that supports both indirection
> table _plus_ hash modifications.
> 
> And combining these two is what leads to this hard to audit, ugly, data structure
> layout.
> 
> There's the indirection table at some offset, then the key at some other offset.
> This makes it impossible to impose type checking of any kind on both objects.
> 
> And the data structure is furthermore named in a way that suggests it's just for
> doing things with the RSS hash.
> 
> Just add:
> 
> struct ethtool_rxfh {
> 	__u32	cmd;
> 	__u32	key_size;
> 	__u32	key[0];
> };
> 
> And that's it.
> 
> To change the indirection table, call "ETHTOOL_SRXFH".  To set the RSS flow
> hash, call "ETHTOOL_SRSSH".

My original patch was exactly the way you suggested.
It went through many changes later with the intention of combining both indirection and hash key setting into one option as both are related to rss configuration.
--
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