Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd.c: Omit unused columns in SB_Global.

2023-06-13 Thread Han Zhou
On Tue, Jun 13, 2023 at 6:44 AM Mark Michelson wrote: > > On 6/12/23 22:07, Han Zhou wrote: > > > > > > On Mon, Jun 12, 2023 at 11:05 AM Mark Michelson > > wrote: > > > > > > Hi Han, > > > > > > Acked-by: Mark Michelson > > > > > > >

Re: [ovs-dev] [PATCH v14 4/4] userspace: Enable L4 checksum offloading by default.

2023-06-13 Thread Ilya Maximets
On 6/2/23 20:59, Mike Pattrick wrote: > The netdev receiving packets is supposed to provide the flags > indicating if the L4 checksum was verified and it is OK or BAD, > otherwise the stack will check when appropriate by software. > > If the packet comes with good checksum, then postpone the > che

Re: [ovs-dev] [PATCH v14 3/4] userspace: Enable IP checksum offloading by default.

2023-06-13 Thread Ilya Maximets
On 6/2/23 20:59, Mike Pattrick wrote: > The netdev receiving packets is supposed to provide the flags > indicating if the IP checksum was verified and it is GOOD or BAD, > otherwise the stack will check when appropriate by software. > > If the packet comes with good checksum, then postpone the > c

[ovs-dev] [PATCH v12 8/8] treewide: Add `ovs_assert` to check for null pointers

2023-06-13 Thread James Raphael Tiovalen
This patch adds an assortment of `ovs_assert` statements to check for null pointers. We use assertions since it should be impossible for any of these pointers to be NULL. Signed-off-by: James Raphael Tiovalen Reviewed-by: Simon Horman --- lib/dp-packet.c| 1 + lib/dpctl.c| 4

[ovs-dev] [PATCH v12 7/8] lib, ovsdb: Add various null pointer checks

2023-06-13 Thread James Raphael Tiovalen
This commit adds various null pointer checks to some files in the `lib` and the `ovsdb` directories to fix several Coverity defects. These changes are grouped together as they perform similar checks, returning early or skipping some action if a null pointer is encountered. Signed-off-by: James Rap

[ovs-dev] [PATCH v12 5/8] file, monitor: Add null pointer assertions for old and new ovsdb_rows

2023-06-13 Thread James Raphael Tiovalen
This commit adds non-null pointer assertions in some code that performs some decisions based on old and new input ovsdb_rows. Signed-off-by: James Raphael Tiovalen Reviewed-by: Simon Horman --- ovsdb/file.c| 2 ++ ovsdb/monitor.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) di

[ovs-dev] [PATCH v12 6/8] ovs-vsctl: Fix crash when routing is enabled

2023-06-13 Thread James Raphael Tiovalen
In the case where routing is enabled, the bridge member of the `vsctl_port` structs is not populated. This can cause a crash if we attempt to access it. This patch fixes the crash by checking if the bridge member is valid before attempting to access it. In the `check_conflicts` function, we print b

[ovs-dev] [PATCH v12 2/8] lib, ovs-vsctl: Add zero-initializations

2023-06-13 Thread James Raphael Tiovalen
This commit adds zero-initializations by changing `SFL_ALLOC` from `malloc` to `xzalloc`, adding a `memset` call to `sflAlloc`, initializing a `pollfd` struct variable with zeroes, and changing some calls to `xmalloc` to `xzalloc`. This is to prevent potential data leaks or undefined behavior from

[ovs-dev] [PATCH v12 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James Raphael Tiovalen
This commit adds a few null pointer assertions and checks to some return values of `ovsdb_table_schema_get_column`. If a null pointer is encountered in these blocks, either the assertion will fail or the control flow will now be redirected to alternative paths which will output the appropriate erro

[ovs-dev] [PATCH v12 3/8] shash, simap, smap: Add assertions to `*_count` functions

2023-06-13 Thread James Raphael Tiovalen
This commit adds assertions in the functions `shash_count`, `simap_count`, and `smap_count` to ensure that the corresponding input struct pointer is not NULL. This ensures that if the return values of `shash_sort`, `simap_sort`, or `smap_sort` are NULL, then the following for loops would not attem

[ovs-dev] [PATCH v12 1/8] lib: Add non-null assertions to some return values of `dp_packet_data`

2023-06-13 Thread James Raphael Tiovalen
This commit adds some `ovs_assert()` checks to some return values of `dp_packet_data()` to ensure that they are not NULL and to prevent null-pointer dereferences, which might lead to unwanted crashes. We use assertions since it should be impossible for these calls to `dp_packet_data()` to return NU

[ovs-dev] [PATCH v12 0/8] treewide: Fix multiple Coverity defects

2023-06-13 Thread James Raphael Tiovalen
This cleanup patchset addresses several high and medium-impact Coverity defects. Unit tests have been successfully executed via `make check` and they successfully passed. The list of revisions so far: v2: Fix some apply-robot checkpatch errors and warnings. v3: Fix some apply-robot checkpatch e

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James R T
On Wed, Jun 14, 2023 at 1:38 AM Simon Horman wrote: > > > That said, I do have one question before I submit the next version of > > this patch. Should I change the log severity from an error to a > > warning instead since it seems that there are test cases that will > > print the log message? Or s

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread Simon Horman
On Wed, Jun 14, 2023 at 12:09:42AM +0800, James R T wrote: > Hi Simon, > > On Mon, Jun 12, 2023 at 10:56 PM Simon Horman > wrote: > > > > On Mon, Jun 05, 2023 at 01:30:03AM +0800, James Raphael Tiovalen wrote: > > > This commit adds a few null pointer assertions and checks to some return > > > v

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James R T
Hi Simon, On Mon, Jun 12, 2023 at 10:56 PM Simon Horman wrote: > > On Mon, Jun 05, 2023 at 01:30:03AM +0800, James Raphael Tiovalen wrote: > > This commit adds a few null pointer assertions and checks to some return > > values of `ovsdb_table_schema_get_column`. If a null pointer is > > encounter

[ovs-dev] [PATCH ovn] tests: Add missing FOR_EACH_NORTHD

2023-06-13 Thread Ales Musil
Add missing OVN_FOR_EACH_NORTHD around FDB aging test. Signed-off-by: Ales Musil --- tests/ovn.at | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ovn.at b/tests/ovn.at index e49148411..7f83ba8ab 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -36181,6 +36181,7 @@ OVS_WAIT_UNTIL([test $

Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd.c: Omit unused columns in SB_Global.

2023-06-13 Thread Mark Michelson
On 6/12/23 22:07, Han Zhou wrote: On Mon, Jun 12, 2023 at 11:05 AM Mark Michelson > wrote: > > Hi Han, > > Acked-by: Mark Michelson > > > I like the changes, but I don't like that patch 1 on its own results in > breaking CI. > >

Re: [ovs-dev] [PATCH 1/2] utilities: add "--detach" option to ovs-ctl

2023-06-13 Thread Vladislav Odintsov
Hi Ilya, thanks for the attention on this patchset. > On 13 Jun 2023, at 14:58, Ilya Maximets wrote: > > On 6/7/23 08:33, Vladislav Odintsov wrote: >> Default is yes (current behavior). This means that after start process >> will be run in background. When "no" is given, process is run in >>

[ovs-dev] [PATCH v3] Add editorconfig file.

2023-06-13 Thread Robin Jarry
EditorConfig is a file format and collection of text editor plugins for maintaining consistent coding styles between different editors and IDEs. Initialize the file following the coding rules in Documentation/internals/contributing/coding-style.rst and add exceptions declared in build-aux/initial-

Re: [ovs-dev] [PATCH 2/2] utilities: add --in-db-ssl option to ovs-ctl

2023-06-13 Thread Ilya Maximets
On 6/7/23 08:33, Vladislav Odintsov wrote: > It is possible to parametrize ovs-ctl script to start ovsdb-server with > DB_SCHEME other than Open_vSwitch. This scheme may not have currently > required table "SSL" with "key", "cert" and "cacert" columns. The db-schema option is primarily exists to

Re: [ovs-dev] [PATCH 1/2] utilities: add "--detach" option to ovs-ctl

2023-06-13 Thread Ilya Maximets
On 6/7/23 08:33, Vladislav Odintsov wrote: > Default is yes (current behavior). This means that after start process > will be run in background. When "no" is given, process is run in > foreground with `exec` call, which replaces current process (ovs-ctl) with > wanted ovs process (ovsdb-server or