[PATCH] app/testpmd: fix indirect action list parameters parsing

[Etelson, Gregory]

Hello Ferruh,

>> About the fault root cause.
>> There were 2 uncoupled resources in that case: static token size and
>> variable size passed to parse_int().
>> parse_int() caller must provide a buffer large enough for token size.
>> Otherwise parse_int() will corrupt memory outside the input buffer.
>>
>
> As you said 'parse_int()' has two sizes, 'token->size' and 'size'
> function argument. Why function ignores 'size' argument and only uses
> 'token->size', I think this is a mistake.
>
>
> If 'parse_int()' doesn't use 'buf' and 'size' arguments at all, why it
> has them?
>
>

parse_int() receives target buffer size as an argument and also has access 
to original token size through the arg pointer.
In that configuration the token size has priority.

parse_int() must compare token and buffer sizes and return an error if 
target buffer size did not fit.

That will not break existing parser_int() functionality.
I'll send a fix in a new patch series.

>> In the generic solution parse_int() caller allocates target buffer using
>> existing knowledge about input token size.
>>
>> Testpmd add_port() imitates the ARGS_ENTRY() macro that extrapolates
>> target buffer size from RTE structure member.
>>
>> Current testpmd cannot use that approach directly because indirect
>> action references internal testpmd ID.
>>
>> Testpmd indirect ID has no defined type or token that leads to indirect
>> ID parser.
>>
>> As a solution, testpmd can provide centralized parser function for all
>> indirect IDs. The function will parse ID value and use the token as the
>> key to indirect database search:
>>
>
> Although it sounds reasonable to have indirect id parser, won't it have
> exact same problem?
>
> If token size if 64 bits as it is now, as far as I can see below code
> will have same stack corruption problem.
>

The function parses indirect ID. That ID is translated into the original 
object of any size according to token type. That means testpmd will have 
common pool of 32 bits IDs for all possible indirect translations.

>
> I think we should update parse_int function, to use either function
> parameters or context values, but changes has potential side effect and
> timing is not good for it, let's continue with your v3 for now.
>
>

[: Thumbs up :]

Regards,
Gregory