Re: [ovs-dev] [PATCH ovn] northd: Allow DHCP request from the lport if enabled DHCPv4
Bleep bloop. Greetings 刘勰, 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: The subject summary should end with a dot. Subject: northd: Allow DHCP request from the lport if enabled DHCPv4 ERROR: Author shylou needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Xie Liu WARNING: Line is 84 characters long (recommended limit is 79) #35 FILE: northd/northd.c:8411: REGBIT_ACL_VERDICT_ALLOW" = 1; next;", Lines checked: 61, Warnings: 3, Errors: 1 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: Allow DHCP request from the lport if enabled DHCPv4
From: shylou
DHCP for VM fails when removing default security group rules
using a CMS like Neutron ML2/OVN [1]. This is because DHCP
requests from VMs may be dropped by ACLs. To fix this issue,
we add a lflow with a priority of 34000 to allow DHCP requests
from the logical port if the CMS has enabled native DHCPv4
for this port.
[1]https://bugs.launchpad.net/neutron/+bug/1926515
Signed-off-by: Xie Liu
---
northd/northd.c | 10 ++
tests/ovn-northd.at | 5 +
2 files changed, 15 insertions(+)
diff --git a/northd/northd.c b/northd/northd.c
index 2c3560ce2..ca641a19e 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8414,6 +8414,16 @@ build_dhcpv4_options_flows(struct ovn_port *op,
meter_groups),
>nbsp->dhcpv4_options->header_,
lflow_ref);
+/* Add 34000 priority flow to allow DHCP request from the lport
+ * if the CMS has enabled native DHCPv4 for this lport.
+ * */
+ovn_lflow_add_with_lport_and_hint(lflows, op->od,
+ S_SWITCH_IN_ACL_EVAL, 34000,
+ ds_cstr(),
+ REGBIT_ACL_VERDICT_ALLOW" = 1;
next;",
+ op->key,
+ >nbsp->header_,
+ lflow_ref);
ds_clear();
/* If REGBIT_DHCP_OPTS_RESULT is set, it means the
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 6fdd761da..7657aefff 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -4897,6 +4897,11 @@ ovn-nbctl --wait=sb lsp-set-dhcpv4-options sw0-port1
$CIDR_UUID
ovn-sbctl dump-flows sw0 > sw0flows
AT_CAPTURE_FILE([sw0flows])
+AT_CHECK([grep "_acl_eval" sw0flows | grep sw0-port1 | ovn_strip_lflows], [0],
[dnl
+ table=??(ls_in_acl_eval ), priority=34000, match=(inport == "sw0-port1"
&& eth.src == 50:54:00:00:00:01 && (ip4.src == {10.0.0.2, 0.0.0.0} && ip4.dst
== {10.0.0.1, 255.255.255.255}) && udp.src == 68 && udp.dst == 67),
action=(reg8[[16]] = 1; next;)
+ table=??(ls_out_acl_eval), priority=34000, match=(outport == "sw0-port1"
&& eth.src == c0:ff:ee:00:00:01 && ip4.src == 10.0.0.1 && udp && udp.src == 67
&& udp.dst == 68), action=(reg8[[16]] = 1; next;)
+])
+
AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_dhcp_options ), priority=0, match=(1), action=(next;)
table=??(ls_in_dhcp_options ), priority=100 , match=(inport == "sw0-port1"
&& eth.src == 50:54:00:00:00:01 && (ip4.src == {10.0.0.2, 0.0.0.0} && ip4.dst
== {10.0.0.1, 255.255.255.255}) && udp.src == 68 && udp.dst == 67),
action=(reg0[[3]] = put_dhcp_opts(offerip = 10.0.0.2, hostname = "foo",
lease_time = 3600, netmask = 255.255.255.0, router = 10.0.0.1, server_id =
10.0.0.1); next;)
--
2.42.0.windows.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Ignore log setting extended ack support failed.
Bleep bloop. Greetings 刘勰, 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: ERROR: Author shylou needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Xie Liu Lines checked: 32, Warnings: 1, Errors: 1 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] tests: Ignore log setting extended ack support failed.
From: shylou
The test error log 'setting extended ack support failed' in the test
results may cause test failures. However, according to the OVS commit[1],
this is not a critical issue, so we can safely ignore this log.
[1]https://github.com/openvswitch/ovs/commit/812164adefc716dd35c50c2101e8e60a15167635
Signed-off-by: Xie Liu
---
tests/ofproto-macros.at | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index bccedbaf7..31a067c1e 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -288,6 +288,7 @@ check_logs () {
/ovs_rcu.*blocked [[0-9]]* ms waiting for .* to quiesce/d
/Dropped [[0-9]]* log messages/d
/.*Trying to release unknown interface.*/d
+/setting extended ack support failed/d
/|WARN|/p
/|ERR|/p
/|EMER|/p" ${logs}
--
2.42.0.windows.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-nbctl: Document "--portrange" in the manpage.
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 is 322 characters long (recommended limit is 79) #28 FILE: utilities/ovn-nbctl.8.xml:1178: [--may-exist] [--stateless] [--gateway-port=GATEWAY_PORT] [--portrange] lr-nat-add router type external_ip logical_ip [logical_port external_mac] [external_port_range] Lines checked: 54, 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] ovn-nbctl: Document "--portrange" in the manpage.
The --portrange option was added in commit [0]. The option appears in the help text for ovn-nbctl, but it is not documented in the manpage at all. This commit adds documentation to the manpage for the --portrange option. Reported-at: https://issues.redhat.com/browse/FDP-537 Signed-off-by: Mark Michelson --- utilities/ovn-nbctl.8.xml | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 39c352212..ea2b201a5 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -1175,7 +1175,7 @@ NAT Commands - [--may-exist] [--stateless] [--gateway-port=GATEWAY_PORT] lr-nat-add router type external_ip logical_ip [logical_port external_mac] + [--may-exist] [--stateless] [--gateway-port=GATEWAY_PORT] [--portrange] lr-nat-add router type external_ip logical_ip [logical_port external_mac] [external_port_range] Adds the specified NAT to router. @@ -1212,6 +1212,18 @@ specify the GATEWAY_PORT. + + If the --portrange option is specified, then a range of + ports may be specified in the external_port_range part + of the lr-nat-add command. If this option is omitted, + then an external port range may not be specified. The format of the + port range is port_low-port_high, where + port_low is a lower number than port_high. When + the packet is NATted, a random port from the range will be selected + as the source port. The range for the + external_port_range is 1-65535. + + When type is dnat, the externally visible IP address external_ip is DNATted to the -- 2.44.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] python: ovsdb-idl: Add custom transaction operations.
It can be useful to be able to send raw transaction operations
through the Idl's connection. For example, to clean up MAC_Binding
entries for floating IPs without having to monitor the MAC_Binding
table which can be quite large.
Signed-off-by: Terry Wilson
---
NEWS | 2 ++
python/ovs/db/idl.py | 18 ++-
tests/ovsdb-idl.at | 27 ++
tests/test-ovsdb.py | 55
4 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index b92cec532..f30b859fd 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ Post-v3.3.0
- The primary development branch has been renamed from 'master' to 'main'.
The OVS tree remains hosted on GitHub.
https://github.com/openvswitch/ovs.git
+ - Python:
+ * Add custom transaction support to the Idl via add_op().
v3.3.0 - 16 Feb 2024
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index a80da84e7..1220e4cc6 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -1703,6 +1703,8 @@ class Transaction(object):
self._inserted_rows = {} # Map from UUID to _InsertedRow
+self._operations = []
+
def add_comment(self, comment):
"""Appends 'comment' to the comments that will be passed to the OVSDB
server when this transaction is committed. (The comment will be
@@ -1838,7 +1840,7 @@ class Transaction(object):
"rows": [rows]})
# Add updates.
-any_updates = False
+any_updates = bool(self._operations)
for row in self._txn_rows.values():
if row._changes is None:
if row._table.is_root:
@@ -1977,6 +1979,7 @@ class Transaction(object):
if self.dry_run:
operations.append({"op": "abort"})
+operations += self._operations
if not any_updates:
self._status = Transaction.UNCHANGED
else:
@@ -1991,6 +1994,19 @@ class Transaction(object):
self.__disassemble()
return self._status
+def add_op(self, op):
+"""Add a raw OVSDB operation to the transaction
+
+This can be useful for re-using the existing Idl connection to take
+actions that are difficult or expensive to do with the Idl itself. e.g.
+deleting a bunch of rows on the server that you don't want to store
+in memory.
+
+:param op: An "op" for an OVSDB "transact" request (rfc 7047 Sec 5.2)
+:type op: dict
+"""
+self._operations.append(op)
+
def commit_block(self):
"""Attempts to commit this transaction, blocking until the commit
either succeeds or fails. Returns the final commit status, which may
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index fb568dd82..487545a13 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2758,6 +2758,33 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent
uuid insert],
[['This UUID would duplicate a UUID already present within the table or
deleted within the same transaction']])
+OVSDB_CHECK_IDL_PY([simple idl, python, add_op],
+ [],
+ [['insert 1, insert 2, insert 3, insert 1' \
+'add_op {"op": "delete", "table": "simple", "where": [["i", "==", 1]]}' \
+'add_op {"op": "insert", "table": "simple", "row": {"i": 2}}, delete 3' \
+'insert 2, add_op {"op": "update", "table": "simple", "row": {"i": 1},
"where": [["i", "==", 2]]}'
+ ]],
+ [[000: empty
+001: commit, status=success
+002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<1>
+002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<2>
+002: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<3>
+002: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<4>
+003: commit, status=success
+004: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<3>
+004: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<4>
+005: commit, status=success
+006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<3>
+006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<5>
+007: commit, status=success
+008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<3>
+008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<5>
+008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
uuid=<6>
+009: done
+]],[],sort)
+
+
m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE],
[AT_SETUP([simple idl, database change aware, online conversion - $1])
AT_KEYWORDS([ovsdb server idl db_change_aware conversion $1])
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 48f8ee2d7..58829f520 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -36,6 +36,53 @@ vlog.set_levels_from_string("console:dbg")
vlog.init(None)
+OBJ_STR =
Re: [ovs-dev] [PATCH] rhel/systemd: Set ovsdb-server timeout to 5 minutes
> On Apr 11, 2024, at 9:43 AM, Chris Riches wrote: > > On 11/04/2024 14:24, Ilya Maximets wrote: >> On 4/11/24 10:59, Chris Riches wrote: >>> From what we know so far, the DB was full of stale connection-tracking >>> information such as the following: >>> >>> [...] >>> >>> Once the host was recovered by putting in the timeout increase, >>> ovsdb-server successfully started and GCed the database down from 2.4 >>> *GB* to 29 *KB*. Had this happened before the host restart, we would >>> have never seen this problem. But since it seems possible to end up >>> booting with such a large DB, we figured a timeout increase was a >>> sensible measure to take. >> Uff. Sounds like ovn-controller went off the rails. >> >> Normally, ovsdb-server compacts the database once in 10-20 minutes, >> if the database doubles the size since the previous check. If all >> the transactions are that small, it would mean ovn-controller made >> about 10K transactions per second in the 10-20 minutes before the >> restart. That's huge. >> >> I wonder if this can be addressed with a better compaction strategy. >> Something like forcing compaction if "the database is more than 10 MB >> and increased 10x" regardless of the time. > > I'm not sure exactly what the test was doing when this was observed, so I > don't know whether that transaction volume is within the realm of possibility > or if we're looking at a failure to perform compaction on time. It would be > nice to have an enhanced safety-net for DB size, as we were only a few > hundred MB away from hitting filesystem space issues as well. > >> Normally, ovsdb-server compacts the database once in 10-20 minutes, if the >> database doubles the size since the previous check. > > I presume you mean if it doubled in size since the previous *compaction*? If > we only compact when it doubles since the last *check*, then it would be easy > for it to slightly-less-than-double every 10-20 minutes and never trigger the > compaction while still growing exponentially. > > I'm happy to discuss compaction approaches (though my expertise is very much > in host service management and not OVS itself), but do you think there's > merit in having this extended timeout as a backstop too? FWIW, I think we should do both extending the time out and tuning up the compaction, as having a situation where a service can get in an endless loop if for whatever reason it takes too long is problematic. Addressing the root cause (compaction, too many calls, some other bug(s) etc) is good, but extending the timeout seems like an easy backstop. Jon > ___ > dev mailing list > d...@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=s883GpUCOChKOHiocYtGcg=NGPRGGo37mQiSXgHKm5rCQ=W-MV_AlPAPbGd0QQE1V3omKJ2hiODNwbKHcM7ION6RNc0sYiyjrAH_TO-iOsIPpm=pGAqsnVB7yeN2KmbcZaS7UGC4ybLp4oJPc4wVMaK02A= ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8] rhel: Make the version, displayed to the user, customizable.
Since on CentOS/RHEL the builds are based on stable branches and not on
tags for debugging purpose it's better to have the downstream version as
version so it's easier to know which commits are included in a build.
This commit adds --with-version-suffix as ./configure option in
order to set an OVS version suffix that should be shown to the user via
ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
utilities.
--with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to have
the version be aligned with the downstream one.
Signed-off-by: Timothy Redaelli
---
v1 -> v2: Use --with-version-suffix= and add version to other utilies
(as requested by Ilya).
v2 -> v3: Add versioning to python utilities and python library itself
(as suggested by Aaron).
v3 -> v4: Remove versioning to python library itself to avoid breaking
PEP440 (as requested by Ilya). Versioning is still used in
python utilities.
v4 -> v5: Re-add versioning to python library itself, but don't use it on
setup.py (to avoid breaking PEP440). This will permit to have the
custom version as ovs.version.VERSION (in case somebody uses it) and,
so, also in python/ovs/unixctl/server.py (as suggested by Ilya).
v5 -> v6: Fix some setup.py leftovers and change the test as a loop
(as suggested by Ilya).
v6 -> v7: Rebase with last master (it should pass CI now)
v7 -> v8: Be sure python-sdist depends on python/setup.py and run flake8 on
setup.py.template instead of setup.py (as suggested by Ilya).
Fix commit summary to make checkpatch.py happy
---
Makefile.am| 1 +
acinclude.m4 | 13 +
configure.ac | 1 +
include/openvswitch/version.h.in | 2 +-
lib/ovsdb-error.c | 2 +-
lib/util.c | 5 +++--
ovsdb/ovsdb-server.c | 3 ++-
python/.gitignore | 1 +
python/automake.mk | 20 ++--
python/{setup.py => setup.py.template} | 9 -
rhel/openvswitch-fedora.spec.in| 1 +
utilities/ovs-dpctl-top.in | 2 +-
utilities/ovs-lib.in | 2 +-
utilities/ovs-parse-backtrace.in | 2 +-
utilities/ovs-pcap.in | 2 +-
utilities/ovs-pki.in | 2 +-
utilities/ovs-tcpdump.in | 4 ++--
utilities/ovs-tcpundump.in | 2 +-
utilities/ovs-vlan-test.in | 2 +-
vswitchd/bridge.c | 3 ++-
20 files changed, 53 insertions(+), 26 deletions(-)
rename python/{setup.py => setup.py.template} (95%)
diff --git a/Makefile.am b/Makefile.am
index e6c90a911..fa89f591b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -163,6 +163,7 @@ SUFFIXES += .in
-e 's,[@]PYTHON3[@],$(PYTHON3),g' \
-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
-e 's,[@]VERSION[@],$(VERSION),g' \
+ -e 's,[@]VERSION_SUFFIX[@],$(VERSION_SUFFIX),g' \
-e 's,[@]localstatedir[@],$(localstatedir),g' \
-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
diff --git a/acinclude.m4 b/acinclude.m4
index f1ba046c2..f7a81a734 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -497,6 +497,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
])
+dnl Append a version suffix
+
+AC_DEFUN([OVS_CHECK_VERSION_SUFFIX], [
+ AC_ARG_WITH([version-suffix],
+ [AS_HELP_STRING([--with-version-suffix=ver_suffix],
+ [Specify a version suffix that will be appended
+ to OVS version])])
+ AC_DEFINE_UNQUOTED([VERSION_SUFFIX], ["$with_version_suffix"],
+ [Package version suffix])
+ AC_SUBST([VERSION_SUFFIX], [$with_version_suffix])
+ ])
+])
+
dnl Checks for net/if_dl.h.
dnl
dnl (We use this as a proxy for checking whether we're building on FreeBSD
diff --git a/configure.ac b/configure.ac
index dd6553fea..8323e481d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -202,6 +202,7 @@ OVS_CHECK_LINUX_SCTP_CT
OVS_CHECK_LINUX_VIRTIO_TYPES
OVS_CHECK_DPDK
OVS_CHECK_PRAGMA_MESSAGE
+OVS_CHECK_VERSION_SUFFIX
AC_SUBST([CFLAGS])
AC_SUBST([OVS_CFLAGS])
AC_SUBST([OVS_LDFLAGS])
diff --git a/include/openvswitch/version.h.in b/include/openvswitch/version.h.in
index 23d8fde4f..231f61e30 100644
--- a/include/openvswitch/version.h.in
+++ b/include/openvswitch/version.h.in
@@ -19,7 +19,7 @@
#define OPENVSWITCH_VERSION_H 1
#define OVS_PACKAGE_STRING "@PACKAGE_STRING@"
-#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@"
+#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@@VERSION_SUFFIX@"
#define OVS_LIB_VERSION @LT_CURRENT@
#define OVS_LIB_REVISION@LT_REVISION@
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index
[ovs-dev] [Patch ovn v3 1/2] actions: New action ct_commit_to_zone.
This change adds a new action 'ct_commit_to_zone' that enables users to commit
the flow into a specific zone in the connection tracker.
A new feature flag, OVN_FEATURE_CT_COMMIT_TO_ZONE, is also included to avoid
issues during upgrade in case the northd is upgraded to a version that supports
this action before the controller is upgraded.
Note that this action is meaningful only in the context of Logical Router
datapath. Logical Switch datapath does not use multiple zones and this action
falls back to committing the connection into the default zone for the Logical
Switch.
Signed-off-by: Martin Kalcok
---
controller/chassis.c | 8 ++
include/ovn/actions.h | 1 +
include/ovn/features.h| 1 +
lib/actions.c | 60 +++
northd/en-global-config.c | 11 +++
northd/en-global-config.h | 1 +
ovn-sb.xml| 24
tests/ovn.at | 16 +++
utilities/ovn-trace.c | 1 +
9 files changed, 123 insertions(+)
diff --git a/controller/chassis.c b/controller/chassis.c
index ad75df288..9bb2eba95 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -371,6 +371,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg
*ovs_cfg,
smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true");
smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true");
smap_replace(config, OVN_FEATURE_CT_COMMIT_NAT_V2, "true");
+smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true");
}
/*
@@ -516,6 +517,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
return true;
}
+if (!smap_get_bool(_rec->other_config,
+ OVN_FEATURE_CT_COMMIT_TO_ZONE,
+ false)) {
+return true;
+}
+
return false;
}
@@ -648,6 +655,7 @@ update_supported_sset(struct sset *supported)
sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP);
sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN);
sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2);
+sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE);
}
static void
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 8e794450c..4bcd1f58c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -68,6 +68,7 @@ struct collector_set_ids;
OVNACT(CT_NEXT, ovnact_ct_next) \
/* CT_COMMIT_V1 is not supported anymore. */ \
OVNACT(CT_COMMIT_V2, ovnact_nest)\
+OVNACT(CT_COMMIT_TO_ZONE, ovnact_ct_commit_nat) \
OVNACT(CT_DNAT, ovnact_ct_nat) \
OVNACT(CT_SNAT, ovnact_ct_nat) \
OVNACT(CT_DNAT_IN_CZONE, ovnact_ct_nat) \
diff --git a/include/ovn/features.h b/include/ovn/features.h
index 08f1d8288..35a5d8ba0 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -28,6 +28,7 @@
#define OVN_FEATURE_FDB_TIMESTAMP "fdb-timestamp"
#define OVN_FEATURE_LS_DPG_COLUMN "ls-dpg-column"
#define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2"
+#define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone"
/* OVS datapath supported features. Based on availability OVN might generate
* different types of openflows.
diff --git a/lib/actions.c b/lib/actions.c
index 39bb5bc8a..f26817018 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -791,6 +791,64 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
ct = ofpacts->header;
ofpact_finish(ofpacts, >ofpact);
+}
+
+static void
+parse_CT_COMMIT_TO_ZONE(struct action_context *ctx)
+{
+add_prerequisite(ctx, "ip");
+
+struct ovnact_ct_commit_nat *ct_commit =
+ovnact_put_CT_COMMIT_TO_ZONE(ctx->ovnacts);
+
+lexer_force_match(ctx->lexer, LEX_T_LPAREN);
+
+if (lexer_match_id(ctx->lexer, "dnat")) {
+ct_commit->dnat_zone = true;
+} else if (lexer_match_id(ctx->lexer, "snat")) {
+ct_commit->dnat_zone = false;
+} else {
+lexer_syntax_error(ctx->lexer, "expecting parameter 'dnat' or 'snat'");
+}
+
+lexer_force_match(ctx->lexer, LEX_T_RPAREN);
+}
+
+static void
+format_CT_COMMIT_TO_ZONE(const struct ovnact_ct_commit_nat *ct_commit,
+ struct ds *s)
+{
+ds_put_format(s, "ct_commit_to_zone(%s);",
+ ct_commit->dnat_zone ? "dnat" : "snat");
+
+}
+
+static void
+encode_CT_COMMIT_TO_ZONE(const struct ovnact_ct_commit_nat *ct_commit,
+ const struct ovnact_encode_params *ep,
+ struct ofpbuf *ofpacts)
+{
+struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
+ct->flags = NX_CT_F_COMMIT;
+ct->recirc_table = NX_CT_RECIRC_NONE;
+ct->zone_src.ofs = 0;
+ct->zone_src.n_bits = 16;
+
+if (ep->is_switch) {
+ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
+} else {
+ct->zone_src.field = mf_from_id(ct_commit->dnat_zone
+
[ovs-dev] [Patch ovn v3 2/2] northd: Fix direct access to SNAT network.
This change fixes bug that breaks ability of machines from external
networks, to communicate with machines in SNATed networks (specifically
when using a Distributed router).
Currently when a machine (S1) on an external network tries to talk
over TCP with a machine (A1) in a network that has enabled SNAT, the
connection is established successfully. However after the three-way
handshake, any packets that come from the A1 machine will have their
source address translated by the Distributed router, breaking the
communication.
Existing rule in `build_lrouter_out_snat_flow` that decides which
packets should be SNATed already tries to avoid SNATing packets in
reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages
in the distributed LR egress pipeline do not initiate the CT state.
Additionally we need to commit new connections that originate from
external networks into CT, so that the packets in the reply direction
(back to the external network) can be properly identified.
Rationale:
In my original RFC [0], there were questions about the motivation for
fixing this issue. I'll try to summarize why I think this is a bug
that should be fixed.
1. Current implementation for Distributed router already tries to
avoid SNATing packets in the reply direction, it's just missing
initialized CT states to make proper decisions.
2. This same scenario works with Gateway Router. I tested with
following setup:
foo -- R1 -- join - R3 -- alice
|
bar --R2
R1 is a Distributed router with SNAT for foo. R2 is a Gateway
router with SNAT for bar. R3 is a Gateway router with no SNAT.
Using 'alice1' as a client I was able to talk over TCP with
'bar1' but connection with 'foo1' failed.
3. Regarding security and "leaking" of internal IPs. Reading through
RFC 4787 [1], 5382 [2] and their update in 7857 [3], the
specifications do not seem to mandate that SNAT implementations
must filter incoming traffic destined directly to the internal
network. Sections like "5. Filtering Behavior" in 4787 and
"4.3 Externally Initiated Connections" in 5382 describe only
behavior for traffic destined to external IP/Port associated
with NAT on the device that performs NAT.
Besides, with the current implementation, it's already possible
to scan the internal network with pings and TCP syn scanning.
4. We do have customers/clouds that depend on this functionality.
This is a scenario that used to work in Openstack with ML2/OVS
and migrating those clouds to ML2/OVN would break it.
[0]https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html
[1]https://datatracker.ietf.org/doc/html/rfc4787
[2]https://datatracker.ietf.org/doc/html/rfc5382
[3]https://datatracker.ietf.org/doc/html/rfc7857
Signed-off-by: Martin Kalcok
---
northd/northd.c | 66 ---
northd/ovn-northd.8.xml | 29
tests/ovn-northd.at | 76 +
tests/system-ovn.at | 68
4 files changed, 220 insertions(+), 19 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 02cf5b234..0726c8429 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -14413,20 +14413,27 @@ build_lrouter_out_is_dnat_local(struct lflow_table
*lflows,
static void
build_lrouter_out_snat_match(struct lflow_table *lflows,
- const struct ovn_datapath *od,
- const struct nbrec_nat *nat, struct ds *match,
- bool distributed_nat, int cidr_bits, bool is_v6,
- struct ovn_port *l3dgw_port,
- struct lflow_ref *lflow_ref)
+ const struct ovn_datapath *od,
+ const struct nbrec_nat *nat,
+ struct ds *match,
+ bool distributed_nat, int cidr_bits,
+ bool is_v6,
+ struct ovn_port *l3dgw_port,
+ struct lflow_ref *lflow_ref,
+ bool is_reverse)
{
ds_clear(match);
-ds_put_format(match, "ip && ip%c.src == %s", is_v6 ? '6' : '4',
+ds_put_format(match, "ip && ip%c.%s == %s",
+ is_v6 ? '6' : '4',
+ is_reverse ? "dst" : "src",
nat->logical_ip);
if (!od->is_gw_router) {
/* Distributed router. */
-ds_put_format(match, " && outport == %s", l3dgw_port->json_key);
+ds_put_format(match, " && %s == %s",
+ is_reverse ? "inport" : "outport",
+ l3dgw_port->json_key);
if (od->n_l3dgw_ports) {
ds_put_format(match, " && is_chassis_resident(\"%s\")",
Re: [ovs-dev] [PATCH ovn v2 3/3] ofctrl: Introduce ecmp_nexthop_monitor.
> Hi Lorenzo,
Hi Mark,
thx for the review.
>
> The code looks fine to me, but I'm a bit confused by the new test.
>
> My understanding of the new feature is that each ECMP nexthop has an ID
> associated with it. This ID gets placed in the ct.label. If the ECMP route
> is removed, then we can find the associated ID, and remove the conntrack
> entry that has this ID in its label.
correct.
>
> In the test, the setup initially seems good. Alice is set up as a server,
> and Bob is set up as a client behind two routers. Bob sends traffic via R2
> and we can see the conntrack entry that ensures traffic from Alice will get
> routed to R2 instead of R3. So far so good.
>
> After this point, I'm a bit confused. I expected something like:
> * Remove the ECMP route from R1 to R2 (or alter its nexthop address)
I remove all the static routes in R1 at the end of the IPv{4,6} test and then
I check we do not have any related entry in the ct table.
$ovn-nbctl --wait=hv lr-route-del R1
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.0.1)])
Am I missing something?
I change the R2_ext mac address during the test, maybe we can get rid of the ct
flushing there, but I guess it is just a leftover of the previous test.
> * Check conntrack to ensure the existing entry is removed.
> * If possible, send new traffic from Bob to Alice and ensure that a new
> conntrack entry is created to show the updated ECMP symmetric reply.
We do it or am I missing something?
Regards,
Lorenzo
>
> Instead, the ECMP routes are never changed. Other configuration changes are
> made, and conntrack is flushed between each stage of the test. Am I missing
> something here?
>
> On 3/12/24 11:59, Lorenzo Bianconi wrote:
> > Introduce ecmp_nexthop_monitor in ovn-controller in order to track and
> > flush ecmp-symmetric reply ct entires when requested by the CMS (e.g
> > removing the related static routes).
> >
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > controller/ofctrl.c | 101 ++
> > controller/ofctrl.h | 2 +
> > controller/ovn-controller.c | 2 +
> > tests/system-ovn-kmod.at| 266
> > tests/system-ovn.at | 4 +
> > 5 files changed, 375 insertions(+)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index f14cd79a8..a10b0f567 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -388,9 +388,24 @@ struct meter_band_entry {
> > static struct shash meter_bands;
> > +static struct hmap ecmp_nexthop_map;
> > +struct ecmp_nexthop_entry {
> > +struct hmap_node node;
> > +bool erase;
> > +
> > +char *nexthop;
> > +int id;
> > +};
> > +
> > static void ofctrl_meter_bands_destroy(void);
> > static void ofctrl_meter_bands_clear(void);
> > +static void ecmp_nexthop_monitor_destroy(void);
> > +static void ecmp_nexthop_monitor_run(
> > +const struct sbrec_ecmp_nexthop_table *enh_table,
> > +struct ovs_list *msgs);
> > +
> > +
> > /* MFF_* field ID for our Geneve option. In S_TLV_TABLE_MOD_SENT, this is
> >* the option we requested (we don't know whether we obtained it yet). In
> >* S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
> > @@ -429,6 +444,7 @@ ofctrl_init(struct ovn_extend_table *group_table,
> > groups = group_table;
> > meters = meter_table;
> > shash_init(_bands);
> > +hmap_init(_nexthop_map);
> > }
> >
> > /* S_NEW, for a new connection.
> > @@ -883,6 +899,7 @@ ofctrl_destroy(void)
> > expr_symtab_destroy();
> > shash_destroy();
> > ofctrl_meter_bands_destroy();
> > +ecmp_nexthop_monitor_destroy();
> > }
> > uint64_t
> > @@ -2306,6 +2323,87 @@ add_meter(struct ovn_extend_table_info *m_desired,
> > ofctrl_meter_bands_alloc(sb_meter, m_desired, msgs);
> > }
> > +static void
> > +ecmp_nexthop_monitor_free_entry(struct ecmp_nexthop_entry *e,
> > +struct ovs_list *msgs)
> > +{
> > +if (msgs) {
> > +ovs_u128 mask = {
> > +/* ct_labels.label BITS[96-127] */
> > +.u64.hi = 0x,
> > +};
> > +uint64_t id = e->id;
> > +ovs_u128 nexthop = {
> > +.u64.hi = id << 32,
> > +};
> > +struct ofp_ct_match match = {
> > +.labels = nexthop,
> > +.labels_mask = mask,
> > +};
> > +struct ofpbuf *msg = ofp_ct_match_encode(, NULL,
> > +
> > rconn_get_version(swconn));
> > +ovs_list_push_back(msgs, >list_node);
> > +}
> > +free(e->nexthop);
> > +free(e);
> > +}
> > +
> > +static void
> > +ecmp_nexthop_monitor_destroy(void)
> > +{
> > +struct ecmp_nexthop_entry *e;
> > +HMAP_FOR_EACH_POP (e, node, _nexthop_map) {
> > +ecmp_nexthop_monitor_free_entry(e, NULL);
> > +}
> > +hmap_destroy(_nexthop_map);
> > +}
> > +
> > +static
Re: [ovs-dev] [PATCH] net: openvswitch: Check vport name
On 13 Apr 2024, at 10:48, jun.gu wrote:
> Check vport name from dev_get_by_name, this can avoid to add and remove
> NIC repeatedly when NIC rename failed at system startup.
>
> Signed-off-by: Jun Gu
> ---
> net/openvswitch/vport-netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 903537a5da22..de8977d7f329 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -78,7 +78,7 @@ struct vport *ovs_netdev_link(struct vport *vport, const
> char *name)
> int err;
>
> vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
> - if (!vport->dev) {
> + if (!vport->dev) || strcmp(name, ovs_vport_name(vport)) {
Hi Jun, not sure if I get the point here, as ovs_vport_name() translates into
vport->dev->name.
So are we trying to catch the interface rename between the dev_get_by_name(),
and the code below? This rename could happen at any other place, so this check
does not guarantee anything. Or am I missing something?
> err = -ENODEV;
> goto error_free_vport;
> }
> --
> 2.25.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
