netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ethtool] ethtool: Support for configurable RSS hash function
@ 2017-03-08 14:03 Gal Pressman
  2017-03-25 21:50 ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Gal Pressman @ 2017-03-08 14:03 UTC (permalink / raw)
  To: netdev, John W. Linville; +Cc: Gal Pressman, Eyal Perry

This ethtool patch adds support to set and get the current RSS hash
function for the device through the new hfunc mask field in the
ethtool_rxfh struct. Kernel supported hash function names are queried
with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
mask according to its index in the string-set.

Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
---
 ethtool.8.in |  6 ++++++
 ethtool.c    | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 5c36c06385f6..2cbe3f158b3e 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -301,6 +301,8 @@ ethtool \- query or control network driver and hardware settings
 .BI weight\  W0
 .IR W1
 .RB ...\ | \ default \ ]
+.RB [ hfunc
+.IR FUNC ]
 .HP
 .B ethtool \-f|\-\-flash
 .I devname file
@@ -865,6 +867,10 @@ Sets RSS hash key of the specified network device. RSS hash key should be of dev
 Hash key format must be in xx:yy:zz:aa:bb:cc format meaning both the nibbles of a byte should be mentioned
 even if a nibble is zero.
 .TP
+.BI hfunc
+Sets RSS hash function of the specified network device.
+List of RSS hash functions which kernel supports is shown as a part of the --show-rxfh command output.
+.TP
 .BI equal\  N
 Sets the receive flow hash indirection table to spread flows evenly
 between the first \fIN\fR receive queues.
diff --git a/ethtool.c b/ethtool.c
index 7af039e26b50..61d5714285f9 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -3640,6 +3640,7 @@ static int do_grxfhindir(struct cmd_context *ctx,
 
 static int do_grxfh(struct cmd_context *ctx)
 {
+	struct ethtool_gstrings *hfuncs = NULL;
 	struct ethtool_rxfh rss_head = {0};
 	struct ethtool_rxnfc ring_count;
 	struct ethtool_rxfh *rss;
@@ -3697,6 +3698,26 @@ static int do_grxfh(struct cmd_context *ctx)
 			printf("%02x:", (u8) hkey[i]);
 	}
 
+	printf("RSS hash function:\n");
+	if (!rss->hfunc) {
+		printf("    Operation not supported\n");
+		goto out;
+	}
+
+	hfuncs = get_stringset(ctx, ETH_SS_RSS_HASH_FUNCS, 0, 1);
+	if (!hfuncs) {
+		perror("Cannot get hash functions names");
+		free(rss);
+		return 1;
+	}
+
+	for (i = 0; i < hfuncs->len; i++)
+		printf("    %s: %s\n",
+		       (const char *)hfuncs->data + i * ETH_GSTRING_LEN,
+		       (rss->hfunc & (1 << i)) ? "on" : "off");
+
+out:
+	free(hfuncs);
 	free(rss);
 	return 0;
 }
@@ -3800,11 +3821,16 @@ static int do_srxfh(struct cmd_context *ctx)
 	struct ethtool_rxfh *rss;
 	struct ethtool_rxnfc ring_count;
 	int rxfhindir_equal = 0, rxfhindir_default = 0;
+	struct ethtool_gstrings *hfuncs = NULL;
 	char **rxfhindir_weight = NULL;
 	char *rxfhindir_key = NULL;
+	char *req_hfunc_name = NULL;
+	char *hfunc_name = NULL;
 	char *hkey = NULL;
 	int err = 0;
+	int i;
 	u32 arg_num = 0, indir_bytes = 0;
+	u32 req_hfunc = 0;
 	u32 entry_size = sizeof(rss_head.rss_config[0]);
 	u32 num_weights = 0;
 
@@ -3836,6 +3862,12 @@ static int do_srxfh(struct cmd_context *ctx)
 		} else if (!strcmp(ctx->argp[arg_num], "default")) {
 			++arg_num;
 			rxfhindir_default = 1;
+		} else if (!strcmp(ctx->argp[arg_num], "hfunc")) {
+			++arg_num;
+			req_hfunc_name = ctx->argp[arg_num];
+			if (!req_hfunc_name)
+				exit_bad_args();
+			++arg_num;
 		} else {
 			exit_bad_args();
 		}
@@ -3868,7 +3900,8 @@ static int do_srxfh(struct cmd_context *ctx)
 
 	rss_head.cmd = ETHTOOL_GRSSH;
 	err = send_ioctl(ctx, &rss_head);
-	if (err < 0 && errno == EOPNOTSUPP && !rxfhindir_key) {
+	if (err < 0 && errno == EOPNOTSUPP && !rxfhindir_key &&
+	    !req_hfunc_name) {
 		return do_srxfhindir(ctx, rxfhindir_default, rxfhindir_equal,
 				     rxfhindir_weight, num_weights);
 	} else if (err < 0) {
@@ -3886,14 +3919,39 @@ static int do_srxfh(struct cmd_context *ctx)
 	if (rxfhindir_equal || rxfhindir_weight)
 		indir_bytes = rss_head.indir_size * entry_size;
 
+	if (rss_head.hfunc && req_hfunc_name) {
+		hfuncs = get_stringset(ctx, ETH_SS_RSS_HASH_FUNCS, 0, 1);
+		if (!hfuncs) {
+			perror("Cannot get hash functions names");
+			return 1;
+		}
+
+		for (i = 0; i < hfuncs->len && !req_hfunc ; i++) {
+			hfunc_name = (char *)(hfuncs->data +
+					      i * ETH_GSTRING_LEN);
+			if (!strncmp(hfunc_name, req_hfunc_name,
+				     ETH_GSTRING_LEN))
+				req_hfunc = (u32)1 << i;
+		}
+
+		if (!req_hfunc) {
+			fprintf(stderr,
+				"Unknown hash function: %s\n", req_hfunc_name);
+			free(hfuncs);
+			return 1;
+		}
+	}
+
 	rss = calloc(1, sizeof(*rss) + indir_bytes + rss_head.key_size);
 	if (!rss) {
 		perror("Cannot allocate memory for RX flow hash config");
-		return 1;
+		err = 1;
+		goto free;
 	}
 	rss->cmd = ETHTOOL_SRSSH;
 	rss->indir_size = rss_head.indir_size;
 	rss->key_size = rss_head.key_size;
+	rss->hfunc = req_hfunc;
 
 	if (fill_indir_table(&rss->indir_size, rss->rss_config, rxfhindir_default,
 			     rxfhindir_equal, rxfhindir_weight, num_weights)) {
@@ -3918,6 +3976,7 @@ free:
 		free(hkey);
 
 	free(rss);
+	free(hfuncs);
 	return err;
 }
 
@@ -4790,7 +4849,8 @@ static const struct option {
 	{ "-X|--set-rxfh-indir|--rxfh", 1, do_srxfh,
 	  "Set Rx flow hash indirection and/or hash key",
 	  "		[ equal N | weight W0 W1 ... ]\n"
-	  "		[ hkey %x:%x:%x:%x:%x:.... ]\n" },
+	  "		[ hkey %x:%x:%x:%x:%x:.... ]\n"
+	  "		[ hfunc FUNC ]\n" },
 	{ "-f|--flash", 1, do_flash,
 	  "Flash firmware image from the specified file to a region on the device",
 	  "               FILENAME [ REGION-NUMBER-TO-FLASH ]\n" },
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH ethtool] ethtool: Support for configurable RSS hash function
  2017-03-08 14:03 [PATCH ethtool] ethtool: Support for configurable RSS hash function Gal Pressman
@ 2017-03-25 21:50 ` Jakub Kicinski
  2017-03-26 10:50   ` Gal Pressman
  2017-03-27 18:02   ` John W. Linville
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Kicinski @ 2017-03-25 21:50 UTC (permalink / raw)
  To: John W. Linville; +Cc: Gal Pressman, netdev, Eyal Perry

On Wed,  8 Mar 2017 16:03:51 +0200, Gal Pressman wrote:
> This ethtool patch adds support to set and get the current RSS hash
> function for the device through the new hfunc mask field in the
> ethtool_rxfh struct. Kernel supported hash function names are queried
> with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
> mask according to its index in the string-set.
> 
> Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
> Signed-off-by: Gal Pressman <galp@mellanox.com>
> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

Hi John,

It seems you have applied both my earlier patch with get support and
this:

adbaa18b9bc1 ("ethtool: Support for configurable RSS hash function")
b932835d2302 ("ethtool: print hash function with ethtool -x|--show-rxfh-indir")

Now we print the RSS function twice:

RX flow hash indirection table for em4 with 4 RX ring(s):
    0:      [...]
RSS hash function: toeplitz  <--- from my adbaa18b9bc1
RSS hash key:
Operation not supported
RSS hash function:           <--- from this patch
    toeplitz: on
    xor: off
    crc32: off

Reverting my patch is probably the easiest way forward, although I find
it more concise and easier to parse in test scripts :)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ethtool] ethtool: Support for configurable RSS hash function
  2017-03-25 21:50 ` Jakub Kicinski
@ 2017-03-26 10:50   ` Gal Pressman
  2017-03-27 18:02   ` John W. Linville
  1 sibling, 0 replies; 6+ messages in thread
From: Gal Pressman @ 2017-03-26 10:50 UTC (permalink / raw)
  To: Jakub Kicinski, John W. Linville; +Cc: netdev, Eyal Perry

On 26/03/2017 00:50, Jakub Kicinski wrote:
> On Wed,  8 Mar 2017 16:03:51 +0200, Gal Pressman wrote:
>> This ethtool patch adds support to set and get the current RSS hash
>> function for the device through the new hfunc mask field in the
>> ethtool_rxfh struct. Kernel supported hash function names are queried
>> with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
>> mask according to its index in the string-set.
>>
>> Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
>> Signed-off-by: Gal Pressman <galp@mellanox.com>
>> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> Hi John,
>
> It seems you have applied both my earlier patch with get support and
> this:
>
> adbaa18b9bc1 ("ethtool: Support for configurable RSS hash function")
> b932835d2302 ("ethtool: print hash function with ethtool -x|--show-rxfh-indir")
>
> Now we print the RSS function twice:
>
> RX flow hash indirection table for em4 with 4 RX ring(s):
>     0:      [...]
> RSS hash function: toeplitz  <--- from my adbaa18b9bc1
> RSS hash key:
> Operation not supported
> RSS hash function:           <--- from this patch
>     toeplitz: on
>     xor: off
>     crc32: off
>
> Reverting my patch is probably the easiest way forward, although I find
> it more concise and easier to parse in test scripts :)
Hi Jakub,
Nice catch!

IMHO we should keep the output from my patch, as it is more informative.
This way the user can query all hash functions supported by the kernel using -x flag, before
trying to change them.
I think it's nicer than "guessing" what's supported and what's not :).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ethtool] ethtool: Support for configurable RSS hash function
  2017-03-25 21:50 ` Jakub Kicinski
  2017-03-26 10:50   ` Gal Pressman
@ 2017-03-27 18:02   ` John W. Linville
  2017-03-29 11:56     ` Gal Pressman
  1 sibling, 1 reply; 6+ messages in thread
From: John W. Linville @ 2017-03-27 18:02 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Gal Pressman, netdev, Eyal Perry

On Sat, Mar 25, 2017 at 02:50:47PM -0700, Jakub Kicinski wrote:
> On Wed,  8 Mar 2017 16:03:51 +0200, Gal Pressman wrote:
> > This ethtool patch adds support to set and get the current RSS hash
> > function for the device through the new hfunc mask field in the
> > ethtool_rxfh struct. Kernel supported hash function names are queried
> > with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
> > mask according to its index in the string-set.
> > 
> > Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
> > Signed-off-by: Gal Pressman <galp@mellanox.com>
> > Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> 
> Hi John,
> 
> It seems you have applied both my earlier patch with get support and
> this:
> 
> adbaa18b9bc1 ("ethtool: Support for configurable RSS hash function")
> b932835d2302 ("ethtool: print hash function with ethtool -x|--show-rxfh-indir")
> 
> Now we print the RSS function twice:
> 
> RX flow hash indirection table for em4 with 4 RX ring(s):
>     0:      [...]
> RSS hash function: toeplitz  <--- from my adbaa18b9bc1
> RSS hash key:
> Operation not supported
> RSS hash function:           <--- from this patch
>     toeplitz: on
>     xor: off
>     crc32: off
> 
> Reverting my patch is probably the easiest way forward, although I find
> it more concise and easier to parse in test scripts :)

Thanks for pointing-out this issue! I apologize for my own confusion.

As you suggest, I will be reverting your patch.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ethtool] ethtool: Support for configurable RSS hash function
  2017-03-27 18:02   ` John W. Linville
@ 2017-03-29 11:56     ` Gal Pressman
  2017-03-29 14:49       ` John W. Linville
  0 siblings, 1 reply; 6+ messages in thread
From: Gal Pressman @ 2017-03-29 11:56 UTC (permalink / raw)
  To: John W. Linville, Jakub Kicinski; +Cc: netdev, Eyal Perry

On 27/03/2017 21:02, John W. Linville wrote:
> On Sat, Mar 25, 2017 at 02:50:47PM -0700, Jakub Kicinski wrote:
>> On Wed,  8 Mar 2017 16:03:51 +0200, Gal Pressman wrote:
>>> This ethtool patch adds support to set and get the current RSS hash
>>> function for the device through the new hfunc mask field in the
>>> ethtool_rxfh struct. Kernel supported hash function names are queried
>>> with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
>>> mask according to its index in the string-set.
>>>
>>> Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
>>> Signed-off-by: Gal Pressman <galp@mellanox.com>
>>> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
>> Hi John,
>>
>> It seems you have applied both my earlier patch with get support and
>> this:
>>
>> adbaa18b9bc1 ("ethtool: Support for configurable RSS hash function")
>> b932835d2302 ("ethtool: print hash function with ethtool -x|--show-rxfh-indir")
>>
>> Now we print the RSS function twice:
>>
>> RX flow hash indirection table for em4 with 4 RX ring(s):
>>     0:      [...]
>> RSS hash function: toeplitz  <--- from my adbaa18b9bc1
>> RSS hash key:
>> Operation not supported
>> RSS hash function:           <--- from this patch
>>     toeplitz: on
>>     xor: off
>>     crc32: off
>>
>> Reverting my patch is probably the easiest way forward, although I find
>> it more concise and easier to parse in test scripts :)
> Thanks for pointing-out this issue! I apologize for my own confusion.
>
> As you suggest, I will be reverting your patch.
>
> Thanks,
>
> John
126464e4da18 ('Revert "ethtool: Support for configurable RSS hash function"')

Seems like you ended up reverting my patch instead of Jakub's?
We lost the set hfunc functionality.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH ethtool] ethtool: Support for configurable RSS hash function
  2017-03-29 11:56     ` Gal Pressman
@ 2017-03-29 14:49       ` John W. Linville
  0 siblings, 0 replies; 6+ messages in thread
From: John W. Linville @ 2017-03-29 14:49 UTC (permalink / raw)
  To: Gal Pressman; +Cc: Jakub Kicinski, netdev, Eyal Perry

On Wed, Mar 29, 2017 at 02:56:38PM +0300, Gal Pressman wrote:
> On 27/03/2017 21:02, John W. Linville wrote:
> > On Sat, Mar 25, 2017 at 02:50:47PM -0700, Jakub Kicinski wrote:
> >> On Wed,  8 Mar 2017 16:03:51 +0200, Gal Pressman wrote:
> >>> This ethtool patch adds support to set and get the current RSS hash
> >>> function for the device through the new hfunc mask field in the
> >>> ethtool_rxfh struct. Kernel supported hash function names are queried
> >>> with ETHTOOL_GSTRINGS - each string is corresponding with a bit in hfunc
> >>> mask according to its index in the string-set.
> >>>
> >>> Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
> >>> Signed-off-by: Gal Pressman <galp@mellanox.com>
> >>> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> >> Hi John,
> >>
> >> It seems you have applied both my earlier patch with get support and
> >> this:
> >>
> >> adbaa18b9bc1 ("ethtool: Support for configurable RSS hash function")
> >> b932835d2302 ("ethtool: print hash function with ethtool -x|--show-rxfh-indir")

Damn it! I was bamboozled...

> >> Now we print the RSS function twice:
> >>
> >> RX flow hash indirection table for em4 with 4 RX ring(s):
> >>     0:      [...]
> >> RSS hash function: toeplitz  <--- from my adbaa18b9bc1

This should have said "<--- from my b932835d2302"...

> >> RSS hash key:
> >> Operation not supported
> >> RSS hash function:           <--- from this patch
> >>     toeplitz: on
> >>     xor: off
> >>     crc32: off
> >>
> >> Reverting my patch is probably the easiest way forward, although I find
> >> it more concise and easier to parse in test scripts :)
> > Thanks for pointing-out this issue! I apologize for my own confusion.
> >
> > As you suggest, I will be reverting your patch.
> >
> > Thanks,
> >
> > John
> 126464e4da18 ('Revert "ethtool: Support for configurable RSS hash function"')
> 
> Seems like you ended up reverting my patch instead of Jakub's?
> We lost the set hfunc functionality.

Confused by the swapped commit IDs above, I dutifully reverted
"adbaa18b9bc1"...ugh...

Alright, I'll have to tighten-up my game a bit! From here I'll revert
b932835d2302 and reinstate adbaa18b9bc1, hopefully bringing this
little misadventure to a close.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-03-29 15:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-08 14:03 [PATCH ethtool] ethtool: Support for configurable RSS hash function Gal Pressman
2017-03-25 21:50 ` Jakub Kicinski
2017-03-26 10:50   ` Gal Pressman
2017-03-27 18:02   ` John W. Linville
2017-03-29 11:56     ` Gal Pressman
2017-03-29 14:49       ` John W. Linville

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).