From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH] app/testpmd: fix flow list command
Date: Wed, 10 Oct 2018 10:37:55 +0200
Message-ID: <20181010083754.GL18937@6wind.com>
References: <20181009225124.25513-1-johndale@cisco.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>
To: John Daley <johndale@cisco.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com
 [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id 6D7531B3DA
 for <dev@dpdk.org>; Wed, 10 Oct 2018 10:38:13 +0200 (CEST)
Received: by mail-wr1-f67.google.com with SMTP id x12-v6so4681995wru.8
 for <dev@dpdk.org>; Wed, 10 Oct 2018 01:38:13 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20181009225124.25513-1-johndale@cisco.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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