All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Raslan Darawsheh <rasland@nvidia.com>,
	dev@dpdk.org, orika@nvidia.com, andrew.rybchenko@oktetlabs.ru
Cc: ivan.malov@oktetlabs.ru, ying.a.wang@intel.com,
	olivier.matz@6wind.com, viacheslavo@nvidia.com,
	shirik@nvidia.com, stable@dpdk.org,
	David Marchand <david.marchand@redhat.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v4 2/2] ethdev: update qfi definition
Date: Tue, 6 Apr 2021 17:09:53 +0100	[thread overview]
Message-ID: <536631b9-c634-ddac-c154-91978ffc29a5@intel.com> (raw)
In-Reply-To: <20210404074552.24190-3-rasland@nvidia.com>

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
>   
> 


  reply	other threads:[~2021-04-06 16:10 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 [this message]
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

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=536631b9-c634-ddac-c154-91978ffc29a5@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ivan.malov@oktetlabs.ru \
    --cc=olivier.matz@6wind.com \
    --cc=orika@nvidia.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.