Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion

[Mordechay Haimovsky <motih@mellanox.com>]

Hi,
 Thanks for reverting https://patches.dpdk.org/patch/46221/ , It was a bad fix in the wrong place. 

Moti H.

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, October 11, 2018 1:14 PM
> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Ori Kam <orika@mellanox.com>; Mordechay Haimovsky
> <motih@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>;
> orika@contextream.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name
> conversion
> 
> 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 <dev-bounces@dpdk.org> On Behalf Of Mordechay
> Haimovsky
> >>>> Sent: Sunday, October 7, 2018 7:22 PM
> >>>> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Shahaf Shuler
> >>>> <shahafs@mellanox.com>; orika@contextream.com
> >>>> Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>
> >>>> 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 <motih@mellanox.com>
> >> <...>
> >>> Acked-by: Ori Kam <orika@mellanox.com>
> >>
> >> 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.