From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DD3D5A053A; Sun, 12 Jul 2020 16:28:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5F5981C298; Sun, 12 Jul 2020 16:28:08 +0200 (CEST) Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr150041.outbound.protection.outlook.com [40.107.15.41]) by dpdk.org (Postfix) with ESMTP id 2797A1BFA4 for ; Sun, 12 Jul 2020 16:28:06 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JF8AaPSXuvy0K0Vb7s7uAdGaf63vU+AqolaKrf1UWjlaAqlur5Wmqv1xIByNk2wthSjphxI99yLA3LaoxMKRKVvU/OilVLWEUMv2dW95s+t3RwSFL58twsvcygEBJ64E4wjidMDj5tBgaBMz/rr/jU125/e1gkGlHOnEAsp9D0gPimBrc8S9jtuMtAKM/kdiiNrByKBd0M5NIejSzZmPnHL3jJpHOIuIQpFKqsKRy0JDWghKuZilf0C3DgorufxZyE6VZBWgZLDUYjs+ZdFcFn/WfV2Bdr+/70wMRHdYlQdG0BGACc3VHj/XC60EF0SEFk3xN4vLQ/G/KTD9nGkrvg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8GeKhJesu+XtjKvK4xM9rFi21NWNhGNpXKYkHj79qyY=; b=P+wCQGZFSbrdA25eAKBlrtNiDWGbPOfvUhp93NQuA65oy9ZWemRrZr28fMNYiVJNw6MLM2xGckXpTv6pYV0io4+3HsL9o6GWIBz2Ct+80tkvyBrvIwhKikTm2ujkGHAjvFPkxG9eWtwSQH2A6JSFjC2KCyA3HnjX/7lx23W4AbTA3zeYqSeDXF7zweqzroYduvBd3VSZ2hRoUPVw8rb9DL+oYhQ3tDiCF+4c9mThTozdOEFC13p7CCVJbc3cwdIngQIUiJsDzdbuyNNjLkt0+XC1xC7Y2U/LsMaflZyEDhOMF010pOW3pCr0Y9dPpJe4RDZzVMCdYvNsz3qwzaOpZQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8GeKhJesu+XtjKvK4xM9rFi21NWNhGNpXKYkHj79qyY=; b=BlR8HgXsnkI+6JGuTIxQy2maG5eQnrwBbQWyk6hu5ElcjOLSdFNZtJg/JK7Xo1217HkoFDsnVyIuukop/PGVpP17ye00QtBjI/QgDa65N2uFFbPiq9bU+YghDM+e8WiblwTvbyY5eBL8DrZv7Ty3M0GEh3xUDbVyhw1kPyHguA0= Received: from VI1PR05MB4192.eurprd05.prod.outlook.com (2603:10a6:803:4e::18) by VI1PR0501MB2829.eurprd05.prod.outlook.com (2603:10a6:800:a0::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.21; Sun, 12 Jul 2020 14:28:03 +0000 Received: from VI1PR05MB4192.eurprd05.prod.outlook.com ([fe80::f53d:1b8c:d023:c5d0]) by VI1PR05MB4192.eurprd05.prod.outlook.com ([fe80::f53d:1b8c:d023:c5d0%7]) with mapi id 15.20.3174.025; Sun, 12 Jul 2020 14:28:03 +0000 From: Bing Zhao To: Olivier Matz CC: Ori Kam , "john.mcnamara@intel.com" , "marko.kovacevic@intel.com" , Thomas Monjalon , "ferruh.yigit@intel.com" , "arybchenko@solarflare.com" , "akhil.goyal@nxp.com" , "dev@dpdk.org" , "wenzhuo.lu@intel.com" , "beilei.xing@intel.com" , "bernard.iremonger@intel.com" Thread-Topic: [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API Thread-Index: AQHWVsbM+Y3elBsbFESaZsWSJ/bx2akBuAAwgAI4FQCAAAvusA== Date: Sun, 12 Jul 2020 14:28:03 +0000 Message-ID: References: <1594136219-133336-1-git-send-email-bingz@mellanox.com> <1594370723-343354-1-git-send-email-bingz@mellanox.com> <1594370723-343354-2-git-send-email-bingz@mellanox.com> <20200710143123.GE5869@platinum> <20200712131740.GI5869@platinum> In-Reply-To: <20200712131740.GI5869@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: 6wind.com; dkim=none (message not signed) header.d=none;6wind.com; dmarc=none action=none header.from=mellanox.com; x-originating-ip: [183.157.60.15] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 8dffbc1c-2a74-4fe6-5f0c-08d8266fc981 x-ms-traffictypediagnostic: VI1PR0501MB2829: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: c7iT97+J+5Aw2NjR41L67SGaZSqvq0GZ1YeR8UzYE/WpMlUKyAznnxqkqM7rT0mZNorqJjH4FDmYyOBfYaokTcyWaGEWxH83kzyH4T6g5lOzunXmEnlifdTC0eaZT0ugSrfDIbvtc4Yod/uJeQJgd7cr9+7d/m6Rf8tzETr275HprdpOxRjk9dK/LvdDqYl+k0J2YIHSTgGXYReEt2e1IjbtHRxy/QWIDjD6vw/wCtPuPAlkxJF8d+xMIEZh+bTU2CefAN+UaRtX6a5RkiV6fKAA++vLVInWtwNFA5Bx3dYSJohHUqrcPWfHRfYfuZMjRR7o7Zfh1sa73x6S3vPmRoj0FAfK915UjbuYZgOr+TFtSxEggPbekZDxGoo4x1eiTAh3z9V3a9hzNdM1APUCrw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR05MB4192.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(136003)(396003)(39860400002)(366004)(346002)(376002)(8676002)(52536014)(83380400001)(76116006)(66476007)(66946007)(66446008)(64756008)(66556008)(7416002)(30864003)(6916009)(478600001)(966005)(55016002)(316002)(53546011)(54906003)(33656002)(5660300002)(9686003)(2906002)(7696005)(8936002)(6506007)(4326008)(86362001)(26005)(71200400001)(186003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: 4qWtChv+OsESqzurtYqVG5nglylNLnP2trKUDHyEQdrPeb98H2471guolnXjPk49e/PaU5x4gobYMBfwUe19ObeMJrgQI4Q/0S2K5ScWyJ3+bHVwa+w1GgEP3zk6bNdej2vN5G8YEqoATmy/28f+RBi2VYISCRgH8F0GQjiUXEIToTzReojQ/VlN7euBUVG0K8FdMR9t/mhrxSB0Q5hV8V+QKKX80Ac96fmnqv1MV+q5L6AvePJsUosa2Wl1zkICZnt9nSGtUJka508ruR2/VWMblApb/lXkXFfbSUdSuWWw98gZxsyIGBjD0yEuHnJzKi385bPIq/dRNPnk6pzfE4LoIlQ+h/XSCxJetCQsKO/TQ13yL7YLjgds4gghmkNM6qw/U7A2FuzYi4e38JfRpuCWaoiSLcD0RV+ReTqrH8LJzgIzKg5UUqa4mK1kC4eVtgMFZNjj99a+4a6XLJLnKrPGNkUXw2S298vTOfxi4nc= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR05MB4192.eurprd05.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8dffbc1c-2a74-4fe6-5f0c-08d8266fc981 X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jul 2020 14:28:03.4640 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Q2W56X06IfrYk44DNS+plxQb8UUiHDeGYRd6DJRPrYK6hjbo5qHoXmQ8rRVFWox4zsZta+t5JnALCcje+bNURA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0501MB2829 Subject: Re: [dpdk-dev] [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi Olivier, Thanks BR. Bing > -----Original Message----- > From: Olivier Matz > Sent: Sunday, July 12, 2020 9:18 PM > To: Bing Zhao > Cc: Ori Kam ; john.mcnamara@intel.com; > marko.kovacevic@intel.com; Thomas Monjalon > ; ferruh.yigit@intel.com; > arybchenko@solarflare.com; akhil.goyal@nxp.com; dev@dpdk.org; > wenzhuo.lu@intel.com; beilei.xing@intel.com; > bernard.iremonger@intel.com > Subject: Re: [PATCH v5 1/2] rte_flow: add eCPRI key fields to flow API >=20 > Hi Bing, >=20 > On Sat, Jul 11, 2020 at 04:25:49AM +0000, Bing Zhao wrote: > > Hi Olivier, > > Many thanks for your comments. >=20 > [...] >=20 > > > > +/** > > > > + * eCPRI Common Header > > > > + */ > > > > +RTE_STD_C11 > > > > +struct rte_ecpri_common_hdr { > > > > +#if RTE_BYTE_ORDER =3D=3D RTE_LITTLE_ENDIAN > > > > + uint32_t size:16; /**< Payload Size */ > > > > + uint32_t type:8; /**< Message Type */ > > > > + uint32_t c:1; /**< Concatenation Indicator > > > */ > > > > + uint32_t res:3; /**< Reserved */ > > > > + uint32_t revision:4; /**< Protocol Revision */ > > > > +#elif RTE_BYTE_ORDER =3D=3D RTE_BIG_ENDIAN > > > > + uint32_t revision:4; /**< Protocol Revision */ > > > > + uint32_t res:3; /**< Reserved */ > > > > + uint32_t c:1; /**< Concatenation Indicator > > > */ > > > > + uint32_t type:8; /**< Message Type */ > > > > + uint32_t size:16; /**< Payload Size */ > > > > +#endif > > > > +} __rte_packed; > > > > > > Does it really need to be packed? Why next types do not need it? > > > It looks only those which have bitfields are. > > > > > > > Nice catch, thanks. For the common header, there is no need to use > the > > packed attribute, because it is a u32 and the bitfields will be > > aligned. > > I checked all the definitions again. Only " Type #4: Remote Memory > Access" > > needs to use the packed attribute. > > For other sub-types, "sub-header" part of the message payload will > get > > aligned by nature. For example, u16 after u16, u8 after u16, these > > should be OK. > > But in type #4, the address is 48bits wide, with 16bits MSB and 32bits > > LSB (no detailed description in the specification, correct me if > > anything wrong.) Usually, the 48bits address will be devided as this > > in a system. And there is no 48-bits type at all. So we need to define > two parts for it: 32b LSB follows 16b MSB. > > u32 after u16 should be with packed attribute. Thanks >=20 > What about using a bitfield into a uint64_t ? I mean: >=20 > struct rte_ecpri_msg_rm_access { > if RTE_BYTE_ORDER =3D=3D RTE_LITTLE_ENDIAN > ... > uint64_t length:16; /**< number of bytes > */ > uint64_t addr:48; /**< address */ > #else > ... > uint64_t addr:48; /**< address */ > uint64_t length:16; /**< number of bytes > */ > #endif > }; >=20 Thanks for your suggestion. https://stackoverflow.com/questions/10906238/warning-when-using-bitfield-wi= th-unsigned-char AFAIK (from this page), bitfields support only support bool and int. uint64= _t is some type of "long" and most of the compilers should support it. But I am not sure if it is a s= tandard implementation. >=20 > > > > > > > > I wonder if the 'dw0' could be in this definition instead of in > > > struct rte_ecpri_msg_hdr? > > > > > > Something like this: > > > > > > struct rte_ecpri_common_hdr { > > > union { > > > uint32_t u32; > > > struct { > > > ... > > > }; > > > }; > > > }; > > > > > > I see 2 advantages: > > > > > > - someone that only uses the common_hdr struct can use the .u32 > > > in its software > > > - when using it in messages, it looks clearer to me: > > > msg.common_hdr.u32 =3D value; > > > instead of: > > > msg.dw0 =3D value; > > > > > > What do you think? > > > > Thanks for the suggestion, this is much better, I will change it. > > Indeed, in my original version, no DW(u32) is defined for the header. > > After that, I noticed that it would not be easy for the static casting > > to a u32 from bitfield(the compiler will complain), and it would not > > be clear to swap the endian if the user wants to use this header. I > > added this DW(u32) to simplify the usage of this header. But yes, if I > > do not add it here, it would be not easy or clear for users who just > use this header structure. > > I will change it. Is it OK if I use the name "dw0"? >=20 > In my opinion, u32 is more usual than dw0. >=20 I sent patch set v6 with this change, thanks. > > > > > > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #0: IQ Data */ struct > > > > +rte_ecpri_msg_iq_data { > > > > + rte_be16_t pc_id; /**< Physical channel ID */ > > > > + rte_be16_t seq_id; /**< Sequence ID */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #1: Bit Sequence */ struct > > > > +rte_ecpri_msg_bit_seq { > > > > + rte_be16_t pc_id; /**< Physical channel ID */ > > > > + rte_be16_t seq_id; /**< Sequence ID */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #2: Real-Time Control Data */ > > > struct > > > > +rte_ecpri_msg_rtc_ctrl { > > > > + rte_be16_t rtc_id; /**< Real-Time Control Data ID > > > */ > > > > + rte_be16_t seq_id; /**< Sequence ID */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #3: Generic Data Transfer */ > > > struct > > > > +rte_ecpri_msg_gen_data { > > > > + rte_be32_t pc_id; /**< Physical channel ID */ > > > > + rte_be32_t seq_id; /**< Sequence ID */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #4: Remote Memory Access > */ > > > > +RTE_STD_C11 > > > > +struct rte_ecpri_msg_rm_access { > > > > +#if RTE_BYTE_ORDER =3D=3D RTE_LITTLE_ENDIAN > > > > + uint32_t ele_id:16; /**< Element ID */ > > > > + uint32_t rr:4; /**< Req/Resp */ > > > > + uint32_t rw:4; /**< Read/Write */ > > > > + uint32_t rma_id:8; /**< Remote Memory Access > > > ID */ > > > > +#elif RTE_BYTE_ORDER =3D=3D RTE_BIG_ENDIAN > > > > + uint32_t rma_id:8; /**< Remote Memory Access > > > ID */ > > > > + uint32_t rw:4; /**< Read/Write */ > > > > + uint32_t rr:4; /**< Req/Resp */ > > > > + uint32_t ele_id:16; /**< Element ID */ > > > > +#endif > > > > + rte_be16_t addr_m; /**< 48-bits address (16 MSB) > > > */ > > > > + rte_be32_t addr_l; /**< 48-bits address (32 LSB) */ > > > > + rte_be16_t length; /**< number of bytes */ > > > > +} __rte_packed; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #5: One-Way Delay > Measurement > > > */ > > > > +struct rte_ecpri_msg_delay_measure { > > > > + uint8_t msr_id; /**< Measurement ID */ > > > > + uint8_t act_type; /**< Action Type */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #6: Remote Reset */ struct > > > > +rte_ecpri_msg_remote_reset { > > > > + rte_be16_t rst_id; /**< Reset ID */ > > > > + uint8_t rst_op; /**< Reset Code Op */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header of Type #7: Event Indication */ struct > > > > +rte_ecpri_msg_event_ind { > > > > + uint8_t evt_id; /**< Event ID */ > > > > + uint8_t evt_type; /**< Event Type */ > > > > + uint8_t seq; /**< Sequence Number */ > > > > + uint8_t number; /**< Number of > > > Faults/Notif */ > > > > +}; > > > > + > > > > +/** > > > > + * eCPRI Message Header Format: Common Header + Message > > > Types */ > > > > +RTE_STD_C11 > > > > +struct rte_ecpri_msg_hdr { > > > > + union { > > > > + struct rte_ecpri_common_hdr common; > > > > + uint32_t dw0; > > > > + }; > > > > + union { > > > > + struct rte_ecpri_msg_iq_data type0; > > > > + struct rte_ecpri_msg_bit_seq type1; > > > > + struct rte_ecpri_msg_rtc_ctrl type2; > > > > + struct rte_ecpri_msg_bit_seq type3; > > > > + struct rte_ecpri_msg_rm_access type4; > > > > + struct rte_ecpri_msg_delay_measure type5; > > > > + struct rte_ecpri_msg_remote_reset type6; > > > > + struct rte_ecpri_msg_event_ind type7; > > > > + uint32_t dummy[3]; > > > > + }; > > > > +}; > > > > > > What is the point in having this struct? > > > > > > From a software point of view, I think it is a bit risky, because > > > its size is the size of the largest message. This is probably what > > > you want in your case, but when a software will rx or tx such > > > packet, I think they shouldn't use this one. My understanding is > > > that you only need this structure for the mask in rte_flow. > > > > > > Also, I'm not sure to understand the purpose of dummy[3], even > after > > > reading your answer to Akhil's question. > > > > > > > Basically YES and no. To my understanding, the eCPRI message > format is > > something like the ICMP packet format. The message (packet) itself > > will be parsed into different formats based on the type of the > common > > header. In the message payload part, there is no distinct definition > > of the "sub-header". We can divide them into the sub-header and > data parts based on the specification. > > E.g. physical channel ID / real-time control ID / Event ID + type are > > the parts that datapath forwarding will only care about. The > following > > timestamp or user data parts are the parts that the higher layer in > the application will use. > > 1. If an application wants to create some offload flow, or even > handle > > it in the SW, the common header + first several bytes in the payload > > should be enough. BUT YES, it is not good or safe to use it in the > higher layer of the application. > > 2. A higher layer of the application should have its own definition of > > the whole payload of a specific sub-type, including the parsing of the > user data part after the "sub-header". > > It is better for them just skip the first 4 bytes of the eCPRI message = or > a known offset. > > We do not need to cover the upper layers. >=20 > Let me explain what is my vision of how an application would use the > structures (these are completly dummy examples, as I don't know > ecpri protocol at all). >=20 > Rx: >=20 > int ecpri_input(struct rte_mbuf *m) > { > struct rte_ecpri_common_hdr hdr_copy, *hdr; > struct rte_ecpri_msg_event_ind event_copy, *event; > struct app_specific app_copy, *app; >=20 > hdr =3D rte_pktmbuf_read(m, 0, sizeof(*hdr), > &hdr_copy); > if (unlikely(hdr =3D=3D NULL)) > return -1; > switch (hdr->type) { > ... > case RTE_ECPRI_EVT_IND_NTFY_IND: > event =3D rte_pktmbuf_read(m, sizeof(*hdr), > sizeof(*event), > &event_copy); > if (unlikely(event =3D=3D NULL)) > return -1; > ... > app =3D rte_pktmbuf_read(m, sizeof(*app), > sizeof(*hdr) + sizeof(*event), > &app_copy); > ... >=20 > Tx: >=20 > int ecpri_output(void) > { > struct rte_ecpri_common_hdr *hdr; > struct rte_ecpri_msg_event_ind *event; > struct app_specific *app; >=20 > m =3D rte_pktmbuf_alloc(mp); > if (unlikely(m =3D=3D NULL)) > return -1; >=20 > app =3D rte_pktmbuf_append(m, sizeof(*data)); > if (app =3D=3D NULL) > ... > app->... =3D ...; > ... > event =3D rte_pktmbuf_prepend(m, sizeof(*event)); > if (event =3D=3D NULL) > ... > event->... =3D ...; > ... > hdr =3D rte_pktmbuf_prepend(m, sizeof(*hdr)); > if (hdr =3D=3D NULL) > ... > hdr->... =3D ...; >=20 > return packet_send(m); > } >=20 > In these 2 examples, we never need the unioned structure (struct > rte_ecpri_msg_hdr). >=20 > Using it does not look possible to me, because its size is corresponds to > the largest message, not to the one we are parsing/building. >=20 Yes, in the cases, we do not need the unioned structures at all. Since the common header is always 4 bytes, an application could use the sub-types structures started from an offset of 4 of eCPRI layer, as in your= example. This is in the datapath. My original purpose is for some "control path", ty= pically the flow (not sure if any other new lib implementation) API, then the union could be used there w/o treating the common header and message payload header in a separate way and then combine them together. In this case, only the first several bytes will be accessed and checked, there will be no chan= ge of message itself, and then just passing it to datapath for further handlin= g as in your example. > > I think some comments could be added here, is it OK? > > 3. Regarding this structure, I add it because I do not want to > > introduce a lot of new items in the rte_flow: new items with > > structures, new enum types. I prefer one single structure will cover > most of the cases (subtypes). What do you think? > > 4. About the *dummy* u32, I calculated all the "subheaders" and > choose > > the maximal value of the length. Two purposes (same as the u32 in > the common header): > > a. easy to swap the endianness, but not quite useful. Because some > parts are u16 and u8, > > and should not be swapped in a u32. (some physical channel ID > and address LSB have 32bits width) > > But if some HW parsed the header u32 by u32, then it would be > helpful. > > b. easy for checking in flow API, if the user wants to insert a flow. > Some checking should > > be done to confirm if it is wildcard flow (all eCPRI messages or > eCPRI message in some specific type), > > or some precise flow (to match the pc id or rtc id, for example). > With these fields, 3 DW > > of the masks only need to be check before continuing. Or else, th= e > code needs to check the type > > and a lot of switch-case conditions and go through all different > members of each header. >=20 > Thanks for the clarification. >=20 > I'll tend to say that if the rte_ecpri_msg_hdr structure is only useful f= or > rte_flow, it should be defined inside rte_flow. >=20 Right now, yes it will be used in rte_flow. But I checked the current imple= mentations of each item, almost all the headers are defined in their own protocol file= s. So in v6, I change the name of it, in order not to confuse the users of this API, wou= ld it be OK? Thanks > However, I have some fears about the dummy[3]. You said it could be > enlarged later: I think it is dangerous to change the size of a structure > that may be used to parse data (and this would be an ABI change). > Also, it seems dummy[3] cannot be used easily to swap endianness, so > is it really useful? >=20 It might be enlarger but not for now, until a new revision of this specific= ation. For all the subtypes it has right now, the specification will remain them as sa= me as today. Only new types will be added then. So after several years, we may consider = to change it in the LTS. Is it OK? In most cases, the endianness swap could be easy, we will swap it in each D= W / u32. Tome the exception is that some field crosses the u32 boundary, like the address= in type#4, we may treat it separately. And the most useful case is for the mask checking, it = could simplify the checking to at most 3 (u32=3D=3D0?) without going through each member of di= fferent types. And v6 already sent, I change the code based on your suggestion. Would you = please help to give some comments also? Appreciate for your help and suggestion. >=20 > Thanks, > Olivier >=20 >=20 > > > > + > > > > +#ifdef __cplusplus > > > > +} > > > > +#endif > > > > + > > > > +#endif /* _RTE_ECPRI_H_ */ > > > > diff --git a/lib/librte_net/rte_ether.h > > > > b/lib/librte_net/rte_ether.h index 0ae4e75..184a3f9 100644 > > > > --- a/lib/librte_net/rte_ether.h > > > > +++ b/lib/librte_net/rte_ether.h > > > > @@ -304,6 +304,7 @@ struct rte_vlan_hdr { #define > > > RTE_ETHER_TYPE_LLDP > > > > 0x88CC /**< LLDP Protocol. */ #define RTE_ETHER_TYPE_MPLS > > > 0x8847 /**< > > > > MPLS ethertype. */ #define RTE_ETHER_TYPE_MPLSM 0x8848 > /**< > > > MPLS > > > > multicast ethertype. */ > > > > +#define RTE_ETHER_TYPE_ECPRI 0xAEFE /**< eCPRI ethertype > (.1Q > > > > +supported). */ > > > > > > > > /** > > > > * Extract VLAN tag information into mbuf > > > > -- > > > > 1.8.3.1 > > > > > >