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 4EEA3A04B1; Mon, 7 Sep 2020 18:13:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ADAC91C0C9; Mon, 7 Sep 2020 18:13:14 +0200 (CEST) Received: from mail-qv1-f66.google.com (mail-qv1-f66.google.com [209.85.219.66]) by dpdk.org (Postfix) with ESMTP id 8E0951C0BE for ; Mon, 7 Sep 2020 18:13:13 +0200 (CEST) Received: by mail-qv1-f66.google.com with SMTP id b13so6562081qvl.2 for ; Mon, 07 Sep 2020 09:13:13 -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=/igyRC5uHw1VlBs3L54uya8hRqbRm5up/BucRTNbSeU=; b=Rv68XdYaQ4Y8dAQzCcP2y+9NYuuYg7nYO0zbFgxKAVSzXZ7JFC7eOosxblhrDPHfY+ Zk/gw3LvTVV61Ybv8hO+ToD3urtWzoqwoAc6k9v/Czb0+q7RxBecZlaJ1vec7tFndifQ XgKZjpf9bgSG6J8dMtytCsYhSIyz3zTMRcc4893+qhoOaeygFTJZvncE75tZp9GolJVt AGHZ6BQTTyVsoBglSd7lieQTwmgn81xFfdKsEHLWStF0PrHTuRaMxTZBYI3NsPpQGa3z 0Z+WyV9kVg0bOAqdR0BEmqSUu4xrnDBndmrJe9gHAhs9EVDGUYxY/1o/7LksEseMKJQu 4tVA== 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=/igyRC5uHw1VlBs3L54uya8hRqbRm5up/BucRTNbSeU=; b=J4EKGxFJco6maIYjQVlRq0M/CSF61YYWGQChsXyU3hDwNqggsavQFKW4fqyvUn9ePk oY4UFMRYhkQFB46tWgTsq4p+hx1Y0g6I0em8jKLQTwq1pLmySoEhf4YRm5ICJL0Er6gv w3DxVt9hUZtM7Tsb1E01MctfN2swt59kskKyOJ7EUXSuWg8zYEi2zGzpD0GEKsb3VtlL cilhcQZynbemH/vqy5BbN4ZEcQQ4fGuU1SsQQ8v/C9LHag+kHHISL8zfxAo/AQFz6yvr EIUIGnV/tVRjDpkEDvLqgIgehAo4xxDYsJPr1KCLcaEkmOLvZ2y3gXUrVOS2sM+LgvRz iVxg== X-Gm-Message-State: AOAM53204Dzi3yKfIYCXbNAjNSKspxm8NJIlTN9OE4PfM4LYB7M25t1s KtiTAT2OC+2TLyA9CRFzx8N7HE2oskZDDMZB1zKCFA== X-Google-Smtp-Source: ABdhPJxc5SoM70GQPJHb7ZUqMKUTS1z9iPXiEY8AaAG0B5VvmA+av/XKLwAm72kS0+5RjxKz3nunpzJOGjNKqCTWl0I= X-Received: by 2002:a0c:fd2c:: with SMTP id i12mr17836162qvs.47.1599495192794; Mon, 07 Sep 2020 09:13:12 -0700 (PDT) MIME-Version: 1.0 References: <6e8b7c61a92b51749b11ac3bfae5c0201352f9b3.1596550675.git.dekelp@mellanox.com> <3730207f-8882-094e-16f8-3680c80096c1@mellanox.com> In-Reply-To: From: Maxime Leroy Date: Mon, 7 Sep 2020 18:13:01 +0200 Message-ID: To: Dekel Peled Cc: Eli Britstein , "ferruh.yigit@intel.com" , "arybchenko@solarflare.com" , Ori Kam , Thomas Monjalon , Asaf Penso , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC] ethdev: add VLAN attributes to ETH item 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, First, I don't understand the initial change [1] done on RTE_FLOW API. Before this change, it was possible to match any packets with or without vlan encapsulations. At least, it's not anymore the case with the mlx5 pmd. For example, if I want to match any ssh packets whatever if it's encapsulated with no vlan or N vlan headers: testpmd> flow create 0 ingress pattern eth type spec 0 type mask 0 / ipv4 / udp dst is 22 / end actions mark id 2 / queue index 0 / end By setting the ethernet type mask to 0x0, it means that ethernet type should be ignored. It means if ethernet type is 0x800 (i.e. ipv4) or 0x8100 (i.e. vlan) or 0x88A8 (qinq), the packet should be matched. But if you wanted to match only ethernet packets (and not vlan/qinq one), you can create the following flows: testpmd> flow create 0 ingress pattern eth type spec 0x800 type mask 0xffff / ipv4 / udp dst is 22 / end actions mark id 2 / queue index 0 / end With your new RFC, first I don't understand the needs of the num_of_vlans field. You can create the following follow if you want to match any qinq / ipv4 packets (i.e. 2 vlan level) for example: > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff / vlan inner_type spec is 0x8100 mask is 0xFFFF / vlan / end actions mark id 2 / queue index 0 / end Could you explain to me the utility of this new field ? The cvlan_exist, and svlan_exist seems useless for me. For me, you can already do the same thing with type field. Because, by setting the type mask to 0, you can already give the notion of any ethertype. Let's take some example: 1. with svlan_exists=0, cvlan_exists=0, it can already be configured like that: > flow create 0 ingress pattern eth type spec 0x800 type mask 0xFFFF / ipv4 / end actions mark id 2 / queue index 0 / end 2. With svlan_exists=0, cvlan_exists=1: > flow create 0 ingress pattern eth type spec 0x8100 type mask 0xFFFF / vlan inner_type is 0x800 mask is 0xFFFF / ipv4 / end actions mark id 2 / queue index 0 / end 3. With svlan_exists=1, cvlan_exists=0: it doesn't seem to be a real use case. Anyway, you could have: > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff / vlan inner_type spec is 0x800 mask is 0xFFFF / ipv4 / end actions mark id 2 / queue index 0 / end 4. With svlan_exists=1, cvlan_exists=1: > flow create 0 ingress pattern eth type spec 0x88A8 type mask 0xffff / vlan inner_type spec is 0x8100 mask is 0xFFFF / vlan spec is 0x800 mask 0xFFFF / ipv4 / end actions mark id 2 / queue index 0 / end Could you please explain to me what you try to achieve with this RFC ? I would like to know why ether_type value setted by the user is ignored when I create the following rule: testpmd> flow create 0 ingress pattern eth type spec 0 type mask 0 / ipv4 / udp dst is 22 / end actions mark id 2 / queue index 0 / end with the mlx5 pmd ? (i.e. why this change [1].) [1] http://mails.dpdk.org/archives/dev/2020-May/166257.html Best Regards, Maxime On Wed, Aug 5, 2020 at 9:04 AM Matan Azrad wrote: > > But if the user want to force only one vlan and don't care about others? > > > > -----Original Message----- > > From: Dekel Peled > > Sent: Wednesday, August 5, 2020 9:54 AM > > To: Eli Britstein ; ferruh.yigit@intel.com; > > arybchenko@solarflare.com; Ori Kam ; Thomas > > Monjalon > > Cc: Asaf Penso ; Matan Azrad > > ; dev@dpdk.org > > Subject: RE: [RFC] ethdev: add VLAN attributes to ETH item > > > > Thanks, PSB. > > > > > -----Original Message----- > > > From: Eli Britstein > > > Sent: Tuesday, August 4, 2020 6:47 PM > > > To: Dekel Peled ; ferruh.yigit@intel.com; > > > arybchenko@solarflare.com; Ori Kam ; Thomas > > > Monjalon > > > Cc: Asaf Penso ; Matan Azrad > > ; > > > dev@dpdk.org > > > Subject: Re: [RFC] ethdev: add VLAN attributes to ETH item > > > > > > > > > On 8/4/2020 6:36 PM, Dekel Peled wrote: > > > > In existing code the match on tagged/untagged packets is not explicit. > > > > Recent documentation update [1] describes the different patterns and > > > > clarifies the intended use of different patterns. > > > > > > > > This patch proposes an update to ETH item struct, to clearly define > > > > the required characteristic of a packet, and enable precise match criteria. > > > > > > > > [1] http://mails.dpdk.org/archives/dev/2020-May/166257.html > > > > > > > > Signed-off-by: Dekel Peled > > > > --- > > > > lib/librte_ethdev/rte_flow.h | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > > b/lib/librte_ethdev/rte_flow.h index cf0eccb..345feb5 100644 > > > > --- a/lib/librte_ethdev/rte_flow.h > > > > +++ b/lib/librte_ethdev/rte_flow.h > > > > @@ -726,11 +726,20 @@ struct rte_flow_item_raw { > > > > * 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 fields @p cvlan_exist and @p svlan_exist can be used to > > > > + match specific > > > > + * packet types, instead of using the @p type field. This can be > > > > + used to match > > > > + * on packets that do/don't contain either cvlan, svlan, or both. > > > > + * The field @p num_of_vlans can be used to match packets by the > > > > + exact number > > > > + * of VLANs in header. > > > > */ > > > > 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 cvlan_exist:1; /**< C-tag VLAN exist in header. */ > > > > + uint32_t svlan_exist:1; /**< S-tag VLAN exist in header. */ > > > > + uint32_t reserved:14; /**< Reserved, must be zero. */ > > > > + uint32_t num_of_vlans:16; /**< Number of VLANs in header. */ > > > We can deduct from num_of_vlans the values of cvlan_exist/svlan_exist, > > > so those are redundant fields. Keeping them introduce a conflicting > > > match. For example num_of_vlans=0 and cvlan_exist=1. > > > > Such conflict is simple to validate and reject. > > Even if num_of_vlans is removed, we can still get conflict svlan_exist=1, > > cvlan_exist=0. > > The different fields are proposed to allow flexible match on different VLAN > > attributes. > > Every PMD can choose to support any or none of them. > > > > > > }; > > > > > > > > /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */