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 B7BA741B88; Tue, 31 Jan 2023 04:06:55 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 71B9A4113C; Tue, 31 Jan 2023 04:06:55 +0100 (CET) Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2069.outbound.protection.outlook.com [40.107.244.69]) by mails.dpdk.org (Postfix) with ESMTP id 44B4E40EF0 for ; Tue, 31 Jan 2023 04:06:54 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nH9gvFryLZVN9CBZBxQ8oSPZlvnJUcACM1bL9dvryBVPgcLQoUfAdTWjlgVnauCbv/QKas5gLBn14aXMyfyHJo1m2G1/IAxNMlOd5lO2TFg6n+k42D4e6xtgfgPw+DJzGQVc6UbR8Sdj1r3+m79JVkPo1ZVv197wZtOqKqH+tbnQrSpbfgGNOoDbtLIh1sCV2XBhkX+I/J1fYD7Wu04ufbYuUyscj0emfC7exjlvDLRFK4y7h3CWxgIpADh8LUtI2VDPu9uXhrKJnmsjNsdoXmkwA0/REaeMUA5v4PVQDeALitN66KMKp4cvYEc9lE+otE4H1RGT4j9EYj/RydvTgA== 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=HvS7+snkySnnqd0Yjt6cQ0BNJV68JJlCmQBvj9MfYww=; b=ABX9azfNWUiK7msHVgt9rwhCRSg7AjAH6vt8T1FkTODcPcfPw1M5kquevlgOgmGQLweG13vXe7VA8o5PXMl2BTIph1mFSySH2aPN8GpOXL8TLDTku3CUbG6/01FO0Po5WWCz8sHftJsnDpIl/f0ia2J2mx+U0O54W8E9wYyCUihQ4/zXFQLHKGmNo6t+3QcoMPexDhyUfP2UrqKwd7QiRpzB6RGPeqn4uyd801FF7TxVw9ubxBS3FRFECBrrdH6M69WTc4vLHUhplNVaJk2avsZpF99ASqvC3qHJRPJtpC/zpH7RhmU0ATbFVKSQltkRmbhs7/qflRGliTm/2S0+IA== 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=HvS7+snkySnnqd0Yjt6cQ0BNJV68JJlCmQBvj9MfYww=; b=VygNhw/sY61NSEywPJJRn6TIgURb3BsFNmp/XCTXYLEUQSuICbMWsYu5bBoybWbWvjeU0hiuixe8tIDM9cOa0F5t+Ew349V3z9PekE2Sek+lGQJRo8DWI1rsHKkapZaHdxl/Pp7/WCBS8o68Cp3LkmO2bA1IzFv7gVg2JLRPfbOo1AcvcRrVYoxO5tgwDBlC7xLrk7XfjVoIXkWiuJb00+eA8NUK/De1uJuf4p9y6jxpRH9PsDmYu8XmGRQz4BPDsgEKzHBOWtwjKGGqc6WwIC8OhUNQh+mKAqSbSyI11HRBqjyamX+NKR1pKSgAYLpym8MQ0YF9QG4RVUGqUo+Bug== Received: from BN9PR12MB5273.namprd12.prod.outlook.com (2603:10b6:408:11e::22) by MW3PR12MB4587.namprd12.prod.outlook.com (2603:10b6:303:5d::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.36; Tue, 31 Jan 2023 03:06:51 +0000 Received: from BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::2296:10b9:2661:d795]) by BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::2296:10b9:2661:d795%4]) with mapi id 15.20.6043.036; Tue, 31 Jan 2023 03:06:51 +0000 From: Rongwei Liu To: Ivan Malov CC: Matan Azrad , Slava Ovsiienko , Ori Kam , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Aman Singh , Yuying Zhang , Ferruh Yigit , Andrew Rybchenko , "dev@dpdk.org" , Raslan Darawsheh Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer table Thread-Topic: [PATCH v7] ethdev: add special flags when creating async transfer table Thread-Index: AQHY+CCrvOGvyt+zc0SCwGdhWlPLPa62i3OAgAAbNdCAAGVVAIAAahaggACW6QCAAEFF0A== Date: Tue, 31 Jan 2023 03:06:51 +0000 Message-ID: References: <20221114115946.1074787-1-rongweil@nvidia.com> <2cd321-40eb-799f-abe-d0258f92e6de@arknetworks.am> <2f5f4559-b650-e085-3364-68886cc9b3d6@arknetworks.am> In-Reply-To: <2f5f4559-b650-e085-3364-68886cc9b3d6@arknetworks.am> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: BN9PR12MB5273:EE_|MW3PR12MB4587:EE_ x-ms-office365-filtering-correlation-id: 948817c8-db9e-4ba2-8136-08db03383322 x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: zKBNKoyqTglzyhxNzJAAKujzYp7xDPP/5ZPK78gNW7UL25LQo9vUYggMTzyA0SQ3aCXtQWwDxEfXL0fYxlwYl9XDkLxvvtetOVIZi8nnC+Hn3t4f4qcDiigPFsauqXu4YkJAw5wIvFSB7FFpzHCnZp2BCAHEpE2+Eyp802hGEX17C0I0VLvubJMumU9SUZHBM9pKNQFKOAJiOpN/jQGaiYsJ+CFOSkoQ5vZRRb6JYj/7k8alkvsdo8EZMSmRjD8KOtPDfEWoVcP3ave2JpUq58Axm7XeRjyLZxeHdUCKRnbh6BRrIat8MFvwy8TKWH+wKsC0kIVK3GzeCumVJgiWP3LbPiHItVxofDRxV1gjEbU3S334xU4X8YjL+Js+THl9xAHDVvYEUsCgbPSQHAsFuQin48cC0VWCOzT2UTcF9NKHKxNWgxKwc6jgVCeZOOdWRVXUi/aUJiwO702rPBv0PQyNU9WfK+4kk9MYEu114fbkNEK5I7usyT0QWx6Zb1ORAJqUUiqrOEZBAFp6A9srrlGX85PUJ+LTh+EcuOuKGAYyq4HIg0GUItfDDeafO2R29VNrHqunQGjtELdvOrvKJbTasyOgi6p0b4gZvCI+9RxlTY3hMEUbcAWmcX7DQ1k2dLpeZeyHAP1m30uzJwm3j2NPTkTez9XZjEpX6KmKwMIwY1+Ntz/QxlDmLAXcV5XNQf9w8IYId03MJboe0bK3fQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN9PR12MB5273.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(136003)(396003)(39860400002)(376002)(346002)(366004)(451199018)(2906002)(52536014)(122000001)(316002)(186003)(30864003)(38070700005)(6506007)(26005)(53546011)(5660300002)(54906003)(107886003)(38100700002)(55016003)(9686003)(66899018)(7696005)(33656002)(71200400001)(83380400001)(86362001)(478600001)(4326008)(66446008)(6916009)(66476007)(64756008)(66556008)(8676002)(66946007)(76116006)(8936002)(41300700001)(579004)(559001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?yYW3KCAyXPasynH9/C1G3LVlPUFGL6fLgpkdL+RFVmbJa3hlsQl6uC1xw9tg?= =?us-ascii?Q?b3C71moYN9JVaQ0WID3Vlw9DSyL6d1Nze2FilvcmQyJwOVUwcbeYWo66qNj0?= =?us-ascii?Q?bY/fwwzHM8rYPMKSSmUqlwUpZxJq33AhxUYKJmHlSErRLQOfwucwiqlecnlx?= =?us-ascii?Q?LkvmVxgPjN9whOaU36Ov1EQ6LwoSo+AOMm4XYFd/Pru87hF4FjX2BlWdZ51U?= =?us-ascii?Q?FpS/AW1LJAjtzpktxPomH6Jy6y6xvrXWSFSKCIzuhYtWY5ZbATuJsoPdrkDe?= =?us-ascii?Q?EX05bK1SliC0oJPdRjQjV2LYwF3+2RwheQAE33E9tkgvcmXy1ti5aOl4VPkT?= =?us-ascii?Q?b2igFSq0j6nqObMDulz4Kdnsahm/4OdNaY3kRnahduIkiKH+jwAPX+egn8JM?= =?us-ascii?Q?5hS6jgk7CDWhKXdsXB60i5rJ6dOwkMeFN1lA8kdP79bF9s8WbHhQFNasuAZo?= =?us-ascii?Q?D4FUV9z/sDhPJz9PVMnwpWESplF0Urr3pwSzxk1rcfDmo8ewdPYMPE1tTO7N?= =?us-ascii?Q?MK0rffX/M7hxqkZoAbrBIXLVtrFSGLxoVRc6kaIbQMvRWz1Y828zKc79cxdK?= =?us-ascii?Q?y9inoDlvnDkOiv52yVMwvTFUHTVWdGjep6Q9xu78mxC0sfGG/m8dJPzZG5ff?= =?us-ascii?Q?Ii9kgsCJ8ID+/Bvy4d7AG8AZEPd1Hxvt8L7imqx+j+vHG3/VQ5GsHi6Bx21f?= =?us-ascii?Q?wEuuQyUYUQe7MzKBjptlNVOMcABlOVc+Ux0qiBfZerAlMF+u6oWGTJam7VJ+?= =?us-ascii?Q?rpbSZW8fTfSxd/bPHFArHoxta3aWlzHexNaDxG5NL9+KWxl4tLxK1ZwG0ex1?= =?us-ascii?Q?2bfL4aIR8p+cPSp0ack9nt5RsouWCOoBwjfHDjF9jeFV0AzEMGKQgIng/BvN?= =?us-ascii?Q?j+H8qnxLeuKMKFRlhhoWXY0rGUlnVIrObM+kxqhnXYFy4fXKroupTmpeZVDf?= =?us-ascii?Q?TWoYnnyjMQvzNtgXX3d7RQ2zvonUOTlB6x+ixsN4iuCqpBaudacwEYosPTlh?= =?us-ascii?Q?sWtJPDy9/paE8u6952y3ABop+9T/aY4Mkm4jP9InwkiGz0J3Sz5Nt/dxlXAv?= =?us-ascii?Q?XvZ1PGpnllUmxnY/94ftpFMR3DF5koxQzNQH/7IGa1m1sNVzeadqhWTfXt5D?= =?us-ascii?Q?zF6JIL3RqgtWupLLF3JzVpmDV0Gqa449KFVnx7JALOZtFsZ/Ag+V5ZgiJ45+?= =?us-ascii?Q?82BLtU6nqx4SqKKkfrQpwe1xxjDBaGX7rTBnl44RDnvbDpFe4AWNQc1cZ4HV?= =?us-ascii?Q?xz2PEn/yCorO0S111r5FpQpne024UL09s96irofJSIw10RVcg16Os9gYdQCC?= =?us-ascii?Q?c01pzG0BNJDLoETKzzcgstrEfMDOoXzuQ/Ai8ev9qjcitiYcbPn5oOlIR5y3?= =?us-ascii?Q?5lp4KnEcU5bZJ1u2+4W6TN5uZwV7cL7pEGY+jyPgrbtjLD7TW72ZQ9enwUhC?= =?us-ascii?Q?7b5cvxDCSepja+ZhQuyo9VzSEskGrq5Ygmyp43lX63eClpR4hHIdRhKD45fK?= =?us-ascii?Q?R65x6bVsAFY6cdKkONPaTtT2zJxd3kmnFUOfYpYcioPRWRCLKyYeHkYSIEC1?= =?us-ascii?Q?5zvlHlzp+zUsOFFtnq88JCN/ZjgsxhGTYbN2xNgr?= 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: BN9PR12MB5273.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 948817c8-db9e-4ba2-8136-08db03383322 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jan 2023 03:06:51.3695 (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: jmL4bwWSdjnO1CpTp93zXJfleFMYIVFAtxRCtQpVctYtJFdhbMPXvBoRLtFSL8Y+Xd+m52YUuVHJI6OtYyh0qQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4587 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 HI Ivan BR Rongwei > -----Original Message----- > From: Ivan Malov > Sent: Tuesday, January 31, 2023 07:00 > To: Rongwei Liu > Cc: Matan Azrad ; Slava Ovsiienko > ; Ori Kam ; NBU-Contact- > Thomas Monjalon (EXTERNAL) ; Aman Singh > ; Yuying Zhang ; > Ferruh Yigit ; Andrew Rybchenko > ; dev@dpdk.org; Raslan Darawsheh > > Subject: RE: [PATCH v7] ethdev: add special flags when creating async tra= nsfer > table >=20 > External email: Use caution opening links or attachments >=20 >=20 > Hi Rongwei, >=20 > Thanks for the professional attitude. > Hope this discussion gets us on the > same page. Please see below. Thanks for the suggestion and comments. Hope everything goes well. >=20 > On Mon, 30 Jan 2023, Rongwei Liu wrote: >=20 > > HI Ivan > > > > BR > > Rongwei > > > >> -----Original Message----- > >> From: Ivan Malov > >> Sent: Monday, January 30, 2023 15:40 > >> To: Rongwei Liu > >> Cc: Matan Azrad ; Slava Ovsiienko > >> ; Ori Kam ; NBU-Contact- > >> Thomas Monjalon (EXTERNAL) ; Aman Singh > >> ; Yuying Zhang ; > >> Ferruh Yigit ; Andrew Rybchenko > >> ; dev@dpdk.org; Raslan Darawsheh > >> > >> Subject: RE: [PATCH v7] ethdev: add special flags when creating async > >> transfer table > >> > >> External email: Use caution opening links or attachments > >> > >> > >> Hi Rongwei, > >> > >> For my responses, PSB. > >> > >> By the way, now you mention things like wasting memory and insertion > >> optimisastions, are there any comparative figures to see the effect > >> of this hint on insertion performance / memory footprint? > >> Some "before" / "after" examples would really be helpful. > >> > > Good to hear we reach agreement almost. >=20 > Very well. >=20 > The key point here is that one may agree that some optimisations are inde= ed > needed, yes. I don't deny the fact that some vendors might have issues wi= th > how the API maps to the HW capabilities. > Yes, some undesirable resource overhead shall be avoided, but the high le= vel > hints for that have to be designed with care. >=20 Totally agree. That' why we emphasize "optional for PMD" and "application s= hould take care of hint" > > First, the hint has nothing related to matching, only affects PMD resou= rce > management. >=20 > You say "PMD resource management". For the flow management, that's > mostly vendor-specific, I take it. Let me explain. The application, for i= nstance, > can control the number of Tx descriptors in the queue during setup stage. > Tx descriptors are a common type of HW resource, hence the explicit contr= ol > for it available to applications. For flow library, it's not like that. D= ifferent > vendors have different "underlayer" > representations, they may vary drastically. The resource I mentioned is about "steering logic" not SW datapath.=20 With flow rules offloading, hardware should store the steering logic in its= reachable memory no matter embedded in or mapping from host OS. >=20 > I take it, in the case of the HW you're working with, this hint indeed ma= ps to > something that is entirely resource-related and which does not belong in = this > specific vendor's match criteria. I 100% understand that, in your case, t= hese > are separate. But the point is that, on the high-level programming level > (vendor-neutral), such a hint is in fact a match criterion. Because it te= lls the > driver to limit the scope of matching to just "from net"/"from vport", th= e same > way other metadata items do (represented_port). > The only difference is that it refers to a group of unspecified ports whi= ch have > something in common. > " a group of unspecified ports" means dynamic and flexible, right. IMO it's= valid and fits sync flow perfectly. But in async, when allocating resources (table creation), the group info is= still unknown. We don't want to scatter it into each rule insertion. > So, although I don't strongly object having some hints like this one in t= he > generic API, I nevertheless disagree with describing this as just "resour= ce- > specific" and not being a match criterion. It's just not always the case.= It might > not be valid for *all* NIC vendors. >=20 Agree, not valid for *all* NIC vendors. > > In my local test, it can save around 50% memory in the VxLAN encap/deca= p > example case. >=20 > Forgive me in case this has been already discussed; where's that memory? > I mean, is it some sort of general-purpose memory? Or some HW-specific > table capacity overhead? I'm trying to understand how the feature will be > useful to other vendors, or how common this problem is. >=20 See above. HW always needs memory to store offloaded rules no matter embedd= ed in chip or borrowed from OS. > > Insertion rate has very very few improvements. > >> After all, I'm not objecting this patch. But I believe that other revi= ewers' > >> concerns should nevertheless be addressed anyway. > > Let me try to show the hint is useful. > >> > >> On Mon, 30 Jan 2023, Rongwei Liu wrote: > >> > >>> Hi Ivan, > >>> > >>> BR > >>> Rongwei > >>> > >>>> -----Original Message----- > >>>> From: Ivan Malov > >>>> Sent: Monday, January 30, 2023 08:00 > >>>> To: Rongwei Liu > >>>> Cc: Matan Azrad ; Slava Ovsiienko > >>>> ; Ori Kam ; NBU-Contact- > >>>> Thomas Monjalon (EXTERNAL) ; Aman Singh > >>>> ; Yuying Zhang ; > >>>> Ferruh Yigit ; Andrew Rybchenko > >>>> ; dev@dpdk.org; Raslan Darawsheh > >>>> > >>>> Subject: Re: [PATCH v7] ethdev: add special flags when creating > >>>> async transfer table > >>>> > >>>> External email: Use caution opening links or attachments > >>>> > >>>> > >>>> Hi Rongwei, > >>>> > >>>> Thanks for persevering. I have no strong opinion, but, at least, > >>>> the fact that the new flags are no longer meant for use in > >>>> rte_flow_attr, which is clearly not the right place for such, is an > improvement. > >>>> > >>> Thanks for the suggestion, move it to rte_flow_table_attr now and it' > >> dedicated to async API. > >>>> However, let's take a closer look at the current patch, shall we? > >>>> > >>>> But, before we get to that, I'd like to kindly request that you > >>>> provide a more concrete example of how this feature is supposed to > >>>> be used. Are there some real-life application examples? > >>>> > >>> Sure. > >>>> Also, to me, it's still unclear how an application can obtain the > >>>> knowledge of this hint in the first instance. For example, can Open > >>>> vSwitch somehow tell ethdevs representing physical ports from ones > >>>> representing "vports" (host endpoints)? > >>>> How does it know which attribute to specify? > >>>> > >>> Hint should be initiated by application and application knows it' > >>> traffic > >> pattern which highly relates to deployment. > >>> Let' use VxLAN encap/decap as an example: > >>> 1. Traffic from wire should be VxLAN pattern and do the decap, then > >>> send to > >> different vports. > >>> flow pattern_template 0 create transfer relaxed no > >>> pattern_template_id > >>> 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp / > >>> vxlan / tag index is 0 data is 0x33 / end flow actions_template 0 > >>> create transfer actions_template_id 4 template raw_decap index 0 / > >>> represented_port ethdev_port_id 1 / end mask raw_decap index 0 / > >>> represented_port ethdev_port_id 1 / end flow template_table 0 create > >>> group 1 priority 0 transfer wire_orig table_id 4 rules_number 128 > >>> pattern_template 4 actions_template 4 > >>> > >>> 2. Traffic from vports should be encap with different VxLAN header > >>> and send > >> to wire. > >>> flow actions_template 1 create transfer actions_template_id 5 > >>> template raw_encap index 0 / represented_port ethdev_port_id 0 / end > >>> mask raw_encap index 0 / represented_port ethdev_port_id 0 / end > >>> flow template_table 0 create group 1 priority 0 transfer vport_orig > >>> table_id 5 rules_number 128 pattern_template 4 actions_template 5 > >>> > >>>> For the rest of my notes, PSB. > >>>> > >>>> On Mon, 14 Nov 2022, Rongwei Liu wrote: > >>>> > >>>>> In case flow rules match only one kind of traffic in a flow table, > >>>>> then optimization can be done via allocation of this table. > >>>> > >>>> This wording might confuse readers. Consider rephrasing it, please: > >>>> If multiple flow rules share a common set of match masks, then they > >>>> might belong in a flow table which can be pre-allocated. > >>>> > >>>>> Such optimization is possible only if the application gives a hint > >>>>> about its usage of the table during initial configuration. > >>>>> > >>>>> The transfer domain rules may process traffic from wire or vport, > >>>>> which may correspond to two kinds of underlayer resources. > >>>> > >>>> Why name it a "vport"? Why not "host"? > >>>> > >>>> host =3D packets generated by any of the host's "vport"s wire =3D > >>>> packets arriving at the NIC from the network > >>> Vport is "virtual port" for short and contains "VF/SF" for now. > >>> Per my thoughts, it' clearer and maps to DPDK port probing/management= . > >> > >> I understand that "host" might not be a brilliant name. > >> > >> If "vport" stands for every port of the NIC that is not a network > >> port, then this name might be OK to me, but why doesn't it cover PFs? > >> A PF is clearly not a network / physical port. Why just VF/SF then? Wh= ere > does that "for now" > >> decision come from? Just wondering. > >> > > "For now" stands for my understanding. DPDK is always in evolution, rig= ht? > > You are right, PF should be included in 'vport" concept. > >>>> > >>>>> That's why the first two hints introduced in this patch are about > >>>>> wire and vport traffic specialization. > >>>>> Wire means traffic arrives from the uplink port while vport means > >>>>> traffic initiated from VF/SF. > >>>> > >>>> By the sound of it, the meaning is confined to just VFs/SFs. > >>>> What if the user wants to match packets coming from PFs? > >>>> > >>> It should be "wire_orig". > >> > >> Forgive me, but that does not sound correct. Say, there's an > >> application and it has a PF plugged into it: ethdev index 0. And the > >> application transmits packets using rte_eth_tx_burst() from that port. > >> You say that these packets can be matched via "wire_orig". > >> But they do not come from the wire. They come from PF... > > Hmm. My mistake. > > This may highly depend on PMD implementation. Basically, PFs' traffic > > may contain "from wire"/"wire_orig" and '"from local"/"vport_orig". > > That' why we emphasize it' optional for PMD. > >> > >>>>> > >>>>> There are two possible approaches for providing the hints. > >>>>> Using IPv4 as an example: > >>>>> 1. Use pattern item in both template table and flow rules. > >>>>> > >>>>> pattern_template: pattern ANY_VPORT / eth / ipv4 is 1.1.1.1 / end > >>>>> async flow create: pattern ANY_VPORT / eth / ipv4 is 1.1.1.2 / end > >>>>> > >>>>> "ANY_VPORT" needs to be present in each flow rule even if it's > >>>>> just a hint. No value to match because matching is already done by > >>>>> IPv4 item. > >>>> > >>>> Why no value to match on? How does it prevent rogue tenants from > >>>> spoofing network headers? If the application receives a packet on a > >>>> particular vport's representor, then it may strictly specify item > >>>> represented_port pointing to that vport so that only packets from > >>>> that vport > >> match. > >>>> > >>>> Why isn't security a consideration? > >>>> > >>> There is some misunderstanding here. "ANY_VPORT" is the approach > >>> (new > >> matching item without value) suggested by you. > >> I'm not talking about ANY_VPORT in this particular paragraph. > >> > >> There's item "represented_port" mentioned over there. I'm just asking > >> about this "already done by IPv4 item" bit. Yes, it matches on the > >> header but not on the true origin of the packet (the logical port of > >> the NIC). If the app knows which logical port the packet ingresses > >> the NIC, why not match on it for security? > >> > > Hint is not a matching and it implies how to manage underlayer steering > resource. > > If "vport_orig" is present, PMD will only apply the steering logic to v= port > traffic. > > The resource is allocated in the async table before each rule. Already = cover > security considerations. > > Matching on "represented_port" needs to program each rule, considering = a > port range like index "5-10". > > Hint tells PMD only to take care of traffic from vport regardless the p= ort > index. > > > >>> I was explaining we need to apply it to each flow rule even if it's > >>> only a flag > >> and no value. > >> > >> That's clear. But PSB. > >> > >>>>> > >>>>> 2. Add special flags into table_attr. > >>>>> > >>>>> template_table 0 create table_id 0 group 1 transfer vport_orig > >>>>> > >>>>> Approach 1 needs to specify the pattern in each flow rule which > >>>>> wastes memory and is not user friendly. > >>>> > >>>> What if the user has to insert a group of rules which not only have > >>>> the same set of match masks but also share exactly the same match > >>>> spec values for a limited subset of network items (for example, > >>>> those of an encap. header)? This way, a subset of network item > >>>> specs can remain fixed across many rules. Does that count as wasting > memory? > >>>> > >>> Per my understanding, you are talking "multiple spec and mask mixing"= . > >> > >> Say, there's a group of rules, and each of them matches on exactly > >> the same encap. header (the same in all rules), but different internal= match > field values. > >> So, why don't these "fixed" > >> encap. header items deserve being "optimised" somehow, the same way > >> as this "wire_orig" does? > > We are back to original point. Async approach is trying to pre-allocate > resources and speed up the insertion. > > Resource is allocated in async table stage and we only have mask > information. > > In each rule, the matching value passes in. I guess you are saying to o= ptimize > per different matching values, right? > > This needs dynamic calculations per each rule and wastes the resource i= n > async table(table allocates resource for all possible values). > >> > >> If the application knows for sure that there will be packets with > >> exactly the same encap. header, - that forms this special knowledge > >> that can be used during init times to help the PMD optimise resource > allocation. > >> Isn't that so? Don't these items deserve some form of a "hint"? > >> > > It can deserve some kinds of "hint". But see above, these hints are per= rule > and resource allocation happens before rules. >=20 > That's not per rule. Perhaps I should've worded it differently. >=20 > Suppose, an application has to insert many flow rules, each of which has > match items A and B. Item A not only has the same mask in *all* rule > instances, but also the same spec. > On the other hand, item B only has the same mask in all the rules, but it= s spec > is different for each rule. >=20 > In this example, the application can allocate a template with items A and= B, > but that only provides a fixed mask for them. And the application will HA= VE to > provide item A with exactly the same spec in all rule instances. The PMD,= in > turn, will HAVE to process this item every time, being unable to see it's= in fact > the same at all times. >=20 > To me, this sounds very similar to how you described the need to always > provide item ANY_VPORT in each rule thus facing some waste of memory and > parsing difficulties. >=20 > If the application knows that a certain item (or a certain fraction of it= ems) is > going to be entirely the same (mask + > spec) across all the rules, why shouldn't it be able to express this as a= hint to > the PMD? Why shouldn't it be able to avoid providing such items in every = new > flow rule instance? The same way the "vport_orig" works. >=20 > I'm not demanding that you re-implement or re-design this. > Just trying to find out whether such a problem can indeed be acknowledged= . > Or has it been solved already? If not, then perhaps it pays to just discu= ss > whether solving it can be combined with this "vport_orig" solution. >=20 > What do you think? What do others think? >=20 > >>> We provide a hint in this patch and no assumption on the matching > patterns. > >> > >> So I understand. My point is, certain portions of matching patterns > >> may be "fixed" =3D entirely the same (masks and specs) in all rules of > >> a table. Why not give PMD a "hint" about them, too? > >> > >>> I think matching pattern is totally controlled by application layer. > >> > >> So is the "direction" spec: the app layer has item represented_port > >> to control that. But, still, we're here to discuss a hint for that. > >> Why does the new hint aim exclusively at optimising out this specific > >> meta item? Why isn't it possible to care about a generic portion of > >> "know in advance" all-the-same items? > > " generic portion of know in advance" is some still kind of dynamic app= roach, > right? > > Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN > encap headers. > > This hint approach can work for 10 VFs once. > > In public cloud deployments, each VF/SF may map to different users, but > underlay is almost same(GRE/VxLAN... differ in filed values). > >> > >>> "wasting memory " because your approach needs to scatter in each > >>> rule > >> while this patch only needs to set table_attr once. > >>> No relation with matching patter totally. > >> > >> The slight problem with your proposal is that for some reason only > >> one type of a match criterion deserves a hint moved to the attrs. > >> Whilst in reality the applicaction may know in advance that certain > >> subsets of items will not only have the same masks in all rules but > >> also totally the same specs. If that is a valid use case, why doesn't > >> it deserve the same (more > >> generic) optimisation / a hint? Just wondering... > >> Or has that been addressed already somehow? > >> > > Believe me, the hint helps us to save significant resources already. >=20 > I'm not arguing it can be helpful. You're working round the clock to offe= r a > solution, - that's fine and is greatly appreciated. > But what I'm trying to say is that it looks like the problem might manife= st itself > for other type of knowledge that also may deserve a hint. Hence the quest= ions. > Hence the offer to think of covering more match criteria, not just net/vp= ort. >=20 > > Per my view, your proposal is totally valid in sync approach, but > > please check my response, Async is trying to allocate resources in adva= nce > and speed up insertion ASAP. >=20 > So if it's valid in sync approach, then why can't it be valid in the asyn= c one? > And I guess it can reflect positively on the insertion rate, too. Why lim= it this > "hint" approach to just one aspect then? >=20 > I'm sure we're close to understanding each other here. > Yes, "orig_vport" is just a one-bit knowledge, and seems innocent to add = as a > hint, but why not make it possible to have a hint for an arbitrary set of= always- > the-same match criteria? >=20 > In this case, nobody will ever argue of whether a hint is a match criteri= on or if > it's not. It will be quite a generic instrument, potentially useful to ve= ndors. > I'm afraid I can't think of an immediate example of such usefulness, but = at > least it will appear as generic as possible from the API perspective. >=20 > >>>> If yes, then the problem does not concern just a single pair of > >>>> attributes, but rather deserves a more versatile solution like some > >>>> sort of indirect grouping of constant item specs. > >>>> Have you considered such options? > >>> See above. > >>>> > >>>>> This patch takes the 2nd approach and introduces one new member > >>>>> "specialize" into rte_flow_table_attr to indicate possible flow > >>>>> table optimization. > >>>> > >>>> The name "specialize" might have some drawbacks: > >>>> - spelling difference (specialise/specialize) > >>>> - in grep output, will mix with flows' "spec" > >>>> - quite long > >>>> - not a noun > >>>> > >>>> Why not "scope"? Or something like that? > >>>> > >>> It means special optimization to PMD. "scope" is more rogue. > >> > >> Why is it "rogue"? Scope is something limiting the point of view. > >> So are the suggested flags. Flag "wire_origin" (or whatever it can be > >> named > >> eventually) limits the scope of matching. No? > >> > > Hint won't interfere with matching. It has no knowledge of matching. >=20 > Does specifying "orig_vport" actually provide a *choice* for the packet o= rigin? > Does it filter out everything else? If yes, then, alas, it *is* matching.= Because > matching is choosing something of interest. Let's face it. >=20 > As I said above, I do acknowledge the fact that, for some vendors, this m= atch > criterion, internally goes to a different HW aspect that is separate from > matching on, for example, IPv4 addresses. > That's OK. But for some vendors, this might be just a regular match crite= rion > internally. So let's describe it with care. >=20 > > Instead, it only controls matching resources. "wire_orig" tells PMD to > allocate HW resource for traffic from wire only. >=20 > If it controls "matching resources", it's indeed affiliated with matching= then. > Look. When the application creates a template, it tells the PMD that it i= s going > to match on this, this and this. > Masks... No exact values; they will come at a later stage. But, with this > "wire_orig", the application tells the PMD that not only it will match o= n > *some* "direction", but it actually provides a SPEC for that. If it indic= ates bit > "wire_orig", that equals to setting a "mask" for the "direction enum" > AND a "spec" (WIRE). Isn't that the case? >=20 > If it is, then please see my above concerns about possibly having similar= need > to provide exact-spec hints for other items as well. >=20 > > Then traffic from vport is sliently ignored. Hint doesn't know what are > matched and how many fields are involves. > >>>>> > >>>>> By default, there is no hint, so the behavior of the transfer > >>>>> domain doesn't change. > >>>>> There is no guarantee that the hint will be used by the PMD. > >>>>> > >>>>> Signed-off-by: Rongwei Liu > >>>>> Acked-by: Ori Kam > >>>>> > >>>>> v2: Move the new field to template table attribute. > >>>>> v4: Mark it as optional and clear the concept. > >>>>> v5: Change specialize type to uint32_t. > >>>>> v6: Change the flags to macros and re-construct the commit log. > >>>>> v7: Fix build failure. > >>>>> --- > >>>>> app/test-pmd/cmdline_flow.c | 26 ++++++++++++++++++= + > >>>>> doc/guides/prog_guide/rte_flow.rst | 15 +++++++++++ > >>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++- > >>>>> lib/ethdev/rte_flow.h | 28 ++++++++++++++++++= +++ > >>>>> 4 files changed, 71 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/app/test-pmd/cmdline_flow.c > >>>>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618 100644 > >>>>> --- a/app/test-pmd/cmdline_flow.c > >>>>> +++ b/app/test-pmd/cmdline_flow.c > >>>>> @@ -184,6 +184,8 @@ enum index { > >>>>> TABLE_INGRESS, > >>>>> TABLE_EGRESS, > >>>>> TABLE_TRANSFER, > >>>>> + TABLE_TRANSFER_WIRE_ORIG, > >>>>> + TABLE_TRANSFER_VPORT_ORIG, > >>>>> TABLE_RULES_NUMBER, > >>>>> TABLE_PATTERN_TEMPLATE, > >>>>> TABLE_ACTIONS_TEMPLATE, > >>>>> @@ -1158,6 +1160,8 @@ static const enum index next_table_attr[] =3D= { > >>>>> TABLE_INGRESS, > >>>>> TABLE_EGRESS, > >>>>> TABLE_TRANSFER, > >>>>> + TABLE_TRANSFER_WIRE_ORIG, > >>>>> + TABLE_TRANSFER_VPORT_ORIG, > >>>>> TABLE_RULES_NUMBER, > >>>>> TABLE_PATTERN_TEMPLATE, > >>>>> TABLE_ACTIONS_TEMPLATE, > >>>>> @@ -2933,6 +2937,18 @@ static const struct token token_list[] =3D { > >>>>> .next =3D NEXT(next_table_attr), > >>>>> .call =3D parse_table, > >>>>> }, > >>>>> + [TABLE_TRANSFER_WIRE_ORIG] =3D { > >>>>> + .name =3D "wire_orig", > >>>>> + .help =3D "affect rule direction to transfer", > >>>>> + .next =3D NEXT(next_table_attr), > >>>>> + .call =3D parse_table, > >>>>> + }, > >>>>> + [TABLE_TRANSFER_VPORT_ORIG] =3D { > >>>>> + .name =3D "vport_orig", > >>>>> + .help =3D "affect rule direction to transfer", > >>>>> + .next =3D NEXT(next_table_attr), > >>>>> + .call =3D parse_table, > >>>>> + }, > >>>>> [TABLE_RULES_NUMBER] =3D { > >>>>> .name =3D "rules_number", > >>>>> .help =3D "number of rules in table", @@ -8993,6 > >>>>> +9009,16 @@ parse_table(struct context *ctx, const struct token > >>>>> +*token, > >>>>> case TABLE_TRANSFER: > >>>>> out->args.table.attr.flow_attr.transfer =3D 1; > >>>>> return len; > >>>>> + case TABLE_TRANSFER_WIRE_ORIG: > >>>>> + if (!out->args.table.attr.flow_attr.transfer) > >>>>> + return -1; > >>>>> + out->args.table.attr.specialize =3D > >>>>> RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG; > >>>>> + return len; > >>>>> + case TABLE_TRANSFER_VPORT_ORIG: > >>>>> + if (!out->args.table.attr.flow_attr.transfer) > >>>>> + return -1; > >>>>> + out->args.table.attr.specialize =3D > >>>>> RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG; > >>>>> + return len; > >>>>> default: > >>>>> return -1; > >>>>> } > >>>>> diff --git a/doc/guides/prog_guide/rte_flow.rst > >>>>> b/doc/guides/prog_guide/rte_flow.rst > >>>>> index 3e6242803d..d9ca041ae4 100644 > >>>>> --- a/doc/guides/prog_guide/rte_flow.rst > >>>>> +++ b/doc/guides/prog_guide/rte_flow.rst > >>>>> @@ -3605,6 +3605,21 @@ and pattern and actions templates are > created. > >>>>> &actions_templates, nb_actions_templ, > >>>>> &error); > >>>>> > >>>>> +Table Attribute: Specialize > >>>>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^ > >>>>> + > >>>>> +Application can help optimizing underlayer resources and > >>>>> +insertion rate by specializing template table. > >>>>> +Specialization is done by providing hints in the template table > >>>>> +attribute ``specialize``. > >>>>> + > >>>>> +This attribute is not mandatory for each PMD to implement. > >>>>> +If a hint is not supported, it will be silently ignored, and no > >>>>> +special optimization is done. > >>>> > >>>> Silently ignoring the field does not sit well with the > >>>> application's possible intent to drop represented_port match from th= e > patterns. > >>>> From my point of view, if the application sets this attribute, it > >>>> believes it can rely on it, that is, packets coming from host won't > >>>> match if the attribute asks to match network only, for instance. > >>>> Has this > >> been considered? > >>>> > >>>>> + > >>>>> +If a table is specialized, the application should make sure the > >>>>> +rules comply with the table attribute. > >>>> > >>>> How does the application enforce that? I would appreciate you explai= n it. > >>>> > >>>>> + > >>>>> Asynchronous operations > >>>>> ----------------------- > >>>>> > >>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>> index 96c5ae0fe4..b3238415f4 100644 > >>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>> @@ -3145,7 +3145,8 @@ It is bound to > >>>> ``rte_flow_template_table_create()``:: > >>>>> > >>>>> flow template_table {port_id} create > >>>>> [table_id {id}] [group {group_id}] > >>>>> - [priority {level}] [ingress] [egress] [transfer] > >>>>> + [priority {level}] [ingress] [egress] > >>>>> + [transfer [vport_orig] [wire_orig]] > >>>>> rules_number {number} > >>>>> pattern_template {pattern_template_id} > >>>>> actions_template {actions_template_id} diff --git > >>>>> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index > >>>>> 8858b56428..c27b48c5c1 100644 > >>>>> --- a/lib/ethdev/rte_flow.h > >>>>> +++ b/lib/ethdev/rte_flow.h > >>>>> @@ -5186,6 +5186,29 @@ > rte_flow_actions_template_destroy(uint16_t > >>>>> port_id, */ struct rte_flow_template_table; > >>>>> > >>>>> +/**@{@name Special optional flags for template table attribute > >>>>> + * Each bit is a hint for table specialization, > >>>>> + * offering a potential optimization at PMD layer. > >>>>> + * PMD can ignore the unsupported bits silently. > >>>>> + */ > >>>>> +/** > >>>>> + * Specialize table for transfer flows which come only from wire. > >>>>> + * It allows PMD not to allocate resources for non-wire originated > traffic. > >>>>> + * This bit is not a matching criteria, just an optimization hint. > >>>> > >>>> You intended to spell "criterion", I take it. And still, it *is* a m= atch > criterion. > >>>> I'm not denying the possible need to have this criterion at the > >>>> earliest processing stage. That might be OK, but I still have a > >>>> hunch that this is too specific. > >>>> Please see my comment above about wasting memory. > >>>> I guess this type of criterion is not the only one that may need to > >>>> be provided as a "hint". > >>>> > >>>>> + * Flow rules which match non-wire originated traffic will be > >>>>> + missed > >>>>> + * if the hint is supported. > >>>> > >>>> And what if it's unsupported? Is it indeed OK to silently ignore it? > >>>> > >>>>> + */ > >>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG > >>>> RTE_BIT32(0) > >>>> > >>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ? > >>>> > >>>> To me, TRANSFER looks redundant as this bit is already supposed to > >>>> be ticked in the "struct rte_flow_attr flow_attr" field of the > >>>> "struct rte_flow_template_table_attr". > >>>> > >>>>> +/** > >>>>> + * Specialize table for transfer flows which come only from vport > >>>>> +(e.g. VF, > >>>>> SF). > >>>> > >>>> And PF? > >>>> > >>>>> + * It allows PMD not to allocate resources for non-vport originate= d > traffic. > >>>>> + * This bit is not a matching criteria, just an optimization hint. > >>>>> + * Flow rules which match non-vport originated traffic will be > >>>>> +missed > >>>>> + * if the hint is supported. > >>>>> + */ > >>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG > >>>> RTE_BIT32(1) > >>>> > >>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ? > >>>> > >>>>> +/**@}*/ > >>>>> + > >>>>> /** > >>>>> * @warning > >>>>> * @b EXPERIMENTAL: this API may change without prior notice. > >>>>> @@ -5201,6 +5224,11 @@ struct rte_flow_template_table_attr { > >>>>> * Maximum number of flow rules that this table holds. > >>>>> */ > >>>>> uint32_t nb_flows; > >>>>> + /** > >>>>> + * Optional hint flags for PMD optimization. > >>>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*. > >>>>> + */ > >>>>> + uint32_t specialize; > >>>> > >>>> Why not "scope" or something? > >>>> > >>>>> }; > >>>>> > >>>>> /** > >>>>> -- > >>>>> 2.27.0 > >>>>> > >>>> > >>>> Thank you. > >>> > > >=20 > Thank you.