* [PATCH 1/2] net/bonding: add error hint for invald args
@ 2022-02-14 8:13 Wan Junjie
2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Wan Junjie @ 2022-02-14 8:13 UTC (permalink / raw)
To: dev; +Cc: Chas Williams, Min Hu, Wan Junjie
When invalid args exsit, application exits with no error hint.
Adding a log message here will help users to know the reson.
Signed-off-by: Wan Junjie <wanjunjie@bytedance.com>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index bfa931098e..aa6519f83c 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3439,8 +3439,10 @@ bond_probe(struct rte_vdev_device *dev)
kvlist = rte_kvargs_parse(rte_vdev_device_args(dev),
pmd_bond_init_valid_arguments);
- if (kvlist == NULL)
+ if (kvlist == NULL) {
+ RTE_BOND_LOG(ERR, "Invalid args in %s", rte_vdev_device_args(dev));
return -1;
+ }
/* Parse link bonding mode */
if (rte_kvargs_count(kvlist, PMD_BOND_MODE_KVARG) == 1) {
--
2.33.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting 2022-02-14 8:13 [PATCH 1/2] net/bonding: add error hint for invald args Wan Junjie @ 2022-02-14 8:13 ` Wan Junjie 2022-02-15 9:26 ` Min Hu (Connor) 2022-02-15 10:59 ` [PATCH v2 2/2] net/bonding: fix slaves initializing " Wan Junjie 2022-02-15 9:12 ` [PATCH 1/2] net/bonding: add error hint for invald args Min Hu (Connor) 2022-02-15 10:56 ` [PATCH v2 1/2] net/bonding: add error hint for invalid args Wan Junjie 2 siblings, 2 replies; 13+ messages in thread From: Wan Junjie @ 2022-02-14 8:13 UTC (permalink / raw) To: dev; +Cc: Chas Williams, Min Hu, Wan Junjie When initializing a bonding device with all slaves in vdev string, which means bonding device initilizes slaves, instead of initializing slaves before creating the bonding device, it will fail to call set_mtu api for the bonding device. Fixes:b26bee1 ("ethdev: forbid MTU set before device configure") Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> --- app/test/test_link_bonding.c | 4 +++ app/test/test_link_bonding_rssconf.c | 4 +++ drivers/net/bonding/eth_bond_private.h | 4 +++ drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c index dc6fc46b9c..12c50ef393 100644 --- a/app/test/test_link_bonding.c +++ b/app/test/test_link_bonding.c @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t en_isr) test_params->nb_tx_q, &default_pmd_conf), "rte_eth_dev_configure for port %d failed", port_id); + int ret = rte_eth_dev_set_mtu(port_id, 1550); + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, + "rte_eth_dev_set_mtu for port %d failed", port_id); + for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, RX_RING_SIZE, rte_eth_dev_socket_id(port_id), &rx_conf_default, diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c index f9eae93973..7228965ced 100644 --- a/app/test/test_link_bonding_rssconf.c +++ b/app/test/test_link_bonding_rssconf.c @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct rte_eth_conf *eth_conf, RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure device %u", port_id); + int ret = rte_eth_dev_set_mtu(port_id, 1550); + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, + "rte_eth_dev_set_mtu for port %d failed", port_id); + for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, RXTX_RING_SIZE, rte_eth_dev_socket_id(port_id), NULL, diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h index 156335c425..8222e3cd38 100644 --- a/drivers/net/bonding/eth_bond_private.h +++ b/drivers/net/bonding/eth_bond_private.h @@ -246,6 +246,10 @@ int slave_configure(struct rte_eth_dev *bonded_eth_dev, struct rte_eth_dev *slave_eth_dev); +int +slave_start(struct rte_eth_dev *bonded_eth_dev, + struct rte_eth_dev *slave_eth_dev); + void slave_remove(struct bond_dev_private *internals, struct rte_eth_dev *slave_eth_dev); diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index b78867b125..4ac191c468 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) slave_port_id); return -1; } + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { + internals->slave_count--; + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", + slave_port_id); + return -1; + } } /* Update all slave devices MACs */ diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index aa6519f83c..c31b723071 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -1678,14 +1678,10 @@ int slave_configure(struct rte_eth_dev *bonded_eth_dev, struct rte_eth_dev *slave_eth_dev) { - struct bond_rx_queue *bd_rx_q; - struct bond_tx_queue *bd_tx_q; uint16_t nb_rx_queues; uint16_t nb_tx_queues; int errval; - uint16_t q_id; - struct rte_flow_error flow_error; struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, slave_eth_dev->data->port_id, errval); return errval; } + return 0; +} + +int +slave_start(struct rte_eth_dev *bonded_eth_dev, + struct rte_eth_dev *slave_eth_dev) +{ + int errval = 0; + struct bond_rx_queue *bd_rx_q; + struct bond_tx_queue *bd_tx_q; + uint16_t q_id; + struct rte_flow_error flow_error; + struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; /* Setup Rx Queues */ for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, return errval; } - if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) - rte_flow_destroy(slave_eth_dev->data->port_id, + if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) { + errval = rte_flow_destroy(slave_eth_dev->data->port_id, internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], &flow_error); + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)", + slave_eth_dev->data->port_id, errval); + } errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, slave_eth_dev->data->port_id); @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) internals->slaves[i].port_id); goto out_err; } + if (slave_start(eth_dev, slave_ethdev) != 0) { + RTE_BOND_LOG(ERR, + "bonded port (%d) failed to start slave device (%d)", + eth_dev->data->port_id, + internals->slaves[i].port_id); + goto out_err; + } /* We will need to poll for link status if any slave doesn't * support interrupts */ @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) return -1; } + /* configure slaves so we can pass mtu setting */ + for (i = 0; i < internals->slave_count; i++) { + struct rte_eth_dev *slave_ethdev = + &(rte_eth_devices[internals->slaves[i].port_id]); + if (slave_configure(dev, slave_ethdev) != 0) { + RTE_BOND_LOG(ERR, + "bonded port (%d) failed to configure slave device (%d)", + dev->data->port_id, + internals->slaves[i].port_id); + return -1; + } + } return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting 2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie @ 2022-02-15 9:26 ` Min Hu (Connor) 2022-02-15 10:32 ` [External] " Wan Junjie 2022-02-15 10:59 ` [PATCH v2 2/2] net/bonding: fix slaves initializing " Wan Junjie 1 sibling, 1 reply; 13+ messages in thread From: Min Hu (Connor) @ 2022-02-15 9:26 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams Hi, wan, two comments: 1. wrong spelling in headline 2. comment info should be detailed, such as, show the testpmd commands about how to operate in this two situations. Thanks. 在 2022/2/14 16:13, Wan Junjie 写道: > When initializing a bonding device with all slaves in vdev string, > which means bonding device initilizes slaves, instead of initializing > slaves before creating the bonding device, it will fail to call > set_mtu api for the bonding device. > > Fixes:b26bee1 ("ethdev: forbid MTU set before device configure") > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > --- > app/test/test_link_bonding.c | 4 +++ > app/test/test_link_bonding_rssconf.c | 4 +++ > drivers/net/bonding/eth_bond_private.h | 4 +++ > drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ > drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- > 5 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c > index dc6fc46b9c..12c50ef393 100644 > --- a/app/test/test_link_bonding.c > +++ b/app/test/test_link_bonding.c > @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t en_isr) > test_params->nb_tx_q, &default_pmd_conf), > "rte_eth_dev_configure for port %d failed", port_id); > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > + "rte_eth_dev_set_mtu for port %d failed", port_id); > + > for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) > TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, RX_RING_SIZE, > rte_eth_dev_socket_id(port_id), &rx_conf_default, > diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c > index f9eae93973..7228965ced 100644 > --- a/app/test/test_link_bonding_rssconf.c > +++ b/app/test/test_link_bonding_rssconf.c > @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct rte_eth_conf *eth_conf, > RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure device %u", > port_id); > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > + "rte_eth_dev_set_mtu for port %d failed", port_id); > + > for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { > TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, RXTX_RING_SIZE, > rte_eth_dev_socket_id(port_id), NULL, > diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h > index 156335c425..8222e3cd38 100644 > --- a/drivers/net/bonding/eth_bond_private.h > +++ b/drivers/net/bonding/eth_bond_private.h > @@ -246,6 +246,10 @@ int > slave_configure(struct rte_eth_dev *bonded_eth_dev, > struct rte_eth_dev *slave_eth_dev); > > +int > +slave_start(struct rte_eth_dev *bonded_eth_dev, > + struct rte_eth_dev *slave_eth_dev); > + > void > slave_remove(struct bond_dev_private *internals, > struct rte_eth_dev *slave_eth_dev); > diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c > index b78867b125..4ac191c468 100644 > --- a/drivers/net/bonding/rte_eth_bond_api.c > +++ b/drivers/net/bonding/rte_eth_bond_api.c > @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) > slave_port_id); > return -1; > } > + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { > + internals->slave_count--; > + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", > + slave_port_id); > + return -1; > + } > } > > /* Update all slave devices MACs */ > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index aa6519f83c..c31b723071 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1678,14 +1678,10 @@ int > slave_configure(struct rte_eth_dev *bonded_eth_dev, > struct rte_eth_dev *slave_eth_dev) > { > - struct bond_rx_queue *bd_rx_q; > - struct bond_tx_queue *bd_tx_q; > uint16_t nb_rx_queues; > uint16_t nb_tx_queues; > > int errval; > - uint16_t q_id; > - struct rte_flow_error flow_error; > > struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > slave_eth_dev->data->port_id, errval); > return errval; > } > + return 0; > +} > + > +int > +slave_start(struct rte_eth_dev *bonded_eth_dev, > + struct rte_eth_dev *slave_eth_dev) > +{ > + int errval = 0; > + struct bond_rx_queue *bd_rx_q; > + struct bond_tx_queue *bd_tx_q; > + uint16_t q_id; > + struct rte_flow_error flow_error; > + struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > /* Setup Rx Queues */ > for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { > @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > return errval; > } > > - if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) > - rte_flow_destroy(slave_eth_dev->data->port_id, > + if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) { > + errval = rte_flow_destroy(slave_eth_dev->data->port_id, > internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], > &flow_error); > + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)", > + slave_eth_dev->data->port_id, errval); > + } > > errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, > slave_eth_dev->data->port_id); > @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) > internals->slaves[i].port_id); > goto out_err; > } > + if (slave_start(eth_dev, slave_ethdev) != 0) { > + RTE_BOND_LOG(ERR, > + "bonded port (%d) failed to start slave device (%d)", > + eth_dev->data->port_id, > + internals->slaves[i].port_id); > + goto out_err; > + } > /* We will need to poll for link status if any slave doesn't > * support interrupts > */ > @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) > return -1; > } > > + /* configure slaves so we can pass mtu setting */ > + for (i = 0; i < internals->slave_count; i++) { > + struct rte_eth_dev *slave_ethdev = > + &(rte_eth_devices[internals->slaves[i].port_id]); > + if (slave_configure(dev, slave_ethdev) != 0) { > + RTE_BOND_LOG(ERR, > + "bonded port (%d) failed to configure slave device (%d)", > + dev->data->port_id, > + internals->slaves[i].port_id); > + return -1; > + } > + } > return 0; > } > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting 2022-02-15 9:26 ` Min Hu (Connor) @ 2022-02-15 10:32 ` Wan Junjie 0 siblings, 0 replies; 13+ messages in thread From: Wan Junjie @ 2022-02-15 10:32 UTC (permalink / raw) To: Min Hu (Connor); +Cc: dev, Chas Williams Hi Min Hu, On Tue, Feb 15, 2022 at 5:26 PM Min Hu (Connor) <humin29@huawei.com> wrote: > > Hi, wan, > two comments: > 1. wrong spelling in headline Will fix it in v2. > 2. comment info should be detailed, such as, > show the testpmd commands about how to operate > in this two situations. > Testpmd did not show the issue here. to reproduce, we need call the api by the sequence: rte_eth_dev_configure rte_eth_dev_set_mtu queue setup and start ... for dpdk application, pass the device --vdev="net_bonding0,mode=2,slave=0000:af:00.0" Then application will complain for the slave device, that "Port 0 must be configured before MTU set" Will add it to commit msg in v2 Regards, Wan Junjie > Thanks. > > 在 2022/2/14 16:13, Wan Junjie 写道: > > When initializing a bonding device with all slaves in vdev string, > > which means bonding device initilizes slaves, instead of initializing > > slaves before creating the bonding device, it will fail to call > > set_mtu api for the bonding device. > > > > Fixes:b26bee1 ("ethdev: forbid MTU set before device configure") > > > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > > --- > > app/test/test_link_bonding.c | 4 +++ > > app/test/test_link_bonding_rssconf.c | 4 +++ > > drivers/net/bonding/eth_bond_private.h | 4 +++ > > drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ > > drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- > > 5 files changed, 55 insertions(+), 6 deletions(-) > > > > diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c > > index dc6fc46b9c..12c50ef393 100644 > > --- a/app/test/test_link_bonding.c > > +++ b/app/test/test_link_bonding.c > > @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t en_isr) > > test_params->nb_tx_q, &default_pmd_conf), > > "rte_eth_dev_configure for port %d failed", port_id); > > > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > > + "rte_eth_dev_set_mtu for port %d failed", port_id); > > + > > for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) > > TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, RX_RING_SIZE, > > rte_eth_dev_socket_id(port_id), &rx_conf_default, > > diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c > > index f9eae93973..7228965ced 100644 > > --- a/app/test/test_link_bonding_rssconf.c > > +++ b/app/test/test_link_bonding_rssconf.c > > @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct rte_eth_conf *eth_conf, > > RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure device %u", > > port_id); > > > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > > + "rte_eth_dev_set_mtu for port %d failed", port_id); > > + > > for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { > > TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, RXTX_RING_SIZE, > > rte_eth_dev_socket_id(port_id), NULL, > > diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h > > index 156335c425..8222e3cd38 100644 > > --- a/drivers/net/bonding/eth_bond_private.h > > +++ b/drivers/net/bonding/eth_bond_private.h > > @@ -246,6 +246,10 @@ int > > slave_configure(struct rte_eth_dev *bonded_eth_dev, > > struct rte_eth_dev *slave_eth_dev); > > > > +int > > +slave_start(struct rte_eth_dev *bonded_eth_dev, > > + struct rte_eth_dev *slave_eth_dev); > > + > > void > > slave_remove(struct bond_dev_private *internals, > > struct rte_eth_dev *slave_eth_dev); > > diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c > > index b78867b125..4ac191c468 100644 > > --- a/drivers/net/bonding/rte_eth_bond_api.c > > +++ b/drivers/net/bonding/rte_eth_bond_api.c > > @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) > > slave_port_id); > > return -1; > > } > > + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { > > + internals->slave_count--; > > + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", > > + slave_port_id); > > + return -1; > > + } > > } > > > > /* Update all slave devices MACs */ > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > > index aa6519f83c..c31b723071 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -1678,14 +1678,10 @@ int > > slave_configure(struct rte_eth_dev *bonded_eth_dev, > > struct rte_eth_dev *slave_eth_dev) > > { > > - struct bond_rx_queue *bd_rx_q; > > - struct bond_tx_queue *bd_tx_q; > > uint16_t nb_rx_queues; > > uint16_t nb_tx_queues; > > > > int errval; > > - uint16_t q_id; > > - struct rte_flow_error flow_error; > > > > struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > > > @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > > slave_eth_dev->data->port_id, errval); > > return errval; > > } > > + return 0; > > +} > > + > > +int > > +slave_start(struct rte_eth_dev *bonded_eth_dev, > > + struct rte_eth_dev *slave_eth_dev) > > +{ > > + int errval = 0; > > + struct bond_rx_queue *bd_rx_q; > > + struct bond_tx_queue *bd_tx_q; > > + uint16_t q_id; > > + struct rte_flow_error flow_error; > > + struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > > > /* Setup Rx Queues */ > > for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { > > @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > > return errval; > > } > > > > - if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) > > - rte_flow_destroy(slave_eth_dev->data->port_id, > > + if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) { > > + errval = rte_flow_destroy(slave_eth_dev->data->port_id, > > internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], > > &flow_error); > > + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)", > > + slave_eth_dev->data->port_id, errval); > > + } > > > > errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, > > slave_eth_dev->data->port_id); > > @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) > > internals->slaves[i].port_id); > > goto out_err; > > } > > + if (slave_start(eth_dev, slave_ethdev) != 0) { > > + RTE_BOND_LOG(ERR, > > + "bonded port (%d) failed to start slave device (%d)", > > + eth_dev->data->port_id, > > + internals->slaves[i].port_id); > > + goto out_err; > > + } > > /* We will need to poll for link status if any slave doesn't > > * support interrupts > > */ > > @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) > > return -1; > > } > > > > + /* configure slaves so we can pass mtu setting */ > > + for (i = 0; i < internals->slave_count; i++) { > > + struct rte_eth_dev *slave_ethdev = > > + &(rte_eth_devices[internals->slaves[i].port_id]); > > + if (slave_configure(dev, slave_ethdev) != 0) { > > + RTE_BOND_LOG(ERR, > > + "bonded port (%d) failed to configure slave device (%d)", > > + dev->data->port_id, > > + internals->slaves[i].port_id); > > + return -1; > > + } > > + } > > return 0; > > } > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] net/bonding: fix slaves initializing on mtu setting 2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie 2022-02-15 9:26 ` Min Hu (Connor) @ 2022-02-15 10:59 ` Wan Junjie 2022-02-16 13:22 ` Ferruh Yigit 1 sibling, 1 reply; 13+ messages in thread From: Wan Junjie @ 2022-02-15 10:59 UTC (permalink / raw) To: dev; +Cc: Chas Williams, Min Hu, Wan Junjie If a initial process for the bonding device is like: rte_eth_dev_configure rte_eth_dev_set_mtu queue setup and start, etc. Pass the vdev args to application, and init bonding device only. -a 0000:af:00.0 --vdev="net_bonding0,mode=2,slave=0000:af:00.0" It will fail and compain for the slave device "Port 0 must be configured before MTU set" Test can be reproduced with ovs. Fixes: b26bee1 ("ethdev: forbid MTU set before device configure") Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> --- v2: fix typo in commit add reproduce procedure --- app/test/test_link_bonding.c | 4 +++ app/test/test_link_bonding_rssconf.c | 4 +++ drivers/net/bonding/eth_bond_private.h | 4 +++ drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c index dc6fc46b9c..12c50ef393 100644 --- a/app/test/test_link_bonding.c +++ b/app/test/test_link_bonding.c @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t en_isr) test_params->nb_tx_q, &default_pmd_conf), "rte_eth_dev_configure for port %d failed", port_id); + int ret = rte_eth_dev_set_mtu(port_id, 1550); + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, + "rte_eth_dev_set_mtu for port %d failed", port_id); + for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, RX_RING_SIZE, rte_eth_dev_socket_id(port_id), &rx_conf_default, diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c index f9eae93973..7228965ced 100644 --- a/app/test/test_link_bonding_rssconf.c +++ b/app/test/test_link_bonding_rssconf.c @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct rte_eth_conf *eth_conf, RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure device %u", port_id); + int ret = rte_eth_dev_set_mtu(port_id, 1550); + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, + "rte_eth_dev_set_mtu for port %d failed", port_id); + for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, RXTX_RING_SIZE, rte_eth_dev_socket_id(port_id), NULL, diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h index 156335c425..8222e3cd38 100644 --- a/drivers/net/bonding/eth_bond_private.h +++ b/drivers/net/bonding/eth_bond_private.h @@ -246,6 +246,10 @@ int slave_configure(struct rte_eth_dev *bonded_eth_dev, struct rte_eth_dev *slave_eth_dev); +int +slave_start(struct rte_eth_dev *bonded_eth_dev, + struct rte_eth_dev *slave_eth_dev); + void slave_remove(struct bond_dev_private *internals, struct rte_eth_dev *slave_eth_dev); diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index b78867b125..4ac191c468 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) slave_port_id); return -1; } + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { + internals->slave_count--; + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", + slave_port_id); + return -1; + } } /* Update all slave devices MACs */ diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index aa6519f83c..c31b723071 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -1678,14 +1678,10 @@ int slave_configure(struct rte_eth_dev *bonded_eth_dev, struct rte_eth_dev *slave_eth_dev) { - struct bond_rx_queue *bd_rx_q; - struct bond_tx_queue *bd_tx_q; uint16_t nb_rx_queues; uint16_t nb_tx_queues; int errval; - uint16_t q_id; - struct rte_flow_error flow_error; struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, slave_eth_dev->data->port_id, errval); return errval; } + return 0; +} + +int +slave_start(struct rte_eth_dev *bonded_eth_dev, + struct rte_eth_dev *slave_eth_dev) +{ + int errval = 0; + struct bond_rx_queue *bd_rx_q; + struct bond_tx_queue *bd_tx_q; + uint16_t q_id; + struct rte_flow_error flow_error; + struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; /* Setup Rx Queues */ for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, return errval; } - if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) - rte_flow_destroy(slave_eth_dev->data->port_id, + if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) { + errval = rte_flow_destroy(slave_eth_dev->data->port_id, internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], &flow_error); + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)", + slave_eth_dev->data->port_id, errval); + } errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, slave_eth_dev->data->port_id); @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) internals->slaves[i].port_id); goto out_err; } + if (slave_start(eth_dev, slave_ethdev) != 0) { + RTE_BOND_LOG(ERR, + "bonded port (%d) failed to start slave device (%d)", + eth_dev->data->port_id, + internals->slaves[i].port_id); + goto out_err; + } /* We will need to poll for link status if any slave doesn't * support interrupts */ @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) return -1; } + /* configure slaves so we can pass mtu setting */ + for (i = 0; i < internals->slave_count; i++) { + struct rte_eth_dev *slave_ethdev = + &(rte_eth_devices[internals->slaves[i].port_id]); + if (slave_configure(dev, slave_ethdev) != 0) { + RTE_BOND_LOG(ERR, + "bonded port (%d) failed to configure slave device (%d)", + dev->data->port_id, + internals->slaves[i].port_id); + return -1; + } + } return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] net/bonding: fix slaves initializing on mtu setting 2022-02-15 10:59 ` [PATCH v2 2/2] net/bonding: fix slaves initializing " Wan Junjie @ 2022-02-16 13:22 ` Ferruh Yigit 2022-02-17 8:55 ` Min Hu (Connor) 0 siblings, 1 reply; 13+ messages in thread From: Ferruh Yigit @ 2022-02-16 13:22 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams, Min Hu On 2/15/2022 10:59 AM, Wan Junjie wrote: > If a initial process for the bonding device is like: > rte_eth_dev_configure > rte_eth_dev_set_mtu > queue setup and start, etc. > > Pass the vdev args to application, and init bonding device only. > -a 0000:af:00.0 --vdev="net_bonding0,mode=2,slave=0000:af:00.0" > > It will fail and compain for the slave device > "Port 0 must be configured before MTU set" > Change looks good to me, previously configure(), queue setup & start() all done in .dev_start dev_ops. Now configure part separated and .dev_configure also calls the configure part, .dev_start still does the same. Conner, Chas, if there is no objection from you, I will proceed with the patch and merge it soon. > Test can be reproduced with ovs. > > Fixes: b26bee1 ("ethdev: forbid MTU set before device configure") > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- > v2: > fix typo in commit > add reproduce procedure > --- > app/test/test_link_bonding.c | 4 +++ > app/test/test_link_bonding_rssconf.c | 4 +++ > drivers/net/bonding/eth_bond_private.h | 4 +++ > drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ > drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- > 5 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c > index dc6fc46b9c..12c50ef393 100644 > --- a/app/test/test_link_bonding.c > +++ b/app/test/test_link_bonding.c > @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, uint8_t en_isr) > test_params->nb_tx_q, &default_pmd_conf), > "rte_eth_dev_configure for port %d failed", port_id); > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > + "rte_eth_dev_set_mtu for port %d failed", port_id); > + > for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) > TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, RX_RING_SIZE, > rte_eth_dev_socket_id(port_id), &rx_conf_default, > diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c > index f9eae93973..7228965ced 100644 > --- a/app/test/test_link_bonding_rssconf.c > +++ b/app/test/test_link_bonding_rssconf.c > @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct rte_eth_conf *eth_conf, > RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure device %u", > port_id); > > + int ret = rte_eth_dev_set_mtu(port_id, 1550); > + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, > + "rte_eth_dev_set_mtu for port %d failed", port_id); > + > for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { > TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, RXTX_RING_SIZE, > rte_eth_dev_socket_id(port_id), NULL, > diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h > index 156335c425..8222e3cd38 100644 > --- a/drivers/net/bonding/eth_bond_private.h > +++ b/drivers/net/bonding/eth_bond_private.h > @@ -246,6 +246,10 @@ int > slave_configure(struct rte_eth_dev *bonded_eth_dev, > struct rte_eth_dev *slave_eth_dev); > > +int > +slave_start(struct rte_eth_dev *bonded_eth_dev, > + struct rte_eth_dev *slave_eth_dev); > + > void > slave_remove(struct bond_dev_private *internals, > struct rte_eth_dev *slave_eth_dev); > diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c > index b78867b125..4ac191c468 100644 > --- a/drivers/net/bonding/rte_eth_bond_api.c > +++ b/drivers/net/bonding/rte_eth_bond_api.c > @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) > slave_port_id); > return -1; > } > + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { > + internals->slave_count--; > + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", > + slave_port_id); > + return -1; > + } > } > > /* Update all slave devices MACs */ > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index aa6519f83c..c31b723071 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1678,14 +1678,10 @@ int > slave_configure(struct rte_eth_dev *bonded_eth_dev, > struct rte_eth_dev *slave_eth_dev) > { > - struct bond_rx_queue *bd_rx_q; > - struct bond_tx_queue *bd_tx_q; > uint16_t nb_rx_queues; > uint16_t nb_tx_queues; > > int errval; > - uint16_t q_id; > - struct rte_flow_error flow_error; > > struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > slave_eth_dev->data->port_id, errval); > return errval; > } > + return 0; > +} > + > +int > +slave_start(struct rte_eth_dev *bonded_eth_dev, > + struct rte_eth_dev *slave_eth_dev) > +{ > + int errval = 0; > + struct bond_rx_queue *bd_rx_q; > + struct bond_tx_queue *bd_tx_q; > + uint16_t q_id; > + struct rte_flow_error flow_error; > + struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > > /* Setup Rx Queues */ > for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { > @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, > return errval; > } > > - if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) > - rte_flow_destroy(slave_eth_dev->data->port_id, > + if (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] != NULL) { > + errval = rte_flow_destroy(slave_eth_dev->data->port_id, > internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], > &flow_error); > + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)", > + slave_eth_dev->data->port_id, errval); > + } > > errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, > slave_eth_dev->data->port_id); > @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) > internals->slaves[i].port_id); > goto out_err; > } > + if (slave_start(eth_dev, slave_ethdev) != 0) { > + RTE_BOND_LOG(ERR, > + "bonded port (%d) failed to start slave device (%d)", > + eth_dev->data->port_id, > + internals->slaves[i].port_id); > + goto out_err; > + } > /* We will need to poll for link status if any slave doesn't > * support interrupts > */ > @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) > return -1; > } > > + /* configure slaves so we can pass mtu setting */ > + for (i = 0; i < internals->slave_count; i++) { > + struct rte_eth_dev *slave_ethdev = > + &(rte_eth_devices[internals->slaves[i].port_id]); > + if (slave_configure(dev, slave_ethdev) != 0) { > + RTE_BOND_LOG(ERR, > + "bonded port (%d) failed to configure slave device (%d)", > + dev->data->port_id, > + internals->slaves[i].port_id); > + return -1; > + } > + } > return 0; > } > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] net/bonding: fix slaves initializing on mtu setting 2022-02-16 13:22 ` Ferruh Yigit @ 2022-02-17 8:55 ` Min Hu (Connor) 0 siblings, 0 replies; 13+ messages in thread From: Min Hu (Connor) @ 2022-02-17 8:55 UTC (permalink / raw) To: Ferruh Yigit, Wan Junjie, dev; +Cc: Chas Williams 在 2022/2/16 21:22, Ferruh Yigit 写道: > On 2/15/2022 10:59 AM, Wan Junjie wrote: >> If a initial process for the bonding device is like: >> rte_eth_dev_configure >> rte_eth_dev_set_mtu >> queue setup and start, etc. >> >> Pass the vdev args to application, and init bonding device only. >> -a 0000:af:00.0 --vdev="net_bonding0,mode=2,slave=0000:af:00.0" >> >> It will fail and compain for the slave device >> "Port 0 must be configured before MTU set" >> > > Change looks good to me, previously configure(), queue setup & start() > all done in .dev_start dev_ops. > > Now configure part separated and .dev_configure also calls the configure > part, .dev_start still does the same. > > Conner, Chas, if there is no objection from you, I will proceed with > the patch and merge it soon. > Hi, Ferruh, yeah, I am OK with it. thanks. >> Test can be reproduced with ovs. >> >> Fixes: b26bee1 ("ethdev: forbid MTU set before device configure") >> >> Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > > Tested-by: Ferruh Yigit <ferruh.yigit@intel.com> > >> --- >> v2: >> fix typo in commit >> add reproduce procedure >> --- >> app/test/test_link_bonding.c | 4 +++ >> app/test/test_link_bonding_rssconf.c | 4 +++ >> drivers/net/bonding/eth_bond_private.h | 4 +++ >> drivers/net/bonding/rte_eth_bond_api.c | 6 ++++ >> drivers/net/bonding/rte_eth_bond_pmd.c | 43 ++++++++++++++++++++++---- >> 5 files changed, 55 insertions(+), 6 deletions(-) >> >> diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c >> index dc6fc46b9c..12c50ef393 100644 >> --- a/app/test/test_link_bonding.c >> +++ b/app/test/test_link_bonding.c >> @@ -181,6 +181,10 @@ configure_ethdev(uint16_t port_id, uint8_t start, >> uint8_t en_isr) >> test_params->nb_tx_q, &default_pmd_conf), >> "rte_eth_dev_configure for port %d failed", port_id); >> + int ret = rte_eth_dev_set_mtu(port_id, 1550); >> + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, >> + "rte_eth_dev_set_mtu for port %d failed", port_id); >> + >> for (q_id = 0; q_id < test_params->nb_rx_q; q_id++) >> TEST_ASSERT_SUCCESS(rte_eth_rx_queue_setup(port_id, q_id, >> RX_RING_SIZE, >> rte_eth_dev_socket_id(port_id), &rx_conf_default, >> diff --git a/app/test/test_link_bonding_rssconf.c >> b/app/test/test_link_bonding_rssconf.c >> index f9eae93973..7228965ced 100644 >> --- a/app/test/test_link_bonding_rssconf.c >> +++ b/app/test/test_link_bonding_rssconf.c >> @@ -128,6 +128,10 @@ configure_ethdev(uint16_t port_id, struct >> rte_eth_conf *eth_conf, >> RXTX_QUEUE_COUNT, eth_conf) == 0, "Failed to configure >> device %u", >> port_id); >> + int ret = rte_eth_dev_set_mtu(port_id, 1550); >> + RTE_TEST_ASSERT(ret == 0 || ret == -ENOTSUP, >> + "rte_eth_dev_set_mtu for port %d failed", port_id); >> + >> for (rxq = 0; rxq < RXTX_QUEUE_COUNT; rxq++) { >> TEST_ASSERT(rte_eth_rx_queue_setup(port_id, rxq, >> RXTX_RING_SIZE, >> rte_eth_dev_socket_id(port_id), NULL, >> diff --git a/drivers/net/bonding/eth_bond_private.h >> b/drivers/net/bonding/eth_bond_private.h >> index 156335c425..8222e3cd38 100644 >> --- a/drivers/net/bonding/eth_bond_private.h >> +++ b/drivers/net/bonding/eth_bond_private.h >> @@ -246,6 +246,10 @@ int >> slave_configure(struct rte_eth_dev *bonded_eth_dev, >> struct rte_eth_dev *slave_eth_dev); >> +int >> +slave_start(struct rte_eth_dev *bonded_eth_dev, >> + struct rte_eth_dev *slave_eth_dev); >> + >> void >> slave_remove(struct bond_dev_private *internals, >> struct rte_eth_dev *slave_eth_dev); >> diff --git a/drivers/net/bonding/rte_eth_bond_api.c >> b/drivers/net/bonding/rte_eth_bond_api.c >> index b78867b125..4ac191c468 100644 >> --- a/drivers/net/bonding/rte_eth_bond_api.c >> +++ b/drivers/net/bonding/rte_eth_bond_api.c >> @@ -566,6 +566,12 @@ __eth_bond_slave_add_lock_free(uint16_t >> bonded_port_id, uint16_t slave_port_id) >> slave_port_id); >> return -1; >> } >> + if (slave_start(bonded_eth_dev, slave_eth_dev) != 0) { >> + internals->slave_count--; >> + RTE_BOND_LOG(ERR, "rte_bond_slaves_start: port=%d", >> + slave_port_id); >> + return -1; >> + } >> } >> /* Update all slave devices MACs */ >> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c >> b/drivers/net/bonding/rte_eth_bond_pmd.c >> index aa6519f83c..c31b723071 100644 >> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >> @@ -1678,14 +1678,10 @@ int >> slave_configure(struct rte_eth_dev *bonded_eth_dev, >> struct rte_eth_dev *slave_eth_dev) >> { >> - struct bond_rx_queue *bd_rx_q; >> - struct bond_tx_queue *bd_tx_q; >> uint16_t nb_rx_queues; >> uint16_t nb_tx_queues; >> int errval; >> - uint16_t q_id; >> - struct rte_flow_error flow_error; >> struct bond_dev_private *internals = >> bonded_eth_dev->data->dev_private; >> @@ -1758,6 +1754,19 @@ slave_configure(struct rte_eth_dev >> *bonded_eth_dev, >> slave_eth_dev->data->port_id, errval); >> return errval; >> } >> + return 0; >> +} >> + >> +int >> +slave_start(struct rte_eth_dev *bonded_eth_dev, >> + struct rte_eth_dev *slave_eth_dev) >> +{ >> + int errval = 0; >> + struct bond_rx_queue *bd_rx_q; >> + struct bond_tx_queue *bd_tx_q; >> + uint16_t q_id; >> + struct rte_flow_error flow_error; >> + struct bond_dev_private *internals = >> bonded_eth_dev->data->dev_private; >> /* Setup Rx Queues */ >> for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) { >> @@ -1806,10 +1815,13 @@ slave_configure(struct rte_eth_dev >> *bonded_eth_dev, >> return errval; >> } >> - if >> (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] >> != NULL) >> - rte_flow_destroy(slave_eth_dev->data->port_id, >> + if >> (internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id] >> != NULL) { >> + errval = rte_flow_destroy(slave_eth_dev->data->port_id, >> >> internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], >> &flow_error); >> + RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: >> port=%d, err (%d)", >> + slave_eth_dev->data->port_id, errval); >> + } >> errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev, >> slave_eth_dev->data->port_id); >> @@ -2001,6 +2013,13 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) >> internals->slaves[i].port_id); >> goto out_err; >> } >> + if (slave_start(eth_dev, slave_ethdev) != 0) { >> + RTE_BOND_LOG(ERR, >> + "bonded port (%d) failed to start slave device (%d)", >> + eth_dev->data->port_id, >> + internals->slaves[i].port_id); >> + goto out_err; >> + } >> /* We will need to poll for link status if any slave doesn't >> * support interrupts >> */ >> @@ -3849,6 +3868,18 @@ bond_ethdev_configure(struct rte_eth_dev *dev) >> return -1; >> } >> + /* configure slaves so we can pass mtu setting */ >> + for (i = 0; i < internals->slave_count; i++) { >> + struct rte_eth_dev *slave_ethdev = >> + &(rte_eth_devices[internals->slaves[i].port_id]); >> + if (slave_configure(dev, slave_ethdev) != 0) { >> + RTE_BOND_LOG(ERR, >> + "bonded port (%d) failed to configure slave device >> (%d)", >> + dev->data->port_id, >> + internals->slaves[i].port_id); >> + return -1; >> + } >> + } >> return 0; >> } > > . ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] net/bonding: add error hint for invald args 2022-02-14 8:13 [PATCH 1/2] net/bonding: add error hint for invald args Wan Junjie 2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie @ 2022-02-15 9:12 ` Min Hu (Connor) 2022-02-15 9:50 ` [External] " Wan Junjie 2022-02-15 10:56 ` [PATCH v2 1/2] net/bonding: add error hint for invalid args Wan Junjie 2 siblings, 1 reply; 13+ messages in thread From: Min Hu (Connor) @ 2022-02-15 9:12 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams Hi, wan, The headline has spelling errors. 在 2022/2/14 16:13, Wan Junjie 写道: > When invalid args exsit, application exits with no error hint. > Adding a log message here will help users to know the reson. > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index bfa931098e..aa6519f83c 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -3439,8 +3439,10 @@ bond_probe(struct rte_vdev_device *dev) > > kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), > pmd_bond_init_valid_arguments); > - if (kvlist == NULL) > + if (kvlist == NULL) { > + RTE_BOND_LOG(ERR, "Invalid args in %s", rte_vdev_device_args(dev)); > return -1; > + } > > /* Parse link bonding mode */ > if (rte_kvargs_count(kvlist, PMD_BOND_MODE_KVARG) == 1) { > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH 1/2] net/bonding: add error hint for invald args 2022-02-15 9:12 ` [PATCH 1/2] net/bonding: add error hint for invald args Min Hu (Connor) @ 2022-02-15 9:50 ` Wan Junjie 0 siblings, 0 replies; 13+ messages in thread From: Wan Junjie @ 2022-02-15 9:50 UTC (permalink / raw) To: Min Hu (Connor); +Cc: dev, Chas Williams Hi Min Hu, Thanks for the reply Will fix it in v2. Regards, Wan Junjie On Tue, Feb 15, 2022 at 5:12 PM Min Hu (Connor) <humin29@huawei.com> wrote: > > Hi, wan, > The headline has spelling errors. > > 在 2022/2/14 16:13, Wan Junjie 写道: > > When invalid args exsit, application exits with no error hint. > > Adding a log message here will help users to know the reson. > > > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > > --- > > drivers/net/bonding/rte_eth_bond_pmd.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > > index bfa931098e..aa6519f83c 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -3439,8 +3439,10 @@ bond_probe(struct rte_vdev_device *dev) > > > > kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), > > pmd_bond_init_valid_arguments); > > - if (kvlist == NULL) > > + if (kvlist == NULL) { > > + RTE_BOND_LOG(ERR, "Invalid args in %s", rte_vdev_device_args(dev)); > > return -1; > > + } > > > > /* Parse link bonding mode */ > > if (rte_kvargs_count(kvlist, PMD_BOND_MODE_KVARG) == 1) { > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] net/bonding: add error hint for invalid args 2022-02-14 8:13 [PATCH 1/2] net/bonding: add error hint for invald args Wan Junjie 2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie 2022-02-15 9:12 ` [PATCH 1/2] net/bonding: add error hint for invald args Min Hu (Connor) @ 2022-02-15 10:56 ` Wan Junjie 2022-02-15 11:56 ` Ferruh Yigit 2022-02-16 0:29 ` Min Hu (Connor) 2 siblings, 2 replies; 13+ messages in thread From: Wan Junjie @ 2022-02-15 10:56 UTC (permalink / raw) To: dev; +Cc: Chas Williams, Min Hu, Wan Junjie When invalid args exist, application exits with no error hint. Adding a log message here will help users to know the reason. Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> --- v2: fix typo in commit msg --- drivers/net/bonding/rte_eth_bond_pmd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index bfa931098e..aa6519f83c 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -3439,8 +3439,10 @@ bond_probe(struct rte_vdev_device *dev) kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), pmd_bond_init_valid_arguments); - if (kvlist == NULL) + if (kvlist == NULL) { + RTE_BOND_LOG(ERR, "Invalid args in %s", rte_vdev_device_args(dev)); return -1; + } /* Parse link bonding mode */ if (rte_kvargs_count(kvlist, PMD_BOND_MODE_KVARG) == 1) { -- 2.33.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] net/bonding: add error hint for invalid args 2022-02-15 10:56 ` [PATCH v2 1/2] net/bonding: add error hint for invalid args Wan Junjie @ 2022-02-15 11:56 ` Ferruh Yigit 2022-02-17 11:57 ` Ferruh Yigit 2022-02-16 0:29 ` Min Hu (Connor) 1 sibling, 1 reply; 13+ messages in thread From: Ferruh Yigit @ 2022-02-15 11:56 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams, Min Hu On 2/15/2022 10:56 AM, Wan Junjie wrote: > When invalid args exist, application exits with no error hint. > Adding a log message here will help users to know the reason. > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] net/bonding: add error hint for invalid args 2022-02-15 11:56 ` Ferruh Yigit @ 2022-02-17 11:57 ` Ferruh Yigit 0 siblings, 0 replies; 13+ messages in thread From: Ferruh Yigit @ 2022-02-17 11:57 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams, Min Hu On 2/15/2022 11:56 AM, Ferruh Yigit wrote: > On 2/15/2022 10:56 AM, Wan Junjie wrote: >> When invalid args exist, application exits with no error hint. >> Adding a log message here will help users to know the reason. >> >> Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> Series applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] net/bonding: add error hint for invalid args 2022-02-15 10:56 ` [PATCH v2 1/2] net/bonding: add error hint for invalid args Wan Junjie 2022-02-15 11:56 ` Ferruh Yigit @ 2022-02-16 0:29 ` Min Hu (Connor) 1 sibling, 0 replies; 13+ messages in thread From: Min Hu (Connor) @ 2022-02-16 0:29 UTC (permalink / raw) To: Wan Junjie, dev; +Cc: Chas Williams Acked-by: Min Hu (Connor) <humin29@huawei.com> 在 2022/2/15 18:56, Wan Junjie 写道: > When invalid args exist, application exits with no error hint. > Adding a log message here will help users to know the reason. > > Signed-off-by: Wan Junjie <wanjunjie@bytedance.com> > --- > v2: fix typo in commit msg > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index bfa931098e..aa6519f83c 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -3439,8 +3439,10 @@ bond_probe(struct rte_vdev_device *dev) > > kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), > pmd_bond_init_valid_arguments); > - if (kvlist == NULL) > + if (kvlist == NULL) { > + RTE_BOND_LOG(ERR, "Invalid args in %s", rte_vdev_device_args(dev)); > return -1; > + } > > /* Parse link bonding mode */ > if (rte_kvargs_count(kvlist, PMD_BOND_MODE_KVARG) == 1) { > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-02-17 11:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-14 8:13 [PATCH 1/2] net/bonding: add error hint for invald args Wan Junjie 2022-02-14 8:13 ` [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting Wan Junjie 2022-02-15 9:26 ` Min Hu (Connor) 2022-02-15 10:32 ` [External] " Wan Junjie 2022-02-15 10:59 ` [PATCH v2 2/2] net/bonding: fix slaves initializing " Wan Junjie 2022-02-16 13:22 ` Ferruh Yigit 2022-02-17 8:55 ` Min Hu (Connor) 2022-02-15 9:12 ` [PATCH 1/2] net/bonding: add error hint for invald args Min Hu (Connor) 2022-02-15 9:50 ` [External] " Wan Junjie 2022-02-15 10:56 ` [PATCH v2 1/2] net/bonding: add error hint for invalid args Wan Junjie 2022-02-15 11:56 ` Ferruh Yigit 2022-02-17 11:57 ` Ferruh Yigit 2022-02-16 0:29 ` Min Hu (Connor)
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).