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 CF403A00E6 for ; Tue, 9 Jul 2019 10:38:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C91D71B958; Tue, 9 Jul 2019 10:38:11 +0200 (CEST) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id D58361B955 for ; Tue, 9 Jul 2019 10:38:09 +0200 (CEST) Received: by mail-wr1-f67.google.com with SMTP id z1so15387752wru.13 for ; Tue, 09 Jul 2019 01:38:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=poLXPGUO/ntP/XgMH3clYjaDs7kSDmfnsgJu4PylV0c=; b=Db/m3CLbqaoyYrrz9KbqNGQ2OAHhEDFeGoasz1yIVdC5+qp4CePLsfzyIKcULIgaDZ mI1BZQEc81m+jorQdS9D4tElni/pf2JRyyDiwPXUebfK6/YVQPtLp/6k7LjKzFrgKr3O EhBzW69/L14EY2UbaqHUctEfLKx+eSqlW0VZulkMkp3+2JaK2w9ds32+ZaLQfdNWc/u3 4q7vzG84eu3QVhQ2p6MCsCpirCO8Ort3rQUdA6QE8J46RUlsE3kl0Kpi0hXqN0HywiDp FuoBLNAOmiFWp+kbmw4o0Qttx8szDEkkBRWY5VIiB0lz96lED1O537BYDo7herBQe1GX APtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=poLXPGUO/ntP/XgMH3clYjaDs7kSDmfnsgJu4PylV0c=; b=pZ9jNYVRvc+uhJ8BJYJvqVvug5sd6ZWrI/A65e3kcAPSHs2cjyKzrUDT920xoI/1LN cTDf5jpgj1gSSZemBC6ikC0CJbzQlSkMBzAuuEq3+92d5WYVzAe7nal8AGG5OcbBeIGh x47/+IvEeB6LK8HbNwPFv9H59dMIOWRrBa2u9snfETLnLSOPKwFs6XI9bl/xu4t3afAL AGQx7vP/OpCXZby0oaQ0wLVLicS+yk0yxDMaFXRyj0Tw0w9eSR6RHp6cutRa0NVHABtm kYZoIQ6Apo8MPxbtO0mfDE+fLRQm3I+5j2OCAsR7aw0jk87LgRoZdkAYjs9cSLotRz1L wOBA== X-Gm-Message-State: APjAAAXqxFkaNNYjAdIRUGhHDPED2Iuvek+G5zQKkZLMQTZCAbFKjuaR ZTPZsnf4/G2giVluKEwYs/BKFg== X-Google-Smtp-Source: APXvYqwsE8REt+P+xwpZYOwNOZznkOeexIDE7JnN/97tnIo3SWvLw10Osvi8e+lGHIfIrAmpOU+N6Q== X-Received: by 2002:adf:edd1:: with SMTP id v17mr23446131wro.348.1562661489549; Tue, 09 Jul 2019 01:38:09 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id h14sm17173703wrs.66.2019.07.09.01.38.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 09 Jul 2019 01:38:08 -0700 (PDT) Date: Tue, 9 Jul 2019 10:38:06 +0200 From: Adrien Mazarguil To: Yongseok Koh Cc: Shahaf Shuler , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Olivier Matz , dev , Slava Ovsiienko Message-ID: <20190709083806.GS4512@6wind.com> References: <20190603213231.27020-3-yskoh@mellanox.com> <20190704232302.19582-1-yskoh@mellanox.com> <20190705135404.GR4512@6wind.com> <6EE319CD-4BBC-47BF-AAE5-2165B8C1D491@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6EE319CD-4BBC-47BF-AAE5-2165B8C1D491@mellanox.com> 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 Fri, Jul 05, 2019 at 06:05:50PM +0000, Yongseok Koh wrote: > > On Jul 5, 2019, at 6:54 AM, Adrien Mazarguil wrote: > > > > 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 pattern > >> 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. > >> > >> Some device expose internal registers of its flow processing pipeline and > >> those registers are quite useful for stateful connection tracking as it > >> keeps status of flow matching. Multiple tags are supported by specifying > >> index. > >> > >> Example testpmd commands are: > >> > >> 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 > >> > >> 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 > >> > >> flow create 0 ingress group 1 > >> pattern tag index is 2 value spec 0xaa00bb value mask 0xffff00ff / > >> eth ... / end > >> actions ... jump group 2 / end > >> > >> 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 > >> > >> flow create 0 ingress group 2 > >> pattern tag index is 3 value spec 0x123456 value mask 0xffffff / > >> eth ... / end > >> actions ... / end > >> > >> Signed-off-by: Yongseok Koh > > > > Hi Yongseok, > > > > Only high level questions for now, while it unquestionably looks useful, > > from a user standpoint exposing the separate index seems redundant and not > > necessarily convenient. Using the following example to illustrate: > > > > actions set_tag index 3 value 0x123456 mask 0xfffff > > > > pattern tag index is 3 value spec 0x123456 value mask 0xffffff > > > > I might be missing something, but why isn't this enough: > > > > pattern tag index is 3 # match whatever is stored at index 3 > > > > Assuming it can work, then why bother with providing value spec/mask on > > set_tag? A flow rule pattern matches something, sets some arbitrary tag to > > be matched by a subsequent flow rule and that's it. It even seems like > > relying on the index only on both occasions is enough for identification. > > > > Same question for the opposite approach; relying on the value, never > > mentioning the index. > > > > I'm under the impression that the index is a hardware-specific constraint > > 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 through the > > public API. > > > Thank you for review, Adrien. > Hope you are doing well. It's been long since we talked each other. :-) Yeah clearly! Hope you're doing well too. I'm somewhat busy hence slow to answer these days... hey! no private talks! Back to the topic: > Your approach will work too in general but we have a request from customer that > they want to partition this limited tag storage. Assuming that HW exposes 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 tag, 8bit > table id and 8bit flow id. As they want to split one 32bit storage, I thought it > is better to provide mask when setting/matching the value. Even some customer > wants to store multiple flags bit by bit like ol_flags. They do want to alter > only partial bits. > > 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 32b > storage which users can use for their own sake. > tag[0], tag[1], tag[2], tag[3] 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? Out of curiosity, are these registers independent from META and other items/actions in mlx5, otherwise what happens if they are combined? 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? 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. 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 what can an application do with this that couldn't be done in a more generic fashion through META? 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 common for PMDs to expose a public header that dedicated applications can include to use this kind of features (look for rte_pmd_*.h, e.g. rte_pmd_ixgbe.h). No problem with that. [1] "[PATCH] ethdev: extend flow metadata" https://mails.dpdk.org/archives/dev/2019-July/137305.html [2] "Negative types" https://doc.dpdk.org/guides/prog_guide/rte_flow.html#negative-types -- Adrien Mazarguil 6WIND