On 12/15/23 11:03, Eelco Chaudron wrote:
On 14 Dec 2023, at 19:15, Adrian Moreno wrote:
On 12/13/23 11:47, Adrian Moreno wrote:
On 9/15/23 17:25, Eelco Chaudron wrote:
On 9 Sep 2023, at 1:40, Ilya Maximets wrote:
Currently OVS requires restart of ovs-vswitchd after enabling hardware
offload. This is necessary to make sure all the correct features are
probed and all the internal configurations are in the right state.
Making that process a bit less invasive by re-creating bridges instead
of a full process restart. Documentation is not updated, because
disabling hardware offload still can take effect only after restart.
Hi Ilya,
I like the idea of re-creating the bridges, however not sure if it generated
any side effects other than the ones you mention in the FIXME.
I have taken a look at the patch, tested it and added some rework of the
iface_hints to avoid the datapath ports from being deleted when the bridge is
re-created.
The recreation of the bridge seems to work but my concern is that, unlike a restart
which uses ovs-save to save & restore the flows, groups and tunnel metadata,
doing this will leave the bridge with no OpenFlow configuration.
During the time the controller takes to reconfigure the bridge, reval threads
will wipe the datapath flows. If there is no controller ... well we'll need to
wait for the user to program the flows back. This is quite a big behavior
change so I'd say if we go for this option we better documment it pretty well.
I'm thinking that, considering this hassle (loosing all flows, meters, tunnel
tlv info, etc), isn't it better to simply restart vswitchd?
A more more complex solution (probably big enough for this release which is
coming fast) could be to recreate ovs-save internally and:
- Collect all the flow table, groups, tlv data and meters
- Delete the bridge and recreate it
- Reconfigure everything back
Flow stats will probably be lost in the process, which is a weird outcome of
enabling hwol but it's better than loosing all the flows.
I guess the goal for this patch was to NOT touch any actual resources, only
re-probe the capabilities. See comment below:
+ /* Destroy all the remaining bridges if Flow API status changed
+ * as we need to re-probe supported features. Do not delete
+ * bridge resources to avoid datapath disruption. */
Ilya can you comment on this?
Summarizing the conversation we had with Ilya and adding a bit of context for
the sake of completeness:
When reviewing Eric's series to add OVS_ACTION_ATTR_DROP support for the kernel
datapath. The problem comes when hw-offload is enabled since the tc datapath
does not support this, we need a way to re-evaluate the feature and stop adding
that action.
An approach was proposed (suggested by Eelco) in version 6 [1] of that series:
manually reprobing that feature bit. Ilya raised concerns about thread-safety
and maintainability and proposed exploring this alternative idea.
Given the consequences of this approach (loosing all the OFP flows), it doesn't
seem as a good way to go, and we don't have any other good idea to do this easily.
Therefore, the way to move forward would be to add atomicity to Eric's V6
version. Note that maintainability is still a concern and re-probing independent
features should not be the general case.
Also, we need to make sure reprobing does not add dp flows that affect ongoing
traffic.
[1]
https://patchwork.ozlabs.org/project/openvswitch/patch/20230901194234.3409915-3-e...@garver.life/
--
Adrián Moreno
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
