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

Ferruh Yigit ferruh.yigit at amd.com
Sat Nov 11 03:02:34 CET 2023


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 at 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 at 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?



More information about the dev mailing list