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 9B24941B81; Mon, 30 Jan 2023 15:50:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2ED2F40DFB; Mon, 30 Jan 2023 15:50:00 +0100 (CET) Received: from NAM04-DM6-obe.outbound.protection.outlook.com (mail-dm6nam04on2075.outbound.protection.outlook.com [40.107.102.75]) by mails.dpdk.org (Postfix) with ESMTP id 640C440DF6 for ; Mon, 30 Jan 2023 15:49:58 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NZwisF2vZJ4YgqVHztoxJ5n1yerkqN1gRwHSZqnuxFNl+nvaNY06b+6T/oG2V7jPnE8UDCwPjUuaqiGLq7dvaHogyNxwbzTBIjlXtEYNKLQO6XUc/HtVc+PbHYg4LxxkHWMOU8d2qiqd7I3r7ZUOldYgaZO/um3GNUvHDwNVINRTxvR1PzXEFbhteQGJtGNXMKHMlbJ85yXQl9et0/jrH2kCv5Uzp/fmwbqNKiVz3gxsu9d6d+Is5Z6j9YD2oVa9doON0PxH4uvuGrXrOkCxA/VR2xyXDY/ncNUxhmYwuHD8rOh0YyiBa78pVnF6Z7eIhRKSthvZDnixJ/LDBi/nPA== 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=VzXXebhS1RILBSODszEQiF567I7fxnpbSxShoz303uk=; b=ZgZWwWLul71DRCNy3k85ZvqF02mXMD5VsjGv+05Wjwv/0sO/E7Wd2FHEc0b9U4PV0ZcxjKnUapvdzjDDdg1zSb4TtJk7YqcKWGZjWqrV+vu2ghFjQlswhwKiTXBWhhnJkx11FvldwKrs5WmT0AUpS2TBJLe9NVqbjrqY18WFH77pfStrM+b2449ciYCwlexMNb4/rZAJG7He2kuaJeUNNgzsOkV0Ee/bhvSGX5a/UvmlwZn7O8P2dKSNflw2nXYWwpTyVQK9NGFpQHE0zcUi1kX39nQPJNl0vk95AzZ9VvQKMxE3h7C3AjVjftQ7LJLplv9aVeQ6McUR+Y74A4sN9w== 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=VzXXebhS1RILBSODszEQiF567I7fxnpbSxShoz303uk=; b=qzDDs69aJqVgOwxmo0plTmFDFr57NgIX4at4dPeMtxYtKphttGm+nSWAA6Zsr8I9mhWKFvl1g5Et+QlG8K/uFsMK7Jcpv4NedBpp88JnsssU3ytJqGRw0LhdjhVqhrPu6HJLuW9U1x9UegkPN1WwywCkRUFf6vX49kw2JYEixRlt3kAwb+jbtpv2C/2OnvXl/uzn1gEHHIODNG6/e2uKRzzlGk1HqGNSInt2wuR4PscNCo7T0v+cgvQvKds1G872cGugpvi3LJUuvVAG/c93exrc8azzGd/G1Zg3nvVTgMn79EfQSngMaNkLPRImVrbhFAznRez8JIksZQ0ZjrmwYA== Received: from BN9PR12MB5273.namprd12.prod.outlook.com (2603:10b6:408:11e::22) by BL3PR12MB6473.namprd12.prod.outlook.com (2603:10b6:208:3b9::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.36; Mon, 30 Jan 2023 14:49:55 +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; Mon, 30 Jan 2023 14:49:55 +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+zc0SCwGdhWlPLPa62i3OAgAAbNdCAAGVVAIAAahag Date: Mon, 30 Jan 2023 14:49:55 +0000 Message-ID: References: <20221114115946.1074787-1-rongweil@nvidia.com> <2cd321-40eb-799f-abe-d0258f92e6de@arknetworks.am> In-Reply-To: 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_|BL3PR12MB6473:EE_ x-ms-office365-filtering-correlation-id: 02a08b37-71dd-4bf2-698e-08db02d14072 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: PdHGpRVKSPvHqmd+jRJtXTcZ3E5rIguReZIiSUUN4los1LU6vbuLTk+/bFYZYR8HXYN9QSNPwxIKL1F4gyMZF+cHEDLp8WdlzuKob14obHYNpIIWxOBq5LQgx6qBoptT9fS+SR8Hq4fF/vv5y6fFaakIYvBJrI0Am3MlU8tsnBWoO6kb36i/z0Rg1qtz60lKoIs39ohpPZ/GxLPlVlgiyh93LLhbDlO8ymZYUQd185s8E6hsxZxcUhjdh8Cs680Ju9WwsQLxGgsQ/MYcQj3exThzXuBcOymmRJNDYTf1/rQgYw4LIVKtrPGNiAWzd15k+h+aqHVsKWoPB8dj/EdKnzYJCsPcyE3s+r4YaiFb6t4LaFkefTqemy9+R8TyKpuocvYBLbOMeYct30YGsdQ6l0kgGzp9ZCLEZ+ZsT5cYDJ5AQprC/WovDS3k5YdbXXmEPAa1wPvaTBRiZxtg6dnb7M1Hzkv/g6N/L1ka1OEFss1RtsX1y70IMnhrhAWzb/cwiMO/bxX3jwR0lAK7FfV+E28BhEhknsbWcUxap1AcAxumsW/7NsU6wsFzGN9iY78Y3y/Vgdw5OQVciJRNlBB97dzoXlNhuhF39EL8+hXytKGwY743Wk20oJucOZPBsN0eB1sl8py72fmF+w54ImBpIuWKnXBHQXXNU3r5/nYMYNSUWYpkDWn5dB5/S/Y9QBPJtryusKgC0hdE3df3hrsYVQ== 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)(39860400002)(376002)(346002)(366004)(396003)(451199018)(316002)(54906003)(66946007)(64756008)(8676002)(4326008)(6916009)(76116006)(66556008)(66476007)(66446008)(8936002)(41300700001)(52536014)(5660300002)(38100700002)(122000001)(86362001)(33656002)(38070700005)(107886003)(71200400001)(186003)(53546011)(9686003)(6506007)(26005)(66899018)(30864003)(55016003)(2906002)(478600001)(7696005)(83380400001)(579004); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?iUXgJwwKM5YNd1mU0owdsyvXP5VfzDZZBb7SZilbBoyWmJf0XGmtHYXqlC58?= =?us-ascii?Q?MJle2GAN+a8ww+b4SE/Yg8myjhJpRLae6omYyZdXup6V3U4gXya0bBRUTW1X?= =?us-ascii?Q?YfMFu4KASUnuU3Fp4MnCFeDLPwFSqaw+z/C04MsDaUHKuQDKXKeewuZJ1o8S?= =?us-ascii?Q?qDmicXrZbzOAL/iQpL7YiaAB0RiRJ/9bSAr5pQuZ8muSFIS6L9mNOzRqqBTN?= =?us-ascii?Q?o3DP16W/4+a8QrVsy8rwvag1poJMqwRN825oRz4zMapaeYtDf+ZUuPoi6zsy?= =?us-ascii?Q?ALAW5NqxwXWRMV9+pJA7MpW/dpJEB6LLQ8DL+PKFjF1+kfL4NgVHM7kbNpsl?= =?us-ascii?Q?aj3QvTSmMNoXSSljks5rEvPD0NY9uxkBm9A53/66E5ynxjFEtj/5csL+fbu9?= =?us-ascii?Q?ynDWqvcwI/1DBdjlZUmMJrlu2X9x0x1ZpY2g1U5kcLHRm30lwadezriBw0En?= =?us-ascii?Q?CAXlpzr7ZTEMdmWT0wJYNc24L4QzwX9h5xpUpPk+E2c/QCfss3We0nYcSk+n?= =?us-ascii?Q?H3JefZGxylVIR1WrafRu/Lv6yPfCh8Bl740n9YigLr/Floy7G/gjXTh57I0t?= =?us-ascii?Q?Cn9LU8NG0Mh+mYLz3KrEDvjzX6BE4n5VrmOSPLBOCtxRcwgrMtRvoY0e8eGc?= =?us-ascii?Q?xK/X4qUSY2W8PVZqiYgd8BsT2mpZR8ZIImyCtU04mcBgHtVRsSLyUJ9EjnXJ?= =?us-ascii?Q?O2sJfsBKJsZkt8BODGEXWL2FyCpG6DX1zNgrodtRR/vNdCOO3g7pScFbXmW3?= =?us-ascii?Q?51udnPL79Qo1HtF6sEuMme/gDM18f8ZVbdrtHvbRqmbpHXu5oMmR2+vxT4IW?= =?us-ascii?Q?RtDPl2weS9oJxyef5yO6T7sIIVf9VCUkUFZfLSxSRWABEjYhZsjjowaFtS85?= =?us-ascii?Q?+L12ONp8ztshf3XR1llFQ076xJQM4eU7fd01L//f3N+QgX7J4WaaLHuozQpO?= =?us-ascii?Q?kWDB4s/Gax35hOTSYuZLkLQjpA7fDG59muGGT2Oe4KmEPKcMTl94f9eKlyO4?= =?us-ascii?Q?ypAjv51KY/IiGMlbGuP0hsxQagGS0+HXpnSWRDit8OTf4QAeqPmBFVdHx5eM?= =?us-ascii?Q?VFNjByadwn5zjYiovRx/xSDiDqJjCK2Iy1eDYyctwqfASTsENQGaJg6Qfa8q?= =?us-ascii?Q?c5vo9fTCIL9Dvh1p8Pu62OBCg7Q3J4F9oE0o1kiV9tN8iGbynLPmuAROeKUk?= =?us-ascii?Q?1XrZl3ZE+H5pC8/FDs2bNrWvUNxPWp/XSpti6ehGrmTBWavo0vAuFwrYA1HP?= =?us-ascii?Q?Di3Adp17c3K8SOC/0ewn5KBbBXDzNFpuOk9Vkac/7RehuSLvX3HTEXYdk4F9?= =?us-ascii?Q?/0P6EWUmTFjgweZpbLiZ7qs/C8jjBJ69fa7xuq4SfpGixNdt6+aEGP3Rn3FF?= =?us-ascii?Q?sIxSxKQRh6KIciO/13dUQSFy/HAKCxl4QXs/SJsNRj8ww6jVLxCgsYuPoYzv?= =?us-ascii?Q?x7kF0+QzfzGW/AHdBfDFCs75L5gCFTn1z8b3/D4p3uWd7dE1i6dKlnxOjGTd?= =?us-ascii?Q?T++liemcmgMneeBgtXhK+B3Yv+NC0JKVtcqSl4NAE3Sry60LPInsgIZida4Q?= =?us-ascii?Q?lC6ri1BXjBY0UgtFZZ22Gb9mN4Esuo9CBLcd7XYN?= 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: 02a08b37-71dd-4bf2-698e-08db02d14072 X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Jan 2023 14:49:55.5092 (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: Gi0iHJIjbYNSCxHWGgWuvxlsq27v1tWLS73j2Bv8YyI9K9mRKAG0o1iJmuwYU1souvnx/6/hrxqiRLR7THUq2w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR12MB6473 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: 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 tra= nsfer > table >=20 > External email: Use caution opening links or attachments >=20 >=20 > Hi Rongwei, >=20 > For my responses, PSB. >=20 > By the way, now you mention things like wasting memory and insertion > optimisastions, are there any comparative figures to see the effect of th= is hint > on insertion performance / memory footprint? > Some "before" / "after" examples would really be helpful. >=20 Good to hear we reach agreement almost. First, the hint has nothing related to matching, only affects PMD resource = management. In my local test, it can save around 50% memory in the VxLAN encap/decap ex= ample case. Insertion rate has very very few improvements. > After all, I'm not objecting this patch. But I believe that other reviewe= rs' > concerns should nevertheless be addressed anyway. Let me try to show the hint is useful. >=20 > On Mon, 30 Jan 2023, Rongwei Liu wrote: >=20 > > 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' traff= ic > 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 sen= d 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 pack= ets > >> 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. >=20 > I understand that "host" might not be a brilliant name. >=20 > If "vport" stands for every port of the NIC that is not a network port, t= hen 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? Where does that "for now" > decision come from? Just wondering. >=20 "For now" stands for my understanding. DPDK is always in evolution, right? You are right, PF should be included in 'vport" concept.=20 > >> > >>> 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". >=20 > 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 p= ackets > 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.=20 This may highly depend on PMD implementation. Basically, PFs' traffic may c= ontain=20 "from wire"/"wire_orig" and '"from local"/"vport_orig". That' why we emphasize it' optional for PMD. >=20 > >>> > >>> 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. >=20 > There's item "represented_port" mentioned over there. I'm just asking abo= ut > this "already done by IPv4 item" bit. Yes, it matches on the header but n= ot on > the true origin of the packet (the logical port of the NIC). If the app k= nows > which logical port the packet ingresses the NIC, why not match on it for > security? >=20 Hint is not a matching and it implies how to manage underlayer steering res= ource. If "vport_orig" is present, PMD will only apply the steering logic to vport= traffic. The resource is allocated in the async table before each rule. Already cove= r security considerations. Matching on "represented_port" needs to program each rule, considering a po= rt range like index "5-10". Hint tells PMD only to take care of traffic from vport regardless the port = index. > > I was explaining we need to apply it to each flow rule even if it's onl= y a flag > and no value. >=20 > That's clear. But PSB. >=20 > >>> > >>> 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". >=20 > Say, there's a group of rules, and each of them matches on exactly the sa= me > 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 res= ources and speed up the insertion. Resource is allocated in async table stage and we only have mask informatio= n. In each rule, the matching value passes in. I guess you are saying to optim= ize per different matching values, right? This needs dynamic calculations per each rule and wastes the resource in as= ync table(table allocates resource for all possible values). >=20 > 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"? >=20 It can deserve some kinds of "hint". But see above, these hints are per rul= e and resource allocation happens before rules. > > We provide a hint in this patch and no assumption on the matching patte= rns. >=20 > So I understand. My point is, certain portions of matching patterns may b= e > "fixed" =3D entirely the same (masks and specs) in all rules of a table. = Why not > give PMD a "hint" about them, too? >=20 > > I think matching pattern is totally controlled by application layer. >=20 > So is the "direction" spec: the app layer has item represented_port to co= ntrol > that. But, still, we're here to discuss a hint for that. > Why does the new hint aim exclusively at optimising out this specific met= a > 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 approac= h, right? Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN enca= p headers. This hint approach can work for 10 VFs once. In public cloud deployments, each VF/SF may map to different users, but und= erlay is almost same(GRE/VxLAN... differ in filed values). >=20 > > "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. >=20 > The slight problem with your proposal is that for some reason only one ty= pe of > a match criterion deserves a hint moved to the attrs. > Whilst in reality the applicaction may know in advance that certain subse= ts 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? >=20 Believe me, the hint helps us to save significant resources already. Per my view, your proposal is totally valid in sync approach, but please ch= eck my response, Async is trying to allocate resources in advance and speed up insertion ASA= P. > >> 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. >=20 > 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 nam= ed > eventually) limits the scope of matching. No? >=20 Hint won't interfere with matching. It has no knowledge of matching. Instead, it only controls matching resources. "wire_orig" tells PMD to all= ocate HW resource for traffic from wire only. Then traffic from vport is sliently ignored. Hint doesn't know what are mat= ched 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 *toke= n, > >>> 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 the 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 t= his > 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 explain = 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 t= raffic. > >>> + * This bit is not a matching criteria, just an optimization hint. > >> > >> You intended to spell "criterion", I take it. And still, it *is* a mat= ch 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 originated = 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. > >