Include OVS as a git submodule.

OVN developers have had isssues with the current method by which OVS source code is used by OVN. * There is no way to record the minimum commit/version of OVS to use when compiling OVN. * When debugging issues, bisecting OVN commits may also requires simultaneously changing OVS commits. This makes for multiple moving targets to try to track. * Performance improvements made to OVS libraries and OVSDB may benefit OVN. However, there's no way to encourage the use of the improved OVS source. By using a submodule, it allows for OVN to record a specific commit of OVS that is expected to be used.
zhaiyj · Feb 9, 2021 · 9ea1f09 · 9ea1f09
1 parent 022ea33
commit 9ea1f09
Show file tree

Hide file tree

Showing 10 changed files with 59 additions and 28 deletions.
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
@@ -10,8 +10,7 @@ EXTRA_OPTS="--enable-Werror"
 
 function configure_ovs()
 {
-    git clone https://github.com/openvswitch/ovs.git ovs_src
-    pushd ovs_src
+    pushd ovs
     ./boot.sh && ./configure $* || { cat config.log; exit 1; }
     make -j4 || { cat config.log; exit 1; }
     popd
@@ -22,7 +21,7 @@ function configure_ovn()
     configure_ovs $*
 
     export OVS_CFLAGS="${OVS_CFLAGS} ${OVN_CFLAGS}"
-    ./boot.sh && ./configure --with-ovs-source=$PWD/ovs_src $* || \
+    ./boot.sh && ./configure $* || \
     { cat config.log; exit 1; }
 }
 
@@ -54,7 +53,7 @@ if [ "$TESTSUITE" ]; then
     # Now we only need to prepare the Makefile without sparse-wrapped CC.
     configure_ovn
 
-    export DISTCHECK_CONFIGURE_FLAGS="$OPTS --with-ovs-source=$PWD/ovs_src"
+    export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
     if ! make distcheck -j4 TESTSUITEFLAGS="-j4" RECHECK=yes; then
         # testsuite.log is necessary for debugging.
         cat */_build/sub/tests/testsuite.log

diff --git a/.ci/osx-build.sh b/.ci/osx-build.sh
@@ -7,8 +7,7 @@ EXTRA_OPTS=""
 
 function configure_ovs()
 {
-    git clone https://github.com/openvswitch/ovs.git ovs_src
-    pushd ovs_src
+    pushd ovs
     ./boot.sh && ./configure $*
     make -j4 || { cat config.log; exit 1; }
     popd
@@ -17,7 +16,7 @@ function configure_ovs()
 function configure_ovn()
 {
     configure_ovs $*
-    ./boot.sh && ./configure $* --with-ovs-source=$PWD/ovs_src
+    ./boot.sh && ./configure $*
 }
 
 configure_ovn $EXTRA_OPTS $*
@@ -32,7 +31,7 @@ if ! "$@"; then
     exit 1
 fi
 if [ "$TESTSUITE" ] && [ "$CC" != "clang" ]; then
-    export DISTCHECK_CONFIGURE_FLAGS="$EXTRA_OPTS --with-ovs-source=$PWD/ovs_src"
+    export DISTCHECK_CONFIGURE_FLAGS="$EXTRA_OPTS"
     if ! make distcheck RECHECK=yes; then
         # testsuite.log is necessary for debugging.
         cat */_build/sub/tests/testsuite.log

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -50,6 +50,8 @@ jobs:
     steps:
     - name: checkout
       uses: actions/checkout@v2
+      with:
+        submodules: recursive
 
     - name: update APT cache
       run:  sudo apt update
@@ -104,6 +106,8 @@ jobs:
     steps:
     - name: checkout
       uses: actions/checkout@v2
+      with:
+        submodules: recursive
     - name: install dependencies
       run:  brew install automake libtool
     - name: prepare

diff --git a/.gitmodules b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "ovs"]
+	path = ovs
+	url = https://github.com/openvswitch/ovs.git
diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst
@@ -66,6 +66,10 @@ To compile the userspace programs in the OVN distribution, you will
 need the following software:
 
 - Open vSwitch (https://docs.openvswitch.org/en/latest/intro/install/).
+  Open vSwitch is included as a submodule in the OVN source code. It is
+  kept at the minimum recommended version for OVN to operate optimally.
+  See below for instructions about how to use a different OVS source
+  location.
 
 - GNU make
 
@@ -140,27 +144,44 @@ Bootstrapping
 -------------
 
 This step is not needed if you have downloaded a released tarball. If
-you pulled the sources directly from an Open vSwitch Git tree or got a
-Git tree snapshot, then run boot.sh in the top source directory to build
+you pulled the sources directly from an OVN Git tree or got a Git tree
+snapshot, then run boot.sh in the top source directory to build
 the "configure" script::
 
     $ ./boot.sh
 
-Before configuring OVN, clone, configure and build Open vSwitch.
+Before configuring OVN, build Open vSwitch. The easiest way to do this
+is to use the included OVS submodule in the OVN source::
+
+    $ git submodule update --init
+    $ cd ovs
+    $ ./boot.sh
+    $ ./configure
+    $ make
+    $ cd ..
+
+It is not required to use the included OVS submodule; however the OVS
+submodule is guaranteed to be the minimum recommended version of OVS
+to ensure OVN's optimal operation. If you wish to use OVS source code
+from a different location on the file system, then be sure to configure
+and build OVS before building OVN.
 
 .. _general-configuring:
 
 Configuring
 -----------
 
-Configure the package by running the configure script. You need to
-invoke configure with atleast the argument --with-ovs-source.
-For example::
+Then configure the package by running the configure script::
+
+    $ ./configure
+
+If your OVS source directory is not the included OVS submodule, specify the
+location of the OVS source code using --with-ovs-source::
 
     $ ./configure --with-ovs-source=/path/to/ovs/source
 
-If you have built Open vSwitch in a separate directory, then you
-need to provide that path in the option - --with-ovs-build.
+If you have built Open vSwitch in a separate directory from its source
+code, then you need to provide that path in the option - --with-ovs-build.
 
 By default all files are installed under ``/usr/local``. OVN expects to find
 its database in ``/usr/local/etc/ovn`` by default.

diff --git a/Makefile.am b/Makefile.am
@@ -48,6 +48,8 @@ AM_CFLAGS = -Wstrict-prototypes
 AM_CFLAGS += $(WARNING_FLAGS)
 AM_CFLAGS += $(OVS_CFLAGS)
 
+AM_DISTCHECK_CONFIGURE_FLAGS = --with-ovs-source=$(PWD)/ovs
+
 if NDEBUG
 AM_CPPFLAGS += -DNDEBUG
 AM_CFLAGS += -fomit-frame-pointer
@@ -157,6 +159,7 @@ noinst_HEADERS += $(EXTRA_DIST)
 
 ro_c = echo '/* -*- mode: c; buffer-read-only: t -*- */'
 ro_shell = printf '\043 Generated automatically -- do not modify!    -*- buffer-read-only: t -*-\n'
+submodules = $(shell grep 'path =' .gitmodules | sed -E 's/[\t ]*path =\s*(.*)/\1/g' | xargs)
 
 SUFFIXES += .in
 .in:
@@ -216,6 +219,8 @@ dist-hook-git: distfiles
 	@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1; then \
 	  (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | \
 	    grep -v '\.gitattributes$$' | \
+	    grep -v '\.gitmodules$$' | \
+	    grep -v "$(submodules)" | \
 	    LC_ALL=C sort -u > all-gitfiles; \
 	  LC_ALL=C comm -1 -3 distfiles all-gitfiles > missing-distfiles; \
 	  if test -s missing-distfiles; then \
@@ -247,8 +252,8 @@ ALL_LOCAL += config-h-check
 config-h-check:
 	@cd $(srcdir); \
 	if test -e .git && (git --version) >/dev/null 2>&1 && \
-	  git --no-pager grep -L '#include <config\.h>' `git ls-files | grep '\.c$$' | \
-	    grep -vE '^ovs/datapath|^ovs/lib/sflow|^ovs/datapath-windows|^python|^ovs/python'`; \
+	  git --no-pager grep -L '#include <config\.h>' `git ls-files | grep -v $(submodules) | grep '\.c$$' | \
+	    grep -vE '^python'`; \
 	then \
 	  echo "See above for list of violations of the rule that"; \
 	  echo "every C source file must #include <config.h>."; \
@@ -261,8 +266,7 @@ ALL_LOCAL += printf-check
 printf-check:
 	@cd $(srcdir); \
 	if test -e .git && (git --version) >/dev/null 2>&1 && \
-	  git --no-pager grep -n -E -e '%[-+ #0-9.*]*([ztj]|hh)' --and --not -e 'ovs_scan' `git ls-files | grep '\.[ch]$$' | \
-	    grep -vE '^ovs/datapath|^ovs/lib/sflow'`; \
+	  git --no-pager grep -n -E -e '%[-+ #0-9.*]*([ztj]|hh)' --and --not -e 'ovs_scan' `git ls-files | grep -v $(submodules) | grep '\.[ch]$$'`; \
 	then \
 	  echo "See above for list of violations of the rule that"; \
 	  echo "'z', 't', 'j', 'hh' printf() type modifiers are"; \
@@ -288,7 +292,7 @@ ALL_LOCAL += check-assert-h-usage
 check-assert-h-usage:
 	@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
 	  (cd $(srcdir) && git --no-pager grep -l -E '[<]assert.h[>]') | \
-	  $(EGREP) -v '^ovs/lib/(sflow_receiver|vlog).c$$|^ovs/tests/|^tests/'; \
+	  $(EGREP) -v '^tests/'; \
 	then \
 	  echo "Files listed above unexpectedly #include <""assert.h"">."; \
 	  echo "Please use ovs_assert (from util.h) instead of assert."; \
@@ -304,8 +308,7 @@ ALL_LOCAL += check-endian
 check-endian:
 	@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
 	  (cd $(srcdir) && git --no-pager grep -l -E \
-	   -e 'BIG_ENDIAN|LITTLE_ENDIAN' --and --not -e 'BYTE_ORDER' | \
-	  $(EGREP) -v '^ovs/datapath/|^ovs/include/sparse/rte_'); \
+	   -e 'BIG_ENDIAN|LITTLE_ENDIAN' --and --not -e 'BYTE_ORDER'); \
 	then \
 	  echo "See above for list of files that misuse LITTLE""_ENDIAN"; \
 	  echo "or BIG""_ENDIAN.  Please use WORDS_BIGENDIAN instead."; \
@@ -329,7 +332,7 @@ check-tabs:
 	@cd $(srcdir); \
 	if test -e .git && (git --version) >/dev/null 2>&1 && \
 	  grep -ln "^	" \
-	    `git ls-files \
+	    `git ls-files | grep -v $(submodules) \
 	      | grep -v -f build-aux/initial-tab-whitelist` /dev/null \
 	      | $(EGREP) -v ':[ 	]*/?\*'; \
 	then \
@@ -344,8 +347,7 @@ thread-safety-check:
 	@cd $(srcdir); \
 	if test -e .git && (git --version) >/dev/null 2>&1 && \
 	  grep -n -f build-aux/thread-safety-blacklist \
-	    `git ls-files | grep '\.[ch]$$' \
-	      | $(EGREP) -v '^ovs/datapath|^ovs/lib/sflow'` /dev/null \
+	    `git ls-files | grep -v $(submodules) | grep '\.[ch]$$'` /dev/null \
 	      | $(EGREP) -v ':[ 	]*/?\*'; \
 	then \
 	  echo "See above for list of calls to functions that are"; \

diff --git a/acinclude.m4 b/acinclude.m4
@@ -338,7 +338,7 @@ AC_DEFUN([OVN_CHECK_OVS], [
       AC_ERROR([$OVSDIR is not an OVS source directory])
     fi
   else
-    AC_ERROR([OVS source dir path needs to be specified (use --with-ovs-source)])
+    OVSDIR=`pwd`/ovs
   fi
 
   AC_MSG_RESULT([$OVSDIR])

diff --git a/build-aux/initial-tab-whitelist b/build-aux/initial-tab-whitelist
@@ -8,3 +8,4 @@
 ^xenserver/
 ^debian/rules.modules$
 ^debian/rules$
+^\.gitmodules$
diff --git a/ovs b/ovs
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
@@ -189,7 +189,8 @@ def reset_counters():
 # Don't enforce a requirement that leading whitespace be all spaces on
 # files that include these characters in their name, since these kinds
 # of files need lines with leading tabs.
-leading_whitespace_blacklist = re.compile(r'\.(mk|am|at)$|debian/rules')
+leading_whitespace_blacklist = re.compile(
+    r'\.(mk|am|at)$|debian/rules|\.gitmodules$')
 
 
 def is_subtracted_line(line):