Re: [ovs-dev] [PATCH net v2 0/2] net: openvswitch: limit the recursions from action sets
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski : On Wed, 7 Feb 2024 08:24:14 -0500 you wrote: > Open vSwitch module accepts actions as a list from the netlink socket > and then creates a copy which it uses in the action set processing. > During processing of the action list on a packet, the module keeps a > count of the execution depth and exits processing if the action depth > goes too high. > > However, during netlink processing the recursion depth isn't checked > anywhere, and the copy trusts that kernel has large enough stack to > accommodate it. The OVS sample action was the original action which > could perform this kinds of recursion, and it originally checked that > it didn't exceed the sample depth limit. However, when sample became > optimized to provide the clone() semantics, the recursion limit was > dropped. > > [...] Here is the summary with links: - [net,v2,1/2] net: openvswitch: limit the number of recursions from action sets https://git.kernel.org/netdev/net/c/6e2f90d31fe0 - [net,v2,2/2] selftests: openvswitch: Add validation for the recursion test https://git.kernel.org/netdev/net/c/bd128f62c365 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] appveyor: Move from MinGW 32bit to msys64.
AppVeyor is planning to remove support for MinGW 32bit soon. And we
had a couple of incidents where it wasn't available already, so we
moved to a 'Previous' image.
Move to msys64 instead.
While at it making the CI scripts a little nicer, moving the non-Windows
parts of the preparation and build to separate files.
MSYS2 has its own version of python. However, we do not support
building on Windows with non-Windows python build. The main issue is
the delimiter symbol in PYTHONPATH. In Windows version it has to be
';', while the python supplied with MSYS2 uses ':' as on Linux, while
we detect Windows and pass ';' during the build. Renaming the binary,
so the Windows version is used.
Additionally switched to Python 3.12, 3.7 reached EoL some time back,
though it's still available in AppVeyor.
The stderr has to be redirected to stdout for scripts, because any
message on stderr is treated as fatal failure by powershell.
Scripts are running with 'set -e', so a failure of individual
commands will fail the script.
The OpenSSL download is still failing, but it is out of scope for
this change.
Signed-off-by: Ilya Maximets
---
.ci/windows-build.sh | 17
.ci/windows-prepare.sh | 11
Makefile.am| 2 ++
appveyor.yml | 59 --
4 files changed, 53 insertions(+), 36 deletions(-)
create mode 100644 .ci/windows-build.sh
create mode 100644 .ci/windows-prepare.sh
diff --git a/.ci/windows-build.sh b/.ci/windows-build.sh
new file mode 100644
index 0..22994fcdd
--- /dev/null
+++ b/.ci/windows-build.sh
@@ -0,0 +1,17 @@
+#!/bin/bash
+set -ex
+
+CONFIGURATION=$1
+
+./boot.sh
+./configure CC=build-aux/cccl LD="$(which link)" \
+LIBS="-lws2_32 -lShlwapi -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+--prefix=C:/openvswitch/usr --localstatedir=C:/openvswitch/var \
+--sysconfdir=C:/openvswitch/etc --with-pthread=c:/PTHREADS-BUILT/ \
+--enable-ssl --with-openssl=C:/OpenSSL-Win64 \
+--with-vstudiotarget="${CONFIGURATION}"
+
+make -j4
+make datapath_windows_analyze
+make install
+make windows_installer
diff --git a/.ci/windows-prepare.sh b/.ci/windows-prepare.sh
new file mode 100644
index 0..2d76add71
--- /dev/null
+++ b/.ci/windows-prepare.sh
@@ -0,0 +1,11 @@
+#!/bin/bash
+set -ex
+
+mkdir -p /var/cache/pacman/pkg/
+pacman -S --noconfirm --needed automake autoconf libtool make patch
+
+# Use an MSVC linker and a Windows version of Python.
+mv $(which link) $(which link)_copy
+mv $(which python3) $(which python3)_copy
+
+cd /c/pthreads4w-code && nmake all install
diff --git a/Makefile.am b/Makefile.am
index 94f488d18..45fce1243 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,6 +81,8 @@ EXTRA_DIST = \
.ci/linux-prepare.sh \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
+ .ci/windows-build.sh \
+ .ci/windows-prepare.sh \
.cirrus.yml \
.editorconfig \
.github/workflows/build-and-test.yml \
diff --git a/appveyor.yml b/appveyor.yml
index 5903b90d0..373f01a43 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -1,5 +1,5 @@
version: 1.0.{build}
-image: Previous Visual Studio 2019
+image: Visual Studio 2019
branches:
only:
- master
@@ -7,54 +7,41 @@ configuration:
- Debug
- Release
clone_folder: C:\openvswitch_compile
+shallow_clone: true
init:
-- ps: $env:PATH ="C:\Python37;"+$env:PATH
-- ps: New-Item -Type HardLink -Path "C:\Python37\python3.exe" -Value
"C:\Python37\python.exe"
-- ps: >-
+- ps: $env:PATH ="C:\Python312-x64;"+$env:PATH
+- ps: New-Item -Type HardLink -Path "C:\Python312-x64\python3.exe"
+ -Value "C:\Python312-x64\python.exe"
+- ps: |
mkdir C:\ovs-build-downloads
-mkdir C:\openvswitch\driver
-
$source = "https://slproweb.com/download/Win64OpenSSL-1_0_2u.exe;
-
$destination = "C:\ovs-build-downloads\Win64OpenSSL-1_0_2u.exe"
-
Invoke-WebRequest $source -OutFile $destination
cd C:\ovs-build-downloads
-
.\Win64OpenSSL-1_0_2u.exe /silent /verysilent /sp- /suppressmsgboxes
-
Start-Sleep -s 30
-
-cd C:\openvswitch
-
-git clone -q https://git.code.sf.net/p/pthreads4w/code c:\pthreads4w-code
-
-python3 -m pip install pypiwin32 --disable-pip-version-check
-
cd C:\openvswitch_compile
+- ps: git clone -q https://git.code.sf.net/p/pthreads4w/code c:\pthreads4w-code
+- ps: python3 -m pip install pypiwin32 --disable-pip-version-check
build_script:
- '"C:\Program Files (x86)\Microsoft Visual
Studio\2019\Community\VC\Auxiliary\Build\vcvars64.bat"'
-- C:\MinGW\msys\1.0\bin\bash -lc "echo \"C:/MinGW /mingw\" > /etc/fstab"
-- C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
-# Build pthreads
-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/pthreads4w-code && nmake all install"
-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && ./boot.sh"
-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch_compile && ./configure
CC=build-aux/cccl
Re: [ovs-dev] [PATCH ovn v2] pinctrl: dns: Ignore additional records.
Thank you for the updates, Dumitru.
Acked-by: Mark Michelson
On 2/9/24 10:21, Dumitru Ceara wrote:
EDNS is backwards compatible so it's safe to just ignore additional ARs.
Reported-at: https://github.com/ovn-org/ovn/issues/228
Reported-at: https://issues.redhat.com/browse/FDP-222
Signed-off-by: Dumitru Ceara
---
V2:
- Addressed Mark's comments:
- fixed subject
- added test
---
controller/pinctrl.c | 20 -
tests/ovn.at | 51
2 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index faa3f92260..98b29de9ff 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2887,6 +2887,7 @@ dns_build_ptr_answer(
}
#define DNS_RCODE_SERVER_REFUSE 0x5
+#define DNS_QUERY_TYPE_CLASS_LEN (2 * sizeof(ovs_be16))
/* Called with in the pinctrl_handler thread context. */
static void
@@ -2950,18 +2951,13 @@ pinctrl_handle_dns_lookup(
goto exit;
}
-/* Check if there is an additional record present, which is unsupported */
-if (in_dns_header->arcount) {
-VLOG_DBG_RL(, "Received DNS query with additional records, which"
-" is unsupported");
-goto exit;
-}
-
struct udp_header *in_udp = dp_packet_l4(pkt_in);
size_t udp_len = ntohs(in_udp->udp_len);
size_t l4_len = dp_packet_l4_size(pkt_in);
+uint8_t *l4_start = (uint8_t *) in_udp;
uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len);
uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1);
+uint8_t *in_dns_data_start = in_dns_data;
uint8_t *in_queryname = in_dns_data;
uint16_t idx = 0;
struct ds query_name;
@@ -2985,7 +2981,7 @@ pinctrl_handle_dns_lookup(
in_dns_data += idx;
/* Query should have TYPE and CLASS fields */
-if (in_dns_data + (2 * sizeof(ovs_be16)) > end) {
+if (in_dns_data + DNS_QUERY_TYPE_CLASS_LEN > end) {
ds_destroy(_name);
goto exit;
}
@@ -2999,6 +2995,10 @@ pinctrl_handle_dns_lookup(
goto exit;
}
+uint8_t *rest = in_dns_data + DNS_QUERY_TYPE_CLASS_LEN;
+uint32_t query_size = rest - in_dns_data_start;
+uint32_t query_l4_size = rest - l4_start;
+
uint64_t dp_key = ntohll(pin->flow_metadata.flow.metadata);
const char *answer_data = NULL;
bool ovn_owned = false;
@@ -3081,7 +3081,7 @@ pinctrl_handle_dns_lookup(
goto exit;
}
-uint16_t new_l4_size = ntohs(in_udp->udp_len) + dns_answer.size;
+uint16_t new_l4_size = query_l4_size + dns_answer.size;
size_t new_packet_size = pkt_in->l4_ofs + new_l4_size;
struct dp_packet pkt_out;
dp_packet_init(_out, new_packet_size);
@@ -3118,7 +3118,7 @@ pinctrl_handle_dns_lookup(
out_dns_header->arcount = 0;
/* Copy the Query section. */
-dp_packet_put(_out, dp_packet_data(pkt_in), dp_packet_size(pkt_in));
+dp_packet_put(_out, dp_packet_data(pkt_in), query_size);
/* Copy the answer sections. */
dp_packet_put(_out, dns_answer.data, dns_answer.size);
diff --git a/tests/ovn.at b/tests/ovn.at
index 2cf335cf45..6b0b80bfc1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11455,6 +11455,57 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([dns lookup : EDNS])
+AT_SKIP_IF([test $HAVE_SCAPY = no])
+ovn_start
+
+check ovn-nbctl ls-add ls \
+-- lsp-add ls lsp \
+-- lsp-set-addresses lsp "00:00:00:00:00:01 10.0.0.1"
+
+d=$(ovn-nbctl create dns records={})
+
+check ovn-nbctl set dns $d records:foo.ovn.org="10.0.0.42"
+check ovn-nbctl set Logical_switch ls dns_records="$d"
+
+net_add n1
+sim_add hv1
+
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+set interface hv1-vif1 external-ids:iface-id=lsp \
+options:tx_pcap=hv1/vif1-tx.pcap \
+options:rxq_pcap=hv1/vif1-rx.pcap
+
+OVN_POPULATE_ARP
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+dns_req=$(fmt_pkt "Ether(dst='00:00:00:00:00:02', src='00:00:00:00:00:01') / \
+ IP(dst='10.0.0.254', src='10.0.0.1') / \
+ UDP(sport=42424, dport=53) / \
+ DNS(rd=1, qd=DNSQR(qname='foo.ovn.org'), arcount=1, ar=[ \
+ DNSRR(type='OPT', rclass=4096, \
+ rdata=EDNS0ClientSubnet(source_plen=24, \
+ address='10.0.0.1'))])")
+dns_reply=$(fmt_pkt "Ether(dst='00:00:00:00:00:01', src='00:00:00:00:00:02') /
\
+ IP(dst='10.0.0.1', src='10.0.0.254') / \
+ UDP(sport=53, dport=42424,chksum=0) / \
+ DNS(qr=1, qd=DNSQR(qname='foo.ovn.org'), \
+ an=DNSRR(rrname='foo.ovn.org', type='A', ttl=3600, \
+ rdata='10.0.0.42'))")
+echo ${dns_reply} > expected
+
+as hv1 ovs-appctl
Re: [ovs-dev] [PATCH ovn v3 4/4] tests: Use the ovn-debug binary to determine table numbers.
Thanks Ales,
To be honest, I did not painstakingly inspect every single line of this
change, but my 5 minute look through the changes, plus the fact the
tests are passing, makes me confident this is OK.
Acked-by: Mark Michelson
On 2/8/24 13:17, Ales Musil wrote:
Use the ovn-debug commands to determine OpenFlow table numbers
based on stage name. With this there is no need to hardcode them
and it should be future proof for stage shifts/updates.
Signed-off-by: Ales Musil
---
tests/ovn-controller.at | 342 +++---
tests/ovn.at | 389 ++-
tests/system-ovn-kmod.at | 16 +-
tests/system-ovn.at | 20 +-
4 files changed, 438 insertions(+), 329 deletions(-)
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index f77e032d4..66e870876 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -901,6 +901,10 @@ check ovn-nbctl lsp-add ls1 ls1-lp1 \
wait_for_ports_up
ovn-appctl -t ovn-controller vlog/set file:dbg
+# Get the OF table numbers
+acl_eval=$(ovn-debug lflow-stage-to-oftable ls_out_acl_eval)
+acl_action=$(ovn-debug lflow-stage-to-oftable ls_out_acl_action)
+
dp_key=$(printf "%x" $(fetch_column datapath tunnel_key
external_ids:name=ls1))
port_key=$(printf "%x" $(fetch_column port_binding tunnel_key
logical_port=ls1-lp1))
@@ -918,14 +922,14 @@ for i in $(seq 10); do
check ovn-nbctl add address_set as1 addresses 10.0.0.$i
check ovn-nbctl --wait=hv sync
if test "$i" = 3; then
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=46,reg15=0x$port_key | \
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=$acl_eval,reg15=0x$port_key | \
grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl
-priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.1
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.2
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.3
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
+priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.1
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,$acl_action)
+priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.2
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,$acl_action)
+priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.3
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,$acl_action)
])
fi
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=46 | grep -c
"priority=1100"], [0], [$i
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=$acl_eval | grep -c
"priority=1100"], [0], [$i
])
done
@@ -940,15 +944,15 @@ for i in $(seq 10); do
check ovn-nbctl remove address_set as1 addresses 10.0.0.$i
check ovn-nbctl --wait=hv sync
if test "$i" = 9; then
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=46,reg15=0x$port_key | \
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=$acl_eval,reg15=0x$port_key | \
grep -v reply | awk '{print $7, $8}'], [0], [dnl
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
+priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.10
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,$acl_action)
])
fi
if test "$i" = 10; then
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=46 | grep
"priority=1100"], [1], [ignore])
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=$acl_eval | grep
"priority=1100"], [1], [ignore])
else
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=46 | grep -c
"priority=1100"], [0], [$((10 - $i))
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=$acl_eval | grep -c
"priority=1100"], [0], [$((10 - $i))
])
fi
done
@@ -966,17 +970,17 @@ for i in $(seq 10); do
check ovn-nbctl add address_set as1 addresses 10.0.0.$i,10.0.1.$i
check ovn-nbctl --wait=hv sync
if test "$i" = 3; then
-AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=46,reg15=0x$port_key | \
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
table=$acl_eval,reg15=0x$port_key | \
grep -v reply | awk '{print $7, $8}' | sort], [0], [dnl
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.1
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.2
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.0.3
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.1
actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
-priority=1100,ip,reg15=0x$port_key,metadata=0x$dp_key,nw_src=10.0.1.2
Re: [ovs-dev] [PATCH ovn v3 3/4] utilities: Add ovn-debug binary tool.
Thanks Ales,
Acked-by: Mark Michelson
On 2/8/24 13:17, Ales Musil wrote:
Add ovn-debug binary tool that can be extended with commands that
might be useful for tests/debugging of OVN environment. Currently
the tool supports only two commands:
1) "lflow-stage-to-ltable STAGE_NAME" that converts stage name into
logical flow table id.
2) "lflow-stage-to-oftable STAGE_NAME" that converts stage name into
OpenFlow table id.
For now it will be used in tests to get rid remaining hardcoded table
numbers.
Signed-off-by: Ales Musil
---
NEWS | 5 ++
README.rst | 1 +
debian/ovn-common.install | 1 +
debian/ovn-common.manpages | 1 +
rhel/ovn-fedora.spec.in| 2 +
utilities/.gitignore | 2 +
utilities/automake.mk | 10 ++-
utilities/ovn-debug.8.xml | 28 +++
utilities/ovn-debug.c | 155 +
9 files changed, 204 insertions(+), 1 deletion(-)
create mode 100644 utilities/ovn-debug.8.xml
create mode 100644 utilities/ovn-debug.c
diff --git a/NEWS b/NEWS
index 7114b96d1..b979e54d7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,10 @@
Post v24.03.0
-
+ - Add ovn-debug tool containing two commands.
+"lflow-stage-to-ltable STAGE_NAME" that converts stage name into logical
+flow table id.
+"lflow-stage-to-oftable STAGE_NAME" that converts stage name into OpenFlow
+table id.
OVN v24.03.0 - xx xxx
--
diff --git a/README.rst b/README.rst
index 6fb717742..428cd8ee8 100644
--- a/README.rst
+++ b/README.rst
@@ -56,6 +56,7 @@ The main components of this distribution are:
- ovn-sbctl, a tool for interfacing with the southbound database.
- ovn-trace, a debugging utility that allows for tracing of packets through
the logical network.
+- ovn-debug, a tool to simplify debugging of OVN setup.
- Scripts and specs for building RPMs.
What other documentation is available?
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
index 050d1c63a..fc48f07e4 100644
--- a/debian/ovn-common.install
+++ b/debian/ovn-common.install
@@ -5,6 +5,7 @@ usr/bin/ovn-ic-nbctl
usr/bin/ovn-ic-sbctl
usr/bin/ovn-trace
usr/bin/ovn_detrace.py
+usr/bin/ovn-debug
usr/share/ovn/scripts/ovn-ctl
usr/share/ovn/scripts/ovndb-servers.ocf
usr/share/ovn/scripts/ovn-lib
diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
index 1fa3d9cb3..e864512e3 100644
--- a/debian/ovn-common.manpages
+++ b/debian/ovn-common.manpages
@@ -11,3 +11,4 @@ utilities/ovn-ic-nbctl.8
utilities/ovn-ic-sbctl.8
utilities/ovn-trace.8
utilities/ovn-detrace.1
+utilities/ovn-debug.8
diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
index 03c1f27c5..670f1ca9e 100644
--- a/rhel/ovn-fedora.spec.in
+++ b/rhel/ovn-fedora.spec.in
@@ -495,6 +495,7 @@ fi
%{_bindir}/ovn-appctl
%{_bindir}/ovn-ic-nbctl
%{_bindir}/ovn-ic-sbctl
+%{_bindir}/ovn-debug
%{_datadir}/ovn/scripts/ovn-ctl
%{_datadir}/ovn/scripts/ovn-lib
%{_datadir}/ovn/scripts/ovndb-servers.ocf
@@ -515,6 +516,7 @@ fi
%{_mandir}/man8/ovn-ic.8*
%{_mandir}/man5/ovn-ic-nb.5*
%{_mandir}/man5/ovn-ic-sb.5*
+%{_mandir}/man8/ovn-debug.8*
%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
%config(noreplace) %{_sysconfdir}/logrotate.d/ovn
%{_unitdir}/ovn-db@.service
diff --git a/utilities/.gitignore b/utilities/.gitignore
index da237563b..3ae97b00f 100644
--- a/utilities/.gitignore
+++ b/utilities/.gitignore
@@ -13,6 +13,8 @@
/ovn-trace.8
/ovn_detrace.py
/ovn-detrace.1
+/ovn-debug
+/ovn-debug.8
/ovn-docker-overlay-driver
/ovn-docker-underlay-driver
/ovn-lib
diff --git a/utilities/automake.mk b/utilities/automake.mk
index c44563c26..6a2b96e66 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -11,7 +11,8 @@ man_MANS += \
utilities/ovn-ic-sbctl.8 \
utilities/ovn-trace.8 \
utilities/ovn-detrace.1 \
-utilities/ovn-appctl.8
+utilities/ovn-appctl.8 \
+utilities/ovn-debug.8
MAN_ROOTS += \
utilities/ovn-detrace.1.in
@@ -34,6 +35,7 @@ EXTRA_DIST += \
utilities/ovn-ic-sbctl.8.xml \
utilities/ovn-appctl.8.xml \
utilities/ovn-trace.8.xml \
+utilities/ovn-debug.8.xml \
utilities/ovn_detrace.py.in \
utilities/ovndb-servers.ocf \
utilities/checkpatch.py \
@@ -62,6 +64,7 @@ CLEANFILES += \
utilities/ovn-ic-nbctl.8 \
utilities/ovn-ic-sbctl.8 \
utilities/ovn-trace.8 \
+utilities/ovn-debug.8 \
utilities/ovn-detrace.1 \
utilities/ovn-detrace \
utilities/ovn_detrace.py \
@@ -119,4 +122,9 @@ UNINSTALL_LOCAL += ovn-detrace-uninstall
ovn-detrace-uninstall:
rm -f $(DESTDIR)$(bindir)/ovn-detrace
+# ovn-debug
+bin_PROGRAMS += utilities/ovn-debug
+utilities_ovn_debug_SOURCES = utilities/ovn-debug.c
+utilities_ovn_debug_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la
$(OVS_LIBDIR)/libopenvswitch.la
+
include utilities/bugtool/automake.mk
Re: [ovs-dev] [PATCH ovn v3 2/4] checkpatch: Add rule to check for hardcoded table numbers.
Thanks Ales,
Acked-by: Mark Michelson
On 2/8/24 13:17, Ales Musil wrote:
To avoid issues with hardcoded table numbers in future add rule
into check patch. The rule is only warning because there are still
legitimate use cases and not everything can be abstracted.
Signed-off-by: Ales Musil
---
utilities/checkpatch.py | 12
1 file changed, 12 insertions(+)
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 52d3fa845..9f7a48c7c 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -202,6 +202,7 @@ __regex_if_macros = re.compile(r'^ +(%s)
\([\S]([\s\S]+[\S])*\) { +\\' %
__parenthesized_constructs)
__regex_nonascii_characters = re.compile("[^\u-\u007f]")
__regex_efgrep = re.compile(r'.*[ef]grep.*$')
+__regex_hardcoded_table = re.compile(r'(table=[0-9]+)|(resubmit\(,[0-9]+\))')
skip_leading_whitespace_check = False
skip_trailing_whitespace_check = False
@@ -371,6 +372,10 @@ def has_efgrep(line):
"""Returns TRUE if the current line contains 'egrep' or 'fgrep'."""
return __regex_efgrep.match(line) is not None
+def has_hardcoded_table(line):
+"""Return TRUE if the current line contains table= or
+ resubmit(,)"""
+return __regex_hardcoded_table.match(line) is not None
def filter_comments(current_line, keep=False):
"""remove all of the c-style comments in a line"""
@@ -656,6 +661,13 @@ checks = [
'check': lambda x: has_efgrep(x),
'print':
lambda: print_error("grep -E/-F should be used instead of egrep/fgrep")},
+
+{'regex': r'\.at$', 'match_name': None,
+ 'check': lambda x: has_hardcoded_table(x),
+ 'print':
+ lambda: print_warning("Use of hardcoded table= or"
+ " resubmit=(,) is discouraged in tests."
+ " Consider using MACRO instead.")},
]
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3 1/4] tests: Remove hardcoded numbers from comments
Hi Ales,
I have one comment below, but it's small enough it can be fixed during
merge.
Acked-by: Mark Michelson
On 2/8/24 13:17, Ales Musil wrote:
There were some comments left with hardcoded numbers. Even if it
wouldn't break any test table shift/change it would just left the
comment outdated.
Signed-off-by: Ales Musil
---
tests/ovn-northd.at | 2 +-
tests/ovn.at| 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 591ad5aad..bb5e1f958 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2150,7 +2150,7 @@ AT_CLEANUP
# This test case tests that when a logical switch has load balancers associated
# (with VIPs configured), the below logical flow is added by ovn-northd.
-# table=1 (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]]
= 1; next;)
+# (ls_out_pre_lb ), priority=100 , match=(ip), action=(reg0[[0]] = 1;
next;)
The other changes in this patch are formatted differently than this one.
If this were modeled after the other changes, it would be
# table=ls_out_pr_lb, priority=100...
Should this comment be formatted the same as the others in this change?
# This test case is added for the BZ -
# https://bugzilla.redhat.com/show_bug.cgi?id=1849162
#
diff --git a/tests/ovn.at b/tests/ovn.at
index 2cf335cf4..0af60f893 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -18372,8 +18372,8 @@ AT_CHECK([cat 2.packets], [0], [expout])
# There should be total of 9 flows present with conjunction action and 2 flows
# with conj match. Eg.
-# table=44, priority=2001,conj_id=2,metadata=0x1 actions=resubmit(,45)
-# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
+# table=ls_out_acl_eval, priority=2001,conj_id=2,metadata=0x1
actions=resubmit(,ls_out_acl_action)
+# table=ls_out_acl_eval, priority=2001,conj_id=3,metadata=0x1 actions=drop
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(2,2/2)
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,2/2)
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(2,2/2)
@@ -18413,7 +18413,7 @@ AT_CHECK([cat 2.packets], [0], [])
# properly.
# There should be total of 6 flows present with conjunction action and 1 flow
# with conj match. Eg.
-# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
+# table=ls_out_acl_eval, priority=2001,conj_id=3,metadata=0x1 actions=drop
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,2/2)
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(4,2/2)
# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,2/2)
@@ -34296,7 +34296,7 @@ in_port_sec=OFTABLE_CHK_IN_PORT_SEC
in_port_sec_nd=OFTABLE_CHK_IN_PORT_SEC_ND
out_port_sec=OFTABLE_CHK_OUT_PORT_SEC
-# There should be no flows in table OFTABLE_CHK_IN_PORT_SEC, 74 and 75 in hv1 and hv2
+# There should be no flows in table OFTABLE_CHK_IN_PORT_SEC,
OFTABLE_CHK_IN_PORT_SEC_ND and OFTABLE_CHK_OUT_PORT_SEC in hv1 and hv2
> hv1_t${in_port_sec}_flows.expected
> hv1_t${in_port_sec_nd}_flows.expected
> hv1_t${out_port_sec}_flows.expected
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] bond: Update of recirculation rules requires write-lock.
For some reason annotation is made for a read-lock, while all the
callers are correctly holding a write-lock.
Fixes: 05df16238d43 ("ofproto/bond: Fix bond post recirc rule leak.")
Signed-off-by: Ilya Maximets
---
ofproto/bond.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/bond.c b/ofproto/bond.c
index cfdf44f85..c4b3a4a45 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -423,7 +423,7 @@ update_recirc_rules__(struct bond *bond)
static void
update_recirc_rules(struct bond *bond)
-OVS_REQ_RDLOCK(rwlock)
+OVS_REQ_WRLOCK(rwlock)
{
update_recirc_rules__(bond);
}
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] features.c: Always wait on the rconn.
On 2/9/24 04:21, Dumitru Ceara wrote:
Hi Mark,
Thanks for the fix, sorry for introducing the issue in the first place!
On 2/8/24 21:49, Mark Michelson wrote:
When performing feature detection with OVS, we use an rconn connected to
br-int.mgmt to query for features. If the rconn is not connected, then
we would exit early. However, this early exit did not call
rconn_run_wait() or rconn_recv_wait(). Therefore, if the rconn were in
a state such as BACKOFF or CONNECTING, we would not wait on the rconn's
fd for activity.
In most cases, nobody would notice this because ovn-controller would
likely be waking up constantly due to southbound database changes,
handling CLI commands, or other activity. However, in one of Red Hat
QE's tests, they were seeing ovn-controller take 10 seconds between
when it would start up and when OF flows were installed. This was
because the only thing waking ovn-controller up was regularly-scheduled
probe intervals with the southbound database.
To fix this, we call rconn_run_wait() and rconn_recv_wait() even in
the case when the rconn is not connected. This way, if we end up in a
state other than ACTIVE or IDLE, we can handle traffic from the rconn
when it arrives.
Fixes: e9e716ad531e ("controller: Don't artificially limit group and
meter IDs to 16bit.")
0-day bot flagged this as an issue, the Fixes tag should be on a single
line but that can be fixed when merging the patch.
Whoops, I missed this before pushing. I guess I could force-push a
change, but I think humans will understand the intent.
Reported-at: https://issues.redhat.com/browse/FDP-357
Signed-off-by: Mark Michelson
---
lib/features.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/features.c b/lib/features.c
index 82bce10e9..b04437235 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -150,6 +150,8 @@ ovs_feature_get_openflow_cap(const char *br_name)
rconn_run(swconn);
if (!rconn_is_connected(swconn)) {
+rconn_run_wait(swconn);
+rconn_recv_wait(swconn);
return false;
}
Looks good to me, thanks!
Acked-by: Dumitru Ceara
Regards,
Dumitru
Thanks Dumitru, I pushed this to main and all branches back to 22.12.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] bond: Reset stats when deleting post recirc rule.
On 2/9/24 08:06, Adrian Moreno wrote:
> In order to properly balance bond traffic, ofproto/bond periodically
> reads usage statistics of the post-recirculation rules (which are added
> to a hidden internal table).
>
> To do that, each "struct bond_entry" (which represents a hash within a
> bond) stores the last seen statistics for its rule. When a hash is moved
> to another member (due to a bond rebalance or the previous member going
> down), the rule is typically just modified, i.e: same match different
> actions. In this case, statistics are preserved and accounting continues
> to work.
>
> However, if the rule gets completely deleted (e.g: when all bond members
> go down) and then re-created, the new rule will have 0 tx_bytes but its
> associated entry will still store a non-zero last-seen value.
> This situation leads to an overflow of the delta calculation (computed
> as [current_stats_value - last_seen_value]), which can affect traffic
> as the hash will be considered to carry a lot of traffic and rebalancing
> will kick in.
>
> In order to fix this situation, reset the value of last seen statistics
> on rule deletion.
>
> Implementation note:
> The field ((struct bond_entry *) -> pr_tx_bytes) is guarded by rwlock
> but, as the comment above the function indicates, it's only called
> without having locked it when the bond is being deleted. Given that
> bond->hash is free()ed before deleting the rules (which makes writing to
> the entries a use-after-free anyway), we can use that to avoid
> double-locking.
clang is not happy about this. All the clang jobs failed in CI.
We have to either remove the guarding annotation or just take the
lock in the unref() function. The latter probably makes more sense.
Is there a good reason to not take the lock there?
>
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/319
> Signed-off-by: Adrian Moreno
> ---
> ofproto/bond.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index cfdf44f85..ad56bb96b 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -407,6 +407,10 @@ update_recirc_rules__(struct bond *bond)
>
> VLOG_ERR("failed to remove post recirculation flow %s",
> err_s);
> free(err_s);
> +} else if (bond->hash) {
> +uint32_t hash = pr_op->hmap_node.hash & BOND_MASK;
> +struct bond_entry *entry = >hash[hash];
> +entry->pr_tx_bytes = 0;
A few lines below we will be leaning up the rule pointer regardless
of the error. Should this code be moved there? Potentially re-ordered
with hmap_remove to be sure that the hash is kept intact?
> }
>
> hmap_remove(>pr_rule_ops, _op->hmap_node);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] northd: Added lb_vip_mac config option in Logical_Switch.
On Mon, Feb 5, 2024 at 11:45 PM Priyankar Jain
wrote:
>
> Hi,
>
> On 01/02/24 5:15 am, Numan Siddique wrote:
> > On Mon, Jan 8, 2024 at 8:13 AM Priyankar Jain
> > wrote:
> >> Currently load balancer applied to a logical switch has the
> >> following restriction:
> >> - VIP of the load balancer cannot reside in the subnet prefix as the
> >>clients as OVN does not install ARP responder flows for the LB VIP.
> >>
> > Hi Priyankar,
> >
> > Sorry for the late reviews.
> No worries. Thanks for you comments on the patch.
> >
> > The above statement is actually not correct. OVN does allow the VIP
> > of the load balancer
> > to be from the same subnet prefix. But in order for this to work,
> > this logical switch
> > has to be associated with a logical router.
> My bad. I would repurpose the commit message to make it more clearer.
> >
> > Eg.
> > --
> > ovn-nbctl ls-add sw0
> >
> > ovn-nbctl lsp-add sw0 sw0-p1
> > ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
> > ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3"
> >
> > ovn-nbctl lr-add lr0
> > ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> > ovn-nbctl lsp-add sw0 sw0-lr0
> > ovn-nbctl lsp-set-type sw0-lr0 router
> > ovn-nbctl lsp-set-addresses sw0-lr0 router
> > ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
> >
> > ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80
> > ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> >
> > # This will work once you do
> > ovn-nbctl lr-lb-add lr0 lb1
> > ---
> >
> > Do you have a use case where you attach a load balancer to a logical switch
> > and this logical switch doesn't connect to any logical router ? If
> > so, then perhaps we can consider
> > this patch. However if you always attach a logical router to a
> > logical switch (like how its
> > done in the test case added in this patch), then just attaching the
> > lb to the router would do.
>
> There are actually 2 usecases for us:
>
> 1. Logical switch (with localnet port) is not connected to any logical
> router. Routing is handled by the underlay here. And we have LB attached
> to logical switch.
>
> 2. Load balancer only applied to logical switches. Say we don't want the
> load balancer to be accessible from outside world (NS connectivity).
> Initially i thought it logical_router_policy can help but these gets
> applied (lr_in_policy) only after the LB (lr_in_defrag). Hence, policies
> see only translated addresses. In this case the LB is only applied to
> the logical switches and not to the logical routers.
'>
Thanks for the explanation. I think it makes sense to add this feature.
I'm afraid you have to rework a bit now that the northd lflow I-P
patches are merged.
Few pointers
1. Please move the 'lb_vip_mac' from 'struct od' to 'struct
ls_stateful_record' in northd/en-ls-stateful.h
2. Init this variable here -
https://github.com/ovn-org/ovn/blob/main/northd/en-ls-stateful.c#L314
3. Call the function build_lb_rules_arp_nd_rsp() which you added from
build_ls_stateful_flows() -
https://github.com/ovn-org/ovn/blob/main/northd/northd.c#L15490
Please let me know if you have questions.
Overall the patch LGTM.
Thanks
Numan
> >
> > Can you please confirm if this is sufficient for your use case ?
> > Perhaps we should document it in OVN :)
> >
> > Thanks
> > Numan
> >
> >
> >> This change adds a new config option "lb_vip_mac" in the logical_switch
> >> table which is expected to be a MAC address. If the logical_switch has
> >> this option configured, northd will program an ARP responder flow for
> >> all the LB VIPs of the logical_switch with this MAC address.
> >>
> >> Usecase: With this change, CMS can set the lb_vip_mac value to same as
> >> the default gateway MAC. This allows CMS to allocate VIP of the Load
> >> balancer from any subnet prefix.
> >>
> >> Signed-off-by: Priyankar Jain
> >
> >
> >
> >> ---
> >> northd/northd.c | 71 ++
> >> northd/northd.h | 2 +
> >> northd/ovn-northd.8.xml | 49 ++
> >> tests/ovn.at| 109
> >> 4 files changed, 231 insertions(+)
> >>
> >> diff --git a/northd/northd.c b/northd/northd.c
> >> index db3cd272e..ebca2c073 100644
> >> --- a/northd/northd.c
> >> +++ b/northd/northd.c
> >> @@ -790,8 +790,11 @@ init_lb_for_datapath(struct ovn_datapath *od)
> >> {
> >> if (od->nbs) {
> >> od->has_lb_vip = ls_has_lb_vip(od);
> >> +od->lb_vip_mac = nullable_xstrdup(
> >> +smap_get(>nbs->other_config, "lb_vip_mac"));
> >> } else {
> >> od->has_lb_vip = lr_has_lb_vip(od);
> >> +od->lb_vip_mac = NULL;
> >> }
> >> }
> >>
> >> @@ -800,6 +803,9 @@ destroy_lb_for_datapath(struct ovn_datapath *od)
> >> {
> >> ovn_lb_ip_set_destroy(od->lb_ips);
> >> od->lb_ips = NULL;
> >> +
> >> +free(od->lb_vip_mac);
> >> +od->lb_vip_mac
[ovs-dev] [PATCH ovn v2] pinctrl: dns: Ignore additional records.
EDNS is backwards compatible so it's safe to just ignore additional ARs.
Reported-at: https://github.com/ovn-org/ovn/issues/228
Reported-at: https://issues.redhat.com/browse/FDP-222
Signed-off-by: Dumitru Ceara
---
V2:
- Addressed Mark's comments:
- fixed subject
- added test
---
controller/pinctrl.c | 20 -
tests/ovn.at | 51
2 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index faa3f92260..98b29de9ff 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2887,6 +2887,7 @@ dns_build_ptr_answer(
}
#define DNS_RCODE_SERVER_REFUSE 0x5
+#define DNS_QUERY_TYPE_CLASS_LEN (2 * sizeof(ovs_be16))
/* Called with in the pinctrl_handler thread context. */
static void
@@ -2950,18 +2951,13 @@ pinctrl_handle_dns_lookup(
goto exit;
}
-/* Check if there is an additional record present, which is unsupported */
-if (in_dns_header->arcount) {
-VLOG_DBG_RL(, "Received DNS query with additional records, which"
-" is unsupported");
-goto exit;
-}
-
struct udp_header *in_udp = dp_packet_l4(pkt_in);
size_t udp_len = ntohs(in_udp->udp_len);
size_t l4_len = dp_packet_l4_size(pkt_in);
+uint8_t *l4_start = (uint8_t *) in_udp;
uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len);
uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1);
+uint8_t *in_dns_data_start = in_dns_data;
uint8_t *in_queryname = in_dns_data;
uint16_t idx = 0;
struct ds query_name;
@@ -2985,7 +2981,7 @@ pinctrl_handle_dns_lookup(
in_dns_data += idx;
/* Query should have TYPE and CLASS fields */
-if (in_dns_data + (2 * sizeof(ovs_be16)) > end) {
+if (in_dns_data + DNS_QUERY_TYPE_CLASS_LEN > end) {
ds_destroy(_name);
goto exit;
}
@@ -2999,6 +2995,10 @@ pinctrl_handle_dns_lookup(
goto exit;
}
+uint8_t *rest = in_dns_data + DNS_QUERY_TYPE_CLASS_LEN;
+uint32_t query_size = rest - in_dns_data_start;
+uint32_t query_l4_size = rest - l4_start;
+
uint64_t dp_key = ntohll(pin->flow_metadata.flow.metadata);
const char *answer_data = NULL;
bool ovn_owned = false;
@@ -3081,7 +3081,7 @@ pinctrl_handle_dns_lookup(
goto exit;
}
-uint16_t new_l4_size = ntohs(in_udp->udp_len) + dns_answer.size;
+uint16_t new_l4_size = query_l4_size + dns_answer.size;
size_t new_packet_size = pkt_in->l4_ofs + new_l4_size;
struct dp_packet pkt_out;
dp_packet_init(_out, new_packet_size);
@@ -3118,7 +3118,7 @@ pinctrl_handle_dns_lookup(
out_dns_header->arcount = 0;
/* Copy the Query section. */
-dp_packet_put(_out, dp_packet_data(pkt_in), dp_packet_size(pkt_in));
+dp_packet_put(_out, dp_packet_data(pkt_in), query_size);
/* Copy the answer sections. */
dp_packet_put(_out, dns_answer.data, dns_answer.size);
diff --git a/tests/ovn.at b/tests/ovn.at
index 2cf335cf45..6b0b80bfc1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11455,6 +11455,57 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([dns lookup : EDNS])
+AT_SKIP_IF([test $HAVE_SCAPY = no])
+ovn_start
+
+check ovn-nbctl ls-add ls \
+-- lsp-add ls lsp \
+-- lsp-set-addresses lsp "00:00:00:00:00:01 10.0.0.1"
+
+d=$(ovn-nbctl create dns records={})
+
+check ovn-nbctl set dns $d records:foo.ovn.org="10.0.0.42"
+check ovn-nbctl set Logical_switch ls dns_records="$d"
+
+net_add n1
+sim_add hv1
+
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+set interface hv1-vif1 external-ids:iface-id=lsp \
+options:tx_pcap=hv1/vif1-tx.pcap \
+options:rxq_pcap=hv1/vif1-rx.pcap
+
+OVN_POPULATE_ARP
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+dns_req=$(fmt_pkt "Ether(dst='00:00:00:00:00:02', src='00:00:00:00:00:01') / \
+ IP(dst='10.0.0.254', src='10.0.0.1') / \
+ UDP(sport=42424, dport=53) / \
+ DNS(rd=1, qd=DNSQR(qname='foo.ovn.org'), arcount=1, ar=[ \
+ DNSRR(type='OPT', rclass=4096, \
+ rdata=EDNS0ClientSubnet(source_plen=24, \
+ address='10.0.0.1'))])")
+dns_reply=$(fmt_pkt "Ether(dst='00:00:00:00:00:01', src='00:00:00:00:00:02') /
\
+ IP(dst='10.0.0.1', src='10.0.0.254') / \
+ UDP(sport=53, dport=42424,chksum=0) / \
+ DNS(qr=1, qd=DNSQR(qname='foo.ovn.org'), \
+ an=DNSRR(rrname='foo.ovn.org', type='A', ttl=3600, \
+ rdata='10.0.0.42'))")
+echo ${dns_reply} > expected
+
+as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 ${dns_req}
+OVN_CHECK_PACKETS_REMOVE_BROADCAST([hv1/vif1-tx.pcap], [expected])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
Re: [ovs-dev] [PATCH ovn] ci: Bump CirrusCI Ubuntu image version
On 2/8/24 14:15, Ales Musil wrote: > The Ubuntu 23.04 went EOL, change the image to 23.10. > > Signed-off-by: Ales Musil > --- Thanks, Ales! Applied to main and to all stable branches down to 23.06. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Documentation: Fix broken links in ovn-sandbox.rst.
On 2/7/24 02:48, Nobuhiro MIKI wrote: > Signed-off-by: Nobuhiro MIKI > --- Thanks for the fix! I applied this to main and backported it to all branches down to 22.03. I also added you to the authors list. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-sb.xml: Remove IPv4-only restriction from Service Monitors.
On 2/2/24 10:55, Ales Musil wrote:
> On Fri, Feb 2, 2024 at 4:21 AM Mark Michelson wrote:
>
>> The documentation for Service_Monitors in the southbound database state
>> that only IPv4 is supported. However, IPv6 service monitors have been
>> supported since OVN 23.03 was released.
>>
>> This patch addresses the problem by removing the incorrect
>> documentation.
>>
>> Fixes: 40a686e8e70f ("Add IPv6 support for lb health-check")
>> Signed-off-by: Mark Michelson
>> ---
Thanks, Mark and Ales!
Applied to main and backported all the way to 23.03.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).
On 2/2/24 11:08, Ales Musil wrote: > On Tue, Jan 30, 2024 at 1:07 PM Ilya Maximets wrote: > >> checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using >> outdated Node.js 16 which is now deprecated in GHA [1], so these >> actions may stop working soon. >> >> Updating to most recent major versions with Node.js 20. This stops >> GHA from throwing warnings in every build. >> >> [1] >> https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ >> >> While at it also updating upload-artifact and download-artifact to >> the latest versions. >> >> Removing versions from the upload-artifact comment, since the >> behavior doesn't seem to change much between versions. >> >> New setup-go@v5 attempts to cache dependencies by default. However, >> the default path it uses is go.sum in the root directory. This >> triggers a warning, since the file doesn't exist: >> >> Restore cache failed: Dependencies file is not found in >> /home/runner/work/ovn-kubernetes/ovn-kubernetes. >> Supported file pattern: go.sum >> >> Specify a path to all .sum files we have in the repository to make >> the setup-go happy. This should in theory make the builds a touch >> faster. This change is in line with recent changes in ovn-kubernetes >> itself. >> >> Signed-off-by: Ilya Maximets >> --- Thanks, Ilya and Ales! Applied to main and backported all the way to 22.03. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v5] ovn-ic: Fix global blacklist filter for IPv6 addresses.
Bleep bloop. Greetings Roberto Bartzen Acosta, 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. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 ovn-ic: Fix global blacklist filter for IPv6 addresses. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". 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 v5] ovn-ic: Fix global blacklist filter for IPv6 addresses.
This commit fixes the prefix filter function as the return condition for
IPv6 addresses is disabling the advertisement of all learned prefixes
regardless of the match with the blacklist or not.
Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
Signed-off-by: Roberto Bartzen Acosta
---
ic/ovn-ic.c | 15 +---
tests/ovn-ic.at | 99 +
2 files changed, 108 insertions(+), 6 deletions(-)
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 6f8f5734d..bc9aea057 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1064,12 +1064,15 @@ prefix_is_black_listed(const struct smap *nb_options,
continue;
}
} else {
-struct in6_addr mask = ipv6_create_mask(bl_plen);
-for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
-if ((prefix->s6_addr[i] & mask.s6_addr[i])
-!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
-continue;
-}
+struct in6_addr mask = ipv6_create_mask(plen);
+/* First calculate the difference between bl_prefix and prefix, so
+ * use the bl mask to ensure prefixes are correctly validated.
+ * e.g.: 2005:1734:5678::/50 is a subnet of 2005:1234::/21 */
+struct in6_addr m_prefixes = ipv6_addr_bitand(prefix, _prefix);
+struct in6_addr m_prefix = ipv6_addr_bitand(_prefixes, );
+struct in6_addr m_bl_prefix = ipv6_addr_bitand(_prefix, );
+if (!ipv6_addr_equals(_prefix, _bl_prefix)) {
+continue;
}
}
matched = true;
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index d4c436f84..6eb81e158 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -1274,3 +1274,102 @@ OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
+AT_KEYWORDS([IPv6-route-sync-blacklist])
+
+ovn_init_ic_db
+check ovn-ic-nbctl ts-add ts1
+
+for i in 1 2; do
+ovn_start az$i
+ovn_as az$i
+
+# Enable route learning at AZ level
+check ovn-nbctl set nb_global . options:ic-route-learn=true
+# Enable route advertising at AZ level
+check ovn-nbctl set nb_global . options:ic-route-adv=true
+# Enable blacklist single filter for IPv6
+check ovn-nbctl set nb_global . options:ic-route-blacklist=" \
+2003:db8:1::/64,2004:::/32,2005:1234::/21"
+
+OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
+
+# Create LRP and connect to TS
+check ovn-nbctl lr-add lr$i
+check ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i \
+2001:db8:1::$i/64
+check ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
+-- lsp-set-addresses lsp-ts1-lr$i router \
+-- lsp-set-type lsp-ts1-lr$i router \
+-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
+
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i \
+2002:db8:1::$i/64
+
+# Create blacklisted LRPs and connect to TS
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
+11:11:11:11:11:1$i 2003:db8:1::$i/64
+
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
+22:22:22:22:22:2$i 2004::bbb::$i/48
+
+# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
+33:33:33:33:33:3$i 2005:1734:5678::$i/50
+
+# additional not filtered prefix -> different subnet bits
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
+44:44:44:44:44:4$i 2005:1834:5678::$i/50
+done
+
+for i in 1 2; do
+OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep learned])
+done
+
+AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+# Drop blacklist
+check ovn-nbctl remove nb_global . options ic-route-blacklist
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2003:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
+2005:1734:5678::/50 2001:db8:1::2
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+check ovn-nbctl set nb_global . \
+options:ic-route-blacklist="2003:db8:1::/64,2004:db8:1::/64"
+
+# Create an 'extra' blacklisted LRP and connect to TS
+check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext5$i \
+55:55:55:55:55:5$i 2004:db8:1::$i/64
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
+2005:1734:5678::/50 2001:db8:1::2
+2005:1834:5678::/50 2001:db8:1::2
+])
+
Re: [ovs-dev] [PATCH ovn v4] ovn-ic: Fix global blacklist filter for IPv6 addresses.
Hi Dumitru, thanks for the feedback!
I will change the test file and send a new patch as you suggested, thanks.
Em sex., 9 de fev. de 2024 às 06:47, Dumitru Ceara
escreveu:
> Hi Roberto,
>
> Thanks for the fix! I have a few minor comments though.
>
>
> On 2/5/24 10:01, Ales Musil wrote:
> > On Mon, Feb 5, 2024 at 2:02 AM Roberto Bartzen Acosta via dev <
> > ovs-dev@openvswitch.org> wrote:
> >
> >> This commit fixes the prefix filter function as the return condition for
> >> IPv6 addresses is disabling the advertisement of all learned prefixes
> >> regardless of the match with the blacklist or not.
>
> Please use shorter lines in the commit message.
>
> >>
> >> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
> >> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
> >> Signed-off-by: Roberto Bartzen Acosta
>
> Should we update the .mailmap file too? The AUTHORS file has you listed
> with your gmail address. Anyway, that can be a separate patch or I can
> just add a commit when applying this fix.
>
Sure, go ahead with updating the .mailmap if it's not a problem for you.
Thanks
>
> >> --->> ic/ovn-ic.c | 15 +---
> >> tests/ovn-ic.at | 100
> >> 2 files changed, 109 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> >> index 6f8f5734d..bc9aea057 100644
> >> --- a/ic/ovn-ic.c
> >> +++ b/ic/ovn-ic.c
> >> @@ -1064,12 +1064,15 @@ prefix_is_black_listed(const struct smap
> >> *nb_options,
> >> continue;
> >> }
> >> } else {
> >> -struct in6_addr mask = ipv6_create_mask(bl_plen);
> >> -for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
> >> -if ((prefix->s6_addr[i] & mask.s6_addr[i])
> >> -!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
> >> -continue;
> >> -}
> >> +struct in6_addr mask = ipv6_create_mask(plen);
> >> +/* First calculate the difference between bl_prefix and
> >> prefix, so
> >> + * use the bl mask to ensure prefixes are correctly
> validated.
> >> + * e.g.: 2005:1734:5678::/50 is a subnet of 2005:1234::/21
> */
> >> +struct in6_addr m_prefixes = ipv6_addr_bitand(prefix,
> >> _prefix);
> >> +struct in6_addr m_prefix = ipv6_addr_bitand(_prefixes,
> >> );
> >> +struct in6_addr m_bl_prefix = ipv6_addr_bitand(_prefix,
> >> );
> >> +if (!ipv6_addr_equals(_prefix, _bl_prefix)) {
> >> +continue;
> >> }
> >> }
> >> matched = true;
> >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> >> index d4c436f84..1f9df71e9 100644
> >> --- a/tests/ovn-ic.at
> >> +++ b/tests/ovn-ic.at
> >> @@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
> >>
> >> AT_CLEANUP
> >> ])
> >> +
> >> +OVN_FOR_EACH_NORTHD([
> >> +AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
> >> +AT_KEYWORDS([IPv6-route-sync-blacklist])
> >> +
> >> +ovn_init_ic_db
> >> +ovn-ic-nbctl ts-add ts1
> >> +
> >> +for i in 1 2; do
> >> +ovn_start az$i
> >> +ovn_as az$i
> >> +
> >> +# Enable route learning at AZ level
> >> +ovn-nbctl set nb_global . options:ic-route-learn=true
> >> +# Enable route advertising at AZ level
> >> +ovn-nbctl set nb_global . options:ic-route-adv=true
> >> +# Enable blacklist single filter for IPv6
> >> +ovn-nbctl set nb_global .
> >> options:ic-route-blacklist="2003:db8:1::/64,\
> >> +2004:::/32,2005:1234::/21"
> >> +
> >> +OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
> >> +
> >> +# Create LRP and connect to TS
> >> +ovn-nbctl lr-add lr$i
> >> +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
> >> 2001:db8:1::$i/64
> >> +ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
> >> +-- lsp-set-addresses lsp-ts1-lr$i router \
> >> +-- lsp-set-type lsp-ts1-lr$i router \
> >> +-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
> >> +
> >> +ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i
> >> 2002:db8:1::$i/64
>
> Please prefix all ovn-nbctl commands with "check ". That will fail if
> the command errors out. It turns out your test adds some duplicate
> ports, in the logs:
>
> ovn-nbctl: lrp-lr1-p-ext21: a port with this name already exists
> ovn-nbctl: lrp-lr2-p-ext22: a port with this name already exists
>
> >> +
> >> +# Create blacklisted LRPs and connect to TS
> >> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
> >> +11:11:11:11:11:1$i 2003:db8:1::$i/64
> >> +
> >> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
> >> +22:22:22:22:22:2$i 2004::bbb::$i/48
> >> +
> >> +# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
> >> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
> >> +33:33:33:33:33:3$i 2005:1734:5678::$i/50
> >> +
> >> +# additional not filtered prefix ->
Re: [ovs-dev] [PATCH ovs v4] Documentation: Adding note about using the jemalloc library.
On Mon, Feb 05, 2024 at 09:36:14AM -0300, Roberto Bartzen Acosta via dev wrote: > Updating the reference documentation with the inclusion of possible building > problems with libjemalloc and solution suggestions. > > Reported-at: > https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748 > Signed-off-by: Roberto Bartzen Acosta Thanks Roberto, Eelco, and Frode, I noticed that this did not apply cleanly to main, and there was a leading '$' missing from the 2nd example. I have resolved those problems and applied the slightly revised patch below as. - Documentation: Adding note about using the jemalloc library. https://github.com/openvswitch/ovs/commit/2832faa22aa0 >From 2832faa22aa09b4bde51381fdfe730161fa22248 Mon Sep 17 00:00:00 2001 From: Roberto Bartzen Acosta Date: Mon, 5 Feb 2024 09:36:14 -0300 Subject: [PATCH] Documentation: Adding note about using the jemalloc library. Updating the reference documentation with the inclusion of possible building problems with libjemalloc and solution suggestions. Reported-at: https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748 Signed-off-by: Roberto Bartzen Acosta Acked-by: Eelco Chaudron Reviewed-by: Frode Nordahl [simon: rebased; added leading '$' to last configure example] Signed-off-by: Simon Horman --- Documentation/intro/install/general.rst | 16 1 file changed, 16 insertions(+) diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index 19e360d47ce4..86e85f75dbff 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -344,6 +344,22 @@ you wish to link with jemalloc add it to LIBS:: $ ./configure LIBS=-ljemalloc +.. note:: + Linking Open vSwitch with the jemalloc shared library may not work as + expected in certain operating system development environments. You can + override the automatic compiler decision to avoid possible linker issues by + passing ``-fno-lto`` or ``-fno-builtin`` flag since the jemalloc override + standard built-in memory allocation functions such as malloc, calloc, etc. + Both options can solve possible jemalloc linker issues with pros and cons for + each case, feel free to choose the path that appears best to you. Disabling + LTO flag example:: + + $ ./configure LIBS=-ljemalloc CFLAGS="-g -O2 -fno-lto" + + Disabling built-in flag example:: + + $ ./configure LIBS=-ljemalloc CFLAGS="-g -O2 -fno-builtin" + .. _general-building: Building -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] ovn-ic: Fix global blacklist filter for IPv6 addresses.
Hi Roberto,
Thanks for the fix! I have a few minor comments though.
On 2/5/24 10:01, Ales Musil wrote:
> On Mon, Feb 5, 2024 at 2:02 AM Roberto Bartzen Acosta via dev <
> ovs-dev@openvswitch.org> wrote:
>
>> This commit fixes the prefix filter function as the return condition for
>> IPv6 addresses is disabling the advertisement of all learned prefixes
>> regardless of the match with the blacklist or not.
Please use shorter lines in the commit message.
>>
>> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
>> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
>> Signed-off-by: Roberto Bartzen Acosta
Should we update the .mailmap file too? The AUTHORS file has you listed
with your gmail address. Anyway, that can be a separate patch or I can
just add a commit when applying this fix.
>> --->> ic/ovn-ic.c | 15 +---
>> tests/ovn-ic.at | 100
>> 2 files changed, 109 insertions(+), 6 deletions(-)
>>
>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>> index 6f8f5734d..bc9aea057 100644
>> --- a/ic/ovn-ic.c
>> +++ b/ic/ovn-ic.c
>> @@ -1064,12 +1064,15 @@ prefix_is_black_listed(const struct smap
>> *nb_options,
>> continue;
>> }
>> } else {
>> -struct in6_addr mask = ipv6_create_mask(bl_plen);
>> -for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
>> -if ((prefix->s6_addr[i] & mask.s6_addr[i])
>> -!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
>> -continue;
>> -}
>> +struct in6_addr mask = ipv6_create_mask(plen);
>> +/* First calculate the difference between bl_prefix and
>> prefix, so
>> + * use the bl mask to ensure prefixes are correctly validated.
>> + * e.g.: 2005:1734:5678::/50 is a subnet of 2005:1234::/21 */
>> +struct in6_addr m_prefixes = ipv6_addr_bitand(prefix,
>> _prefix);
>> +struct in6_addr m_prefix = ipv6_addr_bitand(_prefixes,
>> );
>> +struct in6_addr m_bl_prefix = ipv6_addr_bitand(_prefix,
>> );
>> +if (!ipv6_addr_equals(_prefix, _bl_prefix)) {
>> +continue;
>> }
>> }
>> matched = true;
>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>> index d4c436f84..1f9df71e9 100644
>> --- a/tests/ovn-ic.at
>> +++ b/tests/ovn-ic.at
>> @@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
>>
>> AT_CLEANUP
>> ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
>> +AT_KEYWORDS([IPv6-route-sync-blacklist])
>> +
>> +ovn_init_ic_db
>> +ovn-ic-nbctl ts-add ts1
>> +
>> +for i in 1 2; do
>> +ovn_start az$i
>> +ovn_as az$i
>> +
>> +# Enable route learning at AZ level
>> +ovn-nbctl set nb_global . options:ic-route-learn=true
>> +# Enable route advertising at AZ level
>> +ovn-nbctl set nb_global . options:ic-route-adv=true
>> +# Enable blacklist single filter for IPv6
>> +ovn-nbctl set nb_global .
>> options:ic-route-blacklist="2003:db8:1::/64,\
>> +2004:::/32,2005:1234::/21"
>> +
>> +OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
>> +
>> +# Create LRP and connect to TS
>> +ovn-nbctl lr-add lr$i
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
>> 2001:db8:1::$i/64
>> +ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
>> +-- lsp-set-addresses lsp-ts1-lr$i router \
>> +-- lsp-set-type lsp-ts1-lr$i router \
>> +-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
>> +
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i
>> 2002:db8:1::$i/64
Please prefix all ovn-nbctl commands with "check ". That will fail if
the command errors out. It turns out your test adds some duplicate
ports, in the logs:
ovn-nbctl: lrp-lr1-p-ext21: a port with this name already exists
ovn-nbctl: lrp-lr2-p-ext22: a port with this name already exists
>> +
>> +# Create blacklisted LRPs and connect to TS
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
>> +11:11:11:11:11:1$i 2003:db8:1::$i/64
>> +
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
>> +22:22:22:22:22:2$i 2004::bbb::$i/48
>> +
>> +# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
>> +33:33:33:33:33:3$i 2005:1734:5678::$i/50
>> +
>> +# additional not filtered prefix -> different subnet bits
>> +ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
>> +33:33:33:33:33:3$i 2005:1834:5678::$i/50
>> +
Nit: no need for empty line.
>> +done
>> +
>> +for i in 1 2; do
>> +OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep
>> learned])
>> +done
>> +
>> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
>> +awk '/learned/{print $1, $2}' ], [0], [dnl
>> +2002:db8:1::/64 2001:db8:1::2
>> +2005:1834:5678::/50 2001:db8:1::2
>> +])
>> +
>> +for
Re: [ovs-dev] [PATCH ovn] features.c: Always wait on the rconn.
Hi Mark,
Thanks for the fix, sorry for introducing the issue in the first place!
On 2/8/24 21:49, Mark Michelson wrote:
> When performing feature detection with OVS, we use an rconn connected to
> br-int.mgmt to query for features. If the rconn is not connected, then
> we would exit early. However, this early exit did not call
> rconn_run_wait() or rconn_recv_wait(). Therefore, if the rconn were in
> a state such as BACKOFF or CONNECTING, we would not wait on the rconn's
> fd for activity.
>
> In most cases, nobody would notice this because ovn-controller would
> likely be waking up constantly due to southbound database changes,
> handling CLI commands, or other activity. However, in one of Red Hat
> QE's tests, they were seeing ovn-controller take 10 seconds between
> when it would start up and when OF flows were installed. This was
> because the only thing waking ovn-controller up was regularly-scheduled
> probe intervals with the southbound database.
>
> To fix this, we call rconn_run_wait() and rconn_recv_wait() even in
> the case when the rconn is not connected. This way, if we end up in a
> state other than ACTIVE or IDLE, we can handle traffic from the rconn
> when it arrives.
>
> Fixes: e9e716ad531e ("controller: Don't artificially limit group and
> meter IDs to 16bit.")
0-day bot flagged this as an issue, the Fixes tag should be on a single
line but that can be fixed when merging the patch.
>
> Reported-at: https://issues.redhat.com/browse/FDP-357
> Signed-off-by: Mark Michelson
> ---
> lib/features.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/features.c b/lib/features.c
> index 82bce10e9..b04437235 100644
> --- a/lib/features.c
> +++ b/lib/features.c
> @@ -150,6 +150,8 @@ ovs_feature_get_openflow_cap(const char *br_name)
>
> rconn_run(swconn);
> if (!rconn_is_connected(swconn)) {
> +rconn_run_wait(swconn);
> +rconn_recv_wait(swconn);
> return false;
> }
>
Looks good to me, thanks!
Acked-by: Dumitru Ceara
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
