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 51BC0A00BE; Tue, 29 Oct 2019 18:43:40 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6D0C01BF7F; Tue, 29 Oct 2019 18:43:39 +0100 (CET) Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80084.outbound.protection.outlook.com [40.107.8.84]) by dpdk.org (Postfix) with ESMTP id 8E2641BF7A for ; Tue, 29 Oct 2019 18:43:38 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Djlth5TR22EWEYw+KOgMCpI+uR6+SHhnVHComdsZaMKTkZChsIno0JH21RkvxH4uG3vzgTDLG6pQAFyt2Klv4okKui5opItVT2gOGdup/CE9IU4AwNNPVjTxSu5EAzoQToK3sVtgLV/N1RNJ0BYQt+Q3Ml9igiVlv/P7F5TXFZOWMKwc5eYzFT0PmCfgztORl9m0+a3p7CiI46bDVDNvlKetrIuDBu2LXV9Qen5JF1hrKnvLNtwQutxw/YKrWEOkI5eKbYbl8FMtIKC3YcGWvsmJ4t54RD7xKGR0KtAQcQohq2gviaU0HksXE6z5RsBbiEdTqr8IY9ozI4P4cqQgNw== 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=j7PRNmOZZwte3tu6K6Tbb2gAS4hOaOR+RzXNDQr76SM=; b=ZdYDJXYk4CgIc4yGYZI2GV5JdZqWk2HexhztSjAUe+IX/lkP0znGDCgTjpVCRlyrh0SS73Q4L8ImvZ4c8XhZ/JK9p9XM0rcPj5moMX/rnQGx4d0JJKK6bb/4/UbiSKFYF8ENG5Ku7lNbLl6wdbchdclkxRZJQqrzs9VeZaK2yBKi08+kgiQ09F1ZDEvW7eW5ISRZHlKyV18kRuJkX2AmuN+QHLAWEp7mSVlbWEmXuL9k/rCJuMgUzIme9To2TQPHf/nEiARWF0Tm2VTH2zgGQjrRIEfL68o2f7O9XxVi7kflWRT8SPxM9OBBnITws64yajjfRXkfHdKZ7VBmfavwgg== 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=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j7PRNmOZZwte3tu6K6Tbb2gAS4hOaOR+RzXNDQr76SM=; b=cRmqAjapSkhMII3DQdmrgQzQvfbY2dULb36Mxcpnw4i5/ey7uP569IkTc1EDdn00bE1y2irmB0lSZx9k9hYggl2EeEGCMabJKYc2IP4U8IAXIZv4OhRLTv5Jd18OwdRBYLGsqeD0tOZJ482wuNWneF4BpB4hz7HmDka5XvQEHak= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (10.171.188.154) by AM4PR05MB3332.eurprd05.prod.outlook.com (10.171.187.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2387.20; Tue, 29 Oct 2019 17:43:37 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::edab:529f:d14e:d3b]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::edab:529f:d14e:d3b%7]) with mapi id 15.20.2387.027; Tue, 29 Oct 2019 17:43:37 +0000 From: Slava Ovsiienko To: Olivier Matz CC: "dev@dpdk.org" , Thomas Monjalon , Matan Azrad , Ori Kam , Yongseok Koh Thread-Topic: [PATCH v4] ethdev: extend flow metadata Thread-Index: AQHVjnV4dBh/aEw9UUCRkYoPsCf2Zqdx4XAg Date: Tue, 29 Oct 2019 17:43:36 +0000 Message-ID: References: <1571922495-4588-1-git-send-email-viacheslavo@mellanox.com> <1572201636-16374-1-git-send-email-viacheslavo@mellanox.com> <20191029162522.ozj724j7pz7hz753@platinum> In-Reply-To: <20191029162522.ozj724j7pz7hz753@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=viacheslavo@mellanox.com; x-originating-ip: [95.164.10.10] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: cb82a7ba-f776-4b27-0bde-08d75c9786f6 x-ms-traffictypediagnostic: AM4PR05MB3332:|AM4PR05MB3332: 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:5516; x-forefront-prvs: 0205EDCD76 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(366004)(376002)(136003)(346002)(396003)(39860400002)(189003)(199004)(13464003)(446003)(4326008)(71190400001)(53546011)(476003)(6506007)(54906003)(316002)(74316002)(7736002)(229853002)(102836004)(6916009)(71200400001)(256004)(66066001)(14454004)(52536014)(478600001)(99286004)(7696005)(305945005)(14444005)(86362001)(76176011)(6436002)(66946007)(5660300002)(66556008)(66476007)(9686003)(11346002)(64756008)(25786009)(8676002)(2906002)(8936002)(186003)(3846002)(6116002)(55016002)(33656002)(66446008)(76116006)(107886003)(81166006)(81156014)(486006)(6246003)(26005)(309714004); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR05MB3332; H:AM4PR05MB3265.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: vcMFzR++DkjG6eV3Zs+PBy7ffTp+B8vqIbw33eEgvt5qtoQZ2R85esMGWEp2hB80L923EO0b1fOblKAZuT7F2ZRZJcL8uVa9SJauxWDMzw/q/byI3Jl3GQYcYQrLnPPACI8M3JU6fFv21amGMxMQGqYB5q+gO4rMhRmKgOubxZOjqhAtggYoPWPSN0+O+49SRsHLIVlY7JYVpDZf7GnOljTHgZfK37hdBuymdGN6QdxBSKl8FZbFqLC6ow8LK3eHyC1Rn1Kogab3sx3LbuUH73YRjYEA3HMQt65O7FQDCzR97duWn1c/8n36oJJf/eL9bL+AkUQk8MjVFxjSFaaGtO6EXeQqmjAo8gCPEYQcYyF2BCwEbFfai3zzyGs3yx1lVwcooigOlUsYdKgVJj2295XKaxL2ES8O50UDV+DCA0d/TuX0uU7JyX8XYYuOdSb0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: cb82a7ba-f776-4b27-0bde-08d75c9786f6 X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Oct 2019 17:43:36.7305 (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: 1fkds+NDaUbzy62kjQQ3ya2EwdU0CNLHlSOy4CzmuJbEUuc+3DLdBXGKrDXYFJaISEFmwLkRSOtdo6l7oYtmyjruYoan1zx9nzlmJ68tPFA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR05MB3332 Subject: Re: [dpdk-dev] [PATCH v4] ethdev: extend flow metadata 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 a lot for the review. > -----Original Message----- > From: Olivier Matz > Sent: Tuesday, October 29, 2019 18:25 > To: Slava Ovsiienko > Cc: dev@dpdk.org; Thomas Monjalon ; Matan > Azrad ; Ori Kam ; Yongseok > Koh > Subject: Re: [PATCH v4] ethdev: extend flow metadata >=20 > Hi Slava, >=20 > Looks good to me overall. Few minor comments below. >=20 > On Sun, Oct 27, 2019 at 06:40:36PM +0000, Viacheslav Ovsiienko wrote: > > Currently, metadata can be set on egress path via mbuf tx_metadata > > field with PKT_TX_METADATA flag and RTE_FLOW_ITEM_TYPE_META > matches metadata. > > > > This patch extends the metadata feature usability. > > > > 1) RTE_FLOW_ACTION_TYPE_SET_META > > > > When supporting multiple tables, Tx metadata can also be set by a rule > > and matched by another rule. This new action allows metadata to be set > > as a result of flow match. > > > > 2) Metadata on ingress > > > > There's also need to support metadata on ingress. Metadata can be set > > by SET_META action and matched by META item like Tx. The final value > > set by the action will be delivered to application via metadata > > dynamic field of mbuf which can be accessed by > RTE_FLOW_DYNF_METADATA(). > > PKT_RX_DYNF_METADATA flag will be set along with the data. > > > > The mbuf dynamic field must be registered by calling > > rte_flow_dynf_metadata_register() prior to use SET_META action. > > > > The availability of dynamic mbuf metadata field can be checked with > > rte_flow_dynf_metadata_avail() routine. > > > > For loopback/hairpin packet, metadata set on Rx/Tx may or may not be > > propagated to the other path depending on hardware capability. > > > > Signed-off-by: Yongseok Koh > > Signed-off-by: Viacheslav Ovsiienko >=20 > (...) >=20 > > diff --git a/lib/librte_ethdev/rte_ethdev.h > > b/lib/librte_ethdev/rte_ethdev.h index c36c1b6..b19c86b 100644 > > --- a/lib/librte_ethdev/rte_ethdev.h > > +++ b/lib/librte_ethdev/rte_ethdev.h > > @@ -1048,7 +1048,6 @@ struct rte_eth_conf { > > #define DEV_RX_OFFLOAD_KEEP_CRC 0x00010000 > > #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 > > #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 > > - > > #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM > | \ > > DEV_RX_OFFLOAD_UDP_CKSUM | \ > > DEV_RX_OFFLOAD_TCP_CKSUM) >=20 > Undue removed line here. Right, will fix. >=20 > (...) >=20 > > +/* Mbuf dynamic field offset for metadata. */ extern int > > +rte_flow_dynf_metadata_offs; > > + > > +/* Mbuf dynamic field flag mask for metadata. */ extern uint64_t > > +rte_flow_dynf_metadata_mask; > > + > > +/* Mbuf dynamic field pointer for metadata. */ #define > > +RTE_FLOW_DYNF_METADATA(m) \ > > + RTE_MBUF_DYNFIELD((m), rte_flow_dynf_metadata_offs, uint32_t > *) > > + > > +/* Mbuf dynamic flag for metadata. */ #define PKT_RX_DYNF_METADATA > > +(rte_flow_dynf_metadata_mask) > > + > > +__rte_experimental > > +static inline uint32_t > > +rte_flow_dynf_metadata_get(struct rte_mbuf *m) { > > + return *RTE_FLOW_DYNF_METADATA(m); > > +} > > + > > +__rte_experimental > > +static inline void > > +rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t v) { > > + *RTE_FLOW_DYNF_METADATA(m) =3D v; > > +} > > + >=20 > (...) >=20 > > +__rte_experimental > > +static inline int > > +rte_flow_dynf_metadata_avail(void) { > > + return !!rte_flow_dynf_metadata_mask; } >=20 > I think, in DPDK: >=20 > static inline void > rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t v) > { > ... >=20 > is prefered over: >=20 > static inline void > rte_flow_dynf_metadata_set(struct rte_mbuf *m, uint32_t v) { Right. It is strange it passed the validator. Will fix. > ... >=20 > > --- a/lib/librte_mbuf/rte_mbuf_dyn.h > > +++ b/lib/librte_mbuf/rte_mbuf_dyn.h > > @@ -234,6 +234,10 @@ int rte_mbuf_dynflag_lookup(const char *name, > > __rte_experimental void rte_mbuf_dyn_dump(FILE *out); > > > > -/* Placeholder for dynamic fields and flags declarations. */ > > - > > +/* > > + * Placeholder for dynamic fields and flags declarations. > > + * This is centralizing point to gather all field names > > + * and parameters together. > > + */ > > +#define MBUF_DYNF_METADATA_NAME "rte_flow_dynfield_metadata" > > #endif >=20 > The RTE_ prefix is missing. Also, thi name is called dynfield but it is u= sed for > both field and flag. I suggest RTE_MBUF_DYNFIELD_METADATA_NAME and > RTE_MBUF_DYNFLAG_METADATA_NAME, to be consistent with the other > naming conventions in rte_mbuf_dyn.[ch]. Well, it makes sense for complex features, say, with multiple dynaflags. But it is OK for me, will update. >=20 > One more comment: as previously discussed, changing the size or alignemen= t > of a dynamic field should not be allowed, because it can break the users = of > the field. >=20 > Depending on how it is implemented (is the registration function inline? > is the rte_mbuf_dynfield structure private, shared, or static const in a = .h? are > we using #defines for name, size, align?), I think the impact on users wi= ll be > different. This is something we need to think about for next versions: ho= w to > detect these changes before pushing the commit, and/or at runtime? >=20 I'm not sure if we will have a lot of implementation invariants for dynamic= fields. These ones are intended to be used in datapath, performance is a king. I th= ink if=20 someone invent the more efficient way to handle dynafields, we'll update th= e metadata code either. > Regards, > Olivier