> -----Original Message-----
> From: Danylo Vodopianov <dvo-...@napatech.com>
> Sent: Monday, November 18, 2024 12:26
> To: NBU-Contact-Thomas Monjalon (EXTERNAL) <tho...@monjalon.net>;
> aman.deep.si...@intel.com; yuying.zh...@intel.com; Ori Kam
> <or...@nvidia.com>; mko-...@napatech.com; c...@napatech.com; Dariusz
> Sosnowski <dsosnow...@nvidia.com>; sil-...@napatech.com
> Cc: Gregory Etelson <getel...@nvidia.com>; Alexander Kozyrev
> <akozy...@nvidia.com>; dev@dpdk.org; sta...@dpdk.org;
> ferruh.yi...@amd.com
> Subject: [PATCH v2 2/2] app/testpmd: fix aged flow destroy
>
> Avoid removal of additional flows after requested number of flows has been
> already removed.
>
> port_flow_destroy() function goes through all flows and compares given flow
> ‘id’
> with them. However in some cases it can advance pointer with “given ID” and
> thus remove additional flow.
Could you please remove the first two paragraphs in the commit message?
I don't think they are relevant.
>
> port_flow_destroy() function never assumed that rule array can be freed when
> it's executing, and port_flow_aged() just violated that assumption.
Also here, could you please describe what exactly is changed in port_flow_aged?
Current content explains the reason for the bug and the commit should describe
how it's fixed.
>
> Fixes: de956d5ecf08 ("app/testpmd: support age shared action context")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Danylo Vodopianov <dvo-...@napatech.com>
> ---
> app/test-pmd/config.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> c831166431..04de2fe59d 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -4160,8 +4160,9 @@ port_flow_aged(portid_t port_id, uint8_t destroy)
> }
> type = (enum age_action_context_type *)contexts[idx];
> switch (*type) {
> - case ACTION_AGE_CONTEXT_TYPE_FLOW:
> + case ACTION_AGE_CONTEXT_TYPE_FLOW: {
> ctx.pf = container_of(type, struct port_flow,
> age_type);
> + uint64_t flow_id = ctx.pf->id;
Could you please refactor these lines as follows?
```
case ACTION_AGE_CONTEXT_TYPE_FLOW: {
uint64_t flow_id;
ctx.pf = container_of(type, struct port_flow, age_type);
flow_id = ctx.pf->id;
```
This is to make the style aligned - variables declared at the beginning of the
block.
> printf("%-20s\t%" PRIu64 "\t%" PRIu32 "\t%" PRIu32
>
> "\t%c%c%c\t\n",
> "Flow",
> @@ -4172,9 +4173,10 @@ port_flow_aged(portid_t port_id, uint8_t destroy)
> ctx.pf->rule.attr->egress ? 'e' : '-',
> ctx.pf->rule.attr->transfer ? 't' : '-');
> if (destroy && !port_flow_destroy(port_id, 1,
> - &ctx.pf->id, false))
> + &flow_id,
> + false))
> total++;
> break;
> + }
> case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
> ctx.pia = container_of(type,
> struct port_indirect_action,
> age_type);
> --
> 2.43.5
