From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4008BA0579; Thu, 8 Apr 2021 16:10:43 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CA155141083; Thu, 8 Apr 2021 16:10:39 +0200 (CEST) Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by mails.dpdk.org (Postfix) with ESMTP id DA7FD141088 for ; Thu, 8 Apr 2021 16:10:37 +0200 (CEST) Received: by mail-wm1-f42.google.com with SMTP id j20-20020a05600c1914b029010f31e15a7fso3019231wmq.1 for ; Thu, 08 Apr 2021 07:10:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ShnUp/H/2MaUc+QNzvEjeTDrgzxDacu1s34x7dUUoRs=; b=SasuAM4pkaig/qJAIldGcwtX2BT5GZsl9kKporuloyWyGMQWy194r8ANCT/m8ZvwRt H/kqQ6ebGZ4B6/2qIdEIQMTgIxFprmMms65xvM4sl0MtvMOST4Tx1nLOJPySNhP8OmBd 0Dojs8ymRSBUMcN8jvecvBw/7UrnqqmLm3YSSjdhkF9mxbVpi8r+ofNXGCsWjvk3rhiw Cvh8/W2kgxexYsi8BgpfRlEGP3qNlRmADm37QJLvEaxU4Se+3NvMLXysaYovoORMLlWT ipQt3ahCUxyMekHaPbm4I0qpB0i5YkdAchdzR0KvpxLydtRMHEELxd555OLTkkquL8pE hFPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ShnUp/H/2MaUc+QNzvEjeTDrgzxDacu1s34x7dUUoRs=; b=OUbjUnQie/lAEhS2XZ5CuQjH/YtxpNgrXAo4fwBTSO/ZT5viXFmma9zGQ8YTeobgDa AicWRhO1JQRAr61WdTGZhH2m5wykn+KiAk1p4Hfq0kFduizVx8SauIO5p63P07tfiknb wuZhUqBRJ3MRXlPnLi5H4/GOK3PWYYMnE4SrTslpvMLRKTVcj2T7g+1Not9rqOgXipjC pEVti2F4fTK2XrIJzcctlN2LkofDz977ISFuyWWDHr1Ojnck5C3BNvUksJy8g+UuiZSB DgsKampwO3SQbxfQAF1F3Y4Fetl9ZwzKImI7Au/lHKtBXmAc9z1OrxznvlxguwsVl0WH EHfQ== X-Gm-Message-State: AOAM530cWeCjPJs8L8412GWlX3HR+qIgThW8Nmy8+VXFrwQAJau5CEEF QbVqjUoTUt24RystJf2FQOIwOg== X-Google-Smtp-Source: ABdhPJz8W8Su0dMgRtJl+pZAp/mpkXq1zLTGj9+4KRMpnLFRhc1kvnrG4AjiF5PHuOTBJmNzw56SIw== X-Received: by 2002:a05:600c:8a4:: with SMTP id l36mr8462323wmp.15.1617891037636; Thu, 08 Apr 2021 07:10:37 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id l6sm45911537wrt.56.2021.04.08.07.10.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Apr 2021 07:10:37 -0700 (PDT) Date: Thu, 8 Apr 2021 16:10:36 +0200 From: Olivier Matz 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 Message-ID: <20210408141036.GY1650@platinum> References: <20210330075036.6579-2-rasland@nvidia.com> <20210404074552.24190-1-rasland@nvidia.com> <20210404074552.24190-2-rasland@nvidia.com> <20210408122956.GX1650@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v4 1/2] ethdev: add new ext hdr for gtp psc X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Apr 08, 2021 at 12:37:27PM +0000, Raslan Darawsheh wrote: > Hi Olivier, > > > -----Original Message----- > > From: Olivier Matz > > Sent: Thursday, April 8, 2021 3:30 PM > > 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 > > 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 > > > --- > > > 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__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__hdr ; > > struct rte_gtp_psc__hdr ; > > }; > > }; >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