DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Min Hu (Connor)" <humin29@huawei.com>
To: Wan Junjie <wanjunjie@bytedance.com>, <dev@dpdk.org>
Cc: Chas Williams <chas3@att.com>
Subject: Re: [PATCH 2/2] net/bonding: fix slaves initilizing on mtu setting
Date: Tue, 15 Feb 2022 17:26:16 +0800	[thread overview]
Message-ID: <bf2d6fa1-013d-b04d-6c8b-aa0b43d07b0c@huawei.com> (raw)
In-Reply-To: <20220214081344.13637-2-wanjunjie@bytedance.com>

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;
>   }
>   
> 

  reply	other threads:[~2022-02-15  9:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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) [this message]
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)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bf2d6fa1-013d-b04d-6c8b-aa0b43d07b0c@huawei.com \
    --to=humin29@huawei.com \
    --cc=chas3@att.com \
    --cc=dev@dpdk.org \
    --cc=wanjunjie@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).