From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Jie Hai <haijie1@huawei.com>,
dev@dpdk.org, Chas Williams <chas3@att.com>,
"Min Hu (Connor)" <humin29@huawei.com>,
Pablo de Lara <pablo.de.lara.guarch@intel.com>,
Declan Doherty <declan.doherty@intel.com>
Cc: lihuisong@huawei.com, fengchengwen@huawei.com, liudongdong3@huawei.com
Subject: Re: [PATCH] app/test: fix retest link bonding fail
Date: Sat, 11 Nov 2023 02:02:34 +0000 [thread overview]
Message-ID: <a2dae837-a435-4805-af45-477989af7467@amd.com> (raw)
In-Reply-To: <20231110095302.2469090-1-haijie1@huawei.com>
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?
prev parent reply other threads:[~2023-11-11 2:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-10 9:53 Jie Hai
2023-11-11 2:02 ` Ferruh Yigit [this message]
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=a2dae837-a435-4805-af45-477989af7467@amd.com \
--to=ferruh.yigit@amd.com \
--cc=chas3@att.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=haijie1@huawei.com \
--cc=humin29@huawei.com \
--cc=lihuisong@huawei.com \
--cc=liudongdong3@huawei.com \
--cc=pablo.de.lara.guarch@intel.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).