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 52DB6A0032; Thu, 15 Sep 2022 02:58:35 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F18DA4021D; Thu, 15 Sep 2022 02:58:34 +0200 (CEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2083.outbound.protection.outlook.com [40.107.243.83]) by mails.dpdk.org (Postfix) with ESMTP id 6A27340156 for ; Thu, 15 Sep 2022 02:58:33 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YiucGDCgIHFQwZXgNZPLIXqUXKsTjXqVUsTuWAvvYndST2+2pgqvbEBniBAQmtF8GFrZqmQIJUMpna4uheRD8luj6Y5K35uH4q2b+3KC8mqj2oa/MXM5fNgn0RNf06S8faXXAieJ8kGOIifieq1J3pGjj/jLT5vtvOoVcpU0Dr13bEuY9N6ba3h/Ww3V8eQWfpeH0Lx9ucwV5AGsQoRjODHCAaRnhkDTEHVLxfISPhZDIdegjlkFYgwJpKQuwDZ1ISn57IgYmKbagZWycZUeuKUr/QtV7AN0mG7YC7QIGFajz3xX/n3Sv6lONmaM1CZouKKbUI9ougP28xjXEBXoWw== 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=KpWgHRAsspygNV4Y2ZRwU3eaX504goWZHJBB+ua9aH4=; b=TKbtbTKcGn2LEuVRAIsts02ILpj34/UOAOFhc/xldONgjfk2D6DM8MQXF9rLwK3c3fiUgmuWcGz/LGMxONBPFkLOuXzzK6dkDBmUcFii03yUX3F636CdrZ7ITiK2ozDZcs4Lav7HiSEtQ+eXRxl9vt3XfHEaImUkVFqwYIMDi6h4vdNw/yqDdOmxUWwYMEl4QTNw5MTZeYMsiw2O9993Cr8wBj8J0qj6I5SNoZgf5wdCnEvhStUCGmsRc12TCB+kfzgatCO2i5KebIdbZEwu4QCcSbLsEvVFyOmzVUsu+LNMVSsEgoaTjDMQ0rAotkolufS8+NL95+rA170rPS9ROA== 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=KpWgHRAsspygNV4Y2ZRwU3eaX504goWZHJBB+ua9aH4=; b=KBNcCZ53F8tNW5rpwIVVKvZnmh+86LqX5lHQ83EztK/uS5DL0hYRx6qX/X44jRy1AiraKdHBN+mON9SiuS+T42nnZa8PgcIT8CnkVJwaIncBFdDy6zZyN2LTZwR70WS65FECHkMhF44F3X2b3P3+DuDCKGLL4BdLnteP8UhUEFcwC9Z3ld7itNOGWxEpCch6PEHcBPpB3uD4YhmpE6kPKd85gYqNCAjtfawXlIRfu179pE8XjvsoslCrtPB995vK0TMKIPpToJEXWyxQ21i7RkFekFs8S5+yaAoxU9X2TvYi6vdB+NukULCH3xdI7s2D4RxOXi+lG98qzivEmIj0yg== Received: from BN9PR12MB5273.namprd12.prod.outlook.com (2603:10b6:408:11e::22) by LV2PR12MB5966.namprd12.prod.outlook.com (2603:10b6:408:171::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5632.14; Thu, 15 Sep 2022 00:58:30 +0000 Received: from BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::a426:af46:a459:d88b]) by BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::a426:af46:a459:d88b%5]) with mapi id 15.20.5612.022; Thu, 15 Sep 2022 00:58:30 +0000 From: Rongwei Liu To: Ivan Malov CC: Matan Azrad , Slava Ovsiienko , Ori Kam , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Aman Singh , Yuying Zhang , Andrew Rybchenko , "dev@dpdk.org" , Raslan Darawsheh Subject: RE: [PATCH v1] ethdev: add direction info when creating the transfer table Thread-Topic: [PATCH v1] ethdev: add direction info when creating the transfer table Thread-Index: AQHYwmNUlJ87k3rsrk6Ble4p/RnGnq3cDLKAgAFR3ACAABgfgIAA1YvQgABHMQCAABGTUIAAcJcAgACeIvA= Date: Thu, 15 Sep 2022 00:58:30 +0000 Message-ID: References: <20220907024020.2474860-1-rongweil@nvidia.com> <1be72d6-be5b-88b2-f15-16fd2c6d0c0@oktetlabs.ru> <5d8d42b2-7011-cb46-7f2c-1b1019c4151e@oktetlabs.ru> <46841a9-37f1-29a8-ba86-ac5410723e2f@oktetlabs.ru> <6164993a-ba4e-c1ea-aaf5-5cc7c35d3724@oktetlabs.ru> In-Reply-To: <6164993a-ba4e-c1ea-aaf5-5cc7c35d3724@oktetlabs.ru> 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_|LV2PR12MB5966:EE_ x-ms-office365-filtering-correlation-id: 15b706ed-08ba-4e32-7f83-08da96b56806 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: XbCP0dl9u4YTv+9zQEIhnDXeQTXtdWGUGECvd2LL9qvcNzAPu15NTP/D5QiMIlNyOWmpB3cRDqcMCiMWqZFHZOQbJG7YX1tnXF8X7DZvLI/Z5C8xa69vGyCVFqQ/kAG7RLssPlN5ty+ZQZ+AB4UxhSAWCs1JxeZLT+hKkEvyns2uSyPwHZt6I+olnvsf9Pi7wyC+zorr/imq0fupDuIOnY8MtHFtWqKDP+VwZ2NBX6WtqdZsLzQasB3ae5SOaPUMY/yEm8iLvGjKjG6sMlxshy0kR6FFr3esZ54H7JSozIY9G/EhahyKebpfbVVe5DwE6hcYxZ4E7n3C6LTZyvuzepmA3clkJAFsISCyjHjtctoh2/ejD/uV92xy3RGlF04DP+wmPvzSLtWmIx8AyOThsET/qnpUDUPpPc5B2oOPsBZl8Yw12Ph/A8x+dMeadOhHnN5wNk09QUPkDhwC19YxcFhvkHy/1ehTA6pDyM44JTaI7EZ2lDVQxaP1uQd+ulEdmARk5q1hgG1GKVlwgBdJQ95zqA/LH4FBDOS+74vEGlxyA5jp9n47zJdI6YPfA662C3q5ew/+iGXqYAw9bbw+BdCE2bGRqdPd/ylOsxgi2luJs3JurN+vvzkHPrKJi3unGcsoX5xCgJCljeIICQyDGUEKHq8hJlHnBghc3n83J4/2bMTc4taHCkA0mw766n4kHDGfTuBaXrS9S6X6E/rbKd+Bemayb1fZ+9naS5dHhjqZnX/v5XQ/bTTKBaFKkt9FJ8dwTgNNNhBsy9IUIkLVMw== 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:(13230022)(4636009)(136003)(366004)(39860400002)(396003)(346002)(376002)(451199015)(8936002)(66946007)(66446008)(76116006)(33656002)(5660300002)(64756008)(52536014)(316002)(6916009)(66556008)(86362001)(66476007)(4326008)(8676002)(122000001)(38100700002)(83380400001)(107886003)(53546011)(41300700001)(7696005)(6506007)(71200400001)(186003)(26005)(478600001)(9686003)(38070700005)(55016003)(54906003)(2906002)(30864003)(559001)(579004); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?nMoLz9qbSyBWk7xzCSGgqCaJTus93QTYEdXUH6wqmEEWogM9ZsUyb0eNcPyJ?= =?us-ascii?Q?bBTWNO1qM8c+NuICFmAKVpI/ZJub3XTZrh8RiNafNh5OuuwmtoCMCDZmyngU?= =?us-ascii?Q?W9IiaMDV+Jh4222sc2gnx6xjOI4PouGut3epIpryL9le0wk1Iqa1gUF3zvEX?= =?us-ascii?Q?t+8FH0LE0ne0J6KVkPhgOF+yVrtDE8G03R9IExRkreo50MOKievirEf9Ip1o?= =?us-ascii?Q?s/iWebfzl1viausBMZfEulYxKO/NLO6CHuUYmXKe7kubF3pD2gyQTLKPTivO?= =?us-ascii?Q?H9m1iiZUgXL0ZtwoaDjX7POpSNaI51JVtvljmC1UEn02BByqRzd2hbE6zOph?= =?us-ascii?Q?pdoKz4BnnKlWksvSNeB7i0oR7c2zOtBBR8T3XcoWV2wHoBjnaUQmBn4GlWyJ?= =?us-ascii?Q?KoTXYgd9OKXMslFEj35s/S4qO9a1fO45WoWe/+UQdmZdAr3jmXJzMac8yEIZ?= =?us-ascii?Q?2XxG70ZSu6eWP9ZHTeuO4Vjs/30tY9x81QfdYLmz31xNo0OsmPIjGDBYaPUs?= =?us-ascii?Q?RMH0OaZAKIRne7QobIli7JUmQAbXwUFdqkdZawlpuIA1rTcUxga8h21sbjcl?= =?us-ascii?Q?ubUe33OLBb0y1t5Sx5YEU2wZchJO/2I6kVTcepFEd4nqeKhMen439D/U50yt?= =?us-ascii?Q?xwK2mBl72sPESxm0VID6GOnxho/NQaaKD+qbQboqVArtF+jXsKlJYdLeU0Og?= =?us-ascii?Q?Mtsv9ImCD8VSXMtS1Z0Z2ASHJV9oZcVaaSRtKg9dEV8uB+Ii2uFiYAh8z6v3?= =?us-ascii?Q?w15sP7PzwwXq8+9o9Aj4BZVgPgaIEVEUuY5BNqGxg0c8BpPrsNFisQmBkA2d?= =?us-ascii?Q?PWsQ1F4zo73K0PWKLKR7e/Ya2qV36xZ1DvdoKHaCdMLsSlQcwW4kOUFO/ojL?= =?us-ascii?Q?9a+5np16riiVN9Xz/Zt2qeACIQFeR2xuiMq63Gol4HaqxGtY+TZrbQDY2t12?= =?us-ascii?Q?DtP/tN8nrSru2Cf9+v2JSCXxrRjVm9y6cXC7AZ/msOqiPc+2xEz/H1/hY7IW?= =?us-ascii?Q?QUTzV0Pxdd2JyZrWti14ggYg6NK+zfMla5Gp9S8uE2bXeCF2aivjeJEeA5tn?= =?us-ascii?Q?ay8CkENzyTIKMnOyk4DBMJv0g2XDY/XFmf9LrTA95a+2xbEAwGgXJPxdyKfU?= =?us-ascii?Q?Aj4Is4gt5Wlak5lmVmrRohJKsWBPlM5kQ6eBIEqSu8jrteIXYgE2TXUPnek3?= =?us-ascii?Q?2WClCPATKlyEmOO0cYp/HJuMz+inQpa03dClqilVGAm7XofEdniSJ16etkDr?= =?us-ascii?Q?Dwsvp7dSsIO6uIBcsWnLgRHtpw2A9FrB9w0mIGiKFv640a6etkOh9L42qTdL?= =?us-ascii?Q?QrTDI5SaLL45qDt5UAXy3hZhNQjC6Ih69twSk6E6Up7gK+ICntUmeleJjYUY?= =?us-ascii?Q?oosv8QkMyx0t0s/DJ2xWgIrUZnZZ1o8fH6h9o9zSba/T+yAmZTuDyFzxr3Ho?= =?us-ascii?Q?zJREMKyIYnqkF4kqWNqRRYx6FBl54VxvtrXZP0k/9oq6wyk9w5Q4VXDnV6EE?= =?us-ascii?Q?uG1Y2ompA6IY4qWm3zPiS85/iPJguMGapTu/tlDh0N6RnI+1gFA9t6uZgjxl?= =?us-ascii?Q?pandcHH3UVC21R6QjnNsHSeNp8NEvVPfXOhWr08d?= 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: 15b706ed-08ba-4e32-7f83-08da96b56806 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Sep 2022 00:58:30.4381 (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: gou3TI71EM2wgsymrPbZHOxFTOXJVydsNEji5sX44EChC6HQAYbSj6p+p56kujbfDjCNMimsc91jTccRqayi+Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR12MB5966 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: Wednesday, September 14, 2022 23:18 > To: Rongwei Liu > Cc: Matan Azrad ; Slava Ovsiienko > ; Ori Kam ; NBU-Contact- > Thomas Monjalon (EXTERNAL) ; Aman Singh > ; Yuying Zhang ; > Andrew Rybchenko ; dev@dpdk.org; Raslan > Darawsheh > Subject: RE: [PATCH v1] ethdev: add direction info when creating the tran= sfer > table >=20 > External email: Use caution opening links or attachments >=20 >=20 > Hi Rongwei, >=20 > On Wed, 14 Sep 2022, Rongwei Liu wrote: >=20 > > HI > > > > BR > > Rongwei > > > >> -----Original Message----- > >> From: Ivan Malov > >> Sent: Wednesday, September 14, 2022 15:32 > >> To: Rongwei Liu > >> Cc: Matan Azrad ; Slava Ovsiienko > >> ; Ori Kam ; NBU-Contact- > >> Thomas Monjalon (EXTERNAL) ; Aman Singh > >> ; Yuying Zhang ; > >> Andrew Rybchenko ; dev@dpdk.org; > >> Raslan Darawsheh > >> Subject: RE: [PATCH v1] ethdev: add direction info when creating the > >> transfer table > >> > >> External email: Use caution opening links or attachments > >> > >> > >> Hi, > >> > >> On Wed, 14 Sep 2022, Rongwei Liu wrote: > >> > >>> HI > >>> > >>> BR > >>> Rongwei > >>> > >>>> -----Original Message----- > >>>> From: Ivan Malov > >>>> Sent: Tuesday, September 13, 2022 22:33 > >>>> To: Rongwei Liu > >>>> Cc: Matan Azrad ; Slava Ovsiienko > >>>> ; Ori Kam ; NBU-Contact- > >>>> Thomas Monjalon (EXTERNAL) ; Aman Singh > >>>> ; Yuying Zhang ; > >>>> Andrew Rybchenko ; dev@dpdk.org; > >>>> Raslan Darawsheh > >>>> Subject: RE: [PATCH v1] ethdev: add direction info when creating > >>>> the transfer table > >>>> > >>>> External email: Use caution opening links or attachments > >>>> > >>>> > >>>> Hi Rongwei, > >>>> > >>>> PSB > >>>> > >>>> On Tue, 13 Sep 2022, Rongwei Liu wrote: > >>>> > >>>>> Hi > >>>>> > >>>>> BR > >>>>> Rongwei > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Ivan Malov > >>>>>> Sent: Tuesday, September 13, 2022 00:57 > >>>>>> To: Rongwei Liu > >>>>>> Cc: Matan Azrad ; Slava Ovsiienko > >>>>>> ; Ori Kam ; > >>>>>> NBU-Contact- Thomas Monjalon (EXTERNAL) ; > >>>>>> Aman Singh ; Yuying Zhang > >>>>>> ; Andrew Rybchenko > >>>>>> ; dev@dpdk.org; Raslan Darawsheh > >>>>>> > >>>>>> Subject: Re: [PATCH v1] ethdev: add direction info when creating > >>>>>> the transfer table > >>>>>> > >>>>>> External email: Use caution opening links or attachments > >>>>>> > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On Wed, 7 Sep 2022, Rongwei Liu wrote: > >>>>>> > >>>>>>> The transfer domain rule is able to match traffic wire/vf origin > >>>>>>> and it means two directions' underlayer resource. > >>>>>> > >>>>>> The point of fact is that matching traffic coming from some > >>>>>> entity like wire / VF has been long generalised in the form of > representors. > >>>>>> So, a flow rule with attribute "transfer" is able to match > >>>>>> traffic coming from either a REPRESENTED_PORT or from a > >> PORT_REPRESENTOR > >>>> (please find these items). > >>>>>> > >>>>>>> > >>>>>>> In customer deployments, they usually match only one direction > >>>>>>> traffic in single flow table: either from wire or from vf. > >>>>>> > >>>>>> Which customer deployments? Could you please provide detailed > >> examples? > >>>>>> > >>>>>>> > >>>>> > >>>>> We saw a lot of customers' deployment like: > >>>>> 1. Match overlay traffic from wire and do decap, then send to > >>>>> specific > >> vport. > >>>>> 2. Match specific 5-tuples and do encap, then send to wire. > >>>>> The matching criteria has obvious direction preference. > >>>> > >>>> Thank you. My questions are as follows: > >>>> > >>>> In (1), when you say "from wire", do you mean the need to match > >>>> packets arriving via whatever physical ports rather then matching > >>>> packets arriving from some specific phys. port? > >> > >> ^^ > >> > >> Could you please find my question above? Based on your understanding > >> of templates in async flow approach, an answer to this question may > >> help us find the common ground. > > It means traffic arrived from physical ports (transfer_proxy role) or s= outh > band per you concept. >=20 > Transfer proxy has nothing to do with physical ports. And I should stress= out > that "south band" and the likes are NOT my concepts. Instead, I think tha= t > direction designations like "south" or "north" aren't applicable when tal= king > about the embedded switch and its flow (transfer) rules. >=20 > > Traffic from vport (not transfer_proxy) or north band per your concept = won't > hit even if same packets. >=20 > Please see above. Transfer proxy is a completely different concept. > And I never used "north band" concept. >=20 > >> > >> -- > >> > >>>> > >>>> If, however, matching traffic "from wire" in fact means matching > >>>> packets arriving from a *specific* physical port, then for sure > >>>> item REPRESENTED_PORT should perfectly do the job, and the proposed > >>>> attribute is unneeded. > >>>> > >>>> (BTW, in DPDK, it is customary to use term "physical port", not > >>>> "wire") > >>>> > >>>> In (1), what are "vport"s? Please explain. Once again, I should > >>>> remind that, in DPDK, folks prefer terms "represented entity" / > >> "representor" > >>>> over vendor-specific terms like "vport", etc. > >>>> > >>> Vport is virtual port for short such as VF. > >> > >> Thanks. As I say, term "vport" might be confusing to some readers, so > >> it'd be better to provide this explanation (about VF) in the commit > >> description next time. > > Ack. Will add VF as an example. > >> > >>>> As for (2), imagine matching 5-tuple traffic emitted by a VF / guest= . > >>>> Could you please explain, why not just add a match item > >>>> REPRESENTED_PORT pointing to that VF via its representor? Doing so > >>>> should perfectly define the exact direction / traffic source. Isn't > >>>> that > >> sufficient? > >>>> > >>> Per my view, there is matching field and matching value difference. > >>> Like IPv4 src_addr 1.1.1.1, 1.1.1.2. 1.1.1.3, will you treat it as > >>> same or > >> different matching criteria? > >>> I would like to call them same since it can be summarized like > >>> 1.1.1.0/30 REPRESENTED_PORT is just another matching item, no > >>> essential > >> differences and it can't stand for direction info. > >> > >> It looks like we're starting to run into disagreement here. > >> There's no "direction" at all. There's an embedded switch inside the > >> NIC, and there're (logical) switch ports that packets enter the switch= from. > >> > >> When the user submits a "transfer" rule and does not provide neither > >> REPRESENTED_PORT nor PORT_REPRESENTOR in the pattern, the > embedded > >> switch is supposed to match packets coming from ANY ports, be it VFs > >> or physical (wire) ports. > >> > >> But when the user provides, in example, item REPRESENTED_PORT to > >> point to the physical (wire) port, the embedded switch knows exactly > >> which port the packets should enter it from. > >> In this case, it is supposed to match only packets coming from that > >> physical port. And this should be sufficient. > >> This in fact replaces the need to know a "direction". > >> It's just an exact specification of packet's origin. > >> > > There is traffic arriving or leaving the switch, so there is always dir= ection, > implicit or explicit. >=20 > This does not contradict my thoughts above. "Direction" is *defined* by t= wo > points (like in geometry): an initial point (the switch port through whic= h a > packet enters the switch) and the terminal point (the match engine inside= the > switch). If one knows these two points, no extra hints are required to sp= ecify > some "direction". Because direction is already represented by this "vecto= r" of > sorts. That's why presence of the port match item in the pattern is absol= utely > sufficient. Good to see this. Thank for the information. This update leverages the concept exactly defined by you: "an initial point= (the switch port through which a packet enters the switch)" If you think direction not good, we can change to other words like "initial= port"/"origin port" etc. >=20 > However, based on your later explanations, the use of precise port item i= s > simply inconvenient in your use case because you are trying to match traf= fic > from *multiple* ports that have something in common (i.e. all VFs or all = wire > ports). >=20 > And, instead of adding a new item type which would serve exactly your nee= ds, > you for some reason try to add an attribute, which has multiple drawbacks > which I described in my previous letter. >=20 > > For transfer rules, there is a concept transfer_proxy. > > It takes the switch ownership; all switch rules should be configured vi= a > transfer_proxy. >=20 > Yes, such concept exists, but it's a don't care with regard to the proble= m that > we're discussing, sorry. > Furthermore, unlike "switch domain ID" (which is the same for all ethdevs > belonging to a given physical NIC board), nobody guarantees that it's onl= y one > transfer proxy port. Some NIC vendors allows transfer rules to be added v= ia > any ethdev port. >=20 Does any flow rule leverage switchid already. Is it too obscure for end-use= r? > > > > Image a logic switch with one PF and two VFs. > > PF is the transfer proxy and VF belongs to the PF logically. > > When receiving traffic from PF, we can say it comes into the logic swit= ch. >=20 > That's correct. >=20 > > When packet sent from VF (VF belongs to PF), so we can say traffic leav= es > the switch. >=20 > That's not correct. Traffic sent from VF (for example, a guest VM is send= ing > packets) also *enters* the switch. PFs and VFs are in fact *separate* log= ical > ports of the embedded switch. >=20 > > > > Item REPRESENTED_PORT indicates switch to match traffic sent from which > port, comes into, or leave switch. >=20 > That is not correct either. Item REPRESENTED_PORT tells the switch to mat= ch > packets which come into the switch FROM the logical port which is > represented by the given DPDK ethdev. >=20 > For example, if ethdev=3D"E" is the *main* PF which is bound to physical = port "P", > then item REPRESENTED_PORT with ethdev ID being set to "E" tells the swit= ch > that only packet coming to NIC from *wire* via physical port "E" should m= atch. >=20 > > We can say it as one kind of packet metadata. >=20 > Kind of yes, but might be vendor-specific. No need to delve into this. >=20 > > Like you said, DPDK always treat transfer to match any PORTs traffic. >=20 > Slight correction: it treats it this way until it sees an exact port item= . > If the user provides REPRESENTED_PORT (or PORT_REPRESENTOR), it's no > longer *any* ports traffic, it's an exact port traffic. That's it. >=20 > > When REPRESENTED_PORT is specified, the rules are limited to some > dedicated PORTs. >=20 > These rules match only packets arriving TO the embedded switch FROM the > said dedicated ports. >=20 > > Other PORTs are ignored because metadata mismatching. >=20 > Kind of yes, correct. >=20 > > Rules still have the capability to match ANY PORTS if metadata matched. >=20 > This statement is only correct for the cases when the user does NOT use > neither item REPRESENTED_PORT nor item PORT_REPRESENTOR. >=20 > > > > This update will allow user to cut the other PORTs matching capabilitie= s. >=20 > As I explained, this is exactly what items PORT_REPRESENTOR and > REPRESENTED_PORT do. No need to have an extra attribute. >=20 > If the user adds item REPRESENTED_PORT with ethdev_id=3D"E", like in the > above example, to match packets entering NIC via the physical port "P", t= hen > this rule will NOT match packets entering NIC from other points. For exam= ple, > packets transmitted by a virtual machine via a VF will not match in this = case. >=20 > >>> Port id depends on the attach sequence. > >> > >> Unfortunately, this is hardly a good argument because flow rules are > >> supposed to be inserted based on the run-time packet learning. Attach > >> sequence is a don't care here. > >> > >>>> Also please mind that, although I appreciate your explanations > >>>> here, on the mailing list, they should finally be added to the > >>>> commit message, so that readers do not have to look for them elsewhe= re. > >>>> > >>> We have explained the high possibility of single-direction matching, = right? > >> > >> Not quite. As I said, it is not correct to assume any "direction", > >> like in geographical sense ("north", "south", etc.). Application has > >> ethdevs, and they are representors of some "virtual ports" (in your > >> terminology) belonging to the switch, for example, VFs, SFs or physica= l > ports. > >> > >> The user adds an appropriate item to the pattern (REPRESENTED_PORT), > >> and doing so specifies the packet path which it enters the switch. > >> > >>> It' hard to list all the possibilities of traffic matching preference= s. > >> > >> And let's say more: one need never do this. That's exactly the reason > >> why DPDK has abandoned the concept of "direction" in *transfer* rules > >> and switched to the use of precise criteria (REPRESENTED_PORT, etc.). > >> > > As far as I know, DPDK changes "transfer ingress" to "transfer", so it'= more > clear that transfer can match both directions (both ingress and egress). >=20 > Not quite. DPDK has abandoned the use of "ingress / egress" in "transfer" > rules because "ingress" and "egress" are only applicable on the VNIC leve= l. For > example, there is a PF attached to DPDK application: > packets that the application receives through this ethdev, are ingress, a= nd > packets that it transmits (tx_burst) are egress. >=20 > I can explain in other words. Imagine yourself standing *inside* a room w= hich > only has one door. When someone enters the room, it's "ingress", when > someone leaves, it's "egress". It's relative to your viewpoint. > In this example, such a room represents a VNIC / ethdev. >=20 > And now imagine yourself standing *outside* of another room / auditorium > which has multiple doors / exits. You're standing near some particular ex= it "A" > (VNIC / ethdev), but people may enter this room via another door "B" and = then > leave it via yet another door "C". In this case, from your viewpoint, thi= s traffic > cannot be considered neither ingress nor egress. Because these people do = not > approach you. >=20 > Like in this example, embedded switch is like a large auditorium with man= y- > many doors / exits. And there can be many-many > directions: packet can enter the switch via phys. port "P1" > and then leave it via another phys. port "P2". Or it can enter the switch= via > phys. port and the leave it via VF's logical port (to be delivered to a g= uest > machine), or a packet can travel from one VF to another one. >=20 > There's no PRE-DEFINED direction like "north to south" or "east to west". > And this explains why it's very undesirable to use term "direction". >=20 > > REPRESENTED_PORT is the evolution of "port_id", I think, it' only one k= ind of > matching items. >=20 > Yes. But nobody prevents you from defining yet another match item which w= ill > be able to refer to a *group* of ports which have something in common (i.= e. > "all guest ports of this switch" > pointing to all logical ports currently attached to virtual machines / gu= ests, or > "all wire ports of this swtich"). >=20 > > > > For large scale deployment like 10M rules, if we can save resources > significantly by introducing direction, why not? >=20 > I do not deny the fact that you have a use case where resources can be sa= ved > significantly if you give the PMD some extra knowledge when creating a fl= ow > table / pattern template. That's totally OK. What I object is the very > implementation and the use of term "direction". If you add new item types > (like above), then, when you create an async table 1 pattern template, yo= u will > have item ANY_WIRE_PORTS, and, for table 2 pattern template, you'll have > item ANY_GUEST_PORTS. > As you see, the two pattern templates now differ because the match criter= ia > use different items. >=20 > > > > Again, async API: > > 1. pattern template A > > 2. action template B > > 3. table C with pattern template A + action template B. > > 4. rule D, E, F... > > The specified REPRESENTED_PORT is provided in rules (D, E, F...) not pa= ttern > template A or action template B or table C. > > Resources may be allocated early at step 3 since table' rule_nums prope= rty. >=20 > No, item REPRESENTED_PORT *can* be provided inside pattern template A, > but, as you pointed out earlier, the problem is that you can't distinguis= h > different pattern templates which have this item, because pattern templat= es > know nothing about *exact* port IDs and only know item MASKS. Yes, I agre= e > that in your case such problem exists, but, as I say above, it can be sol= ved by > adding new item types: one for referring to all phys. ports of a given NI= C and > another one for pointing to a group of current guest users (VFs). >=20 > >>> The underlay is the one we have met for now. > >>>>> > >>>>>>> Introduce one new member transfer_mode into rte_flow_attr to > >>>>>>> indicate the flow table direction property: from wire, from vf > >>>>>>> or bi-direction(default). > >>>>>> > >>>>>> AFAIK, 'rte_flow_attr' serves both traditional flow rule > >>>>>> insertion and asynchronous (table) approach. The patch adds the > >>>>>> attributes to generic 'rte_flow_attr' but, for some reason, ignore= s non- > table rules. > >>>>>> > >>>>>>> > >>>>> Sync API uses one rule to contain everything. It' hard for PMD to > >>>>> determine > >>>> if this rule has direction preference or not. > >>>>> Image a situation, just for an example: > >>>>> 1. Vport 1 VxLAN do decap send to vport 2. 1 million scale > >>>>> 2. Vport 0 (wire) VxLAN do decap send to vport 3. 1 hundred scale= . > >>>>> 1 and 2 share the same matching conditions (eth / ipv4 / udp / > >>>>> vxlan /...), so > >>>> sync API consider them share matching determination logic. > >>>>> It means "2" have 1M scale capability too. Obviously, it wastes a > >>>>> lot of > >>>> resources. > >>>> > >>>> Strictly speaking, they do not share the same match pattern. > >>>> Your example clearly shows that, in (1), the pattern should request > >>>> packets coming from "vport 1" and, in (2), packets coming from "vpor= t 0". > >>>> > >>>> My point is simple: the "vport" from which packets enter the > >>>> embedded switch is ALSO a match criterion. If you accept this, > >>>> you'll see: the matching conditions differ. > >>>> > >>> See above. > >>> In this case, I think the matching fields are both "port_id + > >>> ipv4_vxlan". They > >> are same. > >>> Only differs with values like vni 100 or 200 vice versa. > >> > >> Not quite. Look closer: you use *different* port IDs for (1) and (2). > >> The value of "ethdev_id" field in item REPRESENTED_PORT differs. > >> > >>>>> > >>>>> In async API, there is pattern_template introduced. We can mark "1" > >>>>> to use > >>>> pattern_tempate id 1 and "2" to use pattern_template 2. > >>>>> They will be separated from each other, don't share anymore. > >>>> > >>>> Consider an example. "Wire" is a physical port represented by PF0 > >>>> which, in turn, is attached to DPDK via ethdev 0. "VF" (vport?) is > >>>> attached to guest and is represented by a representor ethdev 1 in DP= DK. > >>>> > >>>> So, some rules (template 1) are needed to deliver packets from "wire= " > >>>> to "VF" and also decapsulate them. And some rules (template 2) are > >>>> needed to deliver packets in the opposite direction, from "VF" > >>>> to "wire" and also encapsulate them. > >>>> > >>>> My question is, what prevents you from adding match item > >>>> REPRESENTED_PORT[ethdev_id=3D0] to the pattern template 1 and > >>>> REPRESENTED_PORT[ethdev_id=3D1] to the pattern template 2? > >>>> > >>>> As I said previously, if you insert such item before eth / ipv4 / > >>>> etc to your match pattern, doing so defines an *exact* direction / s= ource. > >>>> > >>> Could you check the async API guidance? I think pattern template > >>> focusing > >> on the matching field (mask). > >>> "REPRESENTED_PORT[ethdev_id=3D0] " and > >> "REPRESENTED_PORT[ethdev_id=3D1] "are the same. > >>> 1. pattern template: REPRESENTED_PORT mask 0xffff ... > >>> 2. action template: action1 / actions2. / 3. table create with > >>> pattern_template plus action template.. > >>> REPRESENTED_PORT[ethdev_id=3D0] will be rule1: rule create > >> REPRESENTED_PORT port_id is 0 / actions .... > >>> REPRESENTED_PORT[ethdev_id=3D1] will be rule2: rule create > >> REPRESENTED_PORT port_id is 1 / actions .... > >> > >> OK, so, based on this explanation, it appears that you might be > >> looking to refer > >> to: > >> a) a *set* of any physical (wire) ports > >> b) a *set* of any guest ports (VFs) > >> > > Great, looks we are more and more closer to the agreement. >=20 > Looks so. >=20 > >> You chose to achieve this using an attribute, but: > >> > >> 1) as I explained above, the use of term "direction" is wrong; > >> please hear me out: I'm not saying that your use case and > >> your optimisation is wrong: I'm saying that naming for it > >> is wrong: it has nothing to do with "direction"; > >> > > Do you have any better naming proposal? >=20 > As I said, what you are trying to achieve using a new attribute would be = way > better to achieve using new pattern items which can be easily told one fr= om > another in PMD when pre-allocaing resources for different async flow tabl= es. >=20 > So, I don't have any proposal for *attribute* naming. > What I propose is to consider new items instead. >=20 > >> 2) while naming a *set* of wire ports as "wire_orig" might be OK, > >> sticking with term "vf_orig" for a *set* of guest ports is > >> clearly not, simply because the user may pass another PF > >> to a guest instead of passing a VF; in other words, > >> a better term is needed here; > >> > > Like you said, vport may contain VF, SF etc. vport_orgin is on the logi= c switch > perspective. > > Any proposal is welcome. >=20 > The problem is, vport can be easily confused with a slightly more generic > "lport" (embedded switch's "logical port"), and, logical ports, in turn, = are not > confined to just VFs or PFs. For example, physical (wire) ports are ALSO = logical > ports of the switch. >=20 > >> 3) since it is possible to plug multiple NICs to a DPDK application, > >> even from different vendors, the user may end up having multiple > >> physical ports belonging to different physical NICs attached to > >> the application; if this is the case, then referring to a *set* > >> of wire ports using the new attribute is ambiguous in the > >> sense that it's unclear whether this applies only to > >> wire ports of some specific physical NIC or to the > >> physical ports of *all* NICs managed by the app; > >> > > Not matter how many NICs has been probed by the DPDK, there is always > switch/PF/VF/SF.. concept. >=20 > Correct. >=20 > > Each switch must have an owner identified by transfer_proxy(). Vport (V= F/SF) > can't cross switch in normal case. >=20 > No. That is not correct. This is tricky, but please hear me out: an indiv= idual NIC > board (that is, a given *switch*) is identified only by its switch domain= ID. As I > explained above, "transfer proxy" is just a technical hint for the applca= tion to > indicate an ethdev through which "transfer" rules must be managed. Not al= l > vendors support this concept (and they are not obliged to support it). >=20 > > The traffic comes from one NIC can't be offloaded by other NICs unless > forwarded by the application. >=20 > Right, but forwarding in software (inside DPDK application) is out of sco= pe with > regard to the problem that we're discussing. >=20 > > If user use new attribute to cut one side resource, I think user is sma= rt > enough to management the rules in different NICs. >=20 > As I explained above, I do not deny the existence of the problem that you= r > patch is trying to solve. Now it looks like we're on the same page with r= egard > to understanding the fact that what you're trying to do is to introduce a= match > criterion that would refer to a GROUP of similar ports. In my opinion, th= is is > not an *attribute*, it's a *match criterion*, and it should be implemente= d as > two new items. >=20 > Having two different item types would perfectly fit the need to know the > difference between such "directions" (as per your terminology) early enou= gh, > when parsing templates. >=20 > > No default behavior changed with this update. > > > >> 4) adding an attribute instead of yet another pattern item type > >> is not quite good because PMDs need to be updated separately > >> to detect this attribute and throw an error if it's not > >> supported, whilst with a new item type, the PMDs do not > >> need to be updated =3D if a PMD sees an unsupported item > >> while traversing the item with switch () { case }, it > >> will anyway throw an error; > >> > > PMD also need to check if it supports new matching item or not, right? > > We can't assume NIC vendor' PMD implementation, right? >=20 > No-no-no. Imagine a PMD which does not support "transfer" rules. > In such PMD, in the flow parsing function one would have: >=20 > if (!!attr->transfer) { > print_error("Transfer is not supported"); > return EINVAL; > } >=20 > If you add a new attribute, then PMDs which are NOT going to support it n= eed > to be updated to add similar check. > Otherwise, they will simply ignore presence / absence of the attribute in= the > rule, and validation result will be unreliable. >=20 > Yes, if this attribute is 0x0, then indeed behaviour does nto change. But= what if > it's 0x1 or 0x2? > PMDs that do not support these values must somehow reject such rules on > parsing. >=20 > However, this problem does not manifest itself when parsing items. Typial= ly, in > a PMD, one would have: >=20 > switch (item->type) { > case RTE_FLOW_ITEM_TYPE_VOID: > break; >=20 > case RTE_FLOW_ITEM_TYPE_ETH: > /* blah-blah-blah */ > break; >=20 > default: > return ENOTSUP; > } Are you assuming all PMDs will be implemented in the upper style? This new field targets async API which was added recently. No impact on syn= c API. I don't predict any effort on the existing PMD behavior. But agree with you: we should emphasize it' only for async mode. >=20 > So, if you introduce two new item types to solve your problem, then you w= on't > have to update existing PMDs. If the vendor wants to support the new item= s > (say, MLX or SFC), they'll update their code to accept the items. But oth= er > vendors will not do anything. If the user tries to pass such an item to a= vendor > which doesn't support the feature, the "default" case will just throw an = error. >=20 > This is what I mean when pointing out such difference between adding an > attribute VS adding new item types. >=20 > >> 5) as in (4), a new attribute is not good from documentation > >> standpoint; plase search for "represented_port =3D Y" in > >> documentation =3D this way, all supported items are > >> easily defined for various NIC vendors, but the > >> same isn't true for attributes =3D there is no > >> way to indicate supported attributes in doc. > >> > >> If points (1 - 5) make sense to you, then, if I may be so bold, I'd > >> like to suggest that the idea of adding a new attribute be abandoned. > >> Instead, I'd like to suggest adding new items: > >> > >> (the names are just sketch, for sure, it should be discussed) > >> > >> ANY_PHY_PORTS { switch_domain_id } > >> =3D match packets entering the embedded switch from *whatever* > >> physical ports belonging to the given switch domain > >> > > How many PHY_PORTS can one switch have, per your thought? Can I treat > the PHY_PORTS as the { switch_domain_id } owner as transfer_proxy()? >=20 > A single physical NIC board is supposed to have a single embedded switch > engine. Hence, if the NIC board has, in example, two or four physical por= ts, > these will be the physical ports of the switch. That's it. >=20 > As for the transfer proxy, please see my explanations above. > It's not *always* reliable to tell whether two given ethdevs belong to th= e same > physical NIC board or not. >=20 > Switch domain ID is the right criterion (for applications). >=20 > >> ANY_GUEST_PORTS { switch_domain_id } > >> =3D match packets entering the embedded switch from *whatever* > >> guest ports (VFs, PFs, etc.) belonging to the given > >> switch domain > >> > >> The field "switch_domain_id" is required to tell one physical board / > >> vendor from another (as I explained in point (3)). > >> The application can query this parameter from ethdev's switch info: > >> please see "struct rte_eth_switch_info". > >> > >> What's your opinion? > >> > > How can we handle ANY_PHY_PORTS/ ANY_GUEST_PORTS ' relationship > with REPRESENTED_PORT if conflicts? > > Need future tuning. >=20 > And if you carry on with "vf_orig" / "wire_orig" approach, you will inevi= tably > have the very same problem: possible conflict with items like > REPRESENTED_PORT. So does it matter? Yes, checks need to be done by PMDs > when parsing patterns. >=20 > > Like I said before, offloaded rules can't cross different NIC vendor' > "switch_domain_id". > > If user probes multiple NICs in one application, application should tak= e care > of packet forwarding. > > Also application should be aware which ports belong to which NICs. >=20 > Yes, perhaps, domain ID is not needed in the new items. > But the application still must keep track of switch domain IDs itself so = it knows > which rules to manage via which ethdevs. >=20 > Any other opinions? ANY_PHY_PORTS/ ANY_GUEST_PORTS looks like a super set of ports.=20 This will come another challenge: "why can't we use REPRESENTED_PORT with = mask" or "combine several REPRESENTED_PORT together"? >=20 > >>> > >>>>> > >>>>>> For example, the diff below adds the attributes to "table" > >>>>>> commands in testpmd but does not add them to regular (non-table) > >>>>>> commands like "flow create". Why? > >>>>>> > >>>>>>> > >>>>> > >>>>> "table" command limits pattern_template to single direction or > >>>>> bidirection > >>>> per user specified attribute. > >>>> > >>>> As I say above, the same effect can be achieved by adding item > >>>> REPRESENTED_PORT to the corresponding pattern template. > >>> See above. > >>>> > >>>>> "rule" command must tight with one "table_id", so the rule will > >>>>> inherit the > >>>> "table" direction property, no need to specify again. > >>>> > >>>> You migh've misunderstood. I do not talk about "rule" command > >>>> coupled with some "table". What I talk about is regular, NON-async > >>>> flow insertion commands. > >>>> > >>>> Please take a look at section "/* Validate/create attributes. */" > >>>> in file "app/test-pmd/cmdline_flow.c". When one adds a new flow > >>>> attribute, they should reflect it the same way as VC_INGRESS, > >> VC_TRANSFER, etc. > >>>> > >>>> That's it. > >>> We don't intend to pass this to sync API. The above code example is > >>> for sync > >> API. > >> > >> So I understand. But there's one slight problem: in your patch, you > >> add the new attributes to the structure which is *shared* between > >> sync and async use case scenarios. If one adds an attribute to this > >> structure, they have to provide accessors for it in all sync-related > >> commands in testpmd, but your patch does not do that. > >> > > Like the title said, "creating transfer table" is the ASYNC operation. > > We have limited the scope of this patch. Sync API will be another story= . > > Maybe we can add one more sentence to emphasize async API again. >=20 > No-no-no. There might be slight misunderstanding. I understand that you a= re > limiting the scope of your patch by saying this and this. > That's OK. What I'm trying to point out is the fact that your patch never= theless > touches the COMMON part of the flow API which is shared between two > approaches (sync and async). Yeah, you are right, we should emphasize it for async API not sync in the c= ode and comments. >=20 > Imagine a reader that does not know anything about the async approach. > He just opens the file in vim and goes directly to struct rte_flow_attr. > And, over there, he sees the new attribute "wire_orig". He then immediate= ly > assumes that these attributes can be used in testpmd. Now the reader open= s > testpmd and tries to insert a flow rule using the sync approach: >=20 > flow create priority 0 transfer vf_orig pattern / ... / end actions drop >=20 This is wrong statement. If user has no idea with cmdline usage, he should rely on "tab indication' = not something by guessing. The command prefix "flow" bifurcated now to sync and async now, user may us= e any keyword combinations.=20 He will get "argument error" if it's not good unless he knows what' he is d= oing. Again: we should emphasize it's only for async API only. > And doing so will be a failure, because your patch does not add the new > attribute keyword to sync flow rule syntax parser. That's it. >=20 > Once again, I should ephasize: the reader MAY know nothing about the asyn= c > approach. But if the attribute is present in "struct rte_flow_attr", it > immediately means that it is available everywhere. Both sync and async. >=20 > So, with this in mind, your attempt to limit the scope of the patch to as= ync-only > rules looks a little bit artificial. It's not correct from the *formal* s= tandpoint. >=20 > > > >> In other words, it is wrong to assume that "struct rte_flow_attr" > >> only applies to async approach. It had been introduced long before > >> the async flow design was added to DPDK. That's it. > >> > >>>> > >>>> But, as I say, I still believe that the new attributes aren't needed= . > >>> I think we are not at the same page for now. Can we reach agreement > >>> on the same matching criteria first? > >>>>> > >>>>>>> It helps to save underlayer memory also on insertion rate. > >>>>>> > >>>>>> Which memory? Host memory? NIC memory? Term "underlayer" is > >> vague. > >>>>>> I suggest that the commit message be revised to first explain how > >>>>>> such memory is spent currently, then explain why this is not > >>>>>> optimal and, finally, which way the patch is supposed to improve > >>>>>> that. I.e. be more > >>>> specific. > >>>>>> > >>>>>>> > >>>>> > >>>>> For large scalable rules, HW (depends on implementation) always > >>>>> needs > >>>> memory to hold the rules' patterns and actions, either from NIC or > >>>> from > >> host. > >>>>> The memory footprint highly depends on "user rules' complexity", > >>>>> also diff > >>>> between NICs. > >>>>> ~50% memory saving is expected if one-direction is cut. > >>>> > >>>> Regardless of this talk, this explanation should probably be > >>>> present in the commit description. > >>>> > >>> This number may differ with different NICs or implementation. We > >>> can't say > >> it for sure. > >> > >> Not an exact number, of course, but a brief explanation of: > >> a) what is wrong / not optimal in the current design; > > Please check the commit log, transfer have the capability to match bi- > direction traffic no matter what ports. > >> b) how it is observed in customer deployments; > > Customer have the requirements to save resources and their offloaded ru= les > is direction aware. > >> c) why the proposed patch is a good solution. > > New attributes provide the way to remove one direction and save underla= yer > resource. > > All of the above can be found in the commit log. >=20 > I understand all of that, but my point is, the existing commit message is= way > too brief. Yes, it mentions that SOME customers have SOME deployments, bu= t > it does not shed light on which specifics these deployments have. For exa= mple, > back in the day, when items PORT_REPRESENTOR and REPRESENTED_PORT > were added, the cover letter for that patch series provided details of > deployment specifics (application: OvS, scenario: full offload rules). >=20 > So, it's always better to expand on such specifics so that the reader has= full > picture in their head and doesn't need to look elsewhere. > Not all readers of the commit message will be happy to delve into our > discussions on the mailing list to get the gist. >=20 It' approach diverse. Pattern item approach will attract another discussion= thread, right? We should get a conclusion and reflect in the commit changes&logs, and it's= easy for others to absorb. > > > >> > > > >>>>> > >>>>>>> By default, the transfer domain is bi-direction, and no behavior > changes. > >>>>>>> > >>>>>>> 1. Match wire origin only > >>>>>>> flow template_table 0 create group 0 priority 0 transfer wire_or= ig... > >>>>>>> 2. Match vf origin only > >>>>>>> flow template_table 0 create group 0 priority 0 transfer vf_orig= ... > >>>>>>> > >>>>>>> Signed-off-by: Rongwei Liu > >>>>>>> --- > >>>>>>> app/test-pmd/cmdline_flow.c | 26 > +++++++++++++++++++++ > >>>>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++- > >>>>>>> lib/ethdev/rte_flow.h | 9 ++++++- > >>>>>>> 3 files changed, 36 insertions(+), 2 deletions(-) > >>>>>>> > >>>>>>> diff --git a/app/test-pmd/cmdline_flow.c > >>>>>>> b/app/test-pmd/cmdline_flow.c index 7f50028eb7..b25b595e82 > >>>>>>> 100644 > >>>>>>> --- a/app/test-pmd/cmdline_flow.c > >>>>>>> +++ b/app/test-pmd/cmdline_flow.c > >>>>>>> @@ -177,6 +177,8 @@ enum index { > >>>>>>> TABLE_INGRESS, > >>>>>>> TABLE_EGRESS, > >>>>>>> TABLE_TRANSFER, > >>>>>>> + TABLE_TRANSFER_WIRE_ORIG, > >>>>>>> + TABLE_TRANSFER_VF_ORIG, > >>>>>>> TABLE_RULES_NUMBER, > >>>>>>> TABLE_PATTERN_TEMPLATE, > >>>>>>> TABLE_ACTIONS_TEMPLATE, > >>>>>>> @@ -1141,6 +1143,8 @@ static const enum index next_table_attr[] = =3D > { > >>>>>>> TABLE_INGRESS, > >>>>>>> TABLE_EGRESS, > >>>>>>> TABLE_TRANSFER, > >>>>>>> + TABLE_TRANSFER_WIRE_ORIG, > >>>>>>> + TABLE_TRANSFER_VF_ORIG, > >>>>>>> TABLE_RULES_NUMBER, > >>>>>>> TABLE_PATTERN_TEMPLATE, > >>>>>>> TABLE_ACTIONS_TEMPLATE, > >>>>>>> @@ -2881,6 +2885,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", > >>>>>> > >>>>>> This does not explain the "wire" aspect. It's too broad. > >>>>>> > >>>>>>> + .next =3D NEXT(next_table_attr), > >>>>>>> + .call =3D parse_table, > >>>>>>> + }, > >>>>>>> + [TABLE_TRANSFER_VF_ORIG] =3D { > >>>>>>> + .name =3D "vf_orig", > >>>>>>> + .help =3D "affect rule direction to transfer", > >>>>>> > >>>>>> This explanation simply duplicates such of the "wire_orig". > >>>>>> It does not explain the "vf" part. Should be more specific. > >>>>>> > >>>>>>> + .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", @@ -8894,6 > >>>>>>> +8910,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.flow_attr.transfer_mode =3D 1; > >>>>>>> + return len; > >>>>>>> + case TABLE_TRANSFER_VF_ORIG: > >>>>>>> + if (!out->args.table.attr.flow_attr.transfer) > >>>>>>> + return -1; > >>>>>>> + out->args.table.attr.flow_attr.transfer_mode =3D 2; > >>>>>>> + return len; > >>>>>>> default: > >>>>>>> return -1; > >>>>>>> } > >>>>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>>>> index 330e34427d..603b7988dd 100644 > >>>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > >>>>>>> @@ -3332,7 +3332,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 [vf_orig] [wire_orig]] > >>>>>> > >>>>>> Is it correct? Shouldn't it rather be [transfer] [vf_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 > >>>>>>> a79f1e7ef0..512b08d817 100644 > >>>>>>> --- a/lib/ethdev/rte_flow.h > >>>>>>> +++ b/lib/ethdev/rte_flow.h > >>>>>>> @@ -130,7 +130,14 @@ struct rte_flow_attr { > >>>>>>> * through a suitable port. @see rte_flow_pick_transfer_pro= xy(). > >>>>>>> */ > >>>>>>> uint32_t transfer:1; > >>>>>>> - uint32_t reserved:29; /**< Reserved, must be zero. */ > >>>>>>> + /** > >>>>>>> + * 0 means bidirection, > >>>>>>> + * 0x1 origin uplink, > >>>>>> > >>>>>> What does "uplink" mean? It's too vague. Hardly a good term. > >> > >> I believe this comment should be reworked, in case the idea of having > >> an extra attribute persists. > >> > >>>>>> > >>>>>>> + * 0x2 origin vport, > >>>>>> > >>>>>> What does "origin vport" mean? Hardly a good term as well. > >> > >> I still believe this explanation is way too brief and needs to be > >> reworked to provide more details, to define the use case for the attri= bute > more specifically. > >> > >>>>>> > >>>>>>> + * N/A both set. > >>>>>> > >>>>>> What's this? > >> > >> The question stands. > >> > >>>>>> > >>>>>>> + */ > >>>>>>> + uint32_t transfer_mode:2; > >>>>>>> + uint32_t reserved:27; /**< Reserved, must be zero. */ > >>>>>>> }; > >>>>>>> > >>>>>>> /** > >>>>>>> -- > >>>>>>> 2.27.0 > >>>>>>> > >>>>>> > >>>>>> Since the attributes are added to generic 'struct rte_flow_attr', > >>>>>> non-table > >>>>>> (synchronous) flow rules are supposed to support them, too. If > >>>>>> that is indeed the case, then I'm afraid such proposal does not > >>>>>> agree with the existing items PORT_REPRESENTOR and > REPRESENTED_PORT. > >> They > >>>>>> do exactly the same thing, but they are designed to be way more > >>>>>> generic. Why > >>>> not use them? > >>>> > >>>> The question stands. > >>>> > >>>>>> > >>>>>> Ivan > >>>>> > >>>> > >>>> Ivan > >>> > > >=20 > Thank you.