From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Wed, 10 Oct 2018 10:38:13 +0200 (CEST) Received: by mail-wr1-f67.google.com with SMTP id x12-v6so4681995wru.8 for ; Wed, 10 Oct 2018 01:38:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=bB0WJYGE6GfcbTJM1IceGli7dFBDz1xBmepqzzw2KN0=; b=gV+mZ82bvVC59ksYlw7ISwajalCgH0r1Ap1K1Iy86SWoHOWxnKZCndgooWv64neyZu G11TBpnKIMN1peOwBTzTRN9eGhvKcsdRpPNzRxeDhfI8v+p9COHfEexgLofHJdssmIHf XhnDBt9sQ0V3mCIrAL7VnAJS9c4kwvM+tczdLMBWrs+QQmLjPlnkUPv9BTdNzFI1lwHr Yy+sV1wcENju8F4/Zj6TjqPuenddPEWvYCUdLhAGzT4HLNlKeSdVg4aNbLtrcTOsLY0x Y4bRb+08sUHc2tHz+P/0PXhZladVvFZL5Srxx0ISAWOtdyQRnMDTqd49bdmSZwbFDiXR WHgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=bB0WJYGE6GfcbTJM1IceGli7dFBDz1xBmepqzzw2KN0=; b=igUYzvWxEb2QYI/xznN7WBu0GQ8n9mLYR2J2P5WzVYCzs4gckB48iMxnKG3mN7iWru XiymxvYl+wiOJkVbUfZvtLzoIF3y+CF9zT0+iljLjssocJeTdeiA91tRW8iT1GH6GhU2 oj35yyjRwIuNjOIyLb+kl6tt/NfqugkZZ/PP/LZDYcki0ZLJMQtuAkZHHmeIzyFlgMeP pHFvf19+Wim6TSt2IDK20xWMc5YC+aCnHbc7CP+NuctrSfcPGgaSgkKkbN13x2wyU4Yc vocuywxbMExvAo2K6sVMIQkjnG/WrUeWz2AmXV4s5DJhcEsLAdVkr8h4zqhFxbYRy3Al Ejqw== X-Gm-Message-State: ABuFfog3LBeClNUiES2TpamH+DzKMB8PWrrJW6mmUuhBZShi1pmoSa01 Yt4RzaqRPMHGdNAdDEb3omhNNQ== X-Google-Smtp-Source: ACcGV62FXSyEkJ/HOR5Caktihp3t8nayRCkzpro1jcMGiNP1hCTSDVclDG98RlP5g9vBnwDjk87bLA== X-Received: by 2002:adf:db8e:: with SMTP id u14-v6mr23408442wri.217.1539160693151; Wed, 10 Oct 2018 01:38:13 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id z16-v6sm13920031wmc.0.2018.10.10.01.38.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Oct 2018 01:38:12 -0700 (PDT) Date: Wed, 10 Oct 2018 10:37:55 +0200 From: Adrien Mazarguil To: John Daley Cc: dev@dpdk.org, Ferruh Yigit Message-ID: <20181010083754.GL18937@6wind.com> References: <20181009225124.25513-1-johndale@cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181009225124.25513-1-johndale@cisco.com> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix flow list command X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Oct 2018 08:38:13 -0000 Hi John On Tue, Oct 09, 2018 at 03:51:24PM -0700, John Daley wrote: > This patch fixes the 'flow list ' 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 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