* [PATCH] app/testpmd: fix quit testpmd with vfs and pf
@ 2022-03-22 8:05 Ke Zhang
0 siblings, 0 replies; 5+ messages in thread
From: Ke Zhang @ 2022-03-22 8:05 UTC (permalink / raw)
To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang
When testpmd startups with pf and vfs,this error occurs when quitting,
results in pf is released before vfs ,so the vf would access an
freed heap memory.
The solution is two steps:
1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
2. free the port in reverse order.
Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f7e18aee25..2299aef3dd 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
void
pmd_test_exit(void)
{
+ unsigned int ethports[RTE_MAX_ETHPORTS];
+ unsigned int count = 0;
portid_t pt_id;
unsigned int i;
+ int index;
int ret;
if (test_done == 0)
@@ -3396,15 +3399,30 @@ pmd_test_exit(void)
#endif
if (ports != NULL) {
no_link_check = 1;
+ i = 0;
+
+ /* Fetch the valid port id from port list*/
RTE_ETH_FOREACH_DEV(pt_id) {
- printf("\nStopping port %d...\n", pt_id);
+ ethports[i] = pt_id;
+ i++;
+ }
+
+ count = i;
+ /*
+ * Free the port from Reverse order, as general,
+ * PF port < VF port， VF should be free before PF
+ * be free.
+ */
+ for (index = count - 1 ; index >= 0 ; index--) {
+ printf("\nStopping port %d...\n", ethports[index]);
fflush(stdout);
- stop_port(pt_id);
+ stop_port(ethports[index]);
}
- RTE_ETH_FOREACH_DEV(pt_id) {
- printf("\nShutting down port %d...\n", pt_id);
+
+ for (index = count - 1 ; index >= 0 ; index--) {
+ printf("\nShutting down port %d...\n", ethports[index]);
fflush(stdout);
- close_port(pt_id);
+ close_port(ethports[index]);
}
}
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] app/testpmd: fix quit testpmd with vfs and pf
@ 2022-03-22 7:18 Ke Zhang
2022-03-28 10:06 ` Singh, Aman Deep
0 siblings, 1 reply; 5+ messages in thread
From: Ke Zhang @ 2022-03-22 7:18 UTC (permalink / raw)
To: xiaoyun.li, aman.deep.singh, yuying.zhang, dev; +Cc: Ke Zhang
When testpmd startups with pf and vfs,this error occurs when quitting,
results in pf is released before vfs ,so the vf would access an
freed heap memory.
The solution is two steps:
1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
2. free the port in reverse order.
Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f7e18aee25..ca6c77b14b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
void
pmd_test_exit(void)
{
+ unsigned int ports[RTE_MAX_ETHPORTS];
+ unsigned int count = 0;
portid_t pt_id;
unsigned int i;
+ int index;
int ret;
if (test_done == 0)
@@ -3396,15 +3399,30 @@ pmd_test_exit(void)
#endif
if (ports != NULL) {
no_link_check = 1;
+ i = 0;
+
+ /* Fetch the valid port id from port list*/
RTE_ETH_FOREACH_DEV(pt_id) {
- printf("\nStopping port %d...\n", pt_id);
+ ports[i] = pt_id;
+ i++;
+ }
+
+ count = i;
+ /*
+ * Free the port from Reverse order, as general,
+ * PF port < VF port， VF should be free before PF
+ * be free.
+ */
+ for (index = count - 1 ; index >= 0 ; index--) {
+ printf("\nStopping port %d...\n", ports[index]);
fflush(stdout);
- stop_port(pt_id);
+ stop_port(ports[index]);
}
- RTE_ETH_FOREACH_DEV(pt_id) {
- printf("\nShutting down port %d...\n", pt_id);
+
+ for (index = count - 1 ; index >= 0 ; index--) {
+ printf("\nShutting down port %d...\n", ports[index]);
fflush(stdout);
- close_port(pt_id);
+ close_port(ports[index]);
}
}
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] app/testpmd: fix quit testpmd with vfs and pf
2022-03-22 7:18 Ke Zhang
@ 2022-03-28 10:06 ` Singh, Aman Deep
2022-03-31 8:28 ` Singh, Aman Deep
2022-06-17 18:53 ` Ferruh Yigit
0 siblings, 2 replies; 5+ messages in thread
From: Singh, Aman Deep @ 2022-03-28 10:06 UTC (permalink / raw)
To: Ke Zhang, xiaoyun.li, yuying.zhang, dev
On 3/22/2022 12:48 PM, Ke Zhang wrote:
> When testpmd startups with pf and vfs,this error occurs when quitting,
> results in pf is released before vfs ,so the vf would access an
> freed heap memory.
>
> The solution is two steps:
> 1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
> 2. free the port in reverse order.
>
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
This fix looks similar to series: 21823. Please check Ferruh's comments
on it.
There is a scenario, where this fix may not work.
"if there is an earlier port id which is released, after that new
vf is created taking that port id. In such case port id of vf < pf port id"
This patch covers most of the cases and that way its an improvement.
So I am ok with it.
Acked-by: Aman Singh <aman.deep.singh@intel.com>
> app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index f7e18aee25..ca6c77b14b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
> void
> pmd_test_exit(void)
> {
> + unsigned int ports[RTE_MAX_ETHPORTS];
> + unsigned int count = 0;
> portid_t pt_id;
> unsigned int i;
> + int index;
> int ret;
>
> if (test_done == 0)
> @@ -3396,15 +3399,30 @@ pmd_test_exit(void)
> #endif
> if (ports != NULL) {
> no_link_check = 1;
> + i = 0;
> +
> + /* Fetch the valid port id from port list*/
> RTE_ETH_FOREACH_DEV(pt_id) {
> - printf("\nStopping port %d...\n", pt_id);
> + ports[i] = pt_id;
> + i++;
> + }
> +
> + count = i;
> + /*
> + * Free the port from Reverse order, as general,
> + * PF port < VF port， VF should be free before PF
> + * be free.
> + */
> + for (index = count - 1 ; index >= 0 ; index--) {
> + printf("\nStopping port %d...\n", ports[index]);
> fflush(stdout);
> - stop_port(pt_id);
> + stop_port(ports[index]);
> }
> - RTE_ETH_FOREACH_DEV(pt_id) {
> - printf("\nShutting down port %d...\n", pt_id);
> +
> + for (index = count - 1 ; index >= 0 ; index--) {
> + printf("\nShutting down port %d...\n", ports[index]);
> fflush(stdout);
> - close_port(pt_id);
> + close_port(ports[index]);
> }
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] app/testpmd: fix quit testpmd with vfs and pf
2022-03-28 10:06 ` Singh, Aman Deep
@ 2022-03-31 8:28 ` Singh, Aman Deep
2022-06-17 18:53 ` Ferruh Yigit
1 sibling, 0 replies; 5+ messages in thread
From: Singh, Aman Deep @ 2022-03-31 8:28 UTC (permalink / raw)
To: dev
On 3/28/2022 3:36 PM, Singh, Aman Deep wrote:
>
>
> On 3/22/2022 12:48 PM, Ke Zhang wrote:
>> When testpmd startups with pf and vfs,this error occurs when quitting,
>> results in pf is released before vfs ,so the vf would access an
>> freed heap memory.
>>
>> The solution is two steps:
>> 1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
>> 2. free the port in reverse order.
>>
>> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
>> ---
> This fix looks similar to series: 21823. Please check Ferruh's
> comments on it.
>
> There is a scenario, where this fix may not work.
> "if there is an earlier port id which is released, after that new
> vf is created taking that port id. In such case port id of vf < pf
> port id"
>
> This patch covers most of the cases and that way its an improvement.
> So I am ok with it.
>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>> app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index f7e18aee25..ca6c77b14b 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
>> void
>> pmd_test_exit(void)
>> {
>> + unsigned int ports[RTE_MAX_ETHPORTS];
>> + unsigned int count = 0;
>> portid_t pt_id;
>> unsigned int i;
>> + int index;
>> int ret;
>> if (test_done == 0)
>> @@ -3396,15 +3399,30 @@ pmd_test_exit(void)
>> #endif
>> if (ports != NULL) {
>> no_link_check = 1;
>> + i = 0;
>> +
>> + /* Fetch the valid port id from port list*/
>> RTE_ETH_FOREACH_DEV(pt_id) {
>> - printf("\nStopping port %d...\n", pt_id);
>> + ports[i] = pt_id;
>> + i++;
>> + }
>> +
>> + count = i;
>> + /*
>> + * Free the port from Reverse order, as general,
>> + * PF port < VF port， VF should be free before PF
>> + * be free.
>> + */
>> + for (index = count - 1 ; index >= 0 ; index--) {
>> + printf("\nStopping port %d...\n", ports[index]);
>> fflush(stdout);
>> - stop_port(pt_id);
>> + stop_port(ports[index]);
>> }
>> - RTE_ETH_FOREACH_DEV(pt_id) {
>> - printf("\nShutting down port %d...\n", pt_id);
>> +
>> + for (index = count - 1 ; index >= 0 ; index--) {
>> + printf("\nShutting down port %d...\n", ports[index]);
>> fflush(stdout);
>> - close_port(pt_id);
>> + close_port(ports[index]);
>> }
>> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] app/testpmd: fix quit testpmd with vfs and pf
2022-03-28 10:06 ` Singh, Aman Deep
2022-03-31 8:28 ` Singh, Aman Deep
@ 2022-06-17 18:53 ` Ferruh Yigit
1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2022-06-17 18:53 UTC (permalink / raw)
To: Singh, Aman Deep, Ke Zhang, xiaoyun.li, yuying.zhang, dev
On 3/28/2022 11:06 AM, Singh, Aman Deep wrote:
>
>
> On 3/22/2022 12:48 PM, Ke Zhang wrote:
>> When testpmd startups with pf and vfs,this error occurs when quitting,
>> results in pf is released before vfs ,so the vf would access an
>> freed heap memory.
>>
>> The solution is two steps:
>> 1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
>> 2. free the port in reverse order.
>>
>> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
>> ---
> This fix looks similar to series: 21823. Please check Ferruh's comments
> on it.
>
> There is a scenario, where this fix may not work.
> "if there is an earlier port id which is released, after that new
> vf is created taking that port id. In such case port id of vf < pf port id"
>
> This patch covers most of the cases and that way its an improvement.
> So I am ok with it.
>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
Agree that patch covers most of the cases with simple/small change, so
OK to continue with it.
@Ke, can you please check minor comments below.
>> app/test-pmd/testpmd.c | 28 +++++++++++++++++++++++-----
>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index f7e18aee25..ca6c77b14b 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -3378,8 +3378,11 @@ detach_devargs(char *identifier)
>> void
>> pmd_test_exit(void)
>> {
>> + unsigned int ports[RTE_MAX_ETHPORTS];
testpmd holds port IDs as 'portid_t', so can you please change type form
'int' to 'portid_t'?
>> + unsigned int count = 0;
For 'index' & 'count', no reason to make one 'unsigned int' and other
'int', better to keep type same for both, and both can be 'int'.
>> portid_t pt_id;
>> unsigned int i;
>> + int index;
These new variables only used in one if block, can you please move
declaring them to that scope, to the beginning of that block.
>> int ret;
>> if (test_done == 0)
>> @@ -3396,15 +3399,30 @@ pmd_test_exit(void)
>> #endif
>> if (ports != NULL) {
>> no_link_check = 1;
>> + i = 0;
>> +
>> + /* Fetch the valid port id from port list*/
>> RTE_ETH_FOREACH_DEV(pt_id) {
>> - printf("\nStopping port %d...\n", pt_id);
>> + ports[i] = pt_id;
>> + i++;
Can use 'count' directly, instead of 'i'.
>> + }
>> +
>> + count = i;
>> + /*
>> + * Free the port from Reverse order, as general,
>> + * PF port < VF port， VF should be free before PF
>> + * be free.
>> + */
>> + for (index = count - 1 ; index >= 0 ; index--) {
>> + printf("\nStopping port %d...\n", ports[index]);
>> fflush(stdout);
>> - stop_port(pt_id);
>> + stop_port(ports[index]);
>> }
>> - RTE_ETH_FOREACH_DEV(pt_id) {
>> - printf("\nShutting down port %d...\n", pt_id);
>> +
>> + for (index = count - 1 ; index >= 0 ; index--) {
>> + printf("\nShutting down port %d...\n", ports[index]);
>> fflush(stdout);
>> - close_port(pt_id);
>> + close_port(ports[index]);
>> }
>> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-17 18:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 8:05 [PATCH] app/testpmd: fix quit testpmd with vfs and pf Ke Zhang
-- strict thread matches above, loose matches on Subject: below --
2022-03-22 7:18 Ke Zhang
2022-03-28 10:06 ` Singh, Aman Deep
2022-03-31 8:28 ` Singh, Aman Deep
2022-06-17 18:53 ` Ferruh Yigit
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).