mbox series

[ovs-dev,v2,ovn,0/5] External OVS source support and separate run dir for OVN

Message ID 20190819181330.2700-1-nusiddiq@redhat.com
Headers show
Series External OVS source support and separate run dir for OVN | expand

Message

Numan Siddique Aug. 19, 2019, 6:13 p.m. UTC
From: Numan Siddique <nusiddiq@redhat.com>

This patch series adds support for building OVN from external OVS
sources.

The first patch adds the support to compile OVN from external OVS sources.
The following configuration options are added when configuring OVN
  * --with-ovs-source (mandatory)
  * --with-ovs-build (optional)

Patch 2 adds support to run OVN services using separate
directores 
  - Default run time dir - /usr/local/var/run/ovm
  - Default log dir - /usr/loca/var/log/ovn
  - Default db dir - /usr/loca/etc/ovn

Patch 3 fixes "make rpm-fedora" which is presently broken

Patch 4 runs OVN services as openvswitch user for rhel when rpms are
used.

Patch 5 removes the python subdirectory as that directory belongs
to OVS and uses the required files from the OVS repo.

v1 -> v2
========
 * Addressed the review comments.
 * Swapped the patch 1 and 2 as it was easier to address Mark's comment
   on OVS_RUNDIR/OVN_RUNDIR
 * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few of
   the macros to OVS_* to OVN_*.

Combined the patch 1 and 2 in this series which were submitted
separately earlier.


Numan Siddique (5):
  Build OVN using external OVS directory
  Add support for using OVN specific rundirs
  Fix "make rpm-fedora"
  rhel: Run ovn services with the 'openvswitch' user
  Remove python directory

 .travis/linux-build.sh                        |   17 +-
 .travis/osx-build.sh                          |   13 +-
 Documentation/intro/install/fedora.rst        |   13 +-
 Documentation/intro/install/general.rst       |   63 +-
 Makefile.am                                   |   35 +-
 TODO_SPLIT.rst                                |    2 +
 acinclude.m4                                  |   35 +
 configure.ac                                  |   63 +-
 controller-vtep/automake.mk                   |    2 +-
 controller/ovn-controller.c                   |    4 +-
 include/ovn/version.h.in                      |   28 +
 lib/.gitignore                                |    1 +
 lib/automake.mk                               |   21 +-
 lib/ovn-dirs.c.in                             |  112 +
 lib/ovn-dirs.h                                |   35 +
 lib/ovn-util.c                                |   24 +-
 lib/ovn-util.h                                |    1 +
 lib/ovsdb_automake.mk                         |    7 +-
 m4/{openvswitch.m4 => ovn.m4}                 |   60 +-
 northd/ovn-northd.c                           |    9 +-
 python/.gitignore                             |    2 -
 python/README.rst                             |    1 -
 python/automake.mk                            |  123 -
 python/build/__init__.py                      |    0
 python/build/nroff.py                         |  398 ---
 python/build/soutil.py                        |   56 -
 python/ovs/.gitignore                         |    1 -
 python/ovs/__init__.py                        |    1 -
 python/ovs/_json.c                            |  269 --
 python/ovs/compat/__init__.py                 |    0
 python/ovs/compat/sortedcontainers/LICENSE    |   13 -
 .../ovs/compat/sortedcontainers/__init__.py   |   52 -
 .../ovs/compat/sortedcontainers/sorteddict.py |  741 -----
 .../ovs/compat/sortedcontainers/sortedlist.py | 2508 -----------------
 .../ovs/compat/sortedcontainers/sortedset.py  |  327 ---
 python/ovs/daemon.py                          |  652 -----
 python/ovs/db/__init__.py                     |    1 -
 python/ovs/db/custom_index.py                 |  154 -
 python/ovs/db/data.py                         |  585 ----
 python/ovs/db/error.py                        |   34 -
 python/ovs/db/idl.py                          | 2030 -------------
 python/ovs/db/parser.py                       |  118 -
 python/ovs/db/schema.py                       |  304 --
 python/ovs/db/types.py                        |  647 -----
 python/ovs/dirs.py                            |   31 -
 python/ovs/dirs.py.template                   |   31 -
 python/ovs/fatal_signal.py                    |  183 --
 python/ovs/fcntl_win.py                       |   46 -
 python/ovs/json.py                            |  531 ----
 python/ovs/jsonrpc.py                         |  616 ----
 python/ovs/ovsuuid.py                         |   70 -
 python/ovs/poller.py                          |  290 --
 python/ovs/process.py                         |   41 -
 python/ovs/reconnect.py                       |  608 ----
 python/ovs/socket_util.py                     |  335 ---
 python/ovs/stream.py                          |  831 ------
 python/ovs/timeval.py                         |   81 -
 python/ovs/unixctl/__init__.py                |   91 -
 python/ovs/unixctl/client.py                  |   68 -
 python/ovs/unixctl/server.py                  |  260 --
 python/ovs/util.py                            |   95 -
 python/ovs/vlog.py                            |  475 ----
 python/ovs/winutils.py                        |  266 --
 python/ovstest/__init__.py                    |    1 -
 python/ovstest/args.py                        |  283 --
 python/ovstest/rpcserver.py                   |  383 ---
 python/ovstest/tcp.py                         |  120 -
 python/ovstest/tests.py                       |  250 --
 python/ovstest/udp.py                         |   85 -
 python/ovstest/util.py                        |  253 --
 python/ovstest/vswitch.py                     |  107 -
 python/setup.py                               |  102 -
 rhel/automake.mk                              |    5 +-
 rhel/etc_logrotate.d_ovn                      |   22 +
 rhel/ovn-fedora.spec.in                       |   91 +-
 ...systemd_system_ovn-controller-vtep.service |   15 +-
 ..._lib_systemd_system_ovn-controller.service |    9 +-
 .../usr_lib_systemd_system_ovn-northd.service |   15 +-
 ...are_ovn_scripts_systemd_sysconfig.template |   13 +
 tests/automake.mk                             |    6 +-
 tests/ofproto-macros.at                       |    4 +-
 tests/ovn-controller-vtep.at                  |   14 +-
 tests/ovn-nbctl.at                            |    6 +-
 tests/ovn-sbctl.at                            |   20 +-
 tests/ovn.at                                  |  158 +-
 tests/ovs-macros.at                           |    1 +
 tests/ovsdb-macros.at                         |    2 +-
 tutorial/automake.mk                          |    2 +-
 tutorial/ovs-sandbox                          |  309 +-
 utilities/automake.mk                         |    5 +
 utilities/ovn-ctl                             |   86 +-
 utilities/ovn-ctl.8.xml                       |   12 +-
 utilities/ovn-lib.in                          |  204 ++
 93 files changed, 1081 insertions(+), 16013 deletions(-)
 create mode 100644 include/ovn/version.h.in
 create mode 100644 lib/ovn-dirs.c.in
 create mode 100644 lib/ovn-dirs.h
 rename m4/{openvswitch.m4 => ovn.m4} (94%)
 delete mode 100644 python/.gitignore
 delete mode 100644 python/README.rst
 delete mode 100644 python/automake.mk
 delete mode 100644 python/build/__init__.py
 delete mode 100644 python/build/nroff.py
 delete mode 100755 python/build/soutil.py
 delete mode 100644 python/ovs/.gitignore
 delete mode 100644 python/ovs/__init__.py
 delete mode 100644 python/ovs/_json.c
 delete mode 100644 python/ovs/compat/__init__.py
 delete mode 100644 python/ovs/compat/sortedcontainers/LICENSE
 delete mode 100644 python/ovs/compat/sortedcontainers/__init__.py
 delete mode 100644 python/ovs/compat/sortedcontainers/sorteddict.py
 delete mode 100644 python/ovs/compat/sortedcontainers/sortedlist.py
 delete mode 100644 python/ovs/compat/sortedcontainers/sortedset.py
 delete mode 100644 python/ovs/daemon.py
 delete mode 100644 python/ovs/db/__init__.py
 delete mode 100644 python/ovs/db/custom_index.py
 delete mode 100644 python/ovs/db/data.py
 delete mode 100644 python/ovs/db/error.py
 delete mode 100644 python/ovs/db/idl.py
 delete mode 100644 python/ovs/db/parser.py
 delete mode 100644 python/ovs/db/schema.py
 delete mode 100644 python/ovs/db/types.py
 delete mode 100644 python/ovs/dirs.py
 delete mode 100644 python/ovs/dirs.py.template
 delete mode 100644 python/ovs/fatal_signal.py
 delete mode 100644 python/ovs/fcntl_win.py
 delete mode 100644 python/ovs/json.py
 delete mode 100644 python/ovs/jsonrpc.py
 delete mode 100644 python/ovs/ovsuuid.py
 delete mode 100644 python/ovs/poller.py
 delete mode 100644 python/ovs/process.py
 delete mode 100644 python/ovs/reconnect.py
 delete mode 100644 python/ovs/socket_util.py
 delete mode 100644 python/ovs/stream.py
 delete mode 100644 python/ovs/timeval.py
 delete mode 100644 python/ovs/unixctl/__init__.py
 delete mode 100644 python/ovs/unixctl/client.py
 delete mode 100644 python/ovs/unixctl/server.py
 delete mode 100644 python/ovs/util.py
 delete mode 100644 python/ovs/vlog.py
 delete mode 100644 python/ovs/winutils.py
 delete mode 100644 python/ovstest/__init__.py
 delete mode 100644 python/ovstest/args.py
 delete mode 100644 python/ovstest/rpcserver.py
 delete mode 100644 python/ovstest/tcp.py
 delete mode 100644 python/ovstest/tests.py
 delete mode 100644 python/ovstest/udp.py
 delete mode 100644 python/ovstest/util.py
 delete mode 100644 python/ovstest/vswitch.py
 delete mode 100644 python/setup.py
 create mode 100644 rhel/etc_logrotate.d_ovn
 create mode 100644 rhel/usr_share_ovn_scripts_systemd_sysconfig.template
 create mode 100644 utilities/ovn-lib.in

Comments

Han Zhou Aug. 21, 2019, 4:59 p.m. UTC | #1
On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq@redhat.com>
>
> This patch series adds support for building OVN from external OVS
> sources.
>
> The first patch adds the support to compile OVN from external OVS sources.
> The following configuration options are added when configuring OVN
>   * --with-ovs-source (mandatory)
>   * --with-ovs-build (optional)
>
> Patch 2 adds support to run OVN services using separate
> directores
>   - Default run time dir - /usr/local/var/run/ovm
>   - Default log dir - /usr/loca/var/log/ovn
>   - Default db dir - /usr/loca/etc/ovn
>
> Patch 3 fixes "make rpm-fedora" which is presently broken
>
> Patch 4 runs OVN services as openvswitch user for rhel when rpms are
> used.
>
> Patch 5 removes the python subdirectory as that directory belongs
> to OVS and uses the required files from the OVS repo.
>
> v1 -> v2
> ========
>  * Addressed the review comments.
>  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
>    on OVS_RUNDIR/OVN_RUNDIR
>  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few of
>    the macros to OVS_* to OVN_*.
>
> Combined the patch 1 and 2 in this series which were submitted
> separately earlier.
>
>

Hi Numan,

Thanks for this work. I tried applying this series on master, and then
removed the "ovs" subfolder just to see if it uses the external OVS
completely. However I encountered some error:

/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H
-I.   -I ./include  -I ./include -I ./ovn -I ./include -I
/home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
/home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
/home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
lib/logical-fields.lo lib/logical-fields.c &&\
mv -f $depbase.Tpo $depbase.Plo
lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
lib/actions.c:2592:9: warning: implicit declaration of function
'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
         ofpact_put_CHECK_PKT_LARGER(ofpacts);
         ^
lib/actions.c:2592:9: warning: initialization makes pointer from integer
without a cast
lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
     check_pkt_larger->pkt_len = cipl->pkt_len;
                     ^
lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
     check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
                     ^

Is this expected?

Thanks,
Han
Numan Siddique Aug. 21, 2019, 6:54 p.m. UTC | #2
On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan@gmail.com> wrote:

>
>
> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
> >
> > From: Numan Siddique <nusiddiq@redhat.com>
> >
> > This patch series adds support for building OVN from external OVS
> > sources.
> >
> > The first patch adds the support to compile OVN from external OVS
> sources.
> > The following configuration options are added when configuring OVN
> >   * --with-ovs-source (mandatory)
> >   * --with-ovs-build (optional)
> >
> > Patch 2 adds support to run OVN services using separate
> > directores
> >   - Default run time dir - /usr/local/var/run/ovm
> >   - Default log dir - /usr/loca/var/log/ovn
> >   - Default db dir - /usr/loca/etc/ovn
> >
> > Patch 3 fixes "make rpm-fedora" which is presently broken
> >
> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
> > used.
> >
> > Patch 5 removes the python subdirectory as that directory belongs
> > to OVS and uses the required files from the OVS repo.
> >
> > v1 -> v2
> > ========
> >  * Addressed the review comments.
> >  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
> >    on OVS_RUNDIR/OVN_RUNDIR
> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few of
> >    the macros to OVS_* to OVN_*.
> >
> > Combined the patch 1 and 2 in this series which were submitted
> > separately earlier.
> >
> >
>
> Hi Numan,
>
> Thanks for this work. I tried applying this series on master, and then
> removed the "ovs" subfolder just to see if it uses the external OVS
> completely. However I encountered some error:
>
> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
> -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
> /home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
> /home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
> /home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
>  -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
> -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
> -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
> -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
> -Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
> lib/logical-fields.lo lib/logical-fields.c &&\
> mv -f $depbase.Tpo $depbase.Plo
> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
> lib/actions.c:2592:9: warning: implicit declaration of function
> 'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>          ofpact_put_CHECK_PKT_LARGER(ofpacts);
>          ^
> lib/actions.c:2592:9: warning: initialization makes pointer from integer
> without a cast
> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
>      check_pkt_larger->pkt_len = cipl->pkt_len;
>                      ^
> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
>      check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
>                      ^
>
> Is this expected?
>

Hi Han,

This is not expected. Before submitting I did test by removing the ovs
subdirectory. I tried even now and I don't see any issue.
Looks to me either you have done "make install" in the ovs repo or your
ovs sources are a bit old.

Can you please check if that's the case.

Thanks
Numan


> Thanks,
> Han
>
Han Zhou Aug. 21, 2019, 8:49 p.m. UTC | #3
On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique <nusiddiq@redhat.com> wrote:
>
>
>
> On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan@gmail.com> wrote:
>>
>>
>>
>> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
>> >
>> > From: Numan Siddique <nusiddiq@redhat.com>
>> >
>> > This patch series adds support for building OVN from external OVS
>> > sources.
>> >
>> > The first patch adds the support to compile OVN from external OVS
sources.
>> > The following configuration options are added when configuring OVN
>> >   * --with-ovs-source (mandatory)
>> >   * --with-ovs-build (optional)
>> >
>> > Patch 2 adds support to run OVN services using separate
>> > directores
>> >   - Default run time dir - /usr/local/var/run/ovm
>> >   - Default log dir - /usr/loca/var/log/ovn
>> >   - Default db dir - /usr/loca/etc/ovn
>> >
>> > Patch 3 fixes "make rpm-fedora" which is presently broken
>> >
>> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
>> > used.
>> >
>> > Patch 5 removes the python subdirectory as that directory belongs
>> > to OVS and uses the required files from the OVS repo.
>> >
>> > v1 -> v2
>> > ========
>> >  * Addressed the review comments.
>> >  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
>> >    on OVS_RUNDIR/OVN_RUNDIR
>> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few
of
>> >    the macros to OVS_* to OVN_*.
>> >
>> > Combined the patch 1 and 2 in this series which were submitted
>> > separately earlier.
>> >
>> >
>>
>> Hi Numan,
>>
>> Thanks for this work. I tried applying this series on master, and then
removed the "ovs" subfolder just to see if it uses the external OVS
completely. However I encountered some error:
>>
>> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
-DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
/home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
/home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
/home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
lib/logical-fields.lo lib/logical-fields.c &&\
>> mv -f $depbase.Tpo $depbase.Plo
>> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
>> lib/actions.c:2592:9: warning: implicit declaration of function
'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>>          ofpact_put_CHECK_PKT_LARGER(ofpacts);
>>          ^
>> lib/actions.c:2592:9: warning: initialization makes pointer from integer
without a cast
>> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
>>      check_pkt_larger->pkt_len = cipl->pkt_len;
>>                      ^
>> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
>>      check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
>>                      ^
>>
>> Is this expected?
>
>
> Hi Han,
>
> This is not expected. Before submitting I did test by removing the ovs
subdirectory. I tried even now and I don't see any issue.
> Looks to me either you have done "make install" in the ovs repo or your
 ovs sources are a bit old.
>
> Can you please check if that's the case.
>
> Thanks
> Numan
>
Thanks Numan. You are right that I was on an older branch of OVS. My bad.
Now OVN build is successful after correcting the branch and rebuilding OVS.

Just saw some annoying messages when doing make, but it seems no real
impact there.

comm: all-distfiles: No such file or directory
grep: all-distfiles: No such file or directory
fatal: ovs/lib/aes128.c: no such path in the working tree.
Use 'git <command> -- <path>...' to specify paths that do not exist locally.
fatal: ovs/include/linux/netfilter/nf_conntrack_sctp.h: no such path in the
working tree.
Use 'git <command> -- <path>...' to specify paths that do not exist locally.
grep: ovs/include/linux/netfilter/nf_conntrack_sctp.h: No such file or
directory
grep: ovs/include/linux/pkt_cls.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_pedit.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_skbedit.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_tunnel_key.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_vlan.h: No such file or directory
...

It is great that the duplicated tests problem is solved for "make check",
but a noise still shows up during "make check":

ldd: /home/hzhou/src/ovn/vswitchd/ovs-vswitchd: No such file or directory


For make sandbox. It works well except that some man pages don't work. For
example:

man ovn-nb/ovn-sb
man ovn-architecture

Also, most OVS related man pages doesn't work anymore, such as man
ovs-vsctl, which may be expected. However, some related to ovsdb still
works, such as:

man 7 ovsdb-server
man 5 ovsdb
man 7 ovsdb

This is a little confusing but I didn't dig further.
Numan Siddique Aug. 22, 2019, 6:57 a.m. UTC | #4
On Thu, Aug 22, 2019 at 2:19 AM Han Zhou <zhouhan@gmail.com> wrote:

>
>
> On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique <nusiddiq@redhat.com>
> wrote:
> >
> >
> >
> > On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan@gmail.com> wrote:
> >>
> >>
> >>
> >> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
> >> >
> >> > From: Numan Siddique <nusiddiq@redhat.com>
> >> >
> >> > This patch series adds support for building OVN from external OVS
> >> > sources.
> >> >
> >> > The first patch adds the support to compile OVN from external OVS
> sources.
> >> > The following configuration options are added when configuring OVN
> >> >   * --with-ovs-source (mandatory)
> >> >   * --with-ovs-build (optional)
> >> >
> >> > Patch 2 adds support to run OVN services using separate
> >> > directores
> >> >   - Default run time dir - /usr/local/var/run/ovm
> >> >   - Default log dir - /usr/loca/var/log/ovn
> >> >   - Default db dir - /usr/loca/etc/ovn
> >> >
> >> > Patch 3 fixes "make rpm-fedora" which is presently broken
> >> >
> >> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
> >> > used.
> >> >
> >> > Patch 5 removes the python subdirectory as that directory belongs
> >> > to OVS and uses the required files from the OVS repo.
> >> >
> >> > v1 -> v2
> >> > ========
> >> >  * Addressed the review comments.
> >> >  * Swapped the patch 1 and 2 as it was easier to address Mark's
> comment
> >> >    on OVS_RUNDIR/OVN_RUNDIR
> >> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few
> of
> >> >    the macros to OVS_* to OVN_*.
> >> >
> >> > Combined the patch 1 and 2 in this series which were submitted
> >> > separately earlier.
> >> >
> >> >
> >>
> >> Hi Numan,
> >>
> >> Thanks for this work. I tried applying this series on master, and then
> removed the "ovs" subfolder just to see if it uses the external OVS
> completely. However I encountered some error:
> >>
> >> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
> -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
> /home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
> /home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
> /home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
>  -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
> -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
> -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
> -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
> -Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
> lib/logical-fields.lo lib/logical-fields.c &&\
> >> mv -f $depbase.Tpo $depbase.Plo
> >> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
> >> lib/actions.c:2592:9: warning: implicit declaration of function
> 'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
> >>          ofpact_put_CHECK_PKT_LARGER(ofpacts);
> >>          ^
> >> lib/actions.c:2592:9: warning: initialization makes pointer from
> integer without a cast
> >> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
> >>      check_pkt_larger->pkt_len = cipl->pkt_len;
> >>                      ^
> >> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
> >>      check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
> >>                      ^
> >>
> >> Is this expected?
> >
> >
> > Hi Han,
> >
> > This is not expected. Before submitting I did test by removing the ovs
> subdirectory. I tried even now and I don't see any issue.
> > Looks to me either you have done "make install" in the ovs repo or your
>  ovs sources are a bit old.
> >
> > Can you please check if that's the case.
> >
> > Thanks
> > Numan
> >
> Thanks Numan. You are right that I was on an older branch of OVS. My bad.
> Now OVN build is successful after correcting the branch and rebuilding OVS.
>
> Just saw some annoying messages when doing make, but it seems no real
> impact there.
>
> comm: all-distfiles: No such file or directory
> grep: all-distfiles: No such file or directory
> fatal: ovs/lib/aes128.c: no such path in the working tree.
> Use 'git <command> -- <path>...' to specify paths that do not exist
> locally.
> fatal: ovs/include/linux/netfilter/nf_conntrack_sctp.h: no such path in
> the working tree.
> Use 'git <command> -- <path>...' to specify paths that do not exist
> locally.
> grep: ovs/include/linux/netfilter/nf_conntrack_sctp.h: No such file or
> directory
> grep: ovs/include/linux/pkt_cls.h: No such file or directory
> grep: ovs/include/linux/tc_act/tc_pedit.h: No such file or directory
> grep: ovs/include/linux/tc_act/tc_skbedit.h: No such file or directory
> grep: ovs/include/linux/tc_act/tc_tunnel_key.h: No such file or directory
> grep: ovs/include/linux/tc_act/tc_vlan.h: No such file or directory
> ...
>
> It is great that the duplicated tests problem is solved for "make check",
> but a noise still shows up during "make check":
>
> ldd: /home/hzhou/src/ovn/vswitchd/ovs-vswitchd: No such file or directory
>

I think we see this annoying message even without this patch series. And we
need to address that.


>
>
> For make sandbox. It works well except that some man pages don't work. For
> example:
>
> man ovn-nb/ovn-sb
> man ovn-architecture
>

Yes. This needs to be addressed. If you remember this commit
https://github.com/ovn-org/ovn/commit/5c5726243b34ceaa109cd1b2151997c119cd53c0
we added an entry in the TODO_Split.rst. It's in my TODO list and I will
work and submit  a follow up patch.


>
> Also, most OVS related man pages doesn't work anymore, such as man
> ovs-vsctl, which may be expected. However, some related to ovsdb still
> works, such as:
>
> man 7 ovsdb-server
> man 5 ovsdb
> man 7 ovsdb
>
> This is a little confusing but I didn't dig further.
>

Are you fine addressing this in the follow up patch ?

Thanks
Numan
Numan Siddique Aug. 22, 2019, 6:58 a.m. UTC | #5
On Thu, Aug 22, 2019 at 12:27 PM Numan Siddique <nusiddiq@redhat.com> wrote:

>
>
> On Thu, Aug 22, 2019 at 2:19 AM Han Zhou <zhouhan@gmail.com> wrote:
>
>>
>>
>> On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique <nusiddiq@redhat.com>
>> wrote:
>> >
>> >
>> >
>> > On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan@gmail.com> wrote:
>> >>
>> >>
>> >>
>> >> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
>> >> >
>> >> > From: Numan Siddique <nusiddiq@redhat.com>
>> >> >
>> >> > This patch series adds support for building OVN from external OVS
>> >> > sources.
>> >> >
>> >> > The first patch adds the support to compile OVN from external OVS
>> sources.
>> >> > The following configuration options are added when configuring OVN
>> >> >   * --with-ovs-source (mandatory)
>> >> >   * --with-ovs-build (optional)
>> >> >
>> >> > Patch 2 adds support to run OVN services using separate
>> >> > directores
>> >> >   - Default run time dir - /usr/local/var/run/ovm
>> >> >   - Default log dir - /usr/loca/var/log/ovn
>> >> >   - Default db dir - /usr/loca/etc/ovn
>> >> >
>> >> > Patch 3 fixes "make rpm-fedora" which is presently broken
>> >> >
>> >> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
>> >> > used.
>> >> >
>> >> > Patch 5 removes the python subdirectory as that directory belongs
>> >> > to OVS and uses the required files from the OVS repo.
>> >> >
>> >> > v1 -> v2
>> >> > ========
>> >> >  * Addressed the review comments.
>> >> >  * Swapped the patch 1 and 2 as it was easier to address Mark's
>> comment
>> >> >    on OVS_RUNDIR/OVN_RUNDIR
>> >> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed
>> few of
>> >> >    the macros to OVS_* to OVN_*.
>> >> >
>> >> > Combined the patch 1 and 2 in this series which were submitted
>> >> > separately earlier.
>> >> >
>> >> >
>> >>
>> >> Hi Numan,
>> >>
>> >> Thanks for this work. I tried applying this series on master, and then
>> removed the "ovs" subfolder just to see if it uses the external OVS
>> completely. However I encountered some error:
>> >>
>> >> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
>> -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
>> /home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
>> /home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
>> /home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
>>  -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
>> -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
>> -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
>> -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
>> -Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
>> lib/logical-fields.lo lib/logical-fields.c &&\
>> >> mv -f $depbase.Tpo $depbase.Plo
>> >> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
>> >> lib/actions.c:2592:9: warning: implicit declaration of function
>> 'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>> >>          ofpact_put_CHECK_PKT_LARGER(ofpacts);
>> >>          ^
>> >> lib/actions.c:2592:9: warning: initialization makes pointer from
>> integer without a cast
>> >> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
>> >>      check_pkt_larger->pkt_len = cipl->pkt_len;
>> >>                      ^
>> >> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
>> >>      check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
>> >>                      ^
>> >>
>> >> Is this expected?
>> >
>> >
>> > Hi Han,
>> >
>> > This is not expected. Before submitting I did test by removing the ovs
>> subdirectory. I tried even now and I don't see any issue.
>> > Looks to me either you have done "make install" in the ovs repo or your
>>  ovs sources are a bit old.
>> >
>> > Can you please check if that's the case.
>> >
>> > Thanks
>> > Numan
>> >
>> Thanks Numan. You are right that I was on an older branch of OVS. My bad.
>> Now OVN build is successful after correcting the branch and rebuilding OVS.
>>
>> Just saw some annoying messages when doing make, but it seems no real
>> impact there.
>>
>> comm: all-distfiles: No such file or directory
>> grep: all-distfiles: No such file or directory
>> fatal: ovs/lib/aes128.c: no such path in the working tree.
>> Use 'git <command> -- <path>...' to specify paths that do not exist
>> locally.
>> fatal: ovs/include/linux/netfilter/nf_conntrack_sctp.h: no such path in
>> the working tree.
>> Use 'git <command> -- <path>...' to specify paths that do not exist
>> locally.
>> grep: ovs/include/linux/netfilter/nf_conntrack_sctp.h: No such file or
>> directory
>> grep: ovs/include/linux/pkt_cls.h: No such file or directory
>> grep: ovs/include/linux/tc_act/tc_pedit.h: No such file or directory
>> grep: ovs/include/linux/tc_act/tc_skbedit.h: No such file or directory
>> grep: ovs/include/linux/tc_act/tc_tunnel_key.h: No such file or directory
>> grep: ovs/include/linux/tc_act/tc_vlan.h: No such file or directory
>> ...
>>
>
These messages should go away (or needs to be addressed) when we delete the
ovs subtree.


>
>> It is great that the duplicated tests problem is solved for "make check",
>> but a noise still shows up during "make check":
>>
>> ldd: /home/hzhou/src/ovn/vswitchd/ovs-vswitchd: No such file or directory
>>
>
> I think we see this annoying message even without this patch series. And
> we need to address that.
>
>
>>
>>
>> For make sandbox. It works well except that some man pages don't work.
>> For example:
>>
>> man ovn-nb/ovn-sb
>> man ovn-architecture
>>
>
> Yes. This needs to be addressed. If you remember this commit
> https://github.com/ovn-org/ovn/commit/5c5726243b34ceaa109cd1b2151997c119cd53c0
> we added an entry in the TODO_Split.rst. It's in my TODO list and I will
> work and submit  a follow up patch.
>
>
>>
>> Also, most OVS related man pages doesn't work anymore, such as man
>> ovs-vsctl, which may be expected. However, some related to ovsdb still
>> works, such as:
>>
>> man 7 ovsdb-server
>> man 5 ovsdb
>> man 7 ovsdb
>>
>> This is a little confusing but I didn't dig further.
>>
>
> Are you fine addressing this in the follow up patch ?
>
> Thanks
> Numan
>
>
Han Zhou Aug. 22, 2019, 4:47 p.m. UTC | #6
On Wed, Aug 21, 2019 at 11:59 PM Numan Siddique <nusiddiq@redhat.com> wrote:
>
>
>
> On Thu, Aug 22, 2019 at 12:27 PM Numan Siddique <nusiddiq@redhat.com>
wrote:
>>
>>
>>
>> On Thu, Aug 22, 2019 at 2:19 AM Han Zhou <zhouhan@gmail.com> wrote:
>>>
>>>
>>>
>>> On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique <nusiddiq@redhat.com>
wrote:
>>> >
>>> >
>>> >
>>> > On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan@gmail.com> wrote:
>>> >>
>>> >>
>>> >>
>>> >> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq@redhat.com> wrote:
>>> >> >
>>> >> > From: Numan Siddique <nusiddiq@redhat.com>
>>> >> >
>>> >> > This patch series adds support for building OVN from external OVS
>>> >> > sources.
>>> >> >
>>> >> > The first patch adds the support to compile OVN from external OVS
sources.
>>> >> > The following configuration options are added when configuring OVN
>>> >> >   * --with-ovs-source (mandatory)
>>> >> >   * --with-ovs-build (optional)
>>> >> >
>>> >> > Patch 2 adds support to run OVN services using separate
>>> >> > directores
>>> >> >   - Default run time dir - /usr/local/var/run/ovm
>>> >> >   - Default log dir - /usr/loca/var/log/ovn
>>> >> >   - Default db dir - /usr/loca/etc/ovn
>>> >> >
>>> >> > Patch 3 fixes "make rpm-fedora" which is presently broken
>>> >> >
>>> >> > Patch 4 runs OVN services as openvswitch user for rhel when rpms
are
>>> >> > used.
>>> >> >
>>> >> > Patch 5 removes the python subdirectory as that directory belongs
>>> >> > to OVS and uses the required files from the OVS repo.
>>> >> >
>>> >> > v1 -> v2
>>> >> > ========
>>> >> >  * Addressed the review comments.
>>> >> >  * Swapped the patch 1 and 2 as it was easier to address Mark's
comment
>>> >> >    on OVS_RUNDIR/OVN_RUNDIR
>>> >> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed
few of
>>> >> >    the macros to OVS_* to OVN_*.
>>> >> >
>>> >> > Combined the patch 1 and 2 in this series which were submitted
>>> >> > separately earlier.
>>> >> >
>>> >> >
>>> >>
>>> >> Hi Numan,
>>> >>
>>> >> Thanks for this work. I tried applying this series on master, and
then removed the "ovs" subfolder just to see if it uses the external OVS
completely. However I encountered some error:
>>> >>
>>> >> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
-DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
/home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
/home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
/home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wshadow    -g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
lib/logical-fields.lo lib/logical-fields.c &&\
>>> >> mv -f $depbase.Tpo $depbase.Plo
>>> >> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
>>> >> lib/actions.c:2592:9: warning: implicit declaration of function
'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>>> >>          ofpact_put_CHECK_PKT_LARGER(ofpacts);
>>> >>          ^
>>> >> lib/actions.c:2592:9: warning: initialization makes pointer from
integer without a cast
>>> >> lib/actions.c:2593:21: error: dereferencing pointer to incomplete
type
>>> >>      check_pkt_larger->pkt_len = cipl->pkt_len;
>>> >>                      ^
>>> >> lib/actions.c:2594:21: error: dereferencing pointer to incomplete
type
>>> >>      check_pkt_larger->dst = expr_resolve_field(&cipl->dst);
>>> >>                      ^
>>> >>
>>> >> Is this expected?
>>> >
>>> >
>>> > Hi Han,
>>> >
>>> > This is not expected. Before submitting I did test by removing the
ovs subdirectory. I tried even now and I don't see any issue.
>>> > Looks to me either you have done "make install" in the ovs repo or
your  ovs sources are a bit old.
>>> >
>>> > Can you please check if that's the case.
>>> >
>>> > Thanks
>>> > Numan
>>> >
>>> Thanks Numan. You are right that I was on an older branch of OVS. My
bad. Now OVN build is successful after correcting the branch and rebuilding
OVS.
>>>
>>> Just saw some annoying messages when doing make, but it seems no real
impact there.
>>>
>>> comm: all-distfiles: No such file or directory
>>> grep: all-distfiles: No such file or directory
>>> fatal: ovs/lib/aes128.c: no such path in the working tree.
>>> Use 'git <command> -- <path>...' to specify paths that do not exist
locally.
>>> fatal: ovs/include/linux/netfilter/nf_conntrack_sctp.h: no such path in
the working tree.
>>> Use 'git <command> -- <path>...' to specify paths that do not exist
locally.
>>> grep: ovs/include/linux/netfilter/nf_conntrack_sctp.h: No such file or
directory
>>> grep: ovs/include/linux/pkt_cls.h: No such file or directory
>>> grep: ovs/include/linux/tc_act/tc_pedit.h: No such file or directory
>>> grep: ovs/include/linux/tc_act/tc_skbedit.h: No such file or directory
>>> grep: ovs/include/linux/tc_act/tc_tunnel_key.h: No such file or
directory
>>> grep: ovs/include/linux/tc_act/tc_vlan.h: No such file or directory
>>> ...
>
>
> These messages should go away (or needs to be addressed) when we delete
the ovs subtree.
>
>>>
>>>
>>> It is great that the duplicated tests problem is solved for "make
check", but a noise still shows up during "make check":
>>>
>>> ldd: /home/hzhou/src/ovn/vswitchd/ovs-vswitchd: No such file or
directory
>>
>>
>> I think we see this annoying message even without this patch series. And
we need to address that.
>>
>>>
>>>
>>>
>>> For make sandbox. It works well except that some man pages don't work.
For example:
>>>
>>> man ovn-nb/ovn-sb
>>> man ovn-architecture
>>
>>
>> Yes. This needs to be addressed. If you remember this commit
https://github.com/ovn-org/ovn/commit/5c5726243b34ceaa109cd1b2151997c119cd53c0
>> we added an entry in the TODO_Split.rst. It's in my TODO list and I will
work and submit  a follow up patch.
>>
>>>
>>>
>>> Also, most OVS related man pages doesn't work anymore, such as man
ovs-vsctl, which may be expected. However, some related to ovsdb still
works, such as:
>>>
>>> man 7 ovsdb-server
>>> man 5 ovsdb
>>> man 7 ovsdb
>>>
>>> This is a little confusing but I didn't dig further.
>>
>>
>> Are you fine addressing this in the follow up patch ?
>>

Yes, I am fine with it. Thanks for explain.