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 F19E0A00BE; Wed, 30 Oct 2019 17:13:18 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 684591C034; Wed, 30 Oct 2019 17:13:18 +0100 (CET) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 3FE0C1BFF9 for ; Wed, 30 Oct 2019 17:13:17 +0100 (CET) Received: by mail-wm1-f68.google.com with SMTP id q130so2807099wme.2 for ; Wed, 30 Oct 2019 09:13:17 -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:user-agent; bh=OizZh6+3LfSfvFc0/I3+I/eLv9RQYkAwpRQg0AibWd0=; b=M3sVUHgC2g6Oi7MQsH6e2Rd5ZpPuGib5IorIG1WuQ3pgp8w2PRMB5ewn8P1pzMQ7H0 9VdRh8nCwobPzi641Az+VzQpwPFFwYEPEowoLPnVpzLmRneuAN30zr9YXE9Ss+AfvZix 15UYZTs9JGmNe6gn8EqVgXoshKcVl1pAs7u+O+g6FM223XTkmipJh9YkuvG0zKx/DRsJ cY1CzfTzPDFJyx8gSJMSvHQFJfvtY0o1XvpwKgiuEmJq0GXaiBQs5z0sqFx0j0qW6+iW uwsLmENz76lNzfCBgv+TFUd7174EEb11pdALEuJKA27abzJkpCVvtZgBg2kh9PTrMmti 6aNw== 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:user-agent; bh=OizZh6+3LfSfvFc0/I3+I/eLv9RQYkAwpRQg0AibWd0=; b=f8SIZEWx8yjieBr8lQ53kvpUbfxcd3cEVuO2GsX+80hkKRwsrkVYClyOAR/TgpZIXs iO2AgSCrG4/DCuJJh8Ls7tdVJuTR4VZ6ACrnpE5cqxfxGHF4U7/8deULv76P0TaBALQY XvRIdz5EnvoX8Cn7qirMEZiJa9jf3KJmzssfYhEslO8Eo1G5UwXG7ov4JBEfmqeZL5wR 14lMEqHn5GLYitfj1POiCTT7nmgUR2L5J4LD2/+rgHDMgnIP1X/GZZpuRg5uCziSpiZO Xj72uIXpwmfNXgzaVduxdVYHGmM1lukl512h/l+4tuU6QtpWdXfOHQORt2fsQiAn5pz6 debw== X-Gm-Message-State: APjAAAWatBPLDFF+q2PIBACxRALzDQ0RLc7HILV3vcr5VGxKQoqogj9I nDiU/+rBWVL2jbFVQjGYHL7+pg== X-Google-Smtp-Source: APXvYqxGDJR/SY5Vg7ZIwJLV1GmtNA0IkkjC06yuAS0j/7PfalrU0wZelqOtZDPYlX3gMGlCsGEPaw== X-Received: by 2002:a7b:cd83:: with SMTP id y3mr266615wmj.150.1572451996830; Wed, 30 Oct 2019 09:13:16 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a6000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id l18sm853560wrn.48.2019.10.30.09.13.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2019 09:13:15 -0700 (PDT) Date: Wed, 30 Oct 2019 17:13:15 +0100 From: Olivier Matz To: Slava Ovsiienko Cc: Andrew Rybchenko , "dev@dpdk.org" , Matan Azrad , Raslan Darawsheh , Thomas Monjalon Message-ID: <20191030161315.slvgrwi42ry2exwd@platinum> References: <1572201636-16374-1-git-send-email-viacheslavo@mellanox.com> <1572377502-13620-1-git-send-email-viacheslavo@mellanox.com> <0533b4e8-5bd8-2450-ac3a-b4b190423289@solarflare.com> <20191030152023.q3aubtc6u56afhvn@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20180716 Subject: Re: [dpdk-dev] [PATCH v5] ethdev: extend flow metadata 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 Wed, Oct 30, 2019 at 03:58:04PM +0000, Slava Ovsiienko wrote: > > -----Original Message----- > > From: Olivier Matz > > Sent: Wednesday, October 30, 2019 17:20 > > To: Slava Ovsiienko > > Cc: Andrew Rybchenko ; dev@dpdk.org; > > Matan Azrad ; Raslan Darawsheh > > ; Thomas Monjalon > > Subject: Re: [PATCH v5] ethdev: extend flow metadata > > > > Hi, > > > > On Wed, Oct 30, 2019 at 02:46:06PM +0000, Slava Ovsiienko wrote: > > > > -----Original Message----- > > > > From: Slava Ovsiienko > > > > Sent: Wednesday, October 30, 2019 16:41 > > > > To: Andrew Rybchenko ; dev@dpdk.org > > > > Cc: Matan Azrad ; Raslan Darawsheh > > > > ; Thomas Monjalon ; > > > > olivier.matz@6wind.com > > > > Subject: RE: [PATCH v5] ethdev: extend flow metadata > > > > > > > > > -----Original Message----- > > > > > From: Andrew Rybchenko > > > > > Sent: Wednesday, October 30, 2019 10:02 > > > > > To: Slava Ovsiienko ; dev@dpdk.org > > > > > Cc: Matan Azrad ; Raslan Darawsheh > > > > > ; Thomas Monjalon > > ; > > > > > olivier.matz@6wind.com; Yongseok Koh > > > > > Subject: Re: [PATCH v5] ethdev: extend flow metadata > > > > > > > > > > On 10/29/19 10:31 PM, Viacheslav Ovsiienko wrote: > > > > > > Currently, metadata can be set on egress path via mbuf > > > > > > tx_metadata field with PKT_TX_METADATA flag and > > > > > > RTE_FLOW_ITEM_TYPE_META > > > > > matches metadata. > > > > > > > > > > > > This patch extends the metadata feature usability. > > > > > > > > > > > > 1) RTE_FLOW_ACTION_TYPE_SET_META > > > > > > > > > > > > When supporting multiple tables, Tx metadata can also be set by > > > > > > a rule and matched by another rule. This new action allows > > > > > > metadata to be set as a result of flow match. > > > > > > > > > > > > 2) Metadata on ingress > > > > > > > > > > > > There's also need to support metadata on ingress. Metadata can > > > > > > be set by SET_META action and matched by META item like Tx. The > > > > > > final value set by the action will be delivered to application > > > > > > via metadata dynamic field of mbuf which can be accessed by > > > > > > RTE_FLOW_DYNF_METADATA() macro or with > > > > > > rte_flow_dynf_metadata_set() and rte_flow_dynf_metadata_get() > > > > > > helper routines. PKT_RX_DYNF_METADATA flag will be set along with > > the data. > > > > > > > > We have a problem with PKT_RX_DYNF_METADATA/ > > PKT_TX_DYNF_METADATA. > > > > These ones are referencing to global variable > > > > "rte_flow_dynf_metadata_mask". > > > > And there are routines in rte_mbuf.c (rte_get_rx_ol_flag_list) > > > > which show the names of flags. It is not good if rte_mbuf.c would > > > > require including of rte_flow.h and linking rte_flow.c (not all apps use > > rte_flow or even ethdev). > > > > > > > > What do you think? Should we rename rte_flow_dynf_xxxxx variables to > > > > rte_mbuf_dynf_flow_metadata_xxxx and put ones into the > > rte_mbuf_dyn.c? > > > > The same about PKT_RX_DYNF_METADATA definition, is it candidate to > > > > move to rte_mbuf_dyn.h ? It would allow not to link or include > > > > rte_flow.c/h into rte_mbuf.c > > > > > > > > In rte_mbuf_dyn.c, we maintain a list of registered flags. I think it wouldn't > > be too difficult to introduce the equivalent of > > rte_get_*_ol_flag_list() and *rte_get_*_ol_flag_name() for dynamic flags. > > There is already a dump function (which does both fields and flags), and a > > lookup by name function. > > Nice idea, thanks. I think existing lookup by name is OK, I'll update > flag list routines with dynamic flag support. > > > > > Maybe we could split the dump into fields and flags, and add a lookup by > > offset/bitnum. Would it work for your use-case? > > Not needed right now, I think. > > > > > > It is interesting to note that despite metadata field looks to be > > > related to rte_flow, there is no any reference to this field or flags inside > > rte_flow API implementation. > > > Only datapath references this field. Metadata is gateway between flow > > > HW space and datapath, it tends to be mostly on datapath side not on > > rte_flow. > > > > Yes, only the registration of the field is related to rte_flow. But I don't get > > where are you going with this. Is it a problem? > > It is not a problem, rather some concern. > IMO, the datapath related entities (flag mask and field offset) are put in not appropriate place. > Now we have to add lookup to flag list routines, which we could avoid. The next candidates, > like timestamp or timesync might introduce some new issues. If the underlying question is should we centralize or not centralize the definitions of dynamic fields/flags helpers, I'll tend to say that we should not centralize. The reason is because it will not always be possible: an application or an external library is allowed to register a private dynamic field. Nevertheless, as you say, introducing the next dynamic fields like timestamp may show up some new issues, and we should be ready to rework what has been done when we'll have more experience, with more usages of dynamic fields.