[ovs-dev] [PATCH ovn v9 7/7] ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.

[Dumitru Ceara]

On 12/2/20 7:50 AM, Ben Pfaff wrote:
> From: Leonid Ryzhyk <lryzhyk at vmware.com>
> 
> This implementation is incremental, meaning that it only recalculates
> what is needed for the southbound database when northbound changes
> occur.  It is expected to scale better than the C implementation,
> for large deployments.  (This may take testing and tuning to be
> effective.)
> 
> There are three tests that I'm having mysterious trouble getting
> to work with DDlog.  For now, I've marked the testsuite to skip
> them unless RUN_ANYWAY=yes is set in the environment.
> 
> Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
> Co-authored-by: Justin Pettit <jpettit at ovn.org>
> Signed-off-by: Justin Pettit <jpettit at ovn.org>
> Co-authored-by: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---

Hi Ben,

This is not a full review, just some initial comments.

There are a few tests that fail (also northd-C tests) and I suspect it's
because the OVS IDL split patches [0] this commit depends on don't
include some of the recent fixes that went into the OVS IDL code, at
least the following:
- https://github.com/openvswitch/ovs/commit/97dbef6
- https://github.com/openvswitch/ovs/commit/91a6a45
- https://github.com/openvswitch/ovs/commit/08e130a
- https://github.com/openvswitch/ovs/commit/f0d23f6

[0] https://patchwork.ozlabs.org/project/openvswitch/list/?series=217954

Also, I'm still seeing the libopenvswitchavx512.la related error we
reported earlier:

error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___
--print=file-names -L ../../lib/.libs -L /root/ovs/lib/.libs -lssl
-lcrypto -lcap-ng /root/ovs/lib/libopenvswitchavx512.la -lpthread -lrt
-lm -lunbound -lpthread -lrt -lm -lunbound -Awarnings --crate-type bin
--crate-type rlib --crate-type dylib --crate-type cdylib --crate-type
staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit
code: 1)
  --- stderr
  error: multiple input filenames provided (first two filenames are `-`
and `/root/ovs/lib/libopenvswitchavx512.la`)

I know you didn't hit this on your machines, but please let me know what
additional info you would require.

[...]

>  
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 6e396895826f..2e4eaf7f12f7 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -16982,6 +16982,10 @@ AT_CLEANUP
>  
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([ovn -- IGMP snoop/querier/relay])
> +
> +dnl This test has problems with ovn-northd-ddlog.
> +AT_SKIP_IF([test NORTHD_TYPE = ovn-northd-ddlog && test "$RUN_ANYWAY" != yes])

I might be wrong but I think the ddlog part still misses the logic from
this commit from the ovn-northd C implementation:

https://github.com/ovn-org/ovn/commit/97778ab3e422ac071faa67f9f477fd54977e9c04

Thanks,
Dumitru