On 11/9/2023 6:22 PM, Etelson, Gregory wrote:
> Hello Ferruh,
>
>>> Indirect actions list arguments parser was configured to place target
>>> number into 64bit value, while the code provided 32bits memory.
>>>
>>
>> Hi Gregory,
>>
>> Can you please give more details why 'id' needs to be 64 bits, with
>> callstack or usecase etc?
>> And please describe what is the observed problem with current code?
>>
>
> In rte_flow.h, struct rte_flow_action_indirect_list::handle is a pointer.
>
> Testpmd ACTION_INDIRECT_LIST_HANDLE and ACTION_INDIRECT_LIST_CONF tokens
> define arguments size as uintptr_t.
>
> On 64 bits system, defining the id variable as 32 bits value
> corrupted parse_indlst_id2ptr stack.
>
I can't see how stack corruption can happen, can you please provide call
stack and flow command?
> I'll change the id definition to uintptr_t to match token in v2.
>
> Regards,
> Gregory
>
>>
>> Inside 'parse_indlst_id2ptr()',
>> 'parse_int()' can work or 32bits and 64bits variables, so that one is OK.
>> But both 'port_action_handle_get_by_id()' &
>> 'indirect_action_list_conf_get()' gets 'id' as parameter and they get
>> 32bits argument, when 'id' is 64bit won't it will be cast to 32bits and
>> loose data, should those functions needs to be updated as well.
>>
Can you please reply to above question, about changing 'id' type impact
to other functions using it?
>>
>>
>>> The patch updated variable size for translation results.
>>>
>>> Fixes: 72a3dec7126f ("ethdev: add indirect flow list action")
>>> Signed-off-by: Gregory Etelson <getel...@nvidia.com>
>>> ---
>>> app/test-pmd/cmdline_flow.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 0d521159e9..cf1ca33208 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -11331,7 +11331,7 @@ parse_indlst_id2ptr(struct context *ctx,
>>> const struct token *token,
>>> struct rte_flow_action *action = ctx->object;
>>> struct rte_flow_action_indirect_list *action_conf;
>>> const struct indlst_conf *indlst_conf;
>>> - uint32_t id;
>>> + uint64_t id;
>>> int ret;
>>>
>>> if (!action)
>>> @@ -11350,7 +11350,8 @@ parse_indlst_id2ptr(struct context *ctx,
>>> const struct token *token,
>>> action_conf->handle = (typeof(action_conf->handle))
>>> port_action_handle_get_by_id(ctx->port,
>>> id);
>>> if (!action_conf->handle) {
>>> - printf("no indirect list handle for id %u\n", id);
>>> + printf("no indirect list handle for id
>>> %"PRIu64"\n",
>>> + id);
>>> return -1;
>>> }
>>> break;
>>
>>
