From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id D44C2A0C41;
	Wed, 17 Nov 2021 11:03:02 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id BDDBE41147;
	Wed, 17 Nov 2021 11:03:02 +0100 (CET)
Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com
 [66.111.4.224]) by mails.dpdk.org (Postfix) with ESMTP id 35C444068C
 for <dev@dpdk.org>; Wed, 17 Nov 2021 11:03:01 +0100 (CET)
Received: from compute3.internal (compute3.nyi.internal [10.202.2.43])
 by mailnew.nyi.internal (Postfix) with ESMTP id B4A5A5806F6;
 Wed, 17 Nov 2021 05:02:59 -0500 (EST)
Received: from mailfrontend1 ([10.202.2.162])
 by compute3.internal (MEProxy); Wed, 17 Nov 2021 05:02:59 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding:content-type; s=fm2; bh=
 zD4ofM/bUffgYxL9SiZT1rZg2vbYnKCW/Oq9h3wVHho=; b=cJ+12LqRrV28eufy
 e48eCd/hD4u+PH6V8ASRnNOpBOfLsHQvKiFM79E/I53bxbD9aTk9H99G1N1N2+Ck
 qafm8zFB3o0XdH7wfpoQNIrvSfTn3tcYctmxmNNGm1OBOIiIdAuB6yaJOWy0pSuE
 WRku4lsmbZ5UGap3SPeDkhJv2Qlh+SXxRHF0iRoWjCIDGJ3b7l69pD9jgDsxjiM+
 2HOZ32hOjt7coUihc+Ybtg/viALAHuXaSH1SkO5e7olC57D/XupvVOj0AFwQJPOv
 vZtcHKA45YDK/t2e5WHrBGS7sHseYKNhK+iMy2x/9wkMM/m2TygQyIUhH6IsE/hu
 1RqQrw==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
 :x-sasl-enc; s=fm1; bh=zD4ofM/bUffgYxL9SiZT1rZg2vbYnKCW/Oq9h3wVH
 ho=; b=N4+5BBQvcmfriAOkP35LVQJ64bfBbFQvE30HdrRYml072KyRlG7SwdjLP
 eR/mX/6myeMle2UNqAp8crjQNkDnJVrNZtvHtbnz6yC3RxNbgdgrHhIgkcfGSiAZ
 5MZyX6/8pQeSjId78oFlI41HrntFIw0MfqHVkI6XV3Ir/oFXIHLr2+P3oDhwBf8d
 grlkKTNNl72XUmU/Qp41NNNziPdz4Ks30BwB3zEbpy6gV6UEMhP9lbUeXPYIBTjH
 NY01YL9RZr2ooBrGAbeO008MgfVHPOa6DViQjPMdDNh9da2PZ8YJx/SbOX4A1iCo
 dnRvxNvUeo+1y5uK389N8ZxvC2gOQ==
X-ME-Sender: <xms:UtOUYT_yrY5Gj-WotzDsw5pFi2vUoGPavIQgV7w-dE6Numqra_ckew>
 <xme:UtOUYfvavkN1Epd7umOSKHauKFxwO6cbPzpLulXh6agN9WFEKppcRbH_JXHw6ygDs
 livVGHrgHCzw8rHCg>
X-ME-Received: <xmr:UtOUYRAXxdH_6mEAgJDlG_7FD7rHuVGseZ1WUE75dd1fQJ0wIUtY_ofrl_IWXl3AC0lYXpV6V7-JQ456knye2Uj18g>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrfeeggddthecutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
 ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf
 frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei
 iedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh
 hmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght
X-ME-Proxy: <xmx:UtOUYfedRstzduLasULd4zm0GXRT6fVesAV_0AtZQ7VnKFxxLYfTVg>
 <xmx:UtOUYYPw9Aw3qw4onXWEBijT8VhFiO27GNCXbKou-5UefkNcMd02eQ>
 <xmx:UtOUYRm2CmguHrPI5FGw4WByUbJa2AGhWneEvi4U9xdW_x_k9laSWw>
 <xmx:U9OUYTHcdOGn4pY4AOkBSNW6LNEFuwt8NYOJi4c30liy8DjevwjgjQ>
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed,
 17 Nov 2021 05:02:56 -0500 (EST)
From: Thomas Monjalon <thomas@monjalon.net>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Rasesh Mody <rmody@marvell.com>, Shahed Shaikh <shshaikh@marvell.com>,
 Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
 Qi Zhang <qi.z.zhang@intel.com>, Xiao Wang <xiao.w.wang@intel.com>,
 Ziyang Xuan <xuanziyang2@huawei.com>,
 Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>,
 Guoyang Zhou <zhouguoyang@huawei.com>, Beilei Xing <beilei.xing@intel.com>,
 Jingjing Wu <jingjing.wu@intel.com>, Qiming Yang <qiming.yang@intel.com>,
 Rosen Xu <rosen.xu@intel.com>, Haiyue Wang <haiyue.wang@intel.com>,
 Jiawen Wu <jiawenwu@trustnetic.com>, Jian Wang <jianwang@trustnetic.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>,
 Chenbo Xia <chenbo.xia@intel.com>, Olivier Matz <olivier.matz@6wind.com>,
 dev@dpdk.org
Subject: Re: [PATCH] net: add macro for VLAN header length
Date: Wed, 17 Nov 2021 11:02:55 +0100
Message-ID: <5199283.magWtXZHGd@thomas>
In-Reply-To: <bd227d1e-17de-4655-d143-59b36fbdf666@intel.com>
References: <20211110174029.614449-1-ferruh.yigit@intel.com>
 <7652023.1kbqhHN0E7@thomas> <bd227d1e-17de-4655-d143-59b36fbdf666@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

17/11/2021 11:00, Ferruh Yigit:
> On 11/16/2021 11:14 PM, Thomas Monjalon wrote:
> > 10/11/2021 18:40, Ferruh Yigit:
> >> Multiple drivers are defining macros for VLAN header length, to remove
> >> the redundancy defining macro in the ether header.
> >> And updated drivers to use the new macro.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > [...]
> >> --- a/lib/net/rte_ether.h
> >> +++ b/lib/net/rte_ether.h
> >> +#define RTE_VLAN_HLEN       4
> > 
> > Please could you add a doxygen comment for this constant?
> > 
> 
> ack.
> 
> >> +/** Maximum VLAN frame length, including CRC. */
> >>   #define RTE_ETHER_MAX_VLAN_FRAME_LEN \
> >> -	(RTE_ETHER_MAX_LEN + 4)
> >> -	/**< Maximum VLAN frame length, including CRC. */
> >> +	(RTE_ETHER_MAX_LEN + RTE_VLAN_HLEN)
> > 
> > What about QinQ?
> > 
> 
> I am just replacing hardcoded value with macro in this patch.
> Changing 'RTE_ETHER_MAX_LEN' may have unexpected affect, and
> may not be good thing to the at this stage.

Sure
But can we take this opportunity to note that this macro
does not take QinQ into account?
Just a comment update?

Same for RTE_VLAN_HLEN, it is only one VLAN.