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 6C371A0528; Sat, 11 Jul 2020 06:25:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1AF4B1DABC; Sat, 11 Jul 2020 06:25:55 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2081.outbound.protection.outlook.com [40.107.21.81]) by dpdk.org (Postfix) with ESMTP id 2F9841D561 for ; Sat, 11 Jul 2020 06:25:53 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MnRkp7NYhwyAXpZCCJAuD5QhVyLP8FtmMakiIhhKApIXhpMa+st77ThcXocKzCk9yRKlCThlgFQq02vSqrz+LKY/ClSx6hs4xB4shzGSSCOtlZ7u3HKtFhkkzAaGKcadRspwNMqrGGAB4oPjSm4CTGPzQzd1lACcD0YR3jsTWSouT3pcCe2E/kZhNkuYbxcYTgSdvZqZcwYe2H/+0lJVwc0I0v2eGRWVpUTOXEHYF4FyCHSlvBy6I4vDbGIYfRyGENkalvSEFKyQFFCYIquPPfNOuPAPiAtk41rqeBovXTwqKn8W2ONwL/i3n0olhCpCmFwBkZgkT2yKscWmZ+dxAw== 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=9PBsghiBDWL2XzoVYFsA9+Z68OuXj389fQUEsdnTDrQ=; b=Ch31sg5ndkfzg78oeZom98LX67AP7Stuqf1MQUu+IucNr7urXaKdoORQtsCl/Nw+PENv4CF2rlYN0P3I14TOm8KiRhp6L3NuYf1tTMvJz2dprSGjCcife7V/KclqwfjBupv84WkaSddG5a7vrdML+fz4yJP47mySxWyI1XXv98JRM4fr7k1JbncDoErYjaZoOiHh9JfF+4Y49hB9FriBkmrhWI/xcMz7hyscN5Ty/7oX0a1TczkmeLczVmAF5yIejNpeSjknNoTt0v+zfe+3BXg2kLPs25jKn20KbW1xy6TzRhb90cXuQYzrAdCUBxtO8yk1PNPCfTh8NDBgBWxG7w== 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=9PBsghiBDWL2XzoVYFsA9+Z68OuXj389fQUEsdnTDrQ=; b=NUlwzsXG0XUTAQT0/Q8mX0fw8oC06Fvjf1kIjAiDfcbs+J8HPDuTt9z/vLx5Y73vEUCA2oU13p8d7k7IHoEzVh4AYO6VzaBhngtA7rrsJ/gPcRCwyE6vpyo9B1aU8MeLGr7sWN5iEpKwM33DSHciviPhKTynPAJUIByVMWXvKkM= Received: from VI1PR05MB4192.eurprd05.prod.outlook.com (2603:10a6:803:4e::18) by VI1PR05MB4829.eurprd05.prod.outlook.com (2603:10a6:803:54::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.20; Sat, 11 Jul 2020 04:25:50 +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.023; Sat, 11 Jul 2020 04:25:50 +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/bx2akBuAAw Date: Sat, 11 Jul 2020 04:25:49 +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> In-Reply-To: <20200710143123.GE5869@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: 584fd21e-34cf-4062-6e10-08d825527dcf x-ms-traffictypediagnostic: VI1PR05MB4829: 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:6108; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: kjKOsqX70jyqBI6c+r7nvMBkqd7MRRMnFnHIHmppM81yxp9TwA39mf6YYUOolsHlYvDaZaO/Y5UGHO0pqalsW6SWeDvfzG2thJ9JAdnajiPkkVRVsk/mWH2dmQwV+FK02UzuynLQEI9YN+o6VrwBhJ5Wllq/1nzRottB5Sm73I0L7avgvrsPmblFLU3+aYDasyrt5+RebBHVgPnDd9SZGfqoWi4rZxxLBUA8t5HnS72TGGb0w7Hzq4r7hRk08U19PPZvn0xF5rnFXOYZO2vwxovF+JWKguHySJ7Fc4De6Lo1hjz183u0aJy2JYUMEQ7Y+XzBIxOM8Q1KFJQXQUvlVozL5XN7vW5NRdlKBcPLihb8iO4wDIvxWnTfAw4vEw+23Hv/do5l7ubrERmzFR+UIg== 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)(39860400002)(346002)(376002)(396003)(366004)(136003)(33656002)(55016002)(15974865002)(83380400001)(64756008)(66946007)(66556008)(66476007)(76116006)(66446008)(9686003)(2906002)(6506007)(53546011)(8676002)(71200400001)(7696005)(8936002)(5660300002)(26005)(7416002)(6916009)(52536014)(30864003)(478600001)(45080400002)(4326008)(186003)(316002)(86362001)(966005)(54906003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: 3gkQ8nlAewjfqgMwC0fyyYgMv1XBD30PLagc2nOCzionJnKI87vpTzRDmXFFt4pzTdiKeqw2/bwBmqN9DnulvsqRyiQPL6BFsG2XLwODPYv7fnrqc0MZ0SZID20z4mEMZTvdNc3yQjBT4toDCWO/jagOyCBFIWDShKeFEim89AtJo+rom6eBXvh7S4s4OeNxlxME+1RNDDuurDtWYZTjCDvmoDHCIZ8mOZgJZTz14ZOke+vhpOEZj1kcPrRf+Tz5pURPionIfAXnQ1ibim3TeDlxG9hpRVxuIPMKcEYOFTCttJ+hAPnBK3fA4USNGrz45JwK99ZT5PR8zK/z6Dc5veolaJAKmI+ufSvz5xsEQvVFw7xATXQunzDpN/CJkZsDX+QxZhbXl4nhY+iwrpVGxNmrY79Co6wzmq9DZrYGSFXSAlhjZHSqpqSqlfc5WWgKRTJDs2JCJtHI9a8OFkByMGYH8ncLP0vYlyTyYay63Qs= 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: 584fd21e-34cf-4062-6e10-08d825527dcf X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jul 2020 04:25:49.9688 (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: hDSbusZV92W03oPjBizjwuyaNyTe5TyIHD4deA1/vlqiO8lOJph07wcpxnd4S/VSY88VNPihcpyFxJPStifedg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB4829 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, Many thanks for your comments. > -----Original Message----- > From: Olivier Matz > Sent: Friday, July 10, 2020 10:31 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 Fri, Jul 10, 2020 at 04:45:22PM +0800, Bing Zhao wrote: > > Add a new item "rte_flow_item_ecpri" in order to match eCRPI > header. > > > > eCPRI is a packet based protocol used in the fronthaul interface of 5G > > networks. Header format definition could be found in the > specification > > via the link below: > > > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2F > www. > > gigalight.com%2Fdownloads%2Fstandards%2Fecpri- > specification.pdf&da > > > ta=3D02%7C01%7Cbingz%40mellanox.com%7C99fe8c970b6648d82f7908 > d824dded33%7 > > > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C6372998828794 > 68155&sda > > > ta=3Dhv1rAa9I2%2BPpZkoLKzHdEpjrLyvY%2F4IiaEvW3XHnhW4%3D& > ;reserved=3D0 > > > > eCPRI message can be over Ethernet layer (.1Q supported also) or > over > > UDP layer. Message header formats are the same in these two > variants. > > > > Signed-off-by: Bing Zhao > > Acked-by: Ori Kam > > --- > > doc/guides/prog_guide/rte_flow.rst | 8 ++ > > doc/guides/rel_notes/release_20_08.rst | 5 + > > lib/librte_ethdev/rte_flow.c | 1 + > > lib/librte_ethdev/rte_flow.h | 31 ++++++ > > lib/librte_net/Makefile | 1 + > > lib/librte_net/meson.build | 3 +- > > lib/librte_net/rte_ecpri.h | 182 > +++++++++++++++++++++++++++++++++ > > lib/librte_net/rte_ether.h | 1 + > > 8 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 > > lib/librte_net/rte_ecpri.h > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst > > b/doc/guides/prog_guide/rte_flow.rst > > index d5dd18c..669d519 100644 > > --- a/doc/guides/prog_guide/rte_flow.rst > > +++ b/doc/guides/prog_guide/rte_flow.rst > > @@ -1362,6 +1362,14 @@ Matches a PFCP Header. > > - ``seid``: session endpoint identifier. > > - Default ``mask`` matches s_field and seid. > > > > +Item: ``ECPRI`` > > +^^^^^^^^^^^^^ > > + > > +Matches a eCPRI header. > > + > > +- ``hdr``: eCPRI header definition (``rte_ecpri.h``). > > +- Default ``mask`` matches message type of common header only. > > + > > Actions > > ~~~~~~~ > > > > diff --git a/doc/guides/rel_notes/release_20_08.rst > > b/doc/guides/rel_notes/release_20_08.rst > > index 988474c..19feb68 100644 > > --- a/doc/guides/rel_notes/release_20_08.rst > > +++ b/doc/guides/rel_notes/release_20_08.rst > > @@ -184,6 +184,11 @@ New Features > > which are used to access packet data in a safe manner. Currently > JIT support > > for these instructions is implemented for x86 only. > > > > +* **Added eCPRI protocol support in rte_flow.** > > + > > + The ``ECPRI`` item have been added to support eCPRI packet > > + offloading for 5G network. > > + > > * **Added flow performance test application.** > > > > Added new application to test ``rte_flow`` performance, including: > > diff --git a/lib/librte_ethdev/rte_flow.c > > b/lib/librte_ethdev/rte_flow.c index 1685be5..f8fdd68 100644 > > --- a/lib/librte_ethdev/rte_flow.c > > +++ b/lib/librte_ethdev/rte_flow.c > > @@ -95,6 +95,7 @@ struct rte_flow_desc_data { > > MK_FLOW_ITEM(HIGIG2, sizeof(struct > rte_flow_item_higig2_hdr)), > > MK_FLOW_ITEM(L2TPV3OIP, sizeof(struct > rte_flow_item_l2tpv3oip)), > > MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)), > > + MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)), > > }; > > > > /** Generate flow_action[] entry. */ > > diff --git a/lib/librte_ethdev/rte_flow.h > > b/lib/librte_ethdev/rte_flow.h index b0e4199..8a90226 100644 > > --- a/lib/librte_ethdev/rte_flow.h > > +++ b/lib/librte_ethdev/rte_flow.h > > @@ -28,6 +28,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -527,6 +528,15 @@ enum rte_flow_item_type { > > */ > > RTE_FLOW_ITEM_TYPE_PFCP, > > > > + /** > > + * Matches eCPRI Header. > > + * > > + * Configure flow for eCPRI over ETH or UDP packets. > > + * > > + * See struct rte_flow_item_ecpri. > > + */ > > + RTE_FLOW_ITEM_TYPE_ECPRI, > > + > > }; > > > > /** > > @@ -1547,6 +1557,27 @@ struct rte_flow_item_pfcp { #endif > > > > /** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior > notice > > + * > > + * RTE_FLOW_ITEM_TYPE_ECPRI > > + * > > + * Match eCPRI Header > > + */ > > +struct rte_flow_item_ecpri { > > + struct rte_ecpri_msg_hdr hdr; > > +}; > > + > > +/** Default mask for RTE_FLOW_ITEM_TYPE_ECPRI. */ #ifndef > __cplusplus > > +static const struct rte_flow_item_ecpri rte_flow_item_ecpri_mask =3D > { > > + .hdr =3D { > > + .dw0 =3D 0x0, > > + }, > > +}; > > +#endif > > + > > +/** > > * Matching pattern item definition. > > * > > * A pattern is formed by stacking items starting from the lowest > > protocol diff --git a/lib/librte_net/Makefile > > b/lib/librte_net/Makefile index aa1d6fe..9830e77 100644 > > --- a/lib/librte_net/Makefile > > +++ b/lib/librte_net/Makefile > > @@ -20,5 +20,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include > +=3D > > rte_sctp.h rte_icmp.h rte_arp.h > > SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=3D rte_ether.h > rte_gre.h > > rte_net.h SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=3D > rte_net_crc.h > > rte_mpls.h rte_higig.h SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include > +=3D > > rte_gtp.h rte_vxlan.h > > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=3D rte_ecpri.h > > > > include $(RTE_SDK)/mk/rte.lib.mk > > diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build > > index f799349..24ed825 100644 > > --- a/lib/librte_net/meson.build > > +++ b/lib/librte_net/meson.build > > @@ -15,7 +15,8 @@ headers =3D files('rte_ip.h', > > 'rte_net.h', > > 'rte_net_crc.h', > > 'rte_mpls.h', > > - 'rte_higig.h') > > + 'rte_higig.h', > > + 'rte_ecpri.h') > > > > sources =3D files('rte_arp.c', 'rte_ether.c', 'rte_net.c', > > 'rte_net_crc.c') deps +=3D ['mbuf'] diff --git > > a/lib/librte_net/rte_ecpri.h b/lib/librte_net/rte_ecpri.h new file > > mode 100644 index 0000000..60fb4f7 > > --- /dev/null > > +++ b/lib/librte_net/rte_ecpri.h > > @@ -0,0 +1,182 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright 2020 Mellanox Technologies, Ltd */ > > + > > +#ifndef _RTE_ECPRI_H_ > > +#define _RTE_ECPRI_H_ > > + > > +#include > > +#include > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +/** > > + * eCPRI Protocol Revision 1.0, 1.1, 1.2, 2.0: 0001b > > + * Other values are reserved for future */ > > +#define RTE_ECPRI_REV_UP_TO_20 1 > > + > > +/** > > + * eCPRI message types in specifications > > + * IWF* types will only be supported from rev.2 > > + * 12-63: Reserved for future revision > > + * 64-255: Vendor Specific > > + */ > > +#define RTE_ECPRI_MSG_TYPE_IQ_DATA 0 >=20 > Here the doxygen comment applies to > RTE_ECPRI_MSG_TYPE_IQ_DATA. >=20 > I think it should either be a standard comment (no doxygen), or > something more complex should be done, like grouping: > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2F > stackoverflow.com%2Fquestions%2F30803156%2Fgroup-level- > documentation-for-preprocessor-defines-in- > doxygen&data=3D02%7C01%7Cbingz%40mellanox.com%7C99fe8c9 > 70b6648d82f7908d824dded33%7Ca652971c7d2e4d9ba6a4d149256f46 > 1b%7C0%7C1%7C637299882879478151&sdata=3D0BGV9rUpTYPpKA > Thq9BgixZCtnlMVmILVty0f37xJ7I%3D&reserved=3D0 > (I didn't try) >=20 Thanks, I will have a try to fix this comments. >=20 > > +#define RTE_ECPRI_MSG_TYPE_BIT_SEQ 1 > > +#define RTE_ECPRI_MSG_TYPE_RTC_CTRL 2 > > +#define RTE_ECPRI_MSG_TYPE_GEN_DATA 3 > > +#define RTE_ECPRI_MSG_TYPE_RM_ACC 4 > > +#define RTE_ECPRI_MSG_TYPE_DLY_MSR 5 > > +#define RTE_ECPRI_MSG_TYPE_RMT_RST 6 > > +#define RTE_ECPRI_MSG_TYPE_EVT_IND 7 > > +#define RTE_ECPRI_MSG_TYPE_IWF_UP 8 > > +#define RTE_ECPRI_MSG_TYPE_IWF_OPT 9 > > +#define RTE_ECPRI_MSG_TYPE_IWF_MAP 10 > > +#define RTE_ECPRI_MSG_TYPE_IWF_DCTRL 11 > > + > > +/** > > + * Event Type of Message Type #7: Event Indication > > + * 0x00: Fault(s) Indication > > + * 0x01: Fault(s) Indication Acknowledge > > + * 0x02: Notification(s) Indication > > + * 0x03: Synchronization Request > > + * 0x04: Synchronization Acknowledge > > + * 0x05: Synchronization End Indication > > + * 0x06...0xFF: Reserved > > + */ > > +#define RTE_ECPRI_EVT_IND_FAULT_IND 0 > > +#define RTE_ECPRI_EVT_IND_FAULT_ACK 1 > > +#define RTE_ECPRI_EVT_IND_NTFY_IND 2 > > +#define RTE_ECPRI_EVT_IND_SYNC_REQ 3 > > +#define RTE_ECPRI_EVT_IND_SYNC_ACK 4 > > +#define RTE_ECPRI_EVT_IND_SYNC_END 5 > > + > > +/** > > + * 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; >=20 > Does it really need to be packed? Why next types do not need it? > It looks only those which have bitfields are. >=20 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.) U= sually, 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 > I wonder if the 'dw0' could be in this definition instead of in struct > rte_ecpri_msg_hdr? >=20 > Something like this: >=20 > struct rte_ecpri_common_hdr { > union { > uint32_t u32; > struct { > ... > }; > }; > }; >=20 > I see 2 advantages: >=20 > - 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; >=20 > 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 > > + > > +/** > > + * 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]; > > + }; > > +}; >=20 > What is the point in having this struct? >=20 > From a software point of view, I think it is a bit risky, because its siz= e 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. >=20 > Also, I'm not sure to understand the purpose of dummy[3], even after > reading your answer to Akhil's question. >=20 Basically YES and no. To my understanding, the eCPRI message format is some= thing like the ICMP packet format. The message (packet) itself will be parsed int= o 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 d= ivide 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 p= arts that datapath forwarding will only care about. The following timestamp or u= ser 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 i= n the SW, the common header + first several bytes in the payload should be enough. BUT YE= S, 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 t= he "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 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 si= ngle 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 addre= ss 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. Som= e checking should be done to confirm if it is wildcard flow (all eCPRI messages or eCPR= I message in some specific type), or some precise flow (to match the pc id or rtc id, for example). Wit= h these fields, 3 DW of the masks only need to be check before continuing. Or else, the co= de needs to check the type and a lot of switch-case conditions and go through all different memb= ers of each header. >=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 > >