Re: [dpdk-dev] [PATCH v5 3/6] net/axgbe: use common rte bitoperation APIs instead

[Gavin Hu (Arm Technology China)]

> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Tuesday, December 3, 2019 12:53 AM
> To: Morten Brørup <m...@smartsharesystems.com>
> Cc: tho...@monjalon.net; Gavin Hu (Arm Technology China)
> <gavin...@arm.com>; Joyce Kong (Arm Technology China)
> <joyce.k...@arm.com>; dev@dpdk.org; jer...@marvell.com; Bruce
> Richardson <bruce.richard...@intel.com>; nd <n...@arm.com>;
> david.march...@redhat.com; Honnappa Nagarahalli
> <honnappa.nagaraha...@arm.com>; ravi1.ku...@amd.com;
> rm...@marvell.com; shsha...@marvell.com; xuanziya...@huawei.com;
> cloud.wangxiao...@huawei.com; zhouguoy...@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v5 3/6] net/axgbe: use common rte
> bitoperation APIs instead
> 
> On Mon, 2 Dec 2019 10:24:32 +0100
> Morten Brørup <m...@smartsharesystems.com> wrote:
> 
> > Thomas,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas
> Monjalon
> > > Sent: Monday, December 2, 2019 10:12 AM
> > >
> > > 02/12/2019 07:09, Gavin Hu (Arm Technology China):
> > > > Hi Bruce, Thomas,
> > > >
> > > > This series of patches was reported a compilation issue[1] on 32bit
> > > Ubuntu.
> > > > On mainstream 64-bit OS,  "unsigned long" is 64-bit in size and we
> > > uses the 64-bit variant of APIs. But the 32-bit OS expect 32-bit
> > > 'unsigned long' arguments.
> > > > This is where the error happens.
> > >
> > > Please could you be more specific? What is the exact error?
> >
> > The PMD has a private structure with an unsigned long field.
> >
> > The patch for the PMD uses the 64 bit operations on this field. The patch
> fails to compile for a 32 bit target, because the struct field is only 32 bit 
> there.
> >
> > >
> > > > My question is how 32-bit OSes shall we support, put another way, can
> > > we ignore this compilation issue?
> > > > If we still need to care, how about making 'obsolete' of 'unsigned
> > > long' and use 'uint32' instead to be multi-OS friendly?
> > >
> > > Which unsigned long?
> > > If it is in the (not merged) bit API, it can still be changed no?
> > >
> >
> > The patch for the PMD can be changed to use the 64 or 32 bit operations
> depending on whether it is being compiled for a 64 or 32 bit target.
> >
> > However, the question seems to be if we want to either 1) do something like
> that, or 2) drop support for 32 bit targets, or 3) make these target dependent
> fields obsolete (i.e. ban the use of unsigned long) and require explicit 
> sizes, e.g.
> uint32_t.
> 
> The bitop library should not assume sizeof(unsigned long) == 32 bit.
As discussed, both 32-bit and 64-bit OSes should be supported, and their sizes 
of "unsigned long" are not fixed.
Taking all these into considerations, we will use "unsigned int" or uint32_t 
instead of "unsigned long" in the PMDs to be compatible across 32- or 64-bit 
OSes.
/Gavin