[ovs-dev] OVS / OVN split - post 2.12

Ilya Maximets i.maximets at samsung.com
Fri Jul 26 09:52:43 UTC 2019


On 26.07.2019 12:49, Numan Siddique wrote:
> 
> 
> On Fri, Jul 26, 2019 at 2:00 PM Dumitru Ceara <dceara at redhat.com <mailto:dceara at redhat.com>> wrote:
> 
>     On Fri, Jul 26, 2019 at 8:21 AM Numan Siddique <nusiddiq at redhat.com <mailto:nusiddiq at redhat.com>> wrote:
>     >
>     > Hello All,
>     >
>     > The split work is almost done and we have pushed it to the ovn repo branch
>     > for now - https://github.com/ovn-org/ovn/commits/ovs_ovn_split
>     > If some one is interested, please try it out and let us know any comments
>     > or issues.
> 
>     Hi Numan,
> 
>     It looks like the ovs_ovn_split branch doesn't build:
> 
>     # git clone https://github.com/ovn-org/ovn.git ovn
>     # cd ovn
>     # ./boot.sh
>     # ./configure --enable-Werror --enable-sparse
>     # make
>     make  all-recursive
>     make[1]: Entering directory `/root/ovn'
>     Making all in ovs
>     make[2]: Entering directory `/root/ovn/ovs'
>     make  all-recursive
>     make[3]: Entering directory `/root/ovn/ovs'
>     Making all in datapath
>     make[4]: Entering directory `/root/ovn/ovs/datapath'
>     make[5]: Entering directory `/root/ovn/ovs/datapath'
>     make[5]: Leaving directory `/root/ovn/ovs/datapath'
>     make[4]: Leaving directory `/root/ovn/ovs/datapath'
>     make[4]: Entering directory `/root/ovn/ovs'
>     make[4]: Leaving directory `/root/ovn/ovs'
>     make[3]: Leaving directory `/root/ovn/ovs'
>     make[2]: Leaving directory `/root/ovn/ovs'
>     make[2]: Entering directory `/root/ovn'
>     depbase=`echo lib/acl-log.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
>     /bin/sh ./libtool  --tag=CC   --mode=compile env REAL_CC="gcc
>     -std=gnu99" CHECK="sparse -Wsparse-error -I ./include/sparse -m64 -I
>     /usr/local/include  " cgcc -target=x86_64 -DHAVE_CONFIG_H -I.    -I
>     ./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I ./ovs -I
>     ./ovs -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 -Werror
>     -Werror   -g -O2 -MT lib/acl-log.lo -MD -MP -MF $depbase.Tpo -c -o
>     lib/acl-log.lo lib/acl-log.c &&\
>     mv -f $depbase.Tpo $depbase.Plo
>     libtool: compile:  env "REAL_CC=gcc -std=gnu99" "CHECK=sparse
>     -Wsparse-error -I ./include/sparse -m64 -I /usr/local/include  " cgcc
>     -target=x86_64 -DHAVE_CONFIG_H -I. -I ./ovs/include -I ./ovs/include
>     -I ./ovs/lib -I ./ovs/lib -I ./ovs -I ./ovs -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 -Werror
>     -Werror -g -O2 -MT lib/acl-log.lo -MD -MP -MF lib/.deps/acl-log.Tpo -c
>     lib/acl-log.c -o lib/acl-log.o
>     ./ovs/include/openvswitch/nsh.h:372:11: error: symbol '__v' shadows an
>     earlier one
>     ./ovs/include/openvswitch/nsh.h:372:11: originally declared here
>     ./ovs/include/openvswitch/nsh.h:372:11: error: symbol '__x' shadows an
>     earlier one
>     ./ovs/include/openvswitch/nsh.h:372:11: originally declared here
>     ./ovs/include/openvswitch/nsh.h:383:11: error: symbol '__v' shadows an
>     earlier one
>     ./ovs/include/openvswitch/nsh.h:383:11: originally declared here
>     ./ovs/include/openvswitch/nsh.h:383:11: error: symbol '__x' shadows an
>     earlier one
>     ./ovs/include/openvswitch/nsh.h:383:11: originally declared here
>     make[2]: *** [lib/acl-log.lo] Error 1
> 
>     Building ovs master works fine for me:
> 
>     # git clone https://github.com/openvswitch/ovs.git ovs
>     # cd ovs
>     # ./boot.sh
>     # ./configure --enable-Werror --enable-sparse
>     # make
>     <snip>
>     mv tests/system-afxdp-testsuite.tmp tests/system-afxdp-testsuite
>     mv tests/system-kmod-testsuite.tmp tests/system-kmod-testsuite
>     mv tests/system-userspace-testsuite.tmp tests/system-userspace-testsuite
>     touch -c manpage-check
>     mv tests/testsuite.tmp tests/testsuite
>     make[2]: Leaving directory `/root/ovs'
>     make[1]: Leaving directory `/root/ovs'
> 
> 
> Thanks Dumitru for trying it out.
> 
> I tried compiling the present ovn-org/ovn master  branch with --enable-sparse and I see below error
> 
> ***********
> trict-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 -Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow -Wmultistatement-macros -Wcast-align=strict -Werror -Werror -g -O2 -MT ofproto/libofproto_la-ofproto-dpif-ipfix.lo -MD -MP -MF ofproto/.deps/libofproto_la-ofproto-dpif-ipfix.Tpo -c ../../ovs/ofproto/ofproto-dpif-ipfix.c -o ofproto/libofproto_la-ofproto-dpif-ipfix.o
> ../../ovs/lib/bitmap.h:64:29: error: shift too big (64) for type unsigned long
> make[4]: *** [Makefile:5611: ofproto/libofproto_la-ofproto-dpif.lo] Error 1
> make[4]: *** Waiting for unfinished jobs....
> mv -f ofproto/.deps/libofproto_la-ofproto-dpif-mirror.Tpo ofproto/.deps/libofproto_la-ofproto-dpif-mirror.Plo
> mv -f ofproto/.deps/libofproto_la-ofproto-dpif-monitor.Tpo ofproto/.deps/libofproto_la-ofproto-dpif-monitor.Plo
> mv -f ofproto/.deps/libofproto_la-ofproto-dpif-rid.Tpo ofproto/.deps/libofproto_la-ofproto-dpif-rid.Plo
> mv -f ofproto/.deps/libofproto_la-ofproto-dpif-ipfix.Tpo ofproto/.deps/libofproto_la-ofproto-dpif-ipfix.Plo
> ***************

This one is a false-positive that was fixed by:

commit 46124c18b093bee851f30c9fc938c1ff8c2c75ee
Author: Ilya Maximets <i.maximets at samsung.com>
Date:   Wed Apr 24 16:00:22 2019 +0300

    travis: Fix checks skipping by sparse.
    
    Recent commit in "sparse" broke checking the OVS sources, because
    'make' uses '-MD' flag to generate dependencies as a side effect
    within compilation commands, but "sparse" skips all the build commands
    that contains '-MD' and friends.
    Let's revert the bad commit as a workaround before installing "sparse"
    in TravisCI.
    
    Additionally fixed a false-positive:
    ./lib/bitmap.h:64:29: error: shift too big (64) for type unsigned long
    
    CC: Yi-Hung Wei <yihung.wei at gmail.com>
    Fixes: 879e8238dfdf ("travis: Update sparse git repo")
    Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
    Signed-off-by: Ben Pfaff <blp at ovn.org>


> 
> And with the split branch I am seeing below errors when compiled with --enable-sparse.
> 
> *******
> ib -I ./ovs/lib -I ../ovs -I ./ovs -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 -Wswitch-bool -Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare -Wshift-negative-value -Wduplicated-cond -Wshadow -Wmultistatement-macros -Wcast-align=strict -Werror -Werror   -g -O2 -MT lib/actions.lo -MD -MP -MF $depbase.Tpo -c -o lib/actions.lo ../lib/actions.c &&\
> mv -f $depbase.Tpo $depbase.Plo
> ../ovs/lib/util.h:449:12: error: incorrect type in return expression (different base types)
> ../ovs/lib/util.h:449:12:    expected restricted ovs_be32
> ../ovs/lib/util.h:449:12:    got unsigned int
> ../utilities/ovn-nbctl.c:3426:34: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3426:34:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3426:34:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3426:34: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3426:34:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3426:34:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3426:34: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3426:34:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3426:34:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3426:34: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3426:34:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3426:34:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3428:39: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3428:39:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3428:39:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3428:39: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3428:39:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3428:39:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3428:39: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3428:39:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3428:39:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:3428:39: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:3428:39:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:3428:39:    got restricted ovs_be32 [usertype] network
> ../utilities/ovn-nbctl.c:4610:16: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:4610:16:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:4610:16:    got restricted ovs_be32 const [usertype] addr
> ../utilities/ovn-nbctl.c:4610:39: error: incorrect type in argument 1 (different base types)
> ../utilities/ovn-nbctl.c:4610:39:    expected unsigned int [usertype] __bsx
> ../utilities/ovn-nbctl.c:4610:39:    got restricted ovs_be32 const [usertype] addr
> make[2]: *** [Makefile:2442: utilities/ovn-nbctl.o] Error 1
> ***************
> 
> I am working on compiling OVN with external OVS sources. I will see if I get the same errors there.
> 
> Thanks
> Numan
> 
> 
> 
>     Regards,
>     Dumitru
> 
>     >
>     > We are planning to push the changes to the master branch hopefully Friday
>     > if everything goes fine.
>     >
>     > Thanks
>     > Numan
>     >
>     >
>     > On Fri, Jul 26, 2019 at 12:05 AM Mark Michelson <mmichels at redhat.com <mailto:mmichels at redhat.com>> wrote:
>     >
>     > > On 7/25/19 12:37 PM, Ilya Maximets wrote:
>     > > > Hi.
>     > > >
>     > > > I have a question regarding the split.
>     > > > Sorry if it's obvious from the RFC splitting code which I didn't look at.
>     > > >
>     > > > The question is:
>     > > > How the patches for the common data structures (lists, hash maps) will be
>     > > > applied to OVN? Will we need to post same patch for both repositories in
>     > > the
>     > > > future if it changes/fixes common libraries? Or all this code will be
>     > > hosted
>     > > > only in OVS repo and will be included like a submodule or something like
>     > > that?
>     > > >
>     > > > Best regards, Ilya Maximets.
>     > >
>     > > Hi Ilya,
>     > >
>     > > OVN will be a separate repo from OVS. Initially, OVS will be a subtree
>     > > of OVN, but soon, we will want to be able to build OVN while having a
>     > > completely separate OVS on the system. When all is said and done, it
>     > > should be possible to build OVN as long as you have, say, an ovs-devel
>     > > package installed on your system.
>     > >
>     > > I think the answer to your question is that as an OVS developer, if you
>     > > make a change to a core component, you only need to post the change to
>     > > the OVS repo.
>     > >
>     > >  From an OVN developer's perspective, if an OVN feature/bugfix requires
>     > > a parallel change to be made to a core OVS library, then the OVN
>     > > developer will need to post the library change to OVS as well as the
>     > > usage of that change to OVN. As long as OVS is a subtree of OVN, it's
>     > > just a matter of ensuring the subtree is kept up to date. However, once
>     > > OVS is built separately, then configure-time checks of OVS features or
>     > > minimum version checks will need to be added to OVN to ensure that the
>     > > code can build properly.
>     > >
>     > > This approach should place most of the burden of OVN compatibility with
>     > > OVS on OVN developers rather than OVS developers.
>     > > >
>     > > >> Numan and I have discussed this, and we're planning to perform the split
>     > > >> tomorrow July 25. Numan will be applying the commits he referenced below
>     > > >> to the ovn-org branch to get it up to date. I will be submitting a
>     > > >> review for OVS to remove all of the OVN code, documentation, etc. from
>     > > it.
>     > > >>
>     > > >> If there are any reasons why we should hold off on this, please mention
>     > > >> them as soon as possible.
>     > > >>
>     > > >> Thanks
>     > > >>
>     > > >> On 7/22/19 2:35 PM, Numan Siddique wrote:
>     > > >>> Hi Ben, Mark and All,
>     > > >>>
>     > > >>> Now that branch 2.12 is created, shall we proceed with the OVS/OVN
>     > > split ?
>     > > >>>
>     > > >>> In order to do the split we need to do the below tasks
>     > > >>>
>     > > >>> In ovn-org/ovn repo
>     > > >>> Step 1. Sync the ovs subtree to the latest (from the OVS repo).
>     > > >>> 2. Delete all the ovn related code from the root dir. Right now there
>     > > is
>     > > >>> no history for the OVN files in the ovn-org/ovn repo
>     > > >>> 3. Copy OVN files from openvswitch/ovs repo using git-filter-branch.
>     > > >>> This will preserver the history.
>     > > >>> 4. Sync the test files from ovs subtree so that tests pass.
>     > > >>>
>     > > >>> During this period its better to freeze merging OVN related patches in
>     > > >>> the OVS repo.
>     > > >>> And finally delete the OVN related code from the OVS repo.
>     > > >>>
>     > > >>> I have done a PoC here -
>     > > >>> https://github.com/numansiddique/ovn/commits/ovn_sync_from_ovs_v3/p4
>     > > >>> All the relates commits can be found here.
>     > > >>>
>     > > >>>
>     > > >>> Does these steps seem fine ? Any concerns ?
>     > > >>> If this seems fine, can we choose a date to start this process ?
>     > > >>>
>     > > >>> Thanks
>     > > >>> Numan
>     > > >>>
>     > > >
>     > >
>     > >
>     > _______________________________________________
>     > dev mailing list
>     > dev at openvswitch.org <mailto:dev at openvswitch.org>
>     > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 


More information about the dev mailing list