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

2023-06-14 Thread Ales Musil
On Wed, Jun 14, 2023 at 3:37 PM Dumitru Ceara  wrote:

> On 6/13/23 16:02, Ales Musil wrote:
> > Add missing OVN_FOR_EACH_NORTHD around
> > FDB aging test.
> >
> > Signed-off-by: Ales Musil 
> > ---
>
> Hi Ales,
>
> This misses the "Fixes" tag:
> Fixes: ae9a5488824c ("northd: Add FDB aging mechanism")
>
> I added it and pushed the patch to the main branch.
>
> Regards,
> Dumitru
>
>
Thanks!

Regards,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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


Re: [ovs-dev] [ovs-dev v3] dpif-netdev: fix dpif_netdev_flow_put

2023-06-14 Thread 0-day Robot
Bleep bloop.  Greetings Peng He, I am a robot and I have tried out your patch.
Thanks for your contribution.

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


checkpatch:
ERROR: Author Peng He  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Peng He 
Lines checked: 201, Warnings: 1, Errors: 1


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] [ovs-dev v3] dpif-netdev: fix dpif_netdev_flow_put

2023-06-14 Thread Peng He
OVS allows overlapping megaflows, as long as the actions of these
megaflows are equal. However, the current implementation of action
modification relies on flow_lookup instead of ufid, this could result
in looking up a wrong megaflow and make the ukeys and megaflows inconsistent

Just like the test case in the patch, at first we have a rule with the
prefix:

10.1.2.0/24

and we will get a megaflow with prefixes 10.1.2.2/24 when a packet with IP
10.1.2.2 is received.

Then suppose we change the rule into 10.1.0.0/16. OVS prefers to keep the
10.1.2.2/24 megaflow and just changes its action instead of extending
the prefix into 10.1.2.2/16.

then suppose we have a 10.1.0.2 packet, since it misses the megaflow,
this time, we will have an overlapping megaflow with the right prefix:
10.1.0.2/16

now we have two megaflows:
10.1.2.2/24
10.1.0.2/16

last, suppose we have changed the ruleset again. The revalidator this
time still decides to change the actions of both megaflows instead of
deleting them.

The dpif_netdev_flow_put will search the megaflow to modify with unmasked
keys, however it might lookup the wrong megaflow as the key 10.1.2.2 matches
both 10.1.2.2/24 and 10.1.0.2/16!

This patch changes the megaflow lookup code in modification path into
relying the ufid to find the correct megaflow instead of key lookup.

Signed-off-by: Peng He 
---
 lib/dpif-netdev.c | 62 ++-
 tests/pmd.at  | 48 
 2 files changed, 82 insertions(+), 28 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 70b953ae6..b90ed1542 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4198,36 +4198,43 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
 memset(stats, 0, sizeof *stats);
 }
 
-ovs_mutex_lock(>flow_mutex);
-netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
-if (!netdev_flow) {
-if (put->flags & DPIF_FP_CREATE) {
+if (put->flags & DPIF_FP_CREATE) {
+ovs_mutex_lock(>flow_mutex);
+netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
+if (!netdev_flow) {
 dp_netdev_flow_add(pmd, match, ufid, put->actions,
put->actions_len, ODPP_NONE);
 } else {
-error = ENOENT;
+error = EEXIST;
 }
+ovs_mutex_unlock(>flow_mutex);
 } else {
+netdev_flow = dp_netdev_pmd_find_flow(pmd, ufid,
+  put->key, put->key_len);
+
 if (put->flags & DPIF_FP_MODIFY) {
-struct dp_netdev_actions *new_actions;
-struct dp_netdev_actions *old_actions;
+if (!netdev_flow) {
+error = ENOENT;
+} else {
+struct dp_netdev_actions *new_actions;
+struct dp_netdev_actions *old_actions;
 
-new_actions = dp_netdev_actions_create(put->actions,
-   put->actions_len);
+new_actions = dp_netdev_actions_create(put->actions,
+   put->actions_len);
 
-old_actions = dp_netdev_flow_get_actions(netdev_flow);
-ovsrcu_set(_flow->actions, new_actions);
+old_actions = dp_netdev_flow_get_actions(netdev_flow);
+ovsrcu_set(_flow->actions, new_actions);
 
-queue_netdev_flow_put(pmd, netdev_flow, match,
-  put->actions, put->actions_len,
-  DP_NETDEV_FLOW_OFFLOAD_OP_MOD);
-log_netdev_flow_change(netdev_flow, match, old_actions,
-   put->actions, put->actions_len);
+queue_netdev_flow_put(pmd, netdev_flow, match,
+  put->actions, put->actions_len,
+  DP_NETDEV_FLOW_OFFLOAD_OP_MOD);
+log_netdev_flow_change(netdev_flow, match, old_actions,
+   put->actions, put->actions_len);
 
-if (stats) {
-get_dpif_flow_status(pmd->dp, netdev_flow, stats, NULL);
-}
-if (put->flags & DPIF_FP_ZERO_STATS) {
+if (stats) {
+get_dpif_flow_status(pmd->dp, netdev_flow, stats, NULL);
+}
+if (put->flags & DPIF_FP_ZERO_STATS) {
 /* XXX: The userspace datapath uses thread local statistics
  * (for flows), which should be updated only by the owning
  * thread.  Since we cannot write on stats memory here,
@@ -4237,18 +4244,17 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
  * - Should the need arise, this operation can be implemented
  *   by keeping a base value (to be update here) for each
  *   counter, and subtracting it before outputting the stats 

Re: [ovs-dev] [ovs-dev v2] dpif-netdev: fix dpif_netdev_flow_put

2023-06-14 Thread Peng He
Simon Horman  于2023年6月14日周三 20:37写道:

> On Tue, Jun 13, 2023 at 09:41:42AM +0800, Peng He wrote:
> > Hi, Simon,
> >
> > I guess it's not related, as my patch fixes the code in dpif-netdev.c it
> > should has any impact on the ovsdb side.
> > But I am not sure that.
> > Is this test case a spike test case? Is there any testsuite log?
>
> * Please don't top post
>
> * I am unsure what a spike test is
>
> * The link I provided leads to logs
>
>   [1]
> https://github.com/ovsrobot/ovs/actions/runs/5238809864/jobs/9458063244#step:11:5573
>   [2]
> https://github.com/ovsrobot/ovs/suites/13526199944/artifacts/743672747
>
> * It seems that my report was inaccurate, sorry about that.
>
>   As pointed out by Ilya,
>   it was not the "compacting online - cluster that failed",
>   but rather the test that is added by your patch
>
>   [3] https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405546.html


Thanks for the information!
I check out the log, it seems one megaflow has been missing.
The reason might be one megaflow has been aged and removed. I will send a
v3 to improve that.

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


Re: [ovs-dev] [PATCH v6 ovn] northd: centralized reply lb traffic even if FIP is defined

2023-06-14 Thread Dumitru Ceara
On 6/14/23 15:17, Lorenzo Bianconi wrote:
> In the current codebase for distributed gw router port use-case,
> it is not possible to add a load balancer that redirects the traffic
> to a back-end if it is used as internal IP of a FIP NAT rule since
> the reply traffic is never centralized. Fix the issue centralizing the
> traffic if it is the reply packet for the load balancer.
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2023609
> Acked-by: Mark Michelson 
> Reviewed-by: Simon Horman 
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v5:
> - rely on fmt_pkt macro in unit-tests
> Changes since v4:
> - improve memory footprint
> Changes since v3:
> - add ovn unit-test and get rid of system-ovn unit-test.
> - minor changes in ovn-northd unit-test.
> Changes since v2:
> - rebase on top of ovn main branch
> - fix spelling
> Changes since v1:
> - add new system-test and unit-test
> ---

Thanks, I made the following minor change and then pushed this patch to
the main branch.  I didn't backport it because I'm not completely sure
it's a bug fix.  We can always re-evaluate that and backport it if
needed.

Regards,
Dumitru

diff --git a/tests/ovn.at b/tests/ovn.at
index 2e96adbcc1..544fba1876 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -36344,6 +36344,7 @@ AT_CLEANUP
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([DNAT_SNAT and LB traffic])
 AT_KEYWORDS([dnat-snat-lb])
+AT_SKIP_IF([test $HAVE_SCAPY = no])
 ovn_start

 test_ip_req_packet() {
---

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


Re: [ovs-dev] [PATCH ovn v3] ovn-northd.at: Fix unstable LSP incremental processing test.

2023-06-14 Thread Dumitru Ceara
On 6/14/23 22:37, Han Zhou wrote:
> On Wed, Jun 14, 2023 at 4:16 AM Dumitru Ceara  wrote:
>>
>> On 6/10/23 20:10, Han Zhou wrote:
>>> The test case is unstable because there are many factors that can impact
>>> the number of recomputes. For example, when northd updates both NB and
>>> SB, both sides have in-flight transactions, and if the SB notification
>>> comes back before the NB transaction (for updating nb_cfg) completes,
>>> ovn-northd falls back to recompute because idl_txn for NB is not
>>> available, while if the NB notification comes back first it won't
>>> trigger recompute because the nb_cfg columns are omitted for alert.
>>>
>>> To avoid the instability of the test case, this patch modifies it to
>>> verify the best case (also the most common scenario), by running the
>>> test multiple (10) times and making sure at least 70% of the time all
>>> the recompute counters match expectation.
>>>
>>> (The recompute triggered by in-flight transactions will be improved
>>> later and the success rate in the test can be adjusted accordingly.)
>>>
>>> Reported-by: Dumitru Ceara 
>>> Signed-off-by: Han Zhou 
>>> ---
>>> v1 -> v2: wait ports up and sync after each port binding, to avoid race.
>>> v2 -> v3: refactor and test case and use a probability based test
> strategy
>>> to ensure the test stability (see commit message).
>>
>> Neat, thanks for fixing this!  The only thing with this approach is that
>> the test case takes quite long to execute 30s on my system.  Luckily we
>> don't have too many tests like this so:
>>
>> Acked-by: Dumitru Ceara 
>>
> Thanks Dumitru. That sounds really slow. It is faster on my computer

It was with address and undefined behavior sanitizers enabled and with
optimizations completely disabled (for both OVS and OVN).  With -O2 and
no sanitizers it takes 16s.

> (<10s), not too bad. If we really want it to be faster we can reduce the
> number of runs. I applied it to main for now (I did see several CI failures
> because of this test).
> 

Great, thanks!

> Regards,
> Han
> 
>> Regards,
>> Dumitru
>>
> 

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


[ovs-dev] [PATCH v3] python: Add async DNS support

2023-06-14 Thread Terry Wilson
This adds a Python version of the async DNS support added in:

771680d96 DNS: Add basic support for asynchronous DNS resolving

The above version uses the unbound C library, and this
implimentation uses the SWIG-wrapped Python version of that.

In the event that the Python unbound library is not available,
a warning will be logged and the resolve() method will just
return None. For the case where inet_parse_active() is passed
an IP address, it will not try to resolve it, so existing
behavior should be preserved in the case that the unbound
library is unavailable.

Intentional differences from the C version are as follows:

  OVS_HOSTS_FILE environment variable can bet set to override
  the system 'hosts' file. This is primarily to allow testing to
  be done without requiring network connectivity.

  Since resolution can still be done via hosts file lookup, DNS
  lookups are not disabled when resolv.conf cannot be loaded.

  The Python socket_util module has fallen behind its C equivalent.
  The bare minimum change was done to inet_parse_active() to support
  sync/async dns, as there is no equivalent to
  parse_sockaddr_components(), inet_parse_passive(), etc. A TODO
  was added to bring socket_util.py up to equivalency to the C
  version.

Signed-off-by: Terry Wilson 
---
 .github/workflows/build-and-test.yml|   4 +-
 Documentation/intro/install/general.rst |   4 +-
 Documentation/intro/install/rhel.rst|   2 +-
 Documentation/intro/install/windows.rst |   2 +-
 NEWS|   4 +-
 debian/control.in   |   1 +
 m4/openvswitch.m4   |   8 +-
 python/TODO.rst |   7 +
 python/automake.mk  |   2 +
 python/ovs/dns_resolve.py   | 272 +++
 python/ovs/socket_util.py   |  21 +-
 python/ovs/stream.py|   2 +-
 python/ovs/tests/test_dns_resolve.py| 280 
 python/setup.py |   6 +-
 rhel/openvswitch-fedora.spec.in |   2 +-
 tests/vlog.at   |   2 +
 16 files changed, 601 insertions(+), 18 deletions(-)
 create mode 100644 python/ovs/dns_resolve.py
 create mode 100644 python/ovs/tests/test_dns_resolve.py

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index f66ab43b0..47d239f10 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -183,10 +183,10 @@ jobs:
   run:  sudo apt update || true
 - name: install common dependencies
   run:  sudo apt install -y ${{ env.dependencies }}
-- name: install libunbound libunwind
+- name: install libunbound libunwind python3-unbound
   # GitHub Actions doesn't have 32-bit versions of these libraries.
   if:   matrix.m32 == ''
-  run:  sudo apt install -y libunbound-dev libunwind-dev
+  run:  sudo apt install -y libunbound-dev libunwind-dev python3-unbound
 - name: install 32-bit libraries
   if:   matrix.m32 != ''
   run:  sudo apt install -y gcc-multilib
diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 42b5682fd..19e360d47 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -90,7 +90,7 @@ need the following software:
   If libcap-ng is installed, then Open vSwitch will automatically build with
   support for it.
 
-- Python 3.4 or later.
+- Python 3.6 or later.
 
 - Unbound library, from http://www.unbound.net, is optional but recommended if
   you want to enable ovs-vswitchd and other utilities to use DNS names when
@@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the 
following software:
   from iproute2 (part of all major distributions and available at
   https://wiki.linuxfoundation.org/networking/iproute2).
 
-- Python 3.4 or later.
+- Python 3.6 or later.
 
 On Linux you should ensure that ``/dev/urandom`` exists. To support TAP
 devices, you must also ensure that ``/dev/net/tun`` exists.
diff --git a/Documentation/intro/install/rhel.rst 
b/Documentation/intro/install/rhel.rst
index d1fc42021..f2151d890 100644
--- a/Documentation/intro/install/rhel.rst
+++ b/Documentation/intro/install/rhel.rst
@@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``.
 If python3-sphinx package is not available in your version of RHEL, you can
 install it via pip with 'pip install sphinx'.
 
-Open vSwitch requires python 3.4 or newer which is not available in older
+Open vSwitch requires python 3.6 or newer which is not available in older
 distributions. In the case of RHEL 6.x and its derivatives, one option is
 to install python34 from `EPEL`_.
 
diff --git a/Documentation/intro/install/windows.rst 
b/Documentation/intro/install/windows.rst
index 78f60f35a..fce099d5d 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ 

Re: [ovs-dev] [PATCH ovn v3] ovn-northd.at: Fix unstable LSP incremental processing test.

2023-06-14 Thread Han Zhou
On Wed, Jun 14, 2023 at 4:16 AM Dumitru Ceara  wrote:
>
> On 6/10/23 20:10, Han Zhou wrote:
> > The test case is unstable because there are many factors that can impact
> > the number of recomputes. For example, when northd updates both NB and
> > SB, both sides have in-flight transactions, and if the SB notification
> > comes back before the NB transaction (for updating nb_cfg) completes,
> > ovn-northd falls back to recompute because idl_txn for NB is not
> > available, while if the NB notification comes back first it won't
> > trigger recompute because the nb_cfg columns are omitted for alert.
> >
> > To avoid the instability of the test case, this patch modifies it to
> > verify the best case (also the most common scenario), by running the
> > test multiple (10) times and making sure at least 70% of the time all
> > the recompute counters match expectation.
> >
> > (The recompute triggered by in-flight transactions will be improved
> > later and the success rate in the test can be adjusted accordingly.)
> >
> > Reported-by: Dumitru Ceara 
> > Signed-off-by: Han Zhou 
> > ---
> > v1 -> v2: wait ports up and sync after each port binding, to avoid race.
> > v2 -> v3: refactor and test case and use a probability based test
strategy
> > to ensure the test stability (see commit message).
>
> Neat, thanks for fixing this!  The only thing with this approach is that
> the test case takes quite long to execute 30s on my system.  Luckily we
> don't have too many tests like this so:
>
> Acked-by: Dumitru Ceara 
>
Thanks Dumitru. That sounds really slow. It is faster on my computer
(<10s), not too bad. If we really want it to be faster we can reduce the
number of runs. I applied it to main for now (I did see several CI failures
because of this test).

Regards,
Han

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


Re: [ovs-dev] [PATCH v3] conntrack: Extract l4 information for SCTP.

2023-06-14 Thread Ilya Maximets
On 6/14/23 21:08, Ilya Maximets wrote:
> On 6/14/23 20:11, Paolo Valerio wrote:
>> Ilya Maximets  writes:
>>
>>> On 6/12/23 16:57, Aaron Conole wrote:
 Paolo Valerio  writes:

> since a27d70a89 ("conntrack: add generic IP protocol support") all
> the unrecognized IP protocols get handled using ct_proto_other ops
> and are managed as L3 using 3 tuples.
>
> This patch stores L4 information for SCTP in the conn_key so that
> multiple conn instances, instead of one with ports zeroed, will be
> created when there are multiple SCTP connections between two hosts.
> It also performs crc32c check when not offloaded, and adds SCTP to
> pat_enabled.
>
> With this patch, given two SCTP association between two hosts,
> tracking the connection will result in:
>
> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=55884,dport=5201),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5201,dport=12345),zone=1
> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=59874,dport=5202),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5202,dport=12346),zone=1
>
> instead of:
>
> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=0,dport=0),reply=(src=10.1.1.1,dst=10.1.1.2,sport=0,dport=0),zone=1
>
> Signed-off-by: Paolo Valerio 
> ---

 Thanks for this work - I think it looks good.

 Perhaps it should have a NEWS item mentioned that the userspace
 conntrack now supports matching SCTP l4 data.

 If you do spin a v4 with that change, you can keep my:

 Acked-by: Aaron Conole 
>>>
>>> Hi, Paolo and Aaron.
>>>
>>> I'm getting a consistent test failure while running check-kernel
>>> on Ubuntu 22.10 with 5.19 kernel:
>>>
>>>
>>> ./system-traffic.at:4754: cat ofctl_monitor.log
>>> --- -   2023-06-14 11:26:41.958591125 +
>>> +++ /root/ovs/tests/system-kmod-testsuite.dir/at-groups/105/stdout  
>>> 2023-06-14 11:26:41.95200 +
>>> @@ -12,8 +12,6 @@
>>>  
>>> sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>>>  sctp_csum:9b67e853
>>>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=54 in_port=1 (via action) 
>>> data_len=54 (unbuffered)
>>>  
>>> sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>>>  sctp_csum:bc0e5463
>>> -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=50 
>>> ct_state=est|rpl|trk|dnat,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=132,ct_tp_src=54969,ct_tp_dst=12345,ip,in_port=2
>>>  (via action) data_len=50 (unbuffered)
>>> -sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>>>  sctp_csum:d6ce6b9e
>>>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=50 in_port=1 (via action) 
>>> data_len=50 (unbuffered)
>>> -sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>>>  sctp_csum:add7db93
>>> +sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=54969,tp_dst=12345
>>>  sctp_csum:5db68ce
>>>
>>>
>>> Do you know what can be a problem here?
>>>
>>> Test is passing on Fedora 38 with 6.3 kernel and on rhel 9.2.
>>>
>>
>> Hi Ilya,
>>
>> Uhm, it seems there's a problem with the shutdown sequence.
>> I just ran the on a VM:
>>
>> vagrant@ubuntu2210:~/ovs$ grep CONFIG_NF_CT_PROTO_SCTP 
>> /boot/config-5.19.0-38-generic 
>> CONFIG_NF_CT_PROTO_SCTP=y
>>
>> vagrant@ubuntu2210:~/ovs$ grep VERSION /etc/os-release 
>> VERSION_ID="22.10"
>> VERSION="22.10 (Kinetic Kudu)"
>> VERSION_CODENAME=kinetic
>>
>> vagrant@ubuntu2210:~/ovs$ uname -r
>> 5.19.0-38-generic
> 
> The only difference with my VM is that I have -43-generic kernel.
> 
>>
>> but I can't see the failure.
>> Any chance to see if they are marked for some reason as invalid?
> 
> I dumped conntrack after every packet and here is what I see:
> 
> On RHEL9, where test is working:
> 
> 1. sctp 132 9 CLOSEDsrc=10.1.1.1 dst=10.1.1.2 sport=54969 
> dport=12345 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 
> mark=0 zone=1 use=1
> 2. sctp 132 2 COOKIE_WAIT   src=10.1.1.1 dst=10.1.1.2 sport=54969 
> dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 mark=0 zone=1 
> use=1
> 3. sctp 132 2 COOKIE_ECHOED src=10.1.1.1 dst=10.1.1.2 sport=54969 
> dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 mark=0 zone=1 
> use=1
> 4. sctp 132 431999 ESTABLISHED  src=10.1.1.1 dst=10.1.1.2 sport=54969 
> dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 [ASSURED] 
> mark=0 zone=1 use=1
> 5. sctp 132 431999 ESTABLISHED  src=10.1.1.1 

Re: [ovs-dev] [PATCH v3] rhel: make the version, displayed to the user, customizable

2023-06-14 Thread Ilya Maximets
On 5/26/23 16:41, Timothy Redaelli wrote:
> Since on CentOS/RHEL the builds are based on stable branches and not on
> tags for debugging purpose it's better to have the downstream version as
> version so it's easier to know which commits are included in a build.
> 
> This commit adds --with-version-suffix as ./configure option in
> order to set an OVS version suffix that should be shown to the user via
> ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
> utilities.
> 
> --with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to have
> the version be aligned with the downstream one.
> 
> Signed-off-by: Timothy Redaelli 
> ---
>  Makefile.am  |  1 +
>  acinclude.m4 | 13 +
>  configure.ac |  1 +
>  include/openvswitch/version.h.in |  2 +-
>  lib/ovsdb-error.c|  2 +-
>  lib/util.c   |  5 +++--
>  ovsdb/ovsdb-server.c |  3 ++-
>  python/automake.mk   |  2 +-
>  rhel/openvswitch-fedora.spec.in  |  1 +
>  utilities/ovs-dpctl-top.in   |  2 +-
>  utilities/ovs-lib.in |  2 +-
>  utilities/ovs-parse-backtrace.in |  2 +-
>  utilities/ovs-pcap.in|  2 +-
>  utilities/ovs-pki.in |  2 +-
>  utilities/ovs-tcpdump.in |  4 ++--
>  utilities/ovs-tcpundump.in   |  2 +-
>  utilities/ovs-vlan-test.in   |  2 +-
>  vswitchd/bridge.c|  3 ++-
>  18 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e605187b8..2f4ce9f94 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -157,6 +157,7 @@ SUFFIXES += .in
>   -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
>   -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>   -e 's,[@]VERSION[@],$(VERSION),g' \
> + -e 's,[@]VERSION_SUFFIX[@],$(VERSION_SUFFIX),g' \
>   -e 's,[@]localstatedir[@],$(localstatedir),g' \
>   -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>   -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> diff --git a/acinclude.m4 b/acinclude.m4
> index ac1eab790..a02f5bb00 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -470,6 +470,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>  ])
>  
> +dnl Append a version suffix
> +
> +AC_DEFUN([OVS_CHECK_VERSION_SUFFIX], [
> +  AC_ARG_WITH([version-suffix],
> +  [AS_HELP_STRING([--with-version-suffix=ver_suffix],
> +  [Specify a version suffix that will be appended
> +   to OVS version])])
> +  AC_DEFINE_UNQUOTED([VERSION_SUFFIX], ["$with_version_suffix"],
> + [Package version suffix])
> +  AC_SUBST([VERSION_SUFFIX], [$with_version_suffix])
> +  ])
> +])
> +
>  dnl Checks for net/if_dl.h.
>  dnl
>  dnl (We use this as a proxy for checking whether we're building on FreeBSD
> diff --git a/configure.ac b/configure.ac
> index d05e544b5..3df7156da 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -198,6 +198,7 @@ OVS_CHECK_LINUX_SCTP_CT
>  OVS_CHECK_LINUX_VIRTIO_TYPES
>  OVS_CHECK_DPDK
>  OVS_CHECK_PRAGMA_MESSAGE
> +OVS_CHECK_VERSION_SUFFIX
>  AC_SUBST([CFLAGS])
>  AC_SUBST([OVS_CFLAGS])
>  AC_SUBST([OVS_LDFLAGS])
> diff --git a/include/openvswitch/version.h.in 
> b/include/openvswitch/version.h.in
> index 23d8fde4f..231f61e30 100644
> --- a/include/openvswitch/version.h.in
> +++ b/include/openvswitch/version.h.in
> @@ -19,7 +19,7 @@
>  #define OPENVSWITCH_VERSION_H 1
>  
>  #define OVS_PACKAGE_STRING  "@PACKAGE_STRING@"
> -#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@"
> +#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@@VERSION_SUFFIX@"
>  
>  #define OVS_LIB_VERSION @LT_CURRENT@
>  #define OVS_LIB_REVISION@LT_REVISION@
> diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
> index a75ad36b7..65bbfe876 100644
> --- a/lib/ovsdb-error.c
> +++ b/lib/ovsdb-error.c
> @@ -150,7 +150,7 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
>  ds_put_char(, ')');
>  }
>  
> -ds_put_format(, " (%s %s)", program_name, VERSION);
> +ds_put_format(, " (%s %s)", program_name, VERSION VERSION_SUFFIX);
>  
>  if (inner_error) {
>  char *s = ovsdb_error_to_string_free(inner_error);
> diff --git a/lib/util.c b/lib/util.c
> index 96a71550d..f9e9c5b4c 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -617,8 +617,9 @@ ovs_set_program_name(const char *argv0, const char 
> *version)
>  program_name = basename;
>  
>  free(program_version);
> -if (!strcmp(version, VERSION)) {
> -program_version = xasprintf("%s (Open vSwitch) "VERSION"\n",
> +if (!strcmp(version, VERSION VERSION_SUFFIX)) {
> +program_version = xasprintf("%s (Open vSwitch) "VERSION
> +VERSION_SUFFIX"\n",
>  program_name);
>  } else {
>  program_version = xasprintf("%s %s\n"
> diff --git 

Re: [ovs-dev] [PATCH v3] conntrack: Extract l4 information for SCTP.

2023-06-14 Thread Ilya Maximets
On 6/14/23 20:11, Paolo Valerio wrote:
> Ilya Maximets  writes:
> 
>> On 6/12/23 16:57, Aaron Conole wrote:
>>> Paolo Valerio  writes:
>>>
 since a27d70a89 ("conntrack: add generic IP protocol support") all
 the unrecognized IP protocols get handled using ct_proto_other ops
 and are managed as L3 using 3 tuples.

 This patch stores L4 information for SCTP in the conn_key so that
 multiple conn instances, instead of one with ports zeroed, will be
 created when there are multiple SCTP connections between two hosts.
 It also performs crc32c check when not offloaded, and adds SCTP to
 pat_enabled.

 With this patch, given two SCTP association between two hosts,
 tracking the connection will result in:

 sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=55884,dport=5201),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5201,dport=12345),zone=1
 sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=59874,dport=5202),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5202,dport=12346),zone=1

 instead of:

 sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=0,dport=0),reply=(src=10.1.1.1,dst=10.1.1.2,sport=0,dport=0),zone=1

 Signed-off-by: Paolo Valerio 
 ---
>>>
>>> Thanks for this work - I think it looks good.
>>>
>>> Perhaps it should have a NEWS item mentioned that the userspace
>>> conntrack now supports matching SCTP l4 data.
>>>
>>> If you do spin a v4 with that change, you can keep my:
>>>
>>> Acked-by: Aaron Conole 
>>
>> Hi, Paolo and Aaron.
>>
>> I'm getting a consistent test failure while running check-kernel
>> on Ubuntu 22.10 with 5.19 kernel:
>>
>>
>> ./system-traffic.at:4754: cat ofctl_monitor.log
>> --- -   2023-06-14 11:26:41.958591125 +
>> +++ /root/ovs/tests/system-kmod-testsuite.dir/at-groups/105/stdout  
>> 2023-06-14 11:26:41.95200 +
>> @@ -12,8 +12,6 @@
>>  
>> sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>>  sctp_csum:9b67e853
>>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=54 in_port=1 (via action) 
>> data_len=54 (unbuffered)
>>  
>> sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>>  sctp_csum:bc0e5463
>> -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=50 
>> ct_state=est|rpl|trk|dnat,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=132,ct_tp_src=54969,ct_tp_dst=12345,ip,in_port=2
>>  (via action) data_len=50 (unbuffered)
>> -sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>>  sctp_csum:d6ce6b9e
>>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=50 in_port=1 (via action) 
>> data_len=50 (unbuffered)
>> -sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>>  sctp_csum:add7db93
>> +sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=54969,tp_dst=12345
>>  sctp_csum:5db68ce
>>
>>
>> Do you know what can be a problem here?
>>
>> Test is passing on Fedora 38 with 6.3 kernel and on rhel 9.2.
>>
> 
> Hi Ilya,
> 
> Uhm, it seems there's a problem with the shutdown sequence.
> I just ran the on a VM:
> 
> vagrant@ubuntu2210:~/ovs$ grep CONFIG_NF_CT_PROTO_SCTP 
> /boot/config-5.19.0-38-generic 
> CONFIG_NF_CT_PROTO_SCTP=y
> 
> vagrant@ubuntu2210:~/ovs$ grep VERSION /etc/os-release 
> VERSION_ID="22.10"
> VERSION="22.10 (Kinetic Kudu)"
> VERSION_CODENAME=kinetic
> 
> vagrant@ubuntu2210:~/ovs$ uname -r
> 5.19.0-38-generic

The only difference with my VM is that I have -43-generic kernel.

> 
> but I can't see the failure.
> Any chance to see if they are marked for some reason as invalid?

I dumped conntrack after every packet and here is what I see:

On RHEL9, where test is working:

1. sctp 132 9 CLOSEDsrc=10.1.1.1 dst=10.1.1.2 sport=54969 
dport=12345 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 
mark=0 zone=1 use=1
2. sctp 132 2 COOKIE_WAIT   src=10.1.1.1 dst=10.1.1.2 sport=54969 
dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 mark=0 zone=1 
use=1
3. sctp 132 2 COOKIE_ECHOED src=10.1.1.1 dst=10.1.1.2 sport=54969 
dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 mark=0 zone=1 
use=1
4. sctp 132 431999 ESTABLISHED  src=10.1.1.1 dst=10.1.1.2 sport=54969 
dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 [ASSURED] 
mark=0 zone=1 use=1
5. sctp 132 431999 ESTABLISHED  src=10.1.1.1 dst=10.1.1.2 sport=54969 
dport=12345 src=10.1.1.2 dst=10.1.1.240 sport=12345 dport=34567 [ASSURED] 
mark=0 zone=1 use=1
6. sctp 132 431999 ESTABLISHED  

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

2023-06-14 Thread Mike Pattrick
The netdev receiving packets is supposed to provide the flags
indicating if the L4 checksum was verified and it is OK or BAD,
otherwise the stack will check when appropriate by software.

If the packet comes with good checksum, then postpone the
checksum calculation to the egress device if needed.

When encapsulate a packet with that flag, set the checksum
of the inner L4 header since that is not yet supported.

Calculate the L4 checksum when the packet is going to be sent
over a device that doesn't support the feature.

Linux tap devices allows enabling L3 and L4 offload, so this
patch enables the feature. However, Linux socket interface
remains disabled because the API doesn't allow enabling
those two features without enabling TSO too.

Signed-off-by: Flavio Leitner 
Co-authored-by: Flavio Leitner 
Signed-off-by: Mike Pattrick 
---
 Since v9:
  - Extended miniflow_extract changes into avx512 code
  - Formatting changes
  - Note that we cannot currently enable checksum offloading in
CONFIGURE_VETH_OFFLOADS for check-system-userspace as
netdev-linux.c currently only parses the vnet header if TSO
is enabled.
 Since v10:
  - No change
 Since v11:
  - Added AVX512 IPv6 checksum offload support.
  - Improved error messages and logging.
 Since v12:
  - Added missing mutex annotations
 Since v13:
  - Added TUNGETFEATURES check in netdev-linux
 Since v14:
  - Only check TUNGETFEATURES once
  - Respect FLOW_TNL_F_CSUM flag
---
 lib/conntrack.c  |  15 +-
 lib/dp-packet.c  |  25 +++
 lib/dp-packet.h  |  78 +-
 lib/dpif-netdev-extract-avx512.c |  62 +++-
 lib/flow.c   |  23 +++
 lib/netdev-dpdk.c| 172 +++--
 lib/netdev-linux.c   | 258 ++-
 lib/netdev-native-tnl.c  |  32 +---
 lib/netdev.c |  46 ++
 lib/odp-execute-avx512.c |  88 +++
 lib/packets.c| 175 -
 lib/packets.h|   3 +
 12 files changed, 710 insertions(+), 267 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 78c3e578c..f5ebfa05b 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2060,13 +2060,12 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
*pkt, ovs_be16 dl_type,
 }
 
 if (ok) {
-bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt);
-if (!hwol_bad_l4_csum) {
-bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt)
- || dp_packet_hwol_tx_l4_checksum(pkt);
+if (!dp_packet_l4_checksum_bad(pkt)) {
 /* Validate the checksum only when hwol is not supported. */
 if (extract_l4(>key, l4, dp_packet_l4_size(pkt),
-   >icmp_related, l3, !hwol_good_l4_csum,
+   >icmp_related, l3,
+   !dp_packet_l4_checksum_good(pkt) &&
+   !dp_packet_hwol_tx_l4_checksum(pkt),
NULL)) {
 ctx->hash = conn_key_hash(>key, ct->hash_basis);
 return true;
@@ -3395,8 +3394,10 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
conn_lookup_ctx *ctx,
 adj_seqnum(>tcp_seq, ec->seq_skew);
 }
 
-th->tcp_csum = 0;
-if (!dp_packet_hwol_tx_l4_checksum(pkt)) {
+if (dp_packet_hwol_tx_l4_checksum(pkt)) {
+dp_packet_ol_reset_l4_csum_good(pkt);
+} else {
+th->tcp_csum = 0;
 if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
 th->tcp_csum = packet_csum_upperlayer6(nh6, th, ctx->key.nw_proto,
dp_packet_l4_size(pkt));
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 61c36de44..dfedd0e9b 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -38,6 +38,9 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum 
dp_packet_source so
 dp_packet_init_specific(b);
 /* By default assume the packet type to be Ethernet. */
 b->packet_type = htonl(PT_ETH);
+/* Reset csum start and offset. */
+b->csum_start = 0;
+b->csum_offset = 0;
 }
 
 static void
@@ -544,4 +547,26 @@ dp_packet_ol_send_prepare(struct dp_packet *p, uint64_t 
flags)
 dp_packet_ol_set_ip_csum_good(p);
 dp_packet_hwol_reset_tx_ip_csum(p);
 }
+
+if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) {
+dp_packet_hwol_reset_tx_l4_csum(p);
+return;
+}
+
+if (dp_packet_hwol_l4_is_tcp(p)
+&& !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) {
+packet_tcp_complete_csum(p);
+dp_packet_ol_set_l4_csum_good(p);
+dp_packet_hwol_reset_tx_l4_csum(p);
+} else if (dp_packet_hwol_l4_is_udp(p)
+   && !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) {
+packet_udp_complete_csum(p);
+dp_packet_ol_set_l4_csum_good(p);
+dp_packet_hwol_reset_tx_l4_csum(p);
+} else if 

[ovs-dev] [PATCH v15 1/4] Documentation: Document netdev offload.

2023-06-14 Thread Mike Pattrick
Document the implementation of netdev hardware offloading
in userspace datapath.

Signed-off-by: Flavio Leitner 
Co-authored-by: Flavio Leitner 
Signed-off-by: Mike Pattrick 
---
 Since v9:
  - Renamed documentation to reflect the userspace checksum nature of
this feature
  - Edited for formatting and clarity issues.
 Since v10:
  - No change
 Since v11:
  - Modified document in accordance with Ilya's feedback
 Since v13:
  - Rewrote design section.
 Since v14:
  - No change
---
 Documentation/automake.mk |  1 +
 Documentation/topics/index.rst|  1 +
 .../topics/userspace-checksum-offloading.rst  | 96 +++
 3 files changed, 98 insertions(+)
 create mode 100644 Documentation/topics/userspace-checksum-offloading.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index cdf3c9926..8bd3dbb2b 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -57,6 +57,7 @@ DOC_SOURCE = \
Documentation/topics/record-replay.rst \
Documentation/topics/tracing.rst \
Documentation/topics/usdt-probes.rst \
+   Documentation/topics/userspace-checksum-offloading.rst \
Documentation/topics/userspace-tso.rst \
Documentation/topics/userspace-tx-steering.rst \
Documentation/topics/windows.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 90d4c66e6..f239fcf83 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -55,5 +55,6 @@ OVS
userspace-tso
idl-compound-indexes
ovs-extensions
+   userspace-checksum-offloading
userspace-tx-steering
usdt-probes
diff --git a/Documentation/topics/userspace-checksum-offloading.rst 
b/Documentation/topics/userspace-checksum-offloading.rst
new file mode 100644
index 0..036d3965f
--- /dev/null
+++ b/Documentation/topics/userspace-checksum-offloading.rst
@@ -0,0 +1,96 @@
+..
+  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.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+
+Userspace Datapath - Checksum Offloading
+
+
+This document explains the internals of Open vSwitch support for checksum
+offloading in the userspace datapath.
+
+Design
+--
+
+Open vSwitch strives to forward packets as they arrive regardless of whether
+the checksum is correct or not. OVS is not responsible for fixing external
+checksum issues.
+
+The interface (internally referred to as a netdev) can set flags indicating
+whether each packet's checksum is good or bad upon receipt. If this flag is not
+set, OVS will consider the validity of the packet's checksum to be unknown.
+
+OVS will not re-calculate or update the packet's checksum if the checksum is
+already known to be correct, known to be explicitly incorrect, or destined for
+an egress interface that will recalculate the checksum anyways.
+
+If OVS does invalidate the checksum, and the packet ingresses the datapath with
+a checksum that is not known to be incorrect, OVS postpones checksum updates
+until the packet egresses the datapath. This recalculation can either be
+performed by OVS or, be offloaded onto the NIC if the egress NIC supports
+checksum offloading.
+
+When a packet egress the datapath, the packet flags and the egress interface
+flags are verified to make sure all required offload features to send out the
+packet are available on the egress interface. If not, the data path will fall
+back to equivalent software implementation.
+
+
+Interface (a.k.a. Netdev)
+-
+
+When the interface initiates, it should set the flags to tell the datapath
+which offload features are supported. For example, if the driver supports IP
+checksum offloading, then ``netdev->ol_flags`` should set the flag
+``NETDEV_TX_OFFLOAD_IPV4_CKSUM``.
+
+
+Rules
+-
+
+1) OVS should strive to forward all packets regardless of checksum.
+
+2) OVS must not correct a known bad packet checksum.
+
+3) Packet with flag ``DP_PACKET_OL_RX_IP_CKSUM_GOOD`` means that the IP
+   checksum is present in the packet and it is good.
+
+4) Packet with flag ``DP_PACKET_OL_RX_IP_CKSUM_BAD`` 

[ovs-dev] [PATCH v15 2/4] dpif-netdev: Show netdev offloading flags.

2023-06-14 Thread Mike Pattrick
This patch modifies netdev_get_status to include information about
checksum offload status by port, allowing the user to gain insight into
where checksum offloading is active.

Signed-off-by: Flavio Leitner 
Co-authored-by: Flavio Leitner 
Signed-off-by: Mike Pattrick 
---
 Since v9:
  - Removed entire ovs-appctl dpif-netdev/offload-show command, replaced
with a field in the netdev status.
  - Removed duplicative field tx_tso_offload from netdev-dpdk.c
 Since v10:
  - No change
 Since v11:
  - Removed depricated documentation
  - Clarified offload direction in status field
 Since v13:
  - Refactored statistics column macros
 Since v14:
  - No change
---
 lib/netdev-dpdk.c|  5 -
 lib/netdev.c | 29 ++---
 tests/dpif-netdev.at | 18 ++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8cb1a7703..87b3f0297 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1747,11 +1747,6 @@ netdev_dpdk_get_config(const struct netdev *netdev, 
struct smap *args)
 } else {
 smap_add(args, "rx_csum_offload", "false");
 }
-if (dev->hw_ol_features & NETDEV_TX_TSO_OFFLOAD) {
-smap_add(args, "tx_tso_offload", "true");
-} else {
-smap_add(args, "tx_tso_offload", "false");
-}
 smap_add(args, "lsc_interrupt_mode",
  dev->lsc_interrupt_mode ? "true" : "false");
 
diff --git a/lib/netdev.c b/lib/netdev.c
index c79778378..79b17d006 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -43,6 +43,7 @@
 #include "netdev-provider.h"
 #include "netdev-vport.h"
 #include "odp-netlink.h"
+#include "openvswitch/json.h"
 #include "openflow/openflow.h"
 #include "packets.h"
 #include "openvswitch/ofp-print.h"
@@ -1373,9 +1374,31 @@ netdev_get_next_hop(const struct netdev *netdev,
 int
 netdev_get_status(const struct netdev *netdev, struct smap *smap)
 {
-return (netdev->netdev_class->get_status
-? netdev->netdev_class->get_status(netdev, smap)
-: EOPNOTSUPP);
+int err = EOPNOTSUPP;
+
+/* Set offload status only if relevant. */
+if (netdev_get_dpif_type(netdev) &&
+strcmp(netdev_get_dpif_type(netdev), "system")) {
+
+#define OL_ADD_STAT(name, bit) \
+smap_add(smap, "tx_" name "_offload", \
+ netdev->ol_flags & bit ? "true" : "false");
+
+OL_ADD_STAT("ip_csum", NETDEV_TX_OFFLOAD_IPV4_CKSUM);
+OL_ADD_STAT("tcp_csum", NETDEV_TX_OFFLOAD_TCP_CKSUM);
+OL_ADD_STAT("udp_csum", NETDEV_TX_OFFLOAD_UDP_CKSUM);
+OL_ADD_STAT("sctp_csum", NETDEV_TX_OFFLOAD_SCTP_CKSUM);
+OL_ADD_STAT("tcp_seg", NETDEV_TX_OFFLOAD_TCP_TSO);
+#undef OL_ADD_STAT
+
+err = 0;
+}
+
+if (!netdev->netdev_class->get_status) {
+return err;
+}
+
+return netdev->netdev_class->get_status(netdev, smap);
 }
 
 /* Returns all assigned IP address to  'netdev' and returns 0.
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index baab60a22..60d789beb 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -650,6 +650,24 @@ AT_CHECK([ovs-appctl revalidator/resume])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([dpif-netdev - check tx packet checksum offloading])
+OVS_VSWITCHD_START(
+  [add-port br0 p1 \
+   -- set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p0.sock \
+   -- set bridge br0 datapath-type=dummy \
+ other-config:datapath-id=1234 fail-mode=secure])
+
+AT_CHECK([ovs-vsctl get interface p1 status | sed -n 's/^{\(.*\).*}$/\1/p'], 
[0], [dnl
+tx_ip_csum_offload="false", tx_sctp_csum_offload="false", 
tx_tcp_csum_offload="false", tx_tcp_seg_offload="false", 
tx_udp_csum_offload="false"
+], [])
+
+AT_CHECK([ovs-vsctl get interface br0 status | sed -n 's/^{\(.*\).*}$/\1/p'], 
[0], [dnl
+tx_ip_csum_offload="false", tx_sctp_csum_offload="false", 
tx_tcp_csum_offload="false", tx_tcp_seg_offload="false", 
tx_udp_csum_offload="false"
+], [])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 # SEND_UDP_PKTS([p_name], [p_ofport])
 #
 # Sends 128 packets to port 'p_name' with different UDP destination ports.
-- 
2.31.1

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


[ovs-dev] [PATCH v15 0/4] Enhanced checksum support

2023-06-14 Thread Mike Pattrick
This patch set is a stripped down subset of the initial 17 patchset introduced
by Flavio Leitner in 2021.

The initial omnibus patchset was very complex and included a refactor, which
stymied review and would have made backporting more complex. It also didn't
resolve an ongoing issue with the DPDK netdev where we are currently
incorrectly setting vhost flags, resulting in connectivity inturruptions when
upgrading OVS to the full TSO patchset without restarting the attached virtual
machines.

The current 4-patch set is stripped down to include the following:

 1. Public facing documentation on the phylosophy of how OVS handles checksums
 2. A method for the user to easily check which checksums are offloaded per
interface
 3 & 4. Most checksuming activity delayed until a packet is about to be:
   - sent, or
   - transformed in such a way that checksumming wouldn't be offloadable
 regardless, such as encapsulation

The main benefit of this set in its current state is an improved handling of
checksums when encapsulating packets. But the set lays a groundwork for future
work including improvements to how dpdk negotiates virtio vhost flags and more
efficent handling of checksums in userspace with tso.

This 4-patch reduced set has gone through a lot of revisions so far, including:

v5
 - Refactor was mostly removed, except for valid->good
 - Reset unsupported offload flags in send_prepare
 - Moved send_prepare from process_upcall to netdev_upcall

v6
 - Re-added tests that were incorrectly excluded from v5

v7
 - David Marchand found an issue while upgrading OVS and not rebooting attached
   vhost VMs where we can't change flags post negotiation or check if they have
   already been set.
 - This was temporarily resolved by not setting the offending flags
 - This issue will be addressed in a more robust fasion if this patchset is
   applied.

v8
 - v7 patch 3 failed intel ci, moved some code from patch 4 to patch 3

v9
 - Resolved a 10% performance hit in a DPDK-PVP workload that David Marchand
   found

v10
 - Large amount of formatting and grammar issues
 - Ported change to AVX512 code
 - ovs-appctl command removed
 - new fields added to netdev status including the information removed from
   ovs-appctl

v11
 - AVX512 change introduced a checksum bug due to an incorrectly sized
   datatype, caught by intel-ci and required a recent Xeon to reproduce.

v12
 - Updated documentation
 - Changed tso status field name
 - Excluded the combination of rte_flow offload, userspace tso, and RAW_ENCAP
   from simultaniously ocuring
 - Extended AVX512 support to IPv6

v13
 - Re-added erroniously missing mutex annocations from v12

v14
 - Rewrote a section of the documentation
 - Removed exclusion of the combination of rte_flow offload, userspace tso, and
   RAW_ENCAP
 - Added a TUNGETFEATURES check in netdev-linux.c for better support of early
   2.6 era kernels

v15
 - Moved some new code outside of mutex protection
 - Only check TUNGETFEATURES once
 - Respect FLOW_TNL_F_CSUM flag 

Mike Pattrick (4):
  Documentation: Document netdev offload.
  dpif-netdev: Show netdev offloading flags.
  userspace: Enable IP checksum offloading by default.
  userspace: Enable L4 checksum offloading by default.

 Documentation/automake.mk |   1 +
 Documentation/topics/index.rst|   1 +
 .../topics/userspace-checksum-offloading.rst  |  96 +++
 lib/conntrack.c   |  30 +-
 lib/dp-packet.c   |  40 +++
 lib/dp-packet.h   | 140 +-
 lib/dpif-netdev-extract-avx512.c  |  57 
 lib/dpif-netdev.c |   2 +
 lib/flow.c|  38 ++-
 lib/ipf.c |  11 +-
 lib/netdev-dpdk.c | 230 +++-
 lib/netdev-dummy.c|  22 ++
 lib/netdev-linux.c| 258 +-
 lib/netdev-native-tnl.c   |  53 ++--
 lib/netdev.c  |  81 +++---
 lib/odp-execute-avx512.c  | 108 +---
 lib/odp-execute.c |  21 +-
 lib/packets.c | 209 +++---
 lib/packets.h |   3 +
 tests/dpif-netdev.at  |  96 +++
 20 files changed, 1176 insertions(+), 321 deletions(-)
 create mode 100644 Documentation/topics/userspace-checksum-offloading.rst

-- 
2.31.1

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


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

2023-06-14 Thread Mike Pattrick
The netdev receiving packets is supposed to provide the flags
indicating if the IP checksum was verified and it is GOOD or BAD,
otherwise the stack will check when appropriate by software.

If the packet comes with good checksum, then postpone the
checksum calculation to the egress device if needed.

When encapsulate a packet with that flag, set the checksum
of the inner IP header since that is not yet supported.

Calculate the IP checksum when the packet is going to be sent over
a device that doesn't support the feature.

Linux devices don't support IP checksum offload alone, so the
support is not enabled.

Signed-off-by: Flavio Leitner 
Co-authored-by: Flavio Leitner 
Signed-off-by: Mike Pattrick 
---
 Since v9:
  - Removed duplicative field tx_ip_csum_offload from netdev-dpdk.c
  - Left rx_csum_offload field as it is not duplicative
  - Moved system-userspace-offload.at tests to dpif-netdev.at
  - Various visual changes
  - Extended miniflow_extract changes into avx512 code
 Since v10:
  - avx512 checksum length corrected
 Since v11:
  - If hw-offload and userspace-tso is enabled, don't allow dpdk to
  offload RAW_ENCAP
 Since v13:
  - Removed dpdk hw-offload related code
 Since v14:
  - Moved some netdev_dummy_send code out of mutex
---
 lib/conntrack.c  | 19 
 lib/dp-packet.c  | 15 ++
 lib/dp-packet.h  | 62 +++--
 lib/dpif-netdev-extract-avx512.c |  5 ++
 lib/dpif-netdev.c|  2 +
 lib/flow.c   | 15 --
 lib/ipf.c| 11 +++--
 lib/netdev-dpdk.c| 71 +++--
 lib/netdev-dummy.c   | 22 +
 lib/netdev-native-tnl.c  | 21 ++---
 lib/netdev.c | 16 +++
 lib/odp-execute-avx512.c | 20 +---
 lib/odp-execute.c| 21 +++--
 lib/packets.c| 34 +++---
 tests/dpif-netdev.at | 78 
 15 files changed, 345 insertions(+), 67 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index ce8a63de5..78c3e578c 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2043,16 +2043,15 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
*pkt, ovs_be16 dl_type,
 ctx->key.dl_type = dl_type;
 
 if (ctx->key.dl_type == htons(ETH_TYPE_IP)) {
-bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt);
-if (hwol_bad_l3_csum) {
+if (dp_packet_ip_checksum_bad(pkt)) {
 ok = false;
 COVERAGE_INC(conntrack_l3csum_err);
 } else {
-bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt)
- || dp_packet_hwol_is_ipv4(pkt);
-/* Validate the checksum only when hwol is not supported. */
+/* Validate the checksum only when hwol is not supported and the
+ * packet's checksum status is not known. */
 ok = extract_l3_ipv4(>key, l3, dp_packet_l3_size(pkt), NULL,
- !hwol_good_l3_csum);
+ !dp_packet_hwol_is_ipv4(pkt) &&
+ !dp_packet_ip_checksum_good(pkt));
 }
 } else if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
 ok = extract_l3_ipv6(>key, l3, dp_packet_l3_size(pkt), NULL);
@@ -2063,8 +2062,8 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
*pkt, ovs_be16 dl_type,
 if (ok) {
 bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt);
 if (!hwol_bad_l4_csum) {
-bool  hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt)
-  || dp_packet_hwol_tx_l4_checksum(pkt);
+bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt)
+ || dp_packet_hwol_tx_l4_checksum(pkt);
 /* Validate the checksum only when hwol is not supported. */
 if (extract_l4(>key, l4, dp_packet_l4_size(pkt),
>icmp_related, l3, !hwol_good_l4_csum,
@@ -3373,7 +3372,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
conn_lookup_ctx *ctx,
 }
 if (seq_skew) {
 ip_len = ntohs(l3_hdr->ip_tot_len) + seq_skew;
-if (!dp_packet_hwol_is_ipv4(pkt)) {
+if (dp_packet_hwol_tx_ip_csum(pkt)) {
+dp_packet_ol_reset_ip_csum_good(pkt);
+} else {
 l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
 l3_hdr->ip_tot_len,
 htons(ip_len));
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ae8ab5800..61c36de44 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -21,6 +21,7 @@
 #include "dp-packet.h"
 #include "netdev-afxdp.h"
 #include "netdev-dpdk.h"
+#include 

Re: [ovs-dev] [PATCH v3] conntrack: Extract l4 information for SCTP.

2023-06-14 Thread Paolo Valerio
Ilya Maximets  writes:

> On 6/12/23 16:57, Aaron Conole wrote:
>> Paolo Valerio  writes:
>> 
>>> since a27d70a89 ("conntrack: add generic IP protocol support") all
>>> the unrecognized IP protocols get handled using ct_proto_other ops
>>> and are managed as L3 using 3 tuples.
>>>
>>> This patch stores L4 information for SCTP in the conn_key so that
>>> multiple conn instances, instead of one with ports zeroed, will be
>>> created when there are multiple SCTP connections between two hosts.
>>> It also performs crc32c check when not offloaded, and adds SCTP to
>>> pat_enabled.
>>>
>>> With this patch, given two SCTP association between two hosts,
>>> tracking the connection will result in:
>>>
>>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=55884,dport=5201),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5201,dport=12345),zone=1
>>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=59874,dport=5202),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5202,dport=12346),zone=1
>>>
>>> instead of:
>>>
>>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=0,dport=0),reply=(src=10.1.1.1,dst=10.1.1.2,sport=0,dport=0),zone=1
>>>
>>> Signed-off-by: Paolo Valerio 
>>> ---
>> 
>> Thanks for this work - I think it looks good.
>> 
>> Perhaps it should have a NEWS item mentioned that the userspace
>> conntrack now supports matching SCTP l4 data.
>> 
>> If you do spin a v4 with that change, you can keep my:
>> 
>> Acked-by: Aaron Conole 
>
> Hi, Paolo and Aaron.
>
> I'm getting a consistent test failure while running check-kernel
> on Ubuntu 22.10 with 5.19 kernel:
>
>
> ./system-traffic.at:4754: cat ofctl_monitor.log
> --- -   2023-06-14 11:26:41.958591125 +
> +++ /root/ovs/tests/system-kmod-testsuite.dir/at-groups/105/stdout  
> 2023-06-14 11:26:41.95200 +
> @@ -12,8 +12,6 @@
>  
> sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>  sctp_csum:9b67e853
>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=54 in_port=1 (via action) 
> data_len=54 (unbuffered)
>  
> sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>  sctp_csum:bc0e5463
> -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=50 
> ct_state=est|rpl|trk|dnat,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=132,ct_tp_src=54969,ct_tp_dst=12345,ip,in_port=2
>  (via action) data_len=50 (unbuffered)
> -sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
>  sctp_csum:d6ce6b9e
>  NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=50 in_port=1 (via action) 
> data_len=50 (unbuffered)
> -sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
>  sctp_csum:add7db93
> +sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=54969,tp_dst=12345
>  sctp_csum:5db68ce
>
>
> Do you know what can be a problem here?
>
> Test is passing on Fedora 38 with 6.3 kernel and on rhel 9.2.
>

Hi Ilya,

Uhm, it seems there's a problem with the shutdown sequence.
I just ran the on a VM:

vagrant@ubuntu2210:~/ovs$ grep CONFIG_NF_CT_PROTO_SCTP 
/boot/config-5.19.0-38-generic 
CONFIG_NF_CT_PROTO_SCTP=y

vagrant@ubuntu2210:~/ovs$ grep VERSION /etc/os-release 
VERSION_ID="22.10"
VERSION="22.10 (Kinetic Kudu)"
VERSION_CODENAME=kinetic

vagrant@ubuntu2210:~/ovs$ uname -r
5.19.0-38-generic

but I can't see the failure.
Any chance to see if they are marked for some reason as invalid?

> Best regards, Ilya Maximets.

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


Re: [ovs-dev] [PATCH ovn] controller: disable OpenFlow inactivity probing

2023-06-14 Thread Vladislav Odintsov
Hi Mark,

thanks for taking time to look on this!

Your point with a hung OVS is really an interesting case.

From one hand it’s a possible situation, and from another I guess it’s much 
higher probability for OVS to be busy by some other work rather than to hang in 
a loop. In my installation the first happening sometimes (OVS business by real 
work). The reasons can be different but I can’t say that it’s better to drop OF 
connection in such a moment (possibly triggering ha chassis-redirect ports 
failover).

At the same time if we’re talking about L3GW node, which has a gateway chassis 
redirect port, the hung OVS should be detected by other nodes with BFD probes 
(if it’s a ha chassis-redirect port).
And if it’s a normal hypervisor (with just a vif ports), so what can we do with 
it? I guess if we drop OF connection, it won’t make any positive changes. Maybe 
just release memory needed for handling this connection and not allocate 
buffers…? Unfortunately I can’t evaluate this.

Moreover, the OVN default is a disabled probing. What can be a possible 
recommended values to set probes if to leave it as is? How should users find 
out that they have to configure this? Currently in docs there is only 
information that this option configures probe interval. But it’s not obvious 
when to configure it and which value to set.

I hope this makes sense.

Also, which Han’s patch to OVS you’re talking about? I looked through open OVS 
patches and haven’t seen any. Please point me out.

regards,
Vladislav Odintsov

> On 9 Jun 2023, at 18:28, Mark Michelson  wrote:
> 
> Hi,
> 
> Thanks for linking the email thread, since I had the exact same concern that 
> Numan did about detecting if ovs-vswitchd goes down. It sounds like because 
> of the nature of unix sockets, this disconnection is still detected by OVN 
> and failover can occur as expected.
> 
> But what about if ovs-vswitchd is still running but in a "compromised" state. 
> For instance, what if a coding error in ovs-vswitchd results in it running an 
> infinite loop? In such a case, the unix connection would still be alive, but 
> OVN would not be able to communicate with the ovs-vswitchd process.
> 
> Does this situation eventually lead to OVN disconnecting anyway after it 
> fills up the unix socket's buffer? If so, how long does that take? Or does it 
> lead to OVN slowly growing in memory usage while it waits forever to be able 
> to write to the socket?
> 
> I think removing the hard-coded 5 second probe intervals from pinctrl.c and 
> features.c is a good idea. But I think we should leave the configurable probe 
> interval used by ofctrl.c for the case I described before. Since Han's patch 
> to OVS should disable probe intervals from the OVS side by default, we would 
> not trigger bad behavior simply because OVN has a lot of work to do during a 
> recompute. What do you think?
>> On 6/8/23 10:15, Vladislav Odintsov wrote:
>> OpenFlow connection is established over unix socket, which is a reliable
>> connection and doesn't require additional probing.
>> With this patch openflow probing in ovn-controller -> ovs-vswitchd
>> direction is disabled for all three connections:
>>   - OF flows management connection,
>>   - OF features negotiation connection,
>>   - pinctrl connection.
>> ovn-controller external_ids:ovn-openflow-probe-interval is removed as
>> non-needed anymore.
>> Disablement for ovs-vswitchd -> ovn-controller OF inacivity probing will
>> be done in the next patch.
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2023-May/404625.html
>> Signed-off-by: Vladislav Odintsov 
>> ---
>>  NEWS|  5 +
>>  controller/ofctrl.c | 14 ++
>>  controller/ofctrl.h |  4 +---
>>  controller/ovn-controller.8.xml | 14 --
>>  controller/ovn-controller.c | 21 +
>>  controller/pinctrl.c|  2 +-
>>  lib/features.c  |  7 ++-
>>  7 files changed, 12 insertions(+), 55 deletions(-)
>> diff --git a/NEWS b/NEWS
>> index 645acea1f..bd63b187b 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -1,5 +1,10 @@
>>  Post v23.06.0
>>  -
>> +  - Disable OpenFlow inactivity probing between ovn-controller and OVS.
>> +OF connection is established over unix socket, which is a reliable
>> +connection method and doesn't require additional probing.
>> +external_ids:ovn-openflow-probe-interval configuration option for
>> +ovn-controller is no longer matter.
>>OVN v23.06.0 - 01 Jun 2023
>>  --
>> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
>> index 64a444ff6..788634494 100644
>> --- a/controller/ofctrl.c
>> +++ b/controller/ofctrl.c
>> @@ -415,11 +415,9 @@ static void ofctrl_recv(const struct ofp_header *, enum 
>> ofptype);
>>void
>>  ofctrl_init(struct ovn_extend_table *group_table,
>> -struct ovn_extend_table *meter_table,
>> -int 

Re: [ovs-dev] [PATCH 3/6] dpif-netdev: Add pmd-sleep-show command.

2023-06-14 Thread David Marchand
On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor  wrote:
>
> Max requested sleep time and status for a PMD thread
> is logged at start up or when changed, but it can be
> convenient to have a command to dump this information
> explicitly.
>
> It is envisaged that this will be expanded when future
> additions are added.
>
> Signed-off-by: Kevin Traynor 
> ---
>  Documentation/topics/dpdk/pmd.rst |  4 
>  NEWS  |  2 ++
>  lib/dpif-netdev.c | 40 +++
>  tests/pmd.at  | 29 ++
>  4 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/topics/dpdk/pmd.rst 
> b/Documentation/topics/dpdk/pmd.rst
> index b261e9254..bedd42194 100644
> --- a/Documentation/topics/dpdk/pmd.rst
> +++ b/Documentation/topics/dpdk/pmd.rst
> @@ -349,4 +349,8 @@ time not processing packets will be determined by the 
> sleep and processor
>  wake-up times and should be tested with each system configuration.
>
> +The max sleep time that a PMD thread may request can be shown with::
> +
> +$ ovs-appctl dpif-netdev/pmd-sleep-show
> +
>  Sleep time statistics for 10 secs can be seen with::
>
> diff --git a/NEWS b/NEWS
> index 1ccc6f948..32b9e8167 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -39,4 +39,6 @@ Post-v3.1.0
> - Userspace datapath:
>   * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
> + * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
> +   max sleep request setting of PMD thread cores.
>
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 94c8ca943..dadf17b70 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -702,4 +702,5 @@ enum pmd_info_type {
>  PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
>  PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
> +PMD_INFO_MAX_SLEEP_SHOW,   /* Show max sleep performance details. */
>  };
>
> @@ -884,4 +885,18 @@ sorted_poll_list(struct dp_netdev_pmd_thread *pmd, 
> struct rxq_poll **list,
>  }
>
> +static void
> +pmd_max_sleep_show(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
> +   uint64_t default_max_sleep)
> +{
> +if (pmd->core_id != NON_PMD_CORE_ID) {
> +ds_put_format(reply,
> +  "PMD thread core %3u NUMA %2d: "
> +  "Max sleep request set to",
> +  pmd->core_id, pmd->numa_id);
> +ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
> +ds_put_cstr(reply, "\n");
> +}
> +}
> +

Nit: I would move this new function later, and let pmd_info_show_rxq()
and sorted_poll_list() close to each other.


>  static void
>  pmd_info_show_rxq(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
> @@ -1442,5 +1457,6 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  unsigned long long max_secs = (PMD_INTERVAL_LEN * PMD_INTERVAL_MAX)
>/ INTERVAL_USEC_TO_SEC;
> -bool first_show_rxq = true;
> +bool first_pmd = true;
> +uint64_t default_max_sleep = 0;

You can move this new variable in the only block that cares about it.


>
>  ovs_mutex_lock(_netdev_mutex);
> @@ -1490,5 +1506,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  }
>  if (type == PMD_INFO_SHOW_RXQ) {
> -if (first_show_rxq) {
> +if (first_pmd) {
>  if (!secs || secs > max_secs) {
>  secs = max_secs;
> @@ -1499,5 +1515,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  ds_put_format(, "Displaying last %u seconds "
>"pmd usage %%\n", secs);
> -first_show_rxq = false;
> +first_pmd = false;
>  }
>  pmd_info_show_rxq(, pmd, secs);
> @@ -1508,4 +1524,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
> argc, const char *argv[],
>  } else if (type == PMD_INFO_PERF_SHOW) {
>  pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
> +} else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
> +if (first_pmd) {
> +atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
> +ds_put_format(, "PMD max sleep request is %"PRIu64" "
> +  "usecs.", default_max_sleep);
> +ds_put_cstr(, "\n");
> +ds_put_format(, "PMD load based sleeps are %s.",
> +  default_max_sleep ? "enabled" : "disabled");
> +ds_put_cstr(, "\n");
> +first_pmd = false;
> +}
> +pmd_max_sleep_show(, pmd, default_max_sleep);
>  }
>  }
> @@ -1608,5 +1636,6 @@ dpif_netdev_init(void)
>  static enum pmd_info_type show_aux = PMD_INFO_SHOW_STATS,
>clear_aux = 

Re: [ovs-dev] [PATCH 2/6] pmd.at: Add macro for checking pmd sleep max time and state.

2023-06-14 Thread David Marchand
On Wed, Jun 14, 2023 at 3:37 PM Kevin Traynor  wrote:
>
> This is just cosmetic. There is no change to the tests.

Hum, except one issue in CHECK_DP_SLEEP_MAX, I would tend to agree :-).

>
> Signed-off-by: Kevin Traynor 
> ---
>  tests/pmd.at | 39 ---
>  1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 374ad7217..64d8f6e2b 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -61,4 +61,20 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
>  ])
>
> +dnl CHECK_DP_SLEEP_MAX([max_sleep], [enabled], [+line])
> +dnl
> +dnl Checks correct pmd load based sleep is set for the datapath.
> +dnl Checking starts from line number 'line' in ovs-vswithd.log .
> +m4_define([CHECK_DP_SLEEP_MAX], [
> +SLEEP_TIME="PMD max sleep request is $1 usecs."
> +SLEEP_STATE="PMD load based sleeps are $2."
> +line_st=$4

$3 ?


> +if [[ -z "$line_st" ]]
> +then
> +line_st="+0"
> +fi
> +OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_TIME"])
> +OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_STATE"])
> +])
> +
>  m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
> \)[[0-9]]*:/\1\2:/"])
>  m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
> @@ -1256,46 +1272,39 @@ OVS_VSWITCHD_STOP
>  AT_CLEANUP
>
> -dnl Check default state
>  AT_SETUP([PMD - pmd sleep])
>  OVS_VSWITCHD_START
>
>  dnl Check default
> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD max sleep request is 0 
> usecs."])
> -OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load based sleeps are 
> disabled."])
> +CHECK_DP_SLEEP_MAX([0], [disabled], [])
>
>  dnl Check low value max sleep
>  get_log_next_line_num

Not directly related to this patch.
I did not realise before when those tests were added, but it is
surprising get_log_next_line_num (from tests/alb.at) is visible from
these tests defined in pmd.at.
I would move this helper to a common file instead.


The rest lgtm.


-- 
David Marchand

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


Re: [ovs-dev] [PATCH v4] netdev-dpdk: Drop TSO in case of conflicting virtio features.

2023-06-14 Thread 0-day Robot
Bleep bloop.  Greetings David Marchand, 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: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 netdev-dpdk: Drop TSO in case of conflicting virtio 
features.
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


Re: [ovs-dev] [PATCH v4 8/9] netdev-tc-offloads: Probe for allowing vxlan gbp support

2023-06-14 Thread Gavin Li via dev



On 6/14/2023 9:22 PM, Ilya Maximets wrote:

External email: Use caution opening links or attachments


On 6/14/23 10:22, Roi Dayan wrote:

From: Gavin Li 

Kernels that do not support vxlan gbp would treat the rule that has vxlan
gbp encap action or vxlan gbp id match differently, either reject it or
just skip the action/match and continue processing the knowing ones.

To solve the issue, probe and disallow inserting rules with vxlan gbp
action/match if kernel does not support it.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
---
  lib/netdev-offload-tc.c | 64 +++--
  1 file changed, 61 insertions(+), 3 deletions(-)





@@ -2788,6 +2800,51 @@ probe_tc_block_support(int ifindex)
  }
  }

+static void
+probe_vxlan_gbp_support(int ifindex)
+{
+struct tc_flower flower;
+struct tcf_id id;
+int block_id = 0;
+int prio = 1;
+int error;
+
+error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS);
+if (error) {
+return;
+}
+
+memset(, 0, sizeof flower);
+
+flower.tc_policy = TC_POLICY_SKIP_HW;
+flower.key.eth_type = htons(ETH_P_IP);
+flower.mask.eth_type = OVS_BE16_MAX;
+flower.tunnel = true;
+flower.mask.tunnel.id = 1;
+flower.mask.tunnel.ipv4.ipv4_src = 1;
+flower.mask.tunnel.ipv4.ipv4_dst = 1;
+flower.mask.tunnel.tp_dst = 1;
+flower.mask.tunnel.gbp.id = 512;
+flower.key.tunnel.ipv4.ipv4_src = 117901063;
+flower.key.tunnel.ipv4.ipv4_dst = 134678279;
+flower.key.tunnel.tp_dst = 46354;
+flower.key.tunnel.gbp.id = 512;

Most of the fields above are defined as restricted BE types.
Assigning plain integers breaks the build with sparse.
You can see GHA failures on this patch.  As reported by the
ovsrobot:
   https://mail.openvswitch.org/pipermail/ovs-build/2023-June/031588.html

Best regards, Ilya Maximets.

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


Re: [ovs-dev] [PATCH ovn v2] controller, northd: pass arp/nd from HW VTEP to lrouter pipeline

2023-06-14 Thread Vladislav Odintsov
Thanks Dumitru!

> On 14 Jun 2023, at 16:37, Dumitru Ceara  wrote:
> 
> On 5/30/23 17:03, Vladislav Odintsov wrote:
>> This patch is intended to make next two changes:
>> 
>> 1. Support create/update of MAC_Binding for GARP/ND from HW VTEP.
>> 
>> Prior to this patch MAC_Binding records were created only when LRP issued
>> ARP request/Neighbor solicitation.  If IP to MAC in network attached via
>> vtep lport was changed the old MAC_Binding prevented normal
>> communication.  Now router port (chassisredirect) in logical switch with
>> vtep lport catches GARP or Neighbor Solicitation and updates MAC_Binding.
>> 
>> New flow with max priority was added in ls_in_arp_nd_resp and additional
>> OF rule in table 37 (OFTABLE_REMOTE_OUTPUT) to process multicast packets
>> received from RAMP_SWITCH.
>> 
>> 2. Answer to ARP/ND on requests coming from HW VTEP in lrouter pipeline.
>> 
>> This is needed to reduce duplicated arp-responses, which were generated
>> by OVN arp-responder flows in node, where chassisredirect port located
>> with responses coming directly from VM interfaces.
>> 
>> As a result of this change, now vifs located on same chassis, where
>> chassisredirect ports are located receive ARP/ND mcast packets, which
>> previously were suppressed by arp-responder on the node.
>> 
>> Tests and documentation were updated to conform to new behaviour.
>> 
>> Signed-off-by: Vladislav Odintsov 
>> ---
> 
> Thanks, Vladislav, for the new support!  I applied this to the main
> branch and it will be available in the next release.
> 
> Regards,
> Dumitru
> 
> ___
> dev mailing list
> d...@openvswitch.org 
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Regards,
Vladislav Odintsov

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


[ovs-dev] [PATCH 4/6] dpif-netdev: Remove pmd-sleep-max experimental tag.

2023-06-14 Thread Kevin Traynor
Signed-off-by: Kevin Traynor 
---
 Documentation/topics/dpdk/pmd.rst | 4 ++--
 NEWS  | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index bedd42194..40e6b7843 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -325,6 +325,6 @@ reassignment due to PMD Auto Load Balance. For example, 
this could be set
 (in min) such that a reassignment is triggered at most every few hours.
 
-PMD load based sleeping (Experimental)
---
+PMD load based sleeping
+---
 
 PMD threads constantly poll Rx queues which are assigned to them. In order to
diff --git a/NEWS b/NEWS
index 32b9e8167..89f05039c 100644
--- a/NEWS
+++ b/NEWS
@@ -41,4 +41,5 @@ Post-v3.1.0
  * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
max sleep request setting of PMD thread cores.
+ * Remove experimental tag from PMD load based sleeping.
 
 
-- 
2.40.1

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


[ovs-dev] [PATCH v4] netdev-dpdk: Drop TSO in case of conflicting virtio features.

2023-06-14 Thread David Marchand
At some point in OVS history, some virtio features were announced as
supported (ECN and UFO virtio features).

The userspace TSO code, which has been added later, does not support
those features and tries to disable them.

This breaks OVS upgrades: if an existing VM already negotiated such
features, their lack on reconnection to an upgraded OVS triggers a
vhost socket disconnection by Qemu.
This results in an endless loop because Qemu then retries with the same
set of virtio features.

This patch proposes to try and detect those vhost socket disconnection
and fallback restoring the old virtio features (and disabling TSO for this
vhost port).

Signed-off-by: David Marchand 
---
Note: this series depends on Mike series and won't apply cleanly
without it.

Changelog since v3:
- updated documentation now that the interface offloads status is reported
  in ovsdb,
- fixed one coding style issue,

Changelog since v2:
- reported workaround presence in the ovsdb port status field and
  updated documentation accordingly,
- tried to use "better" names, to distinguish ECN virtio feature from
  TSO OVS netdev feature, 

Changelog since v1:
- added a note in the documentation,
- fixed vhost unregister trigger (so that both disabling and re-enabling
  TSO is handled),
- cleared netdev features when disabling TSO,
- changed level and ratelimited log message on vhost socket disconnect,

---
 Documentation/topics/userspace-tso.rst | 30 -
 lib/netdev-dpdk.c  | 85 +-
 2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/Documentation/topics/userspace-tso.rst 
b/Documentation/topics/userspace-tso.rst
index 5a43c2e86b..d4111f1ab2 100644
--- a/Documentation/topics/userspace-tso.rst
+++ b/Documentation/topics/userspace-tso.rst
@@ -68,7 +68,7 @@ as follows.
 connection is established, `TSO` is thus advertised to the guest as an
 available feature:
 
-QEMU Command Line Parameter::
+1. QEMU Command Line Parameter::
 
 $ sudo $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 \
 ...
@@ -83,6 +83,34 @@ used to enable same::
 $ ethtool -K eth0 tso on
 $ ethtool -k eth0
 
+**Note:** Enabling this feature impacts the virtio features exposed by the DPDK
+vHost User backend to a guest. If a guest was already connected to OvS before
+enabling TSO and restarting OvS, this guest ports won't have TSO available::
+
+$ ovs-vsctl get interface vhost0 status | grep -o '[^ ]*_offload=[^ ]*"'
+tx_ip_csum_offload="true"
+tx_sctp_csum_offload="true"
+tx_tcp_csum_offload="true"
+tx_tcp_seg_offload="false"
+tx_udp_csum_offload="true"
+
+To help diagnose the issue, those ports have some additional information in
+their status field in ovsdb::
+
+$ ovs-vsctl get interface vhost0 status | grep -o 'disabled_tso=[^ ]*"'
+disabled_tso="true"
+
+To restore TSO for this guest ports, this guest QEMU process must be stopped,
+then started again. OvS will then report::
+
+$ ovs-vsctl get interface vhost0 status | grep -o '[^ ]*_offload=[^ ]*"'
+tx_ip_csum_offload="true"
+tx_sctp_csum_offload="true"
+tx_tcp_csum_offload="true"
+tx_tcp_seg_offload="true"
+tx_udp_csum_offload="true"
+$ ovs-vsctl get interface vhost0 status | grep -o 'disabled_tso=[^ ]*"'
+
 ~~~
 Limitations
 ~~~
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c6b41e8365..2dbb8b42e3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -473,6 +473,15 @@ struct netdev_dpdk {
 /* True if vHost device is 'up' and has been reconfigured at least 
once */
 bool vhost_reconfigured;
 
+/* Set on driver start (which means after a vHost connection is
+ * accepted), and cleared when the vHost device gets configured. */
+bool vhost_initial_config;
+
+/* Set on disconnection if an initial configuration did not finish.
+ * This triggers a workaround for Virtio features negotiation, that
+ * makes TSO unavailable. */
+bool vhost_workaround_enable_ecn;
+
 atomic_uint8_t vhost_tx_retries_max;
 /* 2 pad bytes here. */
 );
@@ -1359,6 +1368,7 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 dev->requested_lsc_interrupt_mode = 0;
 ovsrcu_index_init(>vid, -1);
 dev->vhost_reconfigured = false;
+dev->vhost_initial_config = false;
 dev->attached = false;
 dev->started = false;
 dev->reset_needed = false;
@@ -3883,6 +3893,10 @@ netdev_dpdk_vhost_user_get_status(const struct netdev 
*netdev,
 xasprintf("%d", vring.size));
 }
 
+if (dev->vhost_workaround_enable_ecn) {
+smap_add_format(args, "disabled_tso", "true");
+}
+
 ovs_mutex_unlock(>mutex);
 return 0;
 }
@@ -4214,6 +4228,12 @@ netdev_dpdk_remap_txqs(struct netdev_dpdk *dev)
 free(enabled_queues);
 }
 
+static bool
+netdev_dpdk_vhost_tso_enabled(struct netdev_dpdk *dev)
+{
+return userspace_tso_enabled() && 

Re: [ovs-dev] [PATCH ovn v2] controller, northd: pass arp/nd from HW VTEP to lrouter pipeline

2023-06-14 Thread Dumitru Ceara
On 5/30/23 17:03, Vladislav Odintsov wrote:
> This patch is intended to make next two changes:
> 
> 1. Support create/update of MAC_Binding for GARP/ND from HW VTEP.
> 
> Prior to this patch MAC_Binding records were created only when LRP issued
> ARP request/Neighbor solicitation.  If IP to MAC in network attached via
> vtep lport was changed the old MAC_Binding prevented normal
> communication.  Now router port (chassisredirect) in logical switch with
> vtep lport catches GARP or Neighbor Solicitation and updates MAC_Binding.
> 
> New flow with max priority was added in ls_in_arp_nd_resp and additional
> OF rule in table 37 (OFTABLE_REMOTE_OUTPUT) to process multicast packets
> received from RAMP_SWITCH.
> 
> 2. Answer to ARP/ND on requests coming from HW VTEP in lrouter pipeline.
> 
> This is needed to reduce duplicated arp-responses, which were generated
> by OVN arp-responder flows in node, where chassisredirect port located
> with responses coming directly from VM interfaces.
> 
> As a result of this change, now vifs located on same chassis, where
> chassisredirect ports are located receive ARP/ND mcast packets, which
> previously were suppressed by arp-responder on the node.
> 
> Tests and documentation were updated to conform to new behaviour.
> 
> Signed-off-by: Vladislav Odintsov 
> ---

Thanks, Vladislav, for the new support!  I applied this to the main
branch and it will be available in the next release.

Regards,
Dumitru

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


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

2023-06-14 Thread Dumitru Ceara
On 6/13/23 16:02, Ales Musil wrote:
> Add missing OVN_FOR_EACH_NORTHD around
> FDB aging test.
> 
> Signed-off-by: Ales Musil 
> ---

Hi Ales,

This misses the "Fixes" tag:
Fixes: ae9a5488824c ("northd: Add FDB aging mechanism")

I added it and pushed the patch to the main branch.

Regards,
Dumitru

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


[ovs-dev] [PATCH 6/6] pmd-at: Add per pmd max sleep unit tests.

2023-06-14 Thread Kevin Traynor
Add unit tests for new per pmd options of pmd-sleep-max.

Signed-off-by: Kevin Traynor 
---
 tests/pmd.at | 153 +++
 1 file changed, 153 insertions(+)

diff --git a/tests/pmd.at b/tests/pmd.at
index e83206a9a..25f6b86a7 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -77,4 +77,18 @@ m4_define([CHECK_DP_SLEEP_MAX], [
 ])
 
+dnl CHECK_PMD_SLEEP_MAX([core_id], [numa_id], [max_sleep], [+line])
+dnl
+dnl Checks max sleep time of each pmd with core_id.
+dnl Checking starts from line number 'line' in ovs-vswithd.log .
+m4_define([CHECK_PMD_SLEEP_MAX], [
+PATTERN="PMD thread core   *[$1] NUMA  *[$2]: Max sleep request set to  
*[$3] usecs."
+line_st=$4
+if [[ -z "$line_st" ]]
+then
+line_st="+0"
+fi
+OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$PATTERN"])
+])
+
 m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/"])
 m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
@@ -1339,2 +1353,141 @@ PMD load based sleeps are enabled by default.
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([PMD - per pmd sleep])
+OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy-pmd 
options:n_rxq=8 options:numa_id=1], [], [], [--dummy-numa 0,0,0,1,1,8,8])
+
+dnl Check system default
+CHECK_DP_SLEEP_MAX([0], [disabled], [])
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 0 usecs by default.
+PMD load based sleeps are disabled by default.
+PMD thread core   0 NUMA  0: Max sleep request set to0 usecs.
+PMD thread core   3 NUMA  1: Max sleep request set to0 usecs.
+PMD thread core   5 NUMA  8: Max sleep request set to0 usecs.
+])
+
+dnl only a dp default
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-sleep-max=200])
+CHECK_DP_SLEEP_MAX([200], [enabled], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([0], [0], [200], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([3], [1], [200], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([5], [8], [200], [+$LINENUM])
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 200 usecs by default.
+PMD load based sleeps are enabled by default.
+PMD thread core   0 NUMA  0: Max sleep request set to  200 usecs.
+PMD thread core   3 NUMA  1: Max sleep request set to  200 usecs.
+PMD thread core   5 NUMA  8: Max sleep request set to  200 usecs.
+])
+
+dnl only per pmd
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
other_config:pmd-sleep-max=3:300,0:100,5:400])
+CHECK_DP_SLEEP_MAX([0], [disabled], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([0], [0], [100], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([3], [1], [300], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([5], [8], [400], [+$LINENUM])
+
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 0 usecs by default.
+PMD load based sleeps are disabled by default.
+PMD thread core   0 NUMA  0: Max sleep request set to  100 usecs.
+PMD thread core   3 NUMA  1: Max sleep request set to  300 usecs.
+PMD thread core   5 NUMA  8: Max sleep request set to  400 usecs.
+])
+
+dnl mix of not used default and per-pmd
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
other_config:pmd-sleep-max=50,3:300,0:100,5:200])
+CHECK_DP_SLEEP_MAX([50], [enabled], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([0], [0], [100], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([3], [1], [300], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([5], [8], [200], [+$LINENUM])
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 50 usecs by default.
+PMD load based sleeps are enabled by default.
+PMD thread core   0 NUMA  0: Max sleep request set to  100 usecs.
+PMD thread core   3 NUMA  1: Max sleep request set to  300 usecs.
+PMD thread core   5 NUMA  8: Max sleep request set to  200 usecs.
+])
+
+dnl remove a per-pmd entry and use default
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-sleep-max=50,3:300])
+CHECK_DP_SLEEP_MAX([50], [enabled], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([0], [0], [50], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([3], [1], [300], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([5], [8], [50], [+$LINENUM])
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 50 usecs by default.
+PMD load based sleeps are enabled by default.
+PMD thread core   0 NUMA  0: Max sleep request set to   50 usecs.
+PMD thread core   3 NUMA  1: Max sleep request set to  300 usecs.
+PMD thread core   5 NUMA  8: Max sleep request set to   50 usecs.
+])
+
+dnl mix and change values
+
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-sleep-max=3:400,200])
+CHECK_DP_SLEEP_MAX([200], [enabled], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([0], [0], [200], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([3], [1], [400], [+$LINENUM])
+CHECK_PMD_SLEEP_MAX([5], [8], [200], [+$LINENUM])
+
+
+AT_CHECK([ovs-appctl dpif-netdev/pmd-sleep-show], [0], [dnl
+PMD max sleep request is 200 usecs by default.
+PMD load 

[ovs-dev] [PATCH 5/6] dpif-netdev: Add per pmd sleep config.

2023-06-14 Thread Kevin Traynor
Extend 'pmd-sleep-max' so that individual PMD thread cores
may have a specified max sleep request value.

Any PMD thread core without a value will use the datapath default
(no sleep request) or datapath global value set by the user.

To set PMD thread cores 8 and 9 to never request a load based sleep
and all other PMD thread cores to be able to request a max sleep of
50 usecs:

$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50,8:0,9:0

To set PMD thread cores 10 and 11 to request a max sleep of 100 usecs
and all other PMD thread cores to never request a sleep:

$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=10:100,11:100

'pmd-sleep-show' can be used to dump the global and individual PMD thread
core max sleep request values.

Signed-off-by: Kevin Traynor 
---
 Documentation/topics/dpdk/pmd.rst |  23 +++
 lib/dpif-netdev-private-thread.h  |   3 +
 lib/dpif-netdev.c | 225 --
 tests/pmd.at  |  32 ++---
 4 files changed, 252 insertions(+), 31 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index 40e6b7843..eafcbc504 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -375,4 +375,27 @@ system configuration (e.g. enabling processor C-states) 
and workloads.
 rate.
 
+Max sleep request values can be set for individual PMDs using key:value pairs.
+Any PMD that has been assigned a specified value will use that. Any PMD that
+does not have a specified value will use the current global default.
+
+Specified values for individual PMDs can be added or removed at any time.
+
+For example, to set PMD thread cores 8 and 9 to never request a load based
+sleep and all others PMD cores to be able to request a max sleep of 50 usecs::
+
+$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50,8:0,9:0
+
+The max sleep request for each PMD can be checked in the logs or with::
+
+$ ovs-appctl dpif-netdev/pmd-sleep-show
+PMD max sleep request is 50 usecs by default.
+PMD load based sleeps are enabled by default.
+PMD thread core   8 NUMA  0: Max sleep request set to0 usecs.
+PMD thread core   9 NUMA  1: Max sleep request set to0 usecs.
+PMD thread core  10 NUMA  0: Max sleep request set to   50 usecs.
+PMD thread core  11 NUMA  1: Max sleep request set to   50 usecs.
+PMD thread core  12 NUMA  0: Max sleep request set to   50 usecs.
+PMD thread core  13 NUMA  1: Max sleep request set to   50 usecs.
+
 .. _ovs-vswitchd(8):
 http://openvswitch.org/support/dist-docs/ovs-vswitchd.8.html
diff --git a/lib/dpif-netdev-private-thread.h b/lib/dpif-netdev-private-thread.h
index 1ec3cd794..5c72ce5d9 100644
--- a/lib/dpif-netdev-private-thread.h
+++ b/lib/dpif-netdev-private-thread.h
@@ -181,4 +181,7 @@ struct dp_netdev_pmd_thread {
 bool isolated;
 
+/* Max sleep request. UINT64_MAX indicates dp default should be used.*/
+atomic_uint64_t max_sleep;
+
 /* Queue id used by this pmd thread to send packets on all netdevs if
  * XPS disabled for this netdev. All static_tx_qid's are unique and less
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index dadf17b70..ee0137ef1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -180,4 +180,9 @@ static struct odp_support dp_netdev_support = {
 #define PMD_SLEEP_INC_US 1
 
+struct pmd_sleep {
+unsigned core_id;
+uint64_t max_sleep;
+};
+
 struct dpcls {
 struct cmap_node node;  /* Within dp_netdev_pmd_thread.classifiers */
@@ -290,4 +295,6 @@ struct dp_netdev {
 /* Max load based sleep request. */
 atomic_uint64_t pmd_max_sleep;
+/* Max load based sleep request user string. */
+char *max_sleep_list;
 /* Enable the SMC cache from ovsdb config */
 atomic_bool smc_enable_db;
@@ -890,9 +897,15 @@ pmd_max_sleep_show(struct ds *reply, struct 
dp_netdev_pmd_thread *pmd,
 {
 if (pmd->core_id != NON_PMD_CORE_ID) {
+uint64_t pmd_max_sleep;
+
+atomic_read_relaxed(>max_sleep, _max_sleep);
 ds_put_format(reply,
   "PMD thread core %3u NUMA %2d: "
   "Max sleep request set to",
   pmd->core_id, pmd->numa_id);
-ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
+ds_put_format(reply, " %4"PRIu64" usecs.",
+  pmd_max_sleep == UINT64_MAX
+  ? default_max_sleep
+  : pmd_max_sleep);
 ds_put_cstr(reply, "\n");
 }
@@ -1528,7 +1541,8 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
 ds_put_format(, "PMD max sleep request is %"PRIu64" "
-  "usecs.", default_max_sleep);
+  "usecs by default.", default_max_sleep);
 ds_put_cstr(, "\n");
-

[ovs-dev] [PATCH 3/6] dpif-netdev: Add pmd-sleep-show command.

2023-06-14 Thread Kevin Traynor
Max requested sleep time and status for a PMD thread
is logged at start up or when changed, but it can be
convenient to have a command to dump this information
explicitly.

It is envisaged that this will be expanded when future
additions are added.

Signed-off-by: Kevin Traynor 
---
 Documentation/topics/dpdk/pmd.rst |  4 
 NEWS  |  2 ++
 lib/dpif-netdev.c | 40 +++
 tests/pmd.at  | 29 ++
 4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index b261e9254..bedd42194 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -349,4 +349,8 @@ time not processing packets will be determined by the sleep 
and processor
 wake-up times and should be tested with each system configuration.
 
+The max sleep time that a PMD thread may request can be shown with::
+
+$ ovs-appctl dpif-netdev/pmd-sleep-show
+
 Sleep time statistics for 10 secs can be seen with::
 
diff --git a/NEWS b/NEWS
index 1ccc6f948..32b9e8167 100644
--- a/NEWS
+++ b/NEWS
@@ -39,4 +39,6 @@ Post-v3.1.0
- Userspace datapath:
  * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
+ * Add 'ovs-appctl dpif-netdev/pmd-sleep-show' command to get the
+   max sleep request setting of PMD thread cores.
 
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 94c8ca943..dadf17b70 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -702,4 +702,5 @@ enum pmd_info_type {
 PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
 PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
+PMD_INFO_MAX_SLEEP_SHOW,   /* Show max sleep performance details. */
 };
 
@@ -884,4 +885,18 @@ sorted_poll_list(struct dp_netdev_pmd_thread *pmd, struct 
rxq_poll **list,
 }
 
+static void
+pmd_max_sleep_show(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
+   uint64_t default_max_sleep)
+{
+if (pmd->core_id != NON_PMD_CORE_ID) {
+ds_put_format(reply,
+  "PMD thread core %3u NUMA %2d: "
+  "Max sleep request set to",
+  pmd->core_id, pmd->numa_id);
+ds_put_format(reply, " %4"PRIu64" usecs.", default_max_sleep);
+ds_put_cstr(reply, "\n");
+}
+}
+
 static void
 pmd_info_show_rxq(struct ds *reply, struct dp_netdev_pmd_thread *pmd,
@@ -1442,5 +1457,6 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 unsigned long long max_secs = (PMD_INTERVAL_LEN * PMD_INTERVAL_MAX)
   / INTERVAL_USEC_TO_SEC;
-bool first_show_rxq = true;
+bool first_pmd = true;
+uint64_t default_max_sleep = 0;
 
 ovs_mutex_lock(_netdev_mutex);
@@ -1490,5 +1506,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 }
 if (type == PMD_INFO_SHOW_RXQ) {
-if (first_show_rxq) {
+if (first_pmd) {
 if (!secs || secs > max_secs) {
 secs = max_secs;
@@ -1499,5 +1515,5 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 ds_put_format(, "Displaying last %u seconds "
   "pmd usage %%\n", secs);
-first_show_rxq = false;
+first_pmd = false;
 }
 pmd_info_show_rxq(, pmd, secs);
@@ -1508,4 +1524,16 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int 
argc, const char *argv[],
 } else if (type == PMD_INFO_PERF_SHOW) {
 pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
+} else if (type == PMD_INFO_MAX_SLEEP_SHOW) {
+if (first_pmd) {
+atomic_read_relaxed(>pmd_max_sleep, _max_sleep);
+ds_put_format(, "PMD max sleep request is %"PRIu64" "
+  "usecs.", default_max_sleep);
+ds_put_cstr(, "\n");
+ds_put_format(, "PMD load based sleeps are %s.",
+  default_max_sleep ? "enabled" : "disabled");
+ds_put_cstr(, "\n");
+first_pmd = false;
+}
+pmd_max_sleep_show(, pmd, default_max_sleep);
 }
 }
@@ -1608,5 +1636,6 @@ dpif_netdev_init(void)
 static enum pmd_info_type show_aux = PMD_INFO_SHOW_STATS,
   clear_aux = PMD_INFO_CLEAR_STATS,
-  poll_aux = PMD_INFO_SHOW_RXQ;
+  poll_aux = PMD_INFO_SHOW_RXQ,
+  sleep_aux = PMD_INFO_MAX_SLEEP_SHOW;
 
 unixctl_command_register("dpif-netdev/pmd-stats-show", "[-pmd core] [dp]",
@@ -1620,4 +1649,7 @@ dpif_netdev_init(void)
  0, 5, dpif_netdev_pmd_info,
  (void *)_aux);
+

[ovs-dev] [PATCH 2/6] pmd.at: Add macro for checking pmd sleep max time and state.

2023-06-14 Thread Kevin Traynor
This is just cosmetic. There is no change to the tests.

Signed-off-by: Kevin Traynor 
---
 tests/pmd.at | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/tests/pmd.at b/tests/pmd.at
index 374ad7217..64d8f6e2b 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -61,4 +61,20 @@ m4_define([CHECK_PMD_THREADS_CREATED], [
 ])
 
+dnl CHECK_DP_SLEEP_MAX([max_sleep], [enabled], [+line])
+dnl
+dnl Checks correct pmd load based sleep is set for the datapath.
+dnl Checking starts from line number 'line' in ovs-vswithd.log .
+m4_define([CHECK_DP_SLEEP_MAX], [
+SLEEP_TIME="PMD max sleep request is $1 usecs."
+SLEEP_STATE="PMD load based sleeps are $2."
+line_st=$4
+if [[ -z "$line_st" ]]
+then
+line_st="+0"
+fi
+OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_TIME"])
+OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "$SLEEP_STATE"])
+])
+
 m4_define([SED_NUMA_CORE_PATTERN], ["s/\(numa_id \)[[0-9]]*\( core_id 
\)[[0-9]]*:/\1\2:/"])
 m4_define([DUMMY_NUMA], [--dummy-numa="0,0,0,0"])
@@ -1256,46 +1272,39 @@ OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-dnl Check default state
 AT_SETUP([PMD - pmd sleep])
 OVS_VSWITCHD_START
 
 dnl Check default
-OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD max sleep request is 0 
usecs."])
-OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load based sleeps are 
disabled."])
+CHECK_DP_SLEEP_MAX([0], [disabled], [])
 
 dnl Check low value max sleep
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
+CHECK_DP_SLEEP_MAX([1], [enabled], [+$LINENUM])
 
 dnl Check high value max sleep
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
+CHECK_DP_SLEEP_MAX([1], [enabled], [+$LINENUM])
 
 dnl Check setting max sleep to zero
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="0"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 0 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are disabled."])
+CHECK_DP_SLEEP_MAX([0], [disabled], [+$LINENUM])
 
 dnl Check above high value max sleep
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10001"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
+CHECK_DP_SLEEP_MAX([1], [enabled], [+$LINENUM])
 
 dnl Check rounding
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="490"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 490 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
+CHECK_DP_SLEEP_MAX([490], [enabled], [+$LINENUM])
+
 dnl Check rounding
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="499"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 499 usecs."])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
+CHECK_DP_SLEEP_MAX([499], [enabled], [+$LINENUM])
 
 OVS_VSWITCHD_STOP
-- 
2.40.1

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


[ovs-dev] [PATCH 1/6] dpif-netdev: Rename pmd-maxsleep config option.

2023-06-14 Thread Kevin Traynor
other_config:pmd-maxsleep is a config option to allow
PMD thread cores to sleep under low or no load conditions.

Rename it to 'pmd-sleep-max' to allow a more structured
name and so that additional options or command can follow
the 'pmd-sleep-xyz' pattern.

Signed-off-by: Kevin Traynor 
---
 Documentation/topics/dpdk/pmd.rst |  2 +-
 NEWS  |  2 ++
 lib/dpif-netdev.c |  2 +-
 tests/pmd.at  | 12 ++--
 vswitchd/vswitch.xml  |  2 +-
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/topics/dpdk/pmd.rst 
b/Documentation/topics/dpdk/pmd.rst
index e70986d16..b261e9254 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -335,5 +335,5 @@ This can be enabled by setting the max requested sleep time 
(in microseconds)
 for a PMD thread::
 
-$ ovs-vsctl set open_vswitch . other_config:pmd-maxsleep=50
+$ ovs-vsctl set open_vswitch . other_config:pmd-sleep-max=50
 
 With a non-zero max value a PMD may request to sleep by an incrementing amount
diff --git a/NEWS b/NEWS
index cfd43..1ccc6f948 100644
--- a/NEWS
+++ b/NEWS
@@ -37,4 +37,6 @@ Post-v3.1.0
- SRv6 Tunnel Protocol
  * Added support for userspace datapath (only).
+   - Userspace datapath:
+ * Rename 'pmd-maxsleep' other_config to 'pmd-sleep-max'.
 
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 70b953ae6..94c8ca943 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4983,5 +4983,5 @@ dpif_netdev_set_config(struct dpif *dpif, const struct 
smap *other_config)
 set_pmd_auto_lb(dp, autolb_state, log_autolb);
 
-pmd_max_sleep = smap_get_ullong(other_config, "pmd-maxsleep", 0);
+pmd_max_sleep = smap_get_ullong(other_config, "pmd-sleep-max", 0);
 pmd_max_sleep = MIN(PMD_RCU_QUIESCE_INTERVAL, pmd_max_sleep);
 atomic_read_relaxed(>pmd_max_sleep, _pmd_max_sleep);
diff --git a/tests/pmd.at b/tests/pmd.at
index 48f3d432d..374ad7217 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -1266,5 +1266,5 @@ OVS_WAIT_UNTIL([tail ovs-vswitchd.log | grep "PMD load 
based sleeps are disabled
 dnl Check low value max sleep
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="1"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
@@ -1272,5 +1272,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep 
"PMD load based sleeps
 dnl Check high value max sleep
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="1"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="1"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
@@ -1278,5 +1278,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep 
"PMD load based sleeps
 dnl Check setting max sleep to zero
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="0"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="0"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 0 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are disabled."])
@@ -1284,5 +1284,5 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep 
"PMD load based sleeps
 dnl Check above high value max sleep
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="10001"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="10001"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 1 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
@@ -1290,10 +1290,10 @@ OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | 
grep "PMD load based sleeps
 dnl Check rounding
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="490"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="490"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 490 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
 dnl Check rounding
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-maxsleep="499"])
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-sleep-max="499"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD max sleep 
request is 499 usecs."])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD load based 
sleeps are enabled."])
diff --git 

[ovs-dev] [PATCH 0/6] PMD load based sleep updates and per-pmd config.

2023-06-14 Thread Kevin Traynor
Patches 1-4 are about polishing the existing functionality
and preparing for new functionality later in the series.

Patch 1 renames 'pmd-maxsleep' to 'pmd-sleep-max'. I think
this is allowed as it is still experimental, but we can keep
'pmd-maxsleep' as well, if people think it's a problem.

Patches 5-6 adds a new per pmd config option functionality and tests.
These can be considered separate from patches 1-4 but as there
are dependencies, it made sense to add the patches to this
patch set.

GHA: https://github.com/kevintraynor/ovs/actions/runs/5267778815

Kevin Traynor (6):
  dpif-netdev: Rename pmd-maxsleep config option.
  pmd.at: Add macro for checking pmd sleep max time and state.
  dpif-netdev: Add pmd-sleep-show command.
  dpif-netdev: Remove pmd-sleep-max experimental tag.
  dpif-netdev: Add per pmd sleep config.
  pmd-at: Add per pmd max sleep unit tests.

 Documentation/topics/dpdk/pmd.rst |  33 +++-
 NEWS  |   5 +
 lib/dpif-netdev-private-thread.h  |   3 +
 lib/dpif-netdev.c | 259 --
 tests/pmd.at  | 233 ---
 vswitchd/vswitch.xml  |   2 +-
 6 files changed, 494 insertions(+), 41 deletions(-)

-- 
2.40.1

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


Re: [ovs-dev] [PATCH v2] python: Add aync DNS support

2023-06-14 Thread Ilya Maximets
On 5/18/23 05:50, Terry Wilson wrote:
> This adds a Python version of the async DNS support added in:
> 
> 771680d96 DNS: Add basic support for asynchronous DNS resolving
> 
> The above version uses the unbound C library, and this
> implimentation uses the SWIG-wrapped Python version of that.
> 
> In the event that the Python unbound library is not available,
> a warning will be logged and the resolve() method will just
> return None. For the case where inet_parse_active() is passed
> an IP address, it will not try to resolve it, so existing
> behavior should be preserved in the case that the unbound
> library is unavailable.
> 
> Intentional differences from the C version are as follows:
> 
>   OVS_HOSTS_FILE environment variable can bet set to override
>   the system 'hosts' file. This is primarily to allow testing to
>   be done without requiring network connectivity.
> 
>   Since resolution can still be done via hosts file lookup, DNS
>   lookups are not disabled when resolv.conf cannot be loaded.
> 
>   The Python socket_util module has fallen behind its C equivalent.
>   The bare minimum change was done to inet_parse_active() to support
>   sync/async dns, as there is no equivalent to
>   parse_sockaddr_components(), inet_parse_passive(), etc. A TODO
>   was added to bring socket_util.py up to equivalency to the C
>   version.
> 
> Signed-off-by: Terry Wilson 
> ---

Thanks, Terry!  The patch looks good in general, except for a
few things that Adrian pointed out in his review.

See a couple additional comments below.

>  .github/workflows/build-and-test.yml |   4 +-
>  debian/control.in|   1 +
>  python/TODO.rst  |   7 +
>  python/automake.mk   |   2 +
>  python/ovs/dns_resolve.py| 257 
>  python/ovs/socket_util.py|  21 +-
>  python/ovs/stream.py |   2 +-
>  python/ovs/tests/test_dns_resolve.py | 280 +++
>  python/setup.py  |   6 +-
>  rhel/openvswitch-fedora.spec.in  |   2 +-
>  tests/vlog.at|   2 +
>  11 files changed, 575 insertions(+), 9 deletions(-)
>  create mode 100644 python/ovs/dns_resolve.py
>  create mode 100644 python/ovs/tests/test_dns_resolve.py
> 



> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py
> new file mode 100644
> index 0..463a39277
> --- /dev/null
> +++ b/python/ovs/dns_resolve.py
> @@ -0,0 +1,257 @@

Generally, each file with a code should have a copyright header
with a license boilerplate.

It's probably less important for test files, but that is what
our current coding style document is asking to do.

> +import collections
> +import functools
> +import ipaddress
> +import os
> +import threading
> +import time
> +import typing
> +
> +try:
> +import unbound  # type: ignore
> +except ImportError:
> +pass
> +
> +import ovs.vlog
> +
> +vlog = ovs.vlog.Vlog("dns_resolve")
> +
> +
> +class DNSRequest:
> +INVALID = 0
> +PENDING = 1
> +GOOD = 2
> +ERROR = 3
> +
> +def __init__(self, name):
> +self.name = name
> +self.state = self.INVALID
> +self.time = None
> +self.result = None  # set by DNSResolver._callback
> +self.ttl = None
> +
> +@property
> +def expired(self):
> +return time.time() > self.time + self.ttl
> +
> +@property
> +def is_valid(self):
> +return self.state == self.GOOD and not self.expired
> +
> +def __str__(self):
> +return (f"DNSRequest(name={self.name}, state={self.state}, "
> +f"time={self.time}, result={self.result})")
> +
> +
> +class DefaultReqDict(collections.defaultdict):
> +def __init__(self):
> +super().__init__(DNSRequest)
> +
> +def __missing__(self, key):
> +ret = self.default_factory(key)
> +self[key] = ret
> +return ret
> +
> +
> +class UnboundException(Exception):
> +
> +def __init__(self, message, errno):
> +try:
> +msg = f"{message}: {unbound.ub_strerror(errno)}"
> +except NameError:
> +msg = message
> +super().__init__(msg)
> +
> +
> +def dns_enabled(func):
> +@functools.wraps(func)
> +def wrapper(self, *args, **kwargs):
> +if self.dns_enabled:
> +return func(self, *args, **kwargs)
> +vlog.err("DNS support requires the python unbound library")
> +return wrapper
> +
> +
> +class singleton:
> +def __init__(self, klass):
> +self._klass = klass
> +self._instance = None
> +
> +def __call__(self, *args, **kwargs):
> +if self._instance is None:
> +self._instance = self._klass(*args, **kwargs)
> +return self._instance
> +
> +
> +@singleton
> +class DNSResolver:
> +def __init__(self, is_daemon: bool = False):
> +"""Create a resolver instance
> +
> +If is_daemon is true, set the resolver to handle requests
> +  

Re: [ovs-dev] [PATCH v4 8/9] netdev-tc-offloads: Probe for allowing vxlan gbp support

2023-06-14 Thread Ilya Maximets
On 6/14/23 10:22, Roi Dayan wrote:
> From: Gavin Li 
> 
> Kernels that do not support vxlan gbp would treat the rule that has vxlan
> gbp encap action or vxlan gbp id match differently, either reject it or
> just skip the action/match and continue processing the knowing ones.
> 
> To solve the issue, probe and disallow inserting rules with vxlan gbp
> action/match if kernel does not support it.
> 
> Signed-off-by: Gavin Li 
> Reviewed-by: Roi Dayan 
> ---
>  lib/netdev-offload-tc.c | 64 +++--
>  1 file changed, 61 insertions(+), 3 deletions(-)
> 



> @@ -2788,6 +2800,51 @@ probe_tc_block_support(int ifindex)
>  }
>  }
>  
> +static void
> +probe_vxlan_gbp_support(int ifindex)
> +{
> +struct tc_flower flower;
> +struct tcf_id id;
> +int block_id = 0;
> +int prio = 1;
> +int error;
> +
> +error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS);
> +if (error) {
> +return;
> +}
> +
> +memset(, 0, sizeof flower);
> +
> +flower.tc_policy = TC_POLICY_SKIP_HW;
> +flower.key.eth_type = htons(ETH_P_IP);
> +flower.mask.eth_type = OVS_BE16_MAX;
> +flower.tunnel = true;
> +flower.mask.tunnel.id = 1;
> +flower.mask.tunnel.ipv4.ipv4_src = 1;
> +flower.mask.tunnel.ipv4.ipv4_dst = 1;
> +flower.mask.tunnel.tp_dst = 1;
> +flower.mask.tunnel.gbp.id = 512;
> +flower.key.tunnel.ipv4.ipv4_src = 117901063;
> +flower.key.tunnel.ipv4.ipv4_dst = 134678279;
> +flower.key.tunnel.tp_dst = 46354;
> +flower.key.tunnel.gbp.id = 512;

Most of the fields above are defined as restricted BE types.
Assigning plain integers breaks the build with sparse.
You can see GHA failures on this patch.  As reported by the
ovsrobot:
  https://mail.openvswitch.org/pipermail/ovs-build/2023-June/031588.html

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


[ovs-dev] [PATCH v6 ovn] northd: centralized reply lb traffic even if FIP is defined

2023-06-14 Thread Lorenzo Bianconi
In the current codebase for distributed gw router port use-case,
it is not possible to add a load balancer that redirects the traffic
to a back-end if it is used as internal IP of a FIP NAT rule since
the reply traffic is never centralized. Fix the issue centralizing the
traffic if it is the reply packet for the load balancer.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2023609
Acked-by: Mark Michelson 
Reviewed-by: Simon Horman 
Signed-off-by: Lorenzo Bianconi 
---
Changes since v5:
- rely on fmt_pkt macro in unit-tests
Changes since v4:
- improve memory footprint
Changes since v3:
- add ovn unit-test and get rid of system-ovn unit-test.
- minor changes in ovn-northd unit-test.
Changes since v2:
- rebase on top of ovn main branch
- fix spelling
Changes since v1:
- add new system-test and unit-test
---
 northd/northd.c |  20 ++-
 northd/ovn-northd.8.xml |  16 ++
 tests/ovn-northd.at |  50 +
 tests/ovn.at| 117 
 4 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/northd/northd.c b/northd/northd.c
index d073c0b75..93ac644ad 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11232,6 +11232,7 @@ struct lrouter_nat_lb_flows_ctx {
 
 struct ds *new_match;
 struct ds *undnat_match;
+struct ds *gw_redir_action;
 
 struct ovn_lb_vip *lb_vip;
 struct ovn_northd_lb *lb;
@@ -11299,6 +11300,20 @@ build_distr_lrouter_nat_flows_for_lb(struct 
lrouter_nat_lb_flows_ctx *ctx,
 return;
 }
 
+/* We need to centralize the LB traffic to properly perform
+ * the undnat stage.
+ */
+ds_put_format(ctx->undnat_match, ") && outport == %s", dgp->json_key);
+ds_clear(ctx->gw_redir_action);
+ds_put_format(ctx->gw_redir_action, "outport = %s; next;",
+  dgp->cr_port->json_key);
+
+ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_IN_GW_REDIRECT,
+200, ds_cstr(ctx->undnat_match),
+ds_cstr(ctx->gw_redir_action),
+>lb->nlb->header_);
+ds_truncate(ctx->undnat_match, undnat_match_len);
+
 ds_put_format(ctx->undnat_match, ") && (inport == %s || outport == %s)"
   " && is_chassis_resident(%s)", dgp->json_key, dgp->json_key,
   dgp->cr_port->json_key);
@@ -11369,6 +11384,7 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
 struct ds force_snat_act = DS_EMPTY_INITIALIZER;
 struct ds undnat_match = DS_EMPTY_INITIALIZER;
 struct ds unsnat_match = DS_EMPTY_INITIALIZER;
+struct ds gw_redir_action = DS_EMPTY_INITIALIZER;
 
 ds_clear(match);
 ds_clear(action);
@@ -11427,7 +11443,8 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
 .lflows = lflows,
 .meter_groups = meter_groups,
 .new_match = match,
-.undnat_match = _match
+.undnat_match = _match,
+.gw_redir_action = _redir_action,
 };
 
 ctx.new_action[LROUTER_NAT_LB_FLOW_NORMAL] = ds_cstr(action);
@@ -11521,6 +11538,7 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
 ds_destroy(_match);
 ds_destroy(_snat_act);
 ds_destroy(_snat_act);
+ds_destroy(_redir_action);
 
 for (size_t i = 0; i < LROUTER_NAT_LB_FLOW_MAX + 2; i++) {
 bitmap_free(dp_bitmap[i]);
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 8164be300..532c428e9 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -4582,6 +4582,22 @@ icmp6 {
 
 
 
+  
+For all the configured load balancing rules that include an IPv4
+address VIP, and a list of IPv4 backend addresses
+B0, B1 .. Bn defined for the
+VIP a priority-200 flow is added that matches 
+ip4  (ip4.src == B0 || ip4.src == B1
+|| ... || ip4.src == Bn) with an action 
+outport = CR; next; where CR is the
+chassisredirect port representing the instance of the
+logical router distributed gateway port on the gateway chassis.
+If the backend IPv4 address Bx is also configured with
+L4 port PORT of protocol P, then the match
+also includes P.src == PORT.
+Similar flows are added for IPv6.
+  
+
   
 For each NAT rule in the OVN Northbound database that can
 be handled in a distributed manner, a priority-100 logical
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 9f2f15abc..22c124f90 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9541,3 +9541,53 @@ AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE 
inc-engine/show-stats northd recom
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([check fip and lb flows])
+AT_KEYWORDS([fip-lb-flows])
+ovn_start
+
+check ovn-nbctl lr-add R1
+check ovn-nbctl lrp-add R1 R1-S0 02:ac:10:01:00:01 10.0.0.1/24 1000::a/64
+check ovn-nbctl lrp-add R1 R1-PUB 02:ac:20:01:01:01 172.16.0.1/24 

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

2023-06-14 Thread Robin Jarry
Ilya Maximets, Jun 14, 2023 at 13:21:
> While files are present in tabs-allowed it doesn't mean that
> all of them actually use tabs.  I'm not sure about other
> folders, but at least the windows include folder contains files
> with different styles.
>
> Is it possible to just exclude the folder instead of specifying
> a format?

No but I can list only the files which already contain tabs.

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


Re: [ovs-dev] [PATCH v4 9/9] system-offloads-traffic.at: Add vxlan gbp offload test

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 11:22:39AM +0300, Roi Dayan wrote:
> From: Gavin Li 
> 
> Add a vxlan gbp offload test case:
> 
>   vxlan offloads with gbp extention - ping between two ports - offloads
> enabled ok
> 
> Signed-off-by: Gavin Li 

Reviewed-by: Simon Horman 

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


Re: [ovs-dev] [PATCH v4 8/9] netdev-tc-offloads: Probe for allowing vxlan gbp support

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 11:22:38AM +0300, Roi Dayan wrote:
> From: Gavin Li 
> 
> Kernels that do not support vxlan gbp would treat the rule that has vxlan
> gbp encap action or vxlan gbp id match differently, either reject it or
> just skip the action/match and continue processing the knowing ones.
> 
> To solve the issue, probe and disallow inserting rules with vxlan gbp
> action/match if kernel does not support it.
> 
> Signed-off-by: Gavin Li 
> Reviewed-by: Roi Dayan 

Reviewed-by: Simon Horman 

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


Re: [ovs-dev] [PATCH v4 7/9] tc: Add vxlan encap action with gbp option offload

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 11:22:37AM +0300, Roi Dayan wrote:
> From: Gavin Li 
> 
> Add TC offload support for vxlan encap with gbp option
> 
> Signed-off-by: Gavin Li 
> Reviewed-by: Gavi Teitz 
> Reviewed-by: Roi Dayan 

Reviewed-by: Simon Horman 

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


Re: [ovs-dev] [PATCH v4 5/9] tc: Add vxlan gbp option flower match offload

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 11:22:35AM +0300, Roi Dayan wrote:
> From: Gavin Li 
> 
> Add TC offload support for filtering vxlan tunnels with gbp option
> 
> Signed-off-by: Gavin Li 
> Reviewed-by: Gavi Teitz 
> Reviewed-by: Roi Dayan 

Reviewed-by: Simon Horman 

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


Re: [ovs-dev] [PATCH v4 4/9] netlink: Add new function to add NLA_F_NESTED to nested netlink messages

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 11:22:34AM +0300, Roi Dayan wrote:
> From: Gavin Li 
> 
> Linux kernel netlink module added NLA_F_NESTED flag checking for nested
> netlink messages in 5.2. A nested message without the flag set will be
> treated as malformated one. The check is optional and is controlled by
> message policy. To avoid this, add NLA_F_NESTED explicitly for all
> nested netlink messages with a new function
> nl_msg_start_nested_with_flag().
> 
> Signed-off-by: Gavin Li 
> Reviewed-by: Roi Dayan 

Reviewed-by: Simon Horman 

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


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

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 02:34:39AM +0800, James Raphael Tiovalen wrote:
> This commit adds a few null pointer assertions and checks to some return
> values of `ovsdb_table_schema_get_column`. If a null pointer is
> encountered in these blocks, either the assertion will fail or the
> control flow will now be redirected to alternative paths which will
> output the appropriate error messages.
> 
> A few ovsdb-rbac and ovsdb-server tests are also updated to verify the
> expected warning logs by adding said logs to the ALLOWLIST of the
> OVSDB_SERVER_SHUTDOWN statements.
> 
> Signed-off-by: James Raphael Tiovalen 

Reviewed-by: Simon Horman 

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


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

2023-06-14 Thread Simon Horman
On Wed, Jun 14, 2023 at 02:34:37AM +0800, James Raphael Tiovalen wrote:
> This commit adds zero-initializations by changing `SFL_ALLOC` from
> `malloc` to `xzalloc`, adding a `memset` call to `sflAlloc`,
> initializing a `pollfd` struct variable with zeroes, and changing some
> calls to `xmalloc` to `xzalloc`. This is to prevent potential data leaks
> or undefined behavior from potentially uninitialized variables.
> 
> Some variables would always be initialized by either the code flow or
> the compiler. Thus, some of the associated Coverity reports might be
> false positives. That said, it is still considered best practice to
> zero-initialize variables upfront just in case to ensure the overall
> resilience and security of OVS, as long as they do not impact
> performance-critical code. As a bonus, it would also make static
> analyzer tools, such as Coverity, happy.
> 
> Signed-off-by: James Raphael Tiovalen 

Reviewed-by: Simon Horman 

I do see a failure in the Intel test run [1][2],
but it seems unrelated to this patch.

[1] 
https://patchwork.ozlabs.org/project/openvswitch/patch/20230613183443.31540-3-jamestio...@gmail.com/
[2] https://mail.openvswitch.org/pipermail/ovs-build/2023-June/031563.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [ovs-dev v2] dpif-netdev: fix dpif_netdev_flow_put

2023-06-14 Thread Simon Horman
On Tue, Jun 13, 2023 at 09:41:42AM +0800, Peng He wrote:
> Hi, Simon,
> 
> I guess it's not related, as my patch fixes the code in dpif-netdev.c it
> should has any impact on the ovsdb side.
> But I am not sure that.
> Is this test case a spike test case? Is there any testsuite log?

* Please don't top post

* I am unsure what a spike test is

* The link I provided leads to logs

  [1] 
https://github.com/ovsrobot/ovs/actions/runs/5238809864/jobs/9458063244#step:11:5573
  [2] https://github.com/ovsrobot/ovs/suites/13526199944/artifacts/743672747

* It seems that my report was inaccurate, sorry about that.

  As pointed out by Ilya,
  it was not the "compacting online - cluster that failed",
  but rather the test that is added by your patch

  [3] https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405546.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3] conntrack: Extract l4 information for SCTP.

2023-06-14 Thread Ilya Maximets
On 6/12/23 16:57, Aaron Conole wrote:
> Paolo Valerio  writes:
> 
>> since a27d70a89 ("conntrack: add generic IP protocol support") all
>> the unrecognized IP protocols get handled using ct_proto_other ops
>> and are managed as L3 using 3 tuples.
>>
>> This patch stores L4 information for SCTP in the conn_key so that
>> multiple conn instances, instead of one with ports zeroed, will be
>> created when there are multiple SCTP connections between two hosts.
>> It also performs crc32c check when not offloaded, and adds SCTP to
>> pat_enabled.
>>
>> With this patch, given two SCTP association between two hosts,
>> tracking the connection will result in:
>>
>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=55884,dport=5201),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5201,dport=12345),zone=1
>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=59874,dport=5202),reply=(src=10.1.1.1,dst=10.1.1.2,sport=5202,dport=12346),zone=1
>>
>> instead of:
>>
>> sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=0,dport=0),reply=(src=10.1.1.1,dst=10.1.1.2,sport=0,dport=0),zone=1
>>
>> Signed-off-by: Paolo Valerio 
>> ---
> 
> Thanks for this work - I think it looks good.
> 
> Perhaps it should have a NEWS item mentioned that the userspace
> conntrack now supports matching SCTP l4 data.
> 
> If you do spin a v4 with that change, you can keep my:
> 
> Acked-by: Aaron Conole 

Hi, Paolo and Aaron.

I'm getting a consistent test failure while running check-kernel
on Ubuntu 22.10 with 5.19 kernel:


./system-traffic.at:4754: cat ofctl_monitor.log
--- -   2023-06-14 11:26:41.958591125 +
+++ /root/ovs/tests/system-kmod-testsuite.dir/at-groups/105/stdout  
2023-06-14 11:26:41.95200 +
@@ -12,8 +12,6 @@
 
sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
 sctp_csum:9b67e853
 NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=54 in_port=1 (via action) 
data_len=54 (unbuffered)
 
sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
 sctp_csum:bc0e5463
-NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=50 
ct_state=est|rpl|trk|dnat,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=132,ct_tp_src=54969,ct_tp_dst=12345,ip,in_port=2
 (via action) data_len=50 (unbuffered)
-sctp,vlan_tci=0x,dl_src=e6:66:c1:22:22:22,dl_dst=e6:66:c1:11:11:11,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=12345,tp_dst=54969
 sctp_csum:d6ce6b9e
 NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=50 in_port=1 (via action) 
data_len=50 (unbuffered)
-sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.240,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=34567,tp_dst=12345
 sctp_csum:add7db93
+sctp,vlan_tci=0x,dl_src=e6:66:c1:11:11:11,dl_dst=e6:66:c1:22:22:22,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=2,nw_ttl=64,nw_frag=no,tp_src=54969,tp_dst=12345
 sctp_csum:5db68ce


Do you know what can be a problem here?

Test is passing on Fedora 38 with 6.3 kernel and on rhel 9.2.

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


Re: [ovs-dev] [PATCH v2] python: Add aync DNS support

2023-06-14 Thread Adrian Moreno

Hi Terry,

Thanks for the patch. I have some minor comments and questions inline but 
overall it looks pretty good to me.


On 5/18/23 05:50, Terry Wilson wrote:

This adds a Python version of the async DNS support added in:

771680d96 DNS: Add basic support for asynchronous DNS resolving

The above version uses the unbound C library, and this
implimentation uses the SWIG-wrapped Python version of that.

In the event that the Python unbound library is not available,
a warning will be logged and the resolve() method will just
return None. For the case where inet_parse_active() is passed
an IP address, it will not try to resolve it, so existing
behavior should be preserved in the case that the unbound
library is unavailable.

Intentional differences from the C version are as follows:

   OVS_HOSTS_FILE environment variable can bet set to override
   the system 'hosts' file. This is primarily to allow testing to
   be done without requiring network connectivity.

   Since resolution can still be done via hosts file lookup, DNS
   lookups are not disabled when resolv.conf cannot be loaded.

   The Python socket_util module has fallen behind its C equivalent.
   The bare minimum change was done to inet_parse_active() to support
   sync/async dns, as there is no equivalent to
   parse_sockaddr_components(), inet_parse_passive(), etc. A TODO
   was added to bring socket_util.py up to equivalency to the C
   version.

Signed-off-by: Terry Wilson 
---
  .github/workflows/build-and-test.yml |   4 +-
  debian/control.in|   1 +
  python/TODO.rst  |   7 +
  python/automake.mk   |   2 +
  python/ovs/dns_resolve.py| 257 
  python/ovs/socket_util.py|  21 +-
  python/ovs/stream.py |   2 +-
  python/ovs/tests/test_dns_resolve.py | 280 +++
  python/setup.py  |   6 +-
  rhel/openvswitch-fedora.spec.in  |   2 +-
  tests/vlog.at|   2 +
  11 files changed, 575 insertions(+), 9 deletions(-)
  create mode 100644 python/ovs/dns_resolve.py
  create mode 100644 python/ovs/tests/test_dns_resolve.py

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index f66ab43b0..47d239f10 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -183,10 +183,10 @@ jobs:
run:  sudo apt update || true
  - name: install common dependencies
run:  sudo apt install -y ${{ env.dependencies }}
-- name: install libunbound libunwind
+- name: install libunbound libunwind python3-unbound
# GitHub Actions doesn't have 32-bit versions of these libraries.
if:   matrix.m32 == ''
-  run:  sudo apt install -y libunbound-dev libunwind-dev
+  run:  sudo apt install -y libunbound-dev libunwind-dev python3-unbound
  - name: install 32-bit libraries
if:   matrix.m32 != ''
run:  sudo apt install -y gcc-multilib
diff --git a/debian/control.in b/debian/control.in
index 19f590d06..64b0a4ce0 100644
--- a/debian/control.in
+++ b/debian/control.in
@@ -287,6 +287,7 @@ Depends:
  Suggests:
   python3-netaddr,
   python3-pyparsing,
+ python3-unbound,
  Description: Python 3 bindings for Open vSwitch
   Open vSwitch is a production quality, multilayer, software-based,
   Ethernet virtual switch. It is designed to enable massive network
diff --git a/python/TODO.rst b/python/TODO.rst
index 3a53489f1..acc5461e2 100644
--- a/python/TODO.rst
+++ b/python/TODO.rst
@@ -32,3 +32,10 @@ Python Bindings To-do List
  
* Support write-only-changed monitor mode (equivalent of

  OVSDB_IDL_WRITE_CHANGED_ONLY).
+
+* socket_util:
+
+  * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components,
+et al. to better support using async dns. The reconnect code will
+currently log a warning when inet_parse_active() returns w/o yet having
+resolved an address, but will continue to connect and eventually succeed.
diff --git a/python/automake.mk b/python/automake.mk
index d00911828..82a508787 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -16,6 +16,7 @@ ovs_pyfiles = \
python/ovs/compat/sortedcontainers/sorteddict.py \
python/ovs/compat/sortedcontainers/sortedset.py \
python/ovs/daemon.py \
+   python/ovs/dns_resolve.py \
python/ovs/db/__init__.py \
python/ovs/db/custom_index.py \
python/ovs/db/data.py \
@@ -55,6 +56,7 @@ ovs_pyfiles = \
  
  ovs_pytests = \

python/ovs/tests/test_decoders.py \
+   python/ovs/tests/test_dns_resolve.py \
python/ovs/tests/test_filter.py \
python/ovs/tests/test_kv.py \
python/ovs/tests/test_list.py \
diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py
new file mode 100644
index 0..463a39277
--- /dev/null
+++ b/python/ovs/dns_resolve.py
@@ -0,0 +1,257 @@
+import collections
+import 

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

2023-06-14 Thread Ilya Maximets
On 6/13/23 14:25, Robin Jarry wrote:
> EditorConfig is a file format and collection of text editor plugins for
> maintaining consistent coding styles between different editors and IDEs.
> 
> Initialize the file following the coding rules in
> Documentation/internals/contributing/coding-style.rst and add exceptions
> declared in build-aux/initial-tab-allowed-files. Only enforce rules for
> *.c and *.h files. Other files should use the default indenting rules
> from text editors.
> 
> In order for this file to be taken into account (unless they use an
> editor with built-in EditorConfig support), developers will have to
> install a plugin.
> 
> Notes:
> 
> * All matching rules are considered. The last matching rule's properties
>   will override the previous ones.
> * The max_line_length property is only supported by a limited number of
>   EditorConfig plugins. It will be ignored if unsupported.
> 
> Link: https://editorconfig.org/
> Link: https://github.com/editorconfig/editorconfig-emacs
> Link: https://github.com/editorconfig/editorconfig-vim
> Link: 
> https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
> Signed-off-by: Robin Jarry 
> Acked-by: Mike Pattrick 
> Acked-by: Eelco Chaudron 

The patch changed significantly, I don't think you can keep the tags.

> Cc: Ilya Maximets 
> ---
> 
> Notes:
> v3: added exceptions for some *.{c,h} files that are using tabs
> 
>  .editorconfig | 43 +++
>  Makefile.am   |  1 +
>  2 files changed, 44 insertions(+)
>  create mode 100644 .editorconfig
> 
> diff --git a/.editorconfig b/.editorconfig
> new file mode 100644
> index ..6d1842ca0f35
> --- /dev/null
> +++ b/.editorconfig
> @@ -0,0 +1,43 @@
> +# See https://editorconfig.org/ for syntax reference.
> +
> +root = true
> +
> +[*]
> +end_of_line = lf
> +insert_final_newline = true
> +trim_trailing_whitespace = true
> +charset = utf-8
> +
> +[*.{c,h}]
> +indent_style = space
> +indent_size = 4
> +max_line_length = 79
> +
> +[include/linux/**.h]
> +indent_style = tab
> +indent_size = tab
> +tab_width = 8
> +
> +[include/sparse/rte_*.h]
> +indent_style = tab
> +tab_width = 8
> +
> +[include/windows/**.h]
> +indent_style = tab
> +indent_size = tab
> +tab_width = 8

While files are present in tabs-allowed it doesn't mean that
all of them actually use tabs.  I'm not sure about other
folders, but at least the windows include folder contains files
with different styles.

Is it possible to just exclude the folder instead of specifying
a format?

> +
> +[lib/getopt_long.c]
> +indent_style = tab
> +indent_size = tab
> +tab_width = 8
> +
> +[lib/sflow*.{c,h}]
> +indent_style = tab
> +indent_size = tab
> +tab_width = 8
> +
> +[lib/strsep.c]
> +indent_style = tab
> +indent_size = tab
> +tab_width = 8
> diff --git a/Makefile.am b/Makefile.am
> index df9c33dfe631..db341504d37f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -82,6 +82,7 @@ EXTRA_DIST = \
>   .ci/osx-build.sh \
>   .ci/osx-prepare.sh \
>   .cirrus.yml \
> + .editorconfig \
>   .github/workflows/build-and-test.yml \
>   appveyor.yml \
>   boot.sh \

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


Re: [ovs-dev] [PATCH ovn v3] ovn-northd.at: Fix unstable LSP incremental processing test.

2023-06-14 Thread Dumitru Ceara
On 6/10/23 20:10, Han Zhou wrote:
> The test case is unstable because there are many factors that can impact
> the number of recomputes. For example, when northd updates both NB and
> SB, both sides have in-flight transactions, and if the SB notification
> comes back before the NB transaction (for updating nb_cfg) completes,
> ovn-northd falls back to recompute because idl_txn for NB is not
> available, while if the NB notification comes back first it won't
> trigger recompute because the nb_cfg columns are omitted for alert.
> 
> To avoid the instability of the test case, this patch modifies it to
> verify the best case (also the most common scenario), by running the
> test multiple (10) times and making sure at least 70% of the time all
> the recompute counters match expectation.
> 
> (The recompute triggered by in-flight transactions will be improved
> later and the success rate in the test can be adjusted accordingly.)
> 
> Reported-by: Dumitru Ceara 
> Signed-off-by: Han Zhou 
> ---
> v1 -> v2: wait ports up and sync after each port binding, to avoid race.
> v2 -> v3: refactor and test case and use a probability based test strategy
> to ensure the test stability (see commit message).

Neat, thanks for fixing this!  The only thing with this approach is that
the test case takes quite long to execute 30s on my system.  Luckily we
don't have too many tests like this so:

Acked-by: Dumitru Ceara 

Regards,
Dumitru

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


Re: [ovs-dev] [PATCH ovn branch-22.03] Revert "call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy"

2023-06-14 Thread Dumitru Ceara
On 6/14/23 11:36, Igor Zhukov wrote:
> Sure.
> 
> Acked-by: Igor Zhukov 
> 

Thanks, I pushed this to branch-22.03.

Regards,
Dumitru

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


Re: [ovs-dev] [PATCH ovn branch-22.03] Revert "call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy"

2023-06-14 Thread Igor Zhukov
Sure.

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


Re: [ovs-dev] [PATCH ovn] call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy

2023-06-14 Thread Igor Zhukov
I agree.

Yes, we needed to check the build on version 22.03.

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


Re: [ovs-dev] [PATCH ovn branch-22.03] Revert "call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy"

2023-06-14 Thread Dumitru Ceara
On 6/14/23 11:19, Igor Zhukov wrote:
> I agree.
> 
> Yes, we needed to check the build on version 22.03.
> 
> Sorry.
> 

Can I consider this a formal "Acked-by:" and push the patch to unblock CI?

Thanks,
Dumitru

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


Re: [ovs-dev] [PATCH ovn branch-22.03] Revert "call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy"

2023-06-14 Thread Igor Zhukov
I agree.

Yes, we needed to check the build on version 22.03.

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


Re: [ovs-dev] [PATCH ovn] call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy

2023-06-14 Thread Dumitru Ceara
On 6/9/23 21:13, Mark Michelson wrote:
> I pushed this change to main and all branches back to 22.03. Thanks!
> 

Branch 22.03 doesn't have run_update_worker_pool() so this backport
broke compilation there.  I posted a 22.03-only revert patch:

https://patchwork.ozlabs.org/project/ovn/patch/20230614091231.463313-1-dce...@redhat.com/

Regards,
Dumitru

> On 6/7/23 15:21, Mark Michelson wrote:
>> Thanks, Igor.
>>
>> Acked-by: Mark Michelson 
>>
>> On 6/6/23 20:37, Igor Zhukov wrote:
>>> From: Igor Zhukov 
>>>
>>> You can check logs by running: make check-valgrind TESTSUITEFLAGS="246"
>>>
>>> (Actually almost every test affected but for example we need one test)
>>>
>>> Valgrind message looks like
>>>
>>> ==65437== 304 bytes in 1 blocks are possibly lost in loss record 265
>>> of 289
>>> ==65437==    at 0x483DD99: calloc (in
>>> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==65437==    by 0x40149DA: allocate_dtv (dl-tls.c:286)
>>> ==65437==    by 0x40149DA: _dl_allocate_tls (dl-tls.c:532)
>>> ==65437==    by 0x4BE2322: allocate_stack (allocatestack.c:622)
>>> ==65437==    by 0x4BE2322: pthread_create@@GLIBC_2.2.5
>>> (pthread_create.c:660)
>>> ==65437==    by 0x283D46: ovs_thread_create (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x27FFB0: ovsrcu_quiesced (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x28000B: ovsrcu_quiesce_start (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x2AFE04: time_poll (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x2A16A2: poll_block (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x165019: pinctrl_handler (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x283BB1: ovsthread_wrapper (in
>>> /home/ivzhukov/upstream/ovn/controller/ovn-controller)
>>> ==65437==    by 0x4BE1608: start_thread (pthread_create.c:477)
>>> ==65437==    by 0x4F5F132: clone (clone.S:95)
>>>
>>> or
>>>
>>> ==63926== 304 bytes in 1 blocks are possibly lost in loss record 128
>>> of 130
>>> ==63926==    at 0x483DD99: calloc (in
>>> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==63926==    by 0x40149DA: allocate_dtv (dl-tls.c:286)
>>> ==63926==    by 0x40149DA: _dl_allocate_tls (dl-tls.c:532)
>>> ==63926==    by 0x4BE2322: allocate_stack (allocatestack.c:622)
>>> ==63926==    by 0x4BE2322: pthread_create@@GLIBC_2.2.5
>>> (pthread_create.c:660)
>>> ==63926==    by 0x24EE4B: ovs_thread_create (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x24CB63: ovsrcu_quiesced (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x24CBBE: ovsrcu_quiesce_start (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x276DE6: time_poll (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x26BA03: poll_block (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x26A9FD: stopwatch_thread (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x24ECB6: ovsthread_wrapper (in
>>> /home/ivzhukov/upstream/ovn/northd/ovn-northd)
>>> ==63926==    by 0x4BE1608: start_thread (pthread_create.c:477)
>>> ==63926==    by 0x4F5F132: clone (clone.S:95)
>>>
>>> I also noticed that ovs-vswitchd calls ovsrcu_exit() before exit:
>>> https://github.com/openvswitch/ovs/blob/0187eadfce4505d502e57c0e688b830f0a1ec728/vswitchd/ovs-vswitchd.c#L150
>>>
>>> I also stopped a thread pool by calling run_update_worker_pool(0);
>>> because ovsrcu_exit(); stucks without it.
>>>
>>> Signed-off-by: Igor Zhukov 
>>> ---
>>>   controller/ovn-controller.c | 1 +
>>>   northd/ovn-northd.c | 3 +++
>>>   2 files changed, 4 insertions(+)
>>>
>>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>>> index 3a81a13fb..a47406979 100644
>>> --- a/controller/ovn-controller.c
>>> +++ b/controller/ovn-controller.c
>>> @@ -5620,6 +5620,7 @@ loop_done:
>>>   free(cli_system_id);
>>>   }
>>>   service_stop();
>>> +    ovsrcu_exit();
>>>   exit(retval);
>>>   }
>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>>> index 3515b68a2..de807c0a4 100644
>>> --- a/northd/ovn-northd.c
>>> +++ b/northd/ovn-northd.c
>>> @@ -33,6 +33,7 @@
>>>   #include "lib/ovn-l7.h"
>>>   #include "lib/ovn-nb-idl.h"
>>>   #include "lib/ovn-sb-idl.h"
>>> +#include "lib/ovs-rcu.h"
>>>   #include "openvswitch/poll-loop.h"
>>>   #include "simap.h"
>>>   #include "stopwatch.h"
>>> @@ -1030,6 +1031,8 @@ main(int argc, char *argv[])
>>>   ovsdb_idl_loop_destroy(_idl_loop);
>>>   ovsdb_idl_loop_destroy(_idl_loop);
>>>   service_stop();
>>> +    run_update_worker_pool(0);
>>> +    ovsrcu_exit();
>>>   exit(res);
>>>   }
>>
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 

[ovs-dev] [PATCH ovn branch-22.03] Revert "call ovsrcu_exit() before exit in ovn-northd and ovn-controller to make valgrind happy"

2023-06-14 Thread Dumitru Ceara
This reverts commit 6849811833b5a40137288145dc2c3e4ac22f90fd.

The function run_update_worker_pool() doesn't exist on branches <=
22.03 as the parallelization code changed in newer branches.  There's no
need to complicate things just to make valgrind happy.

Fixes: 6849811833b5 ("call ovsrcu_exit() before exit in ovn-northd and 
ovn-controller to make valgrind happy")
Signed-off-by: Dumitru Ceara 
---
 controller/ovn-controller.c | 1 -
 northd/ovn-northd.c | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 307d0fcd57..fbd7a780b1 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -4277,7 +4277,6 @@ loop_done:
 ovs_feature_support_destroy();
 free(ovs_remote);
 service_stop();
-ovsrcu_exit();
 
 exit(retval);
 }
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 731876cee8..4e18519fad 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -32,7 +32,6 @@
 #include "lib/ovn-l7.h"
 #include "lib/ovn-nb-idl.h"
 #include "lib/ovn-sb-idl.h"
-#include "lib/ovs-rcu.h"
 #include "openvswitch/poll-loop.h"
 #include "simap.h"
 #include "stopwatch.h"
@@ -952,8 +951,6 @@ main(int argc, char *argv[])
 ovsdb_idl_loop_destroy(_idl_loop);
 ovsdb_idl_loop_destroy(_idl_loop);
 service_stop();
-run_update_worker_pool(0);
-ovsrcu_exit();
 
 exit(res);
 }
-- 
2.31.1

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


[ovs-dev] [PATCH v4 9/9] system-offloads-traffic.at: Add vxlan gbp offload test

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Add a vxlan gbp offload test case:

  vxlan offloads with gbp extention - ping between two ports - offloads
enabled ok

Signed-off-by: Gavin Li 
---
 tests/system-offloads-traffic.at | 49 
 1 file changed, 49 insertions(+)

diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index ae302a29499a..cabbcfbd168f 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -805,3 +805,52 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device 
ovs-p0/d
 /failed to offload flow/d
 "])
 AT_CLEANUP
+
+AT_SETUP([offloads - ping over vxlan tunnel with gbp - offloads enabled])
+OVS_CHECK_TUNNEL_TSO()
+OVS_CHECK_VXLAN()
+
+OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])
+ADD_BR([br-underlay])
+
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and with a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24], 
[options:exts=gbp])
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=br0 
actions=load:0x200->NXM_NX_TUN_GBP_ID[], output:at_vxlan0]")
+AT_CHECK([ovs-ofctl add-flow br0 "in_port=at_vxlan0, tun_gbp_id=512 
actions=output:br0"])
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+
+ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
[10.1.1.1/24],
+  [id 0 dstport 4789 gbp])
+NS_CHECK_EXEC([at_ns0], [iptables -I OUTPUT -p ip -j MARK --set-mark 512 
2>/dev/null], [0])
+NS_CHECK_EXEC([at_ns0], [iptables -I INPUT -m mark --mark 512 -j ACCEPT 
2>/dev/null], [0], [ignore])
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now check the overlay with different packet sizes
+NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 10.1.1.100 | FORMAT_PING], 
[0], [dnl
+1000 packets transmitted, 1000 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep 
"eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512))" | wc -l], [0], [dnl
+1
+])
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep 
"eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512,flags=0))" | wc -l], 
[0], [dnl
+1
+])
+
+dnl First, check the underlay
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
-- 
2.38.0

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


[ovs-dev] [PATCH v4 8/9] netdev-tc-offloads: Probe for allowing vxlan gbp support

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Kernels that do not support vxlan gbp would treat the rule that has vxlan
gbp encap action or vxlan gbp id match differently, either reject it or
just skip the action/match and continue processing the knowing ones.

To solve the issue, probe and disallow inserting rules with vxlan gbp
action/match if kernel does not support it.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
---
 lib/netdev-offload-tc.c | 64 +++--
 1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index a32ed8021c4b..ed82d8707430 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -52,6 +52,7 @@ static struct hmap tc_to_ufid = HMAP_INITIALIZER(_to_ufid);
 static bool multi_mask_per_prio = false;
 static bool block_support = false;
 static uint16_t ct_state_support;
+static bool vxlan_gbp_support = false;
 
 struct netlink_field {
 int offset;
@@ -668,7 +669,7 @@ static void parse_tc_flower_geneve_opts(struct tc_action 
*action,
 nl_msg_end_nested(buf, geneve_off);
 }
 
-static void
+static int
 parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
   struct ofpbuf *buf)
 {
@@ -676,7 +677,10 @@ parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
 uint32_t gbp_raw;
 
 if (!action->encap.gbp.id_present) {
-return;
+return 0;
+}
+if (!vxlan_gbp_support) {
+return -EOPNOTSUPP;
 }
 
 gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
@@ -684,6 +688,7 @@ parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
  action->encap.gbp.id);
 nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
 nl_msg_end_nested(buf, gbp_off);
+return 0;
 }
 
 static void
@@ -846,6 +851,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, 
struct ofpbuf *buf,
 size_t set_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_SET);
 size_t tunnel_offset =
 nl_msg_start_nested(buf, OVS_KEY_ATTR_TUNNEL);
+int ret;
 
 if (action->encap.id_present) {
 nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID, action->encap.id);
@@ -881,7 +887,10 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, 
struct ofpbuf *buf,
 if (!action->encap.no_csum) {
 nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM);
 }
-parse_tc_flower_vxlan_tun_opts(action, buf);
+ret = parse_tc_flower_vxlan_tun_opts(action, buf);
+if (ret) {
+return ret;
+}
 parse_tc_flower_geneve_opts(action, buf);
 nl_msg_end_nested(buf, tunnel_offset);
 nl_msg_end_nested(buf, set_offset);
@@ -1633,6 +1642,9 @@ parse_put_flow_set_action(struct tc_flower *flower, 
struct tc_action *action,
 }
 break;
 case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: {
+if (!vxlan_gbp_support) {
+return EOPNOTSUPP;
+}
 if (odp_vxlan_tun_opts_from_attr(tun_attr,
  >encap.gbp.id,
  >encap.gbp.flags,
@@ -2788,6 +2800,51 @@ probe_tc_block_support(int ifindex)
 }
 }
 
+static void
+probe_vxlan_gbp_support(int ifindex)
+{
+struct tc_flower flower;
+struct tcf_id id;
+int block_id = 0;
+int prio = 1;
+int error;
+
+error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS);
+if (error) {
+return;
+}
+
+memset(, 0, sizeof flower);
+
+flower.tc_policy = TC_POLICY_SKIP_HW;
+flower.key.eth_type = htons(ETH_P_IP);
+flower.mask.eth_type = OVS_BE16_MAX;
+flower.tunnel = true;
+flower.mask.tunnel.id = 1;
+flower.mask.tunnel.ipv4.ipv4_src = 1;
+flower.mask.tunnel.ipv4.ipv4_dst = 1;
+flower.mask.tunnel.tp_dst = 1;
+flower.mask.tunnel.gbp.id = 512;
+flower.key.tunnel.ipv4.ipv4_src = 117901063;
+flower.key.tunnel.ipv4.ipv4_dst = 134678279;
+flower.key.tunnel.tp_dst = 46354;
+flower.key.tunnel.gbp.id = 512;
+
+id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS);
+error = tc_replace_flower(, );
+if (error) {
+goto out;
+}
+
+tc_del_flower_filter();
+
+vxlan_gbp_support = true;
+VLOG_INFO("probe tc: vxlan gbp is supported.");
+
+out:
+tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS);
+}
+
 static int
 tc_get_policer_action_ids(struct hmap *map)
 {
@@ -2915,6 +2972,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
 
 probe_multi_mask_per_prio(ifindex);
 probe_ct_state_support(ifindex);
+probe_vxlan_gbp_support(ifindex);
 
 ovs_mutex_lock(_police_ids_mutex);
 meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE,
-- 
2.38.0

___
dev mailing list
d...@openvswitch.org

[ovs-dev] [PATCH v4 7/9] tc: Add vxlan encap action with gbp option offload

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Add TC offload support for vxlan encap with gbp option

Signed-off-by: Gavin Li 
Reviewed-by: Gavi Teitz 
Reviewed-by: Roi Dayan 
---
 acinclude.m4 |  7 
 include/linux/tc_act/tc_tunnel_key.h | 17 +++-
 lib/netdev-offload-tc.c  | 31 ++-
 lib/odp-util.c   | 41 ++--
 lib/odp-util.h   |  3 ++
 lib/tc.c | 58 
 lib/tc.h |  1 +
 7 files changed, 143 insertions(+), 15 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index ac1eab790041..690a13c25967 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -191,6 +191,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
 [AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
[Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
 
+  AC_COMPILE_IFELSE([
+AC_LANG_PROGRAM([#include ], [
+int x = TCA_TUNNEL_KEY_ENC_OPTS_VXLAN;
+])],
+[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN], [1],
+   [Define to 1 if TCA_TUNNEL_KEY_ENC_OPTS_VXLAN is available.])])
+
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
 int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
diff --git a/include/linux/tc_act/tc_tunnel_key.h 
b/include/linux/tc_act/tc_tunnel_key.h
index f13acf17dd75..17291b90bf3c 100644
--- a/include/linux/tc_act/tc_tunnel_key.h
+++ b/include/linux/tc_act/tc_tunnel_key.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
 #define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
 
-#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
+#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN)
 #include_next 
 #else
 
@@ -53,6 +53,10 @@ enum {
 * TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
 * attributes
 */
+   TCA_TUNNEL_KEY_ENC_OPTS_VXLAN,  /* Nested
+* TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
+* attributes
+*/
__TCA_TUNNEL_KEY_ENC_OPTS_MAX,
 };
 
@@ -70,6 +74,15 @@ enum {
 #define TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX \
(__TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX - 1)
 
-#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
+enum {
+   TCA_TUNNEL_KEY_ENC_OPT_VXLAN_UNSPEC,
+   TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP,   /* u32 */
+   __TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX,
+};
+
+#define TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX \
+   (__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX - 1)
+
+#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN */
 
 #endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1c97681bc92b..a32ed8021c4b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -668,6 +668,24 @@ static void parse_tc_flower_geneve_opts(struct tc_action 
*action,
 nl_msg_end_nested(buf, geneve_off);
 }
 
+static void
+parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
+  struct ofpbuf *buf)
+{
+size_t gbp_off;
+uint32_t gbp_raw;
+
+if (!action->encap.gbp.id_present) {
+return;
+}
+
+gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
+gbp_raw = odp_encode_gbp_raw(action->encap.gbp.flags,
+ action->encap.gbp.id);
+nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
+nl_msg_end_nested(buf, gbp_off);
+}
+
 static void
 flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
 {
@@ -863,7 +881,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower, 
struct ofpbuf *buf,
 if (!action->encap.no_csum) {
 nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM);
 }
-
+parse_tc_flower_vxlan_tun_opts(action, buf);
 parse_tc_flower_geneve_opts(action, buf);
 nl_msg_end_nested(buf, tunnel_offset);
 nl_msg_end_nested(buf, set_offset);
@@ -1552,6 +1570,7 @@ parse_put_flow_set_action(struct tc_flower *flower, 
struct tc_action *action,
 
 action->type = TC_ACT_ENCAP;
 action->encap.id_present = false;
+action->encap.gbp.id_present = false;
 action->encap.no_csum = 1;
 flower->action_count++;
 NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
@@ -1613,6 +1632,16 @@ parse_put_flow_set_action(struct tc_flower *flower, 
struct tc_action *action,
 action->encap.data.present.len = nl_attr_get_size(tun_attr);
 }
 break;
+case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: {
+if (odp_vxlan_tun_opts_from_attr(tun_attr,
+ >encap.gbp.id,
+ >encap.gbp.flags,
+ >encap.gbp.id_present)) {
+

[ovs-dev] [PATCH v4 5/9] tc: Add vxlan gbp option flower match offload

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Add TC offload support for filtering vxlan tunnels with gbp option

Signed-off-by: Gavin Li 
Reviewed-by: Gavi Teitz 
Reviewed-by: Roi Dayan 
---
 include/linux/pkt_cls.h | 13 ++
 lib/netdev-offload-tc.c | 17 
 lib/tc.c| 92 +++--
 lib/tc.h|  7 
 4 files changed, 117 insertions(+), 12 deletions(-)

diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index a8cd8db5bf88..fb4a7ecea4cc 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -273,6 +273,10 @@ enum {
 * TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
 * attributes
 */
+   TCA_FLOWER_KEY_ENC_OPTS_VXLAN,  /* Nested
+* TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
+* attributes
+*/
__TCA_FLOWER_KEY_ENC_OPTS_MAX,
 };
 
@@ -290,6 +294,15 @@ enum {
 #define TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX \
(__TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX - 1)
 
+enum {
+   TCA_FLOWER_KEY_ENC_OPT_VXLAN_UNSPEC,
+   TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP,   /* u32 */
+   __TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX,
+};
+
+#define TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX \
+   (__TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX - 1)
+
 enum {
TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 4f26dd8cca5f..1c97681bc92b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1234,6 +1234,15 @@ parse_tc_flower_to_match(const struct netdev *netdev,
 match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst,
 flower->mask.tunnel.tp_dst);
 }
+if (flower->mask.tunnel.gbp.id) {
+match_set_tun_gbp_id_masked(match, flower->key.tunnel.gbp.id,
+flower->mask.tunnel.gbp.id);
+}
+if (flower->mask.tunnel.gbp.flags) {
+match_set_tun_gbp_flags_masked(match,
+   flower->key.tunnel.gbp.flags,
+   flower->mask.tunnel.gbp.flags);
+}
 
 if (!strcmp(netdev_get_type(netdev), "geneve")) {
 flower_tun_opt_to_match(match, flower);
@@ -2193,6 +2202,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.key.tunnel.ttl = tnl->ip_ttl;
 flower.key.tunnel.tp_src = tnl->tp_src;
 flower.key.tunnel.tp_dst = tnl->tp_dst;
+flower.key.tunnel.gbp.id = tnl->gbp_id;
+flower.key.tunnel.gbp.flags = tnl->gbp_flags;
+flower.key.tunnel.gbp.id_present = !!tnl_mask->gbp_id;
 
 flower.mask.tunnel.ipv4.ipv4_src = tnl_mask->ip_src;
 flower.mask.tunnel.ipv4.ipv4_dst = tnl_mask->ip_dst;
@@ -2207,6 +2219,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
  * Degrading the flow down to exact match for now as a workaround. */
 flower.mask.tunnel.tp_dst = OVS_BE16_MAX;
 flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? 
tnl_mask->tun_id : 0;
+flower.mask.tunnel.gbp.id = tnl_mask->gbp_id;
+flower.mask.tunnel.gbp.flags = tnl_mask->gbp_flags;
+flower.mask.tunnel.gbp.id_present = !!tnl_mask->gbp_id;
 
 memset(_mask->ip_src, 0, sizeof tnl_mask->ip_src);
 memset(_mask->ip_dst, 0, sizeof tnl_mask->ip_dst);
@@ -2218,6 +2233,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 memset(_mask->tp_dst, 0, sizeof tnl_mask->tp_dst);
 
 memset(_mask->tun_id, 0, sizeof tnl_mask->tun_id);
+memset(_mask->gbp_id, 0, sizeof tnl_mask->gbp_id);
+memset(_mask->gbp_flags, 0, sizeof tnl_mask->gbp_flags);
 tnl_mask->flags &= ~FLOW_TNL_F_KEY;
 
 /* XXX: This is wrong!  We're ignoring DF and CSUM flags configuration
diff --git a/lib/tc.c b/lib/tc.c
index 223fe6e5e5e9..ae1ca57c9d2a 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -39,6 +39,7 @@
 #include "coverage.h"
 #include "netlink-socket.h"
 #include "netlink.h"
+#include "odp-util.h"
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/util.h"
 #include "openvswitch/vlog.h"
@@ -699,6 +700,38 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr,
 return 0;
 }
 
+static int
+nl_parse_vxlan_key(const struct nlattr *in_nlattr,
+   struct tc_flower_tunnel *tunnel)
+{
+const struct ofpbuf *msg;
+struct nlattr *nla;
+struct ofpbuf buf;
+uint32_t gbp_raw;
+size_t left;
+
+nl_attr_get_nested(in_nlattr, );
+msg = 
+
+NL_ATTR_FOR_EACH (nla, left, ofpbuf_at(msg, 0, 0), msg->size) {
+uint16_t type = nl_attr_type(nla);
+
+switch (type) {
+case TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP:

[ovs-dev] [PATCH v4 6/9] tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Most of the data members of struct tc_action{ } are defined as anonymous
struct in place. Instead of passing all members of an anonymous struct,
which is not flexible to new members being added, expose encap as named
struct and pass it entirely.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---
 lib/tc.c | 57 +++-
 lib/tc.h | 38 +++--
 2 files changed, 43 insertions(+), 52 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index ae1ca57c9d2a..7434b0150f74 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2641,13 +2641,9 @@ nl_msg_put_act_tunnel_geneve_option(struct ofpbuf 
*request,
 }
 
 static void
-nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
-  ovs_be64 id, ovs_be32 ipv4_src,
-  ovs_be32 ipv4_dst, struct in6_addr *ipv6_src,
-  struct in6_addr *ipv6_dst,
-  ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
-  struct tun_metadata *tun_metadata,
-  uint8_t no_csum, uint32_t action_pc)
+nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
+  struct tc_action_encap *encap,
+  uint32_t action_pc)
 {
 size_t offset;
 
@@ -2659,30 +2655,33 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, 
bool id_present,
 
 nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, , sizeof tun);
 
-ovs_be32 id32 = be64_to_be32(id);
-if (id_present) {
+ovs_be32 id32 = be64_to_be32(encap->id);
+if (encap->id_present) {
 nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
 }
-if (ipv4_dst) {
-nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC, ipv4_src);
-nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST, ipv4_dst);
-} else if (ipv6_addr_is_set(ipv6_dst)) {
+if (encap->ipv4.ipv4_dst) {
+nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC,
+encap->ipv4.ipv4_src);
+nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST,
+encap->ipv4.ipv4_dst);
+} else if (ipv6_addr_is_set(>ipv6.ipv6_dst)) {
 nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_DST,
-ipv6_dst);
+>ipv6.ipv6_dst);
 nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_SRC,
-ipv6_src);
+>ipv6.ipv6_src);
 }
-if (tos) {
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, tos);
+if (encap->tos) {
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, encap->tos);
 }
-if (ttl) {
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, ttl);
+if (encap->ttl) {
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, encap->ttl);
 }
-if (tp_dst) {
-nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT, tp_dst);
+if (encap->tp_dst) {
+nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT,
+encap->tp_dst);
 }
-nl_msg_put_act_tunnel_geneve_option(request, tun_metadata);
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, no_csum);
+nl_msg_put_act_tunnel_geneve_option(request, >data);
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, encap->no_csum);
 }
 nl_msg_end_nested(request, offset);
 }
@@ -3305,17 +3304,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct 
tc_flower *flower)
 }
 
 act_offset = nl_msg_start_nested(request, act_index++);
-nl_msg_put_act_tunnel_key_set(request, 
action->encap.id_present,
-  action->encap.id,
-  action->encap.ipv4.ipv4_src,
-  action->encap.ipv4.ipv4_dst,
-  >encap.ipv6.ipv6_src,
-  >encap.ipv6.ipv6_dst,
-  action->encap.tp_dst,
-  action->encap.tos,
-  action->encap.ttl,
-  >encap.data,
-  action->encap.no_csum,
+nl_msg_put_act_tunnel_key_set(request, >encap,
   action_pc);
 nl_msg_put_act_flags(request);
 nl_msg_end_nested(request, act_offset);
diff --git a/lib/tc.h b/lib/tc.h
index 95fff37b9b61..1d648282a004 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -210,6 +210,25 @@ enum 

[ovs-dev] [PATCH v4 4/9] netlink: Add new function to add NLA_F_NESTED to nested netlink messages

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Linux kernel netlink module added NLA_F_NESTED flag checking for nested
netlink messages in 5.2. A nested message without the flag set will be
treated as malformated one. The check is optional and is controlled by
message policy. To avoid this, add NLA_F_NESTED explicitly for all
nested netlink messages with a new function
nl_msg_start_nested_with_flag().

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
---
 lib/netlink.c | 9 +
 lib/netlink.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/lib/netlink.c b/lib/netlink.c
index 6215282d6fbe..1e8d5a8ec57d 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -523,6 +523,15 @@ nl_msg_start_nested(struct ofpbuf *msg, uint16_t type)
 return offset;
 }
 
+/* Adds the header for nested Netlink attributes to 'msg', with the specified
+ * 'type', and returns the header's offset within 'msg'. It's similar to
+ * nl_msg_start_nested() and uses NLA_F_NESTED flag mandatorily. */
+size_t
+nl_msg_start_nested_with_flag(struct ofpbuf *msg, uint16_t type)
+{
+return nl_msg_start_nested(msg, type | NLA_F_NESTED);
+}
+
 /* Finalizes a nested Netlink attribute in 'msg'.  'offset' should be the value
  * returned by nl_msg_start_nested(). */
 void
diff --git a/lib/netlink.h b/lib/netlink.h
index e9050c31bacd..008604aa60d1 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -81,6 +81,7 @@ void nl_msg_put_string__(struct ofpbuf *, uint16_t type, 
const char *value,
 void nl_msg_put_string(struct ofpbuf *, uint16_t type, const char *value);
 
 size_t nl_msg_start_nested(struct ofpbuf *, uint16_t type);
+size_t nl_msg_start_nested_with_flag(struct ofpbuf *, uint16_t type);
 void nl_msg_end_nested(struct ofpbuf *, size_t offset);
 void nl_msg_cancel_nested(struct ofpbuf *, size_t offset);
 bool nl_msg_end_non_empty_nested(struct ofpbuf *, size_t offset);
-- 
2.38.0

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


[ovs-dev] [PATCH v4 2/9] odp-util: Extract vxlan gbp option decoding to a function

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Extract vxlan gbp option decoding to odp_decode_gbp_raw to be used in
following commits.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---
 lib/odp-util.c | 9 +++--
 lib/odp-util.h | 8 
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 2ec889c417e5..f62dc86c5f9e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3162,8 +3162,7 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool 
is_mask,
 if (ext[OVS_VXLAN_EXT_GBP]) {
 uint32_t gbp = nl_attr_get_u32(ext[OVS_VXLAN_EXT_GBP]);
 
-tun->gbp_id = htons(gbp & 0x);
-tun->gbp_flags = (gbp >> 16) & 0xFF;
+odp_decode_gbp_raw(gbp, >gbp_id, >gbp_flags);
 }
 
 break;
@@ -3753,12 +3752,10 @@ format_odp_tun_vxlan_opt(const struct nlattr *attr,
 ovs_be16 id, id_mask;
 uint8_t flags, flags_mask = 0;
 
-id = htons(key & 0x);
-flags = (key >> 16) & 0xFF;
+odp_decode_gbp_raw(key, , );
 if (ma) {
 uint32_t mask = nl_attr_get_u32(ma);
-id_mask = htons(mask & 0x);
-flags_mask = (mask >> 16) & 0xFF;
+odp_decode_gbp_raw(mask, _mask, _mask);
 }
 
 ds_put_cstr(ds, "gbp(");
diff --git a/lib/odp-util.h b/lib/odp-util.h
index a1d0d0fba5de..cf762bdc3547 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -374,6 +374,14 @@ void odp_put_push_eth_action(struct ofpbuf *odp_actions,
  const struct eth_addr *eth_src,
  const struct eth_addr *eth_dst);
 
+static inline void odp_decode_gbp_raw(uint32_t gbp_raw,
+  ovs_be16 *id,
+  uint8_t *flags)
+{
+*id = htons(gbp_raw & 0x);
+*flags = (gbp_raw >> 16) & 0xFF;
+}
+
 struct attr_len_tbl {
 int len;
 const struct attr_len_tbl *next;
-- 
2.38.0

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


[ovs-dev] [PATCH v4 3/9] odp-util: Extract vxlan gbp option encoding to a function

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Extract vxlan gbp option encoding to odp_encode_gbp_raw to be used in
following commits.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---
 lib/odp-util.c | 5 +++--
 lib/odp-util.h | 5 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index f62dc86c5f9e..d2414eb559ba 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3278,10 +3278,11 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl 
*tun_key,
 if ((!tnl_type || !strcmp(tnl_type, "vxlan")) &&
 (tun_key->gbp_flags || tun_key->gbp_id)) {
 size_t vxlan_opts_ofs;
+uint32_t gbp_raw;
 
 vxlan_opts_ofs = nl_msg_start_nested(a, 
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
-nl_msg_put_u32(a, OVS_VXLAN_EXT_GBP,
-   (tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id));
+gbp_raw = odp_encode_gbp_raw(tun_key->gbp_flags, tun_key->gbp_id);
+nl_msg_put_u32(a, OVS_VXLAN_EXT_GBP, gbp_raw);
 nl_msg_end_nested(a, vxlan_opts_ofs);
 }
 
diff --git a/lib/odp-util.h b/lib/odp-util.h
index cf762bdc3547..163efe7a87b5 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -382,6 +382,11 @@ static inline void odp_decode_gbp_raw(uint32_t gbp_raw,
 *flags = (gbp_raw >> 16) & 0xFF;
 }
 
+static inline uint32_t odp_encode_gbp_raw(uint8_t flags, ovs_be16 id)
+{
+return (flags << 16) | ntohs(id);
+}
+
 struct attr_len_tbl {
 int len;
 const struct attr_len_tbl *next;
-- 
2.38.0

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


[ovs-dev] [PATCH v4 1/9] tc: Pass tunnel entirely to tunnel option parse and put functions

2023-06-14 Thread Roi Dayan via dev
From: Gavin Li 

Tc flower tunnel key options were encoded in nl_msg_put_flower_tunnel_opts
and decoded in nl_parse_flower_tunnel_opts. Only geneve was supported.

To avoid adding more arguments to the function to support more vxlan
options in the future, change the function arguments to pass tunnel
entirely to it instead of keep adding new arguments.

Signed-off-by: Gavin Li 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---
 lib/tc.c | 15 ---
 lib/tc.h | 34 ++
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/lib/tc.c b/lib/tc.c
index 270dc95ce53b..223fe6e5e5e9 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -701,7 +701,7 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr,
 
 static int
 nl_parse_flower_tunnel_opts(struct nlattr *options,
-struct tun_metadata *metadata)
+struct tc_flower_tunnel *tunnel)
 {
 const struct ofpbuf *msg;
 struct nlattr *nla;
@@ -716,7 +716,7 @@ nl_parse_flower_tunnel_opts(struct nlattr *options,
 uint16_t type = nl_attr_type(nla);
 switch (type) {
 case TCA_FLOWER_KEY_ENC_OPTS_GENEVE:
-err = nl_parse_geneve_key(nla, metadata);
+err = nl_parse_geneve_key(nla, >metadata);
 if (err) {
 return err;
 }
@@ -828,13 +828,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 if (attrs[TCA_FLOWER_KEY_ENC_OPTS] &&
 attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
  err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS],
-   >key.tunnel.metadata);
+   >key.tunnel);
  if (err) {
  return err;
  }
 
  err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK],
-   >mask.tunnel.metadata);
+   >mask.tunnel);
  if (err) {
  return err;
  }
@@ -3446,8 +3446,9 @@ nl_msg_put_masked_value(struct ofpbuf *request, uint16_t 
type,
 
 static void
 nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type,
-  struct tun_metadata *metadata)
+  struct tc_flower_tunnel *tunnel)
 {
+struct tun_metadata *metadata = >metadata;
 struct geneve_opt *opt;
 size_t outer, inner;
 int len, cnt = 0;
@@ -3536,9 +3537,9 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct 
tc_flower *flower)
 nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
 }
 nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS,
-  >key.tunnel.metadata);
+  >key.tunnel);
 nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS_MASK,
-  >mask.tunnel.metadata);
+  >mask.tunnel);
 }
 
 #define FLOWER_PUT_MASKED_VALUE(member, type) \
diff --git a/lib/tc.h b/lib/tc.h
index cdd3b4f60ec8..b9d449677ed9 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -105,6 +105,23 @@ struct tc_cookie {
 size_t len;
 };
 
+struct tc_flower_tunnel {
+struct {
+ovs_be32 ipv4_src;
+ovs_be32 ipv4_dst;
+} ipv4;
+struct {
+struct in6_addr ipv6_src;
+struct in6_addr ipv6_dst;
+} ipv6;
+uint8_t tos;
+uint8_t ttl;
+ovs_be16 tp_src;
+ovs_be16 tp_dst;
+ovs_be64 id;
+struct tun_metadata metadata;
+};
+
 struct tc_flower_key {
 ovs_be16 eth_type;
 uint8_t ip_proto;
@@ -161,22 +178,7 @@ struct tc_flower_key {
 uint8_t rewrite_tclass;
 } ipv6;
 
-struct {
-struct {
-ovs_be32 ipv4_src;
-ovs_be32 ipv4_dst;
-} ipv4;
-struct {
-struct in6_addr ipv6_src;
-struct in6_addr ipv6_dst;
-} ipv6;
-uint8_t tos;
-uint8_t ttl;
-ovs_be16 tp_src;
-ovs_be16 tp_dst;
-ovs_be64 id;
-struct tun_metadata metadata;
-} tunnel;
+struct tc_flower_tunnel tunnel;
 };
 
 enum tc_action_type {
-- 
2.38.0

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


[ovs-dev] [PATCH v4 0/9] Add vxlan gbp offload with TC

2023-06-14 Thread Roi Dayan via dev
Hi,

This series adds TC offload support for filtering vxlan tunnels with gbp option.
First 4 patches do some refactoring and the later patches adds the feature.

Thanks,
Roi

changelog

v4:
- probe TC kernel for vxlan gbp support.
- add test.
- style fix in patch 3.
- log warn instead of err in patch 5.

v3:
- Add function nl_msg_start_nested_with_flag() to be used
  with TC fiedls that require the nested flag. currently
  only vxlan gbp tun opts.
- Split put flower tunnel opts to sub functions for geneve
  and vxlan tun opts.

v2:
- Fix incorrect compat modification in 
  patch "tc: Add vxlan gbp option flower match offload".


Gavin Li (9):
  tc: Pass tunnel entirely to tunnel option parse and put functions
  odp-util: Extract vxlan gbp option decoding to a function
  odp-util: Extract vxlan gbp option encoding to a function
  netlink: Add new function to add NLA_F_NESTED to nested netlink
messages
  tc: Add vxlan gbp option flower match offload
  tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set
  tc: Add vxlan encap action with gbp option offload
  netdev-tc-offloads: Probe for allowing vxlan gbp support
  system-offloads-traffic.at: Add vxlan gbp offload test

 acinclude.m4 |   7 +
 include/linux/pkt_cls.h  |  13 ++
 include/linux/tc_act/tc_tunnel_key.h |  17 ++-
 lib/netdev-offload-tc.c  | 106 -
 lib/netlink.c|   9 ++
 lib/netlink.h|   1 +
 lib/odp-util.c   |  53 ---
 lib/odp-util.h   |  16 ++
 lib/tc.c | 218 ---
 lib/tc.h |  80 +-
 tests/system-offloads-traffic.at |  49 ++
 11 files changed, 462 insertions(+), 107 deletions(-)

-- 
2.38.0

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