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
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
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) {
>
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;
> }
>
>
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) {
> >
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; > > } > > > >
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
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
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>
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) {
>
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; > } >
在 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; >> } > > .
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.