From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v1] ethdev: fix flow API item/action name conversion Date: Thu, 11 Oct 2018 11:14:26 +0100 Message-ID: <7e384a57-2f80-1932-03c6-5b178955a35e@intel.com> References: <1538926667-23009-1-git-send-email-motih@mellanox.com> <1538929311-31815-1-git-send-email-motih@mellanox.com> <1b66f1f8-8eda-19ce-b7ca-04cc3463f3ab@intel.com> <20181009135406.GJ18937@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Ori Kam , Mordechay Haimovsky , Shahaf Shuler , "orika@contextream.com" , "dev@dpdk.org" To: Adrien Mazarguil Return-path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 97D041B2AF for ; Thu, 11 Oct 2018 12:14:35 +0200 (CEST) In-Reply-To: <20181009135406.GJ18937@6wind.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/9/2018 2:54 PM, Adrien Mazarguil wrote: > Hi, > > Jumping in although I cannot spend much time on rte_flow at the moment, > please see below. > > On Tue, Oct 09, 2018 at 02:21:23PM +0100, Ferruh Yigit wrote: >> On 10/7/2018 5:31 PM, Ori Kam wrote: >>> >>> >>>> -----Original Message----- >>>> From: dev On Behalf Of Mordechay Haimovsky >>>> Sent: Sunday, October 7, 2018 7:22 PM >>>> To: Adrien Mazarguil ; Shahaf Shuler >>>> ; orika@contextream.com >>>> Cc: dev@dpdk.org; Mordechay Haimovsky >>>> Subject: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name >>>> conversion >>>> >>>> This patch fixes a typecast bug found in rte_flow_conv_name routine >>>> used in rte_flow item/action name conversion. >>>> >>>> Fixes: 0c2640cbfa7a ("ethdev: add flow API item/action name conversion") >>>> >>>> Signed-off-by: Moti Haimovsky >> <...> >>> Acked-by: Ori Kam >> >> Series applied to dpdk-next-net/master, thanks. >> >> (please confirm latest next-net head) > > Please revert, it breaks something that didn't need to be fixed. I don't > think this patch was validated properly. > > As documented in RTE_FLOW_CONV_OP_ITEM_NAME, RTE_FLOW_CONV_OP_ACTION_NAME, > RTE_FLOW_CONV_OP_ITEM_NAME_PTR and RTE_FLOW_CONV_OP_ACTION_NAME_PTR: > > @p src type: > @code (const void *)enum rte_flow_item_type @endcode > > With the following reminder in rte_flow_conv_name()'s Doxygen documentation: > > @param[in] src > Depending on @p is_action, source pattern item or action type cast as a > pointer. > > Hence the original conversion results in the expected behavior while this > one is almost guaranteed to trigger a segfault: > > - unsigned int type = (uintptr_t)src; > + unsigned int type = *(const unsigned int *)src; > > This can be validated with testpmd. See what happens with "flow list". Thanks Adrien, patch has been reverted on next-net.