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 9446BA04FA; Fri, 27 Dec 2019 15:50:45 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9C1EA1C066; Fri, 27 Dec 2019 15:50:44 +0100 (CET) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 429191C045 for ; Fri, 27 Dec 2019 15:50:43 +0100 (CET) Received: by mail-wr1-f66.google.com with SMTP id y11so26258636wrt.6 for ; Fri, 27 Dec 2019 06:50:43 -0800 (PST) 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=UKvJvnwhvxtuYbkFHywzNsZ/hNGa6U2QEXbAe+eSDtE=; b=L6r4l5xLh/+BVRYL46HSw9h2e6mdxGDp8FYwMdDVdiiqpFyFKGgDlFEqZqnpqfVBDw dppB7nPr2NvqYHc2hsUpIuq4m3jC2iP15h/+GE24gR73oEHlSWIkqesBMb54NEtX5gwk UqawGxSet9sMPR4OaO/kiTmO8dbBlygJpPCatxIuOU46X2LifQtKqT07z7cw5KwIYN69 l7fBS7tEJ6/2VLKHrHpaVSNUerRhl1zQKuN3tRA4CtTjjLQqeyM1ZBZuNKSMvDOeaUzl IeBWBwBnU9+wwknhaVmwyGD10VYYuSgiGCrKAm4HOKrt+8UzvTq+1lGelTbgSo2huhWn WKnA== 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=UKvJvnwhvxtuYbkFHywzNsZ/hNGa6U2QEXbAe+eSDtE=; b=Mtgh7LG4TRUIgvIoxA6hGRsYVEex97M940VAzN8+gH2iiM2CviZ3pkm4tWQxovayFL 4omaIx47D4uslvu48N7K3SrvXrHGHwK0g0C7JJp6jIf7omR3NTPzRmW4taClWVICIqBR Y/ZJ33vjCfcOgESSkQrkwIvPkS2KSGpQw86yJawvfciG1LfqXix746Nxgkl1Rdztjt5F C3SWMYA1S3HWxwY0XPzY6AhaV9Tpxg9875Hb26y9ArjCexYLjovfDX/qVCUN8fv32rEx 64lsjDNeNoAR33IKlqGn3Ys4oATILh/e8sdLqHz+ftNDhyGO2CuzBIkLb/N5kXQszsOr MLvg== X-Gm-Message-State: APjAAAXCw8ltkfUzUohyMuCxGkQp+ByFF/EpbH1Nr1lagFR4710tuQFV uLnWT2y3nG7s5MdVop2Bw4+mjg== X-Google-Smtp-Source: APXvYqwYKrQ5eYtZdyASuZHy5OJBiOmcjtRRs32qb45D7a9lhCzIyrraYt0pG6A+4msaJrzC/WxORw== X-Received: by 2002:a5d:68cf:: with SMTP id p15mr51479885wrw.31.1577458242866; Fri, 27 Dec 2019 06:50:42 -0800 (PST) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id w22sm11186956wmk.34.2019.12.27.06.50.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Dec 2019 06:50:41 -0800 (PST) Date: Fri, 27 Dec 2019 15:50:41 +0100 From: Olivier Matz To: Andrew Rybchenko Cc: Somnath Kotur , dev , Ferruh Yigit , Hemant Agrawal , Sachin Saxena , Thomas Monjalon , David Marchand Message-ID: <20191227145041.GQ22738@platinum> References: <20191216031647.7750-1-somnath.kotur@broadcom.com> <68e1721c-4051-0451-185d-39344a026a38@solarflare.com> <53c0b3d2-5d61-6300-178e-b9500a93a7e3@solarflare.com> <4000b4f4-cdcd-78af-de60-2e8bde1364f7@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4000b4f4-cdcd-78af-de60-2e8bde1364f7@solarflare.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [RFC PATCH] mbuf: fix to update documentation of PKT_RX_QINQ_STRIPPED 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, On Tue, Dec 24, 2019 at 12:53:21PM +0300, Andrew Rybchenko wrote: > On 12/24/19 6:16 AM, Somnath Kotur wrote: > > Given that we haven't heard any objection from anyone in a while on > > this ...can we get this in please? > > I'm sorry, but have you seen below? > It means that PKT_RX_QINQ_STRIPPED, PKT_RX_QINQ, PKT_RX_VLAN > and PKT_RX_VLAN_STRIPPED must be clarified. > > It sounds like change of semantics in order to resolve the > problem, but anyway it is still a small change of semantics. Let's take this packet: packet = ether | outer-vlan | inner-vlan | ... The possible mbufs received from a PMD, depending on configuration, are: 1/ no flag (no offload) 2/ PKT_RX_VLAN packet data is unmodified m->vlan_tci=outer-vlan 3/ PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED outer-vlan is removed from packet data m->vlan_tci=outer-vlan 4/ PKT_RX_VLAN | PKT_RX_QINQ packet data is unmodified m->vlan_tci_outer=outer-vlan m->vlan_tci=inner-vlan 5/ PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ from PKT_RX_VLAN_STRIPPED: A vlan has been stripped by the hardware and its tci is saved in mbuf->vlan_tci. from PKT_RX_QINQ: The RX packet is a double VLAN, and the outer tci has been saved in in mbuf->vlan_tci_outer. To me, it means: inner-vlan is removed from packet data m->vlan_tci_outer=outer-vlan m->vlan_tci=inner-vlan 6/ PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ | PKT_RX_QINQ_STRIPPED both outer-vlan and inner-vlan removed from packet data m->vlan_tci_outer=outer-vlan m->vlan_tci=inner-vlan Other flag combinations are not supported. The proposed patch documents that this new flag combination is now allowed: 7/ PKT_RX_VLAN | PKT_RX_QINQ | PKT_RX_QINQ_STRIPPED outer-vlan is removed from packet data m->vlan_tci_outer=outer-vlan m->vlan_tci=inner-vlan Except if I missed something, I don't see any semantic change in the previously supported cases. I think we should by the way clarify what happens in 5/, probably by saying somewhere that as soon as PKT_RX_QINQ is set, PKT_RX_VLAN* refer to inner vlan. > BTW, it is better to make summary human readable and avoid > PKT_RX_QINQ_STRIPPED (I guess check-git-log.sh yells on it). > > Also RFC patch cannot be applied, non-RFC version is required. > > CC main tree maintainers. > > > Thanks > > > > On Mon, Dec 16, 2019 at 2:43 PM Andrew Rybchenko > > wrote: > >> > >> On 12/16/19 11:47 AM, Somnath Kotur wrote: > >>> On Mon, Dec 16, 2019 at 12:01 PM Andrew Rybchenko > >>> wrote: > >>>> > >>>> On 12/16/19 6:16 AM, Somnath Kotur wrote: > >>>>> Certain hardware may be able to strip and/or save only the outermost > >>>>> VLAN instead of both the VLANs in the mbuf in a QinQ scenario. > >>>>> To handle such cases, we could re-interpret setting of just PKT_RX_QINQ_STRIPPED > >>>>> to indicate that only the outermost VLAN has been stripped by the hardware and > >>>>> saved in mbuf->vlan_tci_outer. > >>>>> Only When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 vlans > >>>>> have been stripped by the hardware and their tci are saved in mbuf->vlan_tci (inner) > >>>>> and mbuf->vlan_tci_outer (outer). > >>>>> > >>>>> Signed-off-by: Somnath Kotur > >>>>> Signed-off-by: JP Lee > >>>>> --- > >>>>> lib/librte_mbuf/rte_mbuf_core.h | 15 +++++++++++---- > >>>>> 1 file changed, 11 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h > >>>>> index 9a8557d..db1070b 100644 > >>>>> --- a/lib/librte_mbuf/rte_mbuf_core.h > >>>>> +++ b/lib/librte_mbuf/rte_mbuf_core.h > >>>>> @@ -124,12 +124,19 @@ > >>>>> #define PKT_RX_FDIR_FLX (1ULL << 14) > >>>>> > >>>>> /** > >>>>> - * The 2 vlans have been stripped by the hardware and their tci are > >>>>> - * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). > >>>>> + * The outer vlan has been stripped by the hardware and their tci are > >>>>> + * saved in mbuf->vlan_tci_outer (outer). > >>>>> * This can only happen if vlan stripping is enabled in the RX > >>>>> * configuration of the PMD. > >>>>> - * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN | > >>>>> - * PKT_RX_VLAN_STRIPPED | PKT_RX_QINQ) must also be set. > >>>>> + * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN | PKT_RX_QINQ) > >>>>> + * must also be set. > >>>>> + * When both PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, the 2 vlans > >>>>> + * have been stripped by the hardware and their tci are saved in > >>>>> + * mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). > >>>>> + * This can only happen if vlan stripping is enabled in the RX configuration > >>>>> + * of the PMD. > >>>>> + * When PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, > >>>>> + * (PKT_RX_VLAN | PKT_RX_QINQ) must also be set. > >>>>> */ > >>>>> #define PKT_RX_QINQ_STRIPPED (1ULL << 15) > >>>>> > >>>> > >>>> I always thought that PKT_RX_VLAN_STRIPPED means *one* VLAN > >>>> stripped regardless if it is outer (if the packet is double > >>>> tagged) or inner (if only one VLAN tag was present). > >>>> > >>>> That's why PKT_RX_QINQ_STRIPPED description says that *two* > >>>> VLANs have been stripped. > >>>> > >>>> What is the problem with such approach? > >>> The problem is that RX_VLAN_STRIPPED implies that the stripped VLAN > >>> (outer or inner) is saved in mbuf->vlan_tci, correct? > >> > >> Yes. > >> > >>> There is no way to convey that it is in QinQ mode and yet only outer > >>> VLAN has been stripped and saved in mbuf->vlan_tci_outer ? > >> > >> Ah, it looks like I understand now that the problem is in > >> PKT_RX_QINQ description which claims that TCI is saved in > >> mbuf->vlan_tci_outer and PKT_RX_QINQ_STRIPPED means that > >> both VLAN tags are stripped regardless (PKT_RX_VLAN_STRIPPED). > >> Moreover PKT_RX_QINQ_STRIPPED requires PKT_RX_VLAN_STRIPPED. > >> > >> It means that PKT_RX_QINQ_STRIPPED, PKT_RX_QINQ, PKT_RX_VLAN > >> and PKT_RX_VLAN_STRIPPED must be clarified. > >> > >> I'm not sure, but it looks like it could affect net/dpaa2, > >> so I'm including driver maintainers in CC. >