Hi Emma,

I started reviewing your patch and found some issues that need investigation.

With the autovalidator, some tests are failing. But before you can run them, 
you need to fix your patch 7, as the autovalidator enablement is not working:

[wsfd-advnetlab44:~/...DK_v21.11/ovs_github]$ git diff
diff --git a/acinclude.m4 b/acinclude.m4
index 98f4599b1..18e61b522 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -28,7 +28,7 @@ AC_DEFUN([OVS_CHECK_ACTIONS_AUTOVALIDATOR], [
   if test "$autovalidator" != yes; then
     AC_MSG_RESULT([no])
   else
-    AC_DEFINE([MFEX_AUTOVALIDATOR_DEFAULT], [1],
+    AC_DEFINE([ACTIONS_AUTOVALIDATOR_DEFAULT], [1],
               [Autovalidator for actions is a default implementation.])
     AC_MSG_RESULT([yes])
   fi


Build OVS (without DPDK) and run the following tests:

# ./configure --prefix=/usr --localstatedir=/var --sysconfdir=/etc && make -j 
32 check TESTSUITEFLAGS="1040 1041 1132 1145 1294 2493"
...
...
## ------------------------------- ##
## openvswitch 2.17.90 test suite. ##
## ------------------------------- ##

dpif-netdev

1040: dpif-netdev - partial hw offload with packet modifications - dummy ok
1041: dpif-netdev - partial hw offload with packet modifications - dummy-pmd ok

ofproto-dpif

1132: ofproto-dpif - controller                       ok
1145: ofproto-dpif - ARP modification slow-path       ok

ofproto-dpif - flow translation resource limits

1294: ofproto-dpif - Neighbor Discovery set-field with checksum update ok

network service header (NSH)

2493: nsh - triangle PTAP bridge setup with NSH over vxlan-gpe ok

## ------------- ##
## Test results. ##
## ------------- ##

All 6 tests were successful.


Now with the auto-validator on an AVX512 machine:

# ./configure --prefix=/usr --localstatedir=/var --sysconfdir=/etc 
--enable-actions-default-autovalidator && make -j 32 check TESTSUITEFLAGS="1040 
1041 1132 1145 1294 2493"
...
...
## ------------------------------- ##
## openvswitch 2.17.90 test suite. ##
## ------------------------------- ##

dpif-netdev

1040: dpif-netdev - partial hw offload with packet modifications - dummy FAILED 
(dpif-netdev.at:542)
1041: dpif-netdev - partial hw offload with packet modifications - dummy-pmd 
FAILED (dpif-netdev.at:543)

ofproto-dpif

1132: ofproto-dpif - controller                       FAILED 
(ofproto-dpif.at:1979)
1145: ofproto-dpif - ARP modification slow-path       FAILED 
(ofproto-dpif.at:3777)

ofproto-dpif - flow translation resource limits

1294: ofproto-dpif - Neighbor Discovery set-field with checksum update FAILED 
(ofproto-dpif.at:9936)

network service header (NSH)

2493: nsh - triangle PTAP bridge setup with NSH over vxlan-gpe FAILED 
(nsh.at:778)

## ------------- ##
## Test results. ##
## ------------- ##

ERROR: All 6 tests were run,
6 failed unexpectedly.


Please start some investigation, but do NOT send out a v8, until I’ve completed 
v7. If there is a small change I can apply that fixes the issue, just sent that 
in this thread.


//Eelco

On 14 Jun 2022, at 13:57, Emma Finn wrote:

> This patchset introduces actions infrastructure changes which allows the
> user to choose between different action implementations based on CPU ISA
> by using different commands.  The infrastructure also provides a way to
> check the correctness of the ISA optimized action version against the
> scalar version.
>
> This series  also introduces optimized versions of the following
> actions:
>  - push_vlan
>  - pop_vlan
>  - set_masked eth
>  - set_masked ipv4
>
> Below is a table indicating the relative performance benefits for these
> actions.
>
> +-----------------------------------------------+-------------------+-----------------+
> | Actions                                       | Salar with series |AVX with 
> series |
> +-----------------------------------------------+-------------------+-----------------+
> | mod_dl_dst                                    | 1.04x             |1.15x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
> | push_vlan                                     | 1.10x             |1.23x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
> | strip_vlan                                    | 1.05x             |1.14x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
> | mod_ipv4 1 x field                            | 1.04x             |1.04x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
> | mod_ipv4 4 x fields                           | 1.04x             |1.23x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
> | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.06x             |1.36x    
>        |
> +-----------------------------------------------+-------------------+-----------------+
>
> ---
> v7:
> - Fix review comments from Eelco.
> ---
> v6:
> - Rebase to master
> - Add ISA implementation of set_masked eth and ipv4 actions
> - Fix incorrect checksums in input packets for ofproto-dpif unit tests
> ---
> v5:
> - Rebase to master
> - Minor change to variable names
> - Added Tags from Harry.
> ---
> v4:
> - Rebase to master
> - Add ISA implementation of push_vlan action
> ---
> v3:
> - Refactored to fix unit test failures
> - Removed some sign-off on commits
> ---
> v2:
> - Fix the CI build issues
> ---
>
> Emma Finn (10):
>   ofproto-dpif: Fix incorrect checksums in input packets
>   odp-execute: Add function pointers to odp-execute for different action
>     implementations.
>   odp-execute: Add function pointer for pop_vlan action.
>   odp-execute: Add auto validation function for actions.
>   odp-execute: Add command to switch action implementation.
>   odp-execute: Add ISA implementation of actions.
>   odp-execute: Add ISA implementation of pop_vlan action.
>   odp-execute: Add ISA implementation of push_vlan action.
>   odp-execute: Add ISA implementation of set_masked ETH
>   odp-execute: Add ISA implementation of set_masked IPv4 action
>
> Kumar Amber (1):
>   dpif-netdev: Add configure option to enable actions autovalidator at
>     build time.
>
>  Documentation/ref/ovs-actions.7.rst |  26 ++
>  Documentation/topics/testing.rst    |  24 +-
>  NEWS                                |  11 +
>  acinclude.m4                        |  21 ++
>  configure.ac                        |   1 +
>  lib/automake.mk                     |   8 +-
>  lib/cpu.c                           |   1 +
>  lib/cpu.h                           |   1 +
>  lib/dp-packet.c                     |  23 ++
>  lib/dp-packet.h                     |   4 +
>  lib/dpif-netdev-unixctl.man         |   8 +
>  lib/dpif-netdev.c                   |  42 +++
>  lib/odp-execute-avx512.c            | 463 ++++++++++++++++++++++++++++
>  lib/odp-execute-private.c           | 266 ++++++++++++++++
>  lib/odp-execute-private.h           |  99 ++++++
>  lib/odp-execute.c                   | 183 ++++++++---
>  lib/odp-execute.h                   |  14 +
>  tests/ofproto-dpif.at               |  10 +-
>  tests/pmd.at                        |  30 ++
>  19 files changed, 1183 insertions(+), 52 deletions(-)
>  create mode 100644 lib/odp-execute-avx512.c
>  create mode 100644 lib/odp-execute-private.c
>  create mode 100644 lib/odp-execute-private.h
>
> -- 
> 2.32.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to