On Fri, Jan 29, 2021 at 11:48:37PM +0100, Ilya Maximets wrote:
> On 1/29/21 11:33 PM, Marcelo Ricardo Leitner wrote:
> > On Fri, Jan 29, 2021 at 10:31:46PM +0100, Ilya Maximets wrote:
> >> On 12/23/20 2:01 PM, Marcelo Ricardo Leitner wrote:
> >>> tc-policy, just like hw-offload, is protected by ovsthread_once_start()
> >>> in netdev_set_flow_api_enabled() so lets document that changing it
> >>> requires a restart in order for it to have effect.
> >>>
> >>> Signed-off-by: Marcelo Ricardo Leitner <[email protected]>
> >>> ---
> >>>  vswitchd/vswitch.xml | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> >>> index 
> >>> df5aa41a23da2622eee69f143a52a7e3b970488a..0b853ff26b76aebec8692ac1c59a54a7333e6330
> >>>  100644
> >>> --- a/vswitchd/vswitch.xml
> >>> +++ b/vswitchd/vswitch.xml
> >>> @@ -267,7 +267,8 @@
> >>>            <ref column="other_config" key="hw-offload"/> is enabled.
> >>>          </p>
> >>>          <p>
> >>> -          The default value is <code>none</code>.
> >>> +          The default value is <code>none</code>. Changing this value 
> >>> requires
> >>> +          restarting the daemon
> >>
> >> That is not fully correct.  Only disabling requires restart.
> >> Changing the value from 'false' to 'true' is allowed in runtime.
> > 
> > I don't see where that is allowed. The only call I see to
> > tc_set_policy() is from netdev_set_flow_api_enabled(), under the
> > protection of ovsthread_once_start().
> 
> Yep.  But that call is inside the 'if':
>   https://github.com/openvswitch/ovs/blob/master/lib/netdev-offload.c#L673
> 
> And this ovsthread_once_start() only executed if 'hw-offload' was set to

Right. That's covered on the paragraph before the change:

        <p>
          This is only relevant if
          <ref column="other_config" key="hw-offload"/> is enabled.
        </p>

> 'true'.  This way you should be able to turn on offloading at any time,
> but turning off requires restart.

Ohh I see what you mean. You're referring to hw-offload then, ok. The
patch here is about tc-policy and not hw-offload, though, that's why I
was not following.

For tc-policy, it is only parsed once when hw-offload is enabled, not
mattering if enabled after starting or starting with it enabled.
Right? Then depending on the below, we could word it as
+          The default value is <code>none</code>. Changing this value requires
+          restarting the daemon if hw-offload was already enabled.

For hw-offload, seems you're right. It would require bridge_run() to
be called again. I see an 'while (!exiting)' in vswitchd main() that
would allow it, but I don't know how one could trigger that?

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

Reply via email to