diff mbox series

[v2] net/bonding: fix error in bonding mode 4 with dedicated queues enabled

Message ID 20220929040551.120717-1-usman.tanveer@emumba.com (mailing list archive)
State Accepted, archived
Delegated to: Andrew Rybchenko
Headers show
Series [v2] net/bonding: fix error in bonding mode 4 with dedicated queues enabled | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing warning Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Usman Tanveer Sept. 29, 2022, 4:05 a.m. UTC
when dedicated queues are enable with bonding mode 4 (mlx5), the
application sets the flow, which cannot be set if the device is not
started. This fixed the issue by starting the device just before
setting the flow. Because device should be started to set the flow.
Also it does not effect other driver codes (I have tried on ixgbe).

Bugzilla ID: 759

Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Comments

Usman Tanveer Sept. 29, 2022, 4:16 a.m. UTC | #1
We cannot configure the queues when the device is started. So, that
part is still before the device starts. I just moved
bond_ethdev_8023ad_flow_set() after the device start.


On Thu, Sep 29, 2022 at 9:05 AM Usman Tanveer <usman.tanveer@emumba.com> wrote:
>
> when dedicated queues are enable with bonding mode 4 (mlx5), the
> application sets the flow, which cannot be set if the device is not
> started. This fixed the issue by starting the device just before
> setting the flow. Because device should be started to set the flow.
> Also it does not effect other driver codes (I have tried on ixgbe).
>
> Bugzilla ID: 759
>
> Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index fd2d95a751..69cbbe19ff 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1828,7 +1828,18 @@ slave_start(struct rte_eth_dev *bonded_eth_dev,
>                         RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)",
>                                 slave_eth_dev->data->port_id, errval);
>                 }
> +       }
> +
> +       /* Start device */
> +       errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
> +       if (errval != 0) {
> +               RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
> +                               slave_eth_dev->data->port_id, errval);
> +               return -1;
> +       }
>
> +       if (internals->mode == BONDING_MODE_8023AD &&
> +                       internals->mode4.dedicated_queues.enabled == 1) {
>                 errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev,
>                                 slave_eth_dev->data->port_id);
>                 if (errval != 0) {
> @@ -1839,14 +1850,6 @@ slave_start(struct rte_eth_dev *bonded_eth_dev,
>                 }
>         }
>
> -       /* Start device */
> -       errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
> -       if (errval != 0) {
> -               RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
> -                               slave_eth_dev->data->port_id, errval);
> -               return -1;
> -       }
> -
>         /* If RSS is enabled for bonding, synchronize RETA */
>         if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
>                 int i;
> --
> 2.25.1
>
Chas Williams Oct. 15, 2022, 3:28 p.m. UTC | #2
Thanks for making the change, IMHO this is much clearer.

Signed-off-by: Chas Williams <3chas3@gmail.com>

On 9/29/22 00:05, Usman Tanveer wrote:
> when dedicated queues are enable with bonding mode 4 (mlx5), the
> application sets the flow, which cannot be set if the device is not
> started. This fixed the issue by starting the device just before
> setting the flow. Because device should be started to set the flow.
> Also it does not effect other driver codes (I have tried on ixgbe).
> 
> Bugzilla ID: 759
> 
> Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index fd2d95a751..69cbbe19ff 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1828,7 +1828,18 @@ slave_start(struct rte_eth_dev *bonded_eth_dev,
>   			RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)",
>   				slave_eth_dev->data->port_id, errval);
>   		}
> +	}
> +
> +	/* Start device */
> +	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
> +	if (errval != 0) {
> +		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
> +				slave_eth_dev->data->port_id, errval);
> +		return -1;
> +	}
>   
> +	if (internals->mode == BONDING_MODE_8023AD &&
> +			internals->mode4.dedicated_queues.enabled == 1) {
>   		errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev,
>   				slave_eth_dev->data->port_id);
>   		if (errval != 0) {
> @@ -1839,14 +1850,6 @@ slave_start(struct rte_eth_dev *bonded_eth_dev,
>   		}
>   	}
>   
> -	/* Start device */
> -	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
> -	if (errval != 0) {
> -		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
> -				slave_eth_dev->data->port_id, errval);
> -		return -1;
> -	}
> -
>   	/* If RSS is enabled for bonding, synchronize RETA */
>   	if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
>   		int i;
Andrew Rybchenko Oct. 17, 2022, 8:28 a.m. UTC | #3
On 10/15/22 18:28, Chas Williams wrote:
> Thanks for making the change, IMHO this is much clearer.
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>

Taking above sentence into account I treat it as:
Acked-by: Chas Williams <3chas3@gmail.com>

> 
> On 9/29/22 00:05, Usman Tanveer wrote:
>> when dedicated queues are enable with bonding mode 4 (mlx5), the
>> application sets the flow, which cannot be set if the device is not
>> started. This fixed the issue by starting the device just before
>> setting the flow. Because device should be started to set the flow.
>> Also it does not effect other driver codes (I have tried on ixgbe).
>>
>> Bugzilla ID: 759
>>
>> Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>

Applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index fd2d95a751..69cbbe19ff 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1828,7 +1828,18 @@  slave_start(struct rte_eth_dev *bonded_eth_dev,
 			RTE_BOND_LOG(ERR, "bond_ethdev_8023ad_flow_destroy: port=%d, err (%d)",
 				slave_eth_dev->data->port_id, errval);
 		}
+	}
+
+	/* Start device */
+	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
+	if (errval != 0) {
+		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
+				slave_eth_dev->data->port_id, errval);
+		return -1;
+	}
 
+	if (internals->mode == BONDING_MODE_8023AD &&
+			internals->mode4.dedicated_queues.enabled == 1) {
 		errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev,
 				slave_eth_dev->data->port_id);
 		if (errval != 0) {
@@ -1839,14 +1850,6 @@  slave_start(struct rte_eth_dev *bonded_eth_dev,
 		}
 	}
 
-	/* Start device */
-	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
-	if (errval != 0) {
-		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
-				slave_eth_dev->data->port_id, errval);
-		return -1;
-	}
-
 	/* If RSS is enabled for bonding, synchronize RETA */
 	if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS) {
 		int i;