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 0709C43BC0;
	Sat, 24 Feb 2024 12:13:48 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 83F21402C8;
	Sat, 24 Feb 2024 12:13:47 +0100 (CET)
Received: from fhigh8-smtp.messagingengine.com
 (fhigh8-smtp.messagingengine.com [103.168.172.159])
 by mails.dpdk.org (Postfix) with ESMTP id 76759402A8
 for <dev@dpdk.org>; Sat, 24 Feb 2024 12:13:46 +0100 (CET)
Received: from compute2.internal (compute2.nyi.internal [10.202.2.46])
 by mailfhigh.nyi.internal (Postfix) with ESMTP id ED8BF114009B;
 Sat, 24 Feb 2024 06:13:45 -0500 (EST)
Received: from mailfrontend1 ([10.202.2.162])
 by compute2.internal (MEProxy); Sat, 24 Feb 2024 06:13:45 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:cc:content-transfer-encoding:content-type:content-type:date
 :date:from:from:in-reply-to:in-reply-to:message-id:mime-version
 :references:reply-to:subject:subject:to:to; s=fm1; t=1708773225;
 x=1708859625; bh=BI3anVy39MfFiLOaXXhSOYpEB/o+b6oBzersXp1sWQk=; b=
 jclmY5vGgfgv6nZQOQcYCdgi+oQeFuMT4GZ7PzUyfs+CWNdou+7o/m4yeiYuUnnC
 YJFFMt+92f4i6x+5p5HvWcFezxMbsyctEIeTXsSNqJZLdXzJzehQOlA7r8Oh8d3M
 TRFMFnp4Xe1aXaEPcgeaQQRSC9ci4drB7eMfrTaD/1tSmTfPwXmBrckz6C0S5toB
 gcebyWz+91h1k3r1WCLrxhSkThPLCDeGxVf3HQE712mEVsOPUdETKz9XOk/uDqpJ
 pCS7pBFVq424164F6MHolzaPfnXtxmkEK2MvyrfUO1Psf0hrLgUpMgNEw74rgk/B
 sE/j7t3HHzMYSYpGU/rFgw==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:cc:content-transfer-encoding
 :content-type:content-type:date:date:feedback-id:feedback-id
 :from:from:in-reply-to:in-reply-to:message-id:mime-version
 :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy
 :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1708773225; x=
 1708859625; bh=BI3anVy39MfFiLOaXXhSOYpEB/o+b6oBzersXp1sWQk=; b=H
 nlf/pgY+i0cbcQfC6meYG5j142WMAWlLzj+CF8Y2Ipi4VNdKExqClt5BYxtOo8od
 o9ImHcdBdrG1zjv+zmEILTAw7L4GUZr1HklomVwGtoMQvN9pdeacmpwy/pTxnPD+
 ZnyIjuXdYFDO3jbgQMGl5Ce83DNHnz8Zk+RsfDV3gCqboMcAfKTHc0nZgQZvSAHN
 IzbeHcWEr5Q/TMSaWhPZ81w86dOe44TQ6NONvtksxzjMJ6iAo8Ckjs4PJPlkt6z8
 pUEMh1spEddGhdKF5pZWIGRYos2Y9LyFpMDW73So2AKtHBrhs9FnGkkz/UItchnv
 GfAHDsgaw2dtzXLWnPw+g==
X-ME-Sender: <xms:aM_ZZY465EbTDJPRDINH8g8f7eg6LRgheLpI-wg63-nGBpaBK4iyuQ>
 <xme:aM_ZZZ6epqL2oJz0eGSt5NIzIi7YD6LGU_1sSRhjoZmEEmk3sozsKrmcKfMdp46aG
 2ZqDOahPGXFFOE3iQ>
X-ME-Received: <xmr:aM_ZZXfVeygrlJFYj5zsPr1SEZGSnSZpNTfixmhkDZ_48_LTpoqFuOOcXXkUpCaLjuB2md5tKY7qPBYxVsp9>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrfeekgddvhecutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhhomhgr
 shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg
 ftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhteduudev
 tdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh
 homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:ac_ZZdI5Gr_wSscZ__E-xpSy7D2LUVL6z-U4e23m5p8xDknsa9FHVA>
 <xmx:ac_ZZcIO-ZNJMqR1fnlgRj-boK6peP6smhn0qQ_2A1QW6a5bowfnQQ>
 <xmx:ac_ZZewwDBxWzh0100J-xCkt0adXUiEvzP8gi4chuAkOGjurIDTRMw>
 <xmx:ac_ZZTgW27g4ck_ma_aURcWo6uNhSVAXif020NGRV8K2TtDoANmOtg>
Feedback-ID: i47234305:Fastmail
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat,
 24 Feb 2024 06:13:39 -0500 (EST)
From: Thomas Monjalon <thomas@monjalon.net>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>,
 Morten =?ISO-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>
Cc: dev@dpdk.org, Ajit Khaparde <ajit.khaparde@broadcom.com>,
 Andrew Boyer <andrew.boyer@amd.com>,
 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
 Bruce Richardson <bruce.richardson@intel.com>,
 Chenbo Xia <chenbox@nvidia.com>, Chengwen Feng <fengchengwen@huawei.com>,
 Dariusz Sosnowski <dsosnowski@nvidia.com>,
 David Christensen <drc@linux.vnet.ibm.com>,
 Hyong Youb Kim <hyonkim@cisco.com>, Jerin Jacob <jerinj@marvell.com>,
 Jie Hai <haijie1@huawei.com>, Jingjing Wu <jingjing.wu@intel.com>,
 John Daley <johndale@cisco.com>, Kevin Laatz <kevin.laatz@intel.com>,
 Kiran Kumar K <kirankumark@marvell.com>,
 Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
 Maciej Czekaj <mczekaj@marvell.com>, Matan Azrad <matan@nvidia.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>,
 Nithin Dabilpuram <ndabilpuram@marvell.com>, Ori Kam <orika@nvidia.com>,
 Ruifeng Wang <ruifeng.wang@arm.com>, Satha Rao <skoteshwar@marvell.com>,
 Somnath Kotur <somnath.kotur@broadcom.com>,
 Suanming Mou <suanmingm@nvidia.com>, Sunil Kumar Kori <skori@marvell.com>,
 Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
 Yisen Zhuang <yisen.zhuang@huawei.com>,
 Yuying Zhang <Yuying.Zhang@intel.com>
Subject: Re: [PATCH v5 00/22] stop using RTE_MARKER extensions
Date: Sat, 24 Feb 2024 12:13:36 +0100
Message-ID: <3245779.AJdgDx1Vlc@thomas>
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F262@smartserver.smartshare.dk>
References: <1706657173-26166-1-git-send-email-roretzla@linux.microsoft.com>
 <1708762927-14126-1-git-send-email-roretzla@linux.microsoft.com>
 <98CBD80474FA8B44BF855DF32C47DC35E9F262@smartserver.smartshare.dk>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
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

24/02/2024 11:42, Morten Br=F8rup:
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > RTE_MARKER typedefs are a GCC extension unsupported by MSVC. This series
> > hides the markers when building with MSVC and updates libraries and
> > drivers to access compatibly typed pointers to the rte_mbuf struct
> > offsets.
> >=20
> > This series, does the following.
> >=20
> > Introduces a new macro __rte_marker(type, name) which is used to
> > conditionally expand RTE_MARKER fields empty when building with GCC.
>=20
> Important typo: GCC -> MSVC.
>=20
> Correct me if I'm wrong: When building with MSVC, the RTE_MARKER fields d=
on't exist at all.

Yes it should trigger a compiler error when using an old marker with MSVC.
So no need of this wrapper __rte_marker().

> > Updates existing inline functions accessing cacheline{0,1} markers in
> > the rte_mbuf struct to stop using the markers and instead uses the mbuf
> > fields directly.
> >=20
> > Introduces 2 new inline functions to allow drivers to access rearm_data
> > and rx_descriptor_fields1 descriptors without using the RTE_MARKER
> > fields.
> >=20
> > Updates all drivers to use the new inline rte_mbuf struct accessors for
> > rearm_data and rx_descriptor_fields1.
>=20
> Accessor functions like these are BS for structures that are part of the =
public API.

What is BS?

> Nobody will use the accessor functions! When developing an application (o=
r lib or driver), the developer will look at the structure and access the r=
elevant fields directly; why would the developer start looking elsewhere fo=
r accessor functions?

Yes that's why we need to reference the accessors inside the mbuf structure.
Also we should check new code (with checkpatch) for not using markers anymo=
re.

> Another alternative would be to remove the rearm_data and rx_descriptor_f=
ields1 fields from the structure, and in the drivers address the first fiel=
d of the group, and preferably add some static_asserts to check the sequenc=
e of the fields they cover. I don't like this alternative, but I'm putting =
it out there for discussion/inspiration.
> I prefer the union+struct approach to visibly group the fields together.

Unions make the mbuf struct more complicate just for compatibility.

> Overall, I dislike approach taken in this version of the patch series.
> On the surface, it has minimal changes to the mbuf structure.
> But underneath, some of the fields (the markers) may or may not exist, de=
pending on compiler, and this fact is not obvious when looking at the struc=
ture. I think this will degrade future MSVC compatibility for both applicat=
ions, libraries and PMDs.

With appropriate checks, we won't use markers anymore.

> As Thomas stressed, we should take special care of the mbuf structure!
> It has to be modified for MSVC compatibility, so we have to find the best=
 compromise.
> Personally, I prefer the previous approach over this version.
>=20
> Maybe we need to compromise on API compatibility to make this a beautiful=
 modification.
>=20
> @Thomas, looking at the mbuf and eal patches, what do you think about thi=
s version of the series?

I prefer this series with following changes:
=2D no __rte_marker wrapper
=2D make sure cache line padding is effective without markers
=2D no direct access of fields for cache line prefetch
=2D comments in mbuf
=2D checkpatch