On 6/1/22 12:37, Eelco Chaudron wrote: > > > 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 :)
One of the possible issues, I think, is if some traffic manages to pass through OVS before hw-offload is enabled, it will be installed to the kernel and will not be offloaded once the knob is turned on. I remember having some weird issues with the initialization sequence, but I don't remember what that was exactly... :/ Anyway, I guess, it's a safe bet to set the value before the start. > >> >> 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]) Here we already have the $2 argument that can be used to set any database values by passing '-- set something' and I don't see it actually being used anywhere in the testsuite. Can we just use that instead of adding a third argument? Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
