From: Ori Kam <orika@nvidia.com>
To: Raslan Darawsheh <rasland@nvidia.com>,
Ferruh Yigit <ferruh.yigit@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"andrew.rybchenko@oktetlabs.ru" <andrew.rybchenko@oktetlabs.ru>
Cc: "ivan.malov@oktetlabs.ru" <ivan.malov@oktetlabs.ru>,
"ying.a.wang@intel.com" <ying.a.wang@intel.com>,
"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
Slava Ovsiienko <viacheslavo@nvidia.com>,
Shiri Kuzin <shirik@nvidia.com>,
"stable@dpdk.org" <stable@dpdk.org>,
David Marchand <david.marchand@redhat.com>,
NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
Date: Tue, 13 Apr 2021 09:24:32 +0000 [thread overview]
Message-ID: <BYAPR12MB4983A5A056028501A24A7056D64F9@BYAPR12MB4983.namprd12.prod.outlook.com> (raw)
In-Reply-To: <DM6PR12MB274857DEB4A438DFA69AD634CF4F9@DM6PR12MB2748.namprd12.prod.outlook.com>
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
> > >
> > >
next prev parent reply other threads:[~2021-04-13 9:24 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR12MB4983A5A056028501A24A7056D64F9@BYAPR12MB4983.namprd12.prod.outlook.com \
--to=orika@nvidia.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=ivan.malov@oktetlabs.ru \
--cc=olivier.matz@6wind.com \
--cc=rasland@nvidia.com \
--cc=shirik@nvidia.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.com \
--cc=ying.a.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is 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.