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 CEBEDA0C47; Tue, 12 Oct 2021 08:42:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B2C0740E01; Tue, 12 Oct 2021 08:42:24 +0200 (CEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2078.outbound.protection.outlook.com [40.107.243.78]) by mails.dpdk.org (Postfix) with ESMTP id 7E44A4067C for ; Tue, 12 Oct 2021 08:42:22 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L20rCED/LeYMzM4iAEAq08oo88lYPFenzylsW6fUMKvHsbebTHTBlLdtf3xzrHJ/aNBLkDL9oBFqBmK2Wh03ngM1lnL7GQTkr27VnOCcZ7jNCuroQkDUmebHvmr8eh5tVfORRSCNYedEgLF5q7EdtnB8qYbxlG70LJo1uCu6I1tfvw/ZiPEjrCh7v6a/2gROi6jV34qjwHMS6rify0l7FauWQr5Hlu70uFo0mVGEhDXuhCyab+6WOuUhUgOeXwHIsDWcNEomr/OygFiegpD+eS7uLb6XwvSLtLAVtbWUGm1U4dqGv0B92jt74Pl3+CzRjKFe+qI28tK+w942giRg5Q== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fwrRqc9PlcAitdCXg6NiU5W860DXugtNXDCFaRo55L4=; b=a9pFMQHx0lgcbRb8j4VDUbCQoED1YoV+YOmRqNDFy7rl0SI88B35jXzt/zhZScKZzW+QKPk4qcOjuOM2N93dNUaeayhC7Mbgo8chHEwX8mPR+H7TlxbspZaqlr+J5l18lukF44Z0Lwa32Fp+bXy9sqb9BtyaVoAckZiMNSsE5M+kfJf92LM9h2ZE5sTswCruNC8jDOwNYmSKMctfblEyjpC7JJxSw+rBqnRB1+0nNHFzwTdxqN3/nWhO6EWRMhV2kDtgXELbo9MrEfQzPKiP8tq5YRzuSP80oknRsER6+Pk4u6fbmpkP+Jqcs0w9YiQPuiQWFHal5WZAvlnsCjplFg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=fwrRqc9PlcAitdCXg6NiU5W860DXugtNXDCFaRo55L4=; b=fZr/3AnyzSMTVwkbpIwGjsSAfJVE+wKYm5ymEeym6kLVUHkdO/Rz/Z6crHBxTOLpkslpNMZ5lbSBJLcXNsCr+oI04AakJ2VAmROb16viaM6OXAMWdr2Pb4zEpgrilKdmwwRAt+zV3ISyC7GWc1BxUCGugEW1PJg88NFMdkgqvjXQWmKUnHTaPsBgocU7bEOc3HBpmWQGLUXUMf4jmpShx56u9IjbYizwqCo3luWkEJeofmGOWB6+Nj3uo20/VGN/u0InaZCFeRIORsK+nuz3HJIH5Wsc9N8KxAxQJ/EbL8fLHzcnSCyLx/eRaoUbu2f9VBuzqHs88uRk3ahxZN3HeA== Received: from DM6PR12MB3753.namprd12.prod.outlook.com (2603:10b6:5:1c7::18) by DM6PR12MB4371.namprd12.prod.outlook.com (2603:10b6:5:2a3::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4587.19; Tue, 12 Oct 2021 06:42:20 +0000 Received: from DM6PR12MB3753.namprd12.prod.outlook.com ([fe80::e550:35a2:96e5:657f]) by DM6PR12MB3753.namprd12.prod.outlook.com ([fe80::e550:35a2:96e5:657f%4]) with mapi id 15.20.4587.026; Tue, 12 Oct 2021 06:42:20 +0000 From: Slava Ovsiienko To: Ori Kam , "dev@dpdk.org" CC: Raslan Darawsheh , Matan Azrad , Shahaf Shuler , Gregory Etelson , NBU-Contact-Thomas Monjalon Thread-Topic: [PATCH v2 01/14] ethdev: introduce configurable flexible item Thread-Index: AQHXu2ul0Vp3eu7eyUqXd3yoMc9zFavNSyTg Date: Tue, 12 Oct 2021 06:42:20 +0000 Message-ID: References: <20210922180418.20663-1-viacheslavo@nvidia.com> <20211001193415.23288-1-viacheslavo@nvidia.com> <20211001193415.23288-2-viacheslavo@nvidia.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: fa784b52-bf23-48a1-b750-08d98d4b70d3 x-ms-traffictypediagnostic: DM6PR12MB4371: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: et+bbjgzpmSrItOACjN1Aep68w2CcjaHWqyyeUSM6uZuO7aQSnugBNyIAVQMc0mz327OHvshKaQtaJJGKhP848WfLu6CeC1yA9O479pmOovSMfClK7FYs3Hm7qoBBa0xWm7EbrzHUJdAu2gVqxSleEGL1916WbUgKc2dLxRElOOMZVhTtcxEgkWQSoRkepXPWzFNCc4eBtTpVI/w/08d+p4wpZsy+78R6aPZXxsjbOAc82cDoF2zLCcY6dNmq3W3eJeuQKF2TAwad9z+raeohvyaDzSwU6jDFSzrfijuMD7fGwGoKO0f14JltnnmfYAIFDYDEZ2Qw1DPhMAxCfVFL2tqHA3vnspxHpM/d4ST6sBBODfgaJVK6XAqWqagJWt8oRfvpMciwwCC07NEU9WwomHe08hLhXBSTHlRBlQ4vEU8dl0DnWzpt8jSiFSMqdScAa2w00BcN+G0PrVc+2Me6HGv0XLIv3tDDcXAOCshQR2sJBoHnEXlPWdsVS5jEOd8nOUriVeIIX2T1R0aJCG/9e2krvQKRyG8oSkoDdOw89lOHbYxt6BJEQjxy9TSry1jUsceNCmrtDdcqOUwVMF8uX+5mZvOvYC8FU8mJUXKEZ3gNjzGpNDdfO9a8QO8Ne6My+4EkEukz6RJ2GUfRQzTAfiRl3hmLi/miOenURczmyos/tqQdRsAUurZ+Xp91rDU1O1jrszW7PKcCGq4TM1Opw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR12MB3753.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(38070700005)(76116006)(66556008)(8676002)(66476007)(86362001)(316002)(122000001)(66946007)(55016002)(8936002)(110136005)(508600001)(4326008)(5660300002)(33656002)(71200400001)(6506007)(7696005)(52536014)(2906002)(30864003)(53546011)(54906003)(66446008)(9686003)(186003)(26005)(38100700002)(83380400001)(64756008)(559001)(579004); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UZjJJ7BVm64q7lzzfY7r0kqSjLF8NZWiYaajnfnlAZHXqWm7zpra3fdSLNcG?= =?us-ascii?Q?ZGWM6y4M6CWWmuVjU0igdA2ciYi3pdWeHICSZIspbUcDV0QAwZD/K6wopABV?= =?us-ascii?Q?Fe2SWGsxR7GGJyJYf9iuN7vgeyznaoYKZlCb/eJf2deW1VpF5/8iJnxUY/D/?= =?us-ascii?Q?PNsrlpJsfjqFLoouB9tjEi5IC1AhzO0o3Z6RQM/gXhS/WhvdnCSrF9qaUgiW?= =?us-ascii?Q?dtM9BhC8SBSUe7agoLsNsY5GvSQ2/yiWDc2WicgIvhebESdonNKTNHVaWOGZ?= =?us-ascii?Q?wiCABlQGiNI1DxCWmbohr0z0KZmAX2cf0H6aY7C1LAM+1XnlevAvQckiIU2A?= =?us-ascii?Q?lsnJ6WN+9EfC1ZWs1uQBZEqlCA+bbiBT4CR+P41DLp5FuE7754VpOZXsoZ/j?= =?us-ascii?Q?Wl0dFe9NidlfAbmgNF58SwozfOlXQ4Krvpe4CSeWlFmFjdQuyO5mxfgS0p9x?= =?us-ascii?Q?8XKynPxK+/7jbSxrHlmKmrG3rEfx7fY1S2ZAHRmX5eTk6fUmkTewjbzUEfPA?= =?us-ascii?Q?zNmfCu1JvvPMaENhA0c8TEZ0L4cG3DxJoodYw6sj44sCZ3Vr6O6rVKpyNV30?= =?us-ascii?Q?1qBjT1Ci05btvAgc9k3fGJd3IebcivVKyxzBzBayYpP2leGwLHiTPh2aQwB6?= =?us-ascii?Q?tqraZe2tFtJBYoShUq75YMWHVuj/wEB0dWD2wMXSZSvL0d3Ox9eivheYJF/Y?= =?us-ascii?Q?w9MdYipSW1HpSM0fxT/5pG4HVD+5+nlymx7FG5bfdFlrViOmWa66fM+jnzd0?= =?us-ascii?Q?6WXo54y+ChYzl11i76K9R3KpmABe/6mEw5xYmyQ87N9vhBmHwLiSB0XgyWan?= =?us-ascii?Q?0mset9kVEmL16rdiVLUXDoXUacmKO2npWYHNSz5boiIcpioOzwYGq9LdenBR?= =?us-ascii?Q?Rn6muOBJw36A9ht65J7X2K7LW4zC0npPmKWkYFXjDnZ6i0YUCXm0K1QFYKjJ?= =?us-ascii?Q?nGS/Tv23BWVGqLRQ7uwjXM3bQ0vfMRXQYHkQ1vldB14/xEAQoyuosmZ2mYNT?= =?us-ascii?Q?u19f8bL0fZ403p47DuzQY5lofC7bPPNOpma0g4thldVeXBNkq0/VyhT1lXht?= =?us-ascii?Q?vATQIDLbk6g55Bso+Pm1KffAveLbs1ID303c4q+kRVCDfC+utpzcZg2b6jE0?= =?us-ascii?Q?0EJdlYefXgIchZlGpmOQcvCUcWSy1y6002lzMBgKU0Qiyn+zgn2DJPATlrc/?= =?us-ascii?Q?mViWDrmPnlBQmwvedwNSK7AwD9XF2mAtuYJKRPM6plLQ1Zg+5fG5qgKsGNwi?= =?us-ascii?Q?R+1paPfNf4S9DcRGZABRhyeLVtkAR5vev15rK2RrB9Qyk1QICDpFgg1dF/zG?= =?us-ascii?Q?vjo=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3753.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: fa784b52-bf23-48a1-b750-08d98d4b70d3 X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Oct 2021 06:42:20.2553 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: y+cY12jTOYHwExXEFmvMG/fNWmssY+i0C7kUC83apKPwAqACFvIqLsJ5UWP01x7bqtsj6kVmxSPzuDUgOQz8Cg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4371 Subject: Re: [dpdk-dev] [PATCH v2 01/14] ethdev: introduce configurable flexible item 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" Hi, Ori Thank you very much for the review, I found some of your comment extremely = useful. Please, see below > -----Original Message----- > From: Ori Kam > Sent: Thursday, October 7, 2021 14:08 > To: Slava Ovsiienko ; dev@dpdk.org > Cc: Raslan Darawsheh ; Matan Azrad > ; Shahaf Shuler ; Gregory Etelson > ; NBU-Contact-Thomas Monjalon > > Subject: RE: [PATCH v2 01/14] ethdev: introduce configurable flexible ite= m >=20 > Hi Slava, >=20 > > -----Original Message----- > > From: Slava Ovsiienko > > Sent: Friday, October 1, 2021 10:34 PM > > Subject: [PATCH v2 01/14] ethdev: introduce configurable flexible item > > > > 1. Introduction and Retrospective .. snip .. > > > > The length field specifies the pattern buffer length in bytes and is > > needed to allow rte_flow_copy() operations. The approach of multiple > > pattern pointers and lengths (per field) was considered and found > > clumsy - it seems to be much suitable for the application to maintain > > the single structure within the single pattern buffer. > > >=20 > I think that the main thing that is unclear to me and I think I understan= d it > from reading the code is that the pattern is the entire flex header struc= ture. > maybe a better word will be header? "pattern is the entire flex header structure" - it is not completely correc= t, sorry Pattern represents the set of fields of the header. Yes, usually it coincid= es with the entire header (it is just simpler for understanding). But it must = not! The flex item can be constructed in more generic way. It can include fields in arbitrary order (in general, we may not follow the strict field order in= the header while defining the flex item), the field can be split into subfields and reordered. Theoretically it even allows to do many interesting things, say the byte order conversion - item will sample byte subfields into single integer field in desired host endianness.=20 Other possibility is to gather some split fields (say we have some offset split into multiple locations in the header) into one. Yes, for this case pattern will not correspond the header structure 1-to-1, but it is not required. 1-to-1 pattern-to-header mapping is just most straightforward way to operate, but it is not the only possible one. > In the beginning I thought that you should only give the matchable fields= . > also you say everything is in bits and suddenly you are talking in bytes. Yes, data are presented as bytes. But we must provide the capability to operate with bitfields. If we introduced some byte alignment for bitfields in the pattern we would lose the opportunity to match with header structure (that might define bitfields) strictly. That's why all offsets in flex config are expressed in bits, it provides the precise control over pattern structure. >=20 > > 4. Flex Item Configuration > > > > The flex item configuration consists of the following parts: > > > > - header field descriptors: > > - next header > > - next protocol > > - sample to match > > - input link descriptors > > - output link descriptors > > > > The field descriptors tell driver and hardware what data should be > > extracted from the packet and then presented to match in the flows. > > Each field is a bit pattern. It has width, offset from the header > > beginning, mode of offset calculation, and offset related parameters. > > >=20 > I'm not sure your indentation is correct for the next header, next protoc= ol, > sample to match. > Since reading the first line means that all fields are going to be matche= d while > in following sections only the sample to match are matchable. "reading the first line means". M-m-m-m, sorry I do not follow. The first line is "- header field descriptors". It tells what field descrip= tors we have. It tells nothing about match. All indented bullets have the same structure = type. Indentation is correct, but the claim=20 The field descriptors tell driver .... then presented to match in the flows= " Is not. So - agree, fixed. >=20 > > The next header field is special, no data are actually taken from the > > packet, but its offset is used as pointer to the next header in the > > packet, in other word the next header offset specifies the size of the > > header being parsed by flex item. > > >=20 > So the name of the next header should be len? We considered using the naming "len". We even started the code development with this one. But there is some level of indirection. The header length can be obtained with indirect methods (offset or bitmask field in the packe= t), and this field descriptor provides rather some "pointer/offset to next head= er" than length itself. So, naming next_header (pointer) is more precise, in my opinion. I would prefer to keep this. >=20 > > There is one more special field - next protocol, it specifies where > > the next protocol identifier is contained and packet data sampled from > > this field will be used to determine the next protocol header type to > continue packet parsing. > > The next protocol field is like eth_type field in MAC2, or proto field > > in IPv4/v6 headers. > > > > The sample fields are used to represent the data be sampled from the > > packet and then matched with established flows. >=20 > Should this be samples? ? IIUC - "sample" is adjective here, "fieldS" is plural. >=20 > > > > There are several methods supposed to calculate field offset in > > runtime depending on configuration and packet content: > > > > - FIELD_MODE_FIXED - fixed offset. The bit offset from > > header beginning is permanent and defined by field_base > > configuration parameter. > > > > - FIELD_MODE_OFFSET - the field bit offset is extracted > > from other header field (indirect offset field). The > > resulting field offset to match is calculated from as: > > > > field_base + (*field_offset & offset_mask) << field_shift > > >=20 > Not all of those fields names are defined later in this patch, and I'm no= t sure > about what they mean. Yes, sorry, missed this fix in commit message once code was updated. > Does * means take the value this is in field_offset? Yes, it means we should calculate indirect field offset, and extract field data from the packet (like by pointer *p from memory). Added the note about this. > How do we know the width of the field (by the value of the mask)? By mask, it is common and advanced way to specify the field. >=20 > > This mode is useful to sample some extra options following > > the main header with field containing main header length. > > Also, this mode can be used to calculate offset to the > > next protocol header, for example - IPv4 header contains > > the 4-bit field with IPv4 header length expressed in dwords. > > One more example - this mode would allow us to skip GENEVE > > header variable length options. > > > > - FIELD_MODE_BITMASK - the field bit offset is extracted > > from other header field (indirect offset field), the latter > > is considered as bitmask containing some number of one bits, > > the resulting field offset to match is calculated as: > > > > field_base + bitcount(*field_offset & offset_mask) << field_shift >=20 > Same comment as above you are using name that are not defined later. Yes, fixed. >=20 > > > > This mode would be useful to skip the GTP header and its > > extra options with specified flags. > > > > - FIELD_MODE_DUMMY - dummy field, optionally used for byte > > boundary alignment in pattern. Pattern mask and data are > > ignored in the match. All configuration parameters besides > > field size and offset are ignored. > > > > The offset mode list can be extended by vendors according to hardware > > supported options. > > > > The input link configuration section tells the driver after what > > protocols and at what conditions the flex item can follow. > > Input link specified the preceding header pattern, for example for > > GENEVE it can be UDP item specifying match on destination port with > > value 6081. The flex item can follow multiple header types and > > multiple input links should be specified. At flow creation type the > > item with one of input link types should precede the flex item and > > driver will select the correct flex item settings, depending on actual = flow > pattern. > > > > The output link configuration section tells the driver how to continue > > packet parsing after the flex item protocol. > > If multiple protocols can follow the flex item header the flex item > > should contain the field with next protocol identifier, and the > > parsing will be continued depending on the data contained in this field= in > the actual packet. > > > > The flex item fields can participate in RSS hash calculation, the > > dedicated flag is present in field description to specify what fields > > should be provided for hashing. > > > > 5. Flex Item Chaining > > > > If there are multiple protocols supposed to be supported with flex > > items in chained fashion - two or more flex items within the same flow > > and these ones might be neighbors in pattern - it means the flex items = are > mutual referencing. > > In this case, the item that occurred first should be created with > > empty output link list or with the list including existing items, and > > then the second flex item should be created referencing the first flex = item as > input arc. > > >=20 > And then I assume we should update the output list. It is supposed to be done by driver on creation the second item. Now update API is not supported (it depends on FW, and now there is no plans to support object modify), so - no support - we should not include the code, and rte_flow_flex_item_update() will be missing, at least in this release. And, currently there is no code supporting chaining, it is just an attempt to consider the potential scenario of flex item chaining and to get as complete API as we can. >=20 > > Also, the hardware resources used by flex items to handle the packet > > can be limited. If there are multiple flex items that are supposed to > > be used within the same flow it would be nice to provide some hint for > > the driver that these two or more flex items are intended for simultane= ous > usage. > > The fields of items should be assigned with hint indices and these > > indices from two or more flex items should not overlap (be unique per > > field). For this case, the driver will try to engage not overlapping > > hardware resources and provide independent handling of the fields with > > unique indices. If the hint index is zero the driver assigns resources = on its > own. > > > > 6. Example of New Protocol Handling > > > > Let's suppose we have the requirements to handle the new tunnel > > protocol that follows UDP header with destination port 0xFADE and is > > followed by MAC header. Let the new protocol header format be like this= : > > > > struct new_protocol_header { > > rte_be32 header_length; /* length in dwords, including options */ > > rte_be32 specific0; /* some protocol data, no intention */ > > rte_be32 specific1; /* to match in flows on these fields */ > > rte_be32 crucial; /* data of interest, match is needed */ > > rte_be32 options[0]; /* optional protocol data, variable length = */ > > }; > > > > The supposed flex item configuration: > > > > struct rte_flow_item_flex_field field0 =3D { > > .field_mode =3D FIELD_MODE_DUMMY, /* Affects match pattern only */ > > .field_size =3D 96, /* three dwords from the beginni= ng */ > > }; > > struct rte_flow_item_flex_field field1 =3D { > > .field_mode =3D FIELD_MODE_FIXED, > > .field_size =3D 32, /* Field size is one dword */ > > .field_base =3D 96, /* Skip three dwords from the beginning *= / > > }; > > struct rte_flow_item_udp spec0 =3D { > > .hdr =3D { > > .dst_port =3D RTE_BE16(0xFADE), > > } > > }; > > struct rte_flow_item_udp mask0 =3D { > > .hdr =3D { > > .dst_port =3D RTE_BE16(0xFFFF), > > } > > }; > > struct rte_flow_item_flex_link link0 =3D { > > .item =3D { > > .type =3D RTE_FLOW_ITEM_TYPE_UDP, > > .spec =3D &spec0, > > .mask =3D &mask0, > > }; > > > > struct rte_flow_item_flex_conf conf =3D { > > .next_header =3D { > > .field_mode =3D FIELD_MODE_OFFSET, > > .field_base =3D 0, > > .offset_base =3D 0, > > .offset_mask =3D 0xFFFFFFFF, > > .offset_shift =3D 2 /* Expressed in dwords, shift left by 2 */ > > }, > > .sample =3D { > > &field0, > > &field1, > > }, >=20 > Why in sample you give both fields? > by your decision we just want to match on field1. Field0 is a placeholder, it covers the gap in pattern and makes sure the pattern has exactly the same format as the protocol header. As option (by application design choice) we can omit field0 and, for this case, we'll get compact pattern structure, but it won't the exact protocol header structure. >=20 > > .sample_num =3D 2, > > .input_link[0] =3D &link0, > > .input_num =3D 1 > > }; > > > > Let's suppose we have created the flex item successfully, and PMD > > returned the handle 0x123456789A. We can use the following item > > pattern to match the crucial field in the packet with value 0x00112233: > > > > struct new_protocol_header spec_pattern =3D > > { > > .crucial =3D RTE_BE32(0x00112233), > > }; > > struct new_protocol_header mask_pattern =3D > > { > > .crucial =3D RTE_BE32(0xFFFFFFFF), > > }; > > struct rte_flow_item_flex spec_flex =3D { > > .handle =3D 0x123456789A > > .length =3D sizeiof(struct new_protocol_header), > > .pattern =3D &spec_pattern, > > }; > > struct rte_flow_item_flex mask_flex =3D { > > .length =3D sizeof(struct new_protocol_header), > > .pattern =3D &mask_pattern, > > }; > > struct rte_flow_item item_to_match =3D { > > .type =3D RTE_FLOW_ITEM_TYPE_FLEX, > > .spec =3D &spec_flex, > > .mask =3D &mask_flex, > > }; > > > > Signed-off-by: Viacheslav Ovsiienko > > --- > > doc/guides/prog_guide/rte_flow.rst | 24 +++ > > doc/guides/rel_notes/release_21_11.rst | 7 + > > lib/ethdev/rte_ethdev.h | 1 + > > lib/ethdev/rte_flow.h | 228 +++++++++++++++++++++++++ > > 4 files changed, 260 insertions(+) > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst > > b/doc/guides/prog_guide/rte_flow.rst > > index 2b42d5ec8c..628f30cea7 100644 > > --- a/doc/guides/prog_guide/rte_flow.rst > > +++ b/doc/guides/prog_guide/rte_flow.rst > > @@ -1425,6 +1425,30 @@ Matches a conntrack state after conntrack > action. > > - ``flags``: conntrack packet state flags. > > - Default ``mask`` matches all state bits. > > > > +Item: ``FLEX`` > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +Matches with the network protocol header of preliminary configured > format. > > +The application describes the desired header structure, defines the > > +header fields attributes and header relations with preceding and > > +following protocols and configures the ethernet devices accordingly > > +via > > +rte_flow_flex_item_create() routine. >=20 > How about: matches a custom header that was created using > rte_flow_flex_item_create Np, fixed. >=20 > > + > > +- ``handle``: the flex item handle returned by the PMD on successful > > + rte_flow_flex_item_create() call. The item handle is unique within > > + the device port, mask for this field is ignored. >=20 > I think you can remove that it is unique handle. >=20 > > +- ``length``: match pattern length in bytes. If the length does not > > +cover > > + all fields defined in item configuration, the pattern spec and mask > > +are > > + supposed to be appended with zeroes till the full configured item le= ngth. >=20 > It looks bugy saying that you can give any length but expect the applicat= ion to > supply the full length. Yes. Application can configure 128B protocol header with rte_flow_flex_item= _create(). "Full configured length is 128B. And provide only 4 bytes of pattern in the flow. The driver should consider the patter as 4 bytes provided and 120 following=20 zero bytes.=20 >=20 > > +- ``pattern``: pattern to match. The protocol header fields are > > +considered > > + as bit fields, all offsets and widths are expressed in bits. The > > +pattern > > + is the buffer containing the bit concatenation of all the fields > > +presented > > + at item configuration time, in the same order and same amount. The > > +most > > + regular way is to define all the header fields in the flex item > > +configuration > > + and directly use the header structure as pattern template, i.e. > > +application > > + just can fill the header structures with desired match values and > > +masks and > > + specify these structures as flex item pattern directly. > > + >=20 > It hard to understand this comment and what the application should set. > I suggest to take the basic approach and just explain it. ( I think those= are the > last few lines) Last few lines is just a supposed option. Generally speaking, there are TWO structures - protocol header and match pa= ttern. The easiest way ("the most regular way") to use flex item - make the= se TWO structures coinciding. But this is not an only way. Fields in patter= n can reference the same protocol header fields multiple times, in arbitrary number and combinations. Application can do gathering split field together,= do byte order conversion, etc.=20 >=20 > > Actions > > ~~~~~~~ > > > > diff --git a/doc/guides/rel_notes/release_21_11.rst > > b/doc/guides/rel_notes/release_21_11.rst > > index 73e377a007..170797f9e9 100644 > > --- a/doc/guides/rel_notes/release_21_11.rst > > +++ b/doc/guides/rel_notes/release_21_11.rst > > @@ -55,6 +55,13 @@ New Features > > Also, make sure to start the actual text at the margin. > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > +* **Introduced RTE Flow Flex Item.** > > + > > + * The configurable RTE Flow Flex Item provides the capability to int= rodude > > + the arbitrary user specified network protocol header, configure th= e > device > > + hardware accordingly, and perform match on this header with > > + desired > > patterns > > + and masks. > > + > > * **Enabled new devargs parser.** > > > > * Enabled devargs syntax > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index > > afdc53b674..e9ad7673e9 100644 > > --- a/lib/ethdev/rte_ethdev.h > > +++ b/lib/ethdev/rte_ethdev.h > > @@ -558,6 +558,7 @@ struct rte_eth_rss_conf { > > * it takes the reserved value 0 as input for the hash function. > > */ > > #define ETH_RSS_L4_CHKSUM (1ULL << 35) > > +#define ETH_RSS_FLEX (1ULL << 36) >=20 > Is the indentation right? > How do you support FLEX RSS if more then on FLEX item is configured? >=20 As we found we missed some options in RSS related API (we have to invent the way how to tell the drivers about flex item fields while creating the i= ndirect RSS action), and there is no PMDs supporting RSSing over flex field= yet, we can omit RSS-related stuff in this release. > > > > /* > > * We use the following macros to combine with above ETH_RSS_* for > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index > > 7b1ed7f110..eccb1e1791 100644 > > --- a/lib/ethdev/rte_flow.h > > +++ b/lib/ethdev/rte_flow.h > > @@ -574,6 +574,15 @@ enum rte_flow_item_type { > > * @see struct rte_flow_item_conntrack. > > */ > > RTE_FLOW_ITEM_TYPE_CONNTRACK, > > + > > + /** > > + * Matches a configured set of fields at runtime calculated offsets > > + * over the generic network header with variable length and > > + * flexible pattern > > + * >=20 > I think it should say matches on application configured header. No, no. Matches on pattern. That may be configured with the same format as protocol header has. Or may be not (more complicated way to operated, bu= t it could provide some optimizations). >=20 > > + * @see struct rte_flow_item_flex. > > + */ > > + RTE_FLOW_ITEM_TYPE_FLEX, > > }; > > > > /** > > @@ -1839,6 +1848,160 @@ struct rte_flow_item { > > const void *mask; /**< Bit-mask applied to spec and last. */ }; > > > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * RTE_FLOW_ITEM_TYPE_FLEX > > + * > > + * Matches a specified set of fields within the network protocol > > + * header. Each field is presented as set of bits with specified > > +width, and > > + * bit offset (this is dynamic one - can be calulated by several > > +methods > > + * in runtime) from the header beginning. > > + * > > + * The pattern is concatenation of all bit fields configured at item > > +creation > > + * by rte_flow_flex_item_create() exactly in the same order and > > +amount, no > > + * fields can be omitted or swapped. The dummy mode field can be used > > +for > > + * pattern byte boundary alignment, least significant bit in byte goes= first. > > + * Only the fields specified in sample_data configuration parameter > > +participate > > + * in pattern construction. > > + * > > + * If pattern length is smaller than configured fields overall length > > +it is > > + * extended with trailing zeroes, both for value and mask. > > + * > > + * This type does not support ranges (struct rte_flow_item.last). > > + */ >=20 > I think it is to complex to understand see my comment above. >=20 > > +struct rte_flow_item_flex { > > + struct rte_flow_item_flex_handle *handle; /**< Opaque item handle. > > */ > > + uint32_t length; /**< Pattern length in bytes. */ > > + const uint8_t *pattern; /**< Combined bitfields pattern to match. */ > > +}; > > +/** > > + * Field bit offset calculation mode. > > + */ > > +enum rte_flow_item_flex_field_mode { > > + /** > > + * Dummy field, used for byte boundary alignment in pattern. > > + * Pattern mask and data are ignored in the match. All configuration > > + * parameters besides field size are ignored. >=20 > Since in the item we just set value and mask what will happen if we set m= ask > to be different then 0 in an offset that we have such a field? Nothing. The mask and value for bits covered with DUMMY fields are just ign= ored. There can be any values and masks, these ones will not be translated = to actual flow matcher. DUMMY is just a placeholder, to align the substant= ial fields with actual protocol header structure. DUMMY usage is optional, = and we need these ones only to build the pattern structure coinciding with proto h= eader, without covering the entire header with actual sampling fields (to = save HW resources). >=20 > > + */ > > + FIELD_MODE_DUMMY =3D 0, > > + /** > > + * Fixed offset field. The bit offset from header beginning is > > + * is permanent and defined by field_base parameter. > > + */ > > + FIELD_MODE_FIXED, > > + /** > > + * The field bit offset is extracted from other header field (indirec= t > > + * offset field). The resulting field offset to match is calculated a= s: > > + * > > + * field_base + (*field_offset & offset_mask) << field_shift >=20 > I can't find those name in the patch and I'm not clear on what they mean. Yes, it is type, fixed, thank you. >=20 > > + */ > > + FIELD_MODE_OFFSET, > > + /** > > + * The field bit offset is extracted from other header field (indirec= t > > + * offset field), the latter is considered as bitmask containing some > > + * number of one bits, the resulting field offset to match is > > + * calculated as: >=20 > Just like above. >=20 > > + * > > + * field_base + bitcount(*field_offset & offset_mask) << field_shi= ft > > + */ > > + FIELD_MODE_BITMASK, > > +}; > > + > > +/** > > + * Flex item field tunnel mode > > + */ > > +enum rte_flow_item_flex_tunnel_mode { > > + FLEX_TUNNEL_MODE_FIRST =3D 0, /**< First item occurrence. */ > > + FLEX_TUNNEL_MODE_OUTER =3D 1, /**< Outer item. */ > > + FLEX_TUNNEL_MODE_INNER =3D 2 /**< Inner item. */ }; > > + >=20 > The '}' should be at a new line. > If the item can be inner and outer do we need to define two flex objects? > Also why enum and not defines? Just looked at rte_flow.h and saw the #defines are not so common there, just for bit flags. For values sets there are mostly enums. And we have upd= ated the tunnel settings, so this enum is not needed anymore. > From API point of view I think it should hav the following options: > Mode_outer , mode_inner, mode_global and mode_tunnel, Why is per field > and not per object. Yes, agree, thank you very much for discovering this arch gap, updated. >=20 > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > +*/ __extension__ struct rte_flow_item_flex_field { > > + /** Defines how match field offset is calculated over the packet. */ > > + enum rte_flow_item_flex_field_mode field_mode; > > + uint32_t field_size; /**< Match field size in bits. */ >=20 > I think it will be better to remove the word Match. Yes, right, fixed >=20 > > + int32_t field_base; /**< Match field offset in bits. */ >=20 > I think it will be better to remove the word Match. Yes, right, fixed >=20 > > + uint32_t offset_base; /**< Indirect offset field offset in bits. */ >=20 > I think a better name will be offset_field /* the offset of the field tha= t holds > the offset that should be used from the field_base */ what do you think? It is just one term (first in sum of resulting offset), so xxxx_base looks = OK. >=20 > Maybe just change from offset_base to offset? >=20 > > + uint32_t offset_mask; /**< Indirect offset field bit mask. */ >=20 > Maybe better wording? > The mask to apply to the value that is set in the offset_field. We have an entity - offset field. We have 3 attributes of entiry - base, mask, shift. So, the naming schema is "entity-name_attribute-name": offset_base - "base" attribute of "offset field" offset_mask - "mask" attribute of "offset field" offset_shift - "shift" attribute of "offset field" field_base - "base" attribute of "field" entity The "field" word is omitted from "offset_field" name in order to make name = shorted and to not intermix with pure "field" entity. >=20 > > + int32_t offset_shift; /**< Indirect offset multiply factor. */ > > + uint16_t tunnel_count:2; /**< 0-first occurrence, 1-outer, > > +2-inner.*/ >=20 > I think this may result in some warning since you try to cast enum to 2 b= its. > Also the same question from above to support inner and outer do we need > two objects? We refactored the tunneling attributes. >=20 > > + uint16_t rss_hash:1; /**< Field participates in RSS hash > > +calculation. */ >=20 > Please see my comment on the RSS, it is not clear how more then one flex > item can be created and the rss will work. Yes, you are right, we must update the RSS API as well. No we have no drive= rs supporting RSS over flex item fields. But we considered the opportunity and= now (as review result) we have better understanding what we should develop= to provide RSS over flex.=20 >=20 > > + uint16_t field_id; /**< device hint, for flows with multiple items. > > +*/ >=20 > How should this be used? > Should be capital D in device. Updated the documentation. >=20 > > +}; > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > +*/ struct rte_flow_item_flex_link { > > + /** > > + * Preceding/following header. The item type must be always > > provided. > > + * For preceding one item must specify the header value/mask to > > match > > + * for the link be taken and start the flex item header parsing. > > + */ > > + struct rte_flow_item item; > > + /** > > + * Next field value to match to continue with one of the configured > > + * next protocols. > > + */ > > + uint32_t next; >=20 > Is this offset of the field or the value? " Next field VALUE" It is the value. Like 0x0800 in eth_type to specify IPv4 next proto. Or 17 in IPv4.proto to specify following UDP. >=20 > > + /** > > + * Specifies whether flex item represents tunnel protocol > > + */ > > + bool tunnel; > > +}; > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > +*/ struct rte_flow_item_flex_conf { > > + /** > > + * The next header offset, it presents the network header size covere= d > > + * by the flex item and can be obtained with all supported offset > > + * calculating methods (fixed, dedicated field, bitmask, etc). > > + */ > > + struct rte_flow_item_flex_field next_header; >=20 > I think a better name will be size/len Replied above about level of indirection. >=20 > > + /** > > + * Specifies the next protocol field to match with link next protocol > > + * values and continue packet parsing with matching link. > > + */ > > + struct rte_flow_item_flex_field next_protocol; > > + /** > > + * The fields will be sampled and presented for explicit match > > + * with pattern in the rte_flow_flex_item. There can be multiple > > + * fields descriptors, the number should be specified by sample_num. > > + */ > > + struct rte_flow_item_flex_field *sample_data; > > + /** Number of field descriptors in the sample_data array. */ > > + uint32_t sample_num; >=20 > nb_samples? >=20 > > + /** > > + * Input link defines the flex item relation with preceding > > + * header. It specified the preceding item type and provides pattern > > + * to match. The flex item will continue parsing and will provide the > > + * data to flow match in case if there is the match with one of input > > + * links. > > + */ > > + struct rte_flow_item_flex_link *input_link; > > + /** Number of link descriptors in the input link array. */ > > + uint32_t input_num; > Nb_inputs OK, let's rename. .. snip .. > > + > > +/** > > + * Modify the flex item on the specified Ethernet device. > > + * > > + * @param port_id > > + * Port identifier of Ethernet device. > > + * @param[in] handle > > + * Handle of the item existing on the specified device. > > + * @param[in] conf > > + * Item new configuration. >=20 > Do you to supply full configuration for each update? > Maybe add a mask? Currently no drivers supporting update. So, let's remove update routine. With best regards, Slava