From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 548B5A0562; Wed, 14 Apr 2021 19:16:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD581161BE6; Wed, 14 Apr 2021 19:16:43 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 45FE14013F for ; Wed, 14 Apr 2021 19:16:42 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1061) id 8FAE820B8001; Wed, 14 Apr 2021 10:16:41 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8FAE820B8001 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1618420601; bh=EWderpcAYuzAm3cx/Waea6OkQ5Lg3Ygx/6hJ6V2zheE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=A0Si1CrYsLt/ugucICiegbkrc8lbszJ5cGYPY77SqCTucNH9Ptl87YsRj03qxEhnc a+8jWZrfmkbdgpFDIhDm2/cGv/Ow/uQ7dmV7ZgJ6+cv6W7s0a+CtQAwupZLHae9LJZ Hwgrpf+k+EpOw2MffGeGyEU39Eo8bQApgyu+9oHQ= Date: Wed, 14 Apr 2021 10:16:41 -0700 From: Jie Zhou To: Dmitry Kozlyuk Cc: dev@dpdk.org, xiaoyun.li@intel.com, roretzla@microsoft.com, pallavi.kadam@intel.com, thomas@monjalon.net, bruce.richardson@intel.com, ferruh.yigit@intel.com Message-ID: <20210414171641.GB27344@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1616172695-28505-1-git-send-email-jizh@linux.microsoft.com> <1618334363-15147-1-git-send-email-jizh@linux.microsoft.com> <1618334363-15147-7-git-send-email-jizh@linux.microsoft.com> <20210413231000.2c7f706b@sovereign> <20210413222257.GA27344@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210413222257.GA27344@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [dpdk-dev] [PATCH v3 6/6] app/testpmd: enable testpmd on Windows X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Apr 13, 2021 at 03:22:57PM -0700, Jie Zhou wrote: > On Tue, Apr 13, 2021 at 11:10:00PM +0300, Dmitry Kozlyuk wrote: > > 2021-04-13 10:19 (UTC-0700), Jie Zhou: > > [...] > > > diff --git a/app/meson.build b/app/meson.build > > > index 50a53dbde..b40b04ca7 100644 > > > --- a/app/meson.build > > > +++ b/app/meson.build > > > @@ -1,10 +1,6 @@ > > > # SPDX-License-Identifier: BSD-3-Clause > > > # Copyright(c) 2017-2019 Intel Corporation > > > > > > -if is_windows > > > - subdir_done() > > > -endif > > > - > > > apps = [ > > > 'pdump', > > > 'proc-info', > > > @@ -19,7 +15,11 @@ apps = [ > > > 'test-pipeline', > > > 'test-pmd', > > > 'test-regex', > > > - 'test-sad'] > > > + 'test-sad' > > > +] > > > + > > > +# for BSD only > > > +lib_execinfo = cc.find_library('execinfo', required: false) > > > > Why is this needed? > > Seems the internal fork I worked on is out of sync with the upstream main. Did not add these but just being there. Will remove it. > > > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > > > index f44116b08..335ed534d 100644 > > > --- a/app/test-pmd/cmdline.c > > > +++ b/app/test-pmd/cmdline.c > > > @@ -8,12 +8,14 @@ > > > #include > > > #include > > > #include > > > + > > > +#ifndef RTE_EXEC_ENV_WINDOWS > > > #include > > > +#endif > > > > You seem to have missed my comment to v2, that #include line can > > just be removed, because provide everything necessary. > > Yeah, sorry, seems missed some of your comments. Thanks Dmitry. Will fix in V4. > > > > > [...] > > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > > > index fb7a3a8bd..9945adcf1 100644 > > > --- a/app/test-pmd/cmdline_flow.c > > > +++ b/app/test-pmd/cmdline_flow.c > > > @@ -31,6 +31,12 @@ > > > > > > #include "testpmd.h" > > > > > > +#ifdef RTE_EXEC_ENV_WINDOWS > > > +#ifndef IPDEFTTL > > > +#define IPDEFTTL 64 > > > +#endif > > > +#endif > > > + > > > /** Parser token indices. */ > > > enum index { > > > /* Special tokens. */ > > > > IPDEFTTL is used in some other apps and examples. > > If you follow Tal's advice and base your work on "Do not expose POSIX > > symbols" series, then please move this to . > > I applied "eal/windows:do not expose POSIX symbols" per your and Tal's advices, but hit error LNK2019: unresolved external symbol strdup reference in function log_save_level". I am using clang version 10.0.0. [Update] Seems the problem is at applying the patch series, even though it showed all sucessfully applied, but somehow in eal_common_log.c the include of was not added. After add include rte_os_shim.h, build completes. > > > > > > > diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build > > > index 7e9c7bdd6..02aea0255 100644 > > > --- a/app/test-pmd/meson.build > > > +++ b/app/test-pmd/meson.build > > > @@ -4,6 +4,10 @@ > > > # override default name to drop the hyphen > > > name = 'testpmd' > > > cflags += '-Wno-deprecated-declarations' > > > + > > > +# Enable using internal APIs in testpmd > > > +cflags += ['-DALLOW_INTERNAL_API'] > > > > Apps except unit tests should not use internal API, which is for DPDK > > components only. See below about memory mapping API. > > Good to know. Thanks. > > > > > [...] > > > @@ -688,13 +691,11 @@ alloc_mem(size_t memsz, size_t pgsz, bool huge) > > > int flags; > > > > > > /* allocate anonymous hugepages */ > > > - flags = MAP_ANONYMOUS | MAP_PRIVATE; > > > + flags = RTE_MAP_ANONYMOUS | RTE_MAP_PRIVATE; > > > if (huge) > > > flags |= HUGE_FLAG | pagesz_flags(pgsz); > > > > It is incorrect to mix RTE_ and POSIX flags. > > I suggest leaving "extmem" part Unix-only for now an not change it. > > Thanks for the advice. I will exclude extmem part on Windows then. In this case, seems Patch 5/6 is unnecessary any more. > > > > > > > > [...] > > > @@ -3824,8 +3827,8 @@ main(int argc, char** argv) > > > latencystats_enabled = 0; > > > #endif > > > > > > - /* on FreeBSD, mlockall() is disabled by default */ > > > -#ifdef RTE_EXEC_ENV_FREEBSD > > > + /* on FreeBSD and Window, mlockall() is disabled by default */ > > > +#if (defined(RTE_EXEC_ENV_FREEBSD) || defined (RTE_EXEC_ENV_WINDOWS)) > > > do_mlockall = 0; > > > #else > > > do_mlockall = 1; > > > > Redundant braces and a typo: "Window". > > Will fix in V4. > > > > > [...] > > > @@ -3971,10 +3978,11 @@ main(int argc, char** argv) > > > return 1; > > > } > > > > > > +#ifndef RTE_EXEC_ENV_WINDOWS > > > ret = rte_eal_cleanup(); > > > if (ret != 0) > > > rte_exit(EXIT_FAILURE, > > > "EAL cleanup failed: %s\n", strerror(-ret)); > > > - > > > +#endif > > > > You seem to have missed my comment on v2 that bypassing cleanup is no longer > > required after b2f24588b5 ("mem: fix cleanup when multi-process is disabled"). > > Missed this one as well. Will remove the bypassing cleanup in V4. Thanks. > > > > > > diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h > > > index 60a623d4c..095f5f13b 100644 > > > --- a/lib/librte_eal/windows/include/rte_os.h > > > +++ b/lib/librte_eal/windows/include/rte_os.h > > > @@ -24,6 +24,14 @@ extern "C" { > > > #define PATH_MAX _MAX_PATH > > > #endif > > > > > > +#define strcasecmp _stricmp > > > +#define open _open > > > +#define read _read > > > + > > > +#ifndef S_ISREG > > > +#define S_ISREG(mode) (((mode)&S_IFMT) == S_IFREG) > > > +#endif > > > + > > > #ifndef sleep > > > #define sleep(x) Sleep(1000 * (x)) > > > #endif > > > > This duplicates patch 3/6, doesn't it? > > Yeah, seems messed this up at spliting the patch. Will fix in V4.