From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <bingz@mellanox.com>
To: Olivier Matz <olivier.matz@6wind.com>
CC: Ori Kam <orika@mellanox.com>, "john.mcnamara@intel.com"
<john.mcnamara@intel.com>, "marko.kovacevic@intel.com"
<marko.kovacevic@intel.com>, Thomas Monjalon <thomas@monjalon.net>,
"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
"arybchenko@solarflare.com" <arybchenko@solarflare.com>,
"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>,
"wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>, "beilei.xing@intel.com"
<beilei.xing@intel.com>, "bernard.iremonger@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: <VI1PR05MB4192DCA65199DAE241671DB5DD630@VI1PR05MB4192.eurprd05.prod.outlook.com>
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>
<VI1PR05MB419258E42CF4981A6D27113FDD620@VI1PR05MB4192.eurprd05.prod.outlook.com>
<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: <VI1PR0501MB2829C05913CE0F8615E3AE11DD630@VI1PR0501MB2829.eurprd05.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi Olivier,
Thanks
BR. Bing
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Sunday, July 12, 2020 9:18 PM
> To: Bing Zhao <bingz@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; 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
> > > >
> >