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 <johnd...@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
