From: Bing Zhao <bingz@nvidia.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>,
"dmitry.kozliuk@gmail.com" <dmitry.kozliuk@gmail.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "aman.deep.singh@intel.com" <aman.deep.singh@intel.com>,
"yuying.zhang@intel.com" <yuying.zhang@intel.com>,
Matan Azrad <matan@nvidia.com>,
"stable@dpdk.org" <stable@dpdk.org>,
Dariusz Sosnowski <dsosnowski@nvidia.com>
Subject: RE: [PATCH] app/testpmd: fix releasing action handle flush memory
Date: Mon, 25 Mar 2024 09:03:36 +0000 [thread overview]
Message-ID: <DM4PR12MB51841F6CBE5F9BB3E31ACAD5D0362@DM4PR12MB5184.namprd12.prod.outlook.com> (raw)
In-Reply-To: <b9def392-50c9-48b4-b8aa-dace3646930c@amd.com>
Hi Ferruh,
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Tuesday, March 19, 2024 10:21 PM
> To: Bing Zhao <bingz@nvidia.com>; dmitry.kozliuk@gmail.com;
> dev@dpdk.org
> Cc: aman.deep.singh@intel.com; yuying.zhang@intel.com; Matan Azrad
> <matan@nvidia.com>; stable@dpdk.org; Dariusz Sosnowski
> <dsosnowski@nvidia.com>
> Subject: Re: [PATCH] app/testpmd: fix releasing action handle flush memory
>
> External email: Use caution opening links or attachments
>
>
> On 3/19/2024 9:32 AM, Bing Zhao wrote:
> > The memory of the indirect action handles should be freed after being
> > destroyed in the flush. The behavior needs to be consistent with the
> > single handle destroy.
> >
> > Or else, there will be some unexpected error when the action handle is
> > destroyed for the 2nd time, for example, the port needs to be closed
> > again.
> >
>
> Ports can be closed only once, so above reasoning is not valid, but I assume
> the purpose of this patch is to prevent memory leak, can you please clarify the
> problem and impact of the patch in commit log?
To close the port twice is something I am testing internally of "errno EBUSY", I didn't describe it clearly.
At the same time, YES, there is some memory leak should be fixed.
>
>
> Also what is "single handle destroy" mentioned above?
I mean the function call port_action_handle_destroy().
>
> The fixed commit is from a few release ago, so this is not something
> introduced in this release, do you think can this wait next release instead of
> merging for -rc4 which is more risky?
Yes, it is not something that blocking the release. The memory should be recycled by the system once the
application quits completely.
I will polish the commit message and send v2.
>
>
> > Fixes: f7352c176bbf ("app/testpmd: fix use of indirect action after
> > port close")
> > Cc: dmitry.kozliuk@gmail.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bing Zhao <bingz@nvidia.com>
> > Reviewed-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> > ---
> > app/test-pmd/config.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > ba1007ace6..f62ba90c87 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1918,8 +1918,7 @@ port_action_handle_flush(portid_t port_id)
> > /* Poisoning to make sure PMDs update it in case of error. */
> > memset(&error, 0x44, sizeof(error));
> > if (pia->handle != NULL) {
> > - ret = pia->type ==
> > - RTE_FLOW_ACTION_TYPE_INDIRECT_LIST ?
> > + ret = pia->type == RTE_FLOW_ACTION_TYPE_INDIRECT_LIST ?
> > rte_flow_action_list_handle_destroy
> > (port_id, pia->list_handle, &error) :
> > rte_flow_action_handle_destroy @@ -1929,11
> > +1928,9 @@ port_action_handle_flush(portid_t port_id)
> > pia->id);
> > ret = port_flow_complain(&error);
> > }
> > - tmp = &pia->next;
> > - } else {
> > - *tmp = pia->next;
> > - free(pia);
> > }
> > + *tmp = pia->next;
> > + free(pia);
> > }
> > return ret;
> > }
Thanks
BR. Bing
next prev parent reply other threads:[~2024-03-25 9:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-19 9:32 Bing Zhao
2024-03-19 14:20 ` Ferruh Yigit
2024-03-25 9:03 ` Bing Zhao [this message]
2024-03-25 10:58 ` [PATCH v2] " Bing Zhao
2024-04-18 21:41 ` Ferruh Yigit
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=DM4PR12MB51841F6CBE5F9BB3E31ACAD5D0362@DM4PR12MB5184.namprd12.prod.outlook.com \
--to=bingz@nvidia.com \
--cc=aman.deep.singh@intel.com \
--cc=dev@dpdk.org \
--cc=dmitry.kozliuk@gmail.com \
--cc=dsosnowski@nvidia.com \
--cc=ferruh.yigit@amd.com \
--cc=matan@nvidia.com \
--cc=stable@dpdk.org \
--cc=yuying.zhang@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).