* [dpdk-dev] [PATCH] ethdev: fix flow API item/action name conversion
@ 2018-10-07 15:38 Mordechay Haimovsky
2018-10-07 16:22 ` [dpdk-dev] [PATCH v1] " Mordechay Haimovsky
0 siblings, 1 reply; 8+ messages in thread
From: Mordechay Haimovsky @ 2018-10-07 15:38 UTC (permalink / raw)
To: Adrien Mazarguil, Shahaf Shuler, orika; +Cc: dev, Mordechay Haimovsky
This patch fixes a typecast bug found in rte_flow_conv_name routine
used in rte_flow item/action name conversion.
Fixes: ae6b2cf49505 ("ethdev: add flow API item/action name conversion")
Signed-off-by: Moti Haimovsky <motih@mellanox.com>
---
lib/librte_ethdev/rte_flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 9c56a97..21a4286 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -767,7 +767,7 @@ enum rte_flow_conv_item_spec_type {
{ rte_flow_desc_action, RTE_DIM(rte_flow_desc_action), },
};
const struct desc_info *const info = &info_rep[!!is_action];
- unsigned int type = (uintptr_t)src;
+ unsigned int type = *(const unsigned int *)src;
if (type >= info->num)
return rte_flow_error_set
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-07 15:38 [dpdk-dev] [PATCH] ethdev: fix flow API item/action name conversion Mordechay Haimovsky
@ 2018-10-07 16:22 ` Mordechay Haimovsky
2018-10-07 16:31 ` Ori Kam
0 siblings, 1 reply; 8+ messages in thread
From: Mordechay Haimovsky @ 2018-10-07 16:22 UTC (permalink / raw)
To: Adrien Mazarguil, Shahaf Shuler, orika; +Cc: dev, Mordechay Haimovsky
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>
---
v1:
Fixed wrong hash number in "Fixes" message.
---
lib/librte_ethdev/rte_flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 9c56a97..21a4286 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -767,7 +767,7 @@ enum rte_flow_conv_item_spec_type {
{ rte_flow_desc_action, RTE_DIM(rte_flow_desc_action), },
};
const struct desc_info *const info = &info_rep[!!is_action];
- unsigned int type = (uintptr_t)src;
+ unsigned int type = *(const unsigned int *)src;
if (type >= info->num)
return rte_flow_error_set
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-07 16:22 ` [dpdk-dev] [PATCH v1] " Mordechay Haimovsky
@ 2018-10-07 16:31 ` Ori Kam
2018-10-09 13:21 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Ori Kam @ 2018-10-07 16:31 UTC (permalink / raw)
To: Mordechay Haimovsky, Adrien Mazarguil, Shahaf Shuler, orika
Cc: dev, Mordechay Haimovsky
> -----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>
> ---
> v1:
> Fixed wrong hash number in "Fixes" message.
> ---
> lib/librte_ethdev/rte_flow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 9c56a97..21a4286 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -767,7 +767,7 @@ enum rte_flow_conv_item_spec_type {
> { rte_flow_desc_action, RTE_DIM(rte_flow_desc_action), },
> };
> const struct desc_info *const info = &info_rep[!!is_action];
> - unsigned int type = (uintptr_t)src;
> + unsigned int type = *(const unsigned int *)src;
>
> if (type >= info->num)
> return rte_flow_error_set
> --
> 1.8.3.1
Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-07 16:31 ` Ori Kam
@ 2018-10-09 13:21 ` Ferruh Yigit
2018-10-09 13:38 ` Ferruh Yigit
2018-10-09 13:54 ` Adrien Mazarguil
0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-09 13:21 UTC (permalink / raw)
To: Ori Kam, Mordechay Haimovsky, Adrien Mazarguil, Shahaf Shuler, orika; +Cc: dev
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)
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-09 13:21 ` Ferruh Yigit
@ 2018-10-09 13:38 ` Ferruh Yigit
2018-10-09 13:54 ` Adrien Mazarguil
1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-09 13:38 UTC (permalink / raw)
To: Ori Kam, Mordechay Haimovsky, Adrien Mazarguil, Shahaf Shuler, orika; +Cc: dev
On 10/9/2018 2:21 PM, 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.
>
Squashed into relevant commit in next-net, thanks.
> (please confirm latest next-net head)
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-09 13:21 ` Ferruh Yigit
2018-10-09 13:38 ` Ferruh Yigit
@ 2018-10-09 13:54 ` Adrien Mazarguil
2018-10-11 10:14 ` Ferruh Yigit
1 sibling, 1 reply; 8+ messages in thread
From: Adrien Mazarguil @ 2018-10-09 13:54 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Ori Kam, Mordechay Haimovsky, Shahaf Shuler, orika, dev
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".
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-09 13:54 ` Adrien Mazarguil
@ 2018-10-11 10:14 ` Ferruh Yigit
2018-10-11 10:36 ` Mordechay Haimovsky
0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-11 10:14 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Ori Kam, Mordechay Haimovsky, Shahaf Shuler, orika, 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 <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.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] ethdev: fix flow API item/action name conversion
2018-10-11 10:14 ` Ferruh Yigit
@ 2018-10-11 10:36 ` Mordechay Haimovsky
0 siblings, 0 replies; 8+ messages in thread
From: Mordechay Haimovsky @ 2018-10-11 10:36 UTC (permalink / raw)
To: Ferruh Yigit, Adrien Mazarguil; +Cc: Ori Kam, Shahaf Shuler, orika, dev
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-11 10:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-07 15:38 [dpdk-dev] [PATCH] ethdev: fix flow API item/action name conversion Mordechay Haimovsky
2018-10-07 16:22 ` [dpdk-dev] [PATCH v1] " Mordechay Haimovsky
2018-10-07 16:31 ` Ori Kam
2018-10-09 13:21 ` Ferruh Yigit
2018-10-09 13:38 ` Ferruh Yigit
2018-10-09 13:54 ` Adrien Mazarguil
2018-10-11 10:14 ` Ferruh Yigit
2018-10-11 10:36 ` Mordechay Haimovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).