From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5B376A00E6 for ; Thu, 11 Jul 2019 04:00:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 36B6831FC; Thu, 11 Jul 2019 04:00:01 +0200 (CEST) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20081.outbound.protection.outlook.com [40.107.2.81]) by dpdk.org (Postfix) with ESMTP id F0CCD2C23 for ; Thu, 11 Jul 2019 03:59:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=32ERFnHC4LJyVPV/29O5hyGefTwDlE7Ft7gALT41ub0=; b=g54PTp+MzI50vTAPD38BTt77meTCWGR/M2cpSSPMh/jDL3Vl1K3plJYjFK7i/LuKHvisSA5GCWLni6PWzHR3mwyDxnKbSnGoMmCf7U0hbdvEW0aDxZRfs2rrORJ8iv80J70EV2xwG0u/PHMDDp5hUQdTSdJ744KVBFvYOL20bLk= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB3963.eurprd05.prod.outlook.com (52.134.72.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2052.19; Thu, 11 Jul 2019 01:59:58 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::69c1:c0d7:1fa1:f89f]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::69c1:c0d7:1fa1:f89f%6]) with mapi id 15.20.2073.008; Thu, 11 Jul 2019 01:59:58 +0000 From: Yongseok Koh To: Adrien Mazarguil CC: Shahaf Shuler , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Olivier Matz , dev , Slava Ovsiienko Thread-Topic: [PATCH] ethdev: add flow tag Thread-Index: AQHVMzke3qf0ca9JGES4nN+uqhRIuqa8UhsAgAWqtACAArVjgA== Date: Thu, 11 Jul 2019 01:59:58 +0000 Message-ID: <20190711015948.GA21363@mtidpdk.mti.labs.mlnx> References: <20190603213231.27020-3-yskoh@mellanox.com> <20190704232302.19582-1-yskoh@mellanox.com> <20190705135404.GR4512@6wind.com> <6EE319CD-4BBC-47BF-AAE5-2165B8C1D491@mellanox.com> <20190709083806.GS4512@6wind.com> In-Reply-To: <20190709083806.GS4512@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: BY5PR13CA0036.namprd13.prod.outlook.com (2603:10b6:a03:180::49) To DB3PR0502MB3980.eurprd05.prod.outlook.com (2603:10a6:8:10::27) authentication-results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3d905ed9-e7b0-42ad-bdc9-08d705a37a24 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(4618075)(2017052603328)(7193020); SRVR:DB3PR0502MB3963; x-ms-traffictypediagnostic: DB3PR0502MB3963: x-ms-exchange-purlcount: 2 x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-forefront-prvs: 0095BCF226 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(39860400002)(376002)(366004)(136003)(346002)(396003)(199004)(189003)(7736002)(81166006)(8676002)(33656002)(386003)(6486002)(6436002)(66476007)(81156014)(66946007)(6306002)(66556008)(26005)(107886003)(229853002)(256004)(14444005)(476003)(6512007)(9686003)(6246003)(2906002)(53936002)(66446008)(64756008)(25786009)(486006)(305945005)(99286004)(53546011)(6506007)(68736007)(186003)(6116002)(3846002)(45080400002)(102836004)(86362001)(11346002)(76176011)(1076003)(66066001)(4326008)(966005)(6916009)(52116002)(316002)(54906003)(478600001)(5660300002)(446003)(8936002)(71200400001)(71190400001)(14454004)(130830200001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3963; H:DB3PR0502MB3980.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: MszacgebjQk2n5EFyAZtOEFe0MUCdf/skxR5d+TGaOHt1a5nSi5bBiwADErrS4FH3NkGTrbvPNZkkg/J3VSIIXy5SJTIXG0e3jlAjpT7XPaBqe6SF5+jM3zzJZjsK+3bvcr9g3/gOxo0oPLyxN5NFdCkT+O46uNY9YTFn3NG/Ot2ROJE0RFagk/i1M3Q4U7UgUpPSNrJtZIsmCMweFwKETVtj7ALsnaau2oQm3feT1pPnXK0f5X2njha7o+bQih5gYzUF/ilePLz/8ptTrlq6cMAlIzLOS3JyHEsPKfWwuql922J8qU7sbHen3hOXBfNoTIvYb2qtjnmUn3ItFBRD0hIqSB56Z3kOLQYVHmdoBR137FxRydUCNB5FhuzN0iPe7r4LLOwyyxYqffHqfeTTzFzw3LR4eKlaW6gWd4/ky4= Content-Type: text/plain; charset="us-ascii" Content-ID: <261B2D28E2E27146954B56FCAF1A684F@eurprd05.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3d905ed9-e7b0-42ad-bdc9-08d705a37a24 X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jul 2019 01:59:58.5379 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: yskoh@mellanox.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB3963 Subject: Re: [dpdk-dev] [PATCH] ethdev: add flow tag X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jul 09, 2019 at 10:38:06AM +0200, Adrien Mazarguil wrote: > On Fri, Jul 05, 2019 at 06:05:50PM +0000, Yongseok Koh wrote: > > > On Jul 5, 2019, at 6:54 AM, Adrien Mazarguil wrote: > > >=20 > > > On Thu, Jul 04, 2019 at 04:23:02PM -0700, Yongseok Koh wrote: > > >> A tag is a transient data which can be used during flow match. This = can be > > >> used to store match result from a previous table so that the same pa= ttern > > >> need not be matched again on the next table. Even if outer header is > > >> decapsulated on the previous match, the match result can be kept. > > >>=20 > > >> Some device expose internal registers of its flow processing pipelin= e and > > >> those registers are quite useful for stateful connection tracking as= it > > >> keeps status of flow matching. Multiple tags are supported by specif= ying > > >> index. > > >>=20 > > >> Example testpmd commands are: > > >>=20 > > >> flow create 0 ingress pattern ... / end > > >> actions set_tag index 2 value 0xaa00bb mask 0xffff00ff / > > >> set_tag index 3 value 0x123456 mask 0xffffff / > > >> vxlan_decap / jump group 1 / end > > >>=20 > > >> flow create 0 ingress pattern ... / end > > >> actions set_tag index 2 value 0xcc00 mask 0xff00 / > > >> set_tag index 3 value 0x123456 mask 0xffffff / > > >> vxlan_decap / jump group 1 / end > > >>=20 > > >> flow create 0 ingress group 1 > > >> pattern tag index is 2 value spec 0xaa00bb value mask 0xffff00ff = / > > >> eth ... / end > > >> actions ... jump group 2 / end > > >>=20 > > >> flow create 0 ingress group 1 > > >> pattern tag index is 2 value spec 0xcc00 value mask 0xff00 / > > >> tag index is 3 value spec 0x123456 value mask 0xffffff / > > >> eth ... / end > > >> actions ... / end > > >>=20 > > >> flow create 0 ingress group 2 > > >> pattern tag index is 3 value spec 0x123456 value mask 0xffffff / > > >> eth ... / end > > >> actions ... / end > > >>=20 > > >> Signed-off-by: Yongseok Koh > > >=20 > > > Hi Yongseok, > > >=20 > > > Only high level questions for now, while it unquestionably looks usef= ul, > > > from a user standpoint exposing the separate index seems redundant an= d not > > > necessarily convenient. Using the following example to illustrate: > > >=20 > > > actions set_tag index 3 value 0x123456 mask 0xfffff > > >=20 > > > pattern tag index is 3 value spec 0x123456 value mask 0xffffff > > >=20 > > > I might be missing something, but why isn't this enough: > > >=20 > > > pattern tag index is 3 # match whatever is stored at index 3 > > >=20 > > > Assuming it can work, then why bother with providing value spec/mask = on > > > set_tag? A flow rule pattern matches something, sets some arbitrary t= ag to > > > be matched by a subsequent flow rule and that's it. It even seems lik= e > > > relying on the index only on both occasions is enough for identificat= ion. > > >=20 > > > Same question for the opposite approach; relying on the value, never > > > mentioning the index. > > >=20 > > > I'm under the impression that the index is a hardware-specific constr= aint > > > that shouldn't be exposed (especially since it's an 8-bit field). If = so, a > > > PMD could keep track of used indices without having them exposed thro= ugh the > > > public API. > >=20 > >=20 > > Thank you for review, Adrien. > > Hope you are doing well. It's been long since we talked each other. :-) >=20 > Yeah clearly! Hope you're doing well too. I'm somewhat busy hence slow to > answer these days... >=20 > hey! > no private talks! >=20 > Back to the topic: >=20 > > Your approach will work too in general but we have a request from custo= mer that > > they want to partition this limited tag storage. Assuming that HW expos= es 32bit > > tags (those are 'registers' in HW pipeline in mlx5 HW). Then, customers= want to > > store multiple data even in a 32-bit storage. For example, 16bit vlan t= ag, 8bit > > table id and 8bit flow id. As they want to split one 32bit storage, I t= hought it > > is better to provide mask when setting/matching the value. Even some cu= stomer > > wants to store multiple flags bit by bit like ol_flags. They do want to= alter > > only partial bits. > >=20 > > And for the index, it is to reference an entry of tags array as HW can = provide > > larger registers than 32-bit. For example, mlx5 HW would provide 4 of 3= 2b > > storage which users can use for their own sake. > > tag[0], tag[1], tag[2], tag[3] >=20 > OK, looks like I missed the point then. I initially took it for a funky > alternative to RTE_FLOW_ITEM_TYPE_META & RTE_FLOW_ACTION_TYPE_SET_META > (ingress extended [1]) but while it could be used like that, it's more of= a > way to temporarily store and retrieve a small amount of data, correct? Correct. > Out of curiosity, are these registers independent from META and other > items/actions in mlx5, otherwise what happens if they are combined? I thought about combining it but I chose this way. Because it is transient.= META can be set by packet descriptor on Tx and can be delivered to host via mbuf= on Rx, but this TAG item can't. If I combine it, users have to query this capability for each 32b storage. And also, there should be a way to request= data from such storages (i.e. new action , e.g. copy_meta). Let's say there are = 4x32b storages - meta[4]. If user wants to get one 32b data (meta[i]) out of them= to mbuf->metadata, it should be something like, ingress / pattern .. / actions ... set_meta index i data x / copy_meta_to_rx index i And if user wants to set meta[i] via mbuf on Tx, egress / pattern meta index is i data is x ... / actions ... copy_meta_to_tx index i For sure, user is also responsible for querying these capabilities per each meta[] storage. As copy_meta_to_tx/rx isn't a real action, this example would confuse user. egress / pattern meta index is i data is x ... / actions ... copy_meta_to_tx index i User might misunderstand the order of two things - item meta and copy_meta action. I also thought about having capability bits per each meta[] storage= but it also looked complex. I do think rte_flow item/action is better to be simple, atomic and intuitiv= e. That's why I made this choice. > Are there other uses for these registers? Say, referencing their contents > from other places in a flow rule so they don't have to be hard-coded? Possible. Actually, this feature is needed by connection tracking of OVS-DPDK. > Right now I'm still uncomfortable with such a feature in the public API > because compared to META [1], this approach looks very hardware-specific = and > seemingly difficult to map on different HW architectures. I wouldn't say it is HW-specific. Like I explained above, I just define thi= s new item/action to make things easy-to-use and intuitive. > However, the main problem is that as described, its end purpose seems > redundant with META, which I think can cover the use cases you gave. So w= hat > can an application do with this that couldn't be done in a more generic > fashion through META? >=20 > I may still be missing something and I'm open to ideas, but assuming it > doesn't make it into the public rte_flow API, it remains an interesting > feature on its own merit which could be added to DPDK as PMD-specific > pattern items/actions [2]. mlx5 doesn't have any yet, but it's pretty com= mon > for PMDs to expose a public header that dedicated applications can includ= e > to use this kind of features (look for rte_pmd_*.h, e.g. rte_pmd_ixgbe.h)= . > No problem with that. That's good info. Thanks. But still considering connection-tracking-like use-cases, this transient storage on multi-table flow pipeline is quite use= ful. thanks, Yongseok > [1] "[PATCH] ethdev: extend flow metadata" > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fm= ails.dpdk.org%2Farchives%2Fdev%2F2019-July%2F137305.html&data=3D02%7C01= %7Cyskoh%40mellanox.com%7Ccd2d2d88786f43d9603708d70448c623%7Ca652971c7d2e4d= 9ba6a4d149256f461b%7C0%7C0%7C636982582929119170&sdata=3D4xI5tJ9pcVn1ooT= wmZ1f0O%2BaY9p%2FL%2F8O23gr2OW7ZpI%3D&reserved=3D0 >=20 > [2] "Negative types" > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fd= oc.dpdk.org%2Fguides%2Fprog_guide%2Frte_flow.html%23negative-types&data= =3D02%7C01%7Cyskoh%40mellanox.com%7Ccd2d2d88786f43d9603708d70448c623%7Ca652= 971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636982582929119170&sdata=3DgFYRs= Od8RzINShMvMR%2FXFKwV5RHAwThsDrvwnCrDIiQ%3D&reserved=3D0