Re: [ovs-dev] [PATCH ovn v2] Include OVS as a git submodule.
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line has non-spaces leading whitespace #108 FILE: .gitmodules:2: path = ovs WARNING: Line has non-spaces leading whitespace #109 FILE: .gitmodules:3: url = https://github.com/openvswitch/ovs.git Lines checked: 301, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] 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. Signed-off-by: Mark Michelson --- v1 -> v2: * Updated CI scripts to use the submodule. * Added AM_DISTCHECK_CONFIGURE_FLAGS setting so `make distcheck` succeeds. * Added an extra 'xargs' call to submodule discovery in Makefile.am since for some reason on OSX, there was extra whitespace around the discovered "ovs" submodule. xargs is a simple way to strip that away. * Updated documentation to more accurately reflect the necessary steps to use the OVS submodule. Also changed wording to indicate the submodule is the minimum recommended version, rather than the minimum required version of OVS. --- .ci/linux-build.sh | 8 +++--- .ci/osx-build.sh| 8 +++--- .gitmodules | 3 +++ Documentation/intro/install/general.rst | 36 +++-- Makefile.am | 23 +--- acinclude.m4| 2 +- build-aux/initial-tab-whitelist | 1 + ovs | 1 + 8 files changed, 55 insertions(+), 27 deletions(-) create mode 100644 .gitmodules create mode 16 ovs diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 2a711a1b9..af81de920 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -10,8 +10,8 @@ EXTRA_OPTS="--enable-Werror" function configure_ovs() { -git clone https://github.com/openvswitch/ovs.git ovs_src -pushd ovs_src +git submodule update --init +pushd ovs ./boot.sh && ./configure $* || { cat config.log; exit 1; } make -j4 || { cat config.log; exit 1; } popd @@ -22,7 +22,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 +54,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 index 6617f0b9d..423d82c1d 100755 --- a/.ci/osx-build.sh +++ b/.ci/osx-build.sh @@ -7,8 +7,8 @@ EXTRA_OPTS="" function configure_ovs() { -git clone https://github.com/openvswitch/ovs.git ovs_src -pushd ovs_src +git submodule update --init +pushd ovs ./boot.sh && ./configure $* make -j4 || { cat config.log; exit 1; } popd @@ -17,7 +17,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 +32,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/.gitmodules b/.gitmodules new file mode 100644 index 0..f0d1f8cbe --- /dev/null +++ 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 index 65b1f4a40..e077bd8fb 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -66,6 +66,9 @@ 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 version required in order for OVN to compile. See + below for instructions about how to use a different OVS source location. - GNU make @@ -140,27 +143,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
Re: [ovs-dev] [PATCH ovs v2] conntrack: Fix the icmp conntrack new state.
xiangxia.m@gmail.com writes: > From: Tonghao Zhang > > The same icmp packet may traverse conntrack module more than once. > Or same icmp packets traverse contranck module in orderly. > > Don't change state to CS_ESTABLISHED before receiving reply or related > packets. > > Fixes: a867c010ee91 ("conntrack: Fix conntrack new state") > Signed-off-by: Tonghao Zhang > --- Thanks for the v2. Acked-by: Aaron Conole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] github: Run clang test with AddressSanitizer enabled.
This commit is based on a similar one from OVN by Dumitru Ceara: a429b24f7bf5 ("ci: Enable AddressSanitizer in Linux clang CI test runs.") It's useful to run testsuite with address sanitizer enabled to catch memory leaks and invalid memory accesses. Skipping re-check if AddressSanitizer reports are present in the test run directory to not lose them. Right now OVS has no memory leaks detected on a testsuite run with -O1. With -O2 there are few false-positive leak reports in test-ovsdb application, so not using this optimization level for now. For the same reason not enabling leak detection by default for everyone. Enabled only in CI. AddressSanitizer increases execution time for this job from ~12 to ~16 minutes, but it looks like a reasonable sacrifice. Signed-off-by: Ilya Maximets --- .ci/linux-build.sh | 9 + .github/workflows/build-and-test.yml | 2 ++ tests/automake.mk| 5 - 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 3e5136fd4..581ab 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -226,6 +226,15 @@ elif [ "$TRAVIS_ARCH" != "aarch64" ]; then CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}" fi +if [ "$ASAN" ]; then +# This will override default option configured in tests/atlocal.in. +export ASAN_OPTIONS='detect_leaks=1' +# -O2 generates few false-positive memory leak reports in test-ovsdb +# application, so lowering optimizations to -O1 here. +CLFAGS_ASAN="-O1 -fno-omit-frame-pointer -fno-common -fsanitize=address" +CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CLFAGS_ASAN}" +fi + save_OPTS="${OPTS} $*" OPTS="${EXTRA_OPTS} ${save_OPTS}" diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index b29c300c5..e24970505 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -14,6 +14,7 @@ jobs: deb_dependencies: | linux-headers-$(uname -r) build-essential fakeroot devscripts equivs AFXDP: ${{ matrix.afxdp }} + ASAN:${{ matrix.asan }} CC: ${{ matrix.compiler }} DEB_PACKAGE: ${{ matrix.deb_package }} DPDK:${{ matrix.dpdk }} @@ -44,6 +45,7 @@ jobs: - compiler: clang testsuite:test kernel: 3.16 +asan: asan - compiler: gcc testsuite:test diff --git a/tests/automake.mk b/tests/automake.mk index 677b99a6b..dfec2ea10 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -189,6 +189,7 @@ check_SCRIPTS += tests/atlocal TESTSUITE = $(srcdir)/tests/testsuite TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch +TESTSUITE_DIR = $(abs_top_builddir)/tests/testsuite.dir SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite SYSTEM_TSO_TESTSUITE = $(srcdir)/tests/system-tso-testsuite @@ -202,7 +203,9 @@ AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(S check-local: set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH); \ - "$$@" $(TESTSUITEFLAGS) || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) + "$$@" $(TESTSUITEFLAGS) || \ + (test -z "$$(find $(TESTSUITE_DIR) -name 'asan.*')" && \ +test X'$(RECHECK)' = Xyes && "$$@" --recheck) # Python Coverage support. # Requires coverage.py http://nedbatchelder.com/code/coverage/. -- 2.26.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] northd: add --event option to enable controller_event for empty_lb
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 207 characters long (recommended limit is 79) #79 FILE: utilities/ovn-nbctl.8.xml:908: [--may-exist | --add-duplicate | --reject | --event] lb-add lb vip ips [protocol] Lines checked: 138, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] northd: add --event option to enable controller_event for empty_lb
Introduce the --event option to enable empty_lb controller event for a load_balancer with no backends (doing so the option is per-lb and not global). $ovn-nbctl --event lb-add lb0 192.168.0.100:80 "" controller_event_en global variable is not removed for backward compatibility but it is deprecated Signed-off-by: Lorenzo Bianconi --- northd/ovn-northd.c | 5 - tests/ovn.at | 7 +++ utilities/ovn-nbctl.8.xml | 10 +- utilities/ovn-nbctl.c | 13 - 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 27df6a379..e92c75726 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4860,7 +4860,9 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows, struct nbrec_load_balancer *lb, int pl, struct shash *meter_groups) { -if (!controller_event_en || lb_vip->n_backends || +bool controller_event = smap_get_bool(>options, "event", false) || +controller_event_en; /* deprecated */ +if (!controller_event || lb_vip->n_backends || lb_vip->empty_backend_rej) { return; } @@ -12611,6 +12613,7 @@ ovnnb_db_run(struct northd_context *ctx, use_logical_dp_groups = smap_get_bool(>options, "use_logical_dp_groups", false); +/* deprecated, use --event instead */ controller_event_en = smap_get_bool(>options, "controller_event", false); check_lsp_is_up = !smap_get_bool(>options, diff --git a/tests/ovn.at b/tests/ovn.at index 8f884241d..5bc8331b1 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -16947,16 +16947,15 @@ ovs-vsctl -- add-port br-int vif33 -- \ options:rxq_pcap=hv$i/vif33-rx.pcap \ ofport-request=33 -ovn-nbctl --wait=hv set NB_Global . options:controller_event=true -ovn-nbctl lb-add lb0 192.168.1.100:80 "" +ovn-nbctl --event lb-add lb0 192.168.1.100:80 "" ovn-nbctl ls-lb-add sw0 lb0 uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0) -ovn-nbctl lb-add lb1 192.168.2.100:80 "" +ovn-nbctl --event lb-add lb1 192.168.2.100:80 "" ovn-nbctl lr-lb-add lr0 lb1 uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1) -ovn-nbctl lb-add lb2 [[2001::10]]:50051 "" +ovn-nbctl --event lb-add lb2 [[2001::10]]:50051 "" ovn-nbctl ls-lb-add sw0 lb2 uuid_lb2=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb2) diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index e6fec9980..6ed8bcb75 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -905,7 +905,7 @@ Load Balancer Commands -[--may-exist | --add-duplicate | --reject] lb-add lb vip ips [protocol] +[--may-exist | --add-duplicate | --reject | --event] lb-add lb vip ips [protocol] Creates a new load balancer named lb with the provided @@ -947,6 +947,14 @@ empty_lb SB controller event for this load balancer. + + If the load balancer is created with --event option and + it has no active backends, whenever the lb receives traffic, the event + is reported in the Controller_Event table in the SB db. + Please note --event option can't be specified with + --reject one. + + The following example adds a load balancer. diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 2342ead4e..6eaae0866 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -2836,6 +2836,13 @@ nbctl_lb_add(struct ctl_context *ctx) bool may_exist = shash_find(>options, "--may-exist") != NULL; bool add_duplicate = shash_find(>options, "--add-duplicate") != NULL; bool empty_backend_rej = shash_find(>options, "--reject") != NULL; +bool empty_backend_event = shash_find(>options, "--event") != NULL; + +if (empty_backend_event && empty_backend_rej) { +ctl_error(ctx, + "--reject and --event can't specified at the same time"); +return; +} const char *lb_proto; bool is_update_proto = false; @@ -2953,6 +2960,10 @@ nbctl_lb_add(struct ctl_context *ctx) const struct smap options = SMAP_CONST1(, "reject", "true"); nbrec_load_balancer_set_options(lb, ); } +if (empty_backend_event) { +const struct smap options = SMAP_CONST1(, "event", "true"); +nbrec_load_balancer_set_options(lb, ); +} out: ds_destroy(_ips_new); @@ -6517,7 +6528,7 @@ static const struct ctl_command_syntax nbctl_commands[] = { nbctl_lr_nat_set_ext_ips, NULL, "--is-exempted", RW}, /* load balancer commands. */ { "lb-add", 3, 4, "LB VIP[:PORT] IP[:PORT]... [PROTOCOL]", NULL, - nbctl_lb_add, NULL, "--may-exist,--add-duplicate,--reject", RW }, + nbctl_lb_add,
[ovs-dev] [PATCH] rhel: Update for DPDK 20.11
With DPDK 20.11, meson and pkgconfig are used instead of the old Makefile-based system and so --with-dpdk option is changed to only accept shared or static instead of the directory. This commit uses --with-dpdk=shared since Fedora and RHEL ship shared libraries of DPDK. Fixes: 252e1e576443 ("dpdk: Update to use DPDK v20.11.") Signed-off-by: Timothy Redaelli --- rhel/openvswitch-fedora.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 2c0c4fa18..21929e6cf 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -162,7 +162,7 @@ This package provides IPsec tunneling support for OVS tunnels. --disable-libcapng \ %endif %if %{with dpdk} ---with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \ +--with-dpdk=shared \ %endif --enable-ssl \ --disable-static \ -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] tests: Fix L2 ACL test.
The ACLs configured by the "ovn -- L2 Drop and Allow ACL w/ Stateful ACL" test were incorrect because they didn't enclose logical port names in quotes. This caused stateful ACLs to never be applied so the L2 drop rules were not properly tested. ovn-controller was logging the following errors: lflow|WARN|error parsing match "reg0[8] == 1 && (inport == lp31 && ip)": Syntax error at `lp31' expecting constant. lflow|WARN|error parsing match "reg0[8] == 1 && (inport == lp31 && ip)": Syntax error at `lp31' expecting constant. lflow|WARN|error parsing match "reg0[8] == 1 && (inport == lp31 && ip)": Syntax error at `lp31' expecting constant. Fixes: 63640c0d1199 ("ovn-northd: ls_*_acl behavior not consistent for untracked flows") Signed-off-by: Dumitru Ceara --- tests/ovn.at | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index 8f88424..718b2ee 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14096,7 +14096,7 @@ for sf in 0 1; do if test ${sf} = 1; then # Add a stateful rule and re-run the check to make sure the # drop rule is still effective.. -ovn-nbctl --wait=hv acl-add lsw0 from-lport 2000 "inport == lp31 && ip" allow-related +ovn-nbctl --wait=hv acl-add lsw0 from-lport 2000 'inport == "lp31" && ip' allow-related fi for is in 1 2 3; do s=${is}1 @@ -14135,7 +14135,7 @@ for sf in 0 1; do if test ${sf} = 1; then # Add a stateful rule and re-run the check to make sure the # allow rule is still effective.. -check ovn-nbctl --wait=hv acl-add lsw0 from-lport 2000 "inport == lp31 && ip" allow-related +check ovn-nbctl --wait=hv acl-add lsw0 from-lport 2000 'inport == "lp31" && ip' allow-related fi # dump information and flows with counters ovn-sbctl dump-flows -- list multicast_group > sbflows$sf -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-controller: Fix wrong conj_id match flows when caching is enabled.
From: Numan Siddique When the below ACL is added - ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow ovn-controller installs the below OF flows table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2) table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2) table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2) table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) When a full recompute is triggered, ovn-controller deletes the last OF flow with the match conj_id=2 and adds the below OF flow table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) For subsequent recomputes, the conj_id keeps increasing by 1. This disrupts the traffic which matches on conjuction action flows. This patch fixes this issue. Fixes: 1213bc8270("ovn-controller: Cache logical flow expr matches.") Signed-off-by: Numan Siddique --- controller/lflow.c | 7 +++ tests/ovn.at | 28 2 files changed, 35 insertions(+) diff --git a/controller/lflow.c b/controller/lflow.c index c02585b1e..a9420a7c4 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -754,6 +754,13 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, >match, , >header_.uuid); ofpbuf_uninit(); } + +if (m->match.wc.masks.conj_id) { +/* Reset the conj_id back to relative conj id. If caching is + * enabled, then processing of the expr match next time (due to + * full recompute) will result in the wrong conj_id match flow. */ +m->match.flow.conj_id -= conj_id_ofs; +} } ofpbuf_uninit(); diff --git a/tests/ovn.at b/tests/ovn.at index 8f884241d..0e297d2f2 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -13884,6 +13884,34 @@ reset_pcap_file hv1-vif2 hv1/vif2 rm -f 2.packets > 2.expected +# Trigger recompute and make sure that the traffic still works as expected. +as hv1 ovn-appctl -t ovn-controller recompute + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. +for src in `seq 1 2`; do +for dst in `seq 3 4`; do +sip=`ip_to_hex 10 0 0 $src` +dip=`ip_to_hex 10 0 0 $dst` + +test_ip 1 f001 f002 $sip $dip 2 +done +done + +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped. +dip=`ip_to_hex 10 0 0 5` +for src in `seq 1 2`; do +sip=`ip_to_hex 10 0 0 $src` + +test_ip 1 f001 f002 $sip $dip +done + +cat 2.expected > expout +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets +AT_CHECK([cat 2.packets], [0], [expout]) +reset_pcap_file hv1-vif2 hv1/vif2 +rm -f 2.packets +> 2.expected + # Add two less restrictive allow ACLs for src IP 10.0.0.1. ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' allow ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev