From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Sean Zhang (Networking SW)" <xiazhang@nvidia.com>,
Ori Kam <orika@nvidia.com>,
"NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
Matan Azrad <matan@nvidia.com>
Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [RFC 1/3] ethdev: support GRE optional fields
Date: Tue, 25 Jan 2022 11:37:17 +0000 [thread overview]
Message-ID: <d154e5c9-f8f7-100b-e4bd-3577371c9cd7@intel.com> (raw)
In-Reply-To: <DM8PR12MB5398F8D0D6396F4C87C9CDCFA25F9@DM8PR12MB5398.namprd12.prod.outlook.com>
On 1/25/2022 9:49 AM, Sean Zhang (Networking SW) wrote:
> Hi,
>
>> -----Original Message-----
>> From: Ori Kam <orika@nvidia.com>
>> Sent: Wednesday, January 19, 2022 6:57 PM
>> To: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
>> Sean Zhang (Networking SW) <xiazhang@nvidia.com>; Matan Azrad
>> <matan@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com>
>> Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org
>> Subject: RE: [RFC 1/3] ethdev: support GRE optional fields
>>
>> Hi,
>>
>>> -----Original Message-----
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>> Subject: Re: [RFC 1/3] ethdev: support GRE optional fields
>>>
>>> 19/01/2022 10:53, Ferruh Yigit:
>>>> On 12/30/2021 3:08 AM, Sean Zhang wrote:
>>>>> --- a/lib/ethdev/rte_flow.h
>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>> /**
>>>>> + * RTE_FLOW_ITEM_TYPE_GRE_OPTION.
>>>>> + *
>>>>> + * Matches GRE optional fields in header.
>>>>> + */
>>>>> +struct rte_gre_hdr_option {
>>>>> + rte_be16_t checksum;
>>>>> + rte_be32_t key;
>>>>> + rte_be32_t sequence;
>>>>> +};
>>>>> +
>>>>
>>>> Hi Ori, Andrew,
>>>>
>>>> The decision was to have protocol structs in the net library and
>>>> flow structs use from there, wasn't it?
>>>> (Btw, a deprecation notice is still pending to clear some existing
>>>> ones)
>>>>
>>>> So for the GRE optional fields, what about having a struct in the
>> 'rte_gre.h'?
>>>> (Also perhaps an GRE extended protocol header can be defined
>>>> combining 'rte_gre_hdr' and optional fields struct.) Later flow API
>>>> struct can embed that struct.
>>>
>>> +1 for using librte_net.
>>> This addition in rte_flow looks to be a mistake.
>>> Please fix the next version.
>>>
>> Nice idea,
>> but my main concern is that the header should have the header is defined.
>> Since some of the fields are optional this will look something like this:
>> gre_hdr_option_checksum {
>> rte_be_16_t checksum;
>> }
>>
>> gre_hdr_option_key {
>> rte_be_32_t key;
>> }
>>
>> gre_hdr_option_ sequence {
>> rte_be_32_t sequence;
>> }
>>
>> I don't want to have so many rte_flow_items, Has more and more protocols
>> have optional data it doesn't make sense to create the item for each.
>>
>> If I'm looking at it from an ideal place, I would like that the optional fields will
>> be part of the original item.
>> For example in test pmd I would like to write:
>> Eth / ipv4 / udp / gre flags is key & checksum checksum is yyy key is xxx / end
>> And not Eth / ipv4 / udp / gre flags is key & checksum / gre_option checksum
>> is yyy key is xxx / end This means that the structure will look like this:
>> struct rte_flow_item_gre {
>> union {
>> struct {
>> /**
>> * Checksum (1b), reserved 0 (12b), version (3b).
>> * Refer to RFC 2784.
>> */
>> rte_be16_t c_rsvd0_ver;
>> rte_be16_t protocol; /**< Protocol type. */
>> }
>> struct rte_gre_hdr hdr
>> }
>> rte_be_16_t checksum;
>> rte_be_32_t key;
>> rte_be_32_t sequence;
>> };
>> The main issue with this is that it breaks ABI, Maybe to solve this we can
>> create a new structure gre_ext?
>>
>> In any way I think we should think how we allow adding members to
>> structures without ABI breakage.
>>
>> Best,
>> Ori
>
> Thanks for the comments and suggestion.
> So the acceptable solution is to have new structs define in rte_gre.h?
> struct gre_hdr_opt_checksum {
> rte_be_16_t checksum;
> }
>
> struct gre_hdr_opt_key {
> rte_be_32_t key;
> }
>
> struct gre_hdr_opt_ sequence {
> rte_be_32_t sequence;
> }
>
> And to add new struct gre_ext defined in rte_flow.h:
> struct gre_ext {
> struct rte_gre_hdr hdr;
> struct gre_hdr_opt_checkum checksum;
> struct rte_hdr_opt_key key;
> struct rte_hdr_opt_seq seq;
> };
>
> And we use struct gre_ext for this new added flow item gre_option.
>
What about having a struct for 'options' and use in in flow item for options,
like:
struct gre_hdr_opt {
struct gre_hdr_opt_checkum checksum;
struct rte_hdr_opt_key key;
struct rte_hdr_opt_seq seq;
}
struct gre_hdr_ext {
struct rte_gre_hdr hdr;
struct gre_hdr_opt;
}
struct rte_flow_item_gre_opt {
struct gre_hdr_opt hdr;
}
> Correct me if my understanding is not right.
>
> Thanks,
> Sean
>
>
next prev parent reply other threads:[~2022-01-25 11:37 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-30 3:08 [RFC 0/3] Add support for GRE optional fields matching Sean Zhang
2021-12-30 3:08 ` [RFC 1/3] ethdev: support GRE optional fields Sean Zhang
2022-01-09 12:30 ` Ori Kam
2022-01-11 3:44 ` Sean Zhang (Networking SW)
2022-01-11 7:24 ` Ori Kam
2022-01-11 8:31 ` Sean Zhang (Networking SW)
2022-01-19 9:53 ` Ferruh Yigit
2022-01-19 10:01 ` Thomas Monjalon
2022-01-19 10:56 ` Ori Kam
2022-01-25 9:49 ` Sean Zhang (Networking SW)
2022-01-25 11:37 ` Ferruh Yigit [this message]
2022-01-25 13:06 ` Ori Kam
2022-01-25 14:29 ` Ferruh Yigit
2022-01-25 16:03 ` Ori Kam
2022-01-26 8:44 ` [v1 0/4] Add support for GRE optional fields matching Sean Zhang
2022-01-26 8:44 ` [v1 1/4] lib: add optional fields in GRE header Sean Zhang
2022-02-01 12:47 ` Ori Kam
2022-01-26 8:44 ` [v1 2/4] ethdev: support GRE optional fields Sean Zhang
2022-02-01 12:57 ` Ori Kam
2022-02-04 15:15 ` Ferruh Yigit
2022-01-26 8:44 ` [v1 3/4] app/testpmd: add gre_option item command Sean Zhang
2022-02-01 12:57 ` Ori Kam
2022-01-26 8:44 ` [v1 4/4] net/mlx5: support matching optional fields of GRE Sean Zhang
2022-02-01 10:50 ` Ferruh Yigit
2022-02-01 11:13 ` [v1 0/4] Add support for GRE optional fields matching Ferruh Yigit
2022-02-11 1:45 ` [v2 " Sean Zhang
2022-02-11 1:45 ` [v2 1/4] lib: add optional fields in GRE header Sean Zhang
2022-02-11 9:38 ` Ferruh Yigit
2022-02-11 10:23 ` Sean Zhang (Networking SW)
2022-02-11 10:37 ` Ferruh Yigit
2022-02-11 10:12 ` Ori Kam
2022-02-11 1:45 ` [v2 2/4] ethdev: support GRE optional fields Sean Zhang
2022-02-11 10:10 ` Ori Kam
2022-02-11 1:45 ` [v2 3/4] app/testpmd: add gre_option item command Sean Zhang
2022-02-11 10:10 ` Ori Kam
2022-02-11 1:45 ` [v2 4/4] net/mlx5: support matching optional fields of GRE Sean Zhang
2022-02-17 6:27 ` [PATCH] " Sean Zhang
2022-02-17 8:33 ` Thomas Monjalon
2022-02-21 3:00 ` Sean Zhang (Networking SW)
2022-02-25 15:31 ` Thomas Monjalon
2022-02-26 0:57 ` Sean Zhang (Networking SW)
2022-02-24 13:18 ` Raslan Darawsheh
2022-02-25 1:18 ` Sean Zhang (Networking SW)
2022-02-25 1:14 ` [v4] " Sean Zhang
2022-02-25 15:32 ` Thomas Monjalon
2022-02-25 17:55 ` Ferruh Yigit
2022-02-25 18:32 ` Thomas Monjalon
2022-02-11 9:36 ` [v2 0/4] Add support for GRE optional fields matching Ferruh Yigit
2022-02-11 10:33 ` Sean Zhang (Networking SW)
2022-02-11 10:38 ` Ferruh Yigit
2022-02-11 16:14 ` Ferruh Yigit
2021-12-30 3:08 ` [RFC 2/3] app/testpmd: add gre_option item command Sean Zhang
2021-12-30 3:08 ` [RFC 3/3] net/mlx5: support matching on optional fields of GRE Sean Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d154e5c9-f8f7-100b-e4bd-3577371c9cd7@intel.com \
--to=ferruh.yigit@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=matan@nvidia.com \
--cc=orika@nvidia.com \
--cc=thomas@monjalon.net \
--cc=xiazhang@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).