Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11

[Ferruh Yigit]

On 1/17/2022 10:15 AM, Ferruh Yigit wrote:

On 1/15/2022 1:06 AM, Min Hu (Connor) wrote:

Hi,
     I got your meaning.
     The key is the API call sequence about bond_ethdev_configure and
rte_eth_bond_slave_add. testpmd ensure that rte_eth_bond_slave_add is
first called before bond_ethdev_configure, so this is no bug.
     Of course I also agree with this patch.
     Thanks.

Hi Yu Wenjun,

The patch is not shown in the patchwork, I assume because of its format,
can you please send a new version as text only?
Using 'git send-email' is the easiet way for it, please check:
https://doc.dpdk.org/guides/contributing/patches.html

Also Connor clarifies the issue above, can you please add this detail
about the issue in the next version?

Btw, please feel free to keep Connor's ack in next version.

Thanks,
ferruh

在 2022/1/14 19:14, 俞文俊_yewu 写道:

Hi, I tested Mellanox CX5 and Intel E810.


The key is the call chain:

1.rte_eth_bond_create()

2.rte_eth_dev_configure(): bond_ethdev_configure() *//internals->rss_key_len = 0, 
internals->rss_key can not be set properly*

3.rte_eth_bond_slave_add(): 
__eth_bond_slave_add_lock_free()->eth_bond_slave_inherit_dev_info_rx_first() *// 
internals->rss_key_len will be set, but internals->rss_key is bad*

4.rte_eth_dev_start()


examples/bond/main.c(bond_port_init()) use this call chain,too many apps refer 
to this example.


thanks

----邮件原文----
From："Min Hu (Connor)" <humi...@huawei.com>
To："俞文俊 _yewu" <yuwenjun_y...@cmss.chinamobile.com>,tangchengchang  
<tangchengch...@huawei.com>,"ferruh.yigit" <ferruh.yi...@intel.com>
Cc: dev  <dev@dpdk.org>,stable  <sta...@dpdk.org>
Date：2022-01-14 16:50:54
Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11

Hi,

在 2022/1/14 15:11, 俞文俊_yewu 写道:
 > Sorry，mq_mode is RTE_ETH_MQ_RX_RSS in rte_eth_conf.
 >
 >
 > call chain:
 >
 > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >
 >
 > Consider this call chain, internals->rss_key_len is 0 in
 > bond_ethdev_configure() when we call rte_eth_dev_configure().
 >
 > If internals->rss_key_len is 0, internals->rss_key can not be set properly.
Have you done any test? Is it true that it doesn't work for RSS? which
NIC you choose ?

As I know, internals->rss_key_len will inherit dev info from slave, just
see "eth_bond_slave_inherit_dev_info_rx_first". If slave port report
hash_key_size, internals->rss_key_len could not be zero.

 >
 > Because memcpy in bond_ethdev_configure() use internals->rss_key_len(it
 > is 0) as copy size, and internals->rss_key will not be set in other
 > functions.
 >
 >
 > ---
 >
 > e.g.:
 >
 > bond_ethdev_configure(struct rte_eth_dev *dev)
 >
 > {
 >
 > const char *name = dev->device->name;
 >
 > struct bond_dev_private *internals = dev->data->dev_private;
 >
 > ...
 >
 >
 > /*
 >
 > * If RSS is enabled, fill table with default values and
 >
 > * set key to the value specified in port RSS configuration.
 >
 > * Fall back to default RSS key if the key is not specified
 >
 > */
 >
 > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >
 > struct rte_eth_rss_conf *rss_conf =
 >
 > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >
 >
 > if (rss_conf->rss_key != NULL) {
 >
 > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >
 > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >
 > rss_conf->rss_key_len);
 >
 > return -EINVAL;
 >
 > }
 >
 >
 > memcpy(internals->rss_key, rss_conf->rss_key,
 >
 >         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;
 >
 > }
 >
 >
 > memcpy(internals->rss_key, default_rss_key,
 >
 >         internals->rss_key_len);
 >
 > }
 >
 >
 >
 > ----Origin----
 > From："Min Hu (Connor)" <humi...@huawei.com>
 > To："yuwenjun_yewu" <yuwenjun_y...@cmss.chinamobile.com>,tangchengchang  
<tangchengch...@huawei.com>,"ferruh.yigit" <ferruh.yi...@intel.com>
 > Cc: dev  <dev@dpdk.org>,stable  <sta...@dpdk.org>
 > Date：2022-01-14 08:59:21
 > Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11
 >
 > Hi,
 >
 > 在 2022/1/12 15:29, 俞文俊_yewu 写道:
 >  >  From 85c4ffffe32996fc262dd6f69d0ce272ae8e8350 Mon Sep 17 00:00:00 2001
 >  >
 >  > From: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > Date: Wed, 12 Jan 2022 15:01:10 +0800
 >  >
 >  > Subject: [PATCH] net/bonding: fix RSS not work for bonding
 >  >
 >  >
 >  > RSS don't work when upgrade to DPDK21.11.
 > Cannot get your meaning, Why RSS don't work?
 > As mq_mode is not RTE_ETH_MQ_RX_RSS in rte_eth_conf, RSS is off.
 > Please make it clearer, thanks.
 >
 >
 >  >
 >  >
 >  > e.g.:
 >  >
 >  > examples/bond/main.c:
 >  >
 >  > conf:
 >  >
 >  > static struct rte_eth_conf port_conf = {
 >  >
 >  > .rxmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_RX_NONE,
 >  >
 >  > .split_hdr_size = 0,
 >  >
 >  > },
 >  >
 >  > .rx_adv_conf = {
 >  >
 >  > .rss_conf = {
 >  >
 >  > .rss_key = NULL,
 >  >
 >  > .rss_hf = RTE_ETH_RSS_IP,
 >  >
 >  > },
 >  >
 >  > },
 >  >
 >  > .txmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_TX_NONE,
 >  >
 >  > },
 >  >
 >  > };
 >  >
 >  >
 >  > call chain:
 >  >
 >  > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >  >
 >  >
 >  > Signed-off-by: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > ---
 >  >
 >  >   drivers/net/bonding/rte_eth_bond_pmd.c | 5 +++++
 >  >
 >  >   1 file changed, 5 insertions(+)
 >  >
 >  >
 >  > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  > b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > index 84f4900ee5..31bcee15cf 100644
 >  >
 >  > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > @@ -3504,6 +3504,11 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 >  >
 >  > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >  >
 >  > struct rte_eth_rss_conf *rss_conf =
 >  >
 >  > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >  >
 >  > +
 >  >
 >  > +if (internals->rss_key_len == 0) {
 >  >
 >  > +internals->rss_key_len = sizeof(default_rss_key);
 >  >
 >  > +}
 >  >
 >  > +
 >  >
 >  > if (rss_conf->rss_key != NULL) {
 >  >
 >  > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >  >
 >  > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >  >
 >  > --
 >  >
 >  > 2.32.0.windows.1
 >  >
 >  >
 >
 > Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11
 >
 > Hi,
 >
 > 在 2022/1/12 15:29, 俞文俊_yewu 写道:
 >  >  From 85c4ffffe32996fc262dd6f69d0ce272ae8e8350 Mon Sep 17 00:00:00 2001
 >  >
 >  > From: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > Date: Wed, 12 Jan 2022 15:01:10 +0800
 >  >
 >  > Subject: [PATCH] net/bonding: fix RSS not work for bonding
 >  >
 >  >
 >  > RSS don't work when upgrade to DPDK21.11.
 > Cannot get your meaning, Why RSS don't work?
 > As mq_mode is not RTE_ETH_MQ_RX_RSS in rte_eth_conf, RSS is off.
 > Please make it clearer, thanks.
 >
 >
 >  >
 >  >
 >  > e.g.:
 >  >
 >  > examples/bond/main.c:
 >  >
 >  > conf:
 >  >
 >  > static struct rte_eth_conf port_conf = {
 >  >
 >  > .rxmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_RX_NONE,
 >  >
 >  > .split_hdr_size = 0,
 >  >
 >  > },
 >  >
 >  > .rx_adv_conf = {
 >  >
 >  > .rss_conf = {
 >  >
 >  > .rss_key = NULL,
 >  >
 >  > .rss_hf = RTE_ETH_RSS_IP,
 >  >
 >  > },
 >  >
 >  > },
 >  >
 >  > .txmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_TX_NONE,
 >  >
 >  > },
 >  >
 >  > };
 >  >
 >  >
 >  > call chain:
 >  >
 >  > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >  >
 >  >
 >  > Signed-off-by: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > ---
 >  >
 >  >   drivers/net/bonding/rte_eth_bond_pmd.c | 5 +++++
 >  >
 >  >   1 file changed, 5 insertions(+)
 >  >
 >  >
 >  > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  > b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > index 84f4900ee5..31bcee15cf 100644
 >  >
 >  > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > @@ -3504,6 +3504,11 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 >  >
 >  > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >  >
 >  > struct rte_eth_rss_conf *rss_conf =
 >  >
 >  > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >  >
 >  > +
 >  >
 >  > +if (internals->rss_key_len == 0) {
 >  >
 >  > +internals->rss_key_len = sizeof(default_rss_key);
 >  >
 >  > +}
 >  >
 >  > +
 >  >
 >  > if (rss_conf->rss_key != NULL) {
 >  >
 >  > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >  >
 >  > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >  >
 >  > --
 >  >
 >  > 2.32.0.windows.1
 >  >
 >  >
 >

Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11

Hi,

在 2022/1/14 15:11, 俞文俊_yewu 写道:
 > Sorry，mq_mode is RTE_ETH_MQ_RX_RSS in rte_eth_conf.
 >
 >
 > call chain:
 >
 > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >
 >
 > Consider this call chain, internals->rss_key_len is 0 in
 > bond_ethdev_configure() when we call rte_eth_dev_configure().
 >
 > If internals->rss_key_len is 0, internals->rss_key can not be set properly.
Have you done any test? Is it true that it doesn't work for RSS? which
NIC you choose ?

As I know, internals->rss_key_len will inherit dev info from slave, just
see "eth_bond_slave_inherit_dev_info_rx_first". If slave port report
hash_key_size, internals->rss_key_len could not be zero.

 >
 > Because memcpy in bond_ethdev_configure() use internals->rss_key_len(it
 > is 0) as copy size, and internals->rss_key will not be set in other
 > functions.
 >
 >
 > ---
 >
 > e.g.:
 >
 > bond_ethdev_configure(struct rte_eth_dev *dev)
 >
 > {
 >
 > const char *name = dev->device->name;
 >
 > struct bond_dev_private *internals = dev->data->dev_private;
 >
 > ...
 >
 >
 > /*
 >
 > * If RSS is enabled, fill table with default values and
 >
 > * set key to the value specified in port RSS configuration.
 >
 > * Fall back to default RSS key if the key is not specified
 >
 > */
 >
 > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >
 > struct rte_eth_rss_conf *rss_conf =
 >
 > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >
 >
 > if (rss_conf->rss_key != NULL) {
 >
 > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >
 > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >
 > rss_conf->rss_key_len);
 >
 > return -EINVAL;
 >
 > }
 >
 >
 > memcpy(internals->rss_key, rss_conf->rss_key,
 >
 >         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;
 >
 > }
 >
 >
 > memcpy(internals->rss_key, default_rss_key,
 >
 >         internals->rss_key_len);
 >
 > }
 >
 >
 >
 > ----Origin----
 > From："Min Hu (Connor)" <humi...@huawei.com>
 > To："yuwenjun_yewu" <yuwenjun_y...@cmss.chinamobile.com>,tangchengchang  
<tangchengch...@huawei.com>,"ferruh.yigit" <ferruh.yi...@intel.com>
 > Cc: dev  <dev@dpdk.org>,stable  <sta...@dpdk.org>
 > Date：2022-01-14 08:59:21
 > Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11
 >
 > Hi,
 >
 > 在 2022/1/12 15:29, 俞文俊_yewu 写道:
 >  >  From 85c4ffffe32996fc262dd6f69d0ce272ae8e8350 Mon Sep 17 00:00:00 2001
 >  >
 >  > From: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > Date: Wed, 12 Jan 2022 15:01:10 +0800
 >  >
 >  > Subject: [PATCH] net/bonding: fix RSS not work for bonding
 >  >
 >  >
 >  > RSS don't work when upgrade to DPDK21.11.
 > Cannot get your meaning, Why RSS don't work?
 > As mq_mode is not RTE_ETH_MQ_RX_RSS in rte_eth_conf, RSS is off.
 > Please make it clearer, thanks.
 >
 >
 >  >
 >  >
 >  > e.g.:
 >  >
 >  > examples/bond/main.c:
 >  >
 >  > conf:
 >  >
 >  > static struct rte_eth_conf port_conf = {
 >  >
 >  > .rxmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_RX_NONE,
 >  >
 >  > .split_hdr_size = 0,
 >  >
 >  > },
 >  >
 >  > .rx_adv_conf = {
 >  >
 >  > .rss_conf = {
 >  >
 >  > .rss_key = NULL,
 >  >
 >  > .rss_hf = RTE_ETH_RSS_IP,
 >  >
 >  > },
 >  >
 >  > },
 >  >
 >  > .txmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_TX_NONE,
 >  >
 >  > },
 >  >
 >  > };
 >  >
 >  >
 >  > call chain:
 >  >
 >  > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >  >
 >  >
 >  > Signed-off-by: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > ---
 >  >
 >  >   drivers/net/bonding/rte_eth_bond_pmd.c | 5 +++++
 >  >
 >  >   1 file changed, 5 insertions(+)
 >  >
 >  >
 >  > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  > b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > index 84f4900ee5..31bcee15cf 100644
 >  >
 >  > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > @@ -3504,6 +3504,11 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 >  >
 >  > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >  >
 >  > struct rte_eth_rss_conf *rss_conf =
 >  >
 >  > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >  >
 >  > +
 >  >
 >  > +if (internals->rss_key_len == 0) {
 >  >
 >  > +internals->rss_key_len = sizeof(default_rss_key);
 >  >
 >  > +}
 >  >
 >  > +
 >  >
 >  > if (rss_conf->rss_key != NULL) {
 >  >
 >  > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >  >
 >  > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >  >
 >  > --
 >  >
 >  > 2.32.0.windows.1
 >  >
 >  >
 >
 > Subject：Re: [PATCH] net/bonding: fix RSS not work for bonding in DPDK21.11
 >
 > Hi,
 >
 > 在 2022/1/12 15:29, 俞文俊_yewu 写道:
 >  >  From 85c4ffffe32996fc262dd6f69d0ce272ae8e8350 Mon Sep 17 00:00:00 2001
 >  >
 >  > From: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > Date: Wed, 12 Jan 2022 15:01:10 +0800
 >  >
 >  > Subject: [PATCH] net/bonding: fix RSS not work for bonding
 >  >
 >  >
 >  > RSS don't work when upgrade to DPDK21.11.
 > Cannot get your meaning, Why RSS don't work?
 > As mq_mode is not RTE_ETH_MQ_RX_RSS in rte_eth_conf, RSS is off.
 > Please make it clearer, thanks.
 >
 >
 >  >
 >  >
 >  > e.g.:
 >  >
 >  > examples/bond/main.c:
 >  >
 >  > conf:
 >  >
 >  > static struct rte_eth_conf port_conf = {
 >  >
 >  > .rxmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_RX_NONE,
 >  >
 >  > .split_hdr_size = 0,
 >  >
 >  > },
 >  >
 >  > .rx_adv_conf = {
 >  >
 >  > .rss_conf = {
 >  >
 >  > .rss_key = NULL,
 >  >
 >  > .rss_hf = RTE_ETH_RSS_IP,
 >  >
 >  > },
 >  >
 >  > },
 >  >
 >  > .txmode = {
 >  >
 >  > .mq_mode = RTE_ETH_MQ_TX_NONE,
 >  >
 >  > },
 >  >
 >  > };
 >  >
 >  >
 >  > call chain:
 >  >
 >  > 
rte_eth_bond_create()->rte_eth_dev_configure()->rte_eth_bond_slave_add()->rte_eth_dev_start()
 >  >
 >  >
 >  > Signed-off-by: Yu Wenjun <yuwenjun_y...@cmss.chinamobile.com>
 >  >
 >  > ---
 >  >
 >  >   drivers/net/bonding/rte_eth_bond_pmd.c | 5 +++++
 >  >
 >  >   1 file changed, 5 insertions(+)
 >  >
 >  >
 >  > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  > b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > index 84f4900ee5..31bcee15cf 100644
 >  >
 >  > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
 >  >
 >  > @@ -3504,6 +3504,11 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 >  >
 >  > if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 >  >
 >  > struct rte_eth_rss_conf *rss_conf =
 >  >
 >  > &dev->data->dev_conf.rx_adv_conf.rss_conf;
 >  >
 >  > +
 >  >
 >  > +if (internals->rss_key_len == 0) {
 >  >
 >  > +internals->rss_key_len = sizeof(default_rss_key);
 >  >
 >  > +}
 >  >
 >  > +
 >  >
 >  > if (rss_conf->rss_key != NULL) {
 >  >
 >  > if (internals->rss_key_len > rss_conf->rss_key_len) {
 >  >
 >  > RTE_BOND_LOG(ERR, "Invalid rss key length(%u)",
 >  >
 >  > --
 >  >
 >  > 2.32.0.windows.1
 >  >
 >  >
 >