[PATCH] be2net: hash key for rss-config cmd not set

[PATCH] be2net: hash key for rss-config cmd not set

[Sathya Perla]

Need a random hash key to effectively hash incoming connections into
multiple RX rings.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/benet/be_cmds.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index 2463b1c..81654ae 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1703,7 +1703,8 @@ int be_cmd_rss_config(struct be_adapter *adapter, u8 *rsstable, u16 table_size)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_rss_config *req;
-	u32 myhash[10];
+	u32 myhash[10] = {0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF,
+			0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF};
 	int status;
 
 	if (mutex_lock_interruptible(&adapter->mbox_lock))
-- 
1.7.4

Re: [PATCH] be2net: hash key for rss-config cmd not set

[Ben Hutchings]

On Mon, 2011-05-23 at 17:55 +0530, Sathya Perla wrote:
> Need a random hash key to effectively hash incoming connections into
> multiple RX rings.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>  drivers/net/benet/be_cmds.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
> index 2463b1c..81654ae 100644
> --- a/drivers/net/benet/be_cmds.c
> +++ b/drivers/net/benet/be_cmds.c
> @@ -1703,7 +1703,8 @@ int be_cmd_rss_config(struct be_adapter *adapter, u8 *rsstable, u16 table_size)
>  {
>  	struct be_mcc_wrb *wrb;
>  	struct be_cmd_req_rss_config *req;
> -	u32 myhash[10];

/* Chosen by a fair dice roll */ ?
> +	u32 myhash[10] = {0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF,
> +			0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF};
>  	int status;
>  
>  	if (mutex_lock_interruptible(&adapter->mbox_lock))

This is of course less random than the stack garbage you were presumably
using before.  Though I don't know to what extent random hash keys are
really a good idea.

I have heard the suggestion that Toeplitz hash keys should in fact be
randomised per-system, so that flows will be hashed to the same queue
numbers on each interface if their hash indirection tales are also the
same.  This would presumably be good for bonding with multiple active
links, forwarding, etc., especially when multiple drivers (which
currently have different behaviour) are involved.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] be2net: hash key for rss-config cmd not set

[David Miller]

From: Sathya Perla <sathya.perla@emulex.com>
Date: Mon, 23 May 2011 17:55:27 +0530

> Need a random hash key to effectively hash incoming connections into
> multiple RX rings.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

You're saying you need a random hash key, yet you're assigning
a constant.

Why not use a real random number, via get_random() or similar?

RE: [PATCH] be2net: hash key for rss-config cmd not set

[Sathya.Perla]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, May 24, 2011 1:22 AM
> To: Perla, Sathya
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH] be2net: hash key for rss-config cmd not set
> 
> From: Sathya Perla <sathya.perla@emulex.com>
> Date: Mon, 23 May 2011 17:55:27 +0530
> 
> > Need a random hash key to effectively hash incoming connections into
> > multiple RX rings.
> >
> > Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> 
> You're saying you need a random hash key, yet you're assigning
> a constant.
> 
> Why not use a real random number, via get_random() or similar?

I meant to say a non-zero, non-descript value is needed as the hash key. That was the reason I had left the hash variable un-initialized; but on some systems I get a zero value and hashing is not effective. The constant value I tried (not of any significance) seems to work fine.

Pls let me know if you want me to re-send the patch with a more-descriptive changelog...

thanks.
-Sathya

Re: [PATCH] be2net: hash key for rss-config cmd not set

[David Miller]

From: <Sathya.Perla@Emulex.Com>
Date: Mon, 23 May 2011 22:46:48 -0700

> Pls let me know if you want me to re-send the patch with a more-descriptive changelog...

Yes, I do.

Re: [PATCH] be2net: hash key for rss-config cmd not set

[David Miller]

From: Sathya Perla <sathya.perla@emulex.com>
Date: Tue, 24 May 2011 11:59:09 +0530

> A non-zero, non-descript value is needed as the hash key. The hash variable was left un-initialized; but sometimes it gets a zero value
> and hashing is not effective. The constant value used now (not of any significance) seems to work fine.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Applied.

Re: [PATCH] be2net: hash key for rss-config cmd not set

[Flavio Leitner]

On 05/24/2011 03:29 AM, Sathya Perla wrote:
> A non-zero, non-descript value is needed as the hash key. The hash variable was left un-initialized; but sometimes it gets a zero value
> and hashing is not effective. The constant value used now (not of any significance) seems to work fine.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>  drivers/net/benet/be_cmds.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
> index 2463b1c..81654ae 100644
> --- a/drivers/net/benet/be_cmds.c
> +++ b/drivers/net/benet/be_cmds.c
> @@ -1703,7 +1703,8 @@ int be_cmd_rss_config(struct be_adapter *adapter, u8 *rsstable, u16 table_size)
>  {
>  	struct be_mcc_wrb *wrb;
>  	struct be_cmd_req_rss_config *req;
> -	u32 myhash[10];
> +	u32 myhash[10] = {0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF,
> +			0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF};
>  	int status;
>  
>  	if (mutex_lock_interruptible(&adapter->mbox_lock))

I know we have the changelog saying that it's not of any significance,
but would be nice to have the same thing in the source as well.

fbl

[PATCH] be2net: hash key for rss-config cmd not set

[Sathya Perla]

A non-zero, non-descript value is needed as the hash key. The hash variable was left un-initialized; but sometimes it gets a zero value
and hashing is not effective. The constant value used now (not of any significance) seems to work fine.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/benet/be_cmds.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index 2463b1c..81654ae 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1703,7 +1703,8 @@ int be_cmd_rss_config(struct be_adapter *adapter, u8 *rsstable, u16 table_size)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_rss_config *req;
-	u32 myhash[10];
+	u32 myhash[10] = {0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF,
+			0x0123, 0x4567, 0x89AB, 0xCDEF, 0x01EF};
 	int status;
 
 	if (mutex_lock_interruptible(&adapter->mbox_lock))
-- 
1.7.4