[ovs-dev] datapath-windows: Support to selectively compile targets

Message ID 20180206212939.13188-1-rams@vmware.com
State New
Headers show
Series
  • [ovs-dev] datapath-windows: Support to selectively compile targets
Related show

Commit Message

Shashank Ram Feb. 6, 2018, 9:29 p.m.
Adds support to selectively compile kernel driver for
target versions. This is useful when environments to
compile for all targets might not be available on the
user's machine, or if the user wants to only compile
some targets selectively.

Also once appveyor has support to build Win10 targets,
we will not pass the "--with-vstudiotargetver" to the
configure script.

Signed-off-by: Shashank Ram <rams@vmware.com>
---
 Makefile.am       | 15 +++++++++++++++
 appveyor.yml      |  4 ++--
 m4/openvswitch.m4 | 28 +++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 3 deletions(-)

Comments

Alin Gabriel Serdean Feb. 8, 2018, 5:32 p.m. | #1
It looks a bit complicated.

What do you think about the following approach:

git diff datapath-windows/automake.mk
diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 3820041f6..164567734 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -85,6 +85,26 @@ EXTRA_DIST += \
        datapath-windows/ovsext/precompsrc.c \
        datapath-windows/ovsext/resource.h

+ARCH=x64
+
 datapath_windows_analyze: all
-       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Analyze"
-       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Analyze"
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Analyze" //p:Platform=$(ARCH)
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Analyze" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_debug: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_release: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8Release" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_1_debug: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_8_1_release: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win8.1Release" //p:Platform=$(ARCH)
+
+datapath_windows_win_10_debug: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10Debug" //p:Platform=$(ARCH)
+
+datapath_windows_win_10_release: all
+       MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln
/target:Build /property:Configuration="Win10Release" //p:Platform=$(ARCH)

./boot.sh; ./configure CC=./build-aux/cccl LD="$(which link)" LIBS="-lws2_32
-liphlpapi -lwbemuuid -lole32 -loleaut32" --prefix="C:/openvswitch/usr"
--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc"
--with-pthread="C:/pthread"; make clean && make -j6; make
datapath_windows_win_8_debug datapath_windows_win_8_release
datapath_windows_win_8_1_debug datapath_windows_win_8_1_release
datapath_windows_win_10_debug datapath_windows_win_10_release

On the plus side you don't need to rerun the configure when you want to
change things. As another plus size you just pass in the parameters for the
userspace and compile the
Debug/release version which the user prefers.

Thanks,
Alin.

-----Mesaj original-----
De la: ovs-dev-bounces@openvswitch.org
[mailto:ovs-dev-bounces@openvswitch.org] În numele Shashank Ram
Trimis: Tuesday, February 6, 2018 11:30 PM
Către: dev@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Support to selectively compile
targets

Adds support to selectively compile kernel driver for target versions. This
is useful when environments to compile for all targets might not be
available on the user's machine, or if the user wants to only compile some
targets selectively.

Also once appveyor has support to build Win10 targets, we will not pass the
"--with-vstudiotargetver" to the configure script.

Signed-off-by: Shashank Ram <rams@vmware.com>
---
Shashank Ram Feb. 8, 2018, 5:49 p.m. | #2
Hi Alin, thanks for the review.
I personally feel we should be consistent and run configure, and have a single make command to build both user space and kernel. What part did you find complicated?

Thanks,
Shashank
Alin Gabriel Serdean Feb. 8, 2018, 6:43 p.m. | #3
Trimming the message a bit.

-----Mesaj original-----
De la: ovs-dev-bounces@openvswitch.org
[mailto:ovs-dev-bounces@openvswitch.org] În numele Shashank Ram
Trimis: Thursday, February 8, 2018 7:50 PM
Către: aserdean@ovn.org; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Support to selectively
compile targets

Hi Alin, thanks for the review.
I personally feel we should be consistent and run configure, and have a
single make command to build both user space and kernel. What part did you
find complicated?
[Alin Serdean] I.e. if I configure to target 8. And after I need to target
10 I need to do a reconfigure (similar, for debug and or other platforms).
The configure part is particularly slow on Windows.
For convenience the old part with selecting Debug/Release and
trying to build for all the compilers found in the system is still there, so
building both
userspace and kernel will still be in a single command.
I don't see a huge issue to specify two or more make commands to build a
particular target of the kernel
via the shell.

Thanks,
Shashank
Anand Kumar Feb. 13, 2018, 4:33 a.m. | #4
Hi,

My thoughts are with Shashank on this, it makes sense to have 1 configure and 1 make command to build a particular target, instead of having flexibility to specify multiple targets.

Thanks,
Anand Kumar 

On 2/8/18, 10:56 AM, "ovs-dev-bounces@openvswitch.org on behalf of Shashank Ram" <ovs-dev-bounces@openvswitch.org on behalf of rams@vmware.com> wrote:

    
    
    
    
    ________________________________________
    From: aserdean@ovn.org <aserdean@ovn.org>

    Sent: Thursday, February 8, 2018 10:43 AM
    To: Shashank Ram; aserdean@ovn.org; dev@openvswitch.org
    Subject: RE: [ovs-dev] [PATCH] datapath-windows: Support to selectively compile targets
    
    Trimming the message a bit.
    
    -----Mesaj original-----
    De la: ovs-dev-bounces@openvswitch.org
    [mailto:ovs-dev-bounces@openvswitch.org] În numele Shashank Ram
    Trimis: Thursday, February 8, 2018 7:50 PM
    Către: aserdean@ovn.org; dev@openvswitch.org
    Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Support to selectively
    compile targets
    
    Hi Alin, thanks for the review.
    I personally feel we should be consistent and run configure, and have a
    single make command to build both user space and kernel. What part did you
    find complicated?
    [Alin Serdean] I.e. if I configure to target 8. And after I need to target
    10 I need to do a reconfigure (similar, for debug and or other platforms).
    [SR]: In an automated environment, this shouldn't happen.
    For local compilation, you should be able to manually compile the kernel.
    
    The configure part is particularly slow on Windows.
    For convenience the old part with selecting Debug/Release and
    trying to build for all the compilers found in the system is still there, so
    building both
    userspace and kernel will still be in a single command.
    I don't see a huge issue to specify two or more make commands to build a
    particular target of the kernel
    via the shell.
    [SR]: I don't think its a big deal either, but its more convenient to run 1 configure and 1
    make command.
    
    I prefer to keep this as is for now and wait for more reviews.
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFBA&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=fKK6KZRD0tZEfwHzLhMsabCH5aXzzYiRP-pJR20Xj9o&s=nEl_7Q-LhJ74AdsiY85DjA-kWy0uESr5DyFrWDQYKjs&e=
Anand Kumar Feb. 16, 2018, 7:04 p.m. | #5
Acked-by: Anand Kumar <kumaranand@vmware.com>
       
        Thanks,
        Anand Kumar
        
        ________________________________________
        From: Shashank Ram <rams@vmware.com>
        Sent: Tuesday, February 6, 2018 1:29 PM
        To: dev@openvswitch.org
        Cc: Shashank Ram
        Subject: [PATCH] datapath-windows: Support to selectively compile targets
        
        Adds support to selectively compile kernel driver for
        target versions. This is useful when environments to
        compile for all targets might not be available on the
        user's machine, or if the user wants to only compile
        some targets selectively.
        
        Also once appveyor has support to build Win10 targets,
        we will not pass the "--with-vstudiotargetver" to the
        configure script.
        
        Signed-off-by: Shashank Ram <rams@vmware.com>
        ---
         Makefile.am       | 15 +++++++++++++++
         appveyor.yml      |  4 ++--
         m4/openvswitch.m4 | 28 +++++++++++++++++++++++++++-
         3 files changed, 44 insertions(+), 3 deletions(-)
        
        diff --git a/Makefile.am b/Makefile.am
        index d397f65..e035a98 100644
        --- a/Makefile.am
        +++ b/Makefile.am
        @@ -411,14 +411,29 @@ if VSTUDIO_DDK
         ALL_LOCAL += ovsext
         ARCH = x64
         ovsext: datapath-windows/ovsext.sln $(srcdir)/datapath-windows/include/OvsDpInterface.h
        +if VSTUDIO_WIN8
                MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
        +endif
        +if VSTUDIO_WIN8_1
                MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
        +endif
        +if VSTUDIO_WIN10
        +        MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win10$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
        +endif
        +
        
         CLEAN_LOCAL += ovsext_clean
         ovsext_clean: datapath-windows/ovsext.sln
        +if VSTUDIO_WIN8
                MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
        +endif
        +if VSTUDIO_WIN8_1
                MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
         endif
        +if VSTUDIO_WIN10
        +        MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win10$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
        +endif
        +endif
         .PHONY: ovsext
        
         clang-analyze: clean
        diff --git a/appveyor.yml b/appveyor.yml
        index 0881e05..da31764 100644
        --- a/appveyor.yml
        +++ b/appveyor.yml
        @@ -1,6 +1,6 @@
         version: 1.0.{build}
         branches:
        -  only:
        +  only:
           - master
         clone_folder: C:\openvswitch
         init:
        @@ -41,6 +41,6 @@ build_script:
         - C:\MinGW\msys\1.0\bin\bash -lc "cp /c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/."
         - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
         - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
        -- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\""
        +- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\" --with-vstudiotargetver=\"Win8,Win8.1\""
         - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
         - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make datapath_windows_analyze"
        diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
        index de4d66c..78082d4 100644
        --- a/m4/openvswitch.m4
        +++ b/m4/openvswitch.m4
        @@ -173,6 +173,32 @@ AC_ARG_WITH([vstudiotarget],
               )
        
           AC_SUBST([VSTUDIO_CONFIG])
        +
        +AC_ARG_WITH([vstudiotargetver],
        +         [AS_HELP_STRING([--with-vstudiotargetver=target_ver1,target_ver2],
        +            [Target versions: Win8,Win8.1,Win10])],
        +         [
        +            targetver=`echo "$withval" | tr -s , ' ' `
        +            for ver in $targetver; do
        +                case "$ver" in
        +                "Win8") VSTUDIO_WIN8=true ;;
        +                "Win8.1")  VSTUDIO_WIN8_1=true ;;
        +                "Win10") VSTUDIO_WIN10=true ;;
        +                *) AC_MSG_ERROR([No valid Visual Studio target version found]) ;;
        +                esac
        +            done
        +
        +         ], [
        +            VSTUDIO_WIN8=true
        +            VSTUDIO_WIN8_1=true
        +            VSTUDIO_WIN10=true
        +         ]
        +      )
        +
        +  AM_CONDITIONAL([VSTUDIO_WIN8], [test -n "$VSTUDIO_WIN8"])
        +  AM_CONDITIONAL([VSTUDIO_WIN8_1], [test -n "$VSTUDIO_WIN8_1"])
        +  AM_CONDITIONAL([VSTUDIO_WIN10], [test -n "$VSTUDIO_WIN10"])
        +
           AC_DEFINE([VSTUDIO_DDK], [1], [System uses the Visual Studio build target.])
           AM_CONDITIONAL([VSTUDIO_DDK], [test -n "$VSTUDIO_CONFIG"])
         ])
        @@ -573,7 +599,7 @@ TEST_ATOMIC_TYPE(unsigned long long int);
         dnl OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(SIZE)
         dnl
         dnl Checks __atomic_always_lock_free(SIZE, 0)
        -AC_DEFUN([OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE],
        +AC_DEFUN([OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE],
           [AC_CACHE_CHECK(
             [value of __atomic_always_lock_free($1)],
             [ovs_cv_atomic_always_lock_free_$1],
        --
        2.9.3.windows.2

Patch

diff --git a/Makefile.am b/Makefile.am
index d397f65..e035a98 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -411,14 +411,29 @@  if VSTUDIO_DDK
 ALL_LOCAL += ovsext
 ARCH = x64
 ovsext: datapath-windows/ovsext.sln $(srcdir)/datapath-windows/include/OvsDpInterface.h
+if VSTUDIO_WIN8
 	MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
+endif
+if VSTUDIO_WIN8_1
 	MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
+endif
+if VSTUDIO_WIN10
+        MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win10$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
+endif
+
 
 CLEAN_LOCAL += ovsext_clean
 ovsext_clean: datapath-windows/ovsext.sln
+if VSTUDIO_WIN8
 	MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
+endif
+if VSTUDIO_WIN8_1
 	MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
 endif
+if VSTUDIO_WIN10
+        MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win10$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH)
+endif
+endif
 .PHONY: ovsext
 
 clang-analyze: clean
diff --git a/appveyor.yml b/appveyor.yml
index 0881e05..da31764 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -1,6 +1,6 @@ 
 version: 1.0.{build}
 branches:
-  only: 
+  only:
   - master
 clone_folder: C:\openvswitch
 init:
@@ -41,6 +41,6 @@  build_script:
 - C:\MinGW\msys\1.0\bin\bash -lc "cp /c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/."
 - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\""
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\" --with-vstudiotargetver=\"Win8,Win8.1\""
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make datapath_windows_analyze"
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index de4d66c..78082d4 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -173,6 +173,32 @@  AC_ARG_WITH([vstudiotarget],
       )
 
   AC_SUBST([VSTUDIO_CONFIG])
+
+AC_ARG_WITH([vstudiotargetver],
+         [AS_HELP_STRING([--with-vstudiotargetver=target_ver1,target_ver2],
+            [Target versions: Win8,Win8.1,Win10])],
+         [
+            targetver=`echo "$withval" | tr -s , ' ' `
+            for ver in $targetver; do
+                case "$ver" in
+                "Win8") VSTUDIO_WIN8=true ;;
+                "Win8.1")  VSTUDIO_WIN8_1=true ;;
+                "Win10") VSTUDIO_WIN10=true ;;
+                *) AC_MSG_ERROR([No valid Visual Studio target version found]) ;;
+                esac
+            done
+
+         ], [
+            VSTUDIO_WIN8=true
+            VSTUDIO_WIN8_1=true
+            VSTUDIO_WIN10=true
+         ]
+      )
+
+  AM_CONDITIONAL([VSTUDIO_WIN8], [test -n "$VSTUDIO_WIN8"])
+  AM_CONDITIONAL([VSTUDIO_WIN8_1], [test -n "$VSTUDIO_WIN8_1"])
+  AM_CONDITIONAL([VSTUDIO_WIN10], [test -n "$VSTUDIO_WIN10"])
+
   AC_DEFINE([VSTUDIO_DDK], [1], [System uses the Visual Studio build target.])
   AM_CONDITIONAL([VSTUDIO_DDK], [test -n "$VSTUDIO_CONFIG"])
 ])
@@ -573,7 +599,7 @@  TEST_ATOMIC_TYPE(unsigned long long int);
 dnl OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(SIZE)
 dnl
 dnl Checks __atomic_always_lock_free(SIZE, 0)
-AC_DEFUN([OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE], 
+AC_DEFUN([OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE],
   [AC_CACHE_CHECK(
     [value of __atomic_always_lock_free($1)],
     [ovs_cv_atomic_always_lock_free_$1],