DPDK patches and discussions
 help / color / Atom feed
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: Tal Shnaiderman <talshn@mellanox.com>
Cc: Ranjit Menon <ranjit.menon@intel.com>,
	Fady Bader <fady@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	Dmitry Malloy <dmitrym@microsoft.com>,
	Narcisa Ana Maria Vasile <Narcisa.Vasile@microsoft.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Olivier Matz <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
Date: Tue, 30 Jun 2020 02:56:20 +0300
Message-ID: <20200630025620.241e6d54@sovereign> (raw)
In-Reply-To: <AM0PR05MB531357486EF9599503580488A96E0@AM0PR05MB5313.eurprd05.prod.outlook.com>

On Mon, 29 Jun 2020 08:12:51 +0000, Tal Shnaiderman wrote:
> > From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Subject: Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows
> > 
> > On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:  
> > > On 6/28/2020 7:20 AM, Fady Bader wrote:  
> > > > Hi Dmitry,
> > > > I'm trying to run test-pmd on Windows and I ran into this error with  
> > cmdline.  
> > > >
> > > > The error log message is :
> > > > In file included from ../app/test-pmd/cmdline_flow.c:23:
> > > > ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64'  
> > redeclared as different kind of symbol  
> > > >    INT64
> > > >
> > > > In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/winnt.h:150,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/minwindef.h:163,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/windef.h:8,  
> > > >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/windows.h:69,  
> > > >                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
> > > >                   from ..\lib/librte_eal/windows/include/pthread.h:20,
> > > >                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
> > > >                   from ..\lib/librte_eal/include/rte_errno.h:18,
> > > >                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
> > > >                   from ../app/test-pmd/cmdline_flow.c:18:
> > > > C:/mingw-w64/x86_64/mingw64/x86_64-w64-  
> > mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was
> > here  
> > > >     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
> > > >
> > > > The same error is for the other types defined in cmdline_numtype.
> > > >
> > > > This problem with windows.h is popping in many places and some of
> > > > them are cmdline and test-pmd and librte_net.
> > > > We should find a way to exclude windows.h from the unneeded places,
> > > > is there any suggestions on how it can be done ?  
> > >
> > > We ran into this same issue when working with the code that is on the
> > > draft repo.
> > >
> > > The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types
> > > in Windows headers for integer types. We found that it is easier to
> > > change the enum in cmdline_parse_num.h than try to play with the
> > > include order of headers. AFAIK, the enums were only used to determine
> > > the type in a series of switch() statements in librte_cmdline, so we
> > > simply renamed the enums. Not sure, if that will be acceptable here.  
> > 
> > +1 for renaming enum values. It's not a problem of librte_cmdline itself
> > +but a
> > problem of its consumption on Windows, however renaming enum values
> > doesn't break ABI and winn make librte_cmdline API "namespaced".
> > 
> > I don't see a clean way not to expose windows.h, because pthread.h
> > depends on it, and if we hide implementation, librte_eal would have to
> > export pthread symbols on Windows, which is a hack (or is it?).  
> 
> test_pmd redefine BOOLEAN and PATTERN in the index enum, I'm not sure how many more conflicts we will face because of this huge include.
>
> Also, DPDK applications will inherit it unknowingly, not sure if this is common for windows libraries.

I never hit these particular conflicts, but you're right that there will be
more, e.g. I remember particularly nasty clashes in failsafe PMD, unrelated
to cmdline token names.


We could take the same approach as with networking headers: copy required
declarations instead of including them from SDK. Here's a list of what
pthread.h uses:

CloseHandle
CreateThread
DeleteSynchronizationBarrier
EnterSynchronizationBarrier
GetThreadAffinityMask
InitializeSynchronizationBarrier
OpenThread
SetPriorityClass
SetThreadAffinityMask
SetThreadPriority
TerminateThread

Windows has strict compatibility policy, so prototypes are unlikely to ever
change. None of the used functions takes string parameters, thus not affected
by A/W macros. Looks a bit messy, but it's limited in scope at least.


An external pthread library would solve the problem, but as I've reported
earlier, I failed to find a good one: [1] and [3] are tied to MinGW, although
of high quality, [2] seems outdated.

[1]: Wnpthreads:
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads/
[2] pthreads-win32: https://sourceware.org/pthreads-win32/
[3] mcfgthread: https://github.com/lhmouse/mcfgthread

-- 
Dmitry Kozlyuk

  reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-20 21:05 [dpdk-dev] [PATCH 0/7] " Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-06-28 14:20   ` Fady Bader
2020-06-29  6:23     ` Ranjit Menon
2020-06-29  7:42       ` Dmitry Kozlyuk
2020-06-29  8:12         ` Tal Shnaiderman
2020-06-29 23:56           ` Dmitry Kozlyuk [this message]
2020-07-08  1:09             ` Dmitry Kozlyuk
2020-06-20 21:05 ` [dpdk-dev] [PATCH 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-07-17 22:16 ` [dpdk-dev] [PATCH 0/7] cmdline: support Windows Narcisa Ana Maria Vasile
2020-07-30 18:08 ` Kadam, Pallavi
2020-07-30 21:06 ` [dpdk-dev] [PATCH v2 " Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 1/7] cmdline: make implementation opaque Dmitry Kozlyuk
2020-08-05  9:31     ` Kinsella, Ray
2020-08-05 11:17       ` Dmitry Kozlyuk
2020-09-30  8:11         ` Kinsella, Ray
2020-09-30 15:26           ` Dmitry Kozlyuk
2020-09-17 13:34     ` Olivier Matz
2020-09-17 17:05       ` Stephen Hemminger
2020-09-18  8:33         ` Bruce Richardson
2020-09-18 12:13           ` Ferruh Yigit
2020-09-18 13:23         ` Kinsella, Ray
2020-09-17 23:13       ` Dmitry Kozlyuk
2020-09-18 13:31       ` Kinsella, Ray
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-07-30 21:06   ` [dpdk-dev] [PATCH v2 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-09-26  1:33     ` [dpdk-dev] [EXTERNAL] " Narcisa Ana Maria Vasile
2020-09-26  6:03       ` Khoa To
2020-09-28 21:50   ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 1/7] cmdline: make implementation logically opaque Dmitry Kozlyuk
2020-09-30  8:12       ` Kinsella, Ray
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 2/7] cmdline: add internal wrappers for terminal handling Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 3/7] cmdline: add internal wrappers for character input Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 4/7] cmdline: add internal wrapper for vdprintf Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 5/7] eal/windows: improve compatibility networking headers Dmitry Kozlyuk
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 6/7] cmdline: support Windows Dmitry Kozlyuk
2020-10-14 22:31       ` Thomas Monjalon
2020-09-28 21:50     ` [dpdk-dev] [PATCH v3 7/7] examples/cmdline: build on Windows Dmitry Kozlyuk
2020-10-14 22:33       ` Thomas Monjalon
2020-10-05 15:33     ` [dpdk-dev] [PATCH v3 0/7] cmdline: support Windows Olivier Matz
2020-10-14 22:41       ` Thomas Monjalon

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200630025620.241e6d54@sovereign \
    --to=dmitry.kozliuk@gmail.com \
    --cc=Narcisa.Vasile@microsoft.com \
    --cc=dev@dpdk.org \
    --cc=dmitrym@microsoft.com \
    --cc=fady@mellanox.com \
    --cc=olivier.matz@6wind.com \
    --cc=ranjit.menon@intel.com \
    --cc=talshn@mellanox.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox