* [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 7:18 [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API Xiaoyu Min
@ 2019-05-14 7:18 ` Xiaoyu Min
2019-05-14 7:34 ` Andrew Rybchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Xiaoyu Min @ 2019-05-14 7:18 UTC (permalink / raw)
To: Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
Cc: dev, jackmin
Add GRE's checksum, key, and sequence field to the
struct rte_flow_item_gre in order to match.
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
lib/librte_ethdev/rte_flow.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 63f84fca65..fb04af3268 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -847,6 +847,10 @@ struct rte_flow_item_gre {
*/
rte_be16_t c_rsvd0_ver;
rte_be16_t protocol; /**< Protocol type. */
+ rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
+ rte_be16_t rsvd1; /**< present when C bit is set, optional. */
+ rte_be32_t key; /**< application specific key value, optional. */
+ rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
};
/** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
--
2.21.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 7:18 [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API Xiaoyu Min
2019-05-14 7:18 ` Xiaoyu Min
@ 2019-05-14 7:34 ` Andrew Rybchenko
2019-05-14 7:34 ` Andrew Rybchenko
2019-05-14 9:00 ` Adrien Mazarguil
2019-05-14 15:25 ` Stephen Hemminger
2019-05-15 13:24 ` [dpdk-dev] [RFC v2] ethdev: add GRE key field " Xiaoyu Min
3 siblings, 2 replies; 14+ messages in thread
From: Andrew Rybchenko @ 2019-05-14 7:34 UTC (permalink / raw)
To: Xiaoyu Min, Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit; +Cc: dev
On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> Add GRE's checksum, key, and sequence field to the
> struct rte_flow_item_gre in order to match.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
> lib/librte_ethdev/rte_flow.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 63f84fca65..fb04af3268 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> */
> rte_be16_t c_rsvd0_ver;
> rte_be16_t protocol; /**< Protocol type. */
> + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> + rte_be32_t key; /**< application specific key value, optional. */
> + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
What is the purpose to match checksum, reserved and sequence number?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 7:34 ` Andrew Rybchenko
@ 2019-05-14 7:34 ` Andrew Rybchenko
2019-05-14 9:00 ` Adrien Mazarguil
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Rybchenko @ 2019-05-14 7:34 UTC (permalink / raw)
To: Xiaoyu Min, Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit; +Cc: dev
On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> Add GRE's checksum, key, and sequence field to the
> struct rte_flow_item_gre in order to match.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
> lib/librte_ethdev/rte_flow.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 63f84fca65..fb04af3268 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> */
> rte_be16_t c_rsvd0_ver;
> rte_be16_t protocol; /**< Protocol type. */
> + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> + rte_be32_t key; /**< application specific key value, optional. */
> + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
What is the purpose to match checksum, reserved and sequence number?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 7:34 ` Andrew Rybchenko
2019-05-14 7:34 ` Andrew Rybchenko
@ 2019-05-14 9:00 ` Adrien Mazarguil
2019-05-14 9:00 ` Adrien Mazarguil
2019-05-15 8:55 ` Jack Min
1 sibling, 2 replies; 14+ messages in thread
From: Adrien Mazarguil @ 2019-05-14 9:00 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: Xiaoyu Min, Thomas Monjalon, Ferruh Yigit, dev
On Tue, May 14, 2019 at 10:34:22AM +0300, Andrew Rybchenko wrote:
> On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> > Add GRE's checksum, key, and sequence field to the
> > struct rte_flow_item_gre in order to match.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> > lib/librte_ethdev/rte_flow.h | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 63f84fca65..fb04af3268 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > */
> > rte_be16_t c_rsvd0_ver;
> > rte_be16_t protocol; /**< Protocol type. */
> > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > + rte_be32_t key; /**< application specific key value, optional. */
> > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > };
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
>
> What is the purpose to match checksum, reserved and sequence number?
I think it's not really an issue, this structure only describes a packet
header as found on the wire like other pattern items; rte_flow users only
have to provide a mask to select the fields to be matched.
However you can't just modify an existing public structure without going
through the lengthy API/ABI deprecation/versioning process.
The reason these fields were not initially part of rte_flow_item_gre is that
each of them is optional, meaning the GRE header has variable length.
They should be handled through separate objects like IPv6 options (struct
rte_flow_item_ipv6_ext), ARP (struct rte_flow_item_arp_eth_ipv4) or ICMPv6
neighbor discovery (struct rte_flow_item_icmp6_nd_opt), either all together
e.g.:
RTE_FLOW_ITEM_TYPE_GRE_OPTS
struct rte_flow_item_gre_opts {
rte_be16_t checksum; /**< Checksum for GRE header and payload (C bit). */
rte_be16_t rsvd1; /**< Reserved bits (C bit). */
rte_be32_t key; /**< Application specific key value (K bit). */
rte_be32_t sequence; /**< Sequence number for GRE packet (S bit). */
};
Or separately, since I guess only key matters no need to define the others:
RTE_FLOW_ITEM_TYPE_GRE_KEY
struct rte_flow_item_gre_key {
rte_be32_t key; /**< Application specific key value (K bit). */
};
In both cases, the default mask for this object should cover "key". Make
sure to update documentation and testpmd in the same patch.
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 9:00 ` Adrien Mazarguil
@ 2019-05-14 9:00 ` Adrien Mazarguil
2019-05-15 8:55 ` Jack Min
1 sibling, 0 replies; 14+ messages in thread
From: Adrien Mazarguil @ 2019-05-14 9:00 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: Xiaoyu Min, Thomas Monjalon, Ferruh Yigit, dev
On Tue, May 14, 2019 at 10:34:22AM +0300, Andrew Rybchenko wrote:
> On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> > Add GRE's checksum, key, and sequence field to the
> > struct rte_flow_item_gre in order to match.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> > lib/librte_ethdev/rte_flow.h | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 63f84fca65..fb04af3268 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > */
> > rte_be16_t c_rsvd0_ver;
> > rte_be16_t protocol; /**< Protocol type. */
> > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > + rte_be32_t key; /**< application specific key value, optional. */
> > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > };
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
>
> What is the purpose to match checksum, reserved and sequence number?
I think it's not really an issue, this structure only describes a packet
header as found on the wire like other pattern items; rte_flow users only
have to provide a mask to select the fields to be matched.
However you can't just modify an existing public structure without going
through the lengthy API/ABI deprecation/versioning process.
The reason these fields were not initially part of rte_flow_item_gre is that
each of them is optional, meaning the GRE header has variable length.
They should be handled through separate objects like IPv6 options (struct
rte_flow_item_ipv6_ext), ARP (struct rte_flow_item_arp_eth_ipv4) or ICMPv6
neighbor discovery (struct rte_flow_item_icmp6_nd_opt), either all together
e.g.:
RTE_FLOW_ITEM_TYPE_GRE_OPTS
struct rte_flow_item_gre_opts {
rte_be16_t checksum; /**< Checksum for GRE header and payload (C bit). */
rte_be16_t rsvd1; /**< Reserved bits (C bit). */
rte_be32_t key; /**< Application specific key value (K bit). */
rte_be32_t sequence; /**< Sequence number for GRE packet (S bit). */
};
Or separately, since I guess only key matters no need to define the others:
RTE_FLOW_ITEM_TYPE_GRE_KEY
struct rte_flow_item_gre_key {
rte_be32_t key; /**< Application specific key value (K bit). */
};
In both cases, the default mask for this object should cover "key". Make
sure to update documentation and testpmd in the same patch.
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 9:00 ` Adrien Mazarguil
2019-05-14 9:00 ` Adrien Mazarguil
@ 2019-05-15 8:55 ` Jack Min
2019-05-15 8:55 ` Jack Min
1 sibling, 1 reply; 14+ messages in thread
From: Jack Min @ 2019-05-15 8:55 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Andrew Rybchenko, Thomas Monjalon, Ferruh Yigit, dev
On Tue, 19-05-14, 11:00, Adrien Mazarguil wrote:
> On Tue, May 14, 2019 at 10:34:22AM +0300, Andrew Rybchenko wrote:
> > On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> > > Add GRE's checksum, key, and sequence field to the
> > > struct rte_flow_item_gre in order to match.
> > >
> > > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > > ---
> > > lib/librte_ethdev/rte_flow.h | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index 63f84fca65..fb04af3268 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > > */
> > > rte_be16_t c_rsvd0_ver;
> > > rte_be16_t protocol; /**< Protocol type. */
> > > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > > + rte_be32_t key; /**< application specific key value, optional. */
> > > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > > };
> > > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
> >
> > What is the purpose to match checksum, reserved and sequence number?
>
> I think it's not really an issue, this structure only describes a packet
> header as found on the wire like other pattern items; rte_flow users only
> have to provide a mask to select the fields to be matched.
>
> However you can't just modify an existing public structure without going
> through the lengthy API/ABI deprecation/versioning process.
>
> The reason these fields were not initially part of rte_flow_item_gre is that
> each of them is optional, meaning the GRE header has variable length.
> They should be handled through separate objects like IPv6 options (struct
> rte_flow_item_ipv6_ext), ARP (struct rte_flow_item_arp_eth_ipv4) or ICMPv6
> neighbor discovery (struct rte_flow_item_icmp6_nd_opt), either all together
> e.g.:
>
> RTE_FLOW_ITEM_TYPE_GRE_OPTS
>
> struct rte_flow_item_gre_opts {
> rte_be16_t checksum; /**< Checksum for GRE header and payload (C bit). */
> rte_be16_t rsvd1; /**< Reserved bits (C bit). */
> rte_be32_t key; /**< Application specific key value (K bit). */
> rte_be32_t sequence; /**< Sequence number for GRE packet (S bit). */
> };
>
> Or separately, since I guess only key matters no need to define the others:
>
> RTE_FLOW_ITEM_TYPE_GRE_KEY
>
> struct rte_flow_item_gre_key {
> rte_be32_t key; /**< Application specific key value (K bit). */
> };
I will go to this approach.
>
> In both cases, the default mask for this object should cover "key". Make
> sure to update documentation and testpmd in the same patch.
>
Sure, I will do.
Thank you, adrien, for your detail explaination.
I'll send v2 RFC soon.
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-15 8:55 ` Jack Min
@ 2019-05-15 8:55 ` Jack Min
0 siblings, 0 replies; 14+ messages in thread
From: Jack Min @ 2019-05-15 8:55 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Andrew Rybchenko, Thomas Monjalon, Ferruh Yigit, dev
On Tue, 19-05-14, 11:00, Adrien Mazarguil wrote:
> On Tue, May 14, 2019 at 10:34:22AM +0300, Andrew Rybchenko wrote:
> > On 5/14/19 10:18 AM, Xiaoyu Min wrote:
> > > Add GRE's checksum, key, and sequence field to the
> > > struct rte_flow_item_gre in order to match.
> > >
> > > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > > ---
> > > lib/librte_ethdev/rte_flow.h | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index 63f84fca65..fb04af3268 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > > */
> > > rte_be16_t c_rsvd0_ver;
> > > rte_be16_t protocol; /**< Protocol type. */
> > > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > > + rte_be32_t key; /**< application specific key value, optional. */
> > > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > > };
> > > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
> >
> > What is the purpose to match checksum, reserved and sequence number?
>
> I think it's not really an issue, this structure only describes a packet
> header as found on the wire like other pattern items; rte_flow users only
> have to provide a mask to select the fields to be matched.
>
> However you can't just modify an existing public structure without going
> through the lengthy API/ABI deprecation/versioning process.
>
> The reason these fields were not initially part of rte_flow_item_gre is that
> each of them is optional, meaning the GRE header has variable length.
> They should be handled through separate objects like IPv6 options (struct
> rte_flow_item_ipv6_ext), ARP (struct rte_flow_item_arp_eth_ipv4) or ICMPv6
> neighbor discovery (struct rte_flow_item_icmp6_nd_opt), either all together
> e.g.:
>
> RTE_FLOW_ITEM_TYPE_GRE_OPTS
>
> struct rte_flow_item_gre_opts {
> rte_be16_t checksum; /**< Checksum for GRE header and payload (C bit). */
> rte_be16_t rsvd1; /**< Reserved bits (C bit). */
> rte_be32_t key; /**< Application specific key value (K bit). */
> rte_be32_t sequence; /**< Sequence number for GRE packet (S bit). */
> };
>
> Or separately, since I guess only key matters no need to define the others:
>
> RTE_FLOW_ITEM_TYPE_GRE_KEY
>
> struct rte_flow_item_gre_key {
> rte_be32_t key; /**< Application specific key value (K bit). */
> };
I will go to this approach.
>
> In both cases, the default mask for this object should cover "key". Make
> sure to update documentation and testpmd in the same patch.
>
Sure, I will do.
Thank you, adrien, for your detail explaination.
I'll send v2 RFC soon.
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 7:18 [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API Xiaoyu Min
2019-05-14 7:18 ` Xiaoyu Min
2019-05-14 7:34 ` Andrew Rybchenko
@ 2019-05-14 15:25 ` Stephen Hemminger
2019-05-14 15:25 ` Stephen Hemminger
2019-05-15 8:56 ` Jack Min
2019-05-15 13:24 ` [dpdk-dev] [RFC v2] ethdev: add GRE key field " Xiaoyu Min
3 siblings, 2 replies; 14+ messages in thread
From: Stephen Hemminger @ 2019-05-14 15:25 UTC (permalink / raw)
To: Xiaoyu Min
Cc: Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, dev
On Tue, 14 May 2019 10:18:29 +0300
Xiaoyu Min <jackmin@mellanox.com> wrote:
> Add GRE's checksum, key, and sequence field to the
> struct rte_flow_item_gre in order to match.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
> lib/librte_ethdev/rte_flow.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 63f84fca65..fb04af3268 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> */
> rte_be16_t c_rsvd0_ver;
> rte_be16_t protocol; /**< Protocol type. */
> + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> + rte_be32_t key; /**< application specific key value, optional. */
> + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
This breaks ABI.
To extend you need to add a new flow item type and keep old
one as legacy.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 15:25 ` Stephen Hemminger
@ 2019-05-14 15:25 ` Stephen Hemminger
2019-05-15 8:56 ` Jack Min
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2019-05-14 15:25 UTC (permalink / raw)
To: Xiaoyu Min
Cc: Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, dev
On Tue, 14 May 2019 10:18:29 +0300
Xiaoyu Min <jackmin@mellanox.com> wrote:
> Add GRE's checksum, key, and sequence field to the
> struct rte_flow_item_gre in order to match.
>
> Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> ---
> lib/librte_ethdev/rte_flow.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 63f84fca65..fb04af3268 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> */
> rte_be16_t c_rsvd0_ver;
> rte_be16_t protocol; /**< Protocol type. */
> + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> + rte_be32_t key; /**< application specific key value, optional. */
> + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
This breaks ABI.
To extend you need to add a new flow item type and keep old
one as legacy.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-14 15:25 ` Stephen Hemminger
2019-05-14 15:25 ` Stephen Hemminger
@ 2019-05-15 8:56 ` Jack Min
2019-05-15 8:56 ` Jack Min
1 sibling, 1 reply; 14+ messages in thread
From: Jack Min @ 2019-05-15 8:56 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, dev
On Tue, 19-05-14, 08:25, Stephen Hemminger wrote:
> On Tue, 14 May 2019 10:18:29 +0300
> Xiaoyu Min <jackmin@mellanox.com> wrote:
>
> > Add GRE's checksum, key, and sequence field to the
> > struct rte_flow_item_gre in order to match.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> > lib/librte_ethdev/rte_flow.h | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 63f84fca65..fb04af3268 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > */
> > rte_be16_t c_rsvd0_ver;
> > rte_be16_t protocol; /**< Protocol type. */
> > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > + rte_be32_t key; /**< application specific key value, optional. */
> > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > };
> >
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
>
> This breaks ABI.
>
> To extend you need to add a new flow item type and keep old
> one as legacy.
OK, I'll add a new flow item.
Thank you.
-Jack
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API
2019-05-15 8:56 ` Jack Min
@ 2019-05-15 8:56 ` Jack Min
0 siblings, 0 replies; 14+ messages in thread
From: Jack Min @ 2019-05-15 8:56 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Adrien Mazarguil, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, dev
On Tue, 19-05-14, 08:25, Stephen Hemminger wrote:
> On Tue, 14 May 2019 10:18:29 +0300
> Xiaoyu Min <jackmin@mellanox.com> wrote:
>
> > Add GRE's checksum, key, and sequence field to the
> > struct rte_flow_item_gre in order to match.
> >
> > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > ---
> > lib/librte_ethdev/rte_flow.h | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 63f84fca65..fb04af3268 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -847,6 +847,10 @@ struct rte_flow_item_gre {
> > */
> > rte_be16_t c_rsvd0_ver;
> > rte_be16_t protocol; /**< Protocol type. */
> > + rte_be16_t checksum; /**< chksum for the header and payload, optional.*/
> > + rte_be16_t rsvd1; /**< present when C bit is set, optional. */
> > + rte_be32_t key; /**< application specific key value, optional. */
> > + rte_be32_t sequence; /**< sequence num for the GRE packet, optional. */
> > };
> >
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GRE. */
>
> This breaks ABI.
>
> To extend you need to add a new flow item type and keep old
> one as legacy.
OK, I'll add a new flow item.
Thank you.
-Jack
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [RFC v2] ethdev: add GRE key field to flow API
2019-05-14 7:18 [dpdk-dev] [RFC] ethdev: add GRE optional fields to flow API Xiaoyu Min
` (2 preceding siblings ...)
2019-05-14 15:25 ` Stephen Hemminger
@ 2019-05-15 13:24 ` Xiaoyu Min
2019-05-15 13:24 ` Xiaoyu Min
3 siblings, 1 reply; 14+ messages in thread
From: Xiaoyu Min @ 2019-05-15 13:24 UTC (permalink / raw)
To: Adrien Mazarguil, John McNamara, Marko Kovacevic,
Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
Cc: dev, jackmin
Add new rte_flow_item_gre_key in order to match the optional key field.
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
v2:
* new rte_flow_item_gre_key added instead of expanding existing
rte_flow_item_gre.
* updated document.
doc/guides/prog_guide/rte_flow.rst | 9 +++++++++
lib/librte_ethdev/rte_flow.c | 1 +
lib/librte_ethdev/rte_flow.h | 27 +++++++++++++++++++++++++++
3 files changed, 37 insertions(+)
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 937f52bce1..8b79818f44 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -980,6 +980,15 @@ 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``
+
+- ``key``: key value.
+- Default ``mask`` matches key only.
+
Item: ``FUZZY``
^^^^^^^^^^^^^^^
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 3277be1edb..1cd5f4d263 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(struct rte_flow_item_gre_key)),
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 63f84fca65..b36c3b980a 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.
+ *
+ * See struct rte_flow_item_gre_key.
+ */
+ RTE_FLOW_ITEM_TYPE_GRE_KEY,
+
/**
* [META]
*
@@ -856,6 +863,26 @@ static const struct rte_flow_item_gre rte_flow_item_gre_mask = {
};
#endif
+/**
+ * RTE_FLOW_ITEM_GRE_KEY.
+ *
+ * Matches the presence of a GRE key.
+ *
+ * Normally preceding by:
+ *
+ * - RTE_FLOW_ITEM_TYPE_GRE
+ */
+struct rte_flow_item_gre_key {
+ rte_be32_t key; /**< Application specific key value (K bit). */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_GRE_KEY. */
+#ifndef __cplusplus
+static const struct rte_flow_item_gre_key rte_flow_gre_key_mask = {
+ .key = RTE_BE32(UINT32_MAX),
+};
+#endif
+
/**
* RTE_FLOW_ITEM_TYPE_FUZZY
*
--
2.21.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [RFC v2] ethdev: add GRE key field to flow API
2019-05-15 13:24 ` [dpdk-dev] [RFC v2] ethdev: add GRE key field " Xiaoyu Min
@ 2019-05-15 13:24 ` Xiaoyu Min
0 siblings, 0 replies; 14+ messages in thread
From: Xiaoyu Min @ 2019-05-15 13:24 UTC (permalink / raw)
To: Adrien Mazarguil, John McNamara, Marko Kovacevic,
Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
Cc: dev, jackmin
Add new rte_flow_item_gre_key in order to match the optional key field.
Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
---
v2:
* new rte_flow_item_gre_key added instead of expanding existing
rte_flow_item_gre.
* updated document.
doc/guides/prog_guide/rte_flow.rst | 9 +++++++++
lib/librte_ethdev/rte_flow.c | 1 +
lib/librte_ethdev/rte_flow.h | 27 +++++++++++++++++++++++++++
3 files changed, 37 insertions(+)
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 937f52bce1..8b79818f44 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -980,6 +980,15 @@ 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``
+
+- ``key``: key value.
+- Default ``mask`` matches key only.
+
Item: ``FUZZY``
^^^^^^^^^^^^^^^
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 3277be1edb..1cd5f4d263 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(struct rte_flow_item_gre_key)),
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 63f84fca65..b36c3b980a 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.
+ *
+ * See struct rte_flow_item_gre_key.
+ */
+ RTE_FLOW_ITEM_TYPE_GRE_KEY,
+
/**
* [META]
*
@@ -856,6 +863,26 @@ static const struct rte_flow_item_gre rte_flow_item_gre_mask = {
};
#endif
+/**
+ * RTE_FLOW_ITEM_GRE_KEY.
+ *
+ * Matches the presence of a GRE key.
+ *
+ * Normally preceding by:
+ *
+ * - RTE_FLOW_ITEM_TYPE_GRE
+ */
+struct rte_flow_item_gre_key {
+ rte_be32_t key; /**< Application specific key value (K bit). */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_GRE_KEY. */
+#ifndef __cplusplus
+static const struct rte_flow_item_gre_key rte_flow_gre_key_mask = {
+ .key = RTE_BE32(UINT32_MAX),
+};
+#endif
+
/**
* RTE_FLOW_ITEM_TYPE_FUZZY
*
--
2.21.0
^ permalink raw reply [flat|nested] 14+ messages in thread