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
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++) { >
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
> -----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++) { > >
在 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++) { >
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
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.
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++) {
>
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? > . >
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.
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.