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 4BDC7A04B5; Fri, 2 Oct 2020 14:39:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 30E751D68D; Fri, 2 Oct 2020 14:39:26 +0200 (CEST) Received: from mail-qt1-f194.google.com (mail-qt1-f194.google.com [209.85.160.194]) by dpdk.org (Postfix) with ESMTP id C9B721D5F7 for ; Fri, 2 Oct 2020 14:39:23 +0200 (CEST) Received: by mail-qt1-f194.google.com with SMTP id m9so355171qth.7 for ; Fri, 02 Oct 2020 05:39:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=E6HUGmywaTGFM3R1uhaIH9JCuDCDwepMH+P1bGDRe38=; b=T+pkPj8/QixUeYEW+7A55Yy3+YPtEHDhH/HrgRolBfJubuyzrVAQqI8ufL7sNqNXXG J37HB4z1c/ZoHqo2vP7Htp9PKFYnbO157h2XJOhJQUTQAylWLvyXbN2Y68ikOjJtMNtl sNR5ihAxKoQmEAJ0q2N/3QhrGASmT4wzU7XW8+P3Wid9moX/00vZaKpcwRWhDFyjaaWt YC5ZNMH1uNZNe3g45MTJD4PQbQU0fn6VRC0bnhHOjm/BFAeMRTGYMPKJyli82nBQID/4 MPOXLJAUwLwT4eEv4tFFqf5ueMFgcn8cIsWVL3NVelm4DGZL3DVhIulZPQs/Z0J9KheI 2t3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=E6HUGmywaTGFM3R1uhaIH9JCuDCDwepMH+P1bGDRe38=; b=Gsme9vFVPR7LTcIqfJSzx+mbThCXTl7DCFxc+0WXlrUufPybdXMebO8SifA6HF7wMO rJ8lkstaRM4j5Wav0w1oTDiyz5G/ZMlUsrLHvWx13DZg7j/rxW9Px6xhvyh5Ta95uaTB s8YZvnnVi/kyhm9NOmf0rhL65PCfzMFVdH3VnC6cS8JgYZzCqAFz4TNzvjluUPqv21F6 674eijsvPE6DnRZE5ctGisiFYIejwN642lExCrkxhFfXx6L1vNJtR+ezuCa0oa2O/YS9 MOx/9fytuLtUtvizAq/pc1YIsvEYk0zMjALiMcS3rkfIl79xgbkemssrpRWtnBhu7HbO wPWw== X-Gm-Message-State: AOAM530vn5BznSg7Rh0eApAdhBenSbsr0Dqb6yPAN5y2+rDo2vJgmzAm y+LRMAkpNIaUOp4uq/789hsSU63CTeJjUIu8kh8lcw== X-Google-Smtp-Source: ABdhPJz5HA3Brk7/Qi6mgp68fHgAgAzKxwm2YzbO8NOj9QwaZulvjBTqONkShK0JRiZB/N99+3xZpHB8NVmrn2U3DjQ= X-Received: by 2002:ac8:5205:: with SMTP id r5mr1929873qtn.371.1601642363192; Fri, 02 Oct 2020 05:39:23 -0700 (PDT) MIME-Version: 1.0 References: <209f5087596180d7866a43f0a0f12c9a032eb7ce.1601577847.git.dekelp@nvidia.com> In-Reply-To: <209f5087596180d7866a43f0a0f12c9a032eb7ce.1601577847.git.dekelp@nvidia.com> From: Maxime Leroy Date: Fri, 2 Oct 2020 14:39:12 +0200 Message-ID: To: Dekel Peled Cc: Ori Kam , Thomas Monjalon , ferruh.yigit@intel.com, arybchenko@solarflare.com, dev@dpdk.org, Dekel Peled Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] ethdev: add VLAN attributes to ETH and VLAN items 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" Hi Dekel, On Thu, Oct 1, 2020 at 8:49 PM Dekel Peled wrote: > > From: Dekel Peled > > This patch implements the change proposes in RFC [1], adding dedicated > fields to ETH and VLAN items structs, to clearly define the required > characteristic of a packet, and enable precise match criteria. > > [1] https://mails.dpdk.org/archives/dev/2020-August/177536.html > > Signed-off-by: Dekel Peled > --- > doc/guides/rel_notes/release_20_11.rst | 7 +++++++ > lib/librte_ethdev/rte_flow.h | 16 +++++++++++++--- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst > index 7f9d0dd..199c60b 100644 > --- a/doc/guides/rel_notes/release_20_11.rst > +++ b/doc/guides/rel_notes/release_20_11.rst > @@ -173,6 +173,13 @@ API Changes > * ``_rte_eth_dev_callback_process()`` -> ``rte_eth_dev_callback_process()`` > * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()`` > > +* ethdev: Added new field ``vlan_exist`` to structure ``rte_flow_item_eth``, > + indicating that at least one VLAN exists in the packet header. > + > +* ethdev: Added new field ``more_vlans_exist`` to structure > + ``rte_flow_item_vlan``, indicating that at least one more VLAN exists in > + packet header, following this VLAN. > + > * rawdev: Added a structure size parameter to the functions > ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``, > ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > index da8bfa5..39d04ef 100644 > --- a/lib/librte_ethdev/rte_flow.h > +++ b/lib/librte_ethdev/rte_flow.h > @@ -723,14 +723,18 @@ struct rte_flow_item_raw { > * If the @p type field contains a TPID value, then only tagged packets with the > * specified TPID will match the pattern. > * Otherwise, only untagged packets will match the pattern. > - * If the @p ETH item is the only item in the pattern, and the @p type field > - * is not specified, then both tagged and untagged packets will match the > - * pattern. > + * The field @p vlan_exist can be used to match specific packet types, instead > + * of using the @p type field. > + * This can be used to match any type of tagged packets. > + * If the @p type and @p vlan_exist fields are not specified, then both tagged > + * and untagged packets will match the pattern. > */ > struct rte_flow_item_eth { > struct rte_ether_addr dst; /**< Destination MAC. */ > struct rte_ether_addr src; /**< Source MAC. */ > rte_be16_t type; /**< EtherType or TPID. */ > + uint32_t vlan_exist:1; /**< At least one VLAN exist in header. */ > + uint32_t reserved:31; /**< Reserved, must be zero. */ > }; To resume: - type and vlan_exists fields not specified: tag and untagged matched - with vlan_exists, match only tag or untagged - with type matching specific ethernet type - vlan_exists and type should not setted at the same time ? With this new specification, I think you address all the use cases. That's great ! > > /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */ > @@ -752,10 +756,16 @@ struct rte_flow_item_eth { > * the preceding pattern item. > * If a @p VLAN item is present in the pattern, then only tagged packets will > * match the pattern. > + * The field @p more_vlans_exist can be used to match specific packet types, > + * instead of using the @p inner_type field. > + * This can be used to match any type of tagged packets. > */ Could you please specify what the expected behavior when inner_type and more_vlans_exist are not specified . What is the default behavior ? > struct rte_flow_item_vlan { > rte_be16_t tci; /**< Tag control information. */ > rte_be16_t inner_type; /**< Inner EtherType or TPID. */ > + uint32_t more_vlans_exist:1; > + /**< At least one more VLAN exist in header, following this VLAN. */ > + uint32_t reserved:31; /**< Reserved, must be zero. */ > }; > > /** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */ > -- > 1.8.3.1 > I am still wondering, why not using a new item 'NOT' for example to match only eth packet not tagged ? example: eth / not vlan. It's a more generic solution. Here in this commit, we add a reference on VLAN fields on ethernet header. But tomorrow, we could do the same for mpls by adding mpls_exists in the eth item and so on. In fact, we have the same needs for IPv6 options. To match for example, ipv6 packet with no fragment option. With a NOT field, it can be easily done: > eth / ipv6 / no ipv6_frag. Adding new fields 'item'_exists into eth and ipv6 do the jobs, but having a NOT attribute is a more generic solution. It could address many other use cases like matching any udp packets that are not vxlan ( eth / ipv4 / vxlan / not udp), Let me know what you think about that. Regards, Maxime