[PATCH] app/test: fix retest link bonding fail

[PATCH] app/test: fix retest link bonding fail

[Jie Hai]

The testcase "test_close_bonding_device" closes the bonding
port shared by several cases. After closed, the port is in
RTE_ETH_DEV_UNUSED state, and could not be used by other cases
anymore. If retest the "link_bonding_autotest", failure occurs.
This patch creates a new bonding device for the closing testcase.

Fixes: 92073ef961ee ("bond: unit tests")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 app/test/test_link_bonding.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 4d54706c21d6..15fb0bc3e108 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -4182,7 +4182,16 @@ test_reconfigure_bonding_device(void)
 static int
 test_close_bonding_device(void)
 {
-	rte_eth_dev_close(test_params->bonding_port_id);
+	int16_t bonding_port_id;
+	char pmd_name[RTE_ETH_NAME_MAX_LEN];
+
+	snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN, "%s_%d",
+		 BONDING_DEV_NAME, ++bonding_id);
+	bonding_port_id = rte_eth_bond_create(pmd_name,
+				test_params->bonding_mode, rte_socket_id());
+	TEST_ASSERT(bonding_port_id >= 0,
+				"Failed to create bonding ethdev %s", pmd_name);
+	rte_eth_dev_close(bonding_port_id);
 	return 0;
 }
 
-- 
2.30.0

Re: [PATCH] app/test: fix retest link bonding fail

[Ferruh Yigit]

On 11/10/2023 9:53 AM, Jie Hai wrote:
> The testcase "test_close_bonding_device" closes the bonding
> port shared by several cases. After closed, the port is in
> RTE_ETH_DEV_UNUSED state, and could not be used by other cases
> anymore. If retest the "link_bonding_autotest", failure occurs.
> This patch creates a new bonding device for the closing testcase.
> 
> Fixes: 92073ef961ee ("bond: unit tests")
> Cc: stable@dpdk.org
> 

This must be from times 'rte_eth_dev_close()' not release all resources,
because there is already some code (in 'test_create_bonding_device()')
that covers re-running unit test.
So probably unit test was working when implemented but it is broken
either when 'rte_eth_dev_close()' updated or its implementation changed
in bonding PMD, can you please investigate it?


> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>  app/test/test_link_bonding.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
> index 4d54706c21d6..15fb0bc3e108 100644
> --- a/app/test/test_link_bonding.c
> +++ b/app/test/test_link_bonding.c
> @@ -4182,7 +4182,16 @@ test_reconfigure_bonding_device(void)
>  static int
>  test_close_bonding_device(void)
>  {
> -	rte_eth_dev_close(test_params->bonding_port_id);
> +	int16_t bonding_port_id;
> +	char pmd_name[RTE_ETH_NAME_MAX_LEN];
> +
> +	snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN, "%s_%d",
> +		 BONDING_DEV_NAME, ++bonding_id);
> +	bonding_port_id = rte_eth_bond_create(pmd_name,
> +				test_params->bonding_mode, rte_socket_id());
> +	TEST_ASSERT(bonding_port_id >= 0,
> +				"Failed to create bonding ethdev %s", pmd_name);
>

Why not use 'rte_eth_bond_free()', it is better than creating a
temporary bonding device just to close it?

Adding 'rte_eth_bond_free()' also requires updating
'test_create_bonding_device()' to allow creating device on each run.


And there are other bonding devices created during tests, they don't
break tests because they use 'bonding_port_id' global variable that is
increased in each run; should we close and remove them too here, to not
leak resources, what do you think?

> +	rte_eth_dev_close(bonding_port_id);
>

Previously 'rte_eth_dev_close()' return type was void, but it is not
anymore, and since test is about closing bonding device, should we test
return value of the API?