* [dpdk-dev] [PATCH] ethdev: update qfi definition
@ 2021-03-23 12:11 Raslan Darawsheh
2021-03-26 13:12 ` Ferruh Yigit
` (2 more replies)
0 siblings, 3 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-23 12:11 UTC (permalink / raw)
To: dev; +Cc: viacheslavo, shirik, ying.a.wang, stable
qfi field is 8 bits which represent single bit for
PPP (paging Policy Presence) single bit for RQI
(Reflective QoS Indicator) and 6 bits for qfi
(QoS Flow Identifier)
This update the doxygen format and the mask for qfi
to properly identify the full 8 bits of the field.
note: changing the default mask would cause different
patterns generated by testpmd.
Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
Cc: ying.a.wang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
lib/librte_ethdev/rte_flow.h | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a27d..dd39c4c3c2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
- ``gtp_psc``: match GTP PDU extension header with type 0x85.
- ``pdu_type {unsigned}``: PDU type.
- - ``qfi {unsigned}``: QoS flow identifier.
+
+ - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
- ``pppoes``, ``pppoed``: match PPPoE header.
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 669e677e91..79106e0246 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1392,14 +1392,14 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
*/
struct rte_flow_item_gtp_psc {
uint8_t pdu_type; /**< PDU type. */
- uint8_t qfi; /**< QoS flow identifier. */
+ uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
};
/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
#ifndef __cplusplus
static const struct rte_flow_item_gtp_psc
rte_flow_item_gtp_psc_mask = {
- .qfi = 0x3f,
+ .qfi = 0xff,
};
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
2021-03-23 12:11 [dpdk-dev] [PATCH] ethdev: update qfi definition Raslan Darawsheh
@ 2021-03-26 13:12 ` Ferruh Yigit
2021-03-29 8:53 ` Ori Kam
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-04-14 12:40 ` [dpdk-dev] [PATCH] " Ferruh Yigit
2 siblings, 1 reply; 37+ messages in thread
From: Ferruh Yigit @ 2021-03-26 13:12 UTC (permalink / raw)
To: Raslan Darawsheh, dev, Ori Kam, Andrew Rybchenko, Ivan Malov
Cc: viacheslavo, shirik, ying.a.wang, stable, Thomas Monjalon,
Olivier Matz
On 3/23/2021 12:11 PM, Raslan Darawsheh wrote:
> qfi field is 8 bits which represent single bit for
> PPP (paging Policy Presence) single bit for RQI
> (Reflective QoS Indicator) and 6 bits for qfi
> (QoS Flow Identifier)
Can you please put a reference for above information?
>
> This update the doxygen format and the mask for qfi
> to properly identify the full 8 bits of the field.
>
> note: changing the default mask would cause different
> patterns generated by testpmd.
>
> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> Cc: ying.a.wang@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> lib/librte_ethdev/rte_flow.h | 4 ++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index f59eb8a27d..dd39c4c3c2 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
>
> - ``pdu_type {unsigned}``: PDU type.
> - - ``qfi {unsigned}``: QoS flow identifier.
> +
> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
>
> - ``pppoes``, ``pppoed``: match PPPoE header.
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 669e677e91..79106e0246 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1392,14 +1392,14 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
> */
> struct rte_flow_item_gtp_psc {
> uint8_t pdu_type; /**< PDU type. */
> - uint8_t qfi; /**< QoS flow identifier. */
> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> };
By design requirement, rte_flow_item_* should start with the relevant protocol
header.
There is already a deprecation notice for using protocol header directly in the
rte_flow_item* [1] and Adrew/Ivan already fixed a few of them [2].
Your patch is not directly related on this, but since you are touching to the
flow_item, would you mind create a protocol struct for it first, and use it in
the "struct rte_flow_item_gtp_psc"?
So the protocol related update can be done in the protocol header, instead of
rte_flow struct.
[1]
https://git.dpdk.org/dpdk/tree/doc/guides/rel_notes/deprecation.rst?h=v21.02#n99
[2]
https://git.dpdk.org/next/dpdk-next-net/commit/?id=4a061a7bd70bfa023de23e8e654e
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
2021-03-26 13:12 ` Ferruh Yigit
@ 2021-03-29 8:53 ` Ori Kam
2021-03-29 9:06 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Ori Kam @ 2021-03-29 8:53 UTC (permalink / raw)
To: Ferruh Yigit, Raslan Darawsheh, dev@dpdk.org, Andrew Rybchenko,
Ivan Malov
Cc: Slava Ovsiienko, Shiri Kuzin, ying.a.wang@intel.com,
stable@dpdk.org, NBU-Contact-Thomas Monjalon, Olivier Matz
Hi
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Subject: Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
>
> On 3/23/2021 12:11 PM, Raslan Darawsheh wrote:
> > qfi field is 8 bits which represent single bit for
> > PPP (paging Policy Presence) single bit for RQI
> > (Reflective QoS Indicator) and 6 bits for qfi
> > (QoS Flow Identifier)
>
> Can you please put a reference for above information?
> >
> > This update the doxygen format and the mask for qfi
> > to properly identify the full 8 bits of the field.
> >
> > note: changing the default mask would cause different
> > patterns generated by testpmd.
> >
> > Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> > Cc: ying.a.wang@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > ---
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> > lib/librte_ethdev/rte_flow.h | 4 ++--
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index f59eb8a27d..dd39c4c3c2 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -3742,7 +3742,8 @@ This section lists supported pattern items and their
> attributes, if any.
> > - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> >
> > - ``pdu_type {unsigned}``: PDU type.
> > - - ``qfi {unsigned}``: QoS flow identifier.
> > +
> > + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> >
> > - ``pppoes``, ``pppoed``: match PPPoE header.
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 669e677e91..79106e0246 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -1392,14 +1392,14 @@ static const struct rte_flow_item_meta
> rte_flow_item_meta_mask = {
> > */
> > struct rte_flow_item_gtp_psc {
> > uint8_t pdu_type; /**< PDU type. */
> > - uint8_t qfi; /**< QoS flow identifier. */
> > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> > };
>
> By design requirement, rte_flow_item_* should start with the relevant protocol
> header.
>
> There is already a deprecation notice for using protocol header directly in the
> rte_flow_item* [1] and Adrew/Ivan already fixed a few of them [2].
>
> Your patch is not directly related on this, but since you are touching to the
> flow_item, would you mind create a protocol struct for it first, and use it in
> the "struct rte_flow_item_gtp_psc"?
> So the protocol related update can be done in the protocol header, instead of
> rte_flow struct.
>
+1
>
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.dpdk.
> org%2Fdpdk%2Ftree%2Fdoc%2Fguides%2Frel_notes%2Fdeprecation.rst%3Fh%3
> Dv21.02%23n99&data=04%7C01%7Corika%40nvidia.com%7C6391a4c0640
> f4592b70b08d8f058e322%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0
> %7C637523611870497932%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdat
> a=vU%2F5oO47zb9erTnZL1pl9j0nHCKzea3NJgOeo1FTW0Q%3D&reserved=
> 0
>
> [2]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.dpdk.
> org%2Fnext%2Fdpdk-next-
> net%2Fcommit%2F%3Fid%3D4a061a7bd70bfa023de23e8e654e&data=04%
> 7C01%7Corika%40nvidia.com%7C6391a4c0640f4592b70b08d8f058e322%7C43
> 083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637523611870497932%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> 1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=WurlQ5VSLqFGVthfRj363xZsC
> No3xJuvxNQCFVcxdkk%3D&reserved=0
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
2021-03-29 8:53 ` Ori Kam
@ 2021-03-29 9:06 ` Raslan Darawsheh
2021-03-29 11:14 ` Ferruh Yigit
0 siblings, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-29 9:06 UTC (permalink / raw)
To: Ori Kam, Ferruh Yigit, dev@dpdk.org, Andrew Rybchenko, Ivan Malov
Cc: Slava Ovsiienko, Shiri Kuzin, ying.a.wang@intel.com,
stable@dpdk.org, NBU-Contact-Thomas Monjalon, Olivier Matz
Hi,
> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Monday, March 29, 2021 11:53 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Raslan Darawsheh
> <rasland@nvidia.com>; dev@dpdk.org; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ivan Malov <ivan.malov@oktetlabs.ru>
> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri Kuzin
> <shirik@nvidia.com>; ying.a.wang@intel.com; stable@dpdk.org; NBU-
> Contact-Thomas Monjalon <thomas@monjalon.net>; Olivier Matz
> <olivier.matz@6wind.com>
> Subject: RE: [dpdk-dev] [PATCH] ethdev: update qfi definition
>
> Hi
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
> >
> > On 3/23/2021 12:11 PM, Raslan Darawsheh wrote:
> > > qfi field is 8 bits which represent single bit for
> > > PPP (paging Policy Presence) single bit for RQI
> > > (Reflective QoS Indicator) and 6 bits for qfi
> > > (QoS Flow Identifier)
> >
> > Can you please put a reference for above information?
> > >
Sure, will send in V2,
> > > This update the doxygen format and the mask for qfi
> > > to properly identify the full 8 bits of the field.
> > >
> > > note: changing the default mask would cause different
> > > patterns generated by testpmd.
> > >
> > > Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> > > Cc: ying.a.wang@intel.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > > ---
> > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> > > lib/librte_ethdev/rte_flow.h | 4 ++--
> > > 2 files changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > index f59eb8a27d..dd39c4c3c2 100644
> > > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
> their
> > attributes, if any.
> > > - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> > >
> > > - ``pdu_type {unsigned}``: PDU type.
> > > - - ``qfi {unsigned}``: QoS flow identifier.
> > > +
> > > + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> > >
> > > - ``pppoes``, ``pppoed``: match PPPoE header.
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index 669e677e91..79106e0246 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -1392,14 +1392,14 @@ static const struct rte_flow_item_meta
> > rte_flow_item_meta_mask = {
> > > */
> > > struct rte_flow_item_gtp_psc {
> > > uint8_t pdu_type; /**< PDU type. */
> > > - uint8_t qfi; /**< QoS flow identifier. */
> > > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> > > };
> >
> > By design requirement, rte_flow_item_* should start with the relevant
> protocol
> > header.
> >
> > There is already a deprecation notice for using protocol header directly in
> the
> > rte_flow_item* [1] and Adrew/Ivan already fixed a few of them [2].
> >
> > Your patch is not directly related on this, but since you are touching to the
> > flow_item, would you mind create a protocol struct for it first, and use it in
> > the "struct rte_flow_item_gtp_psc"?
> > So the protocol related update can be done in the protocol header, instead
> of
> > rte_flow struct.
> >
> +1
Sure, I can create the new protocol and use it , and will send in V2. But, wouldn't it cause any ABI breakage ?
>
> >
> > [1]
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.d
> pdk.
> >
> org%2Fdpdk%2Ftree%2Fdoc%2Fguides%2Frel_notes%2Fdeprecation.rst%3F
> h%3
> >
> Dv21.02%23n99&data=04%7C01%7Corika%40nvidia.com%7C6391a4c064
> 0
> >
> f4592b70b08d8f058e322%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0
> >
> %7C637523611870497932%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> w
> >
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda
> t
> >
> a=vU%2F5oO47zb9erTnZL1pl9j0nHCKzea3NJgOeo1FTW0Q%3D&reserve
> d=
> > 0
> >
> > [2]
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.d
> pdk.
> > org%2Fnext%2Fdpdk-next-
> >
> net%2Fcommit%2F%3Fid%3D4a061a7bd70bfa023de23e8e654e&data=0
> 4%
> >
> 7C01%7Corika%40nvidia.com%7C6391a4c0640f4592b70b08d8f058e322%7C43
> >
> 083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637523611870497932%7CU
> >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik
> >
> 1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=WurlQ5VSLqFGVthfRj363xZs
> C
> > No3xJuvxNQCFVcxdkk%3D&reserved=0
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
2021-03-29 9:06 ` Raslan Darawsheh
@ 2021-03-29 11:14 ` Ferruh Yigit
0 siblings, 0 replies; 37+ messages in thread
From: Ferruh Yigit @ 2021-03-29 11:14 UTC (permalink / raw)
To: Raslan Darawsheh, Ori Kam, dev@dpdk.org, Andrew Rybchenko,
Ivan Malov
Cc: Slava Ovsiienko, Shiri Kuzin, ying.a.wang@intel.com,
stable@dpdk.org, NBU-Contact-Thomas Monjalon, Olivier Matz
On 3/29/2021 10:06 AM, Raslan Darawsheh wrote:
> Hi,
>
>> -----Original Message-----
>> From: Ori Kam <orika@nvidia.com>
>> Sent: Monday, March 29, 2021 11:53 AM
>> To: Ferruh Yigit <ferruh.yigit@intel.com>; Raslan Darawsheh
>> <rasland@nvidia.com>; dev@dpdk.org; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; Ivan Malov <ivan.malov@oktetlabs.ru>
>> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri Kuzin
>> <shirik@nvidia.com>; ying.a.wang@intel.com; stable@dpdk.org; NBU-
>> Contact-Thomas Monjalon <thomas@monjalon.net>; Olivier Matz
>> <olivier.matz@6wind.com>
>> Subject: RE: [dpdk-dev] [PATCH] ethdev: update qfi definition
>>
>> Hi
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>> Subject: Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
>>>
>>> On 3/23/2021 12:11 PM, Raslan Darawsheh wrote:
>>>> qfi field is 8 bits which represent single bit for
>>>> PPP (paging Policy Presence) single bit for RQI
>>>> (Reflective QoS Indicator) and 6 bits for qfi
>>>> (QoS Flow Identifier)
>>>
>>> Can you please put a reference for above information?
>>>>
> Sure, will send in V2,
>
>>>> This update the doxygen format and the mask for qfi
>>>> to properly identify the full 8 bits of the field.
>>>>
>>>> note: changing the default mask would cause different
>>>> patterns generated by testpmd.
>>>>
>>>> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
>>>> Cc: ying.a.wang@intel.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
>>>> ---
>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
>>>> lib/librte_ethdev/rte_flow.h | 4 ++--
>>>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index f59eb8a27d..dd39c4c3c2 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
>> their
>>> attributes, if any.
>>>> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
>>>>
>>>> - ``pdu_type {unsigned}``: PDU type.
>>>> - - ``qfi {unsigned}``: QoS flow identifier.
>>>> +
>>>> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
>>>>
>>>> - ``pppoes``, ``pppoed``: match PPPoE header.
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>>> index 669e677e91..79106e0246 100644
>>>> --- a/lib/librte_ethdev/rte_flow.h
>>>> +++ b/lib/librte_ethdev/rte_flow.h
>>>> @@ -1392,14 +1392,14 @@ static const struct rte_flow_item_meta
>>> rte_flow_item_meta_mask = {
>>>> */
>>>> struct rte_flow_item_gtp_psc {
>>>> uint8_t pdu_type; /**< PDU type. */
>>>> - uint8_t qfi; /**< QoS flow identifier. */
>>>> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
>>>> };
>>>
>>> By design requirement, rte_flow_item_* should start with the relevant
>> protocol
>>> header.
>>>
>>> There is already a deprecation notice for using protocol header directly in
>> the
>>> rte_flow_item* [1] and Adrew/Ivan already fixed a few of them [2].
>>>
>>> Your patch is not directly related on this, but since you are touching to the
>>> flow_item, would you mind create a protocol struct for it first, and use it in
>>> the "struct rte_flow_item_gtp_psc"?
>>> So the protocol related update can be done in the protocol header, instead
>> of
>>> rte_flow struct.
>>>
>> +1
> Sure, I can create the new protocol and use it , and will send in V2. But, wouldn't it cause any ABI breakage ?
For the protocol header, it won't be problem since it is a new struct.
For the flow item struct, it may be, that is why need to use a union for now,
please check what Adrew/Ivan did.
>>
>>>
>>> [1]
>>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.d
>> pdk.
>>>
>> org%2Fdpdk%2Ftree%2Fdoc%2Fguides%2Frel_notes%2Fdeprecation.rst%3F
>> h%3
>>>
>> Dv21.02%23n99&data=04%7C01%7Corika%40nvidia.com%7C6391a4c064
>> 0
>>>
>> f4592b70b08d8f058e322%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0
>>>
>> %7C637523611870497932%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>> w
>>>
>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda
>> t
>>>
>> a=vU%2F5oO47zb9erTnZL1pl9j0nHCKzea3NJgOeo1FTW0Q%3D&reserve
>> d=
>>> 0
>>>
>>> [2]
>>>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.d
>> pdk.
>>> org%2Fnext%2Fdpdk-next-
>>>
>> net%2Fcommit%2F%3Fid%3D4a061a7bd70bfa023de23e8e654e&data=0
>> 4%
>>>
>> 7C01%7Corika%40nvidia.com%7C6391a4c0640f4592b70b08d8f058e322%7C43
>>>
>> 083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637523611870497932%7CU
>>>
>> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
>> 6Ik
>>>
>> 1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=WurlQ5VSLqFGVthfRj363xZs
>> C
>>> No3xJuvxNQCFVcxdkk%3D&reserved=0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support
2021-03-23 12:11 [dpdk-dev] [PATCH] ethdev: update qfi definition Raslan Darawsheh
2021-03-26 13:12 ` Ferruh Yigit
@ 2021-03-30 7:50 ` Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-14 12:40 ` [dpdk-dev] [PATCH] " Ferruh Yigit
2 siblings, 2 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 7:50 UTC (permalink / raw)
To: dev; +Cc: viacheslavo, shirik
This is fixing the gtp psc support to match the RFC 38415-g30
v2: introduce new header definition for gtp psc
update commit msg for rte flow item change.
Raslan Darawsheh (2):
ethdev: add new ext hdr for gtp psc
ethdev: update qfi definition
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +-
lib/librte_ethdev/rte_flow.h | 18 +++++++++--
lib/librte_net/rte_gtp.h | 34 +++++++++++++++++++++
3 files changed, 51 insertions(+), 4 deletions(-)
--
2.29.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support Raslan Darawsheh
@ 2021-03-30 7:50 ` Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 2/2] ethdev: update qfi definition Raslan Darawsheh
1 sibling, 2 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 7:50 UTC (permalink / raw)
To: dev; +Cc: viacheslavo, shirik
Define new rte header for gtp PDU session container
based on RFC 38415-g30
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
index 6a6f9b238d..8af48ea1ec 100644
--- a/lib/librte_net/rte_gtp.h
+++ b/lib/librte_net/rte_gtp.h
@@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
uint8_t next_ext; /**< Next Extension Header Type. */
} __rte_packed;
+/**
+ * Optional extention for GTP with next_ext set to 0x85
+ * defined based on RFC 38415-g30.
+ */
+__extension__
+struct rte_gtp_psc {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ union {
+ struct {
+ uint8_t snp:1; /**< Sequence number presence */
+ uint8_t spare_dl1:2; /**< spare down link bits */
+ };
+ struct {
+ uint8_t dl_delay_ind:1; /**< dl delay result presence */
+ uint8_t ul_delay_ind:1; /**< ul delay result presence */
+ uint8_t snp_ul1:1; /**< Sequence number presence ul */
+ };
+ };
+ union {
+ struct {
+ uint8_t ppp:1; /**< Paging policy presence */
+ uint8_t rqi:1; /**< Reflective Qos Indicator */
+ };
+ struct {
+ uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
+ uint8_t spare_ul2:1; /**< spare up link bits */
+ };
+ };
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t data[0]; /**< data feilds */
+} __rte_packed;
+
/** GTP header length */
#define RTE_ETHER_GTP_HLEN \
(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] ethdev: update qfi definition
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-03-30 7:50 ` Raslan Darawsheh
1 sibling, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 7:50 UTC (permalink / raw)
To: dev; +Cc: viacheslavo, shirik, ying.a.wang, stable
qfi field is 8 bits which represent single bit for
PPP (paging Policy Presence) single bit for RQI
(Reflective QoS Indicator) and 6 bits for qfi
(QoS Flow Identifier) based on RFC 38415-g30
This update the doxygen format and the mask for qfi
to properly identify the full 8 bits of the field.
note: changing the default mask would cause different
patterns generated by testpmd.
Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
Cc: ying.a.wang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
lib/librte_ethdev/rte_flow.h | 18 +++++++++++++++---
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a27d..dd39c4c3c2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
- ``gtp_psc``: match GTP PDU extension header with type 0x85.
- ``pdu_type {unsigned}``: PDU type.
- - ``qfi {unsigned}``: QoS flow identifier.
+
+ - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
- ``pppoes``, ``pppoed``: match PPPoE header.
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 6cc57136ac..1eb9711707 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -20,6 +20,7 @@
#include <rte_arp.h>
#include <rte_common.h>
#include <rte_ether.h>
+#include <rte_gtp.h>
#include <rte_icmp.h>
#include <rte_ip.h>
#include <rte_sctp.h>
@@ -1421,16 +1422,27 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
*
* Matches a GTP PDU extension header with type 0x85.
*/
+RTE_STD_C11
struct rte_flow_item_gtp_psc {
- uint8_t pdu_type; /**< PDU type. */
- uint8_t qfi; /**< QoS flow identifier. */
+ union {
+ struct {
+ /*
+ * These fields are retained for compatibility.
+ * Please switch to the new header field below.
+ */
+ uint8_t pdu_type; /**< PDU type. */
+ uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
+
+ };
+ struct rte_gtp_psc gtp_psc;
+ };
};
/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
#ifndef __cplusplus
static const struct rte_flow_item_gtp_psc
rte_flow_item_gtp_psc_mask = {
- .qfi = 0x3f,
+ .qfi = 0xff,
};
#endif
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-03-30 8:00 ` Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
1 sibling, 2 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 8:00 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, orika, andrew.rybchenko, ivan.malov, ying.a.wang,
olivier.matz, viacheslavo, shirik
This is fixing the gtp psc support to match the RFC 38415-g30
v2: introduce new header definition for gtp psc
update commit msg for rte flow item change.
v3: fixed typo in comment
Cc relevant people.
Raslan Darawsheh (2):
ethdev: add new ext hdr for gtp psc
ethdev: update qfi definition
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +-
lib/librte_ethdev/rte_flow.h | 18 +++++++++--
lib/librte_net/rte_gtp.h | 34 +++++++++++++++++++++
3 files changed, 51 insertions(+), 4 deletions(-)
--
2.29.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support Raslan Darawsheh
@ 2021-03-30 8:00 ` Raslan Darawsheh
2021-04-01 16:51 ` Ferruh Yigit
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition Raslan Darawsheh
1 sibling, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 8:00 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, orika, andrew.rybchenko, ivan.malov, ying.a.wang,
olivier.matz, viacheslavo, shirik
Define new rte header for gtp PDU session container
based on RFC 38415-g30
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
index 6a6f9b238d..bfaae26535 100644
--- a/lib/librte_net/rte_gtp.h
+++ b/lib/librte_net/rte_gtp.h
@@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
uint8_t next_ext; /**< Next Extension Header Type. */
} __rte_packed;
+/**
+ * Optional extension for GTP with next_ext set to 0x85
+ * defined based on RFC 38415-g30.
+ */
+__extension__
+struct rte_gtp_psc {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ union {
+ struct {
+ uint8_t snp:1; /**< Sequence number presence */
+ uint8_t spare_dl1:2; /**< spare down link bits */
+ };
+ struct {
+ uint8_t dl_delay_ind:1; /**< dl delay result presence */
+ uint8_t ul_delay_ind:1; /**< ul delay result presence */
+ uint8_t snp_ul1:1; /**< Sequence number presence ul */
+ };
+ };
+ union {
+ struct {
+ uint8_t ppp:1; /**< Paging policy presence */
+ uint8_t rqi:1; /**< Reflective Qos Indicator */
+ };
+ struct {
+ uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
+ uint8_t spare_ul2:1; /**< spare up link bits */
+ };
+ };
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t data[0]; /**< data feilds */
+} __rte_packed;
+
/** GTP header length */
#define RTE_ETHER_GTP_HLEN \
(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-03-30 8:00 ` Raslan Darawsheh
2021-04-01 16:54 ` Ferruh Yigit
1 sibling, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-03-30 8:00 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, orika, andrew.rybchenko, ivan.malov, ying.a.wang,
olivier.matz, viacheslavo, shirik, stable
qfi field is 8 bits which represent single bit for
PPP (paging Policy Presence) single bit for RQI
(Reflective QoS Indicator) and 6 bits for qfi
(QoS Flow Identifier) based on RFC 38415-g30
This update the doxygen format and the mask for qfi
to properly identify the full 8 bits of the field.
note: changing the default mask would cause different
patterns generated by testpmd.
Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
Cc: ying.a.wang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
lib/librte_ethdev/rte_flow.h | 18 +++++++++++++++---
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a27d..dd39c4c3c2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
- ``gtp_psc``: match GTP PDU extension header with type 0x85.
- ``pdu_type {unsigned}``: PDU type.
- - ``qfi {unsigned}``: QoS flow identifier.
+
+ - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
- ``pppoes``, ``pppoed``: match PPPoE header.
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 6cc57136ac..1eb9711707 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -20,6 +20,7 @@
#include <rte_arp.h>
#include <rte_common.h>
#include <rte_ether.h>
+#include <rte_gtp.h>
#include <rte_icmp.h>
#include <rte_ip.h>
#include <rte_sctp.h>
@@ -1421,16 +1422,27 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
*
* Matches a GTP PDU extension header with type 0x85.
*/
+RTE_STD_C11
struct rte_flow_item_gtp_psc {
- uint8_t pdu_type; /**< PDU type. */
- uint8_t qfi; /**< QoS flow identifier. */
+ union {
+ struct {
+ /*
+ * These fields are retained for compatibility.
+ * Please switch to the new header field below.
+ */
+ uint8_t pdu_type; /**< PDU type. */
+ uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
+
+ };
+ struct rte_gtp_psc gtp_psc;
+ };
};
/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
#ifndef __cplusplus
static const struct rte_flow_item_gtp_psc
rte_flow_item_gtp_psc_mask = {
- .qfi = 0x3f,
+ .qfi = 0xff,
};
#endif
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-04-01 16:51 ` Ferruh Yigit
2021-04-04 7:17 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-01 16:51 UTC (permalink / raw)
To: Raslan Darawsheh, dev
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
On 3/30/2021 9:00 AM, Raslan Darawsheh wrote:
> Define new rte header for gtp PDU session container
> based on RFC 38415-g30
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> index 6a6f9b238d..bfaae26535 100644
> --- a/lib/librte_net/rte_gtp.h
> +++ b/lib/librte_net/rte_gtp.h
> @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> uint8_t next_ext; /**< Next Extension Header Type. */
> } __rte_packed;
>
> +/**
> + * Optional extension for GTP with next_ext set to 0x85
> + * defined based on RFC 38415-g30.
> + */
> +__extension__
> +struct rte_gtp_psc {
general consensus in other protocols seems having '_hdr' suffix in the struct
name, I can see this is extension but do you think does it make to add suffix?
> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> + uint8_t type:4; /**< PDU type */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + union {
> + struct {
> + uint8_t snp:1; /**< Sequence number presence */
> + uint8_t spare_dl1:2; /**< spare down link bits */
> + };
> + struct {
> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> + uint8_t snp_ul1:1; /**< Sequence number presence ul */
> + };
> + };
> + union {
> + struct {
> + uint8_t ppp:1; /**< Paging policy presence */
> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> + };
> + struct {
> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> + uint8_t spare_ul2:1; /**< spare up link bits */
> + };
> + };
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> + uint8_t data[0]; /**< data feilds */
> +} __rte_packed;
> +
> /** GTP header length */
> #define RTE_ETHER_GTP_HLEN \
> (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition Raslan Darawsheh
@ 2021-04-01 16:54 ` Ferruh Yigit
2021-04-04 7:18 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-01 16:54 UTC (permalink / raw)
To: Raslan Darawsheh, dev
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik, stable
On 3/30/2021 9:00 AM, Raslan Darawsheh wrote:
> qfi field is 8 bits which represent single bit for
> PPP (paging Policy Presence) single bit for RQI
> (Reflective QoS Indicator) and 6 bits for qfi
> (QoS Flow Identifier) based on RFC 38415-g30
>
> This update the doxygen format and the mask for qfi
> to properly identify the full 8 bits of the field.
>
> note: changing the default mask would cause different
> patterns generated by testpmd.
>
> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> Cc: ying.a.wang@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> lib/librte_ethdev/rte_flow.h | 18 +++++++++++++++---
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index f59eb8a27d..dd39c4c3c2 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
>
> - ``pdu_type {unsigned}``: PDU type.
> - - ``qfi {unsigned}``: QoS flow identifier.
> +
> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
>
> - ``pppoes``, ``pppoed``: match PPPoE header.
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 6cc57136ac..1eb9711707 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -20,6 +20,7 @@
> #include <rte_arp.h>
> #include <rte_common.h>
> #include <rte_ether.h>
> +#include <rte_gtp.h>
> #include <rte_icmp.h>
> #include <rte_ip.h>
> #include <rte_sctp.h>
> @@ -1421,16 +1422,27 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
> *
> * Matches a GTP PDU extension header with type 0x85.
> */
> +RTE_STD_C11
> struct rte_flow_item_gtp_psc {
> - uint8_t pdu_type; /**< PDU type. */
> - uint8_t qfi; /**< QoS flow identifier. */
> + union {
> + struct {
> + /*
> + * These fields are retained for compatibility.
> + * Please switch to the new header field below.
> + */
> + uint8_t pdu_type; /**< PDU type. */
> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> +
> + };
> + struct rte_gtp_psc gtp_psc;
Again for consistency, what do you think to rename the variable as 'hdr'?
> + };
> };
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> #ifndef __cplusplus
> static const struct rte_flow_item_gtp_psc
> rte_flow_item_gtp_psc_mask = {
> - .qfi = 0x3f,
> + .qfi = 0xff,
Since the protocol header is the preferred way, (individual fields may be
deprecated in the future), can you please switch to new field, like:
.gtp_psc.qfi = 0xff,
> };
> #endif
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc
2021-04-01 16:51 ` Ferruh Yigit
@ 2021-04-04 7:17 ` Raslan Darawsheh
0 siblings, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-04 7:17 UTC (permalink / raw)
To: Ferruh Yigit, dev@dpdk.org
Cc: Ori Kam, andrew.rybchenko@oktetlabs.ru, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, olivier.matz@6wind.com, Slava Ovsiienko,
Shiri Kuzin
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, April 1, 2021 7:51 PM
> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru;
> ivan.malov@oktetlabs.ru; ying.a.wang@intel.com; olivier.matz@6wind.com;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri Kuzin <shirik@nvidia.com>
> Subject: Re: [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc
>
> On 3/30/2021 9:00 AM, Raslan Darawsheh wrote:
> > Define new rte header for gtp PDU session container
> > based on RFC 38415-g30
> >
> > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > ---
> > lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> > index 6a6f9b238d..bfaae26535 100644
> > --- a/lib/librte_net/rte_gtp.h
> > +++ b/lib/librte_net/rte_gtp.h
> > @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> > uint8_t next_ext; /**< Next Extension Header Type. */
> > } __rte_packed;
> >
> > +/**
> > + * Optional extension for GTP with next_ext set to 0x85
> > + * defined based on RFC 38415-g30.
> > + */
> > +__extension__
> > +struct rte_gtp_psc {
>
> general consensus in other protocols seems having '_hdr' suffix in the struct
> name, I can see this is extension but do you think does it make to add suffix?
Yes sure, will send in a new version.
>
> > + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> > + uint8_t type:4; /**< PDU type */
> > + uint8_t qmp:1; /**< Qos Monitoring Packet */
> > + union {
> > + struct {
> > + uint8_t snp:1; /**< Sequence number presence */
> > + uint8_t spare_dl1:2; /**< spare down link bits */
> > + };
> > + struct {
> > + uint8_t dl_delay_ind:1; /**< dl delay result presence
> */
> > + uint8_t ul_delay_ind:1; /**< ul delay result presence
> */
> > + uint8_t snp_ul1:1; /**< Sequence number presence
> ul */
> > + };
> > + };
> > + union {
> > + struct {
> > + uint8_t ppp:1; /**< Paging policy presence */
> > + uint8_t rqi:1; /**< Reflective Qos Indicator */
> > + };
> > + struct {
> > + uint8_t n_delay_ind:1; /**< N3/N9 delay result
> presence */
> > + uint8_t spare_ul2:1; /**< spare up link bits */
> > + };
> > + };
> > + uint8_t qfi:6; /**< Qos Flow Identifier */
> > + uint8_t data[0]; /**< data feilds */
> > +} __rte_packed;
> > +
> > /** GTP header length */
> > #define RTE_ETHER_GTP_HLEN \
> > (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> >
Kindest regards
Raslan Darawsheh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition
2021-04-01 16:54 ` Ferruh Yigit
@ 2021-04-04 7:18 ` Raslan Darawsheh
0 siblings, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-04 7:18 UTC (permalink / raw)
To: Ferruh Yigit, dev@dpdk.org
Cc: Ori Kam, andrew.rybchenko@oktetlabs.ru, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, olivier.matz@6wind.com, Slava Ovsiienko,
Shiri Kuzin, stable@dpdk.org
Hi Ferruh,
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, April 1, 2021 7:54 PM
> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru;
> ivan.malov@oktetlabs.ru; ying.a.wang@intel.com; olivier.matz@6wind.com;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri Kuzin <shirik@nvidia.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v3 2/2] ethdev: update qfi definition
>
> On 3/30/2021 9:00 AM, Raslan Darawsheh wrote:
> > qfi field is 8 bits which represent single bit for
> > PPP (paging Policy Presence) single bit for RQI
> > (Reflective QoS Indicator) and 6 bits for qfi
> > (QoS Flow Identifier) based on RFC 38415-g30
> >
> > This update the doxygen format and the mask for qfi
> > to properly identify the full 8 bits of the field.
> >
> > note: changing the default mask would cause different
> > patterns generated by testpmd.
> >
> > Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> > Cc: ying.a.wang@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > ---
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> > lib/librte_ethdev/rte_flow.h | 18 +++++++++++++++---
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index f59eb8a27d..dd39c4c3c2 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
> their attributes, if any.
> > - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> >
> > - ``pdu_type {unsigned}``: PDU type.
> > - - ``qfi {unsigned}``: QoS flow identifier.
> > +
> > + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> >
> > - ``pppoes``, ``pppoed``: match PPPoE header.
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 6cc57136ac..1eb9711707 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -20,6 +20,7 @@
> > #include <rte_arp.h>
> > #include <rte_common.h>
> > #include <rte_ether.h>
> > +#include <rte_gtp.h>
> > #include <rte_icmp.h>
> > #include <rte_ip.h>
> > #include <rte_sctp.h>
> > @@ -1421,16 +1422,27 @@ static const struct rte_flow_item_meta
> rte_flow_item_meta_mask = {
> > *
> > * Matches a GTP PDU extension header with type 0x85.
> > */
> > +RTE_STD_C11
> > struct rte_flow_item_gtp_psc {
> > - uint8_t pdu_type; /**< PDU type. */
> > - uint8_t qfi; /**< QoS flow identifier. */
> > + union {
> > + struct {
> > + /*
> > + * These fields are retained for compatibility.
> > + * Please switch to the new header field below.
> > + */
> > + uint8_t pdu_type; /**< PDU type. */
> > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> > +
> > + };
> > + struct rte_gtp_psc gtp_psc;
>
> Again for consistency, what do you think to rename the variable as 'hdr'?
Will do,
>
> > + };
> > };
> >
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> > #ifndef __cplusplus
> > static const struct rte_flow_item_gtp_psc
> > rte_flow_item_gtp_psc_mask = {
> > - .qfi = 0x3f,
> > + .qfi = 0xff,
>
> Since the protocol header is the preferred way, (individual fields may be
> deprecated in the future), can you please switch to new field, like:
Yes, will do it accordingly
> .gtp_psc.qfi = 0xff,
>
> > };
> > #endif
> >
> >
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support Raslan Darawsheh
@ 2021-04-04 7:45 ` Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
` (2 more replies)
1 sibling, 3 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-04 7:45 UTC (permalink / raw)
To: dev, ferruh.yigit
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
This is fixing the gtp psc support to match the RFC 38415-g30
v2: introduce new header definition for gtp psc
update commit msg for rte flow item change.
v3: fixed typo in comment
Cc relevant people.
v4: update hdr definition to have hdr suffix.
update variable name to be hdr in the gtp_psc item.
update default max to use the new added hdr.
Raslan Darawsheh (2):
ethdev: add new ext hdr for gtp psc
ethdev: update qfi definition
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +-
lib/librte_ethdev/rte_flow.h | 20 ++++++++++--
lib/librte_net/rte_gtp.h | 34 +++++++++++++++++++++
3 files changed, 53 insertions(+), 4 deletions(-)
--
2.29.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
@ 2021-04-04 7:45 ` Raslan Darawsheh
2021-04-08 12:29 ` Olivier Matz
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 0/1] add new hdr for gtp qfi Raslan Darawsheh
2 siblings, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-04 7:45 UTC (permalink / raw)
To: dev, ferruh.yigit
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
Define new rte header for gtp PDU session container
based on RFC 38415-g30
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
index 6a6f9b238d..088b0b5a53 100644
--- a/lib/librte_net/rte_gtp.h
+++ b/lib/librte_net/rte_gtp.h
@@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
uint8_t next_ext; /**< Next Extension Header Type. */
} __rte_packed;
+/**
+ * Optional extension for GTP with next_ext set to 0x85
+ * defined based on RFC 38415-g30.
+ */
+__extension__
+struct rte_gtp_psc_hdr {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ union {
+ struct {
+ uint8_t snp:1; /**< Sequence number presence */
+ uint8_t spare_dl1:2; /**< spare down link bits */
+ };
+ struct {
+ uint8_t dl_delay_ind:1; /**< dl delay result presence */
+ uint8_t ul_delay_ind:1; /**< ul delay result presence */
+ uint8_t snp_ul1:1; /**< Sequence number presence ul */
+ };
+ };
+ union {
+ struct {
+ uint8_t ppp:1; /**< Paging policy presence */
+ uint8_t rqi:1; /**< Reflective Qos Indicator */
+ };
+ struct {
+ uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
+ uint8_t spare_ul2:1; /**< spare up link bits */
+ };
+ };
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t data[0]; /**< data feilds */
+} __rte_packed;
+
/** GTP header length */
#define RTE_ETHER_GTP_HLEN \
(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-04-04 7:45 ` Raslan Darawsheh
2021-04-06 16:09 ` Ferruh Yigit
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 0/1] add new hdr for gtp qfi Raslan Darawsheh
2 siblings, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-04 7:45 UTC (permalink / raw)
To: dev, ferruh.yigit
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik, stable
qfi field is 8 bits which represent single bit for
PPP (paging Policy Presence) single bit for RQI
(Reflective QoS Indicator) and 6 bits for qfi
(QoS Flow Identifier) based on RFC 38415-g30
This update the doxygen format and the mask for qfi
to properly identify the full 8 bits of the field.
note: changing the default mask would cause different
patterns generated by testpmd.
Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
Cc: ying.a.wang@intel.com
Cc: stable@dpdk.org
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f59eb8a27d..dd39c4c3c2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
- ``gtp_psc``: match GTP PDU extension header with type 0x85.
- ``pdu_type {unsigned}``: PDU type.
- - ``qfi {unsigned}``: QoS flow identifier.
+
+ - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
- ``pppoes``, ``pppoed``: match PPPoE header.
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 6cc57136ac..20b4389429 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -20,6 +20,7 @@
#include <rte_arp.h>
#include <rte_common.h>
#include <rte_ether.h>
+#include <rte_gtp.h>
#include <rte_icmp.h>
#include <rte_ip.h>
#include <rte_sctp.h>
@@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
*
* Matches a GTP PDU extension header with type 0x85.
*/
+RTE_STD_C11
struct rte_flow_item_gtp_psc {
- uint8_t pdu_type; /**< PDU type. */
- uint8_t qfi; /**< QoS flow identifier. */
+ union {
+ struct {
+ /*
+ * These fields are retained for compatibility.
+ * Please switch to the new header field below.
+ */
+ uint8_t pdu_type; /**< PDU type. */
+ uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
+
+ };
+ struct rte_gtp_psc_hdr hdr;
+ };
};
/** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
#ifndef __cplusplus
static const struct rte_flow_item_gtp_psc
rte_flow_item_gtp_psc_mask = {
- .qfi = 0x3f,
+ .hdr.ppp = 1,
+ .hdr.rqi = 1,
+ .hdr.qfi = 0x3f,
};
#endif
--
2.29.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition Raslan Darawsheh
@ 2021-04-06 16:09 ` Ferruh Yigit
2021-04-13 8:14 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-06 16:09 UTC (permalink / raw)
To: Raslan Darawsheh, dev, orika, andrew.rybchenko
Cc: ivan.malov, ying.a.wang, olivier.matz, viacheslavo, shirik,
stable, David Marchand, Thomas Monjalon
On 4/4/2021 8:45 AM, Raslan Darawsheh wrote:
> qfi field is 8 bits which represent single bit for
> PPP (paging Policy Presence) single bit for RQI
> (Reflective QoS Indicator) and 6 bits for qfi
> (QoS Flow Identifier) based on RFC 38415-g30
>
> This update the doxygen format and the mask for qfi
> to properly identify the full 8 bits of the field.
>
> note: changing the default mask would cause different
> patterns generated by testpmd.
>
> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> Cc: ying.a.wang@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index f59eb8a27d..dd39c4c3c2 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and their attributes, if any.
> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
>
> - ``pdu_type {unsigned}``: PDU type.
> - - ``qfi {unsigned}``: QoS flow identifier.
> +
> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
>
> - ``pppoes``, ``pppoed``: match PPPoE header.
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 6cc57136ac..20b4389429 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -20,6 +20,7 @@
> #include <rte_arp.h>
> #include <rte_common.h>
> #include <rte_ether.h>
> +#include <rte_gtp.h>
> #include <rte_icmp.h>
> #include <rte_ip.h>
> #include <rte_sctp.h>
> @@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta rte_flow_item_meta_mask = {
> *
> * Matches a GTP PDU extension header with type 0x85.
> */
> +RTE_STD_C11
> struct rte_flow_item_gtp_psc {
> - uint8_t pdu_type; /**< PDU type. */
> - uint8_t qfi; /**< QoS flow identifier. */
> + union {
> + struct {
> + /*
> + * These fields are retained for compatibility.
> + * Please switch to the new header field below.
> + */
> + uint8_t pdu_type; /**< PDU type. */
> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> +
> + };
> + struct rte_gtp_psc_hdr hdr;
> + };
> };
This will change the struct size even with union, since old version is missing
all fields from protocol header. Struct size will go from 2 --> 5 bytes [1].
Since this is public struct, we can't change its size.
@Ori, Andrew,
Do you have a suggestion for next step?
- We can still add the "struct rte_gtp_psc_hdr", and add a deprecation notice
for "struct rte_flow_item_gtp_psc" to say it will use "struct rte_gtp_psc_hdr"
on 21.11.
- And for this release use the Raslan's first version:
- uint8_t qfi; /**< QoS flow identifier. */
+ uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
Does it make sense? Any comments?
[1]
struct rte_gtp_psc_hdr {
uint8_t ext_hdr_len; /* 0 1 */
uint8_t type:4; /* 1: 0 1 */
uint8_t qmp:1; /* 1: 4 1 */
/* XXX 3 bits hole, try to pack */
union {
struct {
uint8_t snp:1; /* 2: 0 1 */
uint8_t spare_dl1:2; /* 2: 1 1 */
}; /* 2 1 */
struct {
uint8_t dl_delay_ind:1; /* 2: 0 1 */
uint8_t ul_delay_ind:1; /* 2: 1 1 */
uint8_t snp_ul1:1; /* 2: 2 1 */
}; /* 2 1 */
}; /* 2 1 */
union {
struct {
uint8_t ppp:1; /* 3: 0 1 */
uint8_t rqi:1; /* 3: 1 1 */
}; /* 3 1 */
struct {
uint8_t n_delay_ind:1; /* 3: 0 1 */
uint8_t spare_ul2:1; /* 3: 1 1 */
}; /* 3 1 */
}; /* 3 1 */
uint8_t qfi:6; /* 4: 0 1 */
/* XXX 2 bits hole, try to pack */
uint8_t data[]; /* 5 0 */
/* size: 5, cachelines: 1, members: 7 */
/* sum members: 3 */
/* sum bitfield members: 11 bits, bit holes: 2, sum bit holes: 5 bits */
/* last cacheline: 5 bytes */
};
>
> /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> #ifndef __cplusplus
> static const struct rte_flow_item_gtp_psc
> rte_flow_item_gtp_psc_mask = {
> - .qfi = 0x3f,
> + .hdr.ppp = 1,
> + .hdr.rqi = 1,
> + .hdr.qfi = 0x3f,
> };
> #endif
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-04-08 12:29 ` Olivier Matz
2021-04-08 12:37 ` Raslan Darawsheh
2021-04-29 16:29 ` Tyler Retzlaff
0 siblings, 2 replies; 37+ messages in thread
From: Olivier Matz @ 2021-04-08 12:29 UTC (permalink / raw)
To: Raslan Darawsheh
Cc: dev, ferruh.yigit, orika, andrew.rybchenko, ivan.malov,
ying.a.wang, viacheslavo, shirik
Hi Raslan,
On Sun, Apr 04, 2021 at 10:45:51AM +0300, Raslan Darawsheh wrote:
> Define new rte header for gtp PDU session container
> based on RFC 38415-g30
Do you have a link to this RFC?
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> index 6a6f9b238d..088b0b5a53 100644
> --- a/lib/librte_net/rte_gtp.h
> +++ b/lib/librte_net/rte_gtp.h
> @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> uint8_t next_ext; /**< Next Extension Header Type. */
> } __rte_packed;
>
> +/**
> + * Optional extension for GTP with next_ext set to 0x85
> + * defined based on RFC 38415-g30.
> + */
> +__extension__
> +struct rte_gtp_psc_hdr {
> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> + uint8_t type:4; /**< PDU type */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + union {
> + struct {
> + uint8_t snp:1; /**< Sequence number presence */
> + uint8_t spare_dl1:2; /**< spare down link bits */
> + };
> + struct {
> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> + uint8_t snp_ul1:1; /**< Sequence number presence ul */
> + };
> + };
> + union {
> + struct {
> + uint8_t ppp:1; /**< Paging policy presence */
> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> + };
> + struct {
> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> + uint8_t spare_ul2:1; /**< spare up link bits */
> + };
> + };
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> + uint8_t data[0]; /**< data feilds */
> +} __rte_packed;
With this header, sizeof(rte_gtp_psc_hdr) = 5, is it really expected?
It would help to see the specification to have a better idea of how to
split, but a possible solution is to do something like this:
struct rte_gtp_psc_generic_hdr {
uint8_t ext_hdr_len;
uint8_t type:4
uint8_t qmp:1;
uint8_t pad:3;
};
struct rte_gtp_psc_<name1>_hdr {
uint8_t ext_hdr_len;
uint8_t type:4
uint8_t qmp:1;
uint8_t uint8_t snp:1;
uint8_t spare_dl1:2;
...
};
...
struct rte_gtp_psc_hdr {
union {
struct rte_gtp_psc_generic_hdr generic;
struct rte_gtp_psc_<name1>_hdr <name1>;
struct rte_gtp_psc_<name2>_hdr <name2>;
};
};
Also, you need to take care about endianness.
Regards,
Olivier
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-08 12:29 ` Olivier Matz
@ 2021-04-08 12:37 ` Raslan Darawsheh
2021-04-08 14:10 ` Ferruh Yigit
2021-04-08 14:10 ` Olivier Matz
2021-04-29 16:29 ` Tyler Retzlaff
1 sibling, 2 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-08 12:37 UTC (permalink / raw)
To: Olivier Matz
Cc: dev@dpdk.org, ferruh.yigit@intel.com, Ori Kam,
andrew.rybchenko@oktetlabs.ru, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, Slava Ovsiienko, Shiri Kuzin
Hi Olivier,
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, April 8, 2021 3:30 PM
> To: Raslan Darawsheh <rasland@nvidia.com>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
> andrew.rybchenko@oktetlabs.ru; ivan.malov@oktetlabs.ru;
> ying.a.wang@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> Kuzin <shirik@nvidia.com>
> Subject: Re: [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
>
> Hi Raslan,
>
> On Sun, Apr 04, 2021 at 10:45:51AM +0300, Raslan Darawsheh wrote:
> > Define new rte header for gtp PDU session container
> > based on RFC 38415-g30
>
> Do you have a link to this RFC?
Yes sure,
https://www.3gpp.org/ftp/Specs/archive/38_series/38.415/38415-g30.zip
>
> > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > ---
> > lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> > index 6a6f9b238d..088b0b5a53 100644
> > --- a/lib/librte_net/rte_gtp.h
> > +++ b/lib/librte_net/rte_gtp.h
> > @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> > uint8_t next_ext; /**< Next Extension Header Type. */
> > } __rte_packed;
> >
> > +/**
> > + * Optional extension for GTP with next_ext set to 0x85
> > + * defined based on RFC 38415-g30.
> > + */
> > +__extension__
> > +struct rte_gtp_psc_hdr {
> > + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> > + uint8_t type:4; /**< PDU type */
> > + uint8_t qmp:1; /**< Qos Monitoring Packet */
> > + union {
> > + struct {
> > + uint8_t snp:1; /**< Sequence number presence */
> > + uint8_t spare_dl1:2; /**< spare down link bits */
> > + };
> > + struct {
> > + uint8_t dl_delay_ind:1; /**< dl delay result presence
> */
> > + uint8_t ul_delay_ind:1; /**< ul delay result presence
> */
> > + uint8_t snp_ul1:1; /**< Sequence number presence
> ul */
> > + };
> > + };
> > + union {
> > + struct {
> > + uint8_t ppp:1; /**< Paging policy presence */
> > + uint8_t rqi:1; /**< Reflective Qos Indicator */
> > + };
> > + struct {
> > + uint8_t n_delay_ind:1; /**< N3/N9 delay result
> presence */
> > + uint8_t spare_ul2:1; /**< spare up link bits */
> > + };
> > + };
> > + uint8_t qfi:6; /**< Qos Flow Identifier */
> > + uint8_t data[0]; /**< data feilds */
> > +} __rte_packed;
>
> With this header, sizeof(rte_gtp_psc_hdr) = 5, is it really expected?
The data[0] is variable length data, I guess I should send another version to mention that in the comment maybe.
The header size according to the spec should be 4 octets aligned in general.
>
> It would help to see the specification to have a better idea of how to
Sure, I've just posted the link above, please let me know of any suggestion that you have, and I'll be glad to do accordingly.
> split, but a possible solution is to do something like this:
>
> struct rte_gtp_psc_generic_hdr {
> uint8_t ext_hdr_len;
> uint8_t type:4
> uint8_t qmp:1;
> uint8_t pad:3;
> };
>
> struct rte_gtp_psc_<name1>_hdr {
> uint8_t ext_hdr_len;
> uint8_t type:4
> uint8_t qmp:1;
> uint8_t uint8_t snp:1;
> uint8_t spare_dl1:2;
> ...
> };
>
> ...
>
> struct rte_gtp_psc_hdr {
> union {
> struct rte_gtp_psc_generic_hdr generic;
> struct rte_gtp_psc_<name1>_hdr <name1>;
> struct rte_gtp_psc_<name2>_hdr <name2>;
> };
> };
>
> Also, you need to take care about endianness.
>
>
> Regards,
> Olivier
Kindest regards
Raslan Darawsheh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-08 12:37 ` Raslan Darawsheh
@ 2021-04-08 14:10 ` Ferruh Yigit
2021-04-08 14:10 ` Olivier Matz
1 sibling, 0 replies; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-08 14:10 UTC (permalink / raw)
To: Raslan Darawsheh, Olivier Matz
Cc: dev@dpdk.org, Ori Kam, andrew.rybchenko@oktetlabs.ru,
ivan.malov@oktetlabs.ru, ying.a.wang@intel.com, Slava Ovsiienko,
Shiri Kuzin
On 4/8/2021 1:37 PM, Raslan Darawsheh wrote:
> Hi Olivier,
>
>> -----Original Message-----
>> From: Olivier Matz <olivier.matz@6wind.com>
>> Sent: Thursday, April 8, 2021 3:30 PM
>> To: Raslan Darawsheh <rasland@nvidia.com>
>> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
>> andrew.rybchenko@oktetlabs.ru; ivan.malov@oktetlabs.ru;
>> ying.a.wang@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
>> Kuzin <shirik@nvidia.com>
>> Subject: Re: [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
>>
>> Hi Raslan,
>>
>> On Sun, Apr 04, 2021 at 10:45:51AM +0300, Raslan Darawsheh wrote:
>>> Define new rte header for gtp PDU session container
>>> based on RFC 38415-g30
>>
>> Do you have a link to this RFC?
> Yes sure,
> https://www.3gpp.org/ftp/Specs/archive/38_series/38.415/38415-g30.zip
>
>>
>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
>>> ---
>>> lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
>>> index 6a6f9b238d..088b0b5a53 100644
>>> --- a/lib/librte_net/rte_gtp.h
>>> +++ b/lib/librte_net/rte_gtp.h
>>> @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
>>> uint8_t next_ext; /**< Next Extension Header Type. */
>>> } __rte_packed;
>>>
>>> +/**
>>> + * Optional extension for GTP with next_ext set to 0x85
>>> + * defined based on RFC 38415-g30.
>>> + */
>>> +__extension__
>>> +struct rte_gtp_psc_hdr {
>>> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
>>> + uint8_t type:4; /**< PDU type */
>>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>>> + union {
>>> + struct {
>>> + uint8_t snp:1; /**< Sequence number presence */
>>> + uint8_t spare_dl1:2; /**< spare down link bits */
>>> + };
>>> + struct {
>>> + uint8_t dl_delay_ind:1; /**< dl delay result presence
>> */
>>> + uint8_t ul_delay_ind:1; /**< ul delay result presence
>> */
>>> + uint8_t snp_ul1:1; /**< Sequence number presence
>> ul */
>>> + };
>>> + };
>>> + union {
>>> + struct {
>>> + uint8_t ppp:1; /**< Paging policy presence */
>>> + uint8_t rqi:1; /**< Reflective Qos Indicator */
>>> + };
>>> + struct {
>>> + uint8_t n_delay_ind:1; /**< N3/N9 delay result
>> presence */
>>> + uint8_t spare_ul2:1; /**< spare up link bits */
>>> + };
>>> + };
>>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>>> + uint8_t data[0]; /**< data feilds */
>>> +} __rte_packed;
>>
>> With this header, sizeof(rte_gtp_psc_hdr) = 5, is it really expected?
> The data[0] is variable length data, I guess I should send another version to mention that in the comment maybe.
> The header size according to the spec should be 4 octets aligned in general.
The struct is 5 btyes, this is not related to data[0], please check the pahole
output:
http://inbox.dpdk.org/dev/536631b9-c634-ddac-c154-91978ffc29a5@intel.com/
>>
>> It would help to see the specification to have a better idea of how to
> Sure, I've just posted the link above, please let me know of any suggestion that you have, and I'll be glad to do accordingly.
>
>> split, but a possible solution is to do something like this:
>>
>> struct rte_gtp_psc_generic_hdr {
>> uint8_t ext_hdr_len;
>> uint8_t type:4
>> uint8_t qmp:1;
>> uint8_t pad:3;
>> };
>>
>> struct rte_gtp_psc_<name1>_hdr {
>> uint8_t ext_hdr_len;
>> uint8_t type:4
>> uint8_t qmp:1;
>> uint8_t uint8_t snp:1;
>> uint8_t spare_dl1:2;
>> ...
>> };
>>
>> ...
>>
>> struct rte_gtp_psc_hdr {
>> union {
>> struct rte_gtp_psc_generic_hdr generic;
>> struct rte_gtp_psc_<name1>_hdr <name1>;
>> struct rte_gtp_psc_<name2>_hdr <name2>;
>> };
>> };
>>
>> Also, you need to take care about endianness.
>>
>>
>> Regards,
>> Olivier
> Kindest regards
> Raslan Darawsheh
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-08 12:37 ` Raslan Darawsheh
2021-04-08 14:10 ` Ferruh Yigit
@ 2021-04-08 14:10 ` Olivier Matz
2021-04-13 7:45 ` Raslan Darawsheh
1 sibling, 1 reply; 37+ messages in thread
From: Olivier Matz @ 2021-04-08 14:10 UTC (permalink / raw)
To: Raslan Darawsheh
Cc: dev@dpdk.org, ferruh.yigit@intel.com, Ori Kam,
andrew.rybchenko@oktetlabs.ru, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, Slava Ovsiienko, Shiri Kuzin
On Thu, Apr 08, 2021 at 12:37:27PM +0000, Raslan Darawsheh wrote:
> Hi Olivier,
>
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, April 8, 2021 3:30 PM
> > To: Raslan Darawsheh <rasland@nvidia.com>
> > Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
> > andrew.rybchenko@oktetlabs.ru; ivan.malov@oktetlabs.ru;
> > ying.a.wang@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> > Kuzin <shirik@nvidia.com>
> > Subject: Re: [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
> >
> > Hi Raslan,
> >
> > On Sun, Apr 04, 2021 at 10:45:51AM +0300, Raslan Darawsheh wrote:
> > > Define new rte header for gtp PDU session container
> > > based on RFC 38415-g30
> >
> > Do you have a link to this RFC?
> Yes sure,
> https://www.3gpp.org/ftp/Specs/archive/38_series/38.415/38415-g30.zip
>
> >
> > > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > > ---
> > > lib/librte_net/rte_gtp.h | 34 ++++++++++++++++++++++++++++++++++
> > > 1 file changed, 34 insertions(+)
> > >
> > > diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> > > index 6a6f9b238d..088b0b5a53 100644
> > > --- a/lib/librte_net/rte_gtp.h
> > > +++ b/lib/librte_net/rte_gtp.h
> > > @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> > > uint8_t next_ext; /**< Next Extension Header Type. */
> > > } __rte_packed;
> > >
> > > +/**
> > > + * Optional extension for GTP with next_ext set to 0x85
> > > + * defined based on RFC 38415-g30.
> > > + */
> > > +__extension__
> > > +struct rte_gtp_psc_hdr {
> > > + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> > > + uint8_t type:4; /**< PDU type */
> > > + uint8_t qmp:1; /**< Qos Monitoring Packet */
> > > + union {
> > > + struct {
> > > + uint8_t snp:1; /**< Sequence number presence */
> > > + uint8_t spare_dl1:2; /**< spare down link bits */
> > > + };
> > > + struct {
> > > + uint8_t dl_delay_ind:1; /**< dl delay result presence
> > */
> > > + uint8_t ul_delay_ind:1; /**< ul delay result presence
> > */
> > > + uint8_t snp_ul1:1; /**< Sequence number presence
> > ul */
> > > + };
> > > + };
> > > + union {
> > > + struct {
> > > + uint8_t ppp:1; /**< Paging policy presence */
> > > + uint8_t rqi:1; /**< Reflective Qos Indicator */
> > > + };
> > > + struct {
> > > + uint8_t n_delay_ind:1; /**< N3/N9 delay result
> > presence */
> > > + uint8_t spare_ul2:1; /**< spare up link bits */
> > > + };
> > > + };
> > > + uint8_t qfi:6; /**< Qos Flow Identifier */
> > > + uint8_t data[0]; /**< data feilds */
> > > +} __rte_packed;
> >
> > With this header, sizeof(rte_gtp_psc_hdr) = 5, is it really expected?
> The data[0] is variable length data, I guess I should send another version to mention that in the comment maybe.
> The header size according to the spec should be 4 octets aligned in general.
What I wanted to highlight is that using union of structs containing
bitfields does not work as you expect: each union is at least 1 byte.
This results in a structure that does not match the expected header.
> >
> > It would help to see the specification to have a better idea of how to
> Sure, I've just posted the link above, please let me know of any suggestion that you have, and I'll be glad to do accordingly.
>
> > split, but a possible solution is to do something like this:
> >
> > struct rte_gtp_psc_generic_hdr {
> > uint8_t ext_hdr_len;
> > uint8_t type:4
> > uint8_t qmp:1;
> > uint8_t pad:3;
> > };
> >
> > struct rte_gtp_psc_<name1>_hdr {
> > uint8_t ext_hdr_len;
> > uint8_t type:4
> > uint8_t qmp:1;
> > uint8_t uint8_t snp:1;
> > uint8_t spare_dl1:2;
> > ...
> > };
> >
> > ...
> >
> > struct rte_gtp_psc_hdr {
> > union {
> > struct rte_gtp_psc_generic_hdr generic;
> > struct rte_gtp_psc_<name1>_hdr <name1>;
> > struct rte_gtp_psc_<name2>_hdr <name2>;
> > };
> > };
From what I see in the documation, I think this approach should
work. From afar, I suggest:
struct rte_gtp_psc_generic_hdr {
#if big endian
uint8_t type:4
uint8_t qmp:1;
uint8_t pad:3;
#else
uint8_t pad:3;
uint8_t qmp:1;
uint8_t type:4
#endif
};
struct rte_gtp_psc_type0_hdr {
#if big endian
uint8_t type:4
uint8_t qmp:1;
uint8_t snp:1;
uint8_t spare:2;
uint8_t ppp:1;
...
#else
uint8_t pad:3;
uint8_t qmp:1;
uint8_t type:4
uint8_t spare:2;
uint8_t snp:1;
...
#endif
uint8_t data[0]; /* for variable fields */
};
struct rte_gtp_psc_type1_hdr {
... same for fixed fields of type1
uint8_t data[0]; /* for variable fields */
};
I don't see in the spec where is the reference to ext_hdr_len.
Regards,
Olivier
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-08 14:10 ` Olivier Matz
@ 2021-04-13 7:45 ` Raslan Darawsheh
0 siblings, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-13 7:45 UTC (permalink / raw)
To: Olivier Matz
Cc: dev@dpdk.org, ferruh.yigit@intel.com, Ori Kam,
andrew.rybchenko@oktetlabs.ru, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, Slava Ovsiienko, Shiri Kuzin
Hi and sorry for late response,
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, April 8, 2021 5:11 PM
> To: Raslan Darawsheh <rasland@nvidia.com>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
> andrew.rybchenko@oktetlabs.ru; ivan.malov@oktetlabs.ru;
> ying.a.wang@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> Kuzin <shirik@nvidia.com>
> Subject: Re: [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
>
> On Thu, Apr 08, 2021 at 12:37:27PM +0000, Raslan Darawsheh wrote:
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Thursday, April 8, 2021 3:30 PM
> > > To: Raslan Darawsheh <rasland@nvidia.com>
> > > Cc: dev@dpdk.org; ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
> > > andrew.rybchenko@oktetlabs.ru; ivan.malov@oktetlabs.ru;
> > > ying.a.wang@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> > > Kuzin <shirik@nvidia.com>
> > > Subject: Re: [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
> > >
> > > Hi Raslan,
> > >
> > > On Sun, Apr 04, 2021 at 10:45:51AM +0300, Raslan Darawsheh wrote:
> > > > Define new rte header for gtp PDU session container
> > > > based on RFC 38415-g30
> > >
> > > Do you have a link to this RFC?
> > Yes sure,
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w.3gpp.org%2Fftp%2FSpecs%2Farchive%2F38_series%2F38.415%2F38415-
> g30.zip&data=04%7C01%7Crasland%40nvidia.com%7C5279dde172024bc
> d1f6b08d8fa981668%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6
> 37534878435191995%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=
> LuD7%2FvJgqJbZ78ncaM7UrpiLNPUt7zbPPfSMemyb2Y8%3D&reserved
> =0
> >
> > >
> > > > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > > > ---
> > > > lib/librte_net/rte_gtp.h | 34
> ++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 34 insertions(+)
> > > >
> > > > diff --git a/lib/librte_net/rte_gtp.h b/lib/librte_net/rte_gtp.h
> > > > index 6a6f9b238d..088b0b5a53 100644
> > > > --- a/lib/librte_net/rte_gtp.h
> > > > +++ b/lib/librte_net/rte_gtp.h
> > > > @@ -61,6 +61,40 @@ struct rte_gtp_hdr_ext_word {
> > > > uint8_t next_ext; /**< Next Extension Header Type. */
> > > > } __rte_packed;
> > > >
> > > > +/**
> > > > + * Optional extension for GTP with next_ext set to 0x85
> > > > + * defined based on RFC 38415-g30.
> > > > + */
> > > > +__extension__
> > > > +struct rte_gtp_psc_hdr {
> > > > + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> > > > + uint8_t type:4; /**< PDU type */
> > > > + uint8_t qmp:1; /**< Qos Monitoring Packet */
> > > > + union {
> > > > + struct {
> > > > + uint8_t snp:1; /**< Sequence number presence */
> > > > + uint8_t spare_dl1:2; /**< spare down link bits */
> > > > + };
> > > > + struct {
> > > > + uint8_t dl_delay_ind:1; /**< dl delay result presence
> > > */
> > > > + uint8_t ul_delay_ind:1; /**< ul delay result presence
> > > */
> > > > + uint8_t snp_ul1:1; /**< Sequence number presence
> > > ul */
> > > > + };
> > > > + };
> > > > + union {
> > > > + struct {
> > > > + uint8_t ppp:1; /**< Paging policy presence */
> > > > + uint8_t rqi:1; /**< Reflective Qos Indicator */
> > > > + };
> > > > + struct {
> > > > + uint8_t n_delay_ind:1; /**< N3/N9 delay result
> > > presence */
> > > > + uint8_t spare_ul2:1; /**< spare up link bits */
> > > > + };
> > > > + };
> > > > + uint8_t qfi:6; /**< Qos Flow Identifier */
> > > > + uint8_t data[0]; /**< data feilds */
> > > > +} __rte_packed;
> > >
> > > With this header, sizeof(rte_gtp_psc_hdr) = 5, is it really expected?
> > The data[0] is variable length data, I guess I should send another version to
> mention that in the comment maybe.
> > The header size according to the spec should be 4 octets aligned in general.
>
> What I wanted to highlight is that using union of structs containing
> bitfields does not work as you expect: each union is at least 1 byte.
> This results in a structure that does not match the expected header.
I see thanks for explaining.
>
> > >
> > > It would help to see the specification to have a better idea of how to
> > Sure, I've just posted the link above, please let me know of any suggestion
> that you have, and I'll be glad to do accordingly.
> >
> > > split, but a possible solution is to do something like this:
> > >
> > > struct rte_gtp_psc_generic_hdr {
> > > uint8_t ext_hdr_len;
> > > uint8_t type:4
> > > uint8_t qmp:1;
> > > uint8_t pad:3;
> > > };
> > >
> > > struct rte_gtp_psc_<name1>_hdr {
> > > uint8_t ext_hdr_len;
> > > uint8_t type:4
> > > uint8_t qmp:1;
> > > uint8_t uint8_t snp:1;
> > > uint8_t spare_dl1:2;
> > > ...
> > > };
> > >
> > > ...
> > >
> > > struct rte_gtp_psc_hdr {
> > > union {
> > > struct rte_gtp_psc_generic_hdr generic;
> > > struct rte_gtp_psc_<name1>_hdr <name1>;
> > > struct rte_gtp_psc_<name2>_hdr <name2>;
> > > };
> > > };
>
> From what I see in the documation, I think this approach should
> work. From afar, I suggest:
>
> struct rte_gtp_psc_generic_hdr {
> #if big endian
> uint8_t type:4
> uint8_t qmp:1;
> uint8_t pad:3;
> #else
> uint8_t pad:3;
> uint8_t qmp:1;
> uint8_t type:4
> #endif
> };
>
> struct rte_gtp_psc_type0_hdr {
> #if big endian
> uint8_t type:4
> uint8_t qmp:1;
> uint8_t snp:1;
> uint8_t spare:2;
>
> uint8_t ppp:1;
> ...
> #else
> uint8_t pad:3;
> uint8_t qmp:1;
> uint8_t type:4
> uint8_t spare:2;
> uint8_t snp:1;
>
> ...
> #endif
> uint8_t data[0]; /* for variable fields */
> };
>
> struct rte_gtp_psc_type1_hdr {
> ... same for fixed fields of type1
>
>
> uint8_t data[0]; /* for variable fields */
> };
>
Sure, will consider and do it this way in the next version.
> I don't see in the spec where is the reference to ext_hdr_len.
>
The ext_hdr_len is part of the definition for all ext_hdrs see this link for more details:
https://en.wikipedia.org/wiki/GPRS_Tunnelling_Protocol
So, it's not mentioned in the doc.
> Regards,
> Olivier
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-06 16:09 ` Ferruh Yigit
@ 2021-04-13 8:14 ` Raslan Darawsheh
2021-04-13 9:24 ` Ori Kam
0 siblings, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-13 8:14 UTC (permalink / raw)
To: Ferruh Yigit, dev@dpdk.org, Ori Kam,
andrew.rybchenko@oktetlabs.ru
Cc: ivan.malov@oktetlabs.ru, ying.a.wang@intel.com,
olivier.matz@6wind.com, Slava Ovsiienko, Shiri Kuzin,
stable@dpdk.org, David Marchand, NBU-Contact-Thomas Monjalon
Hi,
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, April 6, 2021 7:10 PM
> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org; Ori Kam
> <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru
> Cc: ivan.malov@oktetlabs.ru; ying.a.wang@intel.com;
> olivier.matz@6wind.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> Kuzin <shirik@nvidia.com>; stable@dpdk.org; David Marchand
> <david.marchand@redhat.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>
> Subject: Re: [PATCH v4 2/2] ethdev: update qfi definition
>
> On 4/4/2021 8:45 AM, Raslan Darawsheh wrote:
> > qfi field is 8 bits which represent single bit for
> > PPP (paging Policy Presence) single bit for RQI
> > (Reflective QoS Indicator) and 6 bits for qfi
> > (QoS Flow Identifier) based on RFC 38415-g30
> >
> > This update the doxygen format and the mask for qfi
> > to properly identify the full 8 bits of the field.
> >
> > note: changing the default mask would cause different
> > patterns generated by testpmd.
> >
> > Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> > Cc: ying.a.wang@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > ---
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> > lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
> > 2 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index f59eb8a27d..dd39c4c3c2 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
> their attributes, if any.
> > - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> >
> > - ``pdu_type {unsigned}``: PDU type.
> > - - ``qfi {unsigned}``: QoS flow identifier.
> > +
> > + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> >
> > - ``pppoes``, ``pppoed``: match PPPoE header.
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index 6cc57136ac..20b4389429 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -20,6 +20,7 @@
> > #include <rte_arp.h>
> > #include <rte_common.h>
> > #include <rte_ether.h>
> > +#include <rte_gtp.h>
> > #include <rte_icmp.h>
> > #include <rte_ip.h>
> > #include <rte_sctp.h>
> > @@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta
> rte_flow_item_meta_mask = {
> > *
> > * Matches a GTP PDU extension header with type 0x85.
> > */
> > +RTE_STD_C11
> > struct rte_flow_item_gtp_psc {
> > - uint8_t pdu_type; /**< PDU type. */
> > - uint8_t qfi; /**< QoS flow identifier. */
> > + union {
> > + struct {
> > + /*
> > + * These fields are retained for compatibility.
> > + * Please switch to the new header field below.
> > + */
> > + uint8_t pdu_type; /**< PDU type. */
> > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> > +
> > + };
> > + struct rte_gtp_psc_hdr hdr;
> > + };
> > };
>
> This will change the struct size even with union, since old version is missing
> all fields from protocol header. Struct size will go from 2 --> 5 bytes [1].
>
> Since this is public struct, we can't change its size.
>
> @Ori, Andrew,
>
> Do you have a suggestion for next step?
>
> - We can still add the "struct rte_gtp_psc_hdr", and add a deprecation notice
> for "struct rte_flow_item_gtp_psc" to say it will use "struct
> rte_gtp_psc_hdr"
> on 21.11.
>
> - And for this release use the Raslan's first version:
> - uint8_t qfi; /**< QoS flow identifier. */
> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
>
>
> Does it make sense? Any comments?
>
@Ori Kam, @andrew.rybchenko@oktetlabs.ru,
This is a kind remainder of this patch,
>
>
> [1]
> struct rte_gtp_psc_hdr {
> uint8_t ext_hdr_len; /* 0 1 */
> uint8_t type:4; /* 1: 0 1 */
> uint8_t qmp:1; /* 1: 4 1 */
>
> /* XXX 3 bits hole, try to pack */
>
> union {
> struct {
> uint8_t snp:1; /* 2: 0 1 */
> uint8_t spare_dl1:2; /* 2: 1 1 */
> }; /* 2 1 */
> struct {
> uint8_t dl_delay_ind:1; /* 2: 0 1 */
> uint8_t ul_delay_ind:1; /* 2: 1 1 */
> uint8_t snp_ul1:1; /* 2: 2 1 */
> }; /* 2 1 */
> }; /* 2 1 */
> union {
> struct {
> uint8_t ppp:1; /* 3: 0 1 */
> uint8_t rqi:1; /* 3: 1 1 */
> }; /* 3 1 */
> struct {
> uint8_t n_delay_ind:1; /* 3: 0 1 */
> uint8_t spare_ul2:1; /* 3: 1 1 */
> }; /* 3 1 */
> }; /* 3 1 */
> uint8_t qfi:6; /* 4: 0 1 */
>
> /* XXX 2 bits hole, try to pack */
>
> uint8_t data[]; /* 5 0 */
>
> /* size: 5, cachelines: 1, members: 7 */
> /* sum members: 3 */
> /* sum bitfield members: 11 bits, bit holes: 2, sum bit holes: 5 bits */
> /* last cacheline: 5 bytes */
> };
>
> >
> > /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> > #ifndef __cplusplus
> > static const struct rte_flow_item_gtp_psc
> > rte_flow_item_gtp_psc_mask = {
> > - .qfi = 0x3f,
> > + .hdr.ppp = 1,
> > + .hdr.rqi = 1,
> > + .hdr.qfi = 0x3f,
> > };
> > #endif
> >
> >
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-13 8:14 ` Raslan Darawsheh
@ 2021-04-13 9:24 ` Ori Kam
2021-04-14 12:16 ` Ferruh Yigit
0 siblings, 1 reply; 37+ messages in thread
From: Ori Kam @ 2021-04-13 9:24 UTC (permalink / raw)
To: Raslan Darawsheh, Ferruh Yigit, dev@dpdk.org,
andrew.rybchenko@oktetlabs.ru
Cc: ivan.malov@oktetlabs.ru, ying.a.wang@intel.com,
olivier.matz@6wind.com, Slava Ovsiienko, Shiri Kuzin,
stable@dpdk.org, David Marchand, NBU-Contact-Thomas Monjalon
Hi Raslan,
> -----Original Message-----
> From: Raslan Darawsheh <rasland@nvidia.com>
> Subject: RE: [PATCH v4 2/2] ethdev: update qfi definition
>
> Hi,
>
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Tuesday, April 6, 2021 7:10 PM
> > To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org; Ori Kam
> > <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru
> > Cc: ivan.malov@oktetlabs.ru; ying.a.wang@intel.com;
> > olivier.matz@6wind.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> > Kuzin <shirik@nvidia.com>; stable@dpdk.org; David Marchand
> > <david.marchand@redhat.com>; NBU-Contact-Thomas Monjalon
> > <thomas@monjalon.net>
> > Subject: Re: [PATCH v4 2/2] ethdev: update qfi definition
> >
> > On 4/4/2021 8:45 AM, Raslan Darawsheh wrote:
> > > qfi field is 8 bits which represent single bit for
> > > PPP (paging Policy Presence) single bit for RQI
> > > (Reflective QoS Indicator) and 6 bits for qfi
> > > (QoS Flow Identifier) based on RFC 38415-g30
> > >
> > > This update the doxygen format and the mask for qfi
> > > to properly identify the full 8 bits of the field.
> > >
> > > note: changing the default mask would cause different
> > > patterns generated by testpmd.
> > >
> > > Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> > > Cc: ying.a.wang@intel.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> > > ---
> > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> > > lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
> > > 2 files changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > index f59eb8a27d..dd39c4c3c2 100644
> > > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > > @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
> > their attributes, if any.
> > > - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> > >
> > > - ``pdu_type {unsigned}``: PDU type.
> > > - - ``qfi {unsigned}``: QoS flow identifier.
> > > +
> > > + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> > >
> > > - ``pppoes``, ``pppoed``: match PPPoE header.
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > > index 6cc57136ac..20b4389429 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -20,6 +20,7 @@
> > > #include <rte_arp.h>
> > > #include <rte_common.h>
> > > #include <rte_ether.h>
> > > +#include <rte_gtp.h>
> > > #include <rte_icmp.h>
> > > #include <rte_ip.h>
> > > #include <rte_sctp.h>
> > > @@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta
> > rte_flow_item_meta_mask = {
> > > *
> > > * Matches a GTP PDU extension header with type 0x85.
> > > */
> > > +RTE_STD_C11
> > > struct rte_flow_item_gtp_psc {
> > > - uint8_t pdu_type; /**< PDU type. */
> > > - uint8_t qfi; /**< QoS flow identifier. */
> > > + union {
> > > + struct {
> > > + /*
> > > + * These fields are retained for compatibility.
> > > + * Please switch to the new header field below.
> > > + */
> > > + uint8_t pdu_type; /**< PDU type. */
> > > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> > > +
> > > + };
> > > + struct rte_gtp_psc_hdr hdr;
> > > + };
> > > };
> >
> > This will change the struct size even with union, since old version is missing
> > all fields from protocol header. Struct size will go from 2 --> 5 bytes [1].
> >
> > Since this is public struct, we can't change its size.
> >
> > @Ori, Andrew,
> >
> > Do you have a suggestion for next step?
> >
I think Ferruh is right, and I think that we should at this point just update the documentation.
Sorry for the detour
Just small comment about the original patch.
I don’t think you should change the default mask since it may break existing apps.
> > - We can still add the "struct rte_gtp_psc_hdr", and add a deprecation notice
> > for "struct rte_flow_item_gtp_psc" to say it will use "struct
> > rte_gtp_psc_hdr"
> > on 21.11.
> >
> > - And for this release use the Raslan's first version:
> > - uint8_t qfi; /**< QoS flow identifier. */
> > + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> >
> >
> > Does it make sense? Any comments?
> >
> @Ori Kam, @andrew.rybchenko@oktetlabs.ru,
> This is a kind remainder of this patch,
>
> >
> >
> > [1]
> > struct rte_gtp_psc_hdr {
> > uint8_t ext_hdr_len; /* 0 1 */
> > uint8_t type:4; /* 1: 0 1 */
> > uint8_t qmp:1; /* 1: 4 1 */
> >
> > /* XXX 3 bits hole, try to pack */
> >
> > union {
> > struct {
> > uint8_t snp:1; /* 2: 0 1 */
> > uint8_t spare_dl1:2; /* 2: 1 1 */
> > }; /* 2 1 */
> > struct {
> > uint8_t dl_delay_ind:1; /* 2: 0 1 */
> > uint8_t ul_delay_ind:1; /* 2: 1 1 */
> > uint8_t snp_ul1:1; /* 2: 2 1 */
> > }; /* 2 1 */
> > }; /* 2 1 */
> > union {
> > struct {
> > uint8_t ppp:1; /* 3: 0 1 */
> > uint8_t rqi:1; /* 3: 1 1 */
> > }; /* 3 1 */
> > struct {
> > uint8_t n_delay_ind:1; /* 3: 0 1 */
> > uint8_t spare_ul2:1; /* 3: 1 1 */
> > }; /* 3 1 */
> > }; /* 3 1 */
> > uint8_t qfi:6; /* 4: 0 1 */
> >
> > /* XXX 2 bits hole, try to pack */
> >
> > uint8_t data[]; /* 5 0 */
> >
> > /* size: 5, cachelines: 1, members: 7 */
> > /* sum members: 3 */
> > /* sum bitfield members: 11 bits, bit holes: 2, sum bit holes: 5 bits */
> > /* last cacheline: 5 bytes */
> > };
> >
> > >
> > > /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> > > #ifndef __cplusplus
> > > static const struct rte_flow_item_gtp_psc
> > > rte_flow_item_gtp_psc_mask = {
> > > - .qfi = 0x3f,
> > > + .hdr.ppp = 1,
> > > + .hdr.rqi = 1,
> > > + .hdr.qfi = 0x3f,
> > > };
> > > #endif
> > >
> > >
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-13 9:24 ` Ori Kam
@ 2021-04-14 12:16 ` Ferruh Yigit
2021-04-15 6:33 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-14 12:16 UTC (permalink / raw)
To: Ori Kam, Raslan Darawsheh, dev@dpdk.org,
andrew.rybchenko@oktetlabs.ru
Cc: ivan.malov@oktetlabs.ru, ying.a.wang@intel.com,
olivier.matz@6wind.com, Slava Ovsiienko, Shiri Kuzin,
stable@dpdk.org, David Marchand, NBU-Contact-Thomas Monjalon
On 4/13/2021 10:24 AM, Ori Kam wrote:
> Hi Raslan,
>
>> -----Original Message-----
>> From: Raslan Darawsheh <rasland@nvidia.com>
>> Subject: RE: [PATCH v4 2/2] ethdev: update qfi definition
>>
>> Hi,
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Tuesday, April 6, 2021 7:10 PM
>>> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org; Ori Kam
>>> <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru
>>> Cc: ivan.malov@oktetlabs.ru; ying.a.wang@intel.com;
>>> olivier.matz@6wind.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
>>> Kuzin <shirik@nvidia.com>; stable@dpdk.org; David Marchand
>>> <david.marchand@redhat.com>; NBU-Contact-Thomas Monjalon
>>> <thomas@monjalon.net>
>>> Subject: Re: [PATCH v4 2/2] ethdev: update qfi definition
>>>
>>> On 4/4/2021 8:45 AM, Raslan Darawsheh wrote:
>>>> qfi field is 8 bits which represent single bit for
>>>> PPP (paging Policy Presence) single bit for RQI
>>>> (Reflective QoS Indicator) and 6 bits for qfi
>>>> (QoS Flow Identifier) based on RFC 38415-g30
>>>>
>>>> This update the doxygen format and the mask for qfi
>>>> to properly identify the full 8 bits of the field.
>>>>
>>>> note: changing the default mask would cause different
>>>> patterns generated by testpmd.
>>>>
>>>> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
>>>> Cc: ying.a.wang@intel.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
>>>> ---
>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
>>>> lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
>>>> 2 files changed, 19 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index f59eb8a27d..dd39c4c3c2 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
>>> their attributes, if any.
>>>> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
>>>>
>>>> - ``pdu_type {unsigned}``: PDU type.
>>>> - - ``qfi {unsigned}``: QoS flow identifier.
>>>> +
>>>> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
>>>>
>>>> - ``pppoes``, ``pppoed``: match PPPoE header.
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>>> index 6cc57136ac..20b4389429 100644
>>>> --- a/lib/librte_ethdev/rte_flow.h
>>>> +++ b/lib/librte_ethdev/rte_flow.h
>>>> @@ -20,6 +20,7 @@
>>>> #include <rte_arp.h>
>>>> #include <rte_common.h>
>>>> #include <rte_ether.h>
>>>> +#include <rte_gtp.h>
>>>> #include <rte_icmp.h>
>>>> #include <rte_ip.h>
>>>> #include <rte_sctp.h>
>>>> @@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta
>>> rte_flow_item_meta_mask = {
>>>> *
>>>> * Matches a GTP PDU extension header with type 0x85.
>>>> */
>>>> +RTE_STD_C11
>>>> struct rte_flow_item_gtp_psc {
>>>> - uint8_t pdu_type; /**< PDU type. */
>>>> - uint8_t qfi; /**< QoS flow identifier. */
>>>> + union {
>>>> + struct {
>>>> + /*
>>>> + * These fields are retained for compatibility.
>>>> + * Please switch to the new header field below.
>>>> + */
>>>> + uint8_t pdu_type; /**< PDU type. */
>>>> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
>>>> +
>>>> + };
>>>> + struct rte_gtp_psc_hdr hdr;
>>>> + };
>>>> };
>>>
>>> This will change the struct size even with union, since old version is missing
>>> all fields from protocol header. Struct size will go from 2 --> 5 bytes [1].
>>>
>>> Since this is public struct, we can't change its size.
>>>
>>> @Ori, Andrew,
>>>
>>> Do you have a suggestion for next step?
>>>
>
> I think Ferruh is right, and I think that we should at this point just update the documentation.
> Sorry for the detour
> Just small comment about the original patch.
> I don’t think you should change the default mask since it may break existing apps.
>
I will continue with first patch [1], and will update the protocol reference in
the commit log.
Meanwhile adding 'struct rte_gtp_psc_hdr' (patch v4 1/2) work can continue
separately.
And when it is added we can send a deprecation notice to update 'struct
rte_flow_item_gtp_psc' and finally update it on v21.11 to have 'struct
rte_gtp_psc_hdr' in it. Makes sense?
[1]
https://patches.dpdk.org/project/dpdk/patch/20210323121134.19113-1-rasland@nvidia.com/
>>> - We can still add the "struct rte_gtp_psc_hdr", and add a deprecation notice
>>> for "struct rte_flow_item_gtp_psc" to say it will use "struct
>>> rte_gtp_psc_hdr"
>>> on 21.11.
>>>
>>> - And for this release use the Raslan's first version:
>>> - uint8_t qfi; /**< QoS flow identifier. */
>>> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
>>>
>>>
>>> Does it make sense? Any comments?
>>>
>> @Ori Kam, @andrew.rybchenko@oktetlabs.ru,
>> This is a kind remainder of this patch,
>>
>>>
>>>
>>> [1]
>>> struct rte_gtp_psc_hdr {
>>> uint8_t ext_hdr_len; /* 0 1 */
>>> uint8_t type:4; /* 1: 0 1 */
>>> uint8_t qmp:1; /* 1: 4 1 */
>>>
>>> /* XXX 3 bits hole, try to pack */
>>>
>>> union {
>>> struct {
>>> uint8_t snp:1; /* 2: 0 1 */
>>> uint8_t spare_dl1:2; /* 2: 1 1 */
>>> }; /* 2 1 */
>>> struct {
>>> uint8_t dl_delay_ind:1; /* 2: 0 1 */
>>> uint8_t ul_delay_ind:1; /* 2: 1 1 */
>>> uint8_t snp_ul1:1; /* 2: 2 1 */
>>> }; /* 2 1 */
>>> }; /* 2 1 */
>>> union {
>>> struct {
>>> uint8_t ppp:1; /* 3: 0 1 */
>>> uint8_t rqi:1; /* 3: 1 1 */
>>> }; /* 3 1 */
>>> struct {
>>> uint8_t n_delay_ind:1; /* 3: 0 1 */
>>> uint8_t spare_ul2:1; /* 3: 1 1 */
>>> }; /* 3 1 */
>>> }; /* 3 1 */
>>> uint8_t qfi:6; /* 4: 0 1 */
>>>
>>> /* XXX 2 bits hole, try to pack */
>>>
>>> uint8_t data[]; /* 5 0 */
>>>
>>> /* size: 5, cachelines: 1, members: 7 */
>>> /* sum members: 3 */
>>> /* sum bitfield members: 11 bits, bit holes: 2, sum bit holes: 5 bits */
>>> /* last cacheline: 5 bytes */
>>> };
>>>
>>>>
>>>> /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
>>>> #ifndef __cplusplus
>>>> static const struct rte_flow_item_gtp_psc
>>>> rte_flow_item_gtp_psc_mask = {
>>>> - .qfi = 0x3f,
>>>> + .hdr.ppp = 1,
>>>> + .hdr.rqi = 1,
>>>> + .hdr.qfi = 0x3f,
>>>> };
>>>> #endif
>>>>
>>>>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: update qfi definition
2021-03-23 12:11 [dpdk-dev] [PATCH] ethdev: update qfi definition Raslan Darawsheh
2021-03-26 13:12 ` Ferruh Yigit
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support Raslan Darawsheh
@ 2021-04-14 12:40 ` Ferruh Yigit
2 siblings, 0 replies; 37+ messages in thread
From: Ferruh Yigit @ 2021-04-14 12:40 UTC (permalink / raw)
To: Raslan Darawsheh, dev; +Cc: viacheslavo, shirik, ying.a.wang, stable
On 3/23/2021 12:11 PM, Raslan Darawsheh wrote:
> qfi field is 8 bits which represent single bit for
> PPP (paging Policy Presence) single bit for RQI
> (Reflective QoS Indicator) and 6 bits for qfi
> (QoS Flow Identifier)
>
> This update the doxygen format and the mask for qfi
> to properly identify the full 8 bits of the field.
>
> note: changing the default mask would cause different
> patterns generated by testpmd.
>
> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> Cc: ying.a.wang@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
Sorry for going through multiple versions, hopefully proper solution can be
merged after deprecation notice on v21.11.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
2021-04-14 12:16 ` Ferruh Yigit
@ 2021-04-15 6:33 ` Raslan Darawsheh
0 siblings, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-15 6:33 UTC (permalink / raw)
To: Ferruh Yigit, Ori Kam, dev@dpdk.org,
andrew.rybchenko@oktetlabs.ru
Cc: ivan.malov@oktetlabs.ru, ying.a.wang@intel.com,
olivier.matz@6wind.com, Slava Ovsiienko, Shiri Kuzin,
stable@dpdk.org, David Marchand, NBU-Contact-Thomas Monjalon
Kindest regards,
Raslan Darawsheh
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, April 14, 2021 3:17 PM
> To: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> dev@dpdk.org; andrew.rybchenko@oktetlabs.ru
> Cc: ivan.malov@oktetlabs.ru; ying.a.wang@intel.com;
> olivier.matz@6wind.com; Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri
> Kuzin <shirik@nvidia.com>; stable@dpdk.org; David Marchand
> <david.marchand@redhat.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>
> Subject: Re: [PATCH v4 2/2] ethdev: update qfi definition
>
> On 4/13/2021 10:24 AM, Ori Kam wrote:
> > Hi Raslan,
> >
> >> -----Original Message-----
> >> From: Raslan Darawsheh <rasland@nvidia.com>
> >> Subject: RE: [PATCH v4 2/2] ethdev: update qfi definition
> >>
> >> Hi,
> >>
> >>> -----Original Message-----
> >>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> Sent: Tuesday, April 6, 2021 7:10 PM
> >>> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org; Ori Kam
> >>> <orika@nvidia.com>; andrew.rybchenko@oktetlabs.ru
> >>> Cc: ivan.malov@oktetlabs.ru; ying.a.wang@intel.com;
> >>> olivier.matz@6wind.com; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Shiri
> >>> Kuzin <shirik@nvidia.com>; stable@dpdk.org; David Marchand
> >>> <david.marchand@redhat.com>; NBU-Contact-Thomas Monjalon
> >>> <thomas@monjalon.net>
> >>> Subject: Re: [PATCH v4 2/2] ethdev: update qfi definition
> >>>
> >>> On 4/4/2021 8:45 AM, Raslan Darawsheh wrote:
> >>>> qfi field is 8 bits which represent single bit for
> >>>> PPP (paging Policy Presence) single bit for RQI
> >>>> (Reflective QoS Indicator) and 6 bits for qfi
> >>>> (QoS Flow Identifier) based on RFC 38415-g30
> >>>>
> >>>> This update the doxygen format and the mask for qfi
> >>>> to properly identify the full 8 bits of the field.
> >>>>
> >>>> note: changing the default mask would cause different
> >>>> patterns generated by testpmd.
> >>>>
> >>>> Fixes: 346553db5bd1 ("ethdev: add GTP extension header to flow API")
> >>>> Cc: ying.a.wang@intel.com
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> >>>> ---
> >>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++-
> >>>> lib/librte_ethdev/rte_flow.h | 20 +++++++++++++++++---
> >>>> 2 files changed, 19 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>>> index f59eb8a27d..dd39c4c3c2 100644
> >>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>>> @@ -3742,7 +3742,8 @@ This section lists supported pattern items and
> >>> their attributes, if any.
> >>>> - ``gtp_psc``: match GTP PDU extension header with type 0x85.
> >>>>
> >>>> - ``pdu_type {unsigned}``: PDU type.
> >>>> - - ``qfi {unsigned}``: QoS flow identifier.
> >>>> +
> >>>> + - ``qfi {unsigned}``: PPP, RQI and QoS flow identifier.
> >>>>
> >>>> - ``pppoes``, ``pppoed``: match PPPoE header.
> >>>>
> >>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> >>>> index 6cc57136ac..20b4389429 100644
> >>>> --- a/lib/librte_ethdev/rte_flow.h
> >>>> +++ b/lib/librte_ethdev/rte_flow.h
> >>>> @@ -20,6 +20,7 @@
> >>>> #include <rte_arp.h>
> >>>> #include <rte_common.h>
> >>>> #include <rte_ether.h>
> >>>> +#include <rte_gtp.h>
> >>>> #include <rte_icmp.h>
> >>>> #include <rte_ip.h>
> >>>> #include <rte_sctp.h>
> >>>> @@ -1421,16 +1422,29 @@ static const struct rte_flow_item_meta
> >>> rte_flow_item_meta_mask = {
> >>>> *
> >>>> * Matches a GTP PDU extension header with type 0x85.
> >>>> */
> >>>> +RTE_STD_C11
> >>>> struct rte_flow_item_gtp_psc {
> >>>> - uint8_t pdu_type; /**< PDU type. */
> >>>> - uint8_t qfi; /**< QoS flow identifier. */
> >>>> + union {
> >>>> + struct {
> >>>> + /*
> >>>> + * These fields are retained for compatibility.
> >>>> + * Please switch to the new header field below.
> >>>> + */
> >>>> + uint8_t pdu_type; /**< PDU type. */
> >>>> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> >>>> +
> >>>> + };
> >>>> + struct rte_gtp_psc_hdr hdr;
> >>>> + };
> >>>> };
> >>>
> >>> This will change the struct size even with union, since old version is
> missing
> >>> all fields from protocol header. Struct size will go from 2 --> 5 bytes [1].
> >>>
> >>> Since this is public struct, we can't change its size.
> >>>
> >>> @Ori, Andrew,
> >>>
> >>> Do you have a suggestion for next step?
> >>>
> >
> > I think Ferruh is right, and I think that we should at this point just update
> the documentation.
> > Sorry for the detour
> > Just small comment about the original patch.
> > I don't think you should change the default mask since it may break existing
> apps.
> >
>
> I will continue with first patch [1], and will update the protocol reference in
> the commit log.
>
> Meanwhile adding 'struct rte_gtp_psc_hdr' (patch v4 1/2) work can continue
> separately.
> And when it is added we can send a deprecation notice to update 'struct
> rte_flow_item_gtp_psc' and finally update it on v21.11 to have 'struct
> rte_gtp_psc_hdr' in it. Makes sense?
>
Yes, makes sense to me,
I'll work in the meanwhile on providing the new hdrs definition .
> [1]
> https://patches.dpdk.org/project/dpdk/patch/20210323121134.19113-1-
> rasland@nvidia.com/
>
> >>> - We can still add the "struct rte_gtp_psc_hdr", and add a deprecation
> notice
> >>> for "struct rte_flow_item_gtp_psc" to say it will use "struct
> >>> rte_gtp_psc_hdr"
> >>> on 21.11.
> >>>
> >>> - And for this release use the Raslan's first version:
> >>> - uint8_t qfi; /**< QoS flow identifier. */
> >>> + uint8_t qfi; /**< PPP, RQI, QoS flow identifier. */
> >>>
> >>>
> >>> Does it make sense? Any comments?
> >>>
> >> @Ori Kam, @andrew.rybchenko@oktetlabs.ru,
> >> This is a kind remainder of this patch,
> >>
> >>>
> >>>
> >>> [1]
> >>> struct rte_gtp_psc_hdr {
> >>> uint8_t ext_hdr_len; /* 0 1 */
> >>> uint8_t type:4; /* 1: 0 1 */
> >>> uint8_t qmp:1; /* 1: 4 1 */
> >>>
> >>> /* XXX 3 bits hole, try to pack */
> >>>
> >>> union {
> >>> struct {
> >>> uint8_t snp:1; /* 2: 0 1 */
> >>> uint8_t spare_dl1:2; /* 2: 1 1 */
> >>> }; /* 2 1 */
> >>> struct {
> >>> uint8_t dl_delay_ind:1; /* 2: 0 1 */
> >>> uint8_t ul_delay_ind:1; /* 2: 1 1 */
> >>> uint8_t snp_ul1:1; /* 2: 2 1 */
> >>> }; /* 2 1 */
> >>> }; /* 2 1 */
> >>> union {
> >>> struct {
> >>> uint8_t ppp:1; /* 3: 0 1 */
> >>> uint8_t rqi:1; /* 3: 1 1 */
> >>> }; /* 3 1 */
> >>> struct {
> >>> uint8_t n_delay_ind:1; /* 3: 0 1 */
> >>> uint8_t spare_ul2:1; /* 3: 1 1 */
> >>> }; /* 3 1 */
> >>> }; /* 3 1 */
> >>> uint8_t qfi:6; /* 4: 0 1 */
> >>>
> >>> /* XXX 2 bits hole, try to pack */
> >>>
> >>> uint8_t data[]; /* 5 0 */
> >>>
> >>> /* size: 5, cachelines: 1, members: 7 */
> >>> /* sum members: 3 */
> >>> /* sum bitfield members: 11 bits, bit holes: 2, sum bit holes: 5 bits
> */
> >>> /* last cacheline: 5 bytes */
> >>> };
> >>>
> >>>>
> >>>> /** Default mask for RTE_FLOW_ITEM_TYPE_GTP_PSC. */
> >>>> #ifndef __cplusplus
> >>>> static const struct rte_flow_item_gtp_psc
> >>>> rte_flow_item_gtp_psc_mask = {
> >>>> - .qfi = 0x3f,
> >>>> + .hdr.ppp = 1,
> >>>> + .hdr.rqi = 1,
> >>>> + .hdr.qfi = 0x3f,
> >>>> };
> >>>> #endif
> >>>>
> >>>>
> >
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v5 0/1] add new hdr for gtp qfi
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition Raslan Darawsheh
@ 2021-04-29 8:10 ` Raslan Darawsheh
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2 siblings, 1 reply; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-29 8:10 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, orika, andrew.rybchenko, ivan.malov, ying.a.wang,
olivier.matz, viacheslavo, shirik
This is introducin a new hdr definition of gtp psc
support to match the RFC 38415-g30
v2: introduce new header definition for gtp psc
update commit msg for rte flow item change.
v3: fixed typo in comment
Cc relevant people.
v4: update hdr definition to have hdr suffix.
update variable name to be hdr in the gtp_psc item.
update default max to use the new added hdr.
v5: updated the hdr definition after code review.
dropped the change for rte_flow item psc to avoid ABI breakage
added hdr definition for type0 and type1 psc's
Raslan Darawsheh (1):
ethdev: add new ext hdr for gtp psc
lib/net/rte_gtp.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
--
2.25.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 0/1] add new hdr for gtp qfi Raslan Darawsheh
@ 2021-04-29 8:10 ` Raslan Darawsheh
2021-05-11 11:46 ` Ferruh Yigit
2021-06-08 12:17 ` Andrew Rybchenko
0 siblings, 2 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-04-29 8:10 UTC (permalink / raw)
To: dev
Cc: ferruh.yigit, orika, andrew.rybchenko, ivan.malov, ying.a.wang,
olivier.matz, viacheslavo, shirik
Define new rte header for gtp PDU session container
based on RFC 38415-g30
Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
---
lib/net/rte_gtp.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/lib/net/rte_gtp.h b/lib/net/rte_gtp.h
index 6a6f9b238d..5a850a26e4 100644
--- a/lib/net/rte_gtp.h
+++ b/lib/net/rte_gtp.h
@@ -61,6 +61,84 @@ struct rte_gtp_hdr_ext_word {
uint8_t next_ext; /**< Next Extension Header Type. */
} __rte_packed;
+/**
+ * Optional extension for GTP with next_ext set to 0x85
+ * defined based on RFC 38415-g30.
+ */
+__extension__
+struct rte_gtp_psc_generic_hdr {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ uint8_t pad:3; /**< type specfic pad bits */
+ uint8_t spare:2; /**< type specific spare bits */
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+#else
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t spare:2; /**< type specific spare bits */
+ uint8_t pad:3; /**< type specfic pad bits */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ uint8_t type:4; /**< PDU type */
+#endif
+ uint8_t data[0]; /**< variable length data feilds */
+} __rte_packed;
+
+/**
+ * Optional extension for GTP with next_ext set to 0x85
+ * type0 defined based on RFC 38415-g30
+ */
+__extension__
+struct rte_gtp_psc_type0_hdr {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ uint8_t snp:1; /**< Sequence number presence */
+ uint8_t spare_dl1:2; /**< spare down link bits */
+ uint8_t ppp:1; /**< Paging policy presence */
+ uint8_t rqi:1; /**< Reflective Qos Indicator */
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+#else
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t rqi:1; /**< Reflective Qos Indicator */
+ uint8_t ppp:1; /**< Paging policy presence */
+ uint8_t spare_dl1:2; /**< spare down link bits */
+ uint8_t snp:1; /**< Sequence number presence */
+ uint8_t type:4; /**< PDU type */
+#endif
+ uint8_t data[0]; /**< variable length data feilds */
+} __rte_packed;
+
+/**
+ * Optional extension for GTP with next_ext set to 0x85
+ * type1 defined based on RFC 38415-g30
+ */
+__extension__
+struct rte_gtp_psc_type1_hdr {
+ uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+ uint8_t type:4; /**< PDU type */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ uint8_t dl_delay_ind:1; /**< dl delay result presence */
+ uint8_t ul_delay_ind:1; /**< ul delay result presence */
+ uint8_t snp:1; /**< Sequence number presence ul */
+ uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
+ uint8_t spare_ul2:1; /**< spare up link bits */
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+#else
+ uint8_t qfi:6; /**< Qos Flow Identifier */
+ uint8_t spare_ul2:1; /**< spare up link bits */
+ uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
+ uint8_t snp:1; /**< Sequence number presence ul */
+ uint8_t ul_delay_ind:1; /**< ul delay result presence */
+ uint8_t dl_delay_ind:1; /**< dl delay result presence */
+ uint8_t qmp:1; /**< Qos Monitoring Packet */
+ uint8_t type:4; /**< PDU type */
+#endif
+ uint8_t data[0]; /**< variable length data feilds */
+} __rte_packed;
+
/** GTP header length */
#define RTE_ETHER_GTP_HLEN \
(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
--
2.25.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-08 12:29 ` Olivier Matz
2021-04-08 12:37 ` Raslan Darawsheh
@ 2021-04-29 16:29 ` Tyler Retzlaff
2021-06-08 12:13 ` Andrew Rybchenko
1 sibling, 1 reply; 37+ messages in thread
From: Tyler Retzlaff @ 2021-04-29 16:29 UTC (permalink / raw)
To: Olivier Matz
Cc: Raslan Darawsheh, dev, ferruh.yigit, orika, andrew.rybchenko,
ivan.malov, ying.a.wang, viacheslavo, shirik
On Thu, Apr 08, 2021 at 02:29:56PM +0200, Olivier Matz wrote:
> Hi Raslan,
>
> > +/**
> > + * Optional extension for GTP with next_ext set to 0x85
> > + * defined based on RFC 38415-g30.
> > + */
> > +__extension__
> > +struct rte_gtp_psc_hdr {
> > + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> > + uint8_t type:4; /**< PDU type */
> > + uint8_t qmp:1; /**< Qos Monitoring Packet */
would it be a lot ot ask to have the structure defined using standard C
instead of using compiler specific extensions?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
@ 2021-05-11 11:46 ` Ferruh Yigit
2021-06-08 12:17 ` Andrew Rybchenko
1 sibling, 0 replies; 37+ messages in thread
From: Ferruh Yigit @ 2021-05-11 11:46 UTC (permalink / raw)
To: Raslan Darawsheh, dev
Cc: orika, andrew.rybchenko, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
On 4/29/2021 9:10 AM, Raslan Darawsheh wrote:
> Define new rte header for gtp PDU session container
> based on RFC 38415-g30
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
This patch has not urgent for this release, postponing it to next release.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc
2021-04-29 16:29 ` Tyler Retzlaff
@ 2021-06-08 12:13 ` Andrew Rybchenko
0 siblings, 0 replies; 37+ messages in thread
From: Andrew Rybchenko @ 2021-06-08 12:13 UTC (permalink / raw)
To: Tyler Retzlaff, Olivier Matz
Cc: Raslan Darawsheh, dev, ferruh.yigit, orika, ivan.malov,
ying.a.wang, viacheslavo, shirik
On 4/29/21 7:29 PM, Tyler Retzlaff wrote:
> On Thu, Apr 08, 2021 at 02:29:56PM +0200, Olivier Matz wrote:
>> Hi Raslan,
>>
>>> +/**
>>> + * Optional extension for GTP with next_ext set to 0x85
>>> + * defined based on RFC 38415-g30.
>>> + */
>>> +__extension__
>>> +struct rte_gtp_psc_hdr {
>>> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
>>> + uint8_t type:4; /**< PDU type */
>>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>
> would it be a lot ot ask to have the structure defined using standard C
> instead of using compiler specific extensions?
>
It is a valid request, but I'm afraid bit fields are used
in many-many core DPDK libraries. So, change of the approach
or at least direction for a new code should be discussed and
approved by techboard.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-05-11 11:46 ` Ferruh Yigit
@ 2021-06-08 12:17 ` Andrew Rybchenko
2021-06-08 12:18 ` Andrew Rybchenko
1 sibling, 1 reply; 37+ messages in thread
From: Andrew Rybchenko @ 2021-06-08 12:17 UTC (permalink / raw)
To: Raslan Darawsheh, dev
Cc: ferruh.yigit, orika, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
On 4/29/21 11:10 AM, Raslan Darawsheh wrote:
> Define new rte header for gtp PDU session container
> based on RFC 38415-g30
>
> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> ---
> lib/net/rte_gtp.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/lib/net/rte_gtp.h b/lib/net/rte_gtp.h
> index 6a6f9b238d..5a850a26e4 100644
> --- a/lib/net/rte_gtp.h
> +++ b/lib/net/rte_gtp.h
> @@ -61,6 +61,84 @@ struct rte_gtp_hdr_ext_word {
> uint8_t next_ext; /**< Next Extension Header Type. */
> } __rte_packed;
>
> +/**
> + * Optional extension for GTP with next_ext set to 0x85
> + * defined based on RFC 38415-g30.
> + */
> +__extension__
> +struct rte_gtp_psc_generic_hdr {
> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t type:4; /**< PDU type */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + uint8_t pad:3; /**< type specfic pad bits */
> + uint8_t spare:2; /**< type specific spare bits */
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> +#else
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> + uint8_t spare:2; /**< type specific spare bits */
> + uint8_t pad:3; /**< type specfic pad bits */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + uint8_t type:4; /**< PDU type */
> +#endif
> + uint8_t data[0]; /**< variable length data feilds */
> +} __rte_packed;
> +
> +/**
> + * Optional extension for GTP with next_ext set to 0x85
> + * type0 defined based on RFC 38415-g30
> + */
> +__extension__
> +struct rte_gtp_psc_type0_hdr {
> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t type:4; /**< PDU type */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + uint8_t snp:1; /**< Sequence number presence */
> + uint8_t spare_dl1:2; /**< spare down link bits */
> + uint8_t ppp:1; /**< Paging policy presence */
> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> +#else
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> + uint8_t ppp:1; /**< Paging policy presence */
> + uint8_t spare_dl1:2; /**< spare down link bits */
> + uint8_t snp:1; /**< Sequence number presence */
> + uint8_t type:4; /**< PDU type */
> +#endif
> + uint8_t data[0]; /**< variable length data feilds */
> +} __rte_packed;
> +
> +/**
> + * Optional extension for GTP with next_ext set to 0x85
> + * type1 defined based on RFC 38415-g30
> + */
> +__extension__
> +struct rte_gtp_psc_type1_hdr {
> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> + uint8_t type:4; /**< PDU type */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> + uint8_t snp:1; /**< Sequence number presence ul */
> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> + uint8_t spare_ul2:1; /**< spare up link bits */
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> +#else
> + uint8_t qfi:6; /**< Qos Flow Identifier */
> + uint8_t spare_ul2:1; /**< spare up link bits */
> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> + uint8_t snp:1; /**< Sequence number presence ul */
> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> + uint8_t type:4; /**< PDU type */
> +#endif
> + uint8_t data[0]; /**< variable length data feilds */
> +} __rte_packed;
> +
> /** GTP header length */
> #define RTE_ETHER_GTP_HLEN \
> (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
>
This way the structure is very hard to read.
May I ask to indent field comments as it is done in
other rte_net structures (e.g. rte_ipv4_hdr, rte_udp_hdr,
rte_geneve_hdr). Personally I don't care if it will be
indent by TABs or spaces, however, it looks like TABs are
used in more places.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
2021-06-08 12:17 ` Andrew Rybchenko
@ 2021-06-08 12:18 ` Andrew Rybchenko
2021-06-08 14:07 ` Raslan Darawsheh
0 siblings, 1 reply; 37+ messages in thread
From: Andrew Rybchenko @ 2021-06-08 12:18 UTC (permalink / raw)
To: Raslan Darawsheh, dev
Cc: ferruh.yigit, orika, ivan.malov, ying.a.wang, olivier.matz,
viacheslavo, shirik
On 6/8/21 3:17 PM, Andrew Rybchenko wrote:
> On 4/29/21 11:10 AM, Raslan Darawsheh wrote:
>> Define new rte header for gtp PDU session container
>> based on RFC 38415-g30
>>
>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
>> ---
>> lib/net/rte_gtp.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 78 insertions(+)
>>
>> diff --git a/lib/net/rte_gtp.h b/lib/net/rte_gtp.h
>> index 6a6f9b238d..5a850a26e4 100644
>> --- a/lib/net/rte_gtp.h
>> +++ b/lib/net/rte_gtp.h
>> @@ -61,6 +61,84 @@ struct rte_gtp_hdr_ext_word {
>> uint8_t next_ext; /**< Next Extension Header Type. */
>> } __rte_packed;
>>
>> +/**
>> + * Optional extension for GTP with next_ext set to 0x85
>> + * defined based on RFC 38415-g30.
>> + */
>> +__extension__
>> +struct rte_gtp_psc_generic_hdr {
>> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
>> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
>> + uint8_t type:4; /**< PDU type */
>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>> + uint8_t pad:3; /**< type specfic pad bits */
>> + uint8_t spare:2; /**< type specific spare bits */
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> +#else
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> + uint8_t spare:2; /**< type specific spare bits */
>> + uint8_t pad:3; /**< type specfic pad bits */
>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>> + uint8_t type:4; /**< PDU type */
>> +#endif
>> + uint8_t data[0]; /**< variable length data feilds */
>> +} __rte_packed;
>> +
>> +/**
>> + * Optional extension for GTP with next_ext set to 0x85
>> + * type0 defined based on RFC 38415-g30
>> + */
>> +__extension__
>> +struct rte_gtp_psc_type0_hdr {
>> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
>> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
>> + uint8_t type:4; /**< PDU type */
>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>> + uint8_t snp:1; /**< Sequence number presence */
>> + uint8_t spare_dl1:2; /**< spare down link bits */
>> + uint8_t ppp:1; /**< Paging policy presence */
>> + uint8_t rqi:1; /**< Reflective Qos Indicator */
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> +#else
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> + uint8_t rqi:1; /**< Reflective Qos Indicator */
>> + uint8_t ppp:1; /**< Paging policy presence */
>> + uint8_t spare_dl1:2; /**< spare down link bits */
>> + uint8_t snp:1; /**< Sequence number presence */
>> + uint8_t type:4; /**< PDU type */
>> +#endif
>> + uint8_t data[0]; /**< variable length data feilds */
>> +} __rte_packed;
>> +
>> +/**
>> + * Optional extension for GTP with next_ext set to 0x85
>> + * type1 defined based on RFC 38415-g30
>> + */
>> +__extension__
>> +struct rte_gtp_psc_type1_hdr {
>> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes */
>> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
>> + uint8_t type:4; /**< PDU type */
>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
>> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
>> + uint8_t snp:1; /**< Sequence number presence ul */
>> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
>> + uint8_t spare_ul2:1; /**< spare up link bits */
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> +#else
>> + uint8_t qfi:6; /**< Qos Flow Identifier */
>> + uint8_t spare_ul2:1; /**< spare up link bits */
>> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
>> + uint8_t snp:1; /**< Sequence number presence ul */
>> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
>> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
>> + uint8_t qmp:1; /**< Qos Monitoring Packet */
>> + uint8_t type:4; /**< PDU type */
>> +#endif
>> + uint8_t data[0]; /**< variable length data feilds */
>> +} __rte_packed;
>> +
>> /** GTP header length */
>> #define RTE_ETHER_GTP_HLEN \
>> (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
>>
>
> This way the structure is very hard to read.
> May I ask to indent field comments as it is done in
> other rte_net structures (e.g. rte_ipv4_hdr, rte_udp_hdr,
> rte_geneve_hdr). Personally I don't care if it will be
> indent by TABs or spaces, however, it looks like TABs are
> used in more places.
>
Also there are spelling bugs [1].
[1] http://mails.dpdk.org/archives/test-report/2021-April/191649.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
2021-06-08 12:18 ` Andrew Rybchenko
@ 2021-06-08 14:07 ` Raslan Darawsheh
0 siblings, 0 replies; 37+ messages in thread
From: Raslan Darawsheh @ 2021-06-08 14:07 UTC (permalink / raw)
To: Andrew Rybchenko, dev@dpdk.org
Cc: ferruh.yigit@intel.com, Ori Kam, ivan.malov@oktetlabs.ru,
ying.a.wang@intel.com, olivier.matz@6wind.com, Slava Ovsiienko,
Shiri Kuzin
Hi Andrew,
Thanks for your comments and review, I'll attempt to handle and fix in the next version.
Kindest regards,
Raslan Darawsheh
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, June 8, 2021 3:19 PM
> To: Raslan Darawsheh <rasland@nvidia.com>; dev@dpdk.org
> Cc: ferruh.yigit@intel.com; Ori Kam <orika@nvidia.com>;
> ivan.malov@oktetlabs.ru; ying.a.wang@intel.com; olivier.matz@6wind.com;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Shiri Kuzin <shirik@nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc
>
> On 6/8/21 3:17 PM, Andrew Rybchenko wrote:
> > On 4/29/21 11:10 AM, Raslan Darawsheh wrote:
> >> Define new rte header for gtp PDU session container based on RFC
> >> 38415-g30
> >>
> >> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
> >> ---
> >> lib/net/rte_gtp.h | 78
> >> +++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 78 insertions(+)
> >>
> >> diff --git a/lib/net/rte_gtp.h b/lib/net/rte_gtp.h index
> >> 6a6f9b238d..5a850a26e4 100644
> >> --- a/lib/net/rte_gtp.h
> >> +++ b/lib/net/rte_gtp.h
> >> @@ -61,6 +61,84 @@ struct rte_gtp_hdr_ext_word {
> >> uint8_t next_ext; /**< Next Extension Header Type. */
> >> } __rte_packed;
> >>
> >> +/**
> >> + * Optional extension for GTP with next_ext set to 0x85
> >> + * defined based on RFC 38415-g30.
> >> + */
> >> +__extension__
> >> +struct rte_gtp_psc_generic_hdr {
> >> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes
> >> +*/ #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> >> + uint8_t type:4; /**< PDU type */
> >> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> >> + uint8_t pad:3; /**< type specfic pad bits */
> >> + uint8_t spare:2; /**< type specific spare bits */
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */ #else
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */
> >> + uint8_t spare:2; /**< type specific spare bits */
> >> + uint8_t pad:3; /**< type specfic pad bits */
> >> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> >> + uint8_t type:4; /**< PDU type */
> >> +#endif
> >> + uint8_t data[0]; /**< variable length data feilds */ }
> >> +__rte_packed;
> >> +
> >> +/**
> >> + * Optional extension for GTP with next_ext set to 0x85
> >> + * type0 defined based on RFC 38415-g30 */ __extension__ struct
> >> +rte_gtp_psc_type0_hdr {
> >> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes
> >> +*/ #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> >> + uint8_t type:4; /**< PDU type */
> >> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> >> + uint8_t snp:1; /**< Sequence number presence */
> >> + uint8_t spare_dl1:2; /**< spare down link bits */
> >> + uint8_t ppp:1; /**< Paging policy presence */
> >> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */ #else
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */
> >> + uint8_t rqi:1; /**< Reflective Qos Indicator */
> >> + uint8_t ppp:1; /**< Paging policy presence */
> >> + uint8_t spare_dl1:2; /**< spare down link bits */
> >> + uint8_t snp:1; /**< Sequence number presence */
> >> + uint8_t type:4; /**< PDU type */
> >> +#endif
> >> + uint8_t data[0]; /**< variable length data feilds */ }
> >> +__rte_packed;
> >> +
> >> +/**
> >> + * Optional extension for GTP with next_ext set to 0x85
> >> + * type1 defined based on RFC 38415-g30 */ __extension__ struct
> >> +rte_gtp_psc_type1_hdr {
> >> + uint8_t ext_hdr_len; /**< PDU ext hdr len in multiples of 4 bytes
> >> +*/ #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> >> + uint8_t type:4; /**< PDU type */
> >> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> >> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> >> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> >> + uint8_t snp:1; /**< Sequence number presence ul */
> >> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> >> + uint8_t spare_ul2:1; /**< spare up link bits */
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */ #else
> >> + uint8_t qfi:6; /**< Qos Flow Identifier */
> >> + uint8_t spare_ul2:1; /**< spare up link bits */
> >> + uint8_t n_delay_ind:1; /**< N3/N9 delay result presence */
> >> + uint8_t snp:1; /**< Sequence number presence ul */
> >> + uint8_t ul_delay_ind:1; /**< ul delay result presence */
> >> + uint8_t dl_delay_ind:1; /**< dl delay result presence */
> >> + uint8_t qmp:1; /**< Qos Monitoring Packet */
> >> + uint8_t type:4; /**< PDU type */
> >> +#endif
> >> + uint8_t data[0]; /**< variable length data feilds */ }
> >> +__rte_packed;
> >> +
> >> /** GTP header length */
> >> #define RTE_ETHER_GTP_HLEN \
> >> (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> >>
> >
> > This way the structure is very hard to read.
> > May I ask to indent field comments as it is done in other rte_net
> > structures (e.g. rte_ipv4_hdr, rte_udp_hdr, rte_geneve_hdr).
> > Personally I don't care if it will be indent by TABs or spaces,
> > however, it looks like TABs are used in more places.
> >
>
> Also there are spelling bugs [1].
>
> [1] http://mails.dpdk.org/archives/test-report/2021-April/191649.html
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2021-06-08 14:07 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 12:11 [dpdk-dev] [PATCH] ethdev: update qfi definition Raslan Darawsheh
2021-03-26 13:12 ` Ferruh Yigit
2021-03-29 8:53 ` Ori Kam
2021-03-29 9:06 ` Raslan Darawsheh
2021-03-29 11:14 ` Ferruh Yigit
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-04-01 16:51 ` Ferruh Yigit
2021-04-04 7:17 ` Raslan Darawsheh
2021-03-30 8:00 ` [dpdk-dev] [PATCH v3 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-01 16:54 ` Ferruh Yigit
2021-04-04 7:18 ` Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 0/2] fix gtp psc qfi support Raslan Darawsheh
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-04-08 12:29 ` Olivier Matz
2021-04-08 12:37 ` Raslan Darawsheh
2021-04-08 14:10 ` Ferruh Yigit
2021-04-08 14:10 ` Olivier Matz
2021-04-13 7:45 ` Raslan Darawsheh
2021-04-29 16:29 ` Tyler Retzlaff
2021-06-08 12:13 ` Andrew Rybchenko
2021-04-04 7:45 ` [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-06 16:09 ` Ferruh Yigit
2021-04-13 8:14 ` Raslan Darawsheh
2021-04-13 9:24 ` Ori Kam
2021-04-14 12:16 ` Ferruh Yigit
2021-04-15 6:33 ` Raslan Darawsheh
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 0/1] add new hdr for gtp qfi Raslan Darawsheh
2021-04-29 8:10 ` [dpdk-dev] [PATCH v5 1/1] ethdev: add new ext hdr for gtp psc Raslan Darawsheh
2021-05-11 11:46 ` Ferruh Yigit
2021-06-08 12:17 ` Andrew Rybchenko
2021-06-08 12:18 ` Andrew Rybchenko
2021-06-08 14:07 ` Raslan Darawsheh
2021-03-30 7:50 ` [dpdk-dev] [PATCH v2 2/2] ethdev: update qfi definition Raslan Darawsheh
2021-04-14 12:40 ` [dpdk-dev] [PATCH] " Ferruh Yigit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.