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

[Frode Nordahl]

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 existing approach
executes asynchronously, so there is no way for the human or machine
operator to get the result of the `ovsdb-client` call.

On Mon, Jan 8, 2024 at 10:15 AM 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

nit: a couple of typos above.

> `--db-cluster-skip-upgrade` option is added that allows skipping
> the upgrade.

There is a precedent in the `ovn-ctl` and `ovs-ctl` scripts to use
options in the form of `--no-something` for optional negation of some
feature (`--no-leader-only` `--no-record-hostname` etc).

What do you think about making your new option conforming with this?

> Default value for this option is `no`, to preserve current default
> behavior.
>
> Signed-off-by: Martin Kalcok 
> ---
>  utilities/ovn-ctl   |  7 ++-
>  utilities/ovn-ctl.8.xml | 11 +++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> index 876565c80..011ef9c0c 100755
> --- a/utilities/ovn-ctl
> +++ b/utilities/ovn-ctl
> @@ -184,6 +184,7 @@ start_ovsdb__() {
>  local ovn_db_ssl_cacert
>  local ovn_db_election_timer
>  local relay_mode
> +local skip_cluster_db_upgrade
>  eval db_pid_file=\$DB_${DB}_PIDFILE
>  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
>  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> @@ -212,6 +213,7 @@ start_ovsdb__() {
>  eval ovn_db_election_timer=\$DB_${DB}_ELECTION_TIMER
>  eval relay_mode=\$RELAY_MODE
>  eval relay_remote=\$DB_${DB}_REMOTE
> +eval skip_cluster_db_upgrade=\$DB_CLUSTER_SKIP_UPGRADE
>
>  ovn_install_dir "$OVN_RUNDIR"
>  ovn_install_dir "$ovn_logdir"
> @@ -347,7 +349,7 @@ $cluster_remote_port
>  $(echo ovn-${db}ctl | tr _ -) --no-leader-only --db="unix:$sock" init
>  fi
>
> -if test $mode = cluster; then
> +if test $mode = cluster && test X"$skip_cluster_db_upgrade" = Xno; then
>  upgrade_cluster "$schema" "unix:$sock"
>  fi
>
> @@ -898,6 +900,8 @@ set_defaults () {
>  OVN_SB_RELAY_DB_SSL_CERT=""
>  OVN_SB_RELAY_DB_SSL_CA_CERT=""
>  DB_SB_RELAY_USE_REMOTE_IN_DB="yes"
> +
> +DB_CLUSTER_SKIP_UPGRADE="no"
>  }
>
>  set_option () {
> @@ -1148,6 +1152,7 @@ File location options:
>--ovn-sb-relay-db-ssl-key=KEY OVN_Southbound DB relay SSL private key file
>--ovn-sb-relay-db-ssl-cert=CERT OVN_Southbound DB relay SSL certificate 
> file
>--ovn-sb-relay-db-ssl-ca-cert=CERT OVN OVN_Southbound DB relay SSL CA 
> certificate file
> +  --db-cluster-skip-upgrade=yes|no (default: $DB_CLUSTER_SKIP_UPGRADE)
>
>  Default directories with "configure" option and environment variable 
> override:
>logs: /usr/local/var/log/ovn (--with-logdir, OVN_LOGDIR)
> diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml
> index 01f4aa26b..5762fd3a4 100644
> --- a/utilities/ovn-ctl.8.xml
> +++ b/utilities/ovn-ctl.8.xml
> @@ -156,6 +156,7 @@
>  --db-ic-sb-cluster-remote-addr=IP ADDRESS
>  --db-ic-sb-cluster-remote-port=PORT NUMBER
>  --db-ic-sb-cluster-remote-proto=PROTO 
> (tcp/ssl)
> +--db-cluster-skip-upgrade=yes|no
>
>   Probe interval options 
>  --db-nb-probe-interval-to-active=Time in 
> milliseconds
> @@ -324,4 +325,14 @@
> start_northd
>
>  
> +Avoiding automatic clustered OVN database schema upgrade
> +
> +  If you desire more controll over clustered DB schema upgrade, you can

s/controll/control

> +  opt-out of automatic on-start upgrade attempts with
> +  --db-cluster-skip-upgrade.
> +
> +Start OVN NB (or SB) clustered database on host with IP x.x.x.x 
> without schema upgrade
> +# ovn-ctl start_nb_ovsdb --db-nb-cluster-local-addr=x.x.x.x 
> --db-cluster-skip-upgrade=yes
> +Trigger clustered DB schema upgrade manually
> +# ovsdb-client -t 30 convert unix:/var/run/ovn/ovnnb_db.sock 
> /usr/local/share/ovn/ovn-nb.ovsschema

I'd drop the `-t 30` in this example.

For completeness, should this section also list the command to convert
the southbound database?

-- 
Frode Nordahl

>  
> --
> 2.40.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

Re: [ovs-dev] [RFC 0/5] Introduce cooperative multitasking to improve OVSDB RAFT cluster operation.

[Frode Nordahl]

On Mon, Jan 8, 2024 at 5:52 PM Frode Nordahl
 wrote:
>
> Posting this as RFC as I'm not fully content with my test results.
>
> With this series there are still situations where we overrun the
> time threashold and subsequently get into the situation we are trying
> to fix with this series.
>
> During testing I found a destroy call [0] to take around 500ms, but
> as far as I read the code that ends up being a single call to free()
> so that does not quite make sense.  Working on replicating in a
> different test bed and continuing to pin-point the source of the
> last time threshold overruns.  Will post a follow-up soon.
>
> 0: 
> https://github.com/openvswitch/ovs/blob/4102674b3ecadb0e20e512cc661cddbbc4b3d1f6/ovsdb/monitor.c#L1290

Ah, I conflated the layout of the old and the new json object. The one
being removed is a JSON_OBJECT and not a JSON_SERIALIZED_OBJECT which
indeed involves a loop for destruction [1] and not a single call to
free().

I'll give it similar treatment as done in patch 4 of this series and
see where that gets me.

1: 
https://github.com/openvswitch/ovs/blob/4102674b3ecadb0e20e512cc661cddbbc4b3d1f6/lib/json.c#L397-L410

-- 
Frode Nordahl

> Introduce cooperative multitasking module which allow 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 server does not interfere with stable maintenance of the
> RAFT cluster in subsequent patches.
>
> Relevant discussion:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2022-December/052140.html
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410262.html
>
>
> Frode Nordahl (5):
>   timeval: Make timewarp available for internal callers.
>   lib: Introduce cooperative multitasking module.
>   ovsdb/raft: Register for cooperative multitasking.
>   json: Add yielding serialized object create function.
>   ovsdb-server: Make use of cooperative multitasking.
>
>  NEWS   |   8 +
>  include/openvswitch/json.h |   4 +-
>  lib/automake.mk|   3 +
>  lib/cooperative-multitasking-private.h |  30 +++
>  lib/cooperative-multitasking.c | 192 ++
>  lib/cooperative-multitasking.h |  42 
>  lib/json.c |  43 ++--
>  lib/timeval.c  |  56 --
>  lib/timeval.h  |   4 +
>  ovsdb/file.c   |   2 +
>  ovsdb/jsonrpc-server.c |   3 +
>  ovsdb/monitor.c|  25 ++-
>  ovsdb/ovsdb-server.c   |   5 +
>  ovsdb/raft.c   |  12 +-
>  tests/automake.mk  |   1 +
>  tests/library.at   |  10 +
>  tests/ovsdb-server.at  |   1 +
>  tests/test-cooperative-multitasking.c  | 259 +
>  18 files changed, 673 insertions(+), 27 deletions(-)
>  create mode 100644 lib/cooperative-multitasking-private.h
>  create mode 100644 lib/cooperative-multitasking.c
>  create mode 100644 lib/cooperative-multitasking.h
>  create mode 100644 tests/test-cooperative-multitasking.c
>
> --
> 2.34.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

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

[Han Zhou]

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:
> > > >
> > > > From: Numan Siddique 
> > > >
> > > > Any changes to northd engine node due to load balancers
> > > > are now handled in 'sync_to_sb_lb' node to sync the changed
> > > > load balancers to SB load balancers.
> > > >
> > > > The logic to sync the SB load balancers is changed a bit and it
> > > > now mimics the SB lflow sync.
> > > >
> > > > Below are the scale testing results done with all the patches
applied
> > > > in this series using ovn-heater.  The test ran the scenario  -
> > > > ocp-500-density-heavy.yml [1].
> > > >
> > > > The resuts are:
> > > >
> > > >
> > >
---
> > > > Min (s) Median (s)  90%ile (s)
> > >  99%ile (s)  Max (s) Mean (s)Total (s)   Count
> > > Failed
> > > >
> > >
---
> > > > Iteration Total 0.1368831.1290161.192001
> > >  1.2041671.2127280.66501783.127099   125
0
> > > > Namespace.add_ports 0.0052160.0057360.007034
> > >  0.0154860.0189780.0062110.776373125
0
> > > > WorkerNode.bind_port0.0350300.0460820.052469
> > >  0.0582930.0603110.04597311.493259   250
0
> > > > WorkerNode.ping_port0.0050570.0067271.047692
> > >  1.0692531.0713360.26689666.724094   250
0
> > > >
> > >
---
> > > >
> > > > The results with the present main [2] are:
> > > >
> > > >
> > >
---
> > > > Min (s) Median (s)  90%ile (s)
> > >  99%ile (s)  Max (s) Mean (s)Total (s)   Count
> > > Failed
> > > >
> > >
---
> > > > Iteration Total 0.1354912.2238053.311270
> > >  3.3390783.3453461.729172216.146495  125
0
> > > > Namespace.add_ports 0.0053800.0057440.006819
> > >  0.0187730.0208000.0062920.786532125
0
> > > > WorkerNode.bind_port0.0341790.0460550.053488
> > >  0.0588010.0710430.04611711.529311   250
0
> > > > WorkerNode.ping_port0.0049560.0069523.086952
> > >  3.1917433.1928070.791544197.886026  250
0
> > > >
> > >
---
> > > >
> > > > [1] -
> > >
https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
> > > > [2] - 2a12cda890a7("controller, northd: Wait for cleanup before
replying
> > > to exit")
> > > >
> > > > Signed-off-by: Numan Siddique 
> > > > ---
> > > >  northd/en-sync-sb.c  | 445
+--
> > > >  northd/inc-proc-northd.c |   1 +
> > > >  northd/lflow-mgr.c   | 196 ++---
> > > >  northd/lflow-mgr.h   |  23 +-
> > > >  northd/northd.c  | 238 -
> > > >  northd/northd.h  |   6 -
> > > >  tests/ovn-northd.at  | 103 +
> > > >  7 files changed, 585 insertions(+), 427 deletions(-)
> > > >
> > > > diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
> > > > index 73b30272c4..62c5dbd20f 100644
> > > > --- a/northd/en-sync-sb.c
> > > > +++ b/northd/en-sync-sb.c
> > > > @@ -30,6 +30,7 @@
> > > >  #include "lib/ovn-nb-idl.h"
> > > >  #include "lib/ovn-sb-idl.h"
> > > >  #include "lib/ovn-util.h"
> > > > +#include "lflow-mgr.h"
> > > >  #include "northd.h"
> > > >
> > > >  #include "openvswitch/vlog.h"
> > > > @@ -53,6 +54,38 @@ static void build_port_group_address_set(const
struct
> > > nbrec_port_group *,
> > > >   struct svec *ipv4_addrs,
> > > >   struct svec *ipv6_addrs);
> > > >
> > > > +struct sb_lb_table;
> > > > +struct sb_lb_record;
> > > > +static void sb_lb_table_init(struct sb_lb_table *);
> > > > +static void sb_lb_table_clear(struct sb_lb

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

[Numan Siddique]

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:
> > >
> > > From: Numan Siddique 
> > >
> > > Any changes to northd engine node due to load balancers
> > > are now handled in 'sync_to_sb_lb' node to sync the changed
> > > load balancers to SB load balancers.
> > >
> > > The logic to sync the SB load balancers is changed a bit and it
> > > now mimics the SB lflow sync.
> > >
> > > Below are the scale testing results done with all the patches applied
> > > in this series using ovn-heater.  The test ran the scenario  -
> > > ocp-500-density-heavy.yml [1].
> > >
> > > The resuts are:
> > >
> > >
> > ---
> > > Min (s) Median (s)  90%ile (s)
> >  99%ile (s)  Max (s) Mean (s)Total (s)   Count
> > Failed
> > >
> > ---
> > > Iteration Total 0.1368831.1290161.192001
> >  1.2041671.2127280.66501783.127099   125 0
> > > Namespace.add_ports 0.0052160.0057360.007034
> >  0.0154860.0189780.0062110.776373125 0
> > > WorkerNode.bind_port0.0350300.0460820.052469
> >  0.0582930.0603110.04597311.493259   250 0
> > > WorkerNode.ping_port0.0050570.0067271.047692
> >  1.0692531.0713360.26689666.724094   250 0
> > >
> > ---
> > >
> > > The results with the present main [2] are:
> > >
> > >
> > ---
> > > Min (s) Median (s)  90%ile (s)
> >  99%ile (s)  Max (s) Mean (s)Total (s)   Count
> > Failed
> > >
> > ---
> > > Iteration Total 0.1354912.2238053.311270
> >  3.3390783.3453461.729172216.146495  125 0
> > > Namespace.add_ports 0.0053800.0057440.006819
> >  0.0187730.0208000.0062920.786532125 0
> > > WorkerNode.bind_port0.0341790.0460550.053488
> >  0.0588010.0710430.04611711.529311   250 0
> > > WorkerNode.ping_port0.0049560.0069523.086952
> >  3.1917433.1928070.791544197.886026  250 0
> > >
> > ---
> > >
> > > [1] -
> > https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
> > > [2] - 2a12cda890a7("controller, northd: Wait for cleanup before replying
> > to exit")
> > >
> > > Signed-off-by: Numan Siddique 
> > > ---
> > >  northd/en-sync-sb.c  | 445 +--
> > >  northd/inc-proc-northd.c |   1 +
> > >  northd/lflow-mgr.c   | 196 ++---
> > >  northd/lflow-mgr.h   |  23 +-
> > >  northd/northd.c  | 238 -
> > >  northd/northd.h  |   6 -
> > >  tests/ovn-northd.at  | 103 +
> > >  7 files changed, 585 insertions(+), 427 deletions(-)
> > >
> > > diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
> > > index 73b30272c4..62c5dbd20f 100644
> > > --- a/northd/en-sync-sb.c
> > > +++ b/northd/en-sync-sb.c
> > > @@ -30,6 +30,7 @@
> > >  #include "lib/ovn-nb-idl.h"
> > >  #include "lib/ovn-sb-idl.h"
> > >  #include "lib/ovn-util.h"
> > > +#include "lflow-mgr.h"
> > >  #include "northd.h"
> > >
> > >  #include "openvswitch/vlog.h"
> > > @@ -53,6 +54,38 @@ static void build_port_group_address_set(const struct
> > nbrec_port_group *,
> > >   struct svec *ipv4_addrs,
> > >   struct svec *ipv6_addrs);
> > >
> > > +struct sb_lb_table;
> > > +struct sb_lb_record;
> > > +static void sb_lb_table_init(struct sb_lb_table *);
> > > +static void sb_lb_table_clear(struct sb_lb_table *);
> > > +static struct sb_lb_record *sb_lb_table_find(struct hmap *sb_lbs,
> > > + const struct uuid *);
> > > +static void sb_lb_table_build_and_sync(struct sb_lb_table *,
> > > +   

[ovs-dev] [OVN coredump with ssl-ciphers args]

[aginwala]

Hi:

When setting extra args like ssl-cipers for ovn-controller, it results in
coredump on branch 23.09
compiled with  --with-ovs-source and  --with-ovs-build option, OVS
(branch-3.2)

dump:
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `ovn-controller
--ssl-ciphers=HIGH:!aNULL:!MD5:@SECLEVEL=1 unix:/var/run/openvsw'
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x7fb13d7cf859 in __GI_abort () at abort.c:79
#2  0x563257fac75c in main (argc=, argv=)
at controller/ovn-controller.c:6019
(gdb) frame 2
#2  0x563257fac75c in main (argc=, argv=)
at controller/ovn-controller.c:6019
6019OVS_NOT_REACHED();
(gdb) quit

##ovn-controller --version
ovn-controller 23.09.1
Open vSwitch Library 3.2.2
OpenFlow versions 0x6:0x6
SB DB Schema 20.29.0

##Same happens even on trying with any ovn-* commands
ovn-nbctl --ssl-ciphers='xx'
Aborted (core dumped)
ovn-nbctl --version
ovn-nbctl 23.09.1
Open vSwitch Library 3.2.2
DB Schema 7.1.0

## back trace for ovn-nbctl
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) frame 2
#2  0x562c759485aa in apply_options_direct
(local_options=0x7ffcaa25fbb0, n=1, parsed_options=,
dbctl_options=0x7ffcaa25fc40) at utilities/ovn-dbctl.c:621
621OVS_NOT_REACHED();

--ssl-ciphers works fine when using ovn 20.03 ; directly using ovn debian
ovn-controller 20.03.2
Open vSwitch Library 2.13.8
OpenFlow versions 0x4:0x4
SB DB Schema 2.7.0

## underlying ovs
~/ovn# ovs-vsctl --version
ovs-vsctl (Open vSwitch) 2.16.8
DB Schema 8.3.0

#Kernel/distio:
5.4.0-167-generic/Ubuntu 20.04.6 LTS


To avoid invalidating certs on already running computes setup with old  ovs
pki infra, setting ciphers to HIGH:!aNULL:!MD5:@SECLEVEL=1 works fine part
of bumping  to newer 20.x and avoid connectivity failures to control plane
due mostly due to below error.
SSL_connect: error:1416F086:SSL
routines:tls_process_server_certificate:certificate verify failed while
connecting to control plane.


Not sure if it's a known issue with newer OVS stream-ssl. Core file
attached.


Regards,
Ali
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 03/12] ci: Update the GitHub Ubuntu runner image to Ubuntu 22.04.

[Ilya Maximets]

On 1/8/24 22:02, Ilya Maximets wrote:
> On 1/8/24 17:22, Eelco Chaudron wrote:
>>
>>
>> On 8 Jan 2024, at 17:07, Ilya Maximets wrote:
>>
>>> On 1/8/24 17:01, Eelco Chaudron wrote:


 On 2 Jan 2024, at 12:19, Ilya Maximets wrote:

> On 12/19/23 13:41, Eelco Chaudron wrote:
>> Updating this image is a requirement for the kernel system-traffic
>> tests to pass on Ubuntu. In addition, 20.04 might be replaced,
>> as soon as 24.04 comes out. Or we need to do this when it becomes
>> EOL in April 2025.
>>
>> Signed-off-by: Eelco Chaudron 
>> Acked-by: Simon Horman 
>> ---
>>  .github/workflows/build-and-test.yml |4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/.github/workflows/build-and-test.yml 
>> b/.github/workflows/build-and-test.yml
>> index 5d441157c..acb57ac46 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -12,7 +12,7 @@ jobs:
>>  name: dpdk gcc
>>  outputs:
>>dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
>> -runs-on: ubuntu-20.04
>> +runs-on: ubuntu-22.04
>>  timeout-minutes: 30
>>
>>  steps:
>> @@ -89,7 +89,7 @@ jobs:
>>TESTSUITE:   ${{ matrix.testsuite }}
>>
>>  name: linux ${{ join(matrix.*, ' ') }}
>> -runs-on: ubuntu-20.04
>> +runs-on: ubuntu-22.04
>>  timeout-minutes: 30
>>
>>  strategy:
>>
>
> Hi, Eelco.  Could you also apply this to branches down to 2.17?
> We'll need to keep the image updated there as well since they are
> going to be supported likely beyond 20.04 availability in GHA.
>
> Assuming these branches should work fine with 22.04 (I didn't check).

 I was trying 2.17 and up, and it works from 3.1 as we ditched the kernel 
 compilation.
 On 2.17 and 3.0:
>>>
>>> Hmm.  We're not building the kernel module on 3.0. Are you sure?
>>
>> It builds 5.3 for AFXDP, 
>> https://github.com/chaudron/ovs/actions/runs/7449650175/job/20266758856
> 
> Ah, makes sense.
> 
> I think that we can move this one to 5.4.  We're only using
> 5.3 because it was the first kernel introducing AF_XDP and
> it doesn't support needs_wakeup functionality and we wanted
> to test this code path for some reason.  However, users of
> AF_XDP should not really use that kernel anyway, it is way
> too old and doesn't support important functionality like the
> 'needs_wakeup' flags.  Switching to 5.4 should be fine in
> this case.  And we should actually make needs_wakeup support
> mandatory  at some point and remove the conditional compilation.
> Switching to 5.4 should cover the 3.0 branch.  5.4 is also
> a longterm release.
> 
> For the 2.17, it seems the build fails on kernel 3.16.
> 3.16 got its final release in 2020 along with the end of life
> of Debian 8.  So, I think, it's fine to just remove 3.16
> from the test matrix.

The 'test 3.16' tests do not really require the kernel, i.e.
do not rely on the kernel version much.  Can potentially be
bumped to 4.14, which is the currently oldest supported
upstream kernel.

> 
> What do you think?
> 
>>
>> //Eelco
>>

 HOSTLD  scripts/dtc/dtc
   /usr/bin/ld: scripts/dtc/dtc-parser.tab.o:(.bss+0x20): multiple 
 definition of `yylloc'; scripts/dtc/dtc-lexer.lex.o:(.bss+0x0): first 
 defined here
   collect2: error: ld returned 1 exit status
   make[1]: *** [scripts/Makefile.host:99: scripts/dtc/dtc] Error 1
   make: *** [Makefile:1281: scripts_dtc] Error 2
   Error: Process completed with exit code 2.

 HOSTLD  arch/x86/tools/relocs
   /usr/bin/ld: arch/x86/tools/relocs_64.o:(.bss+0x0): multiple definition 
 of `per_cpu_load_addr'; arch/x86/tools/relocs_32.o:(.bss+0x0): first 
 defined here
   collect2: error: ld returned 1 exit status
   make[1]: *** [scripts/Makefile.host:127: arch/x86/tools/relocs] Error 1

 A quick search online resulted, in using an older compiler, or applying a 
 kernel patch :)

 I guess we could apply the patches needed for the specific versions, but 
 maybe you have better ideas?

 //Eelco

>>
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to include IDL "spurious delete" fix.

[Ilya Maximets]

On 1/8/24 16:40, Dumitru Ceara wrote:
> Specifically the following commit:
>   4102674b3e ovsdb-idl: Preserve change_seqno when deleting rows.
> 
> Without it, in specific cases, the IDL might incorrectly report deletion
> of yet to be seen records.
> 
> Signed-off-by: Dumitru Ceara 
> ---
> NOTE: when backporting this, please make sure the corresponding OVS
> branch-3.X versions of the submodule versions are used.  The IDl fix is
> backported to all required versions.
> ---
>  controller/ofctrl.c | 2 +-
>  ovs | 2 +-
>  tests/test-ovn.c| 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

I didn't test this much, but the change looks correct:

Acked-by: Ilya Maximets 

> 
> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> index 7aac0128bc..cb460a2a47 100644
> --- a/controller/ofctrl.c
> +++ b/controller/ofctrl.c
> @@ -3045,7 +3045,7 @@ ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, 
> const char *flow_s,
>  uint64_t packet_stub[128 / 8];
>  struct dp_packet packet;
>  dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> -flow_compose(&packet, &uflow, NULL, 64);
> +flow_compose(&packet, &uflow, NULL, 64, false);
>  
>  uint64_t ofpacts_stub[1024 / 8];
>  struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
> diff --git a/ovs b/ovs
> index fdbf0bb2ae..4102674b3e 16
> --- a/ovs
> +++ b/ovs
> @@ -1 +1 @@
> -Subproject commit fdbf0bb2aed53e70b455eb1adcfda8d8278ea690
> +Subproject commit 4102674b3ecadb0e20e512cc661cddbbc4b3d1f6
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index aaf2825edc..5326c6e692 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -999,7 +999,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct 
> shash *symtab,
>  
>  if (operation >= OP_FLOW) {
>  bool found = classifier_lookup(&cls, OVS_VERSION_MIN,
> -   &f, NULL) != NULL;
> +   &f, NULL, NULL) != NULL;
>  if (expected != found) {
>  struct ds expr_s, modified_s;
>  
> @@ -1238,7 +1238,7 @@ test_expr_to_packets(struct ovs_cmdl_context *ctx 
> OVS_UNUSED)
>  uint64_t packet_stub[128 / 8];
>  struct dp_packet packet;
>  dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> -flow_compose(&packet, &uflow, NULL, 64);
> +flow_compose(&packet, &uflow, NULL, 64, false);
>  
>  struct ds output = DS_EMPTY_INITIALIZER;
>  const uint8_t *buf = dp_packet_data(&packet);

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 03/12] ci: Update the GitHub Ubuntu runner image to Ubuntu 22.04.

[Ilya Maximets]

On 1/8/24 17:22, Eelco Chaudron wrote:
> 
> 
> On 8 Jan 2024, at 17:07, Ilya Maximets wrote:
> 
>> On 1/8/24 17:01, Eelco Chaudron wrote:
>>>
>>>
>>> On 2 Jan 2024, at 12:19, Ilya Maximets wrote:
>>>
 On 12/19/23 13:41, Eelco Chaudron wrote:
> Updating this image is a requirement for the kernel system-traffic
> tests to pass on Ubuntu. In addition, 20.04 might be replaced,
> as soon as 24.04 comes out. Or we need to do this when it becomes
> EOL in April 2025.
>
> Signed-off-by: Eelco Chaudron 
> Acked-by: Simon Horman 
> ---
>  .github/workflows/build-and-test.yml |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/.github/workflows/build-and-test.yml 
> b/.github/workflows/build-and-test.yml
> index 5d441157c..acb57ac46 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -12,7 +12,7 @@ jobs:
>  name: dpdk gcc
>  outputs:
>dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
> -runs-on: ubuntu-20.04
> +runs-on: ubuntu-22.04
>  timeout-minutes: 30
>
>  steps:
> @@ -89,7 +89,7 @@ jobs:
>TESTSUITE:   ${{ matrix.testsuite }}
>
>  name: linux ${{ join(matrix.*, ' ') }}
> -runs-on: ubuntu-20.04
> +runs-on: ubuntu-22.04
>  timeout-minutes: 30
>
>  strategy:
>

 Hi, Eelco.  Could you also apply this to branches down to 2.17?
 We'll need to keep the image updated there as well since they are
 going to be supported likely beyond 20.04 availability in GHA.

 Assuming these branches should work fine with 22.04 (I didn't check).
>>>
>>> I was trying 2.17 and up, and it works from 3.1 as we ditched the kernel 
>>> compilation.
>>> On 2.17 and 3.0:
>>
>> Hmm.  We're not building the kernel module on 3.0. Are you sure?
> 
> It builds 5.3 for AFXDP, 
> https://github.com/chaudron/ovs/actions/runs/7449650175/job/20266758856

Ah, makes sense.

I think that we can move this one to 5.4.  We're only using
5.3 because it was the first kernel introducing AF_XDP and
it doesn't support needs_wakeup functionality and we wanted
to test this code path for some reason.  However, users of
AF_XDP should not really use that kernel anyway, it is way
too old and doesn't support important functionality like the
'needs_wakeup' flags.  Switching to 5.4 should be fine in
this case.  And we should actually make needs_wakeup support
mandatory  at some point and remove the conditional compilation.
Switching to 5.4 should cover the 3.0 branch.  5.4 is also
a longterm release.

For the 2.17, it seems the build fails on kernel 3.16.
3.16 got its final release in 2020 along with the end of life
of Debian 8.  So, I think, it's fine to just remove 3.16
from the test matrix.

What do you think?

> 
> //Eelco
> 
>>>
>>> HOSTLD  scripts/dtc/dtc
>>>   /usr/bin/ld: scripts/dtc/dtc-parser.tab.o:(.bss+0x20): multiple 
>>> definition of `yylloc'; scripts/dtc/dtc-lexer.lex.o:(.bss+0x0): first 
>>> defined here
>>>   collect2: error: ld returned 1 exit status
>>>   make[1]: *** [scripts/Makefile.host:99: scripts/dtc/dtc] Error 1
>>>   make: *** [Makefile:1281: scripts_dtc] Error 2
>>>   Error: Process completed with exit code 2.
>>>
>>> HOSTLD  arch/x86/tools/relocs
>>>   /usr/bin/ld: arch/x86/tools/relocs_64.o:(.bss+0x0): multiple definition 
>>> of `per_cpu_load_addr'; arch/x86/tools/relocs_32.o:(.bss+0x0): first 
>>> defined here
>>>   collect2: error: ld returned 1 exit status
>>>   make[1]: *** [scripts/Makefile.host:127: arch/x86/tools/relocs] Error 1
>>>
>>> A quick search online resulted, in using an older compiler, or applying a 
>>> kernel patch :)
>>>
>>> I guess we could apply the patches needed for the specific versions, but 
>>> maybe you have better ideas?
>>>
>>> //Eelco
>>>
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS "soft freeze" for 3.3 is in effect.

[Frode Nordahl]

man. 8. jan. 2024, 21:33 skrev Ilya Maximets :

> On 1/8/24 17:59, Frode Nordahl wrote:
> > Hello, Ilya and OVS developers,
> >
> > On Wed, Jan 3, 2024 at 12:59 AM Ilya Maximets 
> wrote:
> >>
> >> Hi.  As described in Documentation/internals/release-process.rst, we are
> >> in a "soft freeze" state:
> >>
> >>During the freeze, we ask committers to refrain from applying
> patches that
> >>add new features unless those patches were already being publicly
> discussed
> >>and reviewed before the freeze began.  Bug fixes are welcome at any
> time.
> >>Please propose and discuss exceptions on ovs-dev.
> >>
> >> As far as I can tell, most of the active patches currently on the
> mailing
> >> list were already reviewed to some extent, so they do qualify for being
> >> accepted during the soft freeze (potentially with some changes made in
> the
> >> additional review rounds).  We do also await new versions for several
> patches
> >> and patch sets for which changes were requested.  These, of course, can
> be
> >> accepted as well.
> >>
> >> If there are some new patches that never been reviewed / not posted yet,
> >> please, propose an exception in reply to this email and it can be
> discussed.
> >
> > On the back of your review feedback in [0] I've been working on an
> > implementation of a cooperative multitasking module for use in OVSDB.
> > The goal of the work is to improve OVSDB's ability to maintain RAFT
> > cluster communication even when it is performing long running tasks
> > such as database schema conversion, processing of monitors and
> > similar. I hereby request a freeze exception for this work.
> >
> > I have posted a RFC with the current state of the work [1], it is
> > posted as RFC because I am still in a close loop with testing to close
> > some cases where the time threshold is still overrun. I'll hopefully
> > figure out the remaining places we need to yield to avoid these and
> > then post a formal patch series with documented testing.
> >
> > 0:
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410262.html
> > 1:
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410593.html
> >
>
> Hi, Frode.  I think it's fine to get this during a soft freeze
> as the code seems to not be overly complex and it was discussed
> a few times before.


Thanks!

Though, on a quick glance, I think that [1]
> takes a little bit of a scattershot approach to the problem in
> terms of yielding seemingly in way too many places, but that,
> I'm sure, can be sorted out during a review process.  I'll take
> a closer look later this week.
>

Yes, the RFC patch set is a snapshot in time while in the middle of trying
to find the culprit and I anticipate the final version to yield a lot less,
especially in monitor.c.

--
Frode Nordahl

Best regards, Ilya Maximets.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS "soft freeze" for 3.3 is in effect.

[Ilya Maximets]

On 1/8/24 17:59, Frode Nordahl wrote:
> Hello, Ilya and OVS developers,
> 
> On Wed, Jan 3, 2024 at 12:59 AM Ilya Maximets  wrote:
>>
>> Hi.  As described in Documentation/internals/release-process.rst, we are
>> in a "soft freeze" state:
>>
>>During the freeze, we ask committers to refrain from applying patches that
>>add new features unless those patches were already being publicly 
>> discussed
>>and reviewed before the freeze began.  Bug fixes are welcome at any time.
>>Please propose and discuss exceptions on ovs-dev.
>>
>> As far as I can tell, most of the active patches currently on the mailing
>> list were already reviewed to some extent, so they do qualify for being
>> accepted during the soft freeze (potentially with some changes made in the
>> additional review rounds).  We do also await new versions for several patches
>> and patch sets for which changes were requested.  These, of course, can be
>> accepted as well.
>>
>> If there are some new patches that never been reviewed / not posted yet,
>> please, propose an exception in reply to this email and it can be discussed.
> 
> On the back of your review feedback in [0] I've been working on an
> implementation of a cooperative multitasking module for use in OVSDB.
> The goal of the work is to improve OVSDB's ability to maintain RAFT
> cluster communication even when it is performing long running tasks
> such as database schema conversion, processing of monitors and
> similar. I hereby request a freeze exception for this work.
> 
> I have posted a RFC with the current state of the work [1], it is
> posted as RFC because I am still in a close loop with testing to close
> some cases where the time threshold is still overrun. I'll hopefully
> figure out the remaining places we need to yield to avoid these and
> then post a formal patch series with documented testing.
> 
> 0: https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410262.html
> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410593.html
> 

Hi, Frode.  I think it's fine to get this during a soft freeze
as the code seems to not be overly complex and it was discussed
a few times before.  Though, on a quick glance, I think that [1]
takes a little bit of a scattershot approach to the problem in
terms of yielding seemingly in way too many places, but that,
I'm sure, can be sorted out during a review process.  I'll take
a closer look later this week.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 07/22] jsonrpc-server: Add functions to convert jsonrpc options to/from json.

[Mike Pattrick]

On Wed, Dec 13, 2023 at 8:05 PM Ilya Maximets  wrote:
>
> These functions will be needed when we'll need to load/save
> configuration of each OVSDB remote separately.
>
> The parsing function is written in a way that it updates the
> provided options and doesn't create a new structure.  This
> is done in order for different callers to have their own
> default values and only update them with what is provided
> by the user explicitly.  For example, replication and relay
> have different default probe intervals.
>
> Signed-off-by: Ilya Maximets 

Acked-by: Mike Pattrick 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS "soft freeze" for 3.3 is in effect.

[Frode Nordahl]

Hello, Ilya and OVS developers,

On Wed, Jan 3, 2024 at 12:59 AM Ilya Maximets  wrote:
>
> Hi.  As described in Documentation/internals/release-process.rst, we are
> in a "soft freeze" state:
>
>During the freeze, we ask committers to refrain from applying patches that
>add new features unless those patches were already being publicly discussed
>and reviewed before the freeze began.  Bug fixes are welcome at any time.
>Please propose and discuss exceptions on ovs-dev.
>
> As far as I can tell, most of the active patches currently on the mailing
> list were already reviewed to some extent, so they do qualify for being
> accepted during the soft freeze (potentially with some changes made in the
> additional review rounds).  We do also await new versions for several patches
> and patch sets for which changes were requested.  These, of course, can be
> accepted as well.
>
> If there are some new patches that never been reviewed / not posted yet,
> please, propose an exception in reply to this email and it can be discussed.

On the back of your review feedback in [0] I've been working on an
implementation of a cooperative multitasking module for use in OVSDB.
The goal of the work is to improve OVSDB's ability to maintain RAFT
cluster communication even when it is performing long running tasks
such as database schema conversion, processing of monitors and
similar. I hereby request a freeze exception for this work.

I have posted a RFC with the current state of the work [1], it is
posted as RFC because I am still in a close loop with testing to close
some cases where the time threshold is still overrun. I'll hopefully
figure out the remaining places we need to yield to avoid these and
then post a formal patch series with documented testing.

0: https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410262.html
1: https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410593.html

-- 
Frode Nordahl

> We should branch for version 3.3 in two weeks from now, on Wednesday, Jan 17.
> With that, release should be on Friday, Feb 16.
>
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Frode Nordahl]

Posting this as RFC as I'm not fully content with my test results.

With this series there are still situations where we overrun the
time threashold and subsequently get into the situation we are trying
to fix with this series.

During testing I found a destroy call [0] to take around 500ms, but
as far as I read the code that ends up being a single call to free()
so that does not quite make sense.  Working on replicating in a
different test bed and continuing to pin-point the source of the
last time threshold overruns.  Will post a follow-up soon.

0: 
https://github.com/openvswitch/ovs/blob/4102674b3ecadb0e20e512cc661cddbbc4b3d1f6/ovsdb/monitor.c#L1290

Introduce cooperative multitasking module which allow 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 server does not interfere with stable maintenance of the
RAFT cluster in subsequent patches.

Relevant discussion:
https://mail.openvswitch.org/pipermail/ovs-discuss/2022-December/052140.html
https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410262.html


Frode Nordahl (5):
  timeval: Make timewarp available for internal callers.
  lib: Introduce cooperative multitasking module.
  ovsdb/raft: Register for cooperative multitasking.
  json: Add yielding serialized object create function.
  ovsdb-server: Make use of cooperative multitasking.

 NEWS   |   8 +
 include/openvswitch/json.h |   4 +-
 lib/automake.mk|   3 +
 lib/cooperative-multitasking-private.h |  30 +++
 lib/cooperative-multitasking.c | 192 ++
 lib/cooperative-multitasking.h |  42 
 lib/json.c |  43 ++--
 lib/timeval.c  |  56 --
 lib/timeval.h  |   4 +
 ovsdb/file.c   |   2 +
 ovsdb/jsonrpc-server.c |   3 +
 ovsdb/monitor.c|  25 ++-
 ovsdb/ovsdb-server.c   |   5 +
 ovsdb/raft.c   |  12 +-
 tests/automake.mk  |   1 +
 tests/library.at   |  10 +
 tests/ovsdb-server.at  |   1 +
 tests/test-cooperative-multitasking.c  | 259 +
 18 files changed, 673 insertions(+), 27 deletions(-)
 create mode 100644 lib/cooperative-multitasking-private.h
 create mode 100644 lib/cooperative-multitasking.c
 create mode 100644 lib/cooperative-multitasking.h
 create mode 100644 tests/test-cooperative-multitasking.c

-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC 2/5] lib: Introduce cooperative multitasking module.

[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 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 server does not interfere with stable maintenance of the
RAFT cluster in subsequent patches.

Suggested-by: Ilya Maximets 
Signed-off-by: Frode Nordahl 
---
 lib/automake.mk|   3 +
 lib/cooperative-multitasking-private.h |  30 +++
 lib/cooperative-multitasking.c | 192 ++
 lib/cooperative-multitasking.h |  42 
 tests/automake.mk  |   1 +
 tests/library.at   |  10 +
 tests/ovsdb-server.at  |   1 +
 tests/test-cooperative-multitasking.c  | 259 +
 8 files changed, 538 insertions(+)
 create mode 100644 lib/cooperative-multitasking-private.h
 create mode 100644 lib/cooperative-multitasking.c
 create mode 100644 lib/cooperative-multitasking.h
 create mode 100644 tests/test-cooperative-multitasking.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 0dc8a35cc..8596171c6 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -94,6 +94,9 @@ lib_libopenvswitch_la_SOURCES = \
lib/conntrack-other.c \
lib/conntrack.c \
lib/conntrack.h \
+   lib/cooperative-multitasking.c \
+   lib/cooperative-multitasking.h \
+   lib/cooperative-multitasking-private.h \
lib/coverage.c \
lib/coverage.h \
lib/cpu.c \
diff --git a/lib/cooperative-multitasking-private.h 
b/lib/cooperative-multitasking-private.h
new file mode 100644
index 0..3f3626463
--- /dev/null
+++ b/lib/cooperative-multitasking-private.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2024 Canonical Ltd.
+ *
+ * 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.
+ */
+
+#ifndef COOPERATIVE_MULTITASKING_PRIVATE_H
+#define COOPERATIVE_MULTITASKING_PRIVATE_H 1
+
+#include "openvswitch/hmap.h"
+
+struct cooperative_multitasking_callback {
+struct hmap_node node;
+void (*cb)(void *);
+void *arg;
+long long int time_threshold;
+long long int last_run;
+};
+
+#endif /* COOPERATIVE_MULTITASKING_PRIVATE_H */
diff --git a/lib/cooperative-multitasking.c b/lib/cooperative-multitasking.c
new file mode 100644
index 0..4ebe3b112
--- /dev/null
+++ b/lib/cooperative-multitasking.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright (c) 2023 Canonical Ltd.
+ *
+ * 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 "backtrace.h"
+#include "cooperative-multitasking-private.h"
+#include "cooperative-multitasking.h"
+#include "hash.h"
+#include "openvswitch/hmap.h"
+#include "openvswitch/vlog.h"
+#include "timeval.h"
+
+VLOG_DEFINE_THIS_MODULE(cooperative_multitasking);
+
+static struct hmap *cooperative_multitasking_callbacks = NULL;
+
+/* One time initialization for process that wants to make use of cooperative
+ * multitasking module.  References to data is stored in 'hmap_container' and
+ * will be referenced by all calls to this module.  The ownership of the
+ * container itself remains with the caller while the data in the hmap is owned
+ * by this module and must be freed with a call to
+ * cooperative_multitasking_destroy().
+ *
+ * The purpose of having the caller own 'hmap_container' is:
+ * 1) Allow runtime decision whether to use cooperative multitasking without
+ *having to pass data between loosely connected parts of a program.  This
+ *is useful for the raft code which is consumed by both the ovsdb-server
+ *daemon and the ovsdb-tool CLI utility.
+ * 2) Allow inspection of internal data by unit tests. */
+void
+cooper

[ovs-dev] [RFC 4/5] json: Add yielding serialized object create function.

[Frode Nordahl]

Creating a serialized JSON object is a time consuming process.

Add a json_serialized_object_create_with_yield() function that
makes use of the cooperative multitasking module to yield during
processing, allowing time sensitive tasks in other parts of the
program to be completed during processing.

Signed-off-by: Frode Nordahl 
---
 include/openvswitch/json.h |  4 +++-
 lib/json.c | 43 +++---
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/include/openvswitch/json.h b/include/openvswitch/json.h
index 35b403c29..252425393 100644
--- a/include/openvswitch/json.h
+++ b/include/openvswitch/json.h
@@ -81,6 +81,7 @@ struct json *json_boolean_create(bool);
 struct json *json_string_create(const char *);
 struct json *json_string_create_nocopy(char *);
 struct json *json_serialized_object_create(const struct json *);
+struct json *json_serialized_object_create_with_yield(const struct json *);
 struct json *json_integer_create(long long int);
 struct json *json_real_create(double);
 
@@ -137,7 +138,8 @@ struct json *json_from_stream(FILE *stream);
 
 enum {
 JSSF_PRETTY = 1 << 0,   /* Multiple lines with indentation, if true. */
-JSSF_SORT = 1 << 1  /* Object members in sorted order, if true. */
+JSSF_SORT = 1 << 1, /* Object members in sorted order, if true. */
+JSSF_YIELD = 1 << 2 /* Yield during processing */
 };
 char *json_to_string(const struct json *, int flags);
 void json_to_ds(const struct json *, int flags, struct ds *);
diff --git a/lib/json.c b/lib/json.c
index aded8bb01..9a264777a 100644
--- a/lib/json.c
+++ b/lib/json.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 
+#include "cooperative-multitasking.h"
 #include "openvswitch/dynamic-string.h"
 #include "hash.h"
 #include "openvswitch/shash.h"
@@ -181,14 +182,26 @@ json_string_create(const char *s)
 return json_string_create_nocopy(xstrdup(s));
 }
 
-struct json *
-json_serialized_object_create(const struct json *src)
+static struct json *
+json_serialized_object_create__(const struct json *src, int flags)
 {
 struct json *json = json_create(JSON_SERIALIZED_OBJECT);
-json->string = json_to_string(src, JSSF_SORT);
+json->string = json_to_string(src, flags);
 return json;
 }
 
+struct json *
+json_serialized_object_create(const struct json *src)
+{
+return json_serialized_object_create__(src, JSSF_SORT);
+}
+
+struct json *
+json_serialized_object_create_with_yield(const struct json *src)
+{
+return json_serialized_object_create__(src, JSSF_SORT | JSSF_YIELD);
+}
+
 struct json *
 json_array_create_empty(void)
 {
@@ -1525,7 +1538,7 @@ static void json_serialize_object(const struct shash 
*object,
   struct json_serializer *);
 static void json_serialize_array(const struct json_array *,
  struct json_serializer *);
-static void json_serialize_string(const char *, struct ds *);
+static void json_serialize_string(const char *, struct json_serializer *);
 
 /* Converts 'json' to a string in JSON format, encoded in UTF-8, and returns
  * that string.  The caller is responsible for freeing the returned string,
@@ -1598,7 +1611,7 @@ json_serialize(const struct json *json, struct 
json_serializer *s)
 break;
 
 case JSON_STRING:
-json_serialize_string(json->string, ds);
+json_serialize_string(json->string, s);
 break;
 
 case JSON_SERIALIZED_OBJECT:
@@ -1631,7 +1644,7 @@ json_serialize_object_member(size_t i, const struct 
shash_node *node,
 indent_line(s);
 }
 
-json_serialize_string(node->name, ds);
+json_serialize_string(node->name, s);
 ds_put_char(ds, ':');
 if (s->flags & JSSF_PRETTY) {
 ds_put_char(ds, ' ');
@@ -1734,7 +1747,7 @@ static const char *chars_escaping[256] = {
 };
 
 static void
-json_serialize_string(const char *string, struct ds *ds)
+json_serialize_string(const char *string, struct json_serializer *s)
 {
 uint8_t c;
 uint8_t c2;
@@ -1742,26 +1755,32 @@ json_serialize_string(const char *string, struct ds *ds)
 const char *escape;
 const char *start;
 
-ds_put_char(ds, '"');
+ds_put_char(s->ds, '"');
 count = 0;
 start = string;
 while ((c = *string++) != '\0') {
+if (s->flags & JSSF_YIELD) {
+cooperative_multitasking_yield();
+}
 if (c >= ' ' && c != '"' && c != '\\') {
 count++;
 } else {
 if (count) {
-ds_put_buffer(ds, start, count);
+ds_put_buffer(s->ds, start, count);
 count = 0;
 }
 start = string;
 escape = chars_escaping[c];
 while ((c2 = *escape++) != '\0') {
-ds_put_char(ds, c2);
+if (s->flags & JSSF_YIELD) {
+cooperative_multitasking_yield();
+}
+ds_put_char(s->ds, c2);
   

[ovs-dev] [RFC 5/5] ovsdb-server: Make use of cooperative multitasking.

[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 reconnecting
clients is time consuming, yield during processing of jsonrpc
server requests.

The processing of OVSDB monitors for reconnecting clients after a
schema conversion is time consuming and a primary offender for
missing deadlines, yield during processing.

TODO: Document testing done with size/timing results.

Signed-off-by: Frode Nordahl 
---
 NEWS   |  8 
 ovsdb/file.c   |  2 ++
 ovsdb/jsonrpc-server.c |  3 +++
 ovsdb/monitor.c| 25 -
 ovsdb/ovsdb-server.c   |  5 +
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 270ed6673..643fddc98 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,9 @@ Post-v3.2.0
from older version is supported but it may trigger more leader elections
during the process, and error logs complaining unrecognized fields may
be observed on old nodes.
+ * Make use of cooperative multitasking to ensure stable maintenance of
+   RAFT cluster during long running processing such as online schema
+   conversion.
- OpenFlow:
  * NXT_CT_FLUSH extension is updated to support flushing connections
based on mark and labels.  'ct-flush' command of ovs-ofctl updated
@@ -36,6 +39,11 @@ Post-v3.2.0
The existing behaviour is maintained and a non key:value pair value
will be applied to all other PMD thread cores.'pmd-sleep-show' is
updated to show the maximum sleep for each PMD thread core.
+   - lib:
+ * Introduce cooperative multitasking module which allow us to interleave
+   important processing with long running tasks while avoiding the
+   additional resource consumption of threads and complexity of
+   asynchronous state machines.
 
 
 v3.2.0 - 17 Aug 2023
diff --git a/ovsdb/file.c b/ovsdb/file.c
index 8bd1d4af3..fb4cbfb77 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -23,6 +23,7 @@
 
 #include "bitmap.h"
 #include "column.h"
+#include "cooperative-multitasking.h"
 #include "log.h"
 #include "openvswitch/json.h"
 #include "lockfile.h"
@@ -308,6 +309,7 @@ ovsdb_convert_table(struct ovsdb_txn *txn,
 }
 
 HMAP_FOR_EACH (src_row, hmap_node, &src_table->rows) {
+cooperative_multitasking_yield();
 struct ovsdb_row *dst_row = ovsdb_row_create(dst_table);
 *ovsdb_row_get_uuid_rw(dst_row) = *ovsdb_row_get_uuid(src_row);
 
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index a3ca48a7b..17215eecd 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -21,6 +21,7 @@
 
 #include "bitmap.h"
 #include "column.h"
+#include "cooperative-multitasking.h"
 #include "openvswitch/dynamic-string.h"
 #include "monitor.h"
 #include "openvswitch/json.h"
@@ -599,6 +600,7 @@ ovsdb_jsonrpc_session_run_all(struct ovsdb_jsonrpc_remote 
*remote)
 struct ovsdb_jsonrpc_session *s;
 
 LIST_FOR_EACH_SAFE (s, node, &remote->sessions) {
+cooperative_multitasking_yield();
 int error = ovsdb_jsonrpc_session_run(s);
 if (error) {
 ovsdb_jsonrpc_session_close(s);
@@ -674,6 +676,7 @@ ovsdb_jsonrpc_session_reconnect_all(struct 
ovsdb_jsonrpc_remote *remote,
 struct ovsdb_jsonrpc_session *s;
 
 LIST_FOR_EACH_SAFE (s, node, &remote->sessions) {
+cooperative_multitasking_yield();
 if (force || !s->db_change_aware) {
 jsonrpc_session_force_reconnect(s->js);
 if (comment && jsonrpc_session_is_connected(s->js)) {
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index d1e466faa..4398761bf 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -20,6 +20,7 @@
 
 #include "bitmap.h"
 #include "column.h"
+#include "cooperative-multitasking.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/json.h"
 #include "jsonrpc.h"
@@ -229,6 +230,7 @@ ovsdb_monitor_json_cache_search(const struct ovsdb_monitor 
*dbmon,
 uint32_t hash = json_cache_hash(version, change_set);
 
 HMAP_FOR_EACH_WITH_HASH(node, hmap_node, hash, &dbmon->json_cache) {
+cooperative_multitasking_yield();
 if (uuid_equals(&node->change_set_uuid, &change_set->uuid) &&
 node->version == version) {
 return node;
@@ -262,6 +264,7 @@ ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon)
 struct ovsdb_monitor_json_cache_node *node;
 
 HMAP_FOR_EACH_POP(node, hmap_node, &dbmon->json_cache) {
+cooperative_multitasking_yield();
 json_destroy(node->json);
 free(node);
 }
@@ -309,6 +312,7 @@ ovsdb_monitor_changes_row_find(
 
 HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid),
  &changes->rows) {
+cooperative_multitasking_yield();
 if (uuid_equals(uuid, &row->uuid)) {

[ovs-dev] [RFC 3/5] ovsdb/raft: Register for cooperative multitasking.

[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 overrun.

Register for cooperative multitasking so that long running
processing elsewhere in the program may yield to allow stable
maintainenance of the cluster.

Signed-off-by: Frode Nordahl 
---
 ovsdb/raft.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index 8effd9ad1..19f31b65c 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include "cooperative-multitasking.h"
 #include "hash.h"
 #include "jsonrpc.h"
 #include "lockfile.h"
@@ -423,6 +424,8 @@ raft_make_address_passive(const char *address_)
 }
 }
 
+#define RAFT_HEARTBEAT_THRESHOLD raft->election_timer / 3
+
 static struct raft *
 raft_alloc(void)
 {
@@ -446,6 +449,9 @@ raft_alloc(void)
 raft->conn_backlog_max_n_msgs = DEFAULT_MAX_BACKLOG_N_MSGS;
 raft->conn_backlog_max_n_bytes = DEFAULT_MAX_BACKLOG_N_BYTES;
 
+COOPERATIVE_MULTITASKING_REGISTER(
+&raft_run, raft, RAFT_HEARTBEAT_THRESHOLD);
+
 return raft;
 }
 
@@ -996,7 +1002,11 @@ raft_reset_election_timer(struct raft *raft)
 static void
 raft_reset_ping_timer(struct raft *raft)
 {
-raft->ping_timeout = time_msec() + raft->election_timer / 3;
+long long int now = time_msec();
+
+raft->ping_timeout = now + RAFT_HEARTBEAT_THRESHOLD;
+COOPERATIVE_MULTITASKING_UPDATE(
+&raft_run, raft, now, RAFT_HEARTBEAT_THRESHOLD);
 }
 
 static void
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[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 separate functions.

This will be used in a patch that implements unit tests for the
cooperative multitasking module.

Signed-off-by: Frode Nordahl 
---
 lib/timeval.c | 56 ---
 lib/timeval.h |  4 
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/lib/timeval.c b/lib/timeval.c
index 193c7bab1..4d4951c42 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -533,6 +533,10 @@ nsec_to_timespec(long long int nsec, struct timespec *ts)
 ts->tv_nsec = nsec;
 }
 
+#define MONOTONIC_CLOCK_HAS_WARP_CONN(c) \
+( (struct clock *) c)->large_warp.conn && \
+(uintptr_t) ( (struct clock *) c)->large_warp.conn != UINTPTR_MAX
+
 static void
 timewarp_work(void)
 {
@@ -540,6 +544,8 @@ timewarp_work(void)
 struct timespec warp;
 
 ovs_mutex_lock(&c->mutex);
+/* note that we do not use MONOTONIC_CLOCK_HAS_WARP_CONN here as we want
+ * to perform work in case of an internal caller has requested warp. */
 if (!c->large_warp.conn) {
 ovs_mutex_unlock(&c->mutex);
 return;
@@ -560,7 +566,9 @@ timewarp_work(void)
 }
 
 if (!c->large_warp.total_warp) {
-unixctl_command_reply(c->large_warp.conn, "warped");
+if (MONOTONIC_CLOCK_HAS_WARP_CONN(c)) {
+unixctl_command_reply(c->large_warp.conn, "warped");
+}
 c->large_warp.conn = NULL;
 }
 
@@ -763,17 +771,22 @@ get_cpu_usage(void)
 
 /* "time/stop" stops the monotonic time returned by e.g. time_msec() from
  * advancing, except due to later calls to "time/warp". */
-static void
-timeval_stop_cb(struct unixctl_conn *conn,
- int argc OVS_UNUSED, const char *argv[] OVS_UNUSED,
- void *aux OVS_UNUSED)
+void
+timeval_stop(void)
 {
 ovs_mutex_lock(&monotonic_clock.mutex);
 atomic_store_relaxed(&monotonic_clock.slow_path, true);
 monotonic_clock.stopped = true;
 xclock_gettime(monotonic_clock.id, &monotonic_clock.cache);
 ovs_mutex_unlock(&monotonic_clock.mutex);
+}
 
+static void
+timeval_stop_cb(struct unixctl_conn *conn,
+ int argc OVS_UNUSED, const char *argv[] OVS_UNUSED,
+ void *aux OVS_UNUSED)
+{
+timeval_stop();
 unixctl_command_reply(conn, NULL);
 }
 
@@ -787,6 +800,25 @@ timeval_stop_cb(struct unixctl_conn *conn,
  * time to run after the clock has been advanced by MSECS.
  *
  * Does not affect wall clock readings. */
+void
+timeval_warp(long long int total_warp, long long int msecs)
+{
+ovs_mutex_lock(&monotonic_clock.mutex);
+if (!MONOTONIC_CLOCK_HAS_WARP_CONN(&monotonic_clock)) {
+/* we do not have a unixctl connection for internal callers, but the
+ * pointer needs to be set for timewarp_work() to know when to
+ * perform work. */
+monotonic_clock.large_warp.conn = (struct unixctl_conn *) UINTPTR_MAX;
+}
+atomic_store_relaxed(&monotonic_clock.slow_path, true);
+monotonic_clock.large_warp.total_warp = total_warp;
+monotonic_clock.large_warp.warp = msecs;
+monotonic_clock.large_warp.main_thread_id = ovsthread_id_self();
+ovs_mutex_unlock(&monotonic_clock.mutex);
+
+timewarp_work();
+}
+
 static void
 timeval_warp_cb(struct unixctl_conn *conn,
 int argc OVS_UNUSED, const char *argv[], void *aux OVS_UNUSED)
@@ -799,25 +831,25 @@ timeval_warp_cb(struct unixctl_conn *conn,
 }
 
 ovs_mutex_lock(&monotonic_clock.mutex);
-if (monotonic_clock.large_warp.conn) {
+if (MONOTONIC_CLOCK_HAS_WARP_CONN(&monotonic_clock)) {
 ovs_mutex_unlock(&monotonic_clock.mutex);
 unixctl_command_reply_error(conn, "A previous warp in progress");
 return;
 }
-atomic_store_relaxed(&monotonic_clock.slow_path, true);
 monotonic_clock.large_warp.conn = conn;
-monotonic_clock.large_warp.total_warp = total_warp;
-monotonic_clock.large_warp.warp = msecs;
-monotonic_clock.large_warp.main_thread_id = ovsthread_id_self();
 ovs_mutex_unlock(&monotonic_clock.mutex);
 
-timewarp_work();
+timeval_warp(total_warp, msecs);
 }
 
+void
+timeval_timewarp_enable(void) {
+timewarp_enabled = true;
+}
 void
 timeval_dummy_register(void)
 {
-timewarp_enabled = true;
+timeval_timewarp_enable();
 unixctl_command_register("time/stop", "", 0, 0, timeval_stop_cb, NULL);
 unixctl_command_register("time/warp", "[large_msecs] msecs", 1, 2,
  timeval_warp_cb, NULL);
diff --git a/lib/timeval.h b/lib/timeval.h
index 502f703d4..ea7977e0c 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -81,6 +81,10 @@ long long int time_boot_msec(void);
 
 void timewarp_run(void);
 
+void timeval_timewarp_enable(

Re: [ovs-dev] [PATCH v5 03/12] ci: Update the GitHub Ubuntu runner image to Ubuntu 22.04.

[Eelco Chaudron]

On 8 Jan 2024, at 17:07, Ilya Maximets wrote:

> On 1/8/24 17:01, Eelco Chaudron wrote:
>>
>>
>> On 2 Jan 2024, at 12:19, Ilya Maximets wrote:
>>
>>> On 12/19/23 13:41, Eelco Chaudron wrote:
 Updating this image is a requirement for the kernel system-traffic
 tests to pass on Ubuntu. In addition, 20.04 might be replaced,
 as soon as 24.04 comes out. Or we need to do this when it becomes
 EOL in April 2025.

 Signed-off-by: Eelco Chaudron 
 Acked-by: Simon Horman 
 ---
  .github/workflows/build-and-test.yml |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/.github/workflows/build-and-test.yml 
 b/.github/workflows/build-and-test.yml
 index 5d441157c..acb57ac46 100644
 --- a/.github/workflows/build-and-test.yml
 +++ b/.github/workflows/build-and-test.yml
 @@ -12,7 +12,7 @@ jobs:
  name: dpdk gcc
  outputs:
dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
 -runs-on: ubuntu-20.04
 +runs-on: ubuntu-22.04
  timeout-minutes: 30

  steps:
 @@ -89,7 +89,7 @@ jobs:
TESTSUITE:   ${{ matrix.testsuite }}

  name: linux ${{ join(matrix.*, ' ') }}
 -runs-on: ubuntu-20.04
 +runs-on: ubuntu-22.04
  timeout-minutes: 30

  strategy:

>>>
>>> Hi, Eelco.  Could you also apply this to branches down to 2.17?
>>> We'll need to keep the image updated there as well since they are
>>> going to be supported likely beyond 20.04 availability in GHA.
>>>
>>> Assuming these branches should work fine with 22.04 (I didn't check).
>>
>> I was trying 2.17 and up, and it works from 3.1 as we ditched the kernel 
>> compilation.
>> On 2.17 and 3.0:
>
> Hmm.  We're not building the kernel module on 3.0. Are you sure?

It builds 5.3 for AFXDP, 
https://github.com/chaudron/ovs/actions/runs/7449650175/job/20266758856

//Eelco

>>
>> HOSTLD  scripts/dtc/dtc
>>   /usr/bin/ld: scripts/dtc/dtc-parser.tab.o:(.bss+0x20): multiple definition 
>> of `yylloc'; scripts/dtc/dtc-lexer.lex.o:(.bss+0x0): first defined here
>>   collect2: error: ld returned 1 exit status
>>   make[1]: *** [scripts/Makefile.host:99: scripts/dtc/dtc] Error 1
>>   make: *** [Makefile:1281: scripts_dtc] Error 2
>>   Error: Process completed with exit code 2.
>>
>> HOSTLD  arch/x86/tools/relocs
>>   /usr/bin/ld: arch/x86/tools/relocs_64.o:(.bss+0x0): multiple definition of 
>> `per_cpu_load_addr'; arch/x86/tools/relocs_32.o:(.bss+0x0): first defined 
>> here
>>   collect2: error: ld returned 1 exit status
>>   make[1]: *** [scripts/Makefile.host:127: arch/x86/tools/relocs] Error 1
>>
>> A quick search online resulted, in using an older compiler, or applying a 
>> kernel patch :)
>>
>> I guess we could apply the patches needed for the specific versions, but 
>> maybe you have better ideas?
>>
>> //Eelco
>>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 03/12] ci: Update the GitHub Ubuntu runner image to Ubuntu 22.04.

[Ilya Maximets]

On 1/8/24 17:01, Eelco Chaudron wrote:
> 
> 
> On 2 Jan 2024, at 12:19, Ilya Maximets wrote:
> 
>> On 12/19/23 13:41, Eelco Chaudron wrote:
>>> Updating this image is a requirement for the kernel system-traffic
>>> tests to pass on Ubuntu. In addition, 20.04 might be replaced,
>>> as soon as 24.04 comes out. Or we need to do this when it becomes
>>> EOL in April 2025.
>>>
>>> Signed-off-by: Eelco Chaudron 
>>> Acked-by: Simon Horman 
>>> ---
>>>  .github/workflows/build-and-test.yml |4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/.github/workflows/build-and-test.yml 
>>> b/.github/workflows/build-and-test.yml
>>> index 5d441157c..acb57ac46 100644
>>> --- a/.github/workflows/build-and-test.yml
>>> +++ b/.github/workflows/build-and-test.yml
>>> @@ -12,7 +12,7 @@ jobs:
>>>  name: dpdk gcc
>>>  outputs:
>>>dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
>>> -runs-on: ubuntu-20.04
>>> +runs-on: ubuntu-22.04
>>>  timeout-minutes: 30
>>>
>>>  steps:
>>> @@ -89,7 +89,7 @@ jobs:
>>>TESTSUITE:   ${{ matrix.testsuite }}
>>>
>>>  name: linux ${{ join(matrix.*, ' ') }}
>>> -runs-on: ubuntu-20.04
>>> +runs-on: ubuntu-22.04
>>>  timeout-minutes: 30
>>>
>>>  strategy:
>>>
>>
>> Hi, Eelco.  Could you also apply this to branches down to 2.17?
>> We'll need to keep the image updated there as well since they are
>> going to be supported likely beyond 20.04 availability in GHA.
>>
>> Assuming these branches should work fine with 22.04 (I didn't check).
> 
> I was trying 2.17 and up, and it works from 3.1 as we ditched the kernel 
> compilation.
> On 2.17 and 3.0:

Hmm.  We're not building the kernel module on 3.0. Are you sure?

> 
> HOSTLD  scripts/dtc/dtc
>   /usr/bin/ld: scripts/dtc/dtc-parser.tab.o:(.bss+0x20): multiple definition 
> of `yylloc'; scripts/dtc/dtc-lexer.lex.o:(.bss+0x0): first defined here
>   collect2: error: ld returned 1 exit status
>   make[1]: *** [scripts/Makefile.host:99: scripts/dtc/dtc] Error 1
>   make: *** [Makefile:1281: scripts_dtc] Error 2
>   Error: Process completed with exit code 2.
> 
> HOSTLD  arch/x86/tools/relocs
>   /usr/bin/ld: arch/x86/tools/relocs_64.o:(.bss+0x0): multiple definition of 
> `per_cpu_load_addr'; arch/x86/tools/relocs_32.o:(.bss+0x0): first defined here
>   collect2: error: ld returned 1 exit status
>   make[1]: *** [scripts/Makefile.host:127: arch/x86/tools/relocs] Error 1
> 
> A quick search online resulted, in using an older compiler, or applying a 
> kernel patch :)
> 
> I guess we could apply the patches needed for the specific versions, but 
> maybe you have better ideas?
> 
> //Eelco
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 03/12] ci: Update the GitHub Ubuntu runner image to Ubuntu 22.04.

[Eelco Chaudron]

On 2 Jan 2024, at 12:19, Ilya Maximets wrote:

> On 12/19/23 13:41, Eelco Chaudron wrote:
>> Updating this image is a requirement for the kernel system-traffic
>> tests to pass on Ubuntu. In addition, 20.04 might be replaced,
>> as soon as 24.04 comes out. Or we need to do this when it becomes
>> EOL in April 2025.
>>
>> Signed-off-by: Eelco Chaudron 
>> Acked-by: Simon Horman 
>> ---
>>  .github/workflows/build-and-test.yml |4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/.github/workflows/build-and-test.yml 
>> b/.github/workflows/build-and-test.yml
>> index 5d441157c..acb57ac46 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -12,7 +12,7 @@ jobs:
>>  name: dpdk gcc
>>  outputs:
>>dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
>> -runs-on: ubuntu-20.04
>> +runs-on: ubuntu-22.04
>>  timeout-minutes: 30
>>
>>  steps:
>> @@ -89,7 +89,7 @@ jobs:
>>TESTSUITE:   ${{ matrix.testsuite }}
>>
>>  name: linux ${{ join(matrix.*, ' ') }}
>> -runs-on: ubuntu-20.04
>> +runs-on: ubuntu-22.04
>>  timeout-minutes: 30
>>
>>  strategy:
>>
>
> Hi, Eelco.  Could you also apply this to branches down to 2.17?
> We'll need to keep the image updated there as well since they are
> going to be supported likely beyond 20.04 availability in GHA.
>
> Assuming these branches should work fine with 22.04 (I didn't check).

I was trying 2.17 and up, and it works from 3.1 as we ditched the kernel 
compilation.
On 2.17 and 3.0:

HOSTLD  scripts/dtc/dtc
  /usr/bin/ld: scripts/dtc/dtc-parser.tab.o:(.bss+0x20): multiple definition of 
`yylloc'; scripts/dtc/dtc-lexer.lex.o:(.bss+0x0): first defined here
  collect2: error: ld returned 1 exit status
  make[1]: *** [scripts/Makefile.host:99: scripts/dtc/dtc] Error 1
  make: *** [Makefile:1281: scripts_dtc] Error 2
  Error: Process completed with exit code 2.

HOSTLD  arch/x86/tools/relocs
  /usr/bin/ld: arch/x86/tools/relocs_64.o:(.bss+0x0): multiple definition of 
`per_cpu_load_addr'; arch/x86/tools/relocs_32.o:(.bss+0x0): first defined here
  collect2: error: ld returned 1 exit status
  make[1]: *** [scripts/Makefile.host:127: arch/x86/tools/relocs] Error 1

A quick search online resulted, in using an older compiler, or applying a 
kernel patch :)

I guess we could apply the patches needed for the specific versions, but maybe 
you have better ideas?

//Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] ovs: Bump submodule to include IDL "spurious delete" fix.

[Dumitru Ceara]

Specifically the following commit:
  4102674b3e ovsdb-idl: Preserve change_seqno when deleting rows.

Without it, in specific cases, the IDL might incorrectly report deletion
of yet to be seen records.

Signed-off-by: Dumitru Ceara 
---
NOTE: when backporting this, please make sure the corresponding OVS
branch-3.X versions of the submodule versions are used.  The IDl fix is
backported to all required versions.
---
 controller/ofctrl.c | 2 +-
 ovs | 2 +-
 tests/test-ovn.c| 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 7aac0128bc..cb460a2a47 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -3045,7 +3045,7 @@ ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, 
const char *flow_s,
 uint64_t packet_stub[128 / 8];
 struct dp_packet packet;
 dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
-flow_compose(&packet, &uflow, NULL, 64);
+flow_compose(&packet, &uflow, NULL, 64, false);
 
 uint64_t ofpacts_stub[1024 / 8];
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
diff --git a/ovs b/ovs
index fdbf0bb2ae..4102674b3e 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit fdbf0bb2aed53e70b455eb1adcfda8d8278ea690
+Subproject commit 4102674b3ecadb0e20e512cc661cddbbc4b3d1f6
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index aaf2825edc..5326c6e692 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -999,7 +999,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct 
shash *symtab,
 
 if (operation >= OP_FLOW) {
 bool found = classifier_lookup(&cls, OVS_VERSION_MIN,
-   &f, NULL) != NULL;
+   &f, NULL, NULL) != NULL;
 if (expected != found) {
 struct ds expr_s, modified_s;
 
@@ -1238,7 +1238,7 @@ test_expr_to_packets(struct ovs_cmdl_context *ctx 
OVS_UNUSED)
 uint64_t packet_stub[128 / 8];
 struct dp_packet packet;
 dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
-flow_compose(&packet, &uflow, NULL, 64);
+flow_compose(&packet, &uflow, NULL, 64, false);
 
 struct ds output = DS_EMPTY_INITIALIZER;
 const uint8_t *buf = dp_packet_data(&packet);
-- 
2.39.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Aaron Conole]

(re-added the list)

Roberto Bartzen Acosta  writes:

> Hi Aaron,
>
> I tried to send a new revision but I think that my Gmail web interface
> is putting some additional formatting even if I paste using
> .
> Do you recommend any tool or interface for sending correctly formatted 
> patches?
> This is my first so I have no experience sending patches via email.

Yes - I suggest setting up git send-email so you can run::

  $ git send-email --to="d...@openvswitch.org" --cc=..CC.. 0001-SomePatch.patch

An example for gmail might be:

  $ git config --global sendemail.smtpUser 
  $ git config --global sendemail.smtpPass 
  $ git config --global sendemail.smtpServer smtp.gmail.com
  $ git config --global sendemail.smtpServerPort 587
  $ git config --global sendemail.smtpEncryption tls

Then you could use the git send-email command to send your patch file.

See https://git-scm.com/docs/git-send-email for more details

> Thank you and have a nice day
>
> Em seg., 8 de jan. de 2024 às 08:20, Roberto Bartzen Acosta
>  escreveu:
>>
>> Em qua., 3 de jan. de 2024 às 13:13, Aaron Conole  
>> escreveu:
>> >
>> > Roberto Bartzen Acosta via dev  writes:
>> >
>> > > 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 OS like Ubuntu Jammy,
>> > > for example, can enable the -flto=auto option during the openvswitch
>> > > building and linking process. In this case, the linked dynamic libraries
>> > > would need to be builded based on the same lto optimization options, at 
>> > > the
>> > > risk of not working, according to documentation [1].
>> > >
>> > > I'm not sure of the real benefits of using this link-time optimization
>> > > option, and since when it is enabled it causes problems with shared libs
>> > > link libjemalloc, for example, it seems safer overwritten compiler 
>> > > decision
>> > > by passing -fno-lto command.
>> > >
>> > > [1] https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto
>> > >
>> > > Reported-at:
>> > > https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748
>> > > Signed-off-by: Roberto Bartzen Acosta 
>> > > ---
>> > >  debian/rules | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/debian/rules b/debian/rules
>> > > index dc5cc8a65..de8771813 100755
>> > > --- a/debian/rules
>> > > +++ b/debian/rules
>> > > @@ -2,7 +2,7 @@
>> > >  # -*- makefile -*-
>> > >  #export DH_VERBOSE=1
>> > >  export DEB_BUILD_MAINT_OPTIONS = hardening=+all
>> > > -export DEB_CFLAGS_MAINT_APPEND = -fPIC
>> > > +export DEB_CFLAGS_MAINT_APPEND = -fPIC -fno-lto
>> > >
>> > >  %:
>> > > dh $@
>> >
>> > I think you have an issue with either your editor or your mail client.
>> > The last context line should be:
>> >
>> >   dh $@
>> >
>> > but you have sent
>> >
>> >   dh $@
>>
>> Thanks, I'll check it out.
>> >

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ilya Maximets]

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 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 does.

 Signed-off-by: Terry Wilson 
>>>
>>> Hi Terry,
>>>
>>> is it possible to add a test to tests/ovsdb-idl.at for this feature?
>>
>> It might be, but it seems like it'd be a bit of work and I'm not sure
>> if I'll have the time to look at it for a while. I'm just trying to
>> bring the functionality up to what the C IDL has and from what I can
>> tell this feature isn't tested in the C IDL either.
> 
> Hi, Terry and Simon.
> 
> I spent some time and came up with the following test for the issue:
>   
> https://patchwork.ozlabs.org/project/openvswitch/patch/20240108145248.1763075-1-i.maxim...@ovn.org/
> The test in the patch will fail without the fix provided here.
> 
> Also, this change is not really a new feature.  Python IDL today claims
> that it is database change aware and implements the monitoring of the
> _Server database, but it is not handling the monitor cancellation that
> is a vital part of being change aware.  So, IMO, it is a bug that should
> be fixed in stable branches as well, unless there are reasons not to.
> 
> The following tag should be added:
> 
> Fixes: c39751e44539 ("python: Monitor Database table to manage lifecycle of 
> IDL client.")
> 
> The test is quite large and requires changing the test utilities, so
> I'm not sure if it should be squashed with the fix or just treated as
> a separate patch.  OTOH, tests should normally go together with the
> fix.  I'm OK with either option, but the commit message of the test
> patch should be preserved as it contains important information about a
> different bug.  What do you think?

Actually, I guess, the following might be an option:

1. I can carve out python-related things from the test I posted, keeping
   it for C IDL only, but easily extendable for python.
2. Get the test reviewed/accepted and backported.
3. Add the python test bits to this fix, so the test is included.

The change for the test will look something like this:

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index b522208c8..003ba6571 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2809,14 +2809,21 @@ m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE],
AT_CAPTURE_FILE([idl-c.out])
AT_CAPTURE_FILE([idl-c.err])
 
+   OVS_DAEMONIZE([$PYTHON3 $srcdir/test-ovsdb.py -t30 \
+   idl $srcdir/idltest.ovsschema unix:socket COND monitor \
+   >idl-python.out 2>idl-python.err], [idl-python.pid])
+   AT_CAPTURE_FILE([idl-python.out])
+   AT_CAPTURE_FILE([idl-python.err])
+
dnl Wait for monitors to receive the data.
OVS_WAIT_UNTIL([grep -q 'third row' idl-c.err])
+   OVS_WAIT_UNTIL([grep -q 'third row' idl-python.err])
 
dnl Convert the database.
AT_CHECK([ovsdb-client convert unix:socket new-idltest.ovsschema])
 
dnl Check for the monitor cancellation and the data being requested again.
-   m4_foreach([FILE], [[idl-c]],
+   m4_foreach([FILE], [[idl-c], [idl-python]],
  [OVS_WAIT_UNTIL([grep -q 'monitor_canceled' FILE.err])
   OVS_WAIT_UNTIL([test 2 -eq $(grep -c 'send request, 
method="monitor_cond_since", params=."idltest"' FILE.err)])
 
---
The changes for my test patch would be a revert of that, of course
(keeping the test-ovsdb.py modification in the test patch to have
parity between C and Python test utilities).

What do you think?

> 
> Dumitru, could you, also, please, take a look at this version of the fix?
> I didn't spent much time on a fix itself, I only checked that it works
> fine with the test I made.
> 
> Best regards, Ilya Maximets.
> 
>> What I'm doing to
>> manually test is to run this:
>>
>> import logging
>> import ovs
>>
>> import ovsdbapp.schema.ovn_northbound.impl_idl as nb_idl
>> from ovsdbapp.backend.ovs_idl import connection, vlog
>>
>> print(f"Using OVS {ovs.__file__}")
>> logging.basicConfig(level=logging.DEBUG)
>> vlog.use_python_logger()
>> LOG = logging.getLogger(__name__)
>>
>> remote = "unix:///home/twilson/src/ovn/tutorial/sandbox/nb1.ovsdb"
>>
>> def get_idl():
>>"""Connection getter."""
>>
>>idl = connection.OvsdbIdl.from_server(remote, "OVN_Northbound",
>>  leader_only=False)
>>return nb_idl.OvnNbApiIdlImpl(connection.Connection(idl, 100))
>>
>> idl = get_idl()
>> LOG.info(f"monitor_id: {str(idl.idl.uuid)}")
>> LOG.info(f"server_monitor_id: {str(idl.idl.server_monitor_uuid)}")
>> input(" Press Enter ")
>> idl.ls_add("test").execute(check_error=True)
>>
>> and then in another window running:
>> ovsdb-client convert
>> unix:/hom

Re: [ovs-dev] [PATCH] lib/backtrace: Fix error in log_backtrace() documentation.

[Mike Pattrick]

On Fri, Jan 5, 2024 at 11:42 AM Frode Nordahl
 wrote:
>
> The documentation for log_backtrace() states the backtrace is
> logged at DEBUG level, while in reality it is logged at ERROR
> level.
>
> Fixes: d0b99d38edab ("backtrace: Add log_backtrace()")
> Signed-off-by: Frode Nordahl 

That looks correct to me.

Acked-by: Mike Pattrick 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ilya Maximets]

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 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 does.
>>>
>>> Signed-off-by: Terry Wilson 
>>
>> Hi Terry,
>>
>> is it possible to add a test to tests/ovsdb-idl.at for this feature?
> 
> It might be, but it seems like it'd be a bit of work and I'm not sure
> if I'll have the time to look at it for a while. I'm just trying to
> bring the functionality up to what the C IDL has and from what I can
> tell this feature isn't tested in the C IDL either.

Hi, Terry and Simon.

I spent some time and came up with the following test for the issue:
  
https://patchwork.ozlabs.org/project/openvswitch/patch/20240108145248.1763075-1-i.maxim...@ovn.org/
The test in the patch will fail without the fix provided here.

Also, this change is not really a new feature.  Python IDL today claims
that it is database change aware and implements the monitoring of the
_Server database, but it is not handling the monitor cancellation that
is a vital part of being change aware.  So, IMO, it is a bug that should
be fixed in stable branches as well, unless there are reasons not to.

The following tag should be added:

Fixes: c39751e44539 ("python: Monitor Database table to manage lifecycle of IDL 
client.")

The test is quite large and requires changing the test utilities, so
I'm not sure if it should be squashed with the fix or just treated as
a separate patch.  OTOH, tests should normally go together with the
fix.  I'm OK with either option, but the commit message of the test
patch should be preserved as it contains important information about a
different bug.  What do you think?

Dumitru, could you, also, please, take a look at this version of the fix?
I didn't spent much time on a fix itself, I only checked that it works
fine with the test I made.

Best regards, Ilya Maximets.

> What I'm doing to
> manually test is to run this:
> 
> import logging
> import ovs
> 
> import ovsdbapp.schema.ovn_northbound.impl_idl as nb_idl
> from ovsdbapp.backend.ovs_idl import connection, vlog
> 
> print(f"Using OVS {ovs.__file__}")
> logging.basicConfig(level=logging.DEBUG)
> vlog.use_python_logger()
> LOG = logging.getLogger(__name__)
> 
> remote = "unix:///home/twilson/src/ovn/tutorial/sandbox/nb1.ovsdb"
> 
> def get_idl():
>"""Connection getter."""
> 
>idl = connection.OvsdbIdl.from_server(remote, "OVN_Northbound",
>  leader_only=False)
>return nb_idl.OvnNbApiIdlImpl(connection.Connection(idl, 100))
> 
> idl = get_idl()
> LOG.info(f"monitor_id: {str(idl.idl.uuid)}")
> LOG.info(f"server_monitor_id: {str(idl.idl.server_monitor_uuid)}")
> input(" Press Enter ")
> idl.ls_add("test").execute(check_error=True)
> 
> and then in another window running:
> ovsdb-client convert
> unix:/home/twilson/src/ovn/tutorial/sandbox/nb1.ovsdb
> /home/twilson/src/ovn/ovn-nb.ovsschema
> 
> and then pressing enter in the original window. Without the patch,
> after running the ovsdb-client convert, you get:
> DEBUG:ovsdbapp.backend.ovs_idl.vlog:unix:///home/twilson/src/ovn/tutorial/sandbox/nb1.ovsdb:
> received unexpected notification message
> and then ovsdbapp starts throwing exceptions. With the patch, things
> work as one would expect.
> 
> The problem with testing is that the client connection needs to be
> active during the monitor_canceled that happens when ovsdb-client
> convert is called. The tests in ovsdb-idl.at use ovsdb-client transact
> for doing everything, so it's not easy to do the convert between
> connection and transaction execution.
> 
> Maybe something could be added test test-ovsdb.py stuff.
> 
> Terry

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ilya Maximets]

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 test cases for IDL with online database
conversion.  Test checks that IDL receives monitor cancellation
notification and successfully re-sends monitor requests.

While at it, adding debug logging to ovsdb-server processes for easier
debugging.

While working on a test the issue was discovered that schema for
standalone databases is not getting updated in the _Server database
after conversion.  Checking the new schema bits only for clustered
databases for now.

Signed-off-by: Ilya Maximets 
---

WARN: The test is going to fail for python IDL implementation without:
  
https://patchwork.ozlabs.org/project/openvswitch/patch/20231218233124.669221-1-twil...@redhat.com/

 tests/ovsdb-idl.at  | 100 +---
 tests/test-ovsdb.c  |   7 
 tests/test-ovsdb.py |  27 +++-
 3 files changed, 119 insertions(+), 15 deletions(-)

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index f17cfdf10..003ba6571 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -29,8 +29,8 @@ m4_define([OVSDB_START_IDLTEST],
   AT_CHECK([ovsdb-tool create db dnl
   m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])])
   PKIDIR=$abs_top_builddir/tests
-  AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl
-  --pidfile --remote=punix:socket dnl
+  AT_CHECK([ovsdb-server -vconsole:warn -vfile:dbg --log-file dnl
+  --detach --no-chdir --pidfile --remote=punix:socket dnl
   m4_if(m4_substr($1, 0, 5), [pssl:],
 [--private-key=$PKIDIR/testpki-privkey2.pem dnl
  --certificate=$PKIDIR/testpki-cert2.pem dnl
@@ -57,9 +57,9 @@ m4_define([OVSDB_CLUSTER_START_IDLTEST],
done
on_exit 'kill $(cat s*.pid)'
for i in $(seq $n); do
- AT_CHECK([ovsdb-server -vraft -vconsole:warn --detach --no-chdir \
-   --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i \
-   --remote=punix:s$i.ovsdb   \
+ AT_CHECK([ovsdb-server -vraft -vconsole:warn -vfile:dbg --detach \
+   --no-chdir --log-file=s$i.log --pidfile=s$i.pid\
+   --unixctl=s$i --remote=punix:s$i.ovsdb \
m4_if([$2], [], [], [--remote=$2]) s$i.db])
done
 
@@ -2756,3 +2756,93 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent 
uuid insert],
 011: done
 ]],
   [['This UUID would duplicate a UUID already present within the table or 
deleted within the same transaction']])
+
+
+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])
+
+   m4_if([$1], [clustered],
+ [OVSDB_CLUSTER_START_IDLTEST([1], [punix:socket])],
+ [OVSDB_START_IDLTEST])
+
+   dnl Add some data.
+   AT_CHECK([[ovsdb-client transact unix:socket '["idltest",
+  {"op": "insert",
+   "table": "simple",
+   "row": {"i": 1,
+   "r": 2.0,
+   "b": true,
+   "s": "first row",
+   "u": ["uuid", "84f5c8f5-ac76-4dbc-a24f-8860eb407fc1"],
+   "ia": ["set", [1, 2, 3]],
+   "ra": ["set", [-0.5]],
+   "ba": ["set", [true]],
+   "sa": ["set", ["abc", "def"]],
+   "ua": ["set", [["uuid", "69443985-7806-45e2-b35f-574a04e720f9"],
+  ["uuid", 
"aad11ef0-816a-4b01-93e6-03b8b4256b98"]]]}},
+  {"op": "insert",
+   "table": "simple",
+   "row": {"b": false, "s": "second row"}},
+  {"op": "insert",
+   "table": "simple",
+   "row": {"b": true, "s": "third row"}}
+   ]']], [0], [stdout])
+
+   dnl Create a new schema by adding 'extra_column' to the 'simple' table.
+   AT_CHECK([sed 's/"ua": {/"extra_column":{"type": "string"},"ua": {/
+  s/1.2.3/1.2.4/' \
+ $abs_srcdir/idltest.ovsschema > new-idltest.ovsschema])
+   dnl Try "needs-conversion".
+   AT_CHECK([ovsdb-client needs-conversion unix:socket 
$abs_srcdir/idltest.ovsschema], [0], [no
+])
+   AT_CHECK([ovsdb-client needs-conversion unix:socket new-idltest.ovsschema], 
[0], [yes
+])
+
+   dnl Conditionally exclude the second row from monitoring.
+   m4_define([COND], [['condition simple [["b","==",true]]']])
+
+   dnl Start monitoring.
+   OVS_DAEMONIZE([test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc 
-t30 \
+   idl unix:socket COND monitor \
+   >idl-c.out 2>idl-c.err], [idl-c.pid])
+   AT_CAPTURE_FILE([idl-c.out])
+   AT_CAPTURE_FILE([idl-c.err])
+
+   OVS_DAEMONIZE([$PYTHON3 $srcdir/test-ovsdb.py -t30 \
+   idl $srcdir/idltest.ovsschema unix:socket C

Re: [ovs-dev] [PATCH ovn] northd: Added lb_vip_mac config option in Logical_Switch.

[0-day Robot]

References:  <20240108131243.53816-1-priyankar.j...@nutanix.com>
 

Bleep bloop.  Greetings Priyankar Jain, 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 93 characters long (recommended limit is 79)
#157 FILE: northd/ovn-northd.8.xml:1624:
  ,

Lines checked: 326, 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

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

[Ilya Maximets]

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 previously written by save_config(). 
>> */
>> -static void
>> +/* Clears and replaces 'remotes' and 'db_conf' by a configuration read from
>> + * 'config_file', which must have been previously written by save_config()
>> + * or provided by the user with --config-file.
>> + * Returns 'true', if parsing was successful, 'false' otherwise. */
>> +static bool
>>  load_config(FILE *config_file, struct shash *remotes,
>>  struct shash *db_conf, char **sync_from,
>>  char **sync_exclude, bool *is_backup)
>> @@ -2890,17 +3117,34 @@ load_config(FILE *config_file, struct shash *remotes,
>>  struct json *json;
> 
> I'm wondering if having an argument for everything we parse out of a
> config file might get a little unwieldy down the road. Say we add
> configuration of SSL stuff, etc. Maybe it's something we could modify
> as it becomes an issue, but it might be nice to have something for
> config options that is similar to what we have for registering unixctl
> commands or struct ctl_command_syntax. Documentation could be added as
> part of the registration/definition of the config option, there could
> be a .get() that parses the values out of the json, and a .run() that
> gets called after all of the parsing is shown to succeed.
> 
> Terry
> 

Hi, Terry.  Yes, I agree that the current interface is far from being
ideal, and I actually tried to rework it multiple times while working
on this patch set.  That's one of the reasons it took so long. :)
Unfortunately it ended up with a huge amount of refactoring every time.
The main reason for that, I think, is the fact that the same function
is used for loading the legacy internal configuration file and the new
user-provided file.  And the code around legacy internal configuration
is not easy to adapt.

I think, it would be much easier to do this kind of refactoring once
we deprecate and remove all the appctl commands that can change the
server configuration.  For now I decided to keep the interface as it is
without this patch set to avoid making the patch set even bigger.
I hope that makes sense.

The idea of registering the options with a common parsing logic sounds
interesting though and definitely worth exploring in the future.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] northd: Added svc_monitor_ipv4 in NB_Global options.

[0-day Robot]

References:  <20240108130004.51995-1-priyankar.j...@nutanix.com>
 

Bleep bloop.  Greetings Priyankar Jain, 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 81 characters long (recommended limit is 79)
#84 FILE: northd/northd.c:8244:
"eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4",

WARNING: Line is 84 characters long (recommended limit is 79)
#93 FILE: northd/northd.c:10331:
  "(eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4)"

Lines checked: 624, 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

[ovs-dev] [PATCH ovn] northd: Added lb_vip_mac config option in Logical_Switch.

[Priyankar Jain]

Currently load balancer applied to a logical switch has the
following restriction:
- VIP of the load balancer cannot reside in the subnet prefix as the
  clients as OVN does not install ARP responder flows for the LB VIP.

This change adds a new config option "lb_vip_mac" in the logical_switch
table which is expected to be a MAC address. If the logical_switch has
this option configured, northd will program an ARP responder flow for
all the LB VIPs of the logical_switch with this MAC address.

Usecase: With this change, CMS can set the lb_vip_mac value to same as
the default gateway MAC. This allows CMS to allocate VIP of the Load
balancer from any subnet prefix.

Signed-off-by: Priyankar Jain 
---
 northd/northd.c |  71 ++
 northd/northd.h |   2 +
 northd/ovn-northd.8.xml |  49 ++
 tests/ovn.at| 109 
 4 files changed, 231 insertions(+)

diff --git a/northd/northd.c b/northd/northd.c
index db3cd272e..ebca2c073 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -790,8 +790,11 @@ init_lb_for_datapath(struct ovn_datapath *od)
 {
 if (od->nbs) {
 od->has_lb_vip = ls_has_lb_vip(od);
+od->lb_vip_mac = nullable_xstrdup(
+smap_get(&od->nbs->other_config, "lb_vip_mac"));
 } else {
 od->has_lb_vip = lr_has_lb_vip(od);
+od->lb_vip_mac = NULL;
 }
 }
 
@@ -800,6 +803,9 @@ destroy_lb_for_datapath(struct ovn_datapath *od)
 {
 ovn_lb_ip_set_destroy(od->lb_ips);
 od->lb_ips = NULL;
+
+free(od->lb_vip_mac);
+od->lb_vip_mac = NULL;
 }
 
 /* A group of logical router datapaths which are connected - either
@@ -12204,6 +12210,70 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
 }
 }
 
+static void
+build_lb_rules_arp_nd_rsp(struct hmap *lflows, struct ovn_lb_datapaths *lb_dps,
+  const struct ovn_datapaths *ls_datapaths,
+  struct ds *match, struct ds *actions)
+{
+if (!lb_dps->n_nb_ls) {
+return;
+}
+
+const struct ovn_northd_lb *lb = lb_dps->lb;
+for (size_t i = 0; i < lb->n_vips; i++) {
+struct ovn_lb_vip *lb_vip = &lb->vips[i];
+
+size_t index;
+BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths), lb_dps->nb_ls_map) {
+struct ovn_datapath *od = ls_datapaths->array[index];
+if (!od->lb_vip_mac) {
+  continue;
+}
+ds_clear(match);
+ds_clear(actions);
+if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) {
+ds_put_format(match, "arp.tpa == %s && arp.op == 1",
+  lb_vip->vip_str);
+ds_put_format(actions,
+"eth.dst = eth.src; "
+"eth.src = %s; "
+"arp.op = 2; /* ARP reply */ "
+"arp.tha = arp.sha; "
+"arp.sha = %s; "
+"arp.tpa = arp.spa; "
+"arp.spa = %s; "
+"outport = inport; "
+"flags.loopback = 1; "
+"output;",
+od->lb_vip_mac, od->lb_vip_mac,
+lb_vip->vip_str);
+} else {
+ds_put_format(match, "nd_ns && nd.target == %s",
+  lb_vip->vip_str);
+ds_put_format(actions,
+"nd_na { "
+"eth.dst = eth.src; "
+"eth.src = %s; "
+"ip6.src = %s; "
+"nd.target = %s; "
+"nd.tll = %s; "
+"outport = inport; "
+"flags.loopback = 1; "
+"output; "
+"};",
+od->lb_vip_mac,
+lb_vip->vip_str,
+lb_vip->vip_str,
+od->lb_vip_mac);
+}
+ovn_lflow_add_with_hint(lflows, od,
+S_SWITCH_IN_ARP_ND_RSP, 130,
+ds_cstr(match), ds_cstr(actions),
+&lb->nlb->header_);
+}
+}
+}
+
 static void
 build_lswitch_flows_for_lb(struct ovn_lb_datapaths *lb_dps,
struct hmap *lflows,
@@ -12255,6 +12325,7 @@ build_lswitch_flows_for_lb(struct ovn_lb_datapaths 
*lb_dps,
 ls_datapaths, match, action);
 build_lb_rules(lflows, lb_dps, ls_datapaths, features, match, action,
meter_groups, svc_monitor_map);
+build_lb_rules_arp_nd_rsp(lflows, lb_dps, ls_datapaths, match, action);
 }
 
 /* If there are any load balancing rules, we should send the packet to
diff --git a/northd/northd.h b/northd/northd.h
index 5be7b5384..3e1b24e2c 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -262,6 +262,8 @@ struct ovn_d

[ovs-dev] [PATCH ovn v2] northd: Added svc_monitor_ipv4 in NB_Global options.

[Priyankar Jain]

This commit adds a new optional svc_monitor_ipv4 config in the NB_Global
options column. This IP address can be used to send the packets to the
OVN controller bypassing most of the logical switches pipelines.

Usage: Currently Load balancer health check requires a source IP address
to be allocated from the subnets where backends are present.
This change removes that requirement and instead user can give one IP
address for the system and set it in NB_Global:options:svc_monitor_ipv4.
While configuring the health check for the load balancer user can
provide this same IP while specifying the ip-port mappings.

Signed-off-by: Priyankar Jain 
---

Changes since v1:
 - Fixed one testcase in ovn-northd.at

---
 northd/en-sync-sb.c | 10 +
 northd/northd.c | 34 +--
 northd/northd.h |  1 +
 northd/ovn-northd.8.xml | 40 +++--
 tests/ovn-northd.at | 96 -
 5 files changed, 116 insertions(+), 65 deletions(-)

diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index 2ec3bf54f..95e9ead29 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -372,6 +372,16 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
 sync_addr_set(ovnsb_txn, "svc_monitor_mac", &svc, &sb_address_sets);
 sorted_array_destroy(&svc);
 
+/* Service monitor IP. */
+const char *svc_monitor_ip4 = northd_get_svc_monitor_ip4();
+int num_addr = 0; /* Create empty address-set by default */
+if (svc_monitor_ip4) {
+num_addr = 1;
+}
+struct sorted_array ip_svc = sorted_array_create(&svc_monitor_ip4,
+ num_addr, false);
+sync_addr_set(ovnsb_txn, "svc_monitor_ip4", &ip_svc, &sb_address_sets);
+
 /* sync port group generated address sets first */
 const struct nbrec_port_group *nb_port_group;
 NBREC_PORT_GROUP_TABLE_FOR_EACH (nb_port_group,
diff --git a/northd/northd.c b/northd/northd.c
index db3cd272e..f6d35143a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -82,6 +82,7 @@ static bool use_common_zone = false;
 static char svc_monitor_mac[ETH_ADDR_STRLEN + 1];
 static struct eth_addr svc_monitor_mac_ea;
 
+static char *svc_monitor_ip4 = NULL;
 /* If this option is 'true' northd will make use of ct.inv match fields.
  * Otherwise, it will avoid using it.  The default is true. */
 static bool use_ct_inv_match = true;
@@ -7197,7 +7198,8 @@ build_pre_acls(struct ovn_datapath *od,
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;");
 
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
-  "eth.dst == $svc_monitor_mac", "next;");
+  "eth.dst == $svc_monitor_mac || ip4.dst == $svc_monitor_ip4",
+  "next;");
 
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
   "eth.src == $svc_monitor_mac", "next;");
@@ -7370,7 +7372,8 @@ build_pre_lb(struct ovn_datapath *od, const struct shash 
*meter_groups,
 
 /* Do not send service monitor packets to conntrack. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
-  "eth.dst == $svc_monitor_mac", "next;");
+  "eth.dst == $svc_monitor_mac || ip4.dst == $svc_monitor_ip4",
+  "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
   "eth.src == $svc_monitor_mac", "next;");
 
@@ -8238,7 +8241,7 @@ build_acls(struct ovn_datapath *od, const struct 
chassis_features *features,
 /* Add a 34000 priority flow to advance the service monitor reply
 * packets to skip applying ingress ACLs. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, 34000,
-"eth.dst == $svc_monitor_mac",
+"eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4",
 REGBIT_ACL_VERDICT_ALLOW" = 1; next;");
 
 /* Add a 34000 priority flow to advance the service monitor packets
@@ -10325,7 +10328,8 @@ build_lswitch_destination_lookup_bmcast(struct 
ovn_datapath *od,
 ovs_assert(od->nbs);
 
 ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
-  "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
+  "(eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4)"
+  " && (tcp || icmp || icmp6)",
   "handle_svc_check(inport);",
   copp_meter_get(COPP_SVC_MONITOR, od->nbs->copp,
  meter_groups));
@@ -17891,6 +17895,22 @@ ovnnb_db_run(struct northd_input *input_data,
 smap_replace(&options, "svc_monitor_mac", svc_monitor_mac);
 }
 
+const char *monitor_ip4 = smap_get(&nb->options, "svc_monitor_ip4");
+if (monitor_ip4) {
+struct sockaddr_storage svc_mon_src_addr;
+if (inet_parse_address(monitor_ip4, &svc_mon_src_addr)) {
+struct ds src_ip_s = DS_EMPTY_INITIALIZER;
+ss_format_address_nobrac

Re: [ovs-dev] [PATCH ovn] northd: Added svc_monitor_ipv4 in NB_Global options.

[0-day Robot]

References:  <20240108122446.48009-1-priyankar.j...@nutanix.com>
 

Bleep bloop.  Greetings Priyankar Jain, 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 81 characters long (recommended limit is 79)
#84 FILE: northd/northd.c:8244:
"eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4",

WARNING: Line is 84 characters long (recommended limit is 79)
#93 FILE: northd/northd.c:10331:
  "(eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4)"

Lines checked: 626, 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

[ovs-dev] [PATCH ovn] northd: Added svc_monitor_ipv4 in NB_Global options.

[Priyankar Jain]

This commit adds a new optional svc_monitor_ipv4 config in the NB_Global
options column. This IP address can be used to send the packets to the
OVN controller bypassing most of the logical switches pipelines.

Usage: Currently Load balancer health check requires a source IP address
to be allocated from the subnets where backends are present.
This change removes that requirement and instead user can give one IP
address for the system and set it in NB_Global:options:svc_monitor_ipv4.
While configuring the health check for the load balancer user can
provide this same IP while specifying the ip-port mappings.

Signed-off-by: Priyankar Jain 
---
 northd/en-sync-sb.c | 10 +
 northd/northd.c | 34 --
 northd/northd.h |  1 +
 northd/ovn-northd.8.xml | 40 +++--
 tests/ovn-northd.at | 98 +
 5 files changed, 118 insertions(+), 65 deletions(-)

diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index 2ec3bf54f..95e9ead29 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -372,6 +372,16 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn,
 sync_addr_set(ovnsb_txn, "svc_monitor_mac", &svc, &sb_address_sets);
 sorted_array_destroy(&svc);
 
+/* Service monitor IP. */
+const char *svc_monitor_ip4 = northd_get_svc_monitor_ip4();
+int num_addr = 0; /* Create empty address-set by default */
+if (svc_monitor_ip4) {
+num_addr = 1;
+}
+struct sorted_array ip_svc = sorted_array_create(&svc_monitor_ip4,
+ num_addr, false);
+sync_addr_set(ovnsb_txn, "svc_monitor_ip4", &ip_svc, &sb_address_sets);
+
 /* sync port group generated address sets first */
 const struct nbrec_port_group *nb_port_group;
 NBREC_PORT_GROUP_TABLE_FOR_EACH (nb_port_group,
diff --git a/northd/northd.c b/northd/northd.c
index db3cd272e..f6d35143a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -82,6 +82,7 @@ static bool use_common_zone = false;
 static char svc_monitor_mac[ETH_ADDR_STRLEN + 1];
 static struct eth_addr svc_monitor_mac_ea;
 
+static char *svc_monitor_ip4 = NULL;
 /* If this option is 'true' northd will make use of ct.inv match fields.
  * Otherwise, it will avoid using it.  The default is true. */
 static bool use_ct_inv_match = true;
@@ -7197,7 +7198,8 @@ build_pre_acls(struct ovn_datapath *od,
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;");
 
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
-  "eth.dst == $svc_monitor_mac", "next;");
+  "eth.dst == $svc_monitor_mac || ip4.dst == $svc_monitor_ip4",
+  "next;");
 
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
   "eth.src == $svc_monitor_mac", "next;");
@@ -7370,7 +7372,8 @@ build_pre_lb(struct ovn_datapath *od, const struct shash 
*meter_groups,
 
 /* Do not send service monitor packets to conntrack. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
-  "eth.dst == $svc_monitor_mac", "next;");
+  "eth.dst == $svc_monitor_mac || ip4.dst == $svc_monitor_ip4",
+  "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
   "eth.src == $svc_monitor_mac", "next;");
 
@@ -8238,7 +8241,7 @@ build_acls(struct ovn_datapath *od, const struct 
chassis_features *features,
 /* Add a 34000 priority flow to advance the service monitor reply
 * packets to skip applying ingress ACLs. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, 34000,
-"eth.dst == $svc_monitor_mac",
+"eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4",
 REGBIT_ACL_VERDICT_ALLOW" = 1; next;");
 
 /* Add a 34000 priority flow to advance the service monitor packets
@@ -10325,7 +10328,8 @@ build_lswitch_destination_lookup_bmcast(struct 
ovn_datapath *od,
 ovs_assert(od->nbs);
 
 ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
-  "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
+  "(eth.dst == $svc_monitor_mac || ip4.dst == 
$svc_monitor_ip4)"
+  " && (tcp || icmp || icmp6)",
   "handle_svc_check(inport);",
   copp_meter_get(COPP_SVC_MONITOR, od->nbs->copp,
  meter_groups));
@@ -17891,6 +17895,22 @@ ovnnb_db_run(struct northd_input *input_data,
 smap_replace(&options, "svc_monitor_mac", svc_monitor_mac);
 }
 
+const char *monitor_ip4 = smap_get(&nb->options, "svc_monitor_ip4");
+if (monitor_ip4) {
+struct sockaddr_storage svc_mon_src_addr;
+if (inet_parse_address(monitor_ip4, &svc_mon_src_addr)) {
+struct ds src_ip_s = DS_EMPTY_INITIALIZER;
+ss_format_address_nobracks(&svc_mon_src_addr, &src_ip_s);
+svc_monitor_ip4 =

Re: [ovs-dev] Next OVN technical community meeting

[Dumitru Ceara]

On 12/6/23 16:08, Dumitru Ceara wrote:
> On 11/29/23 17:49, Mark Michelson wrote:
>> On 11/29/23 08:32, Frode Nordahl wrote:
>>> On Wed, Nov 29, 2023 at 2:21 PM Dumitru Ceara  wrote:

 On 11/28/23 15:58, Dumitru Ceara wrote:
> I went ahead and scheduled another instance (please let me know if the
> date/time doesn't work for you): Date/Time: Monday January 15th 15:00
> UTC Video Link: meet.google.com/zns-gqsd-jdn Meeting notes:
> https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8

 I made the (likely incorrect) assumption that most people are off in the
 first two weeks of January, that's why I selected January 15th.

 If people prefer having the meeting in the first two weeks of the year,
 that's fine by me, I can update the invite.  Please let me know if you
 wish that.
>>>
>>> I'm the odd one out here being off from the weekend before 15th, if
>>> anyone is available in the preceding weeks I'd love it if we could
>>> make it happen, and I'd understand if everyone else is off/busy.
>>>
>>
>> My plan is to be around for the entire month of January. Any date will
>> work for me.
>>
> 
> OK, thanks for the replies!  Let's have the meeting one week earlier
> then, on Monday January 8th.
> 
> Date/Time: Monday January 8th 15:00 UTC
> Meeting link: meet.google.com/zns-gqsd-jdn
> Meeting notes:
> https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8

I just realized the time above is wrong, I think it got skewed when
Europe moved to winter time.. sorry about that.

In my calendar the meeting is scheduled to happen today, Monday 8th, at
16:00 PM UTC (approximately 3.5 hours from now).  Optionally, feel free
to reach out if you want me to add you to the explicit list of invitees
in the google meet.

Hope to see you there and I apologize again if I caused any confusion.

Regards,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[0-day Robot]

Bleep bloop.  Greetings Roberto Bartzen Acosta, I am a robot and I have tried 
out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: patch failed: debian/rules:2
error: debian/rules: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 debian/rules: Fix incorrect use of link-time optimizer.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

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 ovs v3] debian/rules: Fix incorrect use of link-time optimizer.

[Roberto Bartzen Acosta via dev]

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 OS like Ubuntu
Jammy, for example, can enable the -flto=auto option during the
openvswitch building and linking process. In this case, the linked
dynamic libraries would need to be builded based on the same lto
optimization options, at the risk of not working, according to
documentation [1].

I'm not sure of the real benefits of using this link-time optimization
option, and since when it is enabled it causes problems with shared
libs link libjemalloc, for example, it seems safer overwritten
compiler decision by passing -fno-lto command.

[1] https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto

Reported-at: https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748
Signed-off-by: Roberto Bartzen Acosta 
---
 debian/rules | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/rules b/debian/rules
index dc5cc8a65..de8771813 100755
--- a/debian/rules
+++ b/debian/rules
@@ -2,7 +2,7 @@
 # -*- makefile -*-
 #export DH_VERBOSE=1
 export DEB_BUILD_MAINT_OPTIONS = hardening=+all
-export DEB_CFLAGS_MAINT_APPEND = -fPIC
+export DEB_CFLAGS_MAINT_APPEND = -fPIC -fno-lto

 %:
  dh $@

-- 




_‘Esta mensagem é direcionada apenas para os endereços constantes no 
cabeçalho inicial. Se você não está listado nos endereços constantes no 
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa 
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão 
imediatamente anuladas e proibidas’._


* **‘Apesar do Magazine Luiza tomar 
todas as precauções razoáveis para assegurar que nenhum vírus esteja 
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por 
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.*



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 ovn] test: add dedicated test for garp-max-timeout

[Lorenzo Bianconi]

Introduce a dedicated test for garp-max-timeout knob

Signed-off-by: Lorenzo Bianconi 
---
 tests/ovn.at | 65 
 1 file changed, 65 insertions(+)

diff --git a/tests/ovn.at b/tests/ovn.at
index 5615ba1a9..6093e0ed9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -37524,3 +37524,68 @@ wait_for_ports_up
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([gratuitous arp max timeout])
+AT_KEYWORDS([slowtest])
+TAG_UNSTABLE
+AT_SKIP_IF([test $HAVE_TCPDUMP = no])
+AT_SKIP_IF([test $HAVE_SCAPY = no])
+ovn_start
+
+check ovn-nbctl ls-add ls0
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-ls0 f0:00:00:00:00:01 192.168.0.1/24
+check ovn-nbctl lsp-add ls0 ls0-lr0 -- set Logical_Switch_Port ls0-lr0 \
+type=router options:router-port=lr0-ls0 addresses='"f0:00:00:00:00:01"'
+
+check ovn-nbctl lsp-add ls0 ln_port
+check ovn-nbctl lsp-set-addresses ln_port unknown
+check ovn-nbctl lsp-set-type ln_port localnet
+check ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1
+
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl \
+-- add-br br-phys \
+-- add-br br-eth0
+
+ovn_attach n1 br-phys 192.168.0.10
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-bridge-mappings=physnet1:br-eth0])
+AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+# set garp max timeout to 2s
+AT_CHECK([as hv1 ovs-vsctl set Open_vSwitch . 
external-ids:garp-max-timeout-sec=2])
+
+# Wait until the patch ports are created in hv1 to connect br-int to br-eth0
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
+OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv1"])
+
+# sleep for 12s to get a garp every ~ 2s
+sleep 12
+
+n_arp=$(tcpdump -ner hv1/snoopvif-tx.pcap arp | wc -l)
+AT_CHECK([test $n_arp -ge 5 -a $n_arp -lt 10])
+
+# Temporarily remove lr0 chassis
+# Wait for hv confirmation to make sure chassis is removed before we reset pcap
+# Otherwise a garp might be sent after pcap have been reset but before chassis 
is removed
+AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options chassis])
+
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+# set garp max timeout to 1s
+AT_CHECK([as hv1 ovs-vsctl set Open_vSwitch . 
external-ids:garp-max-timeout-sec=1])
+AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
+
+# sleep for 7s to get a garp every ~ 1s
+sleep 7
+
+n_arp=$(tcpdump -ner hv1/snoopvif-tx.pcap arp | wc -l)
+AT_CHECK([test $n_arp -ge 5 -a $n_arp -lt 10])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+])
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] vconn: Count vconn_sent regardless of log level.

[Eelco Chaudron]

On 6 Jan 2024, at 10:23, Cheng Li wrote:

> vconn_sent counter is supposed to increase each time send() return
> 0, no matter if the vconn log debug is on or off.
>
> Signed-off-by: Cheng Li 

Thanks for catching and fixing this! The change looks good to me.

Acked-by: Eelco Chaudron 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn] test: add dedicated test for garp-max-timeout

[Lorenzo Bianconi]

> Hi Lorenzo,

Hi Mark,


thx for the review

> 
> I have some comments below
> 
> On 12/22/23 10:47, Lorenzo Bianconi wrote:
> > Introduce a dedicated test for garp-max-timeout knob
> > 
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> >   tests/ovn.at | 69 
> >   1 file changed, 69 insertions(+)
> > 
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 918f97a9e..deeb2a201 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -37464,3 +37464,72 @@ wait_for_ports_up
> >   OVN_CLEANUP([hv1])
> >   AT_CLEANUP
> >   ])
> > +
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([gratuitous arp max timeout])
> > +AT_KEYWORDS([garp-timeout])
> > +TAG_UNSTABLE
> > +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
> 
> Since this test uses fmt_pkt(), you'll also need to skip the test if
> $HAVE_SCAPY = no .

ack, I will add it.

> 
> > +ovn_start
> > +
> > +check ovn-nbctl ls-add ls0
> > +check ovn-nbctl lr-add lr0
> > +check ovn-nbctl lrp-add lr0 lr0-ls0 f0:00:00:00:00:01 192.168.0.1/24
> > +check ovn-nbctl lsp-add ls0 ls0-lr0 -- set Logical_Switch_Port ls0-lr0 \
> > +type=router options:router-port=lr0-ls0 addresses='"f0:00:00:00:00:01"'
> > +
> > +check ovn-nbctl lsp-add ls0 ln_port
> > +check ovn-nbctl lsp-set-addresses ln_port unknown
> > +check ovn-nbctl lsp-set-type ln_port localnet
> > +check ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1
> > +
> > +# Prepare packets
> > +echo $(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='f0:00:00:00:00:01')/ \
> > +ARP(op=2, hwsrc='f0:00:00:00:00:01', 
> > hwdst='00:00:00:00:00:00', psrc='192.168.0.1', pdst='192.168.0.1')") > 
> > arp_expected
> 
> the arp_expected packet is never referenced after this point. This can
> either be removed (and you can therefore ignore my comment earlier about
> needing to check for HAVE_SCAPY), or you need to check the pcaps for
> accuracy using arp_expected.

ack, I will get rid of it.

> 
> > +
> > +net_add n1
> > +sim_add hv1
> > +as hv1
> > +check ovs-vsctl \
> > +-- add-br br-phys \
> > +-- add-br br-eth0
> > +
> > +ovn_attach n1 br-phys 192.168.0.10
> > +
> > +AT_CHECK([ovs-vsctl set Open_vSwitch . 
> > external-ids:ovn-bridge-mappings=physnet1:br-eth0])
> > +AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
> > options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
> > +
> > +# set garp max timeout to 2s
> > +AT_CHECK([as hv1 ovs-vsctl set Open_vSwitch . 
> > external-ids:garp-max-timeout-sec=2])
> > +
> > +# Wait until the patch ports are created in hv1 to connect br-int to 
> > br-eth0
> > +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv1"])
> > +
> > +# sleep for 20s to get a garp every ~ 2s
> > +sleep 22
> 
> I understand why the sleeps are here, but maybe you could sleep for less
> time and check for fewer packets to arrive? This way you still are testing
> the garp-max-timeout but the test won't take as long to run.
> 
> And if you really do need a total of ~34 seconds of sleeping, then I think
> this test should have AT_KEYWORDS[slowtest].

ok, I will reduce the timeout and count less packets, but I will add the
keyword as well since it is still not fast ;)

> 
> > +
> > +n_arp=$(tcpdump -ner hv1/snoopvif-tx.pcap arp | wc -l)
> > +AT_CHECK([test $n_arp -ge 10 -a $n_arp -lt 15])
> > +
> > +# Temporarily remove lr0 chassis
> > +# Wait for hv confirmation to make sure chassis is removed before we reset 
> > pcap
> > +# Otherwise a garp might be sent after pcap have been reset but before 
> > chassis is removed
> > +AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options chassis])
> > +
> > +as hv1 reset_pcap_file snoopvif hv1/snoopvif
> > +# set garp max timeout to 1s
> > +AT_CHECK([as hv1 ovs-vsctl set Open_vSwitch . 
> > external-ids:garp-max-timeout-sec=1])
> > +hv1_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv1)
> 
> hv1_uuid is never used. You can probably remove this line.

I will get rid of it

> 
> > +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> > +
> > +# sleep for 12s to get a garp every ~ 1s
> > +sleep 12
> 
> Same comment about the sleep here: you can probably sleep for less time and
> check for fewer packets.

ditto.

Regards,
Lorenzo

> 
> > +
> > +n_arp=$(tcpdump -ner hv1/snoopvif-tx.pcap arp | wc -l)
> > +AT_CHECK([test $n_arp -ge 10 -a $n_arp -lt 15])
> > +
> > +OVN_CLEANUP([hv1])
> > +
> > +AT_CLEANUP
> > +])
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[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)
#89 FILE: utilities/ovn-ctl.8.xml:334:
Start OVN NB (or SB) clustered database on host with IP x.x.x.x without 
schema upgrade

WARNING: Line is 113 characters long (recommended limit is 79)
#90 FILE: utilities/ovn-ctl.8.xml:335:
# ovn-ctl start_nb_ovsdb --db-nb-cluster-local-addr=x.x.x.x 
--db-cluster-skip-upgrade=yes

WARNING: Line is 122 characters long (recommended limit is 79)
#92 FILE: utilities/ovn-ctl.8.xml:337:
# ovsdb-client -t 30 convert unix:/var/run/ovn/ovnnb_db.sock 
/usr/local/share/ovn/ovn-nb.ovsschema

Lines checked: 97, Warnings: 3, 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-ctl: Add option to skip schema conversion

[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-skip-upgrade` option is added that allows skipping
the upgrade.

Default value for this option is `no`, to preserve current default
behavior.

Signed-off-by: Martin Kalcok 
---
 utilities/ovn-ctl   |  7 ++-
 utilities/ovn-ctl.8.xml | 11 +++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
index 876565c80..011ef9c0c 100755
--- a/utilities/ovn-ctl
+++ b/utilities/ovn-ctl
@@ -184,6 +184,7 @@ start_ovsdb__() {
 local ovn_db_ssl_cacert
 local ovn_db_election_timer
 local relay_mode
+local skip_cluster_db_upgrade
 eval db_pid_file=\$DB_${DB}_PIDFILE
 eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
 eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
@@ -212,6 +213,7 @@ start_ovsdb__() {
 eval ovn_db_election_timer=\$DB_${DB}_ELECTION_TIMER
 eval relay_mode=\$RELAY_MODE
 eval relay_remote=\$DB_${DB}_REMOTE
+eval skip_cluster_db_upgrade=\$DB_CLUSTER_SKIP_UPGRADE
 
 ovn_install_dir "$OVN_RUNDIR"
 ovn_install_dir "$ovn_logdir"
@@ -347,7 +349,7 @@ $cluster_remote_port
 $(echo ovn-${db}ctl | tr _ -) --no-leader-only --db="unix:$sock" init
 fi
 
-if test $mode = cluster; then
+if test $mode = cluster && test X"$skip_cluster_db_upgrade" = Xno; then
 upgrade_cluster "$schema" "unix:$sock"
 fi
 
@@ -898,6 +900,8 @@ set_defaults () {
 OVN_SB_RELAY_DB_SSL_CERT=""
 OVN_SB_RELAY_DB_SSL_CA_CERT=""
 DB_SB_RELAY_USE_REMOTE_IN_DB="yes"
+
+DB_CLUSTER_SKIP_UPGRADE="no"
 }
 
 set_option () {
@@ -1148,6 +1152,7 @@ File location options:
   --ovn-sb-relay-db-ssl-key=KEY OVN_Southbound DB relay SSL private key file
   --ovn-sb-relay-db-ssl-cert=CERT OVN_Southbound DB relay SSL certificate file
   --ovn-sb-relay-db-ssl-ca-cert=CERT OVN OVN_Southbound DB relay SSL CA 
certificate file
+  --db-cluster-skip-upgrade=yes|no (default: $DB_CLUSTER_SKIP_UPGRADE)
 
 Default directories with "configure" option and environment variable override:
   logs: /usr/local/var/log/ovn (--with-logdir, OVN_LOGDIR)
diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml
index 01f4aa26b..5762fd3a4 100644
--- a/utilities/ovn-ctl.8.xml
+++ b/utilities/ovn-ctl.8.xml
@@ -156,6 +156,7 @@
 --db-ic-sb-cluster-remote-addr=IP ADDRESS
 --db-ic-sb-cluster-remote-port=PORT NUMBER
 --db-ic-sb-cluster-remote-proto=PROTO 
(tcp/ssl)
+--db-cluster-skip-upgrade=yes|no
 
  Probe interval options 
 --db-nb-probe-interval-to-active=Time in 
milliseconds
@@ -324,4 +325,14 @@
start_northd
   
 
+Avoiding automatic clustered OVN database schema upgrade
+
+  If you desire more controll over clustered DB schema upgrade, you can
+  opt-out of automatic on-start upgrade attempts with
+  --db-cluster-skip-upgrade.
+
+Start OVN NB (or SB) clustered database on host with IP x.x.x.x 
without schema upgrade
+# ovn-ctl start_nb_ovsdb --db-nb-cluster-local-addr=x.x.x.x 
--db-cluster-skip-upgrade=yes
+Trigger clustered DB schema upgrade manually
+# ovsdb-client -t 30 convert unix:/var/run/ovn/ovnnb_db.sock 
/usr/local/share/ovn/ovn-nb.ovsschema
 
-- 
2.40.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev