Re: [ovs-dev] [PATCH v5 3/3] system-traffic.at: Test conntrack + FTP server running on a non-standard port.

2024-01-10 Thread Aaron Conole
Viacheslav Galaktionov via dev writes: > All existing test iterations assume that the FTP server is running on a > standard port, which may not always be the case. These tests helped find > problems in conntrack alg processing with non-standard ports. > > Perform the necessary adjustments to

Re: [ovs-dev] [PATCH v5 2/3] conntrack: Use helpers from committed connections.

2024-01-10 Thread Aaron Conole
Viacheslav Galaktionov via dev writes: > When a packet hits a flow rule without an explicitly specified helper, > OvS has to rely on automatic application layer gateway detection to > find related connections. This works as long as services are running on > their standard ports, e.g. when FTP

Re: [ovs-dev] [PATCH v5 1/3] lib/conntrack: Only use given packet in protocol detection.

2024-01-10 Thread Aaron Conole
Viacheslav Galaktionov via dev writes: > The current protocol detection logic relies on two pieces of metadata > passed as arguments: tp_src and tp_dst, which represent the L4 source > and destination port numbers from the flow that triggered the current > flow rule first, and was responsible

Re: [ovs-dev] [PATCH v2] system-tests: Test openflow matching for ct related packets with SNAT.

2024-01-10 Thread Aaron Conole
Brad Cowie writes: > Linux kernel commit ebddb1404900 ("net: move the nat function to > nf_nat_ovs for ovs and tc") introduced a regression into the kernel > datapath which prevented the openvswitch match key from being updated > when nat was undone for packets in the related conntrack state.

Re: [ovs-dev] [PATCH ovn v2] pinctrl: Directly retrieve desired port_binding MAC.

2024-01-10 Thread Han Zhou
On Wed, Jan 10, 2024 at 11:26 AM Mark Michelson wrote: > > A static analyzer determined that if pb->n_mac was 0, then the c_addrs > lport_addresses struct would never be initialized. We would then use > and attempt to free uninitialized memory. > > In reality, pb->n_mac should always be 1. This

Re: [ovs-dev] [PATCH ovn v3 16/16] northd: Add northd change handler for sync_to_sb_lb node.

2024-01-10 Thread Numan Siddique
On Tue, Jan 9, 2024 at 1:18 AM Han Zhou wrote: > > On Mon, Jan 8, 2024 at 3:52 PM Numan Siddique wrote: > > > > On Fri, Jan 5, 2024 at 11:36 AM Numan Siddique wrote: > > > > > > On Tue, Dec 19, 2023 at 2:37 AM Han Zhou wrote: > > > > > > > > On Mon, Nov 27, 2023 at 6:39 PM wrote: > > > > > >

Re: [ovs-dev] [PATCH v2 3/3] timeval: Add coverage counter for long poll interval events.

2024-01-10 Thread Eelco Chaudron
On 10 Jan 2024, at 20:34, Ilya Maximets wrote: > On 1/10/24 12:25, Eelco Chaudron wrote: >> Martin Kennelly observes that even though this data is available to >> humans via the journal/log files, these aren't exactly easy for a >> developer to make any kind of behavioral inferences. This

[ovs-dev] [PATCH v5] ci: Add clang-analyze to GitHub actions.

2024-01-10 Thread Eelco Chaudron
This patch identifies new static analysis issues during a GitHub action run and reports them. The process involves analyzing the changes introduced in the current commit and comparing them to those in the preceding commit. However, there are two cases when the GitHub push action runner does not

Re: [ovs-dev] [PATCH v4] ci: Add clang-analyze to GitHub actions.

2024-01-10 Thread Eelco Chaudron
On 2 Jan 2024, at 16:19, Eelco Chaudron wrote: > On 2 Jan 2024, at 13:43, Ilya Maximets wrote: > > +fi; + +configure_ovs $OPTS +make clean +scan-build -o ./clang-analyzer-results -sarif --use-cc=clang make -j4 >> >> We have a make target for

Re: [ovs-dev] [PATCH ovn] northd: Add option to enable conntrack for router port

2024-01-10 Thread Numan Siddique
On Wed, Jan 10, 2024 at 11:41 AM wrote: > > From: shylou > > By default, OVN skips the conntrack process for router type > LSP within a LS. It seems unnecessary for the LSP whose peer > is l3dgw_port. > > Therefore, we introduce an option named 'enable_router_port_acl', > which defaults to false

Re: [ovs-dev] [PATCH v2 3/3] timeval: Add coverage counter for long poll interval events.

2024-01-10 Thread Ilya Maximets
On 1/10/24 12:25, Eelco Chaudron wrote: > Martin Kennelly observes that even though this data is available to > humans via the journal/log files, these aren't exactly easy for a > developer to make any kind of behavioral inferences. This kind of > log and counter would be useful when checking on

[ovs-dev] [PATCH 5/6] ovsdb-server: Do storage run at top of main loop.

2024-01-10 Thread Frode Nordahl
During testing of the implementation of cooperative multitasking in the ovsdb-server we noticed that the very first yield being called in the jsonrpc-server always fired. This indicates that the operations being after/before storage run is taking too long as it currently is. Moving the storage

[ovs-dev] [PATCH 2/6] lib: Introduce cooperative multitasking module.

2024-01-10 Thread Frode Nordahl
One of the goals of Open vSwitch is to be as resource efficient as possible. Core parts of the program has been implemented as asynchronous state machines, and when absolutely necessary additional threads are used. Introduce cooperative multitasking module which allow us to interleave important

[ovs-dev] [PATCH 6/6] ovsdb-server: Make use of cooperative multitasking.

2024-01-10 Thread Frode Nordahl
Initialize the cooperative multitasking module for the ovsdb-server. The server side schema conversion process used for storage engines such as RAFT is time consuming, yield during processing. After the schema conversion is done, the processing of JSON-RPC sessions and OVSDB monitors for

[ovs-dev] [PATCH 4/6] json: Add yielding json object create/destroy functions.

2024-01-10 Thread Frode Nordahl
Creating and destroying JSON objects may be time consuming. Add yielding counterparts of json_serialized_object_create() and json_destroy__() functions that make use of the cooperative multitasking module to yield during processing, allowing time sensitive tasks in other parts of the program to

[ovs-dev] [PATCH 3/6] ovsdb/raft: Register for cooperative multitasking.

2024-01-10 Thread Frode Nordahl
The OVSDB server is mostly synchronous and single threaded. The OVSDB RAFT storage engine operate under strict deadlines with operational impact should the deadline be overrun. Register for cooperative multitasking so that long running processing elsewhere in the program may yield to allow

[ovs-dev] [PATCH 0/6] Introduce cooperative multitasking to improve OVSDB RAFT cluster operation.

2024-01-10 Thread Frode Nordahl
Introduce cooperative multitasking module which allows us to interleave important processing with long running tasks while avoiding the additional resource consumption of threads and complexity of asynchronous state machines. We will use this module to ensure long running processing in the OVSDB

[ovs-dev] [PATCH 1/6] timeval: Make timewarp available for internal callers.

2024-01-10 Thread Frode Nordahl
At present the timeval timewarp functionality is tightly coupled with the unixctl interface for external operation by test suite. It is however desirable to make use of the timewarp functionality directly in unit tests. Split unixctl callback interface and the actual timewarp functionality into

[ovs-dev] [PATCH ovn v2] pinctrl: Directly retrieve desired port_binding MAC.

2024-01-10 Thread Mark Michelson
A static analyzer determined that if pb->n_mac was 0, then the c_addrs lport_addresses struct would never be initialized. We would then use and attempt to free uninitialized memory. In reality, pb->n_mac should always be 1. This is because the port binding is a representation of a northbound

Re: [ovs-dev] [PATCH v2 2/3] ofproto-dpif-upcall: Add flow_limit coverage counters.

2024-01-10 Thread Ilya Maximets
On 1/10/24 12:25, Eelco Chaudron wrote: > Add new coverage counters that might help debugging flow_limit > related issues. > > Signed-off-by: Eelco Chaudron > --- > > v2: Changed duration into flow increase/decrease counters. > > ofproto/ofproto-dpif-upcall.c | 6 ++ > 1 file changed, 6

Re: [ovs-dev] [PATCH v2 1/3] ofproto-dpif-upcall: Change flow dump duration message to WARN level.

2024-01-10 Thread Ilya Maximets
On 1/10/24 12:25, Eelco Chaudron wrote: > Currently the 'Spent an unreasonably long Xms dumping flows' message > is set to the INFO level. However, based on this, we are also > drastically limiting the number of flows in the datapath, and this > would warrant a WARNING level. > > Acked-by: Simon

Re: [ovs-dev] [PATCH v2 1/3] tests: Set handle_segv fpr UBSAN to allow SIGSEGV tests.

2024-01-10 Thread Ilya Maximets
On 1/10/24 11:22, Eelco Chaudron wrote: > Previously tests that were generating a SIGSEGV were excluded > from UBSAN runs. With the correct environment variable, these > tests can be run. This is working even with the clang version > supplied by Ubuntu 20.04. Hmm, interesting. I'm pretty sure

Re: [ovs-dev] [PATCH ovs v3] debian/rules: Fix incorrect use of link-time optimizer.

2024-01-10 Thread Ilya Maximets
On 1/9/24 13:53, Simon Horman wrote: > On Mon, Jan 08, 2024 at 08:34:36AM -0300, Roberto Bartzen Acosta via dev > wrote: >> Current version of debian/rules simply uses the default lto GCC >> optimization settings during the linkage process. >> >> The main problem with this approach is that GCC on

Re: [ovs-dev] [PATCH] dpdk: Update to use v23.11.

2024-01-10 Thread Kevin Traynor
+cc some others people who may be interested about OVS upgrading DPDK version. On 10/01/2024 16:52, Ilya Maximets wrote: > On 12/13/23 14:06, David Marchand wrote: >> This commit adds support for DPDK v23.11. >> It updates the CI script and documentation and includes the following >> changes

Re: [ovs-dev] [PATCH v2] python: idl: Handle monitor_canceled

2024-01-10 Thread Simon Horman
On Wed, Jan 10, 2024 at 06:38:48PM +0100, Ilya Maximets wrote: > On 1/10/24 18:18, Simon Horman wrote: > > On Wed, Jan 10, 2024 at 12:28:22PM +0100, Dumitru Ceara wrote: > >> On 1/8/24 16:31, Ilya Maximets wrote: > >>> On 1/8/24 16:05, Ilya Maximets wrote: > On 1/5/24 18:35, Terry Wilson

Re: [ovs-dev] [PATCH v2] python: idl: Handle monitor_canceled

2024-01-10 Thread Ilya Maximets
On 1/10/24 18:18, Simon Horman wrote: > On Wed, Jan 10, 2024 at 12:28:22PM +0100, Dumitru Ceara wrote: >> On 1/8/24 16:31, Ilya Maximets wrote: >>> On 1/8/24 16:05, Ilya Maximets wrote: On 1/5/24 18:35, Terry Wilson wrote: > On Fri, Jan 5, 2024 at 9:56 AM Simon Horman wrote: >>

Re: [ovs-dev] [PATCH v2] python: idl: Handle monitor_canceled

2024-01-10 Thread Simon Horman
On Wed, Jan 10, 2024 at 12:28:22PM +0100, Dumitru Ceara wrote: > On 1/8/24 16:31, Ilya Maximets wrote: > > On 1/8/24 16:05, Ilya Maximets wrote: > >> On 1/5/24 18:35, Terry Wilson wrote: > >>> On Fri, Jan 5, 2024 at 9:56 AM Simon Horman wrote: > > On Mon, Dec 18, 2023 at 05:31:24PM

Re: [ovs-dev] [PATCH ovn] northd: Add option to enable conntrack for router port

2024-01-10 Thread 0-day Robot
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: ERROR: Author shylou needs to sign off. WARNING: Unexpected sign-offs from developers who

Re: [ovs-dev] [PATCH ovn] ovn-ctl: Add option to skip schema conversion

2024-01-10 Thread Frode Nordahl
On Wed, Jan 10, 2024 at 2:05 PM Martin Kalcok wrote: > > ovn-ctl script currently automatically attempts to perform clustered > database schema upgrade when starting OVN SB or NB clustered > database. To provide more controll over this procees a See nit from v1. > `--db-cluster-schema-upgrade`

Re: [ovs-dev] [PATCH] dpdk: Update to use v23.11.

2024-01-10 Thread Ilya Maximets
On 12/13/23 14:06, David Marchand wrote: > This commit adds support for DPDK v23.11. > It updates the CI script and documentation and includes the following > changes coming from the dpdk-latest branch: > > - sparse: Add some compiler intrinsics for DPDK build. >

[ovs-dev] [PATCH ovn] northd: Add option to enable conntrack for router port

2024-01-10 Thread numans
From: shylou By default, OVN skips the conntrack process for router type LSP within a LS. It seems unnecessary for the LSP whose peer is l3dgw_port. Therefore, we introduce an option named 'enable_router_port_acl', which defaults to false and can be set to true to enable conntrack for the LSP

Re: [ovs-dev] [PATCH ovn] pinctrl: Directly retrieve desired port_binding MAC.

2024-01-10 Thread Mark Michelson
On 1/9/24 19:16, Han Zhou wrote: On Tue, Jan 9, 2024 at 10:32 AM Mark Michelson > wrote: > > A static analyzer determined that if pb->n_mac was 0, then the c_addrs > lport_addresses struct would never be initialized. We would then use > and attempt to free

Re: [ovs-dev] [PATCH 20/22] ovsdb-server: Allow user-provided config files.

2024-01-10 Thread Terry Wilson
On Mon, Jan 8, 2024 at 8:17 AM Ilya Maximets wrote: > > On 1/5/24 21:26, Terry Wilson wrote: > > On Wed, Dec 13, 2023 at 7:05 PM Ilya Maximets wrote: > > > >> -/* Clears and replaces 'remotes' and 'dbnames' by a configuration read > >> from > >> - * 'config_file', which must have been

Re: [ovs-dev] [PATCH v2 2/2] ovsdb: transaction: Calculate added/removed from diff.

2024-01-10 Thread Dumitru Ceara
On 1/9/24 20:54, Ilya Maximets wrote: > void ovsdb_datum_diff(struct ovsdb_datum *diff, > diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c > index b69d03b5a..e1a87bb74 100644 > --- a/ovsdb/transaction.c > +++ b/ovsdb/transaction.c > @@ -347,12 +347,13 @@ update_row_ref_count(struct

Re: [ovs-dev] [PATCH v2 2/2] ovsdb: transaction: Calculate added/removed from diff.

2024-01-10 Thread Dumitru Ceara
On 1/9/24 20:54, Ilya Maximets wrote: > In case the difference between 'old' and 'new' rows is readily > available, it can be used to construct added/removed datums > instead. Diffs are typically much smaller than the column > itself. This change more than doubles the performance of a >

Re: [ovs-dev] [PATCH v6] system-dpdk: Test with mlx5 devices.

2024-01-10 Thread Kevin Traynor
On 10/01/2024 10:04, David Marchand wrote: > The DPDK unit test only runs if vfio or igb_uio kernel modules are loaded: > on systems with only mlx5, this test is always skipped. > > Besides, the test tries to grab the first device listed by dpdk-devbind.py, > regardless of the PCI device status

Re: [ovs-dev] [PATCH ovn] ovn-ctl: Add option to skip schema conversion

2024-01-10 Thread 0-day Robot
Bleep bloop. Greetings Martin Kalcok, 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 99 characters long (recommended limit is 79) #92 FILE:

Re: [ovs-dev] [PATCH v2 1/2] ovsdb: Preserve column diffs read from the storage.

2024-01-10 Thread Dumitru Ceara
On 1/9/24 20:54, Ilya Maximets wrote: > Database file contains the column diff, but it is discarded once > the 'new' state of a row is constructed. Keep it in the transaction > row, as it can be used later by other parts of the code. > > Diffs do not live long, we keep them around only while

[ovs-dev] [PATCH ovn] ovn-ctl: Add option to skip schema conversion

2024-01-10 Thread Martin Kalcok
ovn-ctl script currently automatically attempts to perform clustered database schema upgrade when starting OVN SB or NB clustered database. To provide more controll over this procees a `--db-cluster-schema-upgrade` option is added. Default value for this option is `yes`, to preserve current

Re: [ovs-dev] [PATCH v2 1/3] ofproto-dpif-upcall: Change flow dump duration message to WARN level.

2024-01-10 Thread Eelco Chaudron
Recheck-request: github-robot On 10 Jan 2024, at 12:25, Eelco Chaudron wrote: > Currently the 'Spent an unreasonably long Xms dumping flows' message > is set to the INFO level. However, based on this, we are also > drastically limiting the number of flows in the datapath, and this > would

[ovs-dev] [PATCH ovn 2/2] system-tests: Wait for the meter in CoPP tests

2024-01-10 Thread Ales Musil
The CoPP test modifies a meter from drop=1 to drop=10, there are two issues with this change: 1) It takes some time for this change to propagate into OvS. 2) Depending on the timing the 10 packet limit might not fit into single pktps bucket. To address those issues lower the pktps to 5, this has

[ovs-dev] [PATCH ovn 1/2] tests: Reduce flakiness of daemon ssl files change test

2024-01-10 Thread Ales Musil
The test is changing certificates when the ovn-nbctl is running as daemon and expects that the mismatch will be detected. In most cases this is fine because the main loop of the ovn-nbctl will be woken up by something external. However, when the host is busy it might not be woken up in time. Wake

Re: [ovs-dev] [PATCH v2 3/3] ci: Add kernel and userspace ASAN/UBSAN tests.

2024-01-10 Thread Eelco Chaudron
Recheck-request: github-robot On 10 Jan 2024, at 11:22, Eelco Chaudron wrote: > This patch adds ASAN and UBSAN GitHub action tests for both > the userspace and kernel datapaths. > > Signed-off-by: Eelco Chaudron > --- > .github/workflows/build-and-test.yml | 14 ++ > 1 file

Re: [ovs-dev] [PATCH v2 11/17] ovsdb: Embed jsonrpc session options into ovsdb jsonrpc options.

2024-01-10 Thread Ilya Maximets
On 1/9/24 23:49, Ilya Maximets wrote: > Just introduced structure 'jsonrpc_session_options' is the same > as part of the 'ovsdb_jsonrpc_options'. In fact, these options > do really belong to a lower layer. So, replace a copy of these > fields with a structure, so it can be easily passed to

Re: [ovs-dev] [PATCH v2] python: idl: Handle monitor_canceled

2024-01-10 Thread Dumitru Ceara
On 1/8/24 16:31, Ilya Maximets wrote: > On 1/8/24 16:05, Ilya Maximets wrote: >> On 1/5/24 18:35, Terry Wilson wrote: >>> On Fri, Jan 5, 2024 at 9:56 AM Simon Horman wrote: On Mon, Dec 18, 2023 at 05:31:24PM -0600, Terry Wilson wrote: > Currently python-ovs claims to be "db change

Re: [ovs-dev] [PATCH v2] python: idl: Handle monitor_canceled

2024-01-10 Thread Dumitru Ceara
On 12/19/23 00:31, Terry Wilson wrote: > Currently python-ovs claims to be "db change aware" but does not > parse the "monitor_canceled" notification. Transactions can continue > being made, but the monitor updates will not be sent. This handles > monitor_cancel similarly to how ovsdb-cs currently

Re: [ovs-dev] [PATCH] ovsdb-idl.at: Test IDL behavior during database conversion.

2024-01-10 Thread Dumitru Ceara
On 1/8/24 15:52, Ilya Maximets wrote: > Add new 'monitor' command to test-ovsdb utilities to make them just > run IDL loop infinitely. Other commands can still be placed before > the 'monitor', e.g. setting up conditions, tracking, running a few > transactions. > > Having that, adding a couple

[ovs-dev] [PATCH v2 3/3] timeval: Add coverage counter for long poll interval events.

2024-01-10 Thread Eelco Chaudron
Martin Kennelly observes that even though this data is available to humans via the journal/log files, these aren't exactly easy for a developer to make any kind of behavioral inferences. This kind of log and counter would be useful when checking on system health to let us know that an Open

[ovs-dev] [PATCH v2 2/3] ofproto-dpif-upcall: Add flow_limit coverage counters.

2024-01-10 Thread Eelco Chaudron
Add new coverage counters that might help debugging flow_limit related issues. Signed-off-by: Eelco Chaudron --- v2: Changed duration into flow increase/decrease counters. ofproto/ofproto-dpif-upcall.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/ofproto/ofproto-dpif-upcall.c

[ovs-dev] [PATCH v2 1/3] ofproto-dpif-upcall: Change flow dump duration message to WARN level.

2024-01-10 Thread Eelco Chaudron
Currently the 'Spent an unreasonably long Xms dumping flows' message is set to the INFO level. However, based on this, we are also drastically limiting the number of flows in the datapath, and this would warrant a WARNING level. Acked-by: Simon Horman Signed-off-by: Eelco Chaudron ---

Re: [ovs-dev] [PATCH ovn] ovn-ctl: Add option to skip schema conversion

2024-01-10 Thread martin . kalcok
On Tue, 2024-01-09 at 08:23 +0100, Frode Nordahl wrote: > Hello, Martin, > > Thank you for working on this! > > While the automatic conversion is nice when it works, it is a source > of problems when it does not, so I welcome this change. > > An additional argument for this change is that the

[ovs-dev] [PATCH v2 2/3] ci: Combine the ubsan and asan sanitizer runs.

2024-01-10 Thread Eelco Chaudron
This patch combines the existing ubsan and asan GitHub actions tests into one. Signed-off-by: Eelco Chaudron --- v2: - Combined the log path of both asan and ubsan - Directly pass the sanitizer options to -fsanitize .ci/linux-build.sh | 14 --

[ovs-dev] [PATCH v2 3/3] ci: Add kernel and userspace ASAN/UBSAN tests.

2024-01-10 Thread Eelco Chaudron
This patch adds ASAN and UBSAN GitHub action tests for both the userspace and kernel datapaths. Signed-off-by: Eelco Chaudron --- .github/workflows/build-and-test.yml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/build-and-test.yml

[ovs-dev] [PATCH v2 1/3] tests: Set handle_segv fpr UBSAN to allow SIGSEGV tests.

2024-01-10 Thread Eelco Chaudron
Previously tests that were generating a SIGSEGV were excluded from UBSAN runs. With the correct environment variable, these tests can be run. This is working even with the clang version supplied by Ubuntu 20.04. Signed-off-by: Eelco Chaudron --- tests/atlocal.in | 10 --

[ovs-dev] [PATCH v2 1/3] tests: Set handle_segv fpr UBSAN to allow SIGSEGV tests.

2024-01-10 Thread Eelco Chaudron
Previously tests that were generating a SIGSEGV were excluded from UBSAN runs. With the correct environment variable, these tests can be run. This is working even with the clang version supplied by Ubuntu 20.04. Signed-off-by: Eelco Chaudron --- tests/atlocal.in | 10 --

[ovs-dev] [PATCH v6] system-dpdk: Test with mlx5 devices.

2024-01-10 Thread David Marchand
The DPDK unit test only runs if vfio or igb_uio kernel modules are loaded: on systems with only mlx5, this test is always skipped. Besides, the test tries to grab the first device listed by dpdk-devbind.py, regardless of the PCI device status regarding kmod binding. Remove dependency on this