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 CC1FB45D44;
	Tue, 19 Nov 2024 17:23:39 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id B8D1840B94;
	Tue, 19 Nov 2024 17:23:39 +0100 (CET)
Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182])
 by mails.dpdk.org (Postfix) with ESMTP id DB0324028A;
 Tue, 19 Nov 2024 17:23:38 +0100 (CET)
Received: by linux.microsoft.com (Postfix, from userid 1213)
 id 3403C206BCEE; Tue, 19 Nov 2024 08:23:38 -0800 (PST)
DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3403C206BCEE
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com;
 s=default; t=1732033418;
 bh=sIiDYwh1QywaJm1s2cFlmSXIJF5GiVvXOGabRWH4GGk=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
 b=MqVaRYhBRwJhjEtGMdxj439tYBM0yww+cXx83KNcr/S9ISVv6eUXS+OGwXDNQjq6v
 CmSPA9Xz8nj6VenpiHjgoUP8s477Rkt/u6+WkexMC3/CjHZOKW3i6kym7eslMl5Whr
 9neGGIDasRrve+msrwgn58tIHgppMiDeOirqKbbY=
Date: Tue, 19 Nov 2024 08:23:38 -0800
From: Andre Muezerie <andremue@linux.microsoft.com>
To: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Cc: Morten =?iso-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>,
 "roretzla@linux.microsoft.com" <roretzla@linux.microsoft.com>,
 "techboard@dpdk.org" <techboard@dpdk.org>,
 "Yuying.Zhang@intel.com" <Yuying.Zhang@intel.com>,
 "aman.deep.singh@intel.com" <aman.deep.singh@intel.com>,
 "anatoly.burakov@intel.com" <anatoly.burakov@intel.com>,
 "bruce.richardson@intel.com" <bruce.richardson@intel.com>,
 "byron.marohn@intel.com" <byron.marohn@intel.com>,
 "conor.walsh@intel.com" <conor.walsh@intel.com>,
 "cristian.dumitrescu@intel.com" <cristian.dumitrescu@intel.com>,
 "david.hunt@intel.com" <david.hunt@intel.com>,
 "dev@dpdk.org" <dev@dpdk.org>,
 "dsosnowski@nvidia.com" <dsosnowski@nvidia.com>,
 "gakhil@marvell.com" <gakhil@marvell.com>,
 "jerinj@marvell.com" <jerinj@marvell.com>,
 "jingjing.wu@intel.com" <jingjing.wu@intel.com>,
 "kirill.rybalchenko@intel.com" <kirill.rybalchenko@intel.com>,
 "konstantin.v.ananyev@yandex.ru" <konstantin.v.ananyev@yandex.ru>,
 "matan@nvidia.com" <matan@nvidia.com>,
 "orika@nvidia.com" <orika@nvidia.com>,
 "radu.nicolau@intel.com" <radu.nicolau@intel.com>,
 "ruifeng.wang@arm.com" <ruifeng.wang@arm.com>,
 "sameh.gobriel@intel.com" <sameh.gobriel@intel.com>,
 "sivaprasad.tummala@amd.com" <sivaprasad.tummala@amd.com>,
 "skori@marvell.com" <skori@marvell.com>,
 "stephen@networkplumber.org" <stephen@networkplumber.org>,
 "suanmingm@nvidia.com" <suanmingm@nvidia.com>,
 "vattunuru@marvell.com" <vattunuru@marvell.com>,
 "viacheslavo@nvidia.com" <viacheslavo@nvidia.com>,
 "vladimir.medvedkin@intel.com" <vladimir.medvedkin@intel.com>,
 "yipeng1.wang@intel.com" <yipeng1.wang@intel.com>
Subject: Re: [PATCH v5 01/16] eal: provide pack start macro for MSVC
Message-ID: <20241119162338.GA1105@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1710968771-16435-1-git-send-email-roretzla@linux.microsoft.com>
 <1731990941-10001-1-git-send-email-andremue@linux.microsoft.com>
 <1731990941-10001-2-git-send-email-andremue@linux.microsoft.com>
 <98CBD80474FA8B44BF855DF32C47DC35E9F8D2@smartserver.smartshare.dk>
 <b7ffc64af92c440fbc01aa4ba0e8d394@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b7ffc64af92c440fbc01aa4ba0e8d394@huawei.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
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

On Tue, Nov 19, 2024 at 11:00:56AM +0000, Konstantin Ananyev wrote:
> 
> 
> > -----Original Message-----
> > From: Morten Brørup <mb@smartsharesystems.com>
> > Sent: Tuesday, November 19, 2024 8:32 AM
> > To: Andre Muezerie <andremue@linux.microsoft.com>; roretzla@linux.microsoft.com; techboard@dpdk.org
> > Cc: Yuying.Zhang@intel.com; aman.deep.singh@intel.com; anatoly.burakov@intel.com; bruce.richardson@intel.com;
> > byron.marohn@intel.com; conor.walsh@intel.com; cristian.dumitrescu@intel.com; david.hunt@intel.com; dev@dpdk.org;
> > dsosnowski@nvidia.com; gakhil@marvell.com; jerinj@marvell.com; jingjing.wu@intel.com; kirill.rybalchenko@intel.com;
> > konstantin.v.ananyev@yandex.ru; matan@nvidia.com; orika@nvidia.com; radu.nicolau@intel.com; ruifeng.wang@arm.com;
> > sameh.gobriel@intel.com; sivaprasad.tummala@amd.com; skori@marvell.com; stephen@networkplumber.org;
> > suanmingm@nvidia.com; vattunuru@marvell.com; viacheslavo@nvidia.com; vladimir.medvedkin@intel.com;
> > yipeng1.wang@intel.com
> > Subject: RE: [PATCH v5 01/16] eal: provide pack start macro for MSVC
> > 
> > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > > Sent: Tuesday, 19 November 2024 05.35
> > >
> > > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > >
> > > MSVC struct packing is not compatible with GCC. Provide a macro that
> > > can be used to push existing pack value and sets packing to 1-byte.
> > > The existing __rte_packed macro is then used to restore the pack value
> > > prior to the push.
> > >
> > > Instead of providing macros exclusively for MSVC and for GCC the
> > > existing macro is deliberately utilized to trigger a warning if no
> > > existing packing has been pushed allowing easy identification of
> > > locations where the __rte_msvc_pack is missing.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > ---
> > >  lib/eal/include/rte_common.h | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/eal/include/rte_common.h
> > > b/lib/eal/include/rte_common.h
> > > index 4d299f2b36..409890863e 100644
> > > --- a/lib/eal/include/rte_common.h
> > > +++ b/lib/eal/include/rte_common.h
> > > @@ -103,8 +103,10 @@ typedef uint16_t unaligned_uint16_t;
> > >   * Force a structure to be packed
> > >   */
> > >  #ifdef RTE_TOOLCHAIN_MSVC
> > > -#define __rte_packed
> > > +#define __rte_msvc_pack __pragma(pack(push, 1))
> > > +#define __rte_packed __pragma(pack(pop))
> > >  #else
> > > +#define __rte_msvc_pack
> > >  #define __rte_packed __attribute__((__packed__))
> > >  #endif
> > >
> > > --
> > > 2.47.0.vfs.0.3
> > 
> > Before proceeding with this, can we please discuss the alternative, proposed here:
> > https://inbox.dpdk.org/dev/CAJFAV8yStgiBbe+Nkt9mC30r0+ZP64_kGuRHOzqd90RD2HXZyw@mail.gmail.com/
> > 
> > The definition of the packing macro in OVS, for reference:
> > https://github.com/openvswitch/ovs/blob/main/include/openvswitch/compiler.h#L209
> 
> Yes, that one looks much nicer to me too.
> Could I also ask to avoid creeping _msvc_ into DPDK namespace.
> 

These are all good comments. I'll look more into it.

> > 
> > The current solution requires __rte_packed to be placed at the end of a structure, although __attribute__((packed)) is normally
> > allowed at the beginning (between the "struct" tag and the name of the structure), which introduces a high risk of contributors placing
> > it "incorrectly", thus causing errors.
> > 
> > I have a strong preference for an __RTE_PACKED(decl) variant.
> > 
> > Here's a third alternative:
> > #ifdef RTE_TOOLCHAIN_MSVC
> > #define __rte_msvc_pack_begin __pragma(pack(push, 1))
> > #define __rte_msvc_pack_end   __pragma(pack(pop))
> > #else
> > #define __rte_msvc_pack_begin
> > #define __rte_msvc_pack_end
> > #endif
> > 
> > The third alternative is also problematic, e.g. if a contributor forgets the _end after the structure declaration, or adds another
> > structure declaration before the _end.
> > 
> > -Morten