Re: [ovs-dev] [OVN v3] OVN - Add Support for Remote Port Mirroring
On Wed, Jun 22, 2022 at 1:36 PM Abhiram R N wrote: > > Added changes in ovn-nbctl, ovn-sbctl, northd and in ovn-controller. > While Mirror creation just creates the mirror, the lsp-attach-mirror > triggers the sequence to create Mirror in OVS DB on compute node. > OVS already supports Port Mirroring. > > Note: This is targeted to mirror to destinations anywhere outside the > cluster where the analyser resides and it need not be an OVN node. > > Example commands are as below: > > Mirror creation > ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.2 > > Attach a logical port to the mirror. > ovn-nbctl lsp-attach-mirror sw0-port1 mirror1 > > Detach a source from Mirror > ovn-nbctl lsp-detach-mirror sw0-port1 mirror1 > > Mirror deletion > ovn-nbctl mirror-del mirror1 > > Co-authored-by: Veda Barrenkala > Signed-off-by: Veda Barrenkala > Signed-off-by: Abhiram R N > --- Hi Abhiram, Thanks for the v3. I did some testing of your patch. I see some issues. I created a mirror - ovn-nbctl mirror-add m1 gre 0 to-lport 192.168.1.12 and also attached it to a logical port - sw0-port1 Issue 1. After that I ran - ovn-nbctl set mirror . sink=192.168.1.13 and I don't see the SB mirror table getting updated with the new mirror value. Issue 2: ovn-controller crashed in my testing once. From the logs I see it crashed while executing ovn_port_mirror_run(). I could not get the core dump though. Here are some logs with json rpc dbg logs enabled - https://paste.openstack.org/show/blc7KJr2eChGLjjpan8x/ Issue 3. Let's say mirror m1 is attached to logical ports sw0-port1 and sw0-port2. When the ovn-controller releases these port bindings due to the ovs ports getting deleted, then the gre interface is not deleted by ovn-controller and the mirror row in the local ovs database is also not deleted. I'd suggest covering all these scenarios in the test cases. > V2 --> V3 : Addressed review comments (as below) > i) Avoided big 'if' and added chassis check > ii) Avoided loop to get interface table >iii) Edited "run" function to do as suggested. > iv) Addressed the ovn-performance.at issue. > > controller/automake.mk | 4 +- > controller/mirror.c | 382 > controller/mirror.h | 53 + > controller/ovn-controller.c | 220 - > northd/en-northd.c | 4 + > northd/inc-proc-northd.c| 4 + > northd/northd.c | 118 +++ > northd/northd.h | 2 + > ovn-nb.ovsschema| 31 ++- > ovn-nb.xml | 57 ++ > ovn-sb.ovsschema| 23 ++- > ovn-sb.xml | 46 + > tests/ovn-nbctl.at | 80 > tests/ovn.at| 155 +++ > utilities/ovn-nbctl.c | 345 > utilities/ovn-sbctl.c | 4 + > 16 files changed, 1522 insertions(+), 6 deletions(-) > create mode 100644 controller/mirror.c > create mode 100644 controller/mirror.h > > diff --git a/controller/automake.mk b/controller/automake.mk > index c2ab1bbe6..334672b4d 100644 > --- a/controller/automake.mk > +++ b/controller/automake.mk > @@ -41,7 +41,9 @@ controller_ovn_controller_SOURCES = \ > controller/ovsport.h \ > controller/ovsport.c \ > controller/vif-plug.h \ > - controller/vif-plug.c > + controller/vif-plug.c \ > + controller/mirror.h \ > + controller/mirror.c > > controller_ovn_controller_LDADD = lib/libovn.la > $(OVS_LIBDIR)/libopenvswitch.la > man_MANS += controller/ovn-controller.8 > diff --git a/controller/mirror.c b/controller/mirror.c > new file mode 100644 > index 0..1c96d490a > --- /dev/null > +++ b/controller/mirror.c > @@ -0,0 +1,382 @@ > +/* Copyright (c) 2022 Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include > +#include > + > +/* library headers */ > +#include "lib/sset.h" > +#include "lib/util.h" > + > +/* OVS includes. */ > +#include "lib/vswitch-idl.h" > +#include "openvswitch/vlog.h" > + > +/* OVN includes. */ > +#include "binding.h" > +#include "lib/ovn-sb-idl.h" > +#include "mirror.h" > + > +VLOG_DEFINE_THIS_MODULE(port_mirror); > + > +/* Static function declarations */ > + > +static const struct ovsrec_port * > +get_port_for_iface(const struct ovsrec_interface *iface, > + const struct
[ovs-dev] [RFC ovn] Multi-chassis port + MTU behavior
Hi folks, looking for some advices on MTU / IP behavior for multi-chassis ports. 22.06 got the new multichassis port concept introduced where the same port may be present on multiple chassis, which can be used as a performance optimization for VM live migration, among other things. Migration with sub-0.1ms network downtime is achieved by cloning all traffic directed towards a multichassis port to all its locations, making all chassis hosting it receive relevant traffic at any given moment until migration is complete. For logical switches with a localnet port where traffic usually goes through the localnet port and not tunnels, it means enforcement of tunneling of egress and ingress traffic for a multichassis port. (The rest of the traffic between other, non-multichassis, ports keeps going through localnet port.) Tunneling enforcement is done because traffic sent through localnet port won't generally be cloned to all port binding locations by the upstream switch. A problem comes down when ingress or egress traffic of a multichassis port, being redirected through a tunnel, gets lost because of geneve header overhead or because the interface used for tunneling has a different MTU from the physical bridge backing up the localnet port. This is happening when: - tunnel_iface_mtu < localnet_mtu + geneve_overhead This makes for an unfortunate situation where, for a multichassis port, SOME traffic (e.g. regular ICMP requests) pass through without any problems, while OTHER traffic (e.g. produced with 'ping -s) doesn't. (Test scenario demonstrating it is included below.) Here are ideas I came up with on how this could be resolved or at least mitigated: a) pass "oversized" traffic through localnet, and the rest through tunnels. Apart from confusing pattern on the wire where packets that belong to the same TCP session may go through two paths (arguably not a big problem and should be expected by other network hardware), this gets us back to the problem of upstream switch not delivering packets to all binding chassis. b) fragmentation needed. We could send ICMP Fragmentation Needed ICMP errors on attempts to send oversized packets to or from a multichassis port. Then TCP sessions could adjust their properties to reflect the new recommended MTU. We already have a similar mechanism for router ports. There are several caveats and limitations with fragmentation needed (b): - AFAIU this works for some protocols but not others. It also depends on the client network stack to reflect the change in network path. TCP should probably work. - It may be confusing for users that some L2 paths between ports of the same switch have reduced MTU while others have the regular, depending on the type of a port (single- or multi-chassis). c) implement fragmentation inside L2 domain. I actually don't know if that's even compliant with RFCs. Usually packet fragmentation is implemented on L2 domain boundary, by a router. In this scenario, a peer port on the same switch would receive fragments for a packet that was sent as a single piece. I currently lean towards (b) though it's not a universal fix since it requires collaboration of the underlying network stack. But (a) leaves cloning broken, and (c) is even more invasive, taking action on port's packets (fragmenting them) without the port's owner knowledge. Perhaps this is all unnecessary and there's a way to make OVN transparently split and reassemble packets as needed, though I doubt it since it doesn't encapsulate tunneled traffic into another application layer. But if there's a way to achieve transparent re-assemble, or there are other alternatives beyond (a)-(c), let me know. Please let me know what you think of (a)-(c) regardless. Thanks, Ihar --- tests/ovn.at | 92 1 file changed, 92 insertions(+) diff --git a/tests/ovn.at b/tests/ovn.at index c346975e6..4ec1d37c3 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14966,6 +14966,98 @@ OVN_CLEANUP([hv1],[hv2],[hv3]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([localnet connectivity with multiple requested-chassis, max mtu]) +AT_KEYWORDS([multi-chassis]) +ovn_start + +net_add n1 +for i in 1 2; do +sim_add hv$i +as hv$i +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.$i +check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys +done + +check ovn-nbctl ls-add ls0 +check ovn-nbctl lsp-add ls0 first +check ovn-nbctl lsp-add ls0 second +check ovn-nbctl lsp-add ls0 migrator +check ovn-nbctl lsp-set-addresses first "00:00:00:00:00:01 10.0.0.1" +check ovn-nbctl lsp-set-addresses second "00:00:00:00:00:02 10.0.0.2" +check ovn-nbctl lsp-set-addresses migrator "00:00:00:00:00:ff 10.0.0.100" + +check ovn-nbctl lsp-add ls0 public +check ovn-nbctl lsp-set-type public localnet +check ovn-nbctl lsp-set-addresses public unknown +check ovn-nbctl lsp-set-options public network_name=phys + +check ovn-nbctl lsp-set-options
Re: [ovs-dev] [PATCH v4 9/9] debian: Add option to build without DPDK.
On 7/15/2022 4:01 AM, Ilya Maximets wrote:
On 7/15/22 10:53, Ilya Maximets wrote:
On 7/15/22 05:04, Gregory Rose wrote:
On 7/14/2022 6:45 PM, Ilya Maximets wrote:
On 7/14/22 17:55, Frode Nordahl wrote:
From: Ilya Maximets
Signed-off-by: Ilya Maximets
Co-authored-by: Frode Nordahl
Signed-off-by: Frode Nordahl
---
.ci/linux-build.sh | 6 ++-
.github/workflows/build-and-test.yml | 12 +-
.gitignore | 1 +
Documentation/intro/install/debian.rst | 27 ++--
debian/.gitignore | 21 +
debian/automake.mk | 22 +-
debian/clean | 4 +-
debian/{control => control.in} | 60 +-
debian/rules | 6 +++
9 files changed, 110 insertions(+), 49 deletions(-)
create mode 100644 debian/.gitignore
rename debian/{control => control.in} (85%)
diff --git a/debian/automake.mk b/debian/automake.mk
index a3c2d7289..2f94c2999 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -3,6 +3,7 @@ EXTRA_DIST += \
debian/changelog \
debian/clean \
debian/control \
+ debian/control.in \
debian/copyright \
debian/copyright.in \
debian/dirs \
@@ -88,4 +89,23 @@ $(srcdir)/debian/copyright: AUTHORS.rst debian/copyright.in
sed -e '1,/%AUTHORS%/d' $(srcdir)/debian/copyright.in; \
} > $@
-DISTCLEANFILES += debian/copyright
+$(srcdir)/debian/control: debian/control.in config.h
+if DPDK_NETDEV
+ sed -e 's/^# DPDK_NETDEV //' \
+ < $(srcdir)/debian/control.in > $(srcdir)/debian/control
+else
+ grep -v "^# DPDK_NETDEV" \
+ < $(srcdir)/debian/control.in > $(srcdir)/debian/control
+endif
+
+debian: $(srcdir)/debian/copyright $(srcdir)/debian/control
+.PHONY: debian
+
+
+debian-deb: debian
+ make distclean
+if DPDK_NETDEV
+ DEB_BUILD_OPTIONS="nocheck parallel=`nproc`" fakeroot debian/rules binary
+else
+ DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" fakeroot debian/rules
binary
+endif
There are several chicken-and-egg problems with this one patch:
- Since targets for control and copyright files are trying to
re-write the file in $(srcdir), the distcheck is failing,
because during distcheck, the source directory is read-only.
- Since control file has a dependency on config.h, it is getting
re-built during distcheck. copyright is not, that's why
distcheck is not failing without this patch. But we do need
a dependency for control, otherwise it will not be re-built
after re-configuration.
- Since control and copyright are auto-generated files, they
technically should not be part of the distribution. If we will
remove them from EXTRA_DIST and add to CLEANFILES, they will
be cleaned up before packing the source archive and that fixes
the distcheck, IIRC (It's been a few hours of experiments, so
I don't really remember if that fixed the distcheck).
- However, 'debian/rules binary' requires the source directory
to not be configured, so the distclean. But distclean removes
generated control and copyright. We could copy them, distclean
and copy back, but...
- 'make distcean' is really not enough. Most of the information
about the previous build of debian packages is preserved, so
some old directories are getting re-used all the time and
packages are not really what they are supposed to be. We have
to call 'debian/rules clean', but that will execute distclean
and fail on removed control file...
- If we'll add control and copyright back to the distribution,
but will allow their generation in the build directory instead
of a source directory, that helps with distcheck a bit, but
fails at distcleancheck, because files are not cleaned.
- And you need dpdk-dev already installed for ./configure
--with-dpdk=shared to succeed.
So, after lots of experiments the only solution that appears to
work seems to be following:
- Remove control and copyright from the distribution.
- Add them to CLEANFILES
- Create files in build directory, not source.
- In the debian-deb target, call distclean first, then re-generate
both control and copyright, then call 'debian/rules clean'.
It won't call distclean again, because the directory is not
configured after the first distclean.
- Install libdpdk-dev beforehand in GHA.
- Add the builddir sanity check as 'make debian-deb' doesn't
make a lot of sense if not building from the source directory.
With that schema, by the time we're calling 'debian/rules binary'
we have all files in place. And if they are getting generated
during distcheck, it will be possible to create them and they
will be correctly cleaned up in the process. Also, we're always
performing a clean build, so no need for the documentation note
about 'debian/rules clean'. And I got a green build in
Re: [ovs-dev] [PATCH v5] ovsdb idl: Add the support to specify the uuid for row insert.
Bleep bloop. Greetings Numan Siddique, 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 108 characters long (recommended limit is 79) #119 FILE: lib/db-ctl-base.man:206: .IP "[\fB\-\-id=(@\fIname\fR | \fIuuid\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..." WARNING: Line is 174 characters long (recommended limit is 79) #142 FILE: lib/db-ctl-base.xml:313: [--id=(@name|uuid)] create table column[:key]=value... Lines checked: 492, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] ovsdb idl: Add the support to specify the uuid for row insert.
On Tue, Jul 12, 2022 at 6:02 AM Ilya Maximets wrote:
>
> On 6/29/22 18:57, num...@ovn.org wrote:
> > From: Numan Siddique
> >
> > ovsdb-server allows the OVSDB clients to specify the uuid for
> > the row inserts [1]. The C IDL client library is missing this
> > feature. This patch adds this support.
> >
> > For each schema table, a new function is generated -
> > insert_persistent_uuid(txn, uuid) and the users
> > of IDL client library can make use of this function.
> >
> > ovs-vsctl and other derivatives of ctl now supports the same
> > in the generic 'create' command with the option "--id=".
> >
> > [1] - a529e3cd1f("ovsdb-server: Allow OVSDB clients to specify the UUID for
> > inserted rows.:)
> >
> > Signed-off-by: Numan Siddique
> > Acked-by: Adrian Moreno
> > Acked-by: Han Zhou
> > ---
>
> Hi, Numan. Thanks for the patch. It looks OK in general,
> see some comments inline.
Thanks for the review comments.
>
> >
> > v3 -> v4
> > ---
> > * Added an entry in python/TODO.rst.
> >
> > v2 -> v3
> >
> > * Addressed review comments from Han
> > - Added test case for --id ctl option
> >
> > v1 -> v2
> > -
> > * Addressed review comments from Adrian Moreno
> > * Added the support in generic 'create' command to specify the uuid in
> > --id option.
> >
> >
> > lib/db-ctl-base.c| 38 --
> > lib/db-ctl-base.man | 5 ++-
> > lib/db-ctl-base.xml | 4 ++
> > lib/ovsdb-idl-provider.h | 1 +
> > lib/ovsdb-idl.c | 86 +---
> > lib/ovsdb-idl.h | 3 ++
> > ovsdb/ovsdb-idlc.in | 15 +++
> > python/TODO.rst | 2 +
> > tests/ovs-vsctl.at | 25
> > tests/ovsdb-idl.at | 27 +
> > tests/test-ovsdb.c | 59 +++
> > 11 files changed, 229 insertions(+), 36 deletions(-)
>
> We need a NEWS entry for this.
>
> >
> > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> > index 707456158..f39e090a0 100644
> > --- a/lib/db-ctl-base.c
> > +++ b/lib/db-ctl-base.c
> > @@ -1732,28 +1732,42 @@ cmd_create(struct ctl_context *ctx)
> > const struct ovsdb_idl_row *row;
> > const struct uuid *uuid = NULL;
> > int i;
> > +struct uuid uuid_;
> > +bool persist_uuid = false;
>
> Reverse x-mass tree, please.
Done in v5
>
> >
> > ctx->error = get_table(table_name, );
> > if (ctx->error) {
> > return;
> > }
> > +
> > if (id) {
> > -struct ovsdb_symbol *symbol = NULL;
> > +if (uuid_from_string(_, id)) {
> > +uuid = _;
> > +persist_uuid = true;
> > +} else {
> > +struct ovsdb_symbol *symbol = NULL;
> >
> > -ctx->error = create_symbol(ctx->symtab, id, , NULL);
> > -if (ctx->error) {
> > -return;
> > -}
> > -if (table->is_root) {
> > -/* This table is in the root set, meaning that rows created in
> > it
> > - * won't disappear even if they are unreferenced, so disable
> > - * warnings about that by pretending that there is a
> > reference. */
> > -symbol->strong_ref = true;
> > +ctx->error = create_symbol(ctx->symtab, id, , NULL);
> > +if (ctx->error) {
> > +return;
> > +}
> > +if (table->is_root) {
> > +/* This table is in the root set, meaning that rows
> > created in
> > +* it won't disappear even if they are unreferenced, so
> > disable
> > +* warnings about that by pretending that there is a
> > +* reference. */
>
> Subsequent lines of a comment block are 1 spaece off.
Done in v5.
>
> > +symbol->strong_ref = true;
> > +}
> > +uuid = >uuid;
> > }
> > -uuid = >uuid;
> > }
> >
> > -row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
> > +if (persist_uuid) {
> > +row = ovsdb_idl_txn_insert_persist_uuid(ctx->txn, table, uuid);
> > +} else {
> > +row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
> > +}
> > +
> > for (i = 2; i < ctx->argc; i++) {
> > ctx->error = set_column(table, row, ctx->argv[i], ctx->symtab);
> > if (ctx->error) {
> > diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man
> > index 9c786f298..4dc165f94 100644
> > --- a/lib/db-ctl-base.man
> > +++ b/lib/db-ctl-base.man
> > @@ -203,7 +203,7 @@ Without \fB\-\-if-exists\fR, it is an error if
> > \fIrecord\fR does not
> > exist. With \fB\-\-if-exists\fR, this command does nothing if
> > \fIrecord\fR does not exist.
> > .
> > -.IP "[\fB\-\-id=@\fIname\fR] \fBcreate\fR \fItable
> > column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
> > +.IP "[\fB\-\-id=@\fIname\fR or \fB\-\-id=\fIuuid\fR] \fBcreate\fR \fItable
> > column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
>
> I think, it should be [--id=(@name|uuid)]
Ack. Done in v5.
>
> >
[ovs-dev] [PATCH v5] ovsdb idl: Add the support to specify the uuid for row insert.
From: Numan Siddique
ovsdb-server allows the OVSDB clients to specify the uuid for
the row inserts [1]. The C IDL client library is missing this
feature. This patch adds this support.
For each schema table, a new function is generated -
insert_persistent_uuid(txn, uuid) and the users
of IDL client library can make use of this function.
ovs-vsctl and other derivatives of ctl now supports the same
in the generic 'create' command with the option "--id=".
[1] - a529e3cd1f("ovsdb-server: Allow OVSDB clients to specify the UUID for
inserted rows.:)
Signed-off-by: Numan Siddique
Acked-by: Adrian Moreno
Acked-by: Han Zhou
---
v4 -> v5
---
* Addressed review comments from Ilya.
- Added NEWS item entry.
v3 -> v4
---
* Added an entry in python/TODO.rst.
v2 -> v3
* Addressed review comments from Han
- Added test case for --id ctl option
v1 -> v2
-
* Addressed review comments from Adrian Moreno
* Added the support in generic 'create' command to specify the uuid in
--id option.
NEWS | 3 +-
lib/db-ctl-base.c| 38 --
lib/db-ctl-base.man | 5 ++-
lib/db-ctl-base.xml | 6 ++-
lib/ovsdb-idl-provider.h | 1 +
lib/ovsdb-idl.c | 85 +---
lib/ovsdb-idl.h | 3 ++
ovsdb/ovsdb-idlc.in | 15 +++
python/TODO.rst | 2 +
tests/ovs-vsctl.at | 25
tests/ovsdb-idl.at | 27 +
tests/test-ovsdb.c | 59
12 files changed, 231 insertions(+), 38 deletions(-)
diff --git a/NEWS b/NEWS
index 024fa4428..41316da14 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
Post-v3.0.0
-
+ - OVSDB-IDL:
+ * Add the support to specify the uuid for row insert.
v3.0.0 - xx xxx
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index bc85e9921..856832a04 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1731,29 +1731,43 @@ cmd_create(struct ctl_context *ctx)
const struct ovsdb_idl_table_class *table;
const struct ovsdb_idl_row *row;
const struct uuid *uuid = NULL;
+bool persist_uuid = false;
+struct uuid uuid_;
int i;
ctx->error = get_table(table_name, );
if (ctx->error) {
return;
}
+
if (id) {
-struct ovsdb_symbol *symbol = NULL;
+if (uuid_from_string(_, id)) {
+uuid = _;
+persist_uuid = true;
+} else {
+struct ovsdb_symbol *symbol = NULL;
-ctx->error = create_symbol(ctx->symtab, id, , NULL);
-if (ctx->error) {
-return;
-}
-if (table->is_root) {
-/* This table is in the root set, meaning that rows created in it
- * won't disappear even if they are unreferenced, so disable
- * warnings about that by pretending that there is a reference. */
-symbol->strong_ref = true;
+ctx->error = create_symbol(ctx->symtab, id, , NULL);
+if (ctx->error) {
+return;
+}
+if (table->is_root) {
+/* This table is in the root set, meaning that rows created in
+ * it won't disappear even if they are unreferenced, so disable
+ * warnings about that by pretending that there is a
+ * reference. */
+symbol->strong_ref = true;
+}
+uuid = >uuid;
}
-uuid = >uuid;
}
-row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
+if (persist_uuid) {
+row = ovsdb_idl_txn_insert_persist_uuid(ctx->txn, table, uuid);
+} else {
+row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
+}
+
for (i = 2; i < ctx->argc; i++) {
ctx->error = set_column(table, row, ctx->argv[i], ctx->symtab);
if (ctx->error) {
diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man
index a529d8b4d..c8111c9ef 100644
--- a/lib/db-ctl-base.man
+++ b/lib/db-ctl-base.man
@@ -203,7 +203,7 @@ Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR
does not
exist. With \fB\-\-if-exists\fR, this command does nothing if
\fIrecord\fR does not exist.
.
-.IP "[\fB\-\-id=@\fIname\fR] \fBcreate\fR \fItable
column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
+.IP "[\fB\-\-id=(@\fIname\fR | \fIuuid\fR] \fBcreate\fR \fItable
column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
Creates a new record in \fItable\fR and sets the initial values of
each \fIcolumn\fR. Columns not explicitly set will receive their
default values. Outputs the UUID of the new row.
@@ -212,6 +212,9 @@ If \fB@\fIname\fR is specified, then the UUID for the new
row may be
referred to by that name elsewhere in the same \fB\*(PN\fR
invocation in contexts where a UUID is expected. Such references may
precede or follow the \fBcreate\fR command.
+.IP
+If a valid \fIuuid\fR is specified, then it is used as the UUID
+of the new
Re: [ovs-dev] [ovs-build] |fail| pw1656877 [ovs-dev, 2/2] Prepare for post-3.0.0 (3.0.90).
On 7/15/22 20:33, Ilya Maximets wrote: > On 7/15/22 20:24, Stokes, Ian wrote: >>> Looks like new MTU tests are unstable. >>> Ian, Michael, could you, please, check? >>> >>> Best regards, Ilya Maximets. >> >> Hey Ilya, >> >> Let me take a look at this, it passed our internal CI so not sure why it's >> failed externally, worst case I can revert this as it was really just a nice >> to have, if it's holding up the branching would you prefer this? > > No, it's not holding up branching. Just wanted to highlight the > issue. Would be great to fix it in a near future, but it's not > critical for right now. Thanks. I think, the tests are lacking the 'Clean up the testpmd now' part at least. If testpmd is still running while we're tearing down OVS, there is a fair chance that some packets will be received while the OpenFlow port is already removed. That migth explain some of the failures. Not sure if that what happened in this particular case thouhg. Also, looksing at other failure reports I see some weird things like: Unreasonably long 1140ms poll interval (408ms user, 263ms system) 408 + 263 != 1140. The main thread got preempted for half a second in this case. Are you running something else on the same cores on that system? Or maybe some processes are not getting stopped properly after the test? > > Best regards, Ilya Maximets. > >> >> Thanks >> Ian >>> >>> On 7/15/22 18:28, ovs_jenk...@intel.com wrote: Test-Label: intel-ovs-compilation Test-Status: fail http://patchwork.ozlabs.org/api/patches/1656877/ AVX-512_compilation: failed DPLCS Test: success DPIF Test: success MFEX Test: fail Errors in DPCLS test: None Errors in DPIF test: None Errors in MFEX test: > 2022-07-15T16:27:27.850Z|00046|coverage|INFO|stream_open >>> 0.0/sec 0.000/sec0./sec total: 1 > 2022-07-15T16:27:27.850Z|00047|coverage|INFO|util_xalloc >>> 0.0/sec 0.000/sec0./sec total: 8198 > 2022-07-15T16:27:27.850Z|00048|coverage|INFO|netlink_received >>> 0.0/sec 0.000/sec0./sec total: 29 > 2022-07-15T16:27:27.850Z|00049|coverage|INFO|netlink_recv_jumbo >>> 0.0/sec 0.000/sec0./sec total: 3 > 2022-07-15T16:27:27.850Z|00050|coverage|INFO|netlink_sent >>> 0.0/sec 0.000/sec0./sec total: 27 > 2022-07-15T16:27:27.850Z|00051|coverage|INFO|134 events never >>> hit > 2022-07-15T16:27:27.851Z|00052|bridge|INFO|ovs-vswitchd (Open >>> vSwitch) 3.0.90 > 2022-07-15T16:27:27.957Z|00053|pmd_perf|INFO|DPDK provided TSC >>> frequency: 240 KHz > 2022-07-15T16:27:27.958Z|00054|dpif_netdev_extract|INFO|Default >>> MFEX Extract implementation is autovalidator. > 2022-07-15T16:27:27.969Z|00055|dpif_netdev_impl|INFO|Default >>> DPIF implementation is dpif_scalar. > 2022-07-15T16:27:27.982Z|00056|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports recirculation > 2022-07-15T16:27:27.982Z|00057|ofproto_dpif|INFO|netdev@ovs- >>> netdev: VLAN header stack length probed as 1 > 2022-07-15T16:27:27.982Z|00058|ofproto_dpif|INFO|netdev@ovs- >>> netdev: MPLS label stack length probed as 3 > 2022-07-15T16:27:27.982Z|00059|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports truncate action > 2022-07-15T16:27:27.982Z|00060|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports unique flow ids > 2022-07-15T16:27:27.982Z|00061|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports clone action > 2022-07-15T16:27:27.982Z|00062|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Max sample nesting level probed as 10 > 2022-07-15T16:27:27.982Z|00063|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports eventmask in conntrack action > 2022-07-15T16:27:27.982Z|00064|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_clear action > 2022-07-15T16:27:27.982Z|00065|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Max dp_hash algorithm probed to be 1 > 2022-07-15T16:27:27.982Z|00066|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports check_pkt_len action > 2022-07-15T16:27:27.982Z|00067|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports timeout policy in conntrack action > 2022-07-15T16:27:27.982Z|00068|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_zero_snat > 2022-07-15T16:27:27.983Z|00069|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports add_mpls action > 2022-07-15T16:27:27.983Z|00070|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_state > 2022-07-15T16:27:27.983Z|00071|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_zone > 2022-07-15T16:27:27.983Z|00072|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_mark > 2022-07-15T16:27:27.983Z|00073|ofproto_dpif|INFO|netdev@ovs- >>> netdev: Datapath supports ct_label > 2022-07-15T16:27:27.983Z|00074|ofproto_dpif|INFO|netdev@ovs- >>> netdev:
Re: [ovs-dev] [v12 00/10] Actions Infrastructure + Optimizations
On 18 Jul 2022, at 14:49, Finn, Emma wrote: >> -Original Message- >> From: Eelco Chaudron >> Sent: Monday 18 July 2022 10:41 >> To: Stokes, Ian ; Finn, Emma >> Cc: d...@openvswitch.org; Van Haaren, Harry ; >> Amber, Kumar ; i.maxim...@ovn.org >> Subject: Re: [v12 00/10] Actions Infrastructure + Optimizations >> >> >> >> On 15 Jul 2022, at 13:39, Stokes, Ian wrote: >> > > This series also introduces optimized versions of the following actions: - push_vlan - pop_vlan - set_masked eth - set_masked ipv4 Below is a table indicating some relative performance benefits for these actions. +---+---+-+ | Actions | Scalar with series|AVX with series | +---+---+-+ | mod_dl_dst| 1.01x |1.13x | +---+---+-+ | push_vlan | 1.01x |1.10x | +---+---+-+ | strip_vlan| 1.01x |1.11x | +---+---+-+ | mod_ipv4 1 x field| 1.01x |1.02x | +---+---+-+ | mod_ipv4 4 x fields | 1.01x |1.21x | +---+---+-+ | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x |1.36x | +---+---+-+ >>> >>> Thanks all for the work/reviews on this, given that it has been under >> discussion prior to the soft freeze I have merged this to master. >> >> Thanks, still would like some answers on the performance numbers. >> > > > Hi Eelco, > > These performance numbers were updated on the cover letter for the v8 of this > series as this was when some changes were made to the AVX code. > I didn't re run on v9/10/11/12. > However, I just re ran on the v12 and checked a few of actions (mod_dl_dst, > strip_vlan, mod_ipv4 4 x fields, strip_vlan + mod_dl_dst + mod_ipv4 4 x > fields), and compared with the v8 data to ensure consistency. > Performance results are < 1% delta from the v8 posted data. Thanks Emma for the update on the performance numbers. So < 1% sounds good to me. //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [v12 00/10] Actions Infrastructure + Optimizations
> -Original Message- > From: Eelco Chaudron > Sent: Monday 18 July 2022 10:41 > To: Stokes, Ian ; Finn, Emma > Cc: d...@openvswitch.org; Van Haaren, Harry ; > Amber, Kumar ; i.maxim...@ovn.org > Subject: Re: [v12 00/10] Actions Infrastructure + Optimizations > > > > On 15 Jul 2022, at 13:39, Stokes, Ian wrote: > > >> This series also introduces optimized versions of the following > >> actions: > >> - push_vlan > >> - pop_vlan > >> - set_masked eth > >> - set_masked ipv4 > >> Below is a table indicating some relative performance benefits for > >> these actions. > >> +---+---+-+ > >> | Actions | Scalar with series|AVX > >> with series | > >> +---+---+-+ > >> | mod_dl_dst| 1.01x |1.13x > >> | > >> +---+---+-+ > >> | push_vlan | 1.01x |1.10x > >> | > >> +---+---+-+ > >> | strip_vlan| 1.01x |1.11x > >> | > >> +---+---+-+ > >> | mod_ipv4 1 x field| 1.01x |1.02x > >> | > >> +---+---+-+ > >> | mod_ipv4 4 x fields | 1.01x |1.21x > >> | > >> +---+---+-+ > >> | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x |1.36x > >> | > >> +---+---+-+ > >> > > > > Thanks all for the work/reviews on this, given that it has been under > discussion prior to the soft freeze I have merged this to master. > > Thanks, still would like some answers on the performance numbers. > Hi Eelco, These performance numbers were updated on the cover letter for the v8 of this series as this was when some changes were made to the AVX code. I didn't re run on v9/10/11/12. However, I just re ran on the v12 and checked a few of actions (mod_dl_dst, strip_vlan, mod_ipv4 4 x fields, strip_vlan + mod_dl_dst + mod_ipv4 4 x fields), and compared with the v8 data to ensure consistency. Performance results are < 1% delta from the v8 posted data. Thanks, Emma ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 ovn] lflow: fix possible use-after-free in add_lb_vip_hairpin_reply_action
On 7/16/22 08:45, Lorenzo Bianconi wrote:
> ofpbuf_put_zeros routine can rellocate the buffer if the requested size
> is bigger than buffer tailroom. Reload ol pointer before running
> ofpact_finish_LEARN in order to avoid a possible use-after-free in
> add_lb_vip_hairpin_reply_action routine.
>
> Fixes: 022ea339c8 ("lflow: Use learn() action to generate LB hairpin reply
> flows.")
> Signed-off-by: Lorenzo Bianconi
> ---
> Changes since v1:
> - rely on ofpbuf_at_assert in order to not overwrite possible former actions
> ---
Looks good to me, thanks!
Acked-by: Dumitru Ceara
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ci: Prefer pip3 to install unit test dependencies
While it looks like the right python3 versions of those dependencies
seems to be installed in the CI, prefer calling this via pip3 like the
rest of the script.
Fixes: 445dceb88461 ("python: Introduce unit tests.")
Signed-off-by: David Marchand
---
.ci/linux-prepare.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 1698a0713b..16a7aec0b5 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -43,7 +43,7 @@ if [ "$M32" ]; then
fi
# Install python test dependencies
-pip install -r python/test_requirements.txt
+pip3 install -r python/test_requirements.txt
# IPv6 is supported by kernel but disabled in TravisCI images:
# https://github.com/travis-ci/travis-ci/issues/8891
--
2.36.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 13/17] python: introduce unit tests
On 7/17/22 16:07, David Marchand wrote:
> On Fri, Jul 8, 2022 at 8:09 PM Adrian Moreno wrote:
>> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
>> index a0635cf56..de741c6b4 100755
>> --- a/.ci/linux-prepare.sh
>> +++ b/.ci/linux-prepare.sh
>> @@ -45,6 +45,9 @@ if [ "$M32" ]; then
>> sudo apt-get install -y $pkgs
>> fi
>>
>> +# Install python test dependencies
>> +pip install -r python/test_requirements.txt
>> +
>
> Should it be pip3?
>
> We still run Ubuntu 18.04, and "pip" points at python2 pip.
> At some point, we preferred pip3 (24e697080948 ("travis: Use pip3 to
> install the python packages on linux")).
GHA seems to do a right thing and install correct py3 versions of
packages. Maybe pyenv integration is better there?
But I agree that it's better to be consistent and use pip3.
Feel free to submit a patch.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb: Remove extra make target dependency for local-config.5.
On 7/15/22 18:04, Ilya Maximets wrote:
> ovsdb/ directory should not be a dependency, otherwise the man
> page is getting re-built every time unrelated files are changed.
>
> Fixes: 6f24c2bc769a ("ovsdb: Add Local_Config schema.")
> Signed-off-by: Ilya Maximets
> ---
Acked-by: Dumitru Ceara
Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 2/2] python-c-ext: fix a couple of build warnings
ovs/_json.c:67:20: warning: assignment discards ‘const’ qualifier from pointer
target type [-Wdiscarded-qualifiers]
ovs/_json.c:132:27: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
Signed-off-by: Timothy Redaelli
---
python/ovs/_json.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/ovs/_json.c b/python/ovs/_json.c
index 80bf9dea3..0b980038b 100644
--- a/python/ovs/_json.c
+++ b/python/ovs/_json.c
@@ -50,7 +50,7 @@ Parser_feed(json_ParserObject * self, PyObject * args)
Py_ssize_t input_sz;
PyObject *input;
size_t rd;
-char *input_str;
+const char *input_str;
if (self->_parser == NULL) {
return NULL;
@@ -111,7 +111,7 @@ json_to_python(struct json *json)
return dict;
}
case JSON_ARRAY:{
-int i;
+size_t i;
PyObject *arr = PyList_New(json->array.n);
if (arr == NULL) {
--
2.36.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 0/2] fix a couple of build warnings
v1 -> v2: - Removed Python 2 support as requested by Ilya Timothy Redaelli (2): python-c-ext: Remove Python 2 support python-c-ext: fix a couple of build warnings python/ovs/_json.c | 38 +++--- 1 file changed, 3 insertions(+), 35 deletions(-) -- 2.36.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 1/2] python-c-ext: Remove Python 2 support
Since Python 2 is not supported anymore, remove Python 2 support from C
extension too
Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
Signed-off-by: Timothy Redaelli
---
python/ovs/_json.c | 34 +-
1 file changed, 1 insertion(+), 33 deletions(-)
diff --git a/python/ovs/_json.c b/python/ovs/_json.c
index ef7bb4b8e..80bf9dea3 100644
--- a/python/ovs/_json.c
+++ b/python/ovs/_json.c
@@ -2,10 +2,6 @@
#include
#include "structmember.h"
-#if PY_MAJOR_VERSION >= 3
-#define IS_PY3K
-#endif
-
typedef struct {
PyObject_HEAD
struct json_parser *_parser;
@@ -63,21 +59,13 @@ Parser_feed(json_ParserObject * self, PyObject * args)
if (!PyArg_UnpackTuple(args, "input", 1, 1, )) {
return NULL;
}
-#ifdef IS_PY3K
if ((input_str = PyUnicode_AsUTF8AndSize(input, _sz)) == NULL) {
-#else
-if (PyString_AsStringAndSize(input, _str, _sz) < 0) {
-#endif
return NULL;
}
rd = json_parser_feed(self->_parser, input_str, (size_t) input_sz);
-#ifdef IS_PY3K
return PyLong_FromSize_t(rd);
-#else
-return PyInt_FromSize_t(rd);
-#endif
}
static PyObject *
@@ -144,11 +132,7 @@ json_to_python(struct json *json)
return PyFloat_FromDouble(json->real);
} /* fall through to treat 0 as int */
case JSON_INTEGER:
-#ifdef IS_PY3K
return PyLong_FromLong((long) json->integer);
-#else
-return PyInt_FromLong((long) json->integer);
-#endif
case JSON_STRING:
return PyUnicode_FromString(json->string);
@@ -225,7 +209,6 @@ static PyTypeObject json_ParserType = {
Parser_new, /* tp_new */
};
-#ifdef IS_PY3K
static struct PyModuleDef moduledef = {
PyModuleDef_HEAD_INIT,
"ovs._json",/* m_name */
@@ -238,32 +221,17 @@ static struct PyModuleDef moduledef = {
0, /* m_free */
};
-#define INITERROR return NULL
-#else /* !IS_PY3K */
-#define INITERROR return
-#endif
-
PyMODINIT_FUNC
-#ifdef IS_PY3K
PyInit__json(void)
-#else
-init_json(void)
-#endif
{
PyObject *m;
if (PyType_Ready(_ParserType) < 0) {
-INITERROR;
+return NULL;
}
-#ifdef IS_PY3K
m = PyModule_Create();
-#else
-m = Py_InitModule3("ovs._json", NULL, "OVS JSON Parser module");
-#endif
Py_INCREF(_ParserType);
PyModule_AddObject(m, "Parser", (PyObject *) & json_ParserType);
-#ifdef IS_PY3K
return m;
-#endif
}
--
2.36.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [v12 00/10] Actions Infrastructure + Optimizations
On 15 Jul 2022, at 13:39, Stokes, Ian wrote: >> This patchset introduces actions infrastructure changes which allows >> the user to choose between different action implementations based on >> CPU ISA by using different commands. The infrastructure also >> provides a way to check the correctness of the ISA optimized action >> version against the scalar version. >> This series also introduces optimized versions of the following >> actions: >> - push_vlan >> - pop_vlan >> - set_masked eth >> - set_masked ipv4 >> Below is a table indicating some relative performance benefits for >> these actions. >> +---+---+-+ >> | Actions | Scalar with series|AVX >> with series | >> +---+---+-+ >> | mod_dl_dst| 1.01x |1.13x >> | >> +---+---+-+ >> | push_vlan | 1.01x |1.10x >> | >> +---+---+-+ >> | strip_vlan| 1.01x |1.11x >> | >> +---+---+-+ >> | mod_ipv4 1 x field| 1.01x |1.02x >> | >> +---+---+-+ >> | mod_ipv4 4 x fields | 1.01x |1.21x >> | >> +---+---+-+ >> | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x |1.36x >> | >> +---+---+-+ >> > > Thanks all for the work/reviews on this, given that it has been under > discussion prior to the soft freeze I have merged this to master. Thanks, still would like some answers on the performance numbers. //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
