On 1 Jun 2022, at 12:14, Roi Dayan wrote:
> On 2022-05-16 6:38 PM, 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]>
>> Acked-by: Mike Pattrick <[email protected]>
>> ---
>> v2:
>> - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's
>>
>
> just a note. setting hw-offload true first time doesn't actually
> require restarting ovs as netdev_set_flow_api_enabled() calls
> ovsthread_once_start() and does its logic once hw-offload is true.
> But changing from true to false does require restarting ovs.
Thanks for the review Roi! Unfortunately, I can remember a case where I was
doing some testing and then enabled offloading, and it would not work. After a
restart, it would, and it was definitely related to not being enabled at
startup. When I brought it up someone pointed me to the documentation, and I
did not bother :( Hopefully, my memory will come back, and I remember how I did
it :)
>
> Acked-by: Roi Dayan <[email protected]>
>
>
>> tests/ofproto-macros.at | 6 +++++-
>> 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, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
>> index 7051d9539..de6bd6c2e 100644
>> --- a/tests/ofproto-macros.at
>> +++ b/tests/ofproto-macros.at
>> @@ -138,7 +138,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]
>> [pre-vswitchd-commands])
>> #
>> # Creates an empty database and starts ovsdb-server.
>> # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'.
>> @@ -159,6 +159,9 @@ 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])
>> @@ -174,6 +177,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..a8eadc483 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],
>> [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
>> @@ -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..705a50079 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([], [], [ovs-vsctl --no-wait 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([], [], [ovs-vsctl --no-wait 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([], [], [ovs-vsctl --no-wait 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..24ece8e5c 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],
>> [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 -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [|
>> uuidfilt])], [0], [$2])
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
