Re: [PATCH] app/testpmd: fix flow list command

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

Hi John

On Tue, Oct 09, 2018 at 03:51:24PM -0700, John Daley wrote:
> This patch fixes the 'flow list <port id>' command which caused a
> segfault when passing the action or item 'type' field instead
> of the action or item struct pointer in the call to rte_flow_conv.
> 
> Fixes: 7d94dcedf7ce ("app/testpmd: rely on flow API conversion function")
> 
> Signed-off-by: John Daley <johndale@cisco.com>

That bug was introduced by a broken fix, it wasn't present in the original
patch, please see yesterday's discussion [1].

RTE_FLOW_CONV_OP_(ITEM|ACTION)_NAME[_PTR] operations are documented as using
an integer type (enum rte_flow_item_type) cast as (void *) for src because
they convert item/action *types* to corresponding strings, i.e. no need to
allocate temporary items/actions just to retrieve their names. I thought it
would be more versatile and efficient that way.

[1] "ethdev: fix flow API item/action name conversion"
    https://mails.dpdk.org/archives/dev/2018-October/115054.html

> ---
>  app/test-pmd/config.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 86c205806..2ce40f3e1 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1354,7 +1354,7 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
>  		while (item->type != RTE_FLOW_ITEM_TYPE_END) {
>  			if (rte_flow_conv(RTE_FLOW_CONV_OP_ITEM_NAME_PTR,
>  					  &name, sizeof(name),
> -					  (void *)(uintptr_t)item->type,
> +					  (void *)(uintptr_t)item,

Also, while it does work because type is the first field, it should have
read "&item->type" for correctness. Anyway this patch shouldn't be needed
assuming the broken fix is reverted.

>  					  NULL) <= 0)
>  				name = "[UNKNOWN]";
>  			if (item->type != RTE_FLOW_ITEM_TYPE_VOID)
> @@ -1365,7 +1365,7 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
>  		while (action->type != RTE_FLOW_ACTION_TYPE_END) {
>  			if (rte_flow_conv(RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
>  					  &name, sizeof(name),
> -					  (void *)(uintptr_t)action->type,
> +					  (void *)(uintptr_t)action,

Ditto.

>  					  NULL) <= 0)
>  				name = "[UNKNOWN]";
>  			if (action->type != RTE_FLOW_ACTION_TYPE_VOID)
> -- 
> 2.16.2
> 

Thanks.

-- 
Adrien Mazarguil
6WIND