From: Ben Hutchings <redacted>
Date: Fri, 21 Mar 2014 14:33:13 +0000

On Wed, 2014-03-19 at 15:59 -0400, David Miller wrote:

quoted

From: Venkat Duvvuru <redacted>
Date: Mon, 17 Mar 2014 18:01:33 +0530

quoted

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.

That's what I asked for, because I see the hash key and indirection
table as being a single logical object (RSS context).

quoted

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.

[...]

Which is why I previously suggested making the ethtool core find the two
arrays and pass those into the driver operations.  We do that for other
ethtool operations that use even a single variable-length array.

Sure, we could hide the dirt in the generic ethtool code and pass
separated out pointers to the drivers.

But you have to recognize that the dirt will still exist in userspace
when it calls this stuff.

Nothing stops ethtool from providing a combined operation on the
command line, but that doesn't mean we necessarily have to have
a single kernel interface doing both thing.

I really still think that adding just a new key operation is the way
to go.