app/test: fix retest link bonding fail
Checks
Commit Message
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(-)
Comments
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?
@@ -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;
}