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

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

[Venkata Duvvuru]

This ethtool patch primarily copies the ioctl command data structures from/to the User space and invokes the driver hook.

Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
---
 include/linux/ethtool.h      |    2 ++
 include/uapi/linux/ethtool.h |   19 +++++++++++++++++++
 net/core/ethtool.c           |   40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index c8e3e7e..2c30fd1 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -227,6 +227,8 @@ struct ethtool_ops {
 	int	(*get_rxnfc)(struct net_device *,
 			     struct ethtool_rxnfc *, u32 *rule_locs);
 	int	(*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
+	int     (*set_rsshkey) (struct net_device *, struct ethtool_rss_hkey *);
+	int     (*get_rsshkey) (struct net_device *, struct ethtool_rss_hkey *);
 	int	(*flash_device)(struct net_device *, struct ethtool_flash *);
 	int	(*reset)(struct net_device *, u32 *);
 	u32	(*get_rxfh_indir_size)(struct net_device *);
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index 38dbafa..f39d82f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -678,6 +678,22 @@ struct ethtool_rx_ntuple {
 	struct ethtool_rx_ntuple_flow_spec	fs;
 };
 
+
+/**
+ * struct ethtool_rss_hkey - command to set/get RSS hash key of the device.
+ * @cmd: Command number - %ETHTOOL_SET_RSS_HKEY/ETHTOOL_GET_RSS_HKEY
+ * @data: 40 or 16 byte rss hash key
+ * @data_len: rss hash key length
+ */
+
+#define RSS_HASH_KEY_LEN	40
+/* RSS Hash key */
+struct ethtool_rss_hkey {
+	__u32   cmd;            /* ETHTOOL_SET/GET_RSS_HKEY */
+	__u8    data[RSS_HASH_KEY_LEN];
+	__u32	data_len;
+};
+
 #define ETHTOOL_FLASH_MAX_FILENAME	128
 enum ethtool_flash_op_type {
 	ETHTOOL_FLASH_ALL_REGIONS	= 0,
@@ -901,6 +917,9 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_GEEE		0x00000044 /* Get EEE settings */
 #define ETHTOOL_SEEE		0x00000045 /* Set EEE settings */
 
+#define ETHTOOL_SET_RSS_HKEY	0x00000046 /* Set RSS hash key */
+#define ETHTOOL_GET_RSS_HKEY	0x00000047 /* Get RSS hash key */
+
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 30071de..70f68ff 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -501,6 +501,40 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
 	return 0;
 }
 
+static noinline_for_stack int ethtool_get_rsshkey(struct net_device *dev,
+						  void __user *useraddr)
+{
+	struct ethtool_rss_hkey info;
+	int rc;
+
+	if (!dev->ethtool_ops->get_rsshkey)
+		return -EOPNOTSUPP;
+
+	rc = dev->ethtool_ops->get_rsshkey(dev, &info);
+
+	if (copy_to_user(useraddr, &info, sizeof(info)))
+		rc = -EFAULT;
+
+	return rc;
+}
+
+static noinline_for_stack int ethtool_set_rsshkey(struct net_device *dev,
+						  void __user *useraddr)
+{
+	struct ethtool_rss_hkey info;
+	int rc;
+
+	if (!dev->ethtool_ops->set_rsshkey)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&info, useraddr, sizeof(info)))
+		return -EFAULT;
+
+	rc = dev->ethtool_ops->set_rsshkey(dev, &info);
+
+	return rc;
+}
+
 static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev,
 						u32 cmd, void __user *useraddr)
 {
@@ -1612,6 +1646,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXCLSRLINS:
 		rc = ethtool_set_rxnfc(dev, ethcmd, useraddr);
 		break;
+	case ETHTOOL_GET_RSS_HKEY:
+		rc = ethtool_get_rsshkey(dev, useraddr);
+		break;
+	case ETHTOOL_SET_RSS_HKEY:
+		rc = ethtool_set_rsshkey(dev, useraddr);
+		break;
 	case ETHTOOL_FLASHDEV:
 		rc = ethtool_flash_device(dev, useraddr);
 		break;
--
1.7.1

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

[Ben Hutchings]

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

On Fri, 2014-01-17 at 13:02 +0000, Venkata Duvvuru wrote:
> This ethtool patch primarily copies the ioctl command data structures from/to the User space and invokes the driver hook.
[...]
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -678,6 +678,22 @@ struct ethtool_rx_ntuple {
>  	struct ethtool_rx_ntuple_flow_spec	fs;
>  };
>  
> +
> +/**
> + * struct ethtool_rss_hkey - command to set/get RSS hash key of the device.
> + * @cmd: Command number - %ETHTOOL_SET_RSS_HKEY/ETHTOOL_GET_RSS_HKEY
> + * @data: 40 or 16 byte rss hash key
> + * @data_len: rss hash key length
> + */
> +
> +#define RSS_HASH_KEY_LEN	40

This should have an 'ETHTOOL_' or 'ETH_' prefix.  But I wonder whether
there should actually be a static maximum length.

> +/* RSS Hash key */
> +struct ethtool_rss_hkey {
> +	__u32   cmd;            /* ETHTOOL_SET/GET_RSS_HKEY */
> +	__u8    data[RSS_HASH_KEY_LEN];
> +	__u32	data_len;
> +};
[...]

How about putting data after the data_len and giving it a length of 0,
so this is extensible to an arbitrary length key?

If we're extending the RSS configuration interface, there are a few
other things that might be worth doing at the same time:

- Single commands to get/set both the key and the indirection table at
the same time
- Add a field to distinguish multiple RSS contexts (some hardware can
use RSS contexts together with filters, though RX NFC does not support
that yet)

Ben.

-- 
Ben Hutchings
friends: People who know you well, but like you anyway.

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

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

[Venkata Duvvuru]

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Sunday, January 19, 2014 11:57 PM
> To: Venkata Duvvuru
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash
> key.
> 
> On Fri, 2014-01-17 at 13:02 +0000, Venkata Duvvuru wrote:
> > This ethtool patch primarily copies the ioctl command data structures
> from/to the User space and invokes the driver hook.
> [...]
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -678,6 +678,22 @@ struct ethtool_rx_ntuple {
> >  	struct ethtool_rx_ntuple_flow_spec	fs;
> >  };
> >
> > +
> > +/**
> > + * struct ethtool_rss_hkey - command to set/get RSS hash key of the
> device.
> > + * @cmd: Command number -
> %ETHTOOL_SET_RSS_HKEY/ETHTOOL_GET_RSS_HKEY
> > + * @data: 40 or 16 byte rss hash key
> > + * @data_len: rss hash key length
> > + */
> > +
> > +#define RSS_HASH_KEY_LEN	40
> 
> This should have an 'ETHTOOL_' or 'ETH_' prefix.  But I wonder whether there
> should actually be a static maximum length.
> 
> > +/* RSS Hash key */
> > +struct ethtool_rss_hkey {
> > +	__u32   cmd;            /* ETHTOOL_SET/GET_RSS_HKEY */
> > +	__u8    data[RSS_HASH_KEY_LEN];
> > +	__u32	data_len;
> > +};
> [...]
> 
> How about putting data after the data_len and giving it a length of 0, so this is
> extensible to an arbitrary length key?
> 
> If we're extending the RSS configuration interface, there are a few other
> things that might be worth doing at the same time:
> 
> - Single commands to get/set both the key and the indirection table at the
> same time
> - Add a field to distinguish multiple RSS contexts (some hardware can use RSS
> contexts together with filters, though RX NFC does not support that yet)
Are you referring to the filter-id that is created at the time of config-nfc? Pls clarify.
> 
> Ben.
> 
> --
> Ben Hutchings
> friends: People who know you well, but like you anyway.

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

[Ben Hutchings]

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

On Mon, 2014-01-20 at 12:23 +0000, Venkata Duvvuru wrote:
[...]
> > > +/* RSS Hash key */
> > > +struct ethtool_rss_hkey {
> > > +	__u32   cmd;            /* ETHTOOL_SET/GET_RSS_HKEY */
> > > +	__u8    data[RSS_HASH_KEY_LEN];
> > > +	__u32	data_len;
> > > +};
> > [...]
> > 
> > How about putting data after the data_len and giving it a length of 0, so this is
> > extensible to an arbitrary length key?
> > 
> > If we're extending the RSS configuration interface, there are a few other
> > things that might be worth doing at the same time:
> > 
> > - Single commands to get/set both the key and the indirection table at the
> > same time
> > - Add a field to distinguish multiple RSS contexts (some hardware can use RSS
> > contexts together with filters, though RX NFC does not support that yet)
> Are you referring to the filter-id that is created at the time of config-nfc? Pls clarify.

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.

Ben.

-- 
Ben Hutchings
One of the nice things about standards is that there are so many of them.

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

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

[Venkata Duvvuru]

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Monday, January 20, 2014 6:50 PM
> To: Venkata Duvvuru
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash
> key.
> 
> On Mon, 2014-01-20 at 12:23 +0000, Venkata Duvvuru wrote:
> [...]
> > > > +/* RSS Hash key */
> > > > +struct ethtool_rss_hkey {
> > > > +	__u32   cmd;            /* ETHTOOL_SET/GET_RSS_HKEY */
> > > > +	__u8    data[RSS_HASH_KEY_LEN];
> > > > +	__u32	data_len;
> > > > +};
> > > [...]
> > >
> > > How about putting data after the data_len and giving it a length of
> > > 0, so this is extensible to an arbitrary length key?
> > >
> > > If we're extending the RSS configuration interface, there are a few
> > > other things that might be worth doing at the same time:
> > >
> > > - Single commands to get/set both the key and the indirection table
> > > at the same time
> > > - Add a field to distinguish multiple RSS contexts (some hardware
> > > can use RSS contexts together with filters, though RX NFC does not
> > > support that yet)
> > Are you referring to the filter-id that is created at the time of config-nfc?
> Pls clarify.
> 
> 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?
> 
> Ben.
> 
> --
> Ben Hutchings
> One of the nice things about standards is that there are so many of them.

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

[Ben Hutchings]

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

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.

Ben.

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

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

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

[Venkata Duvvuru]

> -----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.

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

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

[Ben Hutchings]

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

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.

Ben.

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

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

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

[Venkata Duvvuru]

> -----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. If we fail the command then we are mandating the drivers to implement both the operations.
> 
> Ben.
> 
> --
> Ben Hutchings
> compatible: Gracefully accepts erroneous data from any source

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

[Ben Hutchings]

[-- 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 --]