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 E3AB9A0542; Fri, 7 Feb 2020 15:30:02 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BDA691C000; Fri, 7 Feb 2020 15:30:02 +0100 (CET) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id 830491C117 for ; Fri, 7 Feb 2020 15:30:01 +0100 (CET) Received: by mail-wr1-f65.google.com with SMTP id w12so2963876wrt.2 for ; Fri, 07 Feb 2020 06:30:01 -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=t1UMtl6dPgmRKAmmsAD1n7f0Hdco0Ya4hs4ygcI8bK4=; b=J/TQplrX1X8s/7fEYD7l5VmUDJ/ez7xVeq6EWcDMBaLolM4yKetULmqxnGwQOQkEm4 ajlFW1/rEBMfERcVLxkHTkeR5v7EYMl3ucb9/gIWpDtoBFFxJKQZOjsWqMIzRnO2GcG+ ej5OgzIH72eWf+SfVVbtgT/5SY31vQAJ3J6EPjA7ot/3TJg3tSYlFcBfs0lHxcIC+scf reYvcnzvr+fj3QC0eX0OW8u5VB8+tQet7B0Ga+EdxF8u4/++axDLqYxAWqwXy3NK/xZs skmwDFn3BxVlSRo4+1eVqgIY9INrbihCrh/5UfFKvnN21Q51FwGLTxQp9ragl1xiXWN/ Z8tQ== 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=t1UMtl6dPgmRKAmmsAD1n7f0Hdco0Ya4hs4ygcI8bK4=; b=EFKTHBEIuDe9EWqZGbMFZGlkhZwzYbPstaqGKq2PqCNcYM2HvbfpGByIwlqYOEQz6P nQxsFnBYPoTrDn708hix4jAdw6g8+wJaCwPY4+BGPMPSMHWSp6W9IFDvhq2dfnmxT570 AaqKtKbGsvRPF9lYUh+V79ocFQ20nMsbgeoEW6Ey4meo8drD89bYOxRXl5oNgJLBeept ruPrUx2h3OOO0iJ7fHnPv/OjORy4ttPDGf4LyS5riLPPBLoU3XELo/aqK5ZI0Sv/h+Bv riilBSeIl7GtzdEAlUjYr3DlFp81efXrRtcTQIHHUtGKtZqQ1+RMYMq3/QufI3AGFzHN axrw== X-Gm-Message-State: APjAAAWS/KUg9Ll4Qw5kCXfp9CAPLlYgpiePtPAAMkz1iE0eBiYA/VlX WLr46wD5YWX8TVrbQQ0Rxacqpw== X-Google-Smtp-Source: APXvYqwo4LRtCdstZYbqqUOiJ85H6Wi98r5IVZApVR+iEPXet+gvLdPIzX8mC1ibvlZPcD3SHC6W8g== X-Received: by 2002:a5d:6151:: with SMTP id y17mr4959553wrt.110.1581085801129; Fri, 07 Feb 2020 06:30:01 -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 x17sm3482863wrt.74.2020.02.07.06.29.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Feb 2020 06:30:00 -0800 (PST) Date: Fri, 7 Feb 2020 15:29:59 +0100 From: Olivier Matz To: Somnath Kotur Cc: dev , Ferruh Yigit Message-ID: <20200207142959.GR22738@platinum> References: <20200106083423.26600-1-somnath.kotur@broadcom.com> <20200206172500.GQ22738@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH] mbuf: fix to update documentation of QinQ stripped bit interpretation 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 Somnath, On Fri, Feb 07, 2020 at 07:13:04PM +0530, Somnath Kotur wrote: > Olivier, > > On Thu, Feb 6, 2020 at 10:55 PM Olivier Matz wrote: > > > > Hi Somnath, > > > > Sorry for the delay, please find some comments below. > > > > I suggest the following title instead: > > > > mbuf: extend meaning of QinQ stripped bit > > > > On Mon, Jan 06, 2020 at 02:04:23PM +0530, 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 and saved in mbuf->vlan_tci_outer. > > > > To be sure we're on the same page: we are talking about case 7 of this > > link: http://inbox.dpdk.org/dev/20191227145041.GQ22738@platinum/ > > I'm not sure we are on the same page then, please see my response inline below > > > > So, even if the inner vlan_tci is not stripped from packet data, it has > > to be saved in m->vlan_tci, because it is implied by PKT_RX_VLAN. > > > > From the same link, the case where the driver only strips+saves the > > outer vlan without saving or stripping the inner one is case 3. > > > While this is how it works currently, I'm wondering how will the > application know if this was > a double VLAN pkt, correct? If the hardware supports it and configured for it, the flag PKT_RX_QINQ is set when there are 2 vlans. FYI, the m->packet_type field can also be useful. It describes what is really present in the packet data, as described in rte_mbuf_core.h: /* * The packet type, which is the combination of outer/inner L2, L3, L4 * and tunnel types. The packet_type is about data really present in the * mbuf. Example: if vlan stripping is enabled, a received vlan packet * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the * vlan is stripped from the data. */ > Also when i look at options 5 and 7 I don't really see the difference > in semantics between them ? > Both seem to store the outer-vlan and inner-vlan in m->vlan_tci_outer > and m->vlan_tci_inner respectively > > 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 The difference is about what was stripped or not from mbuf data. > I was hoping that with the new interpretation of PKT_RX_QINQ_STRIPPED, > it only meant that outer-vlan is removed from packet data > and m->vlan_tci_outer = outer_vlan, while PKT_RX_QINQ implies it is a > double-vlan pkt and PKT_RX_VLAN implies that pkt has VLAN > associated with it? Not m->vlan_tci = inner-vlan The meaning of each flag should be as simple as possible, I think we can summarize them like this: - PKT_RX_VLAN: the vlan is saved in vlan tci. - PKT_RX_VLAN_STRIPPED: the vlan hdr is removed from packet data. - PKT_RX_QINQ: the outer vlan is saved in vlan tci. - PKT_RX_QINQ_STRIPPED: the inner vlan is stripped from packet data. - When PKT_RX_QINQ is set, PKT_RX_VLAN* refer to the inner vlan of initial packet, else it refers to the first vlan of the packet. There is a link between vlan flag and vlan_tci field, and qinq flag and vlan_tci_outer field. I'm still not sure to understand what you expect. Can you give an example with flags (which are set), and the expected content of m->vlan_tci and m->vlan_tci_outer? By the way, the case 5/ is not very well described too, maybe we should add something about it. Thanks, Olivier > > Thanks > Som > > > > > 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 > > > --- > > > 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). > > > > their tci are -> its tci is > > > > > * 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. > > > > ok > > > > > + * 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 is correct, but I'd use a bullet list to add another sentence: > > > > * - If 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). > > * - If PKT_RX_QINQ_STRIPPED is set and PKT_RX_VLAN_STRIPPED is unset, only the > > * outer vlan is removed from packet data, but both 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. > > > > The same exact sentence is above, this one can be removed. > > > > > + * When PKT_RX_QINQ_STRIPPED and PKT_RX_VLAN_STRIPPED are set, > > > + * (PKT_RX_VLAN | PKT_RX_QINQ) must also be set. > > > > This can be removed too as it is redundant with above sentence: > > > > * When PKT_RX_QINQ_STRIPPED is set, the flags (PKT_RX_VLAN | PKT_RX_QINQ) > > * must also be set. > > > > > > Thanks, > > Olivier