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

Han Zhou zhouhan at gmail.com
Thu Aug 22 16:47:27 UTC 2019


On Wed, Aug 21, 2019 at 11:59 PM Numan Siddique <nusiddiq at redhat.com> wrote:
>
>
>
> On Thu, Aug 22, 2019 at 12:27 PM Numan Siddique <nusiddiq at redhat.com>
wrote:
>>
>>
>>
>> On Thu, Aug 22, 2019 at 2:19 AM Han Zhou <zhouhan at gmail.com> wrote:
>>>
>>>
>>>
>>> On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique <nusiddiq at redhat.com>
wrote:
>>> >
>>> >
>>> >
>>> > On Wed, Aug 21, 2019 at 10:30 PM Han Zhou <zhouhan at gmail.com> wrote:
>>> >>
>>> >>
>>> >>
>>> >> On Mon, Aug 19, 2019 at 11:13 AM <nusiddiq at redhat.com> wrote:
>>> >> >
>>> >> > From: Numan Siddique <nusiddiq at 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.


More information about the dev mailing list