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 9F4C3A04FA; Tue, 2 Jun 2020 21:04:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B56851BEA3; Tue, 2 Jun 2020 21:04:15 +0200 (CEST) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id 7F9311BE99 for ; Tue, 2 Jun 2020 21:04:14 +0200 (CEST) Received: by mail-wr1-f65.google.com with SMTP id l10so4473918wrr.10 for ; Tue, 02 Jun 2020 12:04:14 -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:content-transfer-encoding:in-reply-to; bh=HsTe0XLPJ/CJlB831GDCfgi+pCm4tLMB9l6ZuMpG9f8=; b=EuJhP/c8oUE7XbnNd0/cbI47t4nR6+aToc/dFo5uU4h1pmgkIA/oSjZ26ElOt8CR4J az1IN8vZ8ns70hN5WgALNdikK6gLQNJRIWd9M/gOf/rwi41SYo0HiPU8k70I5n7meNlf bfO+HuZ6za6Uij4iZ3IoFSYnq5DZ0OM7vafbmh0Acxq2BMZVouzUS5AwX02qdzL60jjl aKRs4jhq8Q4ASz+XwSM0cNBKgZ7IbYryQaCrIGgnBTiwv38iDjEL5QQT2I4U5RQVdzkB m+fCsoIhTiCrOlpPRWPjJs5Q8F+3MLRQbhnGnwKTA5CiUti3dThJZZGhqJbsYHaWx3EB ztlA== 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:content-transfer-encoding :in-reply-to; bh=HsTe0XLPJ/CJlB831GDCfgi+pCm4tLMB9l6ZuMpG9f8=; b=ctumSi4Fun/NFem8IQTNzX5LxRA2YGCmsqglpegcH96p2XuMPVvOLgUnOP8nkNZFZH jewCSZtrYNnpJ0oqxcpQVV+Nd2ofc8gzgnNopULsvplUTL6ZXzvCXC1RrNjk+w2Fjr8B pNCrG/U6fvBCalCiDMKKA4FtkXL2x6Ehi5IwYRdk25Xa9T/NWA86QwLrSDPOwG4uAj5Q vn3RvKfLfqlXNAMFdB/F6PDaxhi+lvv9d+xcAss7rNuzuHzlsIHrvW2/kJEjnp29fPXI IVhL0Yp9WB8MG5FDZ1Xx2r7NHtMO6xID1337LP3xIGRpgb9D1V3Emi912fiiSa6Dgf80 i7qA== X-Gm-Message-State: AOAM531mMES1byrwAig4klxhw8dqXZm9l9If5c/mV8KXXEJdT4a5SlEB Kjfy68VfKNnuDDqebFkfzgM4PQ== X-Google-Smtp-Source: ABdhPJw58R6O8AJPcoYvD6bhMm4NLaYYMsbEt8XgxP3v8xdqHuegGLhYm1/DJ3giq/tj4VIa4Wab6A== X-Received: by 2002:a5d:6884:: with SMTP id h4mr29573902wru.198.1591124653956; Tue, 02 Jun 2020 12:04:13 -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 88sm5825578wre.45.2020.06.02.12.04.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Jun 2020 12:04:12 -0700 (PDT) Date: Tue, 2 Jun 2020 21:04:10 +0200 From: Adrien Mazarguil To: Ori Kam Cc: Andrew Rybchenko , Dekel Peled , "ferruh.yigit@intel.com" , "john.mcnamara@intel.com" , "marko.kovacevic@intel.com" , Asaf Penso , Matan Azrad , Eli Britstein , "dev@dpdk.org" , Ivan Malov Message-ID: <20200602190410.GU26320@6wind.com> References: <5f9b4d30b81fa68ed875106785419a43cc7a6166.1590935677.git.dekelp@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [dpdk-dev] [RFC] ethdev: add fragment attribute to IPv6 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 Ori, Andrew, Delek, (been a while eh?) On Tue, Jun 02, 2020 at 06:28:41PM +0000, Ori Kam wrote: > Hi Andrew, > > PSB, [...] > > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > > > index b0e4199..3bc8ce1 100644 > > > --- a/lib/librte_ethdev/rte_flow.h > > > +++ b/lib/librte_ethdev/rte_flow.h > > > @@ -787,6 +787,8 @@ struct rte_flow_item_ipv4 { > > > */ > > > struct rte_flow_item_ipv6 { > > > struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */ > > > + uint32_t is_frag:1; /**< Is IPv6 packet fragmented/non-fragmented. */ > > > + uint32_t reserved:31; /**< Reserved, must be zero. */ > > > > The solution is simple, but hardly generic and adds an > > example for the future extensions. I doubt that it is a > > right way to go. > > > I agree with you that this is not the most generic way possible, > but the IPV6 extensions are very unique. So the solution is also unique. > In general, I'm always in favor of finding the most generic way, but sometimes > it is better to keep things simple, and see how it goes. Same feeling here, it doesn't look right. > > May be we should add 256-bit string with one bit for each > > IP protocol number and apply it to extension headers only? > > If bit A is set in the mask: > > - if bit A is set in spec as well, extension header with > > IP protocol (1 << A) number must present > > - if bit A is clear in spec, extension header with > > IP protocol (1 << A) number must absent > > If bit is clear in the mask, corresponding extension header > > may present and may absent (i.e. don't care). > > > There are only 12 possible extension headers and currently none of them > are supported in rte_flow. So adding a logic to parse the 256 just to get a max of 12 > possible values is an overkill. Also, if we disregard the case of the extension, > the application must select only one next proto. For example, the application > can't select udp + tcp. There is the option to add a flag for each of the > possible extensions, does it makes more sense to you? Each of these extension headers has its own structure, we first need the ability to match them properly by adding the necessary pattern items. > > The RFC indirectly touches IPv6 proto (next header) matching > > logic. > > > > If logic used in ETH+VLAN is applied on IPv6 as well, it would > > make pattern specification and handling complicated. E.g.: > > eth / ipv6 / udp / end > > should match UDP over IPv6 without any extension headers only. > > > The issue with VLAN I agree is different since by definition VLAN is > layer 2.5. We can add the same logic also to the VLAN case, maybe it will > be easier. > In any case, in your example above and according to the RFC we will > get all ipv6 udp traffic with and without extensions. > > > And how to specify UPD over IPv6 regardless extension headers? > > Please see above the rule will be eth / ipv6 /udp. > > > eth / ipv6 / ipv6_ext / udp / end > > with a convention that ipv6_ext is optional if spec and mask > > are NULL (or mask is empty). > > > I would guess that this flow should match all ipv6 that has one ext and the next > proto is udp. In my opinion RTE_FLOW_ITEM_TYPE_IPV6_EXT is a bit useless on its own. It's only for matching packets that contain some kind of extension header, not a specific one, more about that below. > > I'm wondering if any driver treats it this way? > > > I'm not sure, we can support only the frag ext by default, but if required we can support other > ext. > > > I agree that the problem really comes when we'd like match > > IPv6 frags or even worse not fragments. > > > > Two patterns for fragments: > > eth / ipv6 (proto=FRAGMENT) / end > > eth / ipv6 / ipv6_ext (next_hdr=FRAGMENT) / end > > > > Any sensible solution for not-fragments with any other > > extension headers? > > > The one propose in this mail 😊 > > > INVERT exists, but hardly useful, since it simply says > > that patches which do not match pattern without INVERT > > matches the pattern with INVERT and > > invert / eth / ipv6 (proto=FRAGMENT) / end > > will match ARP, IPv4, IPv6 with an extension header before > > fragment header and so on. > > > I agree with you, INVERT in this doesn’t help. > We were considering adding some kind of not mask / item per item. > some think around this line: > user request ipv6 unfragmented udp packets. The flow would look something > like this: > Eth / ipv6 / Not (Ipv6.proto = frag_proto) / udp > But it makes the rules much harder to use, and I don't think that there > is any HW that support not, and adding such feature to all items is overkill. > > > > Bit string suggested above will allow to match: > > - UDP over IPv6 with any extension headers: > > eth / ipv6 (ext_hdrs mask empty) / udp / end > > - UDP over IPv6 without any extension headers: > > eth / ipv6 (ext_hdrs mask full, spec empty) / udp / end > > - UDP over IPv6 without fragment header: > > eth / ipv6 (ext.spec & ~FRAGMENT, ext.mask | FRAGMENT) / udp / end > > - UDP over IPv6 with fragment header > > eth / ipv6 (ext.spec | FRAGMENT, ext.mask | FRAGMENT) / udp / end > > > > where FRAGMENT is 1 << IPPROTO_FRAGMENT. > > > Please see my response regarding this above. > > > Above I intentionally keep 'proto' unspecified in ipv6 > > since otherwise it would specify the next header after IPv6 > > header. > > > > Extension headers mask should be empty by default. This is a deliberate design choice/issue with rte_flow: an empty pattern matches everything; adding items only narrows the selection. As Andrew said there is currently no way to provide a specific item to reject, it can only be done globally on a pattern through INVERT that no PMD implements so far. So we have two requirements here: the ability to specifically match IPv6 fragment headers and the ability to reject them. To match IPv6 fragment headers, we need a dedicated pattern item. The generic RTE_FLOW_ITEM_TYPE_IPV6_EXT is useless for that on its own, it must be completed with RTE_FLOW_ITEM_TYPE_IPV6_EXT_FRAG and associated object to match individual fields if needed (like all the others protocols/headers). Then to reject a pattern item... My preference goes to a new "NOT" meta item affecting the meaning of the item coming immediately after in the pattern list. That would be ultra generic, wouldn't break any ABI/API and like INVERT, wouldn't even require a new object associated with it. To match UDPv6 traffic when there is no fragment header, one could then do something like: eth / ipv6 / not / ipv6_ext_frag / udp PMD support would be trivial to implement (I'm sure!) We may later implement other kinds of "operator" items as Andrew suggested, for bit-wise stuff and so on. Let's keep adding features on a needed basis though. -- Adrien Mazarguil 6WIND