On 1/30/21 12:36 AM, Marcelo Ricardo Leitner wrote: > 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.
I misunderstood too. At some point I started to think that this change is for 'hw-offload', I don't know why. But anyway, my words seem to have some meaning. :) And we're on a same page now. > > 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. Right. And this sounds better, yes. But, please, use <ref column="other_config" key="hw-offload"/> to reference the 'hw-offload' configuration knob. > > 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? It's just an infinite event processing loop. It sleeps inside the poll_block() and wakes up on any event, e.g. some data on a database connection socket (due to db update) that needs processing or interface state changes or whatever else. > >> >>> >>>> >>>>> </p> >>>>> </column> >>>>> >>>>> >>>> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
