[PATCH] net/bonding: fix rss key configuration when the key length is 52

[PATCH] net/bonding: fix rss key configuration when the key length is 52

[Ke Zhang]

when creating a bonding device, if the slave device's rss key length
is 52, then bonding device will be same as slave, in function
bond_ethdev_configure(), the default_rss_key length is 40, it
is not matched, so it should calculate a new key for bonding
device if the deault key could not be used.

Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b305b6a35b..4214b33f40 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3617,13 +3617,13 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 			       internals->rss_key_len);
 		} else {
 			if (internals->rss_key_len > sizeof(default_rss_key)) {
-				RTE_BOND_LOG(ERR,
-				       "There is no suitable default hash key");
-				return -EINVAL;
+				/* If the rss_key_len is 52, it should calculate the hash key */
+				for (i = 0; i < internals->rss_key_len; i++)
+					internals->rss_key[i] = (uint8_t)rte_rand();
+			} else {
+				memcpy(internals->rss_key, default_rss_key,
+					internals->rss_key_len);
 			}
-
-			memcpy(internals->rss_key, default_rss_key,
-			       internals->rss_key_len);
 		}
 
 		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
-- 
2.25.1

Re: [PATCH] net/bonding: fix rss key configuration when the key length is 52

[Min Hu (Connor)]

Hi,

在 2022/4/7 17:36, Ke Zhang 写道:
> when creating a bonding device, if the slave device's rss key length
> is 52, then bonding device will be same as slave, in function
> bond_ethdev_configure(), the default_rss_key length is 40, it
> is not matched, so it should calculate a new key for bonding
> device if the deault key could not be used.
wrong spelling.
> 
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b305b6a35b..4214b33f40 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -3617,13 +3617,13 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
>   			       internals->rss_key_len);
>   		} else {
>   			if (internals->rss_key_len > sizeof(default_rss_key)) {
> -				RTE_BOND_LOG(ERR,
> -				       "There is no suitable default hash key");
> -				return -EINVAL;
> +				/* If the rss_key_len is 52, it should calculate the hash key */
I think the comment should be more common, no need to emphysize '52'.
> +				for (i = 0; i < internals->rss_key_len; i++)
> +					internals->rss_key[i] = (uint8_t)rte_rand();
> +			} else {
> +				memcpy(internals->rss_key, default_rss_key,
> +					internals->rss_key_len);
>   			}
> -
> -			memcpy(internals->rss_key, default_rss_key,
> -			       internals->rss_key_len);
>   		}
>   
>   		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
> 

[PATCH v2 1/1] net/bonding: fix rss key configuration when the key length is 52

[Ke Zhang]

when creating a bonding device, if the slave device's rss key length
is 52, then bonding device will be same as slave, in function
bond_ethdev_configure(), the default_rss_key length is 40, it
is not matched, so it should calculate a new key for bonding
device if the deault key could not be used.

Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b305b6a35b..027339b0d9 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3617,13 +3617,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 			       internals->rss_key_len);
 		} else {
 			if (internals->rss_key_len > sizeof(default_rss_key)) {
-				RTE_BOND_LOG(ERR,
-				       "There is no suitable default hash key");
-				return -EINVAL;
+				/*
+				 * If the rss_key includes standard_rss_key and
+				 * extended_hash_key, the rss key length will
+				 * larger than default rss key length, so it should
+				 * re-calculate the hash key
+				 */
+				for (i = 0; i < internals->rss_key_len; i++)
+					internals->rss_key[i] = (uint8_t)rte_rand();
+			} else {
+				memcpy(internals->rss_key, default_rss_key,
+					internals->rss_key_len);
 			}
-
-			memcpy(internals->rss_key, default_rss_key,
-			       internals->rss_key_len);
 		}
 
 		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
-- 
2.25.1

RE: [PATCH] net/bonding: fix rss key configuration when the key length is 52

[Zhang, Ke1X]

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Friday, April 8, 2022 10:33 AM
> To: Zhang, Ke1X <ke1x.zhang@intel.com>; chas3@att.com; dev@dpdk.org
> Subject: Re: [PATCH] net/bonding: fix rss key configuration when the key
> length is 52
> 
> Hi,
> 
> 在 2022/4/7 17:36, Ke Zhang 写道:
> > when creating a bonding device, if the slave device's rss key length
> > is 52, then bonding device will be same as slave, in function
> > bond_ethdev_configure(), the default_rss_key length is 40, it is not
> > matched, so it should calculate a new key for bonding device if the
> > deault key could not be used.
> wrong spelling.

This is a coding waring as below:
 
 _coding style issues_
 
WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
#64:
when creating a bonding device, if the slave device's rss key length

WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
#65:
 is 52, then bonding device will be same as slave, in function

total: 0 errors, 2 warnings, 0 checks, 19 lines checked

the 'slave' is correct, for exsample, testpmd cmd:
add bonding slave 0 2

> >
> > Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> > ---
> >   drivers/net/bonding/rte_eth_bond_pmd.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> > b/drivers/net/bonding/rte_eth_bond_pmd.c
> > index b305b6a35b..4214b33f40 100644
> > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> > @@ -3617,13 +3617,13 @@ bond_ethdev_configure(struct rte_eth_dev
> *dev)
> >   			       internals->rss_key_len);
> >   		} else {
> >   			if (internals->rss_key_len > sizeof(default_rss_key))
> {
> > -				RTE_BOND_LOG(ERR,
> > -				       "There is no suitable default hash key");
> > -				return -EINVAL;
> > +				/* If the rss_key_len is 52, it should calculate
> the hash key */
> I think the comment should be more common, no need to emphysize '52'.
> > +				for (i = 0; i < internals->rss_key_len; i++)
> > +					internals->rss_key[i] =
> (uint8_t)rte_rand();
> > +			} else {
> > +				memcpy(internals->rss_key, default_rss_key,
> > +					internals->rss_key_len);
> >   			}
> > -
> > -			memcpy(internals->rss_key, default_rss_key,
> > -			       internals->rss_key_len);
> >   		}
> >
> >   		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
> >

Re: [PATCH v2 1/1] net/bonding: fix rss key configuration when the key length is 52

[Min Hu (Connor)]

在 2022/4/11 11:02, Ke Zhang 写道:
> when creating a bonding device, if the slave device's rss key length
> is 52, then bonding device will be same as slave, in function
> bond_ethdev_configure(), the default_rss_key length is 40, it
> is not matched, so it should calculate a new key for bonding
> device if the deault key could not be used.
> 
I mean 'deault ' is wrong.

> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b305b6a35b..027339b0d9 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -3617,13 +3617,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
>   			       internals->rss_key_len);
>   		} else {
>   			if (internals->rss_key_len > sizeof(default_rss_key)) {
> -				RTE_BOND_LOG(ERR,
> -				       "There is no suitable default hash key");
> -				return -EINVAL;
> +				/*
> +				 * If the rss_key includes standard_rss_key and
> +				 * extended_hash_key, the rss key length will
will be
> +				 * larger than default rss key length, so it should
> +				 * re-calculate the hash key
recalculate

key.
> +				 */
> +				for (i = 0; i < internals->rss_key_len; i++)
> +					internals->rss_key[i] = (uint8_t)rte_rand();
> +			} else {
> +				memcpy(internals->rss_key, default_rss_key,
> +					internals->rss_key_len);
>   			}
> -
> -			memcpy(internals->rss_key, default_rss_key,
> -			       internals->rss_key_len);
>   		}
>   
>   		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
> 

[PATCH v3 1/1] net/bonding: fix rss key configuration when the key length is 52

[Ke Zhang]

when creating a bonding device, if the slave device's rss key length
= standard_rss_key length + extended_hash_key length, then bonding
device will be same as slave, in function bond_ethdev_configure(),
the default_rss_key length is 40, it is not matched, so it should
calculate a new key for bonding device if the default key could not
be used.

Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b305b6a35b..5cbe89031b 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3617,13 +3617,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 			       internals->rss_key_len);
 		} else {
 			if (internals->rss_key_len > sizeof(default_rss_key)) {
-				RTE_BOND_LOG(ERR,
-				       "There is no suitable default hash key");
-				return -EINVAL;
+				/*
+				 * If the rss_key includes standard_rss_key and
+				 * extended_hash_key, the rss key length will be
+				 * larger than default rss key length, so it should
+				 * re-calculate the hash key.
+				 */
+				for (i = 0; i < internals->rss_key_len; i++)
+					internals->rss_key[i] = (uint8_t)rte_rand();
+			} else {
+				memcpy(internals->rss_key, default_rss_key,
+					internals->rss_key_len);
 			}
-
-			memcpy(internals->rss_key, default_rss_key,
-			       internals->rss_key_len);
 		}
 
 		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
-- 
2.25.1

Re: [PATCH] net/bonding: fix rss key configuration when the key length is 52

[Stephen Hemminger]

On Mon, 11 Apr 2022 03:06:46 +0000
"Zhang, Ke1X" <ke1x.zhang@intel.com> wrote:

> This is a coding waring as below:
>  
>  _coding style issues_
>  
> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> #64:
> when creating a bonding device, if the slave device's rss key length
> 
> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> #65:
>  is 52, then bonding device will be same as slave, in function
> 
> total: 0 errors, 2 warnings, 0 checks, 19 lines checked
> 
> the 'slave' is correct, for exsample, testpmd cmd:
> add bonding slave 0 2

Just a heads up, for 22.11 I plan to replace/deprecate use of master/slave
terminology in bonding driver.  The terms master/slave are legacy and most
commercial products have switched to the terms from the 802 standard.

Re: [PATCH v3 1/1] net/bonding: fix rss key configuration when the key length is 52

[Min Hu (Connor)]

Acked-by: Min Hu (Connor) <humin29@huawei.com>

在 2022/4/11 13:40, Ke Zhang 写道:
> when creating a bonding device, if the slave device's rss key length
> = standard_rss_key length + extended_hash_key length, then bonding
> device will be same as slave, in function bond_ethdev_configure(),
> the default_rss_key length is 40, it is not matched, so it should
> calculate a new key for bonding device if the default key could not
> be used.
> 
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b305b6a35b..5cbe89031b 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -3617,13 +3617,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
>   			       internals->rss_key_len);
>   		} else {
>   			if (internals->rss_key_len > sizeof(default_rss_key)) {
> -				RTE_BOND_LOG(ERR,
> -				       "There is no suitable default hash key");
> -				return -EINVAL;
> +				/*
> +				 * If the rss_key includes standard_rss_key and
> +				 * extended_hash_key, the rss key length will be
> +				 * larger than default rss key length, so it should
> +				 * re-calculate the hash key.
> +				 */
> +				for (i = 0; i < internals->rss_key_len; i++)
> +					internals->rss_key[i] = (uint8_t)rte_rand();
> +			} else {
> +				memcpy(internals->rss_key, default_rss_key,
> +					internals->rss_key_len);
>   			}
> -
> -			memcpy(internals->rss_key, default_rss_key,
> -			       internals->rss_key_len);
>   		}
>   
>   		for (i = 0; i < RTE_DIM(internals->reta_conf); i++) {
> 

Re: [PATCH] net/bonding: fix rss key configuration when the key length is 52

[Min Hu (Connor)]

Hi, Stephen,

在 2022/4/12 23:19, Stephen Hemminger 写道:
> On Mon, 11 Apr 2022 03:06:46 +0000
> "Zhang, Ke1X" <ke1x.zhang@intel.com> wrote:
> 
>> This is a coding waring as below:
>>   
>>   _coding style issues_
>>   
>> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
>> #64:
>> when creating a bonding device, if the slave device's rss key length
>>
>> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
>> #65:
>>   is 52, then bonding device will be same as slave, in function
>>
>> total: 0 errors, 2 warnings, 0 checks, 19 lines checked
>>
>> the 'slave' is correct, for exsample, testpmd cmd:
>> add bonding slave 0 2
> 
> Just a heads up, for 22.11 I plan to replace/deprecate use of master/slave
> terminology in bonding driver.  The terms master/slave are legacy and most
> commercial products have switched to the terms from the 802 standard.
What are the terms ? Could you show them for us?
> .
> 

Re: [PATCH] net/bonding: fix rss key configuration when the key length is 52

[Stephen Hemminger]

On Wed, 13 Apr 2022 17:03:07 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Hi, Stephen,
> 
> 在 2022/4/12 23:19, Stephen Hemminger 写道:
> > On Mon, 11 Apr 2022 03:06:46 +0000
> > "Zhang, Ke1X" <ke1x.zhang@intel.com> wrote:
> >   
> >> This is a coding waring as below:
> >>   
> >>   _coding style issues_
> >>   
> >> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> >> #64:
> >> when creating a bonding device, if the slave device's rss key length
> >>
> >> WARNING:TYPO_SPELLING: 'slave' may be misspelled - perhaps 'secondary'?
> >> #65:
> >>   is 52, then bonding device will be same as slave, in function
> >>
> >> total: 0 errors, 2 warnings, 0 checks, 19 lines checked
> >>
> >> the 'slave' is correct, for exsample, testpmd cmd:
> >> add bonding slave 0 2  
> > 
> > Just a heads up, for 22.11 I plan to replace/deprecate use of master/slave
> > terminology in bonding driver.  The terms master/slave are legacy and most
> > commercial products have switched to the terms from the 802 standard.  
> What are the terms ? Could you show them for us?

The current IEEE std is https://standards.ieee.org/ieee/802.1AX/6768/
which is unfortunately paywalled.

The terms in standard are "Aggregator Client" for the ports used in the device
and "Aggregator Partner/Multiplexor" but they are not quite the same.

Probably going to use client and multiplexor as terms; but open for discussion.

Re: [PATCH v3 1/1] net/bonding: fix rss key configuration when the key length is 52

[Ferruh Yigit]

On 4/13/2022 10:01 AM, Min Hu (Connor) wrote:

<...>

> 在 2022/4/11 13:40, Ke Zhang 写道:
>> when creating a bonding device, if the slave device's rss key length
>> = standard_rss_key length + extended_hash_key length, then bonding
>> device will be same as slave, in function bond_ethdev_configure(),
>> the default_rss_key length is 40, it is not matched, so it should
>> calculate a new key for bonding device if the default key could not
>> be used.
>>
>> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> 
> Acked-by: Min Hu (Connor) <humin29@huawei.com>
> 
     Fixes: 6b1a001ec546 ("net/bonding: fix RSS key length")
     Cc: stable@dpdk.org

(The fixes line not exactly true, since 52 byte keys seems not 
considered from the begging, but above commit seems the closest one to 
make the patch backported to proper releases.)

Applied to dpdk-next-net/main, thanks.