Hi Mike/Roi,

This is the patch with the changes in this revision. I removed your ACKs, so 
can you please re-review this patch?

Below are the changes compared to the previous version to make things easy for 
you!

Thanks,

Eelco

1:  f0020c3d5 ! 1:  6c21ec1f9 system-offloads-traffic: Properly initialize 
offload before testing.
    @@ Commit message
         restart once configured).

         Signed-off-by: Eelco Chaudron <[email protected]>
    -    Acked-by: Mike Pattrick <[email protected]>

    +    v4:
    +      - Use the existing dbinit-aux-args argument, rather than
    +        creating a new pre-vswitchd command option.
    +    v3:
    +      - No changes.
         v2:
           - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's

    @@ tests/ofproto-macros.at: m4_divert_pop([PREPARE_TESTS])
      m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m'])

     -# _OVS_VSWITCHD_START([vswitchd-aux-args])
    -+# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args] 
[pre-vswitchd-commands])
    ++# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args])
      #
      # Creates an empty database and starts ovsdb-server.
      # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'.
    -@@ tests/ofproto-macros.at: m4_define([_OVS_VSWITCHD_START],
    -    dnl Initialize database.
    -    AT_CHECK([ovs-vsctl --no-wait init $2])
    -
    -+   dnl Run extra commands before ovs-vswitchd starts.
    -+   AT_CHECK([:; $3])
    -+
    -    dnl Start ovs-vswitchd.
    -    AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file 
-vvconn -vofproto_dpif -vunixctl], [0], [], [stderr])
    -    AT_CAPTURE_FILE([ovs-vswitchd.log])
     @@ tests/ofproto-macros.at: m4_define([_OVS_VSWITCHD_START],
      /ofproto|INFO|datapath ID changed to fedcba9876543210/d
      /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d
    @@ tests/system-kmod-macros.at
      m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 
protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 
fail-mode=secure ]])

     -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
    -+# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
[pre-vswitchd-commands])
    ++# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
[dbinit-aux-args]])
      #
      # Creates a database and starts ovsdb-server, starts ovs-vswitchd
      # connected to that database, calls ovs-vsctl to create a bridge named
    @@ tests/system-kmod-macros.at: m4_define([OVS_TRAFFIC_VSWITCHD_START],
         on_exit 'ovs-dpctl del-dp ovs-system'
         on_exit 'ovs-appctl dpctl/flush-conntrack'
     -   _OVS_VSWITCHD_START([])
    -+   _OVS_VSWITCHD_START([], [], [$3])
    ++   _OVS_VSWITCHD_START([], [$3])
         dnl Add bridges, ports, etc.
         AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
uuidfilt])], [0], [$2])
      ])
    @@ tests/system-offloads-traffic.at: AT_CLEANUP

      AT_SETUP([offloads - ping between two ports - offloads enabled])
     -OVS_TRAFFIC_VSWITCHD_START()
    -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch 
. other_config:hw-offload=true])
    ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])

     -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
      AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
    @@ tests/system-offloads-traffic.at: AT_CLEANUP
      AT_SKIP_IF([test $HAVE_TC = "no"])
     -OVS_TRAFFIC_VSWITCHD_START()
     -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
    -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch 
. other_config:hw-offload=true])
    ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])
      AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
      ADD_NAMESPACES(at_ns0)
      ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
    @@ tests/system-offloads-traffic.at: AT_CLEANUP
      AT_SKIP_IF([test $SUPPORT_TC_INGRESS_PPS = "no"])
     -OVS_TRAFFIC_VSWITCHD_START()
     -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
    -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch 
. other_config:hw-offload=true])
    ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])
      AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
      ADD_NAMESPACES(at_ns0)
      ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
    @@ tests/system-userspace-macros.at
      m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" 
protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 
fail-mode=secure ]])

     -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
    -+# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
[pre-vswitchd-commands])
    ++# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
[dbinit-aux-args])
      #
      # Creates a database and starts ovsdb-server, starts ovs-vswitchd
      # connected to that database, calls ovs-vsctl to create a bridge named
    @@ tests/system-userspace-macros.at: m4_define([_ADD_BR], [[add-br $1 -- 
set Bridge
        [
         OVS_WAIT_WHILE([ip link show ovs-netdev])
     -   _OVS_VSWITCHD_START([--disable-system])
    -+   _OVS_VSWITCHD_START([--disable-system] [] [$3])
    ++   _OVS_VSWITCHD_START([--disable-system] [$3])
         dnl Add bridges, ports, etc.
         OVS_WAIT_WHILE([ip link show br0])
         AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
uuidfilt])], [0], [$2])



On 3 Jun 2022, at 10:58, Eelco Chaudron wrote:

> This patch will properly initialize offload, as it requires the
> setting to be enabled before starting ovs-vswitchd (or do a
> restart once configured).
>
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
> v4:
>   - Use the existing dbinit-aux-args argument, rather than
>     creating a new pre-vswitchd command option.
> v3:
>   - No changes.
> v2:
>   - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's
>
>  tests/ofproto-macros.at          |    3 ++-
>  tests/system-kmod-macros.at      |    4 ++--
>  tests/system-offloads-traffic.at |    9 +++------
>  tests/system-tso-macros.at       |    4 ++--
>  tests/system-userspace-macros.at |    4 ++--
>  5 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index b18f0fbc1..af956bdcb 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -153,7 +153,7 @@ m4_divert_pop([PREPARE_TESTS])
>
>  m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m'])
>
> -# _OVS_VSWITCHD_START([vswitchd-aux-args])
> +# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args])
>  #
>  # Creates an empty database and starts ovsdb-server.
>  # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'.
> @@ -189,6 +189,7 @@ m4_define([_OVS_VSWITCHD_START],
>  /ofproto|INFO|datapath ID changed to fedcba9876543210/d
>  /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d
>  /netlink_socket|INFO|netlink: could not enable listening to all nsid/d
> +/netdev_offload|INFO|netdev: Flow API Enabled/d
>  /probe tc:/d
>  /setting extended ack support failed/d
>  /tc: Using policy/d']])
> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
> index 86d633ac4..b8a6b67c8 100644
> --- a/tests/system-kmod-macros.at
> +++ b/tests/system-kmod-macros.at
> @@ -4,7 +4,7 @@
>  # appropriate type and properties
>  m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 
> protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 
> fail-mode=secure ]])
>
> -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
> +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
> [dbinit-aux-args]])
>  #
>  # Creates a database and starts ovsdb-server, starts ovs-vswitchd
>  # connected to that database, calls ovs-vsctl to create a bridge named
> @@ -24,7 +24,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
>                ])
>     on_exit 'ovs-dpctl del-dp ovs-system'
>     on_exit 'ovs-appctl dpctl/flush-conntrack'
> -   _OVS_VSWITCHD_START([])
> +   _OVS_VSWITCHD_START([], [$3])
>     dnl Add bridges, ports, etc.
>     AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
> uuidfilt])], [0], [$2])
>  ])
> diff --git a/tests/system-offloads-traffic.at 
> b/tests/system-offloads-traffic.at
> index 80bc1dd5c..f7373664c 100644
> --- a/tests/system-offloads-traffic.at
> +++ b/tests/system-offloads-traffic.at
> @@ -39,9 +39,8 @@ AT_CLEANUP
>
>
>  AT_SETUP([offloads - ping between two ports - offloads enabled])
> -OVS_TRAFFIC_VSWITCHD_START()
> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
> other_config:hw-offload=true])
>
> -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>  AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>
>  ADD_NAMESPACES(at_ns0, at_ns1)
> @@ -97,8 +96,7 @@ AT_CLEANUP
>  AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - 
> offloads enabled])
>  AT_KEYWORDS([ingress_policing])
>  AT_SKIP_IF([test $HAVE_TC = "no"])
> -OVS_TRAFFIC_VSWITCHD_START()
> -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
> other_config:hw-offload=true])
>  AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>  ADD_NAMESPACES(at_ns0)
>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> @@ -146,8 +144,7 @@ AT_CLEANUP
>  AT_SETUP([offloads - set ingress_policing_kpkts_rate and 
> ingress_policing_kpkts_burst - offloads enabled])
>  AT_KEYWORDS([ingress_policing_kpkts])
>  AT_SKIP_IF([test $SUPPORT_TC_INGRESS_PPS = "no"])
> -OVS_TRAFFIC_VSWITCHD_START()
> -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
> other_config:hw-offload=true])
>  AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>  ADD_NAMESPACES(at_ns0)
>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> diff --git a/tests/system-tso-macros.at b/tests/system-tso-macros.at
> index 1a8004761..1881c28fb 100644
> --- a/tests/system-tso-macros.at
> +++ b/tests/system-tso-macros.at
> @@ -4,7 +4,7 @@
>  # appropriate type and properties
>  m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" 
> protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 
> fail-mode=secure ]])
>
> -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
> +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], 
> [pre-vswitchd-commands])
>  #
>  # Creates a database and starts ovsdb-server, starts ovs-vswitchd
>  # connected to that database, calls ovs-vsctl to create a bridge named
> @@ -15,7 +15,7 @@ m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 
> datapath_type="netdev" protoco
>  m4_define([OVS_TRAFFIC_VSWITCHD_START],
>    [
>     OVS_WAIT_WHILE([ip link show ovs-netdev])
> -   _OVS_VSWITCHD_START([--disable-system])
> +   _OVS_VSWITCHD_START([--disable-system] [] [$3])
>     dnl Add bridges, ports, etc.
>     OVS_WAIT_WHILE([ip link show br0])
>     AT_CHECK([ovs-vsctl set Open_vSwitch . 
> other_config:userspace-tso-enable=true])
> diff --git a/tests/system-userspace-macros.at 
> b/tests/system-userspace-macros.at
> index f639ba53a..d9499e87b 100644
> --- a/tests/system-userspace-macros.at
> +++ b/tests/system-userspace-macros.at
> @@ -4,7 +4,7 @@
>  # appropriate type and properties
>  m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" 
> protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 
> fail-mode=secure ]])
>
> -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override])
> +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args])
>  #
>  # Creates a database and starts ovsdb-server, starts ovs-vswitchd
>  # connected to that database, calls ovs-vsctl to create a bridge named
> @@ -15,7 +15,7 @@ m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 
> datapath_type="netdev" protoco
>  m4_define([OVS_TRAFFIC_VSWITCHD_START],
>    [
>     OVS_WAIT_WHILE([ip link show ovs-netdev])
> -   _OVS_VSWITCHD_START([--disable-system])
> +   _OVS_VSWITCHD_START([--disable-system] [$3])
>     dnl Add bridges, ports, etc.
>     OVS_WAIT_WHILE([ip link show br0])
>     AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
> uuidfilt])], [0], [$2])
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Reply via email to