On 4/8/2021 1:37 PM, Raslan Darawsheh wrote:
Hi Olivier,

-----Original Message-----
From: Olivier Matz <olivier.m...@6wind.com>
Sent: Thursday, April 8, 2021 3:30 PM
To: Raslan Darawsheh <rasl...@nvidia.com>
Cc: dev@dpdk.org; ferruh.yi...@intel.com; Ori Kam <or...@nvidia.com>;
andrew.rybche...@oktetlabs.ru; ivan.ma...@oktetlabs.ru;
ying.a.w...@intel.com; Slava Ovsiienko <viachesl...@nvidia.com>; Shiri
Kuzin <shi...@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 <rasl...@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-91978ffc2...@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


Reply via email to