[ovs-dev] [ovsrobot] Announcing recheck feature

[Aaron Conole]

Greetings,

The ovsrobot has learned a new trick - it can now rerun the tests for a
specific patch (provided the patch is still in a state to be accepted).

Simply reply to the patch with a single comment:

Recheck-request: github-robot

or for OVN based projects, you can also run:

Recheck-request: github-robot-_Build_and_Test
Recheck-request: github-robot-_ovn-kubernetes

Note that the Recheck-request format looks as follows:

^Recheck-request: [a-zA-Z ,-]*

Other labs can implement the same support and the way to specify
multiple tests is via comma separated values, ex:

Recheck-request: github-robot-_Build_and_Test, github-robot-_ovn-kubernetes

Please see https://github.com/ovsrobot/pw-ci/blob/master/RECHECK.rst for
more details on the matching infrastructure.  Or feel free to ask here.

Thanks,
-Aaron

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

Re: [ovs-dev] [PATCH ovn] ovn-macros: Make sure stopped daemons continue before the test ends.

[Xavier Simonart]

Hi Dumitru

Thanks for the patch.

On Thu, Dec 7, 2023 at 2:12 PM Dumitru Ceara  wrote:

> In case there's a test failure while daemons are stopped ensure that we
> send a SIGCONT on exit so that they properly clean up.
>
> 30952c248d4f ("binding: fixed ovn-installed not properly removed
> (recomputes)")
>
Missing "Fixes: "

> Fixes: 0794a6edf40b ("qos: fix potential double deletion of ovs idl row")
> Fixes: feb918434172 ("northd: Skip transient IDL records.")
> Suggested-by: Ilya Maximets 
> Signed-off-by: Dumitru Ceara 
> ---
>  tests/ovn-macros.at | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 94bdaff2c4..dd5df40b35 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -894,6 +894,7 @@ start_scapy_server() {
>  sleep_northd() {
>echo Northd going to sleep
>AT_CHECK([kill -STOP $(cat northd/ovn-northd.pid)])
> +  on_exit "kill -CONT $(cat northd/ovn-northd.pid)"
>  }
>
>  wake_up_northd() {
> @@ -904,6 +905,7 @@ wake_up_northd() {
>  sleep_sb() {
>echo SB going to sleep
>AT_CHECK([kill -STOP $(cat ovn-sb/ovsdb-server.pid)])
> +  on_exit "kill -CONT $(cat ovn-sb/ovsdb-server.pid)"
>  }
>  wake_up_sb() {
>echo SB waking up
> @@ -927,6 +929,7 @@ sleep_ovs() {
>hv=$1
>echo ovs $hv going to sleep
>AT_CHECK([kill -STOP $(cat $hv/ovs-vswitchd.pid)])
> +  on_exit "kill -CONT $(cat $hv/ovs-vswitchd.pid)"
>  }
>
>  wake_up_ovs() {
> @@ -938,6 +941,7 @@ wake_up_ovs() {
>  sleep_ovsdb() {
>echo OVSDB $1 going to sleep
>AT_CHECK([kill -STOP $(cat $1/ovsdb-server.pid)])
> +  on_exit "kill -CONT $(cat $1/ovsdb-server.pid)"
>  }
>  wake_up_ovsdb() {
>echo OVSDB $1 waking up
> --
> 2.39.3
>
> Except for the missing "Fixes" in the commit message, it looks good to me.
Acked-by: Xavier Simonart 

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

Re: [ovs-dev] [PATCH ovn v3 3/3] Documentation: Add note about pinning the container after release

[Dumitru Ceara]

On 11/23/23 13:38, Ales Musil wrote:
> Add note that one the patches after branching should
> pin the container to the current LTS version so the
> CI on that branch remains stable as long as possible.
> 
> Signed-off-by: Ales Musil 
> ---
>  Documentation/internals/release-process.rst | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/internals/release-process.rst 
> b/Documentation/internals/release-process.rst
> index ec79fe48c..26d3f8d4d 100644
> --- a/Documentation/internals/release-process.rst
> +++ b/Documentation/internals/release-process.rst
> @@ -64,6 +64,10 @@ Scheduling`_ for the timing of each stage:
> branch.  Features to be added to release branches should be limited in 
> scope
> and risk and discussed on ovs-dev before creating the branch.
>  
> +   In order to keep the CI stable on the new release branch, the Ubuntu
> +   container should be pinned to the current LTS version in the Dockerfile
> +   e.g. registry.hub.docker.com/library/ubuntu:22.04.
> +

Should we already move stable branches to ubuntu:22.04?

Regardless:

Acked-by: Dumitru Ceara 

>  3. When committers come to rough consensus that the release is ready, they
> release the .0 release on its branch, e.g. 25.09.0 for branch-25.09.  To
> make the actual release, a committer pushes a signed tag named, e.g.

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

Re: [ovs-dev] [PATCH ovn v3 2/3] ci: Cover more container posibilities

[Dumitru Ceara]

On 11/23/23 13:38, Ales Musil wrote:
> Add more conditions to the image prepare process.
> This allows us to test the prebuilt images for both
> Fedora and Ubuntu.
> 
> Run the weekly image on main branch with the
> use of Fedora for the weekly runs.
> 
> Reported-at: https://issues.redhat.com/browse/FDP-149
> Signed-off-by: Ales Musil 
> ---
> v3: Add comment that explains the matrix that we have for the prepare-image 
> job.
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara 


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

Re: [ovs-dev] [PATCH ovn v3 1/3] ci: Build container image before very job

[Dumitru Ceara]

On 11/23/23 13:38, Ales Musil wrote:
> Build the image before every job to allow more
> fine-grained dependency pinning. This is especially
> useful for stable branches as they might need to stay
> on specific distribution or Python version.
> 
> This also allows us to switch to Ubuntu instead
> of Fedora in the Cirrus CI for consistency reasons.
> 
> Signed-off-by: Ales Musil 
> ---
> v3: Fix the Cirrus CI architecture to ensure it is really running on ARM64.
> Add comment why do we need separate task in the Cirrus CI.
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara 

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

Re: [ovs-dev] [PATCH v2] system-dpdk: Wait for MTU changes to be applied.

[Kevin Traynor]

On 01/12/2023 14:29, David Marchand wrote:

Because a DPDK backed netdev configuration is done in an asynchronous way,
and a MTU change requires a reconfiguration, directly checking
ovs-vswitchd logs or querying ovsdb for the interface current MTU value
is racy.

Add synchronisation points on the interface MTU value in ovsdb as it
ensures that a netdev (re)configuration did happen.
With those synchronisation points in place, error messages may be checked
in logs afterward.

Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
Signed-off-by: David Marchand
---
Changes since v1:
- dropped test output,


---
  tests/system-dpdk.at | 42 --
  1 file changed, 12 insertions(+), 30 deletions(-)


Thanks David. Applied on master branch and backported with a minor 
rebase to the relevant branches (3.0/3.1/3.2).


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

Re: [ovs-dev] [PATCH ovn v3] northd: forward arp request to lrp snat on.

[Dumitru Ceara]

On 12/6/23 02:56, Daniel Ding wrote:
> Hi Dumitru!
> 
> On Tue, 5 Dec 2023 at 23:59, Dumitru Ceara  wrote:
> 
>> On 12/5/23 13:58, Daniel Ding wrote:
>>>
>>>
>>> On Tue, 5 Dec 2023 at 18:58, Dumitru Ceara >> > wrote:
>>>
>>> Hi Daniel,
>>>
>>> Thanks for this new revision but why is it v3?  I don't think I saw
>> v2
>>> posted anywhere but maybe I missed it.
>>>
>>>
>>> Sorry, that is my mistake.
>>>
>>
>> No problem.
>>
>>>
>>> On 12/5/23 08:33, Daniel Ding wrote:
>>> > If the router has a snat rule and it's external ip isn't lrp
>> address,
>>> > when the arp request from other router for this external ip, will
>>> > be drop, because of this external ip use same mac address as lrp,
>> so
>>> > can not forward to MC_FLOOD.
>>> >
>>> > Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain
>>> whenever possible.")
>>> > Reported-at: https://github.com/ovn-org/ovn/issues/209
>>> 
>>> >
>>> > Signed-off-by: Daniel Ding >> >
>>> > Acked-by: Dumitru Ceara > dce...@redhat.com>>
>>>
>>> Please don't add an "Acked-by: ... " if the person never explicitly
>>> replied with "Acked-by: ... " on the previous version of the patch or
>>> if you didn't have explicit agreement from that person to do so.
>>>
>>> Quoting from my previous reply to your v1, I said:
>>>
>>> "I think it makes sense to do what you're suggesting."
>>>
>>>
>> https://patchwork.ozlabs.org/project/ovn/patch/callnitdhfcetzapkhjtddyx1cx_2cys2otqscrhpv6jse0m...@mail.gmail.com/#3214934
>> <
>> https://patchwork.ozlabs.org/project/ovn/patch/callnitdhfcetzapkhjtddyx1cx_2cys2otqscrhpv6jse0m...@mail.gmail.com/#3214934
>>>
>>>
>>> That doesn't mean I acked the patch.
>>>
>>>
>>> Got it. Thx!
>>>
>>
>> No worries.
>>
>>>
>>> > ---
>>> >  northd/northd.c | 18 +-
>>> >  tests/ovn-northd.at  | 12 
>>> >  2 files changed, 29 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/northd/northd.c b/northd/northd.c
>>> > index e9cb906e2..99fb30f46 100644
>>> > --- a/northd/northd.c
>>> > +++ b/northd/northd.c
>>> > @@ -8974,6 +8974,9 @@ build_lswitch_rport_arp_req_flows(struct
>>> ovn_port *op,
>>> >  }
>>> >  }
>>> >
>>> > +struct sset snat_ips_v4 = SSET_INITIALIZER(&snat_ips_v4);
>>> > +struct sset snat_ips_v6 = SSET_INITIALIZER(&snat_ips_v6);
>>> > +
>>> >  for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
>>> >  struct ovn_nat *nat_entry = &op->od->nat_entries[i];
>>> >  const struct nbrec_nat *nat = nat_entry->nb;
>>> > @@ -8983,7 +8986,17 @@ build_lswitch_rport_arp_req_flows(struct
>>> ovn_port *op,
>>> >  }
>>> >
>>> >  if (!strcmp(nat->type, "snat")) {
>>> > -continue;
>>> > +if (nat_entry_is_v6(nat_entry)) {
>>> > +if (sset_contains(&snat_ips_v6,
>> nat->external_ip)) {
>>> > +continue;
>>> > +}
>>> > +sset_add(&snat_ips_v6, nat->external_ip);
>>> > +} else {
>>> > +if (sset_contains(&snat_ips_v4,
>> nat->external_ip)) {
>>> > +continue;
>>> > +}
>>> > +sset_add(&snat_ips_v4, nat->external_ip);
>>> > +}
>>>
>>> Essentially this just makes sure we don't skip NAT entries and that
>> we
>>> consider unique external_ips.  I'm fine with relaxing the second
>> part of
>>> the condition in which case, as mentioned on v1, I think we can just
>>> remove the whole "if (!strcmp(nat->type, "snat")) {" block.
>>>
>>> With the following incremental change applied (it removes the block)
>>> your test still passes:
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index df7d2d60a5..20efd3b74c 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -9372,9 +9372,6 @@ build_lswitch_rport_arp_req_flows(struct
>>> ovn_port *op,
>>>  }
>>>  }
>>>
>>> -struct sset snat_ips_v4 = SSET_INITIALIZER(&snat_ips_v4);
>>> -struct sset snat_ips_v6 = SSET_INITIALIZER(&snat_ips_v6);
>>> -
>>>  for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
>>>  struct ovn_nat *nat_entry = &op->od->nat_entries[i];
>>>  const struct nbrec_nat *nat = nat_entry->nb;
>>> @@ -9383,20 +9380,6 @@ build_lswitch_rport_arp_req_flows(struct
>>> ovn_port *op,
>>>  continue;
>>>  }
>>>
>>> -if (!strcmp(nat->type, "snat")) {
>>> -if (nat_entry_is_v6(nat_entry)) {
>>> -if (sset_contains(&snat_ips_v6, nat->external_ip)) {
>>> -continue;
>>> -   

Re: [ovs-dev] [PATCH ovn] system-test: Fix tcpdump usage in LB template tests.

[Dumitru Ceara]

On 12/7/23 13:18, Eelco Chaudron wrote:
> 
> 
> On 5 Dec 2023, at 0:09, Dumitru Ceara wrote:
> 
>> Make sure the capture is up before continuing the test.
>> A similar fix was committed earlier via 6c4ffe5f111f
>> ("system-test: Use OVS_WAIT_UNTIL for tcpdump start instead
>> fo sleep") but other tests were added in the meantime.
>>
>> Suggested-by: Xavier Simonart 
>> Fixes: 086744a645a7 ("northd: Use LB port_str in northd.")
>> Reported-at: https://issues.redhat.com/browse/FDP-192
>> Signed-off-by: Dumitru Ceara 
> 
> This change looks good to me, we use similar checks in OVS.
> 
> Acked-by: Eelco Chaudron 
> 

Thanks, Eelco!  I pushed this to main and stable branches down to 22.12.

Regards,
Dumitru

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

[ovs-dev] [PATCH ovn] ovn-macros: Make sure stopped daemons continue before the test ends.

[Dumitru Ceara]

In case there's a test failure while daemons are stopped ensure that we
send a SIGCONT on exit so that they properly clean up.

30952c248d4f ("binding: fixed ovn-installed not properly removed (recomputes)")
Fixes: 0794a6edf40b ("qos: fix potential double deletion of ovs idl row")
Fixes: feb918434172 ("northd: Skip transient IDL records.")
Suggested-by: Ilya Maximets 
Signed-off-by: Dumitru Ceara 
---
 tests/ovn-macros.at | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 94bdaff2c4..dd5df40b35 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -894,6 +894,7 @@ start_scapy_server() {
 sleep_northd() {
   echo Northd going to sleep
   AT_CHECK([kill -STOP $(cat northd/ovn-northd.pid)])
+  on_exit "kill -CONT $(cat northd/ovn-northd.pid)"
 }
 
 wake_up_northd() {
@@ -904,6 +905,7 @@ wake_up_northd() {
 sleep_sb() {
   echo SB going to sleep
   AT_CHECK([kill -STOP $(cat ovn-sb/ovsdb-server.pid)])
+  on_exit "kill -CONT $(cat ovn-sb/ovsdb-server.pid)"
 }
 wake_up_sb() {
   echo SB waking up
@@ -927,6 +929,7 @@ sleep_ovs() {
   hv=$1
   echo ovs $hv going to sleep
   AT_CHECK([kill -STOP $(cat $hv/ovs-vswitchd.pid)])
+  on_exit "kill -CONT $(cat $hv/ovs-vswitchd.pid)"
 }
 
 wake_up_ovs() {
@@ -938,6 +941,7 @@ wake_up_ovs() {
 sleep_ovsdb() {
   echo OVSDB $1 going to sleep
   AT_CHECK([kill -STOP $(cat $1/ovsdb-server.pid)])
+  on_exit "kill -CONT $(cat $1/ovsdb-server.pid)"
 }
 wake_up_ovsdb() {
   echo OVSDB $1 waking up
-- 
2.39.3

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

Re: [ovs-dev] [PATCH ovn 0/3] Improve in-tree ovn-northd perf testing.

[Dumitru Ceara]

On 11/24/23 16:20, Dumitru Ceara wrote:
> It might not be very known but the OVN code base actually has its
> own integrated performance self tests for ovn-northd.
> 
> These allow benchmarking northd performance for a couple of common
> scenarios.
> 
> In order to run them one needs to use the 'make check-perf' command
> (also documented in 'Documentation/topics/testing.rst'.
> 
> This series improves the performance testing by:
> - avoiding to start ovn-controllers (they're not relevant in this
>   context)
> - adding more stopwatch information about various northd components
> - triggering and measuring the effect of a full northd recompute
>   at the end of a run.
> 
> Dumitru Ceara (3):
>   perf-northd.at: Don't start ovn-controllers.
>   perf-northd.at: Parse and display more stopwatch data.
>   perf-northd.at: Add ovn-northd recompute statistics.
> 

Thanks, Mark and Numan, for the review (see replies on individual
patches).  I applied this series to main.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] system-tests: Consolidate wait condition in CoPP test

[Dumitru Ceara]

On 11/21/23 10:52, Ales Musil wrote:
> Add missing wait condition and at the same time
> move the wait to the last ovn-nbctl command
> of the chain, as there is no need to wait
> if there are more command afterward.
> 
> Signed-off-by: Ales Musil 
> ---

Thanks for the cleanup, Ales!  I applied this to main and backported it
down to 22.03.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] pinctrl: Fix up comments about sending RST packets for healthcheck.

[Dumitru Ceara]

On 11/23/23 15:36, Ales Musil wrote:
> 
> 
> On Mon, Nov 20, 2023 at 8:45 PM Dumitru Ceara  > wrote:
> 
> We're sending TCP RST not RST-ACK as the comment suggests.
> 
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409288.html 
> 
> Reported-by: Mark Michelson  >
> Fixes: a35725a7a24b ("pinctrl: send RST instead of RST_ACK bit for
> lb hc")
> Signed-off-by: Dumitru Ceara  >
> ---
>  controller/pinctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 62cf4da324..1b176490ea 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -7887,7 +7887,7 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn,
>          svc_mon->n_success++;
>          svc_mon->state = SVC_MON_S_ONLINE;
> 
> -        /* Send RST-ACK packet. */
> +        /* Send RST packet. */
>          svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST,
>                                              htonl(tcp_ack),
>                                              htonl(0), th->tcp_dst);
> -- 
> 2.39.3
> 
> ___
> dev mailing list
> d...@openvswitch.org 
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> 
> 
> Looks good to me, thanks.
> 
> Acked-by: Ales Musil mailto:amu...@redhat.com>>
> 

Thanks, Ales!  Applied to main and backported down to 22.03.

Regards,
Dumitru


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

Re: [ovs-dev] [PATCH ovn 23.06] controller: make garp_max_timeout configurable

[Dumitru Ceara]

On 12/6/23 17:34, Lorenzo Bianconi wrote:
> When using VLAN backed networks and OVN routers leveraging the
> 'ovn-chassis-mac-mappings' option for east-west traffic, the eth.src field is
> replaced by the chassis mac address in order to not expose the router mac
> address from different nodes and confuse the TOR switch. However doing so
> the TOR switch is not able to learn the port/mac bindings for routed E/W
> traffic and it is force to always flood it. Fix this issue adding the
> capability to configure a given timeout for garp sent by ovn-controller
> and not disable it after the exponential backoff in order to keep
> refreshing the entries in TOR swtich fdb table.
> More into about the issue can be found here [0].
> 
> [0] 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779
> 
> Signed-off-by: Lorenzo Bianconi 
> Acked-by: Ales Musil 
> Signed-off-by: Mark Michelson 
> ---

Thanks for the backport patches, Lorenzo!  I applied them to branches
22.03 -> 23.09.

Next time though, please:
- add a "cherry picked from ... " line to the commit
- if the backport is straightforward (e.g., this one only had NEWS
conflicts) just request the backport on the original patch email.
- use the correct "[branch-xy.zt]" prefix to the patch subject;
otherwise it confuses the 0-day bot.

Why do we need to backport this to 21.12 though?  That's before the last
LTS and not supported.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH OVN 2/2] tests: Move SCTP test from kernel only to general OVN system tests.

[Dumitru Ceara]

On 12/5/23 08:52, Eelco Chaudron wrote:
> 
> 
> On 4 Dec 2023, at 19:57, Mark Michelson wrote:
> 
>> Thanks for the updates, Eelco. What is the minimum OVS version required for 
>> the SCTP load balancer test to pass when using the userspace datapath? This 
>> will help me to backport this series to the proper OVN branches.
> 
> Hi Mark,
> 
> It’s in 3.2 and above:
> 
>   $ git tag --contains 501f665a5a4b3eafa75f020ab77c1d62f7840172
>   v3.2.0
>   v3.2.1
> 

Thanks, Eelco, Mark and Simon!  I applied this to main and branch-23.09.

Regards,
Dumitru

> Cheers,
> 
> Eelco
> 
> 
>> On 11/30/23 09:18, Eelco Chaudron wrote:
>>> This patch moves the SCTP test from the kernel only, to the general OVN
>>> system tests, enabling its execution in both the system-userspace and
>>> system-dpdk test scenarios.
>>>
>>> Signed-off-by: Eelco Chaudron 
>>> ---
>>>   tests/system-ovn-kmod.at |  215 
>>> --
>>>   tests/system-ovn.at  |  211 
>>> +
>>>   2 files changed, 211 insertions(+), 215 deletions(-)
>>>
>>> diff --git a/tests/system-ovn-kmod.at b/tests/system-ovn-kmod.at
>>> index 9eaf07147..f777376c6 100644
>>> --- a/tests/system-ovn-kmod.at
>>> +++ b/tests/system-ovn-kmod.at
>>> @@ -1,220 +1,5 @@
>>>   AT_BANNER([system-ovn-kmod])
>>>  -# SCTP and userspace conntrack do not mix. Therefore this
>>> -# test only can be run with kernel datapath. Otherwise,
>>> -# this is mostly a copy of existing load balancer tests
>>> -# in system-ovn.at
>>> -AT_SETUP([load balancing in gateway router - SCTP])
>>> -AT_SKIP_IF([test $HAVE_SCTP = no])
>>> -AT_SKIP_IF([test $HAVE_NC = no])
>>> -AT_KEYWORDS([ovnlb sctp])
>>> -
>>> -# Make sure the SCTP kernel module is loaded.
>>> -LOAD_MODULE([sctp])
>>> -
>>> -CHECK_CONNTRACK()
>>> -CHECK_CONNTRACK_NAT()
>>> -ovn_start
>>> -OVS_TRAFFIC_VSWITCHD_START()
>>> -ADD_BR([br-int])
>>> -
>>> -# Set external-ids in br-int needed for ovn-controller
>>> -ovs-vsctl \
>>> --- set Open_vSwitch . external-ids:system-id=hv1 \
>>> --- set Open_vSwitch . 
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>> --- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>> --- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>> --- set bridge br-int fail-mode=secure 
>>> other-config:disable-in-band=true
>>> -
>>> -# Start ovn-controller
>>> -start_daemon ovn-controller
>>> -
>>> -# Logical network:
>>> -# Two LRs - R1 and R2 that are connected to each other via LS "join"
>>> -# in 20.0.0.0/24 network. R1 has switchess foo (192.168.1.0/24) and
>>> -# bar (192.168.2.0/24) connected to it. R2 has alice (172.16.1.0/24) 
>>> connected
>>> -# to it.  R2 is a gateway router on which we add load-balancing rules.
>>> -#
>>> -#foo -- R1 -- join - R2 -- alice
>>> -#   |
>>> -#bar 
>>> -
>>> -ovn-nbctl create Logical_Router name=R1
>>> -ovn-nbctl create Logical_Router name=R2 options:chassis=hv1
>>> -
>>> -ovn-nbctl ls-add foo
>>> -ovn-nbctl ls-add bar
>>> -ovn-nbctl ls-add alice
>>> -ovn-nbctl ls-add join
>>> -
>>> -# Connect foo to R1
>>> -ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03 192.168.1.1/24
>>> -ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \
>>> -type=router options:router-port=foo addresses=\"00:00:01:01:02:03\"
>>> -
>>> -# Connect bar to R1
>>> -ovn-nbctl lrp-add R1 bar 00:00:01:01:02:04 192.168.2.1/24
>>> -ovn-nbctl lsp-add bar rp-bar -- set Logical_Switch_Port rp-bar \
>>> -type=router options:router-port=bar addresses=\"00:00:01:01:02:04\"
>>> -
>>> -# Connect alice to R2
>>> -ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24
>>> -ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \
>>> -type=router options:router-port=alice addresses=\"00:00:02:01:02:03\"
>>> -
>>> -# Connect R1 to join
>>> -ovn-nbctl lrp-add R1 R1_join 00:00:04:01:02:03 20.0.0.1/24
>>> -ovn-nbctl lsp-add join r1-join -- set Logical_Switch_Port r1-join \
>>> -type=router options:router-port=R1_join addresses='"00:00:04:01:02:03"'
>>> -
>>> -# Connect R2 to join
>>> -ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04 20.0.0.2/24
>>> -ovn-nbctl lsp-add join r2-join -- set Logical_Switch_Port r2-join \
>>> -type=router options:router-port=R2_join addresses='"00:00:04:01:02:04"'
>>> -
>>> -# Static routes.
>>> -ovn-nbctl lr-route-add R1 172.16.1.0/24 20.0.0.2
>>> -ovn-nbctl lr-route-add R2 192.168.0.0/16 20.0.0.1
>>> -
>>> -# Logical port 'foo1' in switch 'foo'.
>>> -ADD_NAMESPACES(foo1)
>>> -ADD_VETH(foo1, foo1, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \
>>> - "192.168.1.1")
>>> -ovn-nbctl lsp-add foo foo1 \
>>> --- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2"
>>> -
>>> -# Logical port 'alice1' in switch 'alice'.
>>> -ADD_NAMESPACES(alice1)
>>> -ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24", "f0:00:00:01:02:04", \
>>> - "172.16.1.1")
>>> -ovn-nbctl lsp-add alice alice1 \
>>

Re: [ovs-dev] [PATCH OVN 1/2] tests: Remove 'protoinfo' from the conntrack entries for SCTP tests.

[Dumitru Ceara]

On 12/1/23 15:28, Simon Horman wrote:
> On Thu, Nov 30, 2023 at 03:18:03PM +0100, Eelco Chaudron wrote:
>> The userspace lacks the supplementary protocol state machine for SCTP,
>> resulting in the absence of 'protoinfo' fields. Nevertheless, this SCTP
>> test doesn't need this feature, making the check for it unnecessary.
>>
>> Signed-off-by: Eelco Chaudron 
> 
> Acked-by: Simon Horman 
> 

Thanks, Eelco and Simon!  I applied this to main and branch-23.09.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn] northd: Skip transient IDL records.

[Dumitru Ceara]

On 12/6/23 22:29, Numan Siddique wrote:
> On Wed, Dec 6, 2023 at 9:03 AM Dumitru Ceara  wrote:
>>
>> When multiple OVSDB updates have been received since the last northd run
>> it's possible that the IDL tracks changes for database entities that
>> were added _and also_ removed from the last time the northd processing
>> engin run.  In some cases, those may appear as being simultaneously
>> "new" and "deleted".
>>
>> Currently, the only tables for which can be a problem are
>> NB.Load_Balancer and NB.Load_Balancer_Group.
>>
>> Skip these "transient records" to avoid adding soon to be deleted rows
>> to the tracked_lb_data->crupdated_lbs records.  These are used to build
>> 'northd' I-P engine state in northd_handle_lb_data_changes().
>>
>> We also avoid crashing if "unexpected" deletes are reported by the IDL.
>> This is likely due to a bug in the IDL [0] but it's easy to avoid on the
>> northd side.
>>
>> This commit also adds a test case which _might_ detect the issue when
>> run under valgrind.  The test case can't always detect the problem
>> because a prerequisite for a Load Balancer to be "transient" is that the
>> IDL processes the update that removes it from the NB Load_Balancer table
>> and from the Load_Balancer_Group row that was referring to it in the
>> following order: Load_Balancer_Group table update first and then
>> Load_Balancer deletion.  The order is controlled by the way
>> 'struct shash' hashes records (table names in this case) and that's
>> arch and/or compiler dependent.
>>
>> [0] https://issues.redhat.com/browse/FDP-193
>>
>> Fixes: a24eed5cc6b4 ("northd: Add initial I-P for load balancer and load 
>> balancer groups")
>> Reported-at: https://issues.redhat.com/browse/FDP-181
>> Signed-off-by: Dumitru Ceara 
> 
> Thanks for fixing this issue.
> 
> Acked-by: Numan Siddique 
> 
> Please see below a couple of nits.
> 

Thanks for the review!

>> ---
>>  northd/en-lb-data.c | 52 +
>>  tests/ovn-macros.at | 10 +
>>  tests/ovn-northd.at | 37 
>>  3 files changed, 81 insertions(+), 18 deletions(-)
>>
>> diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
>> index 250cec848b..d06f46a54b 100644
>> --- a/northd/en-lb-data.c
>> +++ b/northd/en-lb-data.c
>> @@ -144,6 +144,12 @@ lb_data_load_balancer_handler(struct engine_node *node, 
>> void *data)
>>
>>  const struct nbrec_load_balancer *tracked_lb;
>>  NBREC_LOAD_BALANCER_TABLE_FOR_EACH_TRACKED (tracked_lb, nb_lb_table) {
>> +/* "New" + "Deleted" is a no-op. */
>> +if (nbrec_load_balancer_is_new(tracked_lb)
>> +&& nbrec_load_balancer_is_deleted(tracked_lb)) {
>> +continue;
>> +}
>> +
>>  struct ovn_northd_lb *lb;
>>  if (nbrec_load_balancer_is_new(tracked_lb)) {
>>  /* New load balancer. */
>> @@ -153,19 +159,22 @@ lb_data_load_balancer_handler(struct engine_node 
>> *node, void *data)
>>  add_crupdated_lb_to_tracked_data(lb, trk_lb_data,
>>   lb->health_checks);
>>  trk_lb_data->has_routable_lb |= lb->routable;
>> -} else if (nbrec_load_balancer_is_deleted(tracked_lb)) {
>> -lb = ovn_northd_lb_find(&lb_data->lbs,
>> -&tracked_lb->header_.uuid);
>> -ovs_assert(lb);
>> +continue;
>> +}
>> +
>> +/* Protect against "spurious" deletes reported by the IDL. */
>> +lb = ovn_northd_lb_find(&lb_data->lbs, &tracked_lb->header_.uuid);
>> +if (!lb) {
>> +continue;
>> +}
>> +
>> +if (nbrec_load_balancer_is_deleted(tracked_lb)) {
>>  hmap_remove(&lb_data->lbs, &lb->hmap_node);
>>  add_deleted_lb_to_tracked_data(lb, trk_lb_data,
>> lb->health_checks);
>>  trk_lb_data->has_routable_lb |= lb->routable;
>>  } else {
>>  /* Load balancer updated. */
>> -lb = ovn_northd_lb_find(&lb_data->lbs,
>> -&tracked_lb->header_.uuid);
>> -ovs_assert(lb);
>>  bool health_checks = lb->health_checks;
>>  struct sset old_ips_v4 = SSET_INITIALIZER(&old_ips_v4);
>>  struct sset old_ips_v6 = SSET_INITIALIZER(&old_ips_v6);
>> @@ -217,6 +226,12 @@ lb_data_load_balancer_group_handler(struct engine_node 
>> *node, void *data)
>>  const struct nbrec_load_balancer_group *tracked_lb_group;
>>  NBREC_LOAD_BALANCER_GROUP_TABLE_FOR_EACH_TRACKED (tracked_lb_group,
>>nb_lbg_table) {
>> +/* "New" + "Deleted" is a no-op. */
>> +if (nbrec_load_balancer_group_is_new(tracked_lb_group)
>> +&& nbrec_load_balancer_group_is_deleted(tracked_lb_group)) {
>> +continue;
>> +}
>> +
>>  if (nbrec_load_balancer_group_is_new(

Re: [ovs-dev] [PATCH ovn] system-test: Fix tcpdump usage in LB template tests.

[Eelco Chaudron]

On 5 Dec 2023, at 0:09, Dumitru Ceara wrote:

> Make sure the capture is up before continuing the test.
> A similar fix was committed earlier via 6c4ffe5f111f
> ("system-test: Use OVS_WAIT_UNTIL for tcpdump start instead
> fo sleep") but other tests were added in the meantime.
>
> Suggested-by: Xavier Simonart 
> Fixes: 086744a645a7 ("northd: Use LB port_str in northd.")
> Reported-at: https://issues.redhat.com/browse/FDP-192
> Signed-off-by: Dumitru Ceara 

This change looks good to me, we use similar checks in OVS.

Acked-by: Eelco Chaudron 

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

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

[Eelco Chaudron]

On 1 Dec 2023, at 15:37, David Marchand wrote:

> On Tue, Nov 28, 2023 at 9:40 AM David Marchand
>  wrote:
>>
>> On Wed, Nov 22, 2023 at 5:34 PM David Marchand
>>  wrote:
>>>
>>> The DPDK unit test only runs if vfio or igb_uio kernel modules are loaded:
>>> on systems with only mlx5, this test is always skipped.
>>>
>>> Besides, the test tries to grab the first device listed by dpdk-devbind.py,
>>> regardless of the PCI device status regarding kmod binding.
>>>
>>> Remove dependency on this DPDK script and use a minimal script that
>>> reads PCI sysfs.
>>>
>>> This script is not perfect, as one can imagine PCI devices bound to
>>> vfio-pci for virtual machines.
>>> Plus, this script only tries to take over vfio-pci devices. mlx5 devices
>>> can't be taken over blindly as it could mean losing connectivity to the
>>> machine if the netdev was in use for this system.
>>>
>>> For those two reasons, add a new environment variable DPDK_PCI_ADDR for
>>> testers to select the PCI device of their liking.
>>> For consistency and grep, the temporary file PCI_ADDR is renamed
>>> to DPDK_PCI_ADDR.
>>>
>>> Reviewed-by: Maxime Coquelin 
>>> Acked-by: Eelco Chaudron 
>>> Signed-off-by: David Marchand 
>>
>> This patch can't be merged as is.
>> I am preparing some fixes for the system-dpdk MTU tests that got
>> merged since my v4.
>
> Coming back on this.
> I sent a fix 
> https://patchwork.ozlabs.org/project/openvswitch/patch/20231201142931.1782046-1-david.march...@redhat.com/.
> This current patch on making it possible to select a PCI device to
> test is kind of orthogonal to this fix.
> Yet, people will likely want to test with both applied.

Tested with both patches applied and it still looks good to me. Re-Acking for 
the record ;)

//Eelco

Acked-by: Eelco Chaudron 

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

Re: [ovs-dev] [PATCH v2] system-dpdk: Wait for MTU changes to be applied.

[Eelco Chaudron]

On 1 Dec 2023, at 15:29, David Marchand wrote:

> Because a DPDK backed netdev configuration is done in an asynchronous way,
> and a MTU change requires a reconfiguration, directly checking
> ovs-vswitchd logs or querying ovsdb for the interface current MTU value
> is racy.
>
> Add synchronisation points on the interface MTU value in ovsdb as it
> ensures that a netdev (re)configuration did happen.
> With those synchronisation points in place, error messages may be checked
> in logs afterward.
>
> Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
> Signed-off-by: David Marchand 
> ---
> Changes since v1:
> - dropped test output,

Thanks for following this true. The changes look 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] reconnect: Set defaults from environment variables.

[Eelco Chaudron]

On 4 Dec 2023, at 11:38, Felix Huettner wrote:

> Hi Eelco,
>
> thanks for the feedback. I updated it and send a v3.
>
> On Fri, Dec 01, 2023 at 10:38:16AM +0100, Eelco Chaudron wrote:
>>
>>
>> On 8 Nov 2023, at 11:07, Felix Huettner via dev wrote:
>>
>>> this exposes the old constants regarding min backoff, max backoff and
>>
>> This
>>
>
> sorry, i missed that one, so its still wrong. But if there is a v4 it
> will be included.

Thanks for fixing the items mentioned below. I’ll wait for Ilya to take a look 
at the v3 to see if we need more changes.

Cheers,

Eelco


>>>
>>> +/* Returns the default min_backoff value for reconnect. It uses the 
>>> environment
>>> + * variable OVS_RECONNECT_MIN_BACKOFF if set and valid or otherwise 
>>> defaults
>>> + * to 1 second. The return value is in ms. */
>>> +unsigned int reconnect_default_min_backoff(void) {
>>> +static unsigned int default_min_backoff = 0;
>>> +if (default_min_backoff == 0) {
>>> +char *env = getenv("OVS_RECONNECT_MIN_BACKOFF");
>>> +if (env && env[0]) {
>>> +str_to_uint(env, 10, &default_min_backoff);
>>> +}
>>> +if (default_min_backoff == 0) {
>>
>> I think we should define some sane minimum value. Or are we ok with 1ms? 
>> Some for all the others.
>
> I now defined 1000ms as minimum for the min_backoff and probe_interval
> and used 2000ms for max_backoff. Let me know if you find other values
> more reasonable.
>
>>
>>> +default_min_backoff = 1000;
>>
>> I think we should keep the default definitions, and use them here rather 
>> than the hard-coded numbers.
>> So this will become:
>>
>>   default_min_backoff = RECONNECT_DEFAULT_MIN_BACKOFF
>>
>
> Yes i readded them as defines at the top of the file (so they migrate
> from reconnect.h to reconnect.c)
>
> Thanks
> Felix
> Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die 
> Verwertung durch den vorgesehenen Empfänger bestimmt.
> Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender 
> bitte unverzüglich in Kenntnis und löschen diese E Mail.
>
> Hinweise zum Datenschutz finden Sie hier .
>
>
> This e-mail may contain confidential content and is intended only for the 
> specified recipient/s.
> If you are not the intended recipient, please inform the sender immediately 
> and delete this e-mail.
>
> Information on data protection can be found 
> here .

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

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

[Eelco Chaudron]

On 7 Dec 2023, at 10:31, Eelco Chaudron wrote:

> This patch detects new static analyze issues, and report them.
> It does this by reporting on the delta for this branch, compared
> to the base branch.
>
> For example the error might look like this:
>
>   error level: +0 -0 no changes
>   warning level: +2 +0
> New issue "deadcode.DeadStores Value stored to 'remote' is never read" (1 
> occurrence)
>  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
> New issue "unix.Malloc Potential leak of memory pointed to by 'remote'" 
> (1 occurrence)
>  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
>   note level: +0 -0 no changes
>   all levels: +2 +0
>
> Signed-off-by: Eelco Chaudron 

Please ignore this one, as I forgot to add the version to the subject :( I will 
send a new one with v3 in the subject line.

//Eelco

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

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

[Eelco Chaudron]

This patch detects new static analyze issues, and report them.
It does this by reporting on the delta for this branch, compared
to the base branch.

For example the error might look like this:

  error level: +0 -0 no changes
  warning level: +2 +0
New issue "deadcode.DeadStores Value stored to 'remote' is never read" (1 
occurrence)
  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
New issue "unix.Malloc Potential leak of memory pointed to by 'remote'" (1 
occurrence)
  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
  note level: +0 -0 no changes
  all levels: +2 +0

Signed-off-by: Eelco Chaudron 
---
changes in v2:
  - When it's a new branch, it compares it to the HEAD of the default branch.

changes in v3:
  - Include the clang version as part of the cache
  - Change the way it looks for the 'default' branch so it will work
for patch branches.
  - Also compare to the base branch for forced commits.

 .ci/linux-build.sh   |   29 +
 .github/workflows/build-and-test.yml |  106 ++
 2 files changed, 135 insertions(+)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index aa2ecc505..fedf1398a 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -49,6 +49,30 @@ function build_ovs()
 make -j4
 }
 
+function clang_analyze()
+{
+[ -d "./base-clang-analyzer-results" ] && cache_build=false \
+   || cache_build=true
+if [ "$cache_build" = true ]; then
+# If this is a cache build, proceed to the base branch's directory.
+cd base_ovs_main
+fi;
+
+configure_ovs $OPTS
+make clean
+scan-build -o ./clang-analyzer-results -sarif --use-cc=clang make -j4
+
+if [ "$cache_build" = true ]; then
+# Move results, so it will be picked up by the cache.
+mv ./clang-analyzer-results ../base-clang-analyzer-results
+cd ..
+else
+# Only do the compare on the none cache builds.
+sarif --check note diff ./base-clang-analyzer-results \
+./clang-analyzer-results
+fi;
+}
+
 if [ "$DEB_PACKAGE" ]; then
 ./boot.sh && ./configure --with-dpdk=$DPDK && make debian
 mk-build-deps --install --root-cmd sudo --remove debian/control
@@ -116,6 +140,11 @@ fi
 
 OPTS="${EXTRA_OPTS} ${OPTS} $*"
 
+if [ "$CLANG_ANALYZE" ]; then
+clang_analyze
+exit 0
+fi
+
 if [ "$TESTSUITE" = 'test' ]; then
 # 'distcheck' will reconfigure with required options.
 # Now we only need to prepare the Makefile without sparse-wrapped CC.
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 09654205e..f8a000490 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -223,6 +223,112 @@ jobs:
 name: logs-linux-${{ join(matrix.*, '-') }}
 path: logs.tgz
 
+  build-clang-analyze:
+needs: build-dpdk
+env:
+  dependencies: |
+automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
+libunbound-dev libunwind-dev libssl-dev libtool llvm-dev \
+python3-unbound
+  CC:   clang
+  DPDK: dpdk
+  CLANG_ANALYZE: true
+name: clang-analyze
+runs-on: ubuntu-22.04
+timeout-minutes: 30
+
+steps:
+- name: checkout
+  uses: actions/checkout@v3
+  with:
+fetch-depth: 0
+
+- name: get base branch sha
+  id: base_branch
+  env:
+BASE_SHA: ${{ github.event.pull_request.base.sha }}
+EVENT_BEFORE: ${{ github.event.before }}
+DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
+FORCED_PUSH: ${{ github.event.forced }}
+  run: |
+if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
+  echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
+else
+  if [ "$FORCED_PUSH" = true ] || [ "$EVENT_BEFORE" -eq 0 ]; then
+set sha=$(git describe --all --no-abbrev --always\
+   --match master --match main   \
+   --match branch-[0-9].[0-9]* | \
+  sed 's/.*\///')
+[ -z "$sha" ] && echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT \
+  || echo "sha=$sha" >> $GITHUB_OUTPUT
+  else
+echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
+  fi
+fi
+
+- name: checkout base branch
+  uses: actions/checkout@v3
+  with:
+ref: ${{ steps.base_branch.outputs.sha }}
+path: base_ovs_main
+
+- name: update PATH
+  run: |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+- name: generate cache key
+  id: cache_key
+  run: |
+ver=$(clang -v 2>&1 | grep version | \
+  sed 's/.*version \([0-9]*\.[0-9]*\.[0-9]*\).*/\1/g')
+echo "key=clang-${ver}-analyze-$(git -C base_ovs_main rev-parse HEAD)" 
\
+  >

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

[Eelco Chaudron]

This patch detects new static analyze issues, and report them.
It does this by reporting on the delta for this branch, compared
to the base branch.

For example the error might look like this:

  error level: +0 -0 no changes
  warning level: +2 +0
New issue "deadcode.DeadStores Value stored to 'remote' is never read" (1 
occurrence)
  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
New issue "unix.Malloc Potential leak of memory pointed to by 'remote'" (1 
occurrence)
  file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
  note level: +0 -0 no changes
  all levels: +2 +0

Signed-off-by: Eelco Chaudron 
---

changes in v2:
  - When it's a new branch, it compares it to the HEAD of the default branch.

changes in v3:
  - Include the clang version as part of the cache
  - Change the way it looks for the 'default' branch so it will work
for patch branches.
  - Also compare to the base branch for forced commits.

 .ci/linux-build.sh   |   29 +
 .github/workflows/build-and-test.yml |  106 ++
 2 files changed, 135 insertions(+)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index aa2ecc505..fedf1398a 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -49,6 +49,30 @@ function build_ovs()
 make -j4
 }
 
+function clang_analyze()
+{
+[ -d "./base-clang-analyzer-results" ] && cache_build=false \
+   || cache_build=true
+if [ "$cache_build" = true ]; then
+# If this is a cache build, proceed to the base branch's directory.
+cd base_ovs_main
+fi;
+
+configure_ovs $OPTS
+make clean
+scan-build -o ./clang-analyzer-results -sarif --use-cc=clang make -j4
+
+if [ "$cache_build" = true ]; then
+# Move results, so it will be picked up by the cache.
+mv ./clang-analyzer-results ../base-clang-analyzer-results
+cd ..
+else
+# Only do the compare on the none cache builds.
+sarif --check note diff ./base-clang-analyzer-results \
+./clang-analyzer-results
+fi;
+}
+
 if [ "$DEB_PACKAGE" ]; then
 ./boot.sh && ./configure --with-dpdk=$DPDK && make debian
 mk-build-deps --install --root-cmd sudo --remove debian/control
@@ -116,6 +140,11 @@ fi
 
 OPTS="${EXTRA_OPTS} ${OPTS} $*"
 
+if [ "$CLANG_ANALYZE" ]; then
+clang_analyze
+exit 0
+fi
+
 if [ "$TESTSUITE" = 'test' ]; then
 # 'distcheck' will reconfigure with required options.
 # Now we only need to prepare the Makefile without sparse-wrapped CC.
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 09654205e..f8a000490 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -223,6 +223,112 @@ jobs:
 name: logs-linux-${{ join(matrix.*, '-') }}
 path: logs.tgz
 
+  build-clang-analyze:
+needs: build-dpdk
+env:
+  dependencies: |
+automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
+libunbound-dev libunwind-dev libssl-dev libtool llvm-dev \
+python3-unbound
+  CC:   clang
+  DPDK: dpdk
+  CLANG_ANALYZE: true
+name: clang-analyze
+runs-on: ubuntu-22.04
+timeout-minutes: 30
+
+steps:
+- name: checkout
+  uses: actions/checkout@v3
+  with:
+fetch-depth: 0
+
+- name: get base branch sha
+  id: base_branch
+  env:
+BASE_SHA: ${{ github.event.pull_request.base.sha }}
+EVENT_BEFORE: ${{ github.event.before }}
+DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
+FORCED_PUSH: ${{ github.event.forced }}
+  run: |
+if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
+  echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
+else
+  if [ "$FORCED_PUSH" = true ] || [ "$EVENT_BEFORE" -eq 0 ]; then
+set sha=$(git describe --all --no-abbrev --always\
+   --match master --match main   \
+   --match branch-[0-9].[0-9]* | \
+  sed 's/.*\///')
+[ -z "$sha" ] && echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT \
+  || echo "sha=$sha" >> $GITHUB_OUTPUT
+  else
+echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
+  fi
+fi
+
+- name: checkout base branch
+  uses: actions/checkout@v3
+  with:
+ref: ${{ steps.base_branch.outputs.sha }}
+path: base_ovs_main
+
+- name: update PATH
+  run: |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+- name: generate cache key
+  id: cache_key
+  run: |
+ver=$(clang -v 2>&1 | grep version | \
+  sed 's/.*version \([0-9]*\.[0-9]*\.[0-9]*\).*/\1/g')
+echo "key=clang-${ver}-analyze-$(git -C base_ovs_main rev-parse HEAD)" 
\
+