* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
@ 2019-07-02 9:51 Ori Kam
0 siblings, 0 replies; 6+ messages in thread
From: Ori Kam @ 2019-07-02 9:51 UTC (permalink / raw)
To: Jack Min, Adrien Mazarguil, John McNamara, Marko Kovacevic,
Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
Cc: dev
> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Tuesday, July 2, 2019 12:46 PM
> To: Ori Kam <orika@mellanox.com>; Adrien Mazarguil
> <adrien.mazarguil@6wind.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>
> Cc: dev@dpdk.org
> Subject: [Suspected-Phishing][PATCH v4 1/4] ethdev: add GRE key field to flow
> API
>
> Add new rte_flow_item_gre_key in order to match the optional key field.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori Kam.
> doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
> lib/librte_ethdev/rte_flow.c | 1 +
> lib/librte_ethdev/rte_flow.h | 7 +++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index a34d012e55..f4b7baa3c3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -980,6 +980,14 @@ Matches a GRE header.
> - ``protocol``: protocol type.
> - Default ``mask`` matches protocol only.
>
> +Item: ``GRE_KEY``
> +^^^^^^^^^^^^^^^^^
> +
> +Matches a GRE key field.
> +This should be preceded by item ``GRE``
> +
> +- Value to be matched is a big-endian 32 bit integer
> +
> Item: ``FUZZY``
> ^^^^^^^^^^^^^^^
>
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 3277be1edb..f3e56d0bbe 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -55,6 +55,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
> MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
> MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
> MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
> + MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
> MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
> MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
> MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index f3a8fb103f..5d3702a44c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -289,6 +289,13 @@ enum rte_flow_item_type {
> */
> RTE_FLOW_ITEM_TYPE_GRE,
>
> + /**
> + * Matches a GRE optional key field.
> + *
> + * The value should a big-endian 32bit integer.
> + */
> + RTE_FLOW_ITEM_TYPE_GRE_KEY,
> +
> /**
> * [META]
> *
> --
> 2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
2019-07-03 15:25 ` Adrien Mazarguil
@ 2019-07-04 2:43 ` Jack Min
0 siblings, 0 replies; 6+ messages in thread
From: Jack Min @ 2019-07-04 2:43 UTC (permalink / raw)
To: Adrien Mazarguil
Cc: Ori Kam, John McNamara, Marko Kovacevic, Thomas Monjalon,
Ferruh Yigit, Andrew Rybchenko, dev
On Wed, 19-07-03, 17:25, Adrien Mazarguil wrote:
> On Tue, Jul 02, 2019 at 05:45:52PM +0800, Xiaoyu Min wrote:
> > Add new rte_flow_item_gre_key in order to match the optional key field.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
>
> OK with adding this feature, however I still have a bunch of comments below.
>
> > ---
> > doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
> > lib/librte_ethdev/rte_flow.c | 1 +
> > lib/librte_ethdev/rte_flow.h | 7 +++++++
> > 3 files changed, 16 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > index a34d012e55..f4b7baa3c3 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -980,6 +980,14 @@ Matches a GRE header.
> > - ``protocol``: protocol type.
> > - Default ``mask`` matches protocol only.
> >
> > +Item: ``GRE_KEY``
> > +^^^^^^^^^^^^^^^^^
> > +
> > +Matches a GRE key field.
> > +This should be preceded by item ``GRE``
>
> Nit: missing ending "."
>
Ok, I'll add it.
> > +
> > +- Value to be matched is a big-endian 32 bit integer
> > +
> > Item: ``FUZZY``
> > ^^^^^^^^^^^^^^^
> >
> > diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> > index 3277be1edb..f3e56d0bbe 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -55,6 +55,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
> > MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
> > MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
> > MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
> > + MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
>
> Hmm? Adding a new item in the middle?
>
I'll add it at the end.
> > MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
> > MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
> > MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index f3a8fb103f..5d3702a44c 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -289,6 +289,13 @@ enum rte_flow_item_type {
> > */
> > RTE_FLOW_ITEM_TYPE_GRE,
> >
> > + /**
> > + * Matches a GRE optional key field.
> > + *
> > + * The value should a big-endian 32bit integer.
> > + */
> > + RTE_FLOW_ITEM_TYPE_GRE_KEY,
> > +
>
> Same comment. While I understand the intent to group GRE and GRE_KEY, doing
> so causes ABI breakage by shifting the value of all subsequent pattern
> items (see IPV6 and IPV6_EXT for instance).
>
Oh, I was't aware of this. Thank you for explaination.
> We could later decide to sort them while knowingly breaking ABI on purpose,
> however right now there's no choice but adding new pattern items and actions
> at the end of their respective enums, please do that.
>
Yes, I'll do this.
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
2019-07-03 14:06 ` Thomas Monjalon
@ 2019-07-04 2:18 ` Jack Min
0 siblings, 0 replies; 6+ messages in thread
From: Jack Min @ 2019-07-04 2:18 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Adrien Mazarguil, dev, Ori Kam, John McNamara, Marko Kovacevic,
Ferruh Yigit, Andrew Rybchenko
On Wed, 19-07-03, 16:06, Thomas Monjalon wrote:
> 02/07/2019 11:45, Xiaoyu Min:
> > + /**
> > + * Matches a GRE optional key field.
> > + *
> > + * The value should a big-endian 32bit integer.
> > + */
> > + RTE_FLOW_ITEM_TYPE_GRE_KEY,
>
> We probably want to use the same format as in Dekel's patch
> for doxygen documentation of the action value:
>
OK, I'll update it accordingly.
Thanks ~
> /**
> * Increase sequence number in the outermost TCP header.
> *
> * Action configuration specifies the value to increase
> * TCP sequence number as a big-endian 32 bit integer.
> *
> * @p conf type:
> * @code rte_be32_t * @endcode
> *
> * Using this action on non-matching traffic will result in
> * undefined behavior.
> */
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-03 14:06 ` Thomas Monjalon
@ 2019-07-03 15:25 ` Adrien Mazarguil
2019-07-04 2:43 ` Jack Min
1 sibling, 1 reply; 6+ messages in thread
From: Adrien Mazarguil @ 2019-07-03 15:25 UTC (permalink / raw)
To: Xiaoyu Min
Cc: orika, John McNamara, Marko Kovacevic, Thomas Monjalon,
Ferruh Yigit, Andrew Rybchenko, dev
On Tue, Jul 02, 2019 at 05:45:52PM +0800, Xiaoyu Min wrote:
> Add new rte_flow_item_gre_key in order to match the optional key field.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
OK with adding this feature, however I still have a bunch of comments below.
> ---
> doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
> lib/librte_ethdev/rte_flow.c | 1 +
> lib/librte_ethdev/rte_flow.h | 7 +++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index a34d012e55..f4b7baa3c3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -980,6 +980,14 @@ Matches a GRE header.
> - ``protocol``: protocol type.
> - Default ``mask`` matches protocol only.
>
> +Item: ``GRE_KEY``
> +^^^^^^^^^^^^^^^^^
> +
> +Matches a GRE key field.
> +This should be preceded by item ``GRE``
Nit: missing ending "."
> +
> +- Value to be matched is a big-endian 32 bit integer
> +
> Item: ``FUZZY``
> ^^^^^^^^^^^^^^^
>
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index 3277be1edb..f3e56d0bbe 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -55,6 +55,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
> MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
> MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
> MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
> + MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
Hmm? Adding a new item in the middle?
> MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
> MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
> MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index f3a8fb103f..5d3702a44c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -289,6 +289,13 @@ enum rte_flow_item_type {
> */
> RTE_FLOW_ITEM_TYPE_GRE,
>
> + /**
> + * Matches a GRE optional key field.
> + *
> + * The value should a big-endian 32bit integer.
> + */
> + RTE_FLOW_ITEM_TYPE_GRE_KEY,
> +
Same comment. While I understand the intent to group GRE and GRE_KEY, doing
so causes ABI breakage by shifting the value of all subsequent pattern
items (see IPV6 and IPV6_EXT for instance).
We could later decide to sort them while knowingly breaking ABI on purpose,
however right now there's no choice but adding new pattern items and actions
at the end of their respective enums, please do that.
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
@ 2019-07-03 14:06 ` Thomas Monjalon
2019-07-04 2:18 ` Jack Min
2019-07-03 15:25 ` Adrien Mazarguil
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2019-07-03 14:06 UTC (permalink / raw)
To: Xiaoyu Min, Adrien Mazarguil
Cc: dev, orika, John McNamara, Marko Kovacevic, Ferruh Yigit,
Andrew Rybchenko
02/07/2019 11:45, Xiaoyu Min:
> + /**
> + * Matches a GRE optional key field.
> + *
> + * The value should a big-endian 32bit integer.
> + */
> + RTE_FLOW_ITEM_TYPE_GRE_KEY,
We probably want to use the same format as in Dekel's patch
for doxygen documentation of the action value:
/**
* Increase sequence number in the outermost TCP header.
*
* Action configuration specifies the value to increase
* TCP sequence number as a big-endian 32 bit integer.
*
* @p conf type:
* @code rte_be32_t * @endcode
*
* Using this action on non-matching traffic will result in
* undefined behavior.
*/
RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 0/4] match on GRE's key Xiaoyu Min
@ 2019-07-02 9:45 ` Xiaoyu Min
2019-07-03 14:06 ` Thomas Monjalon
2019-07-03 15:25 ` Adrien Mazarguil
0 siblings, 2 replies; 6+ messages in thread
From: Xiaoyu Min @ 2019-07-02 9:45 UTC (permalink / raw)
To: orika, Adrien Mazarguil, John McNamara, Marko Kovacevic,
Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
Cc: dev
Add new rte_flow_item_gre_key in order to match the optional key field.
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
doc/guides/prog_guide/rte_flow.rst | 8 ++++++++
lib/librte_ethdev/rte_flow.c | 1 +
lib/librte_ethdev/rte_flow.h | 7 +++++++
3 files changed, 16 insertions(+)
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index a34d012e55..f4b7baa3c3 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -980,6 +980,14 @@ Matches a GRE header.
- ``protocol``: protocol type.
- Default ``mask`` matches protocol only.
+Item: ``GRE_KEY``
+^^^^^^^^^^^^^^^^^
+
+Matches a GRE key field.
+This should be preceded by item ``GRE``
+
+- Value to be matched is a big-endian 32 bit integer
+
Item: ``FUZZY``
^^^^^^^^^^^^^^^
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 3277be1edb..f3e56d0bbe 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -55,6 +55,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = {
MK_FLOW_ITEM(NVGRE, sizeof(struct rte_flow_item_nvgre)),
MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)),
MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)),
+ MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
MK_FLOW_ITEM(FUZZY, sizeof(struct rte_flow_item_fuzzy)),
MK_FLOW_ITEM(GTP, sizeof(struct rte_flow_item_gtp)),
MK_FLOW_ITEM(GTPC, sizeof(struct rte_flow_item_gtp)),
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index f3a8fb103f..5d3702a44c 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -289,6 +289,13 @@ enum rte_flow_item_type {
*/
RTE_FLOW_ITEM_TYPE_GRE,
+ /**
+ * Matches a GRE optional key field.
+ *
+ * The value should a big-endian 32bit integer.
+ */
+ RTE_FLOW_ITEM_TYPE_GRE_KEY,
+
/**
* [META]
*
--
2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-04 2:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02 9:51 [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API Ori Kam
-- strict thread matches above, loose matches on Subject: below --
2019-06-24 15:40 [dpdk-dev] [PATCH 0/4] " Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 0/4] match on GRE's key Xiaoyu Min
2019-07-02 9:45 ` [dpdk-dev] [PATCH v4 1/4] ethdev: add GRE key field to flow API Xiaoyu Min
2019-07-03 14:06 ` Thomas Monjalon
2019-07-04 2:18 ` Jack Min
2019-07-03 15:25 ` Adrien Mazarguil
2019-07-04 2:43 ` Jack Min
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).