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.

[
 This is not a full implementation, see the FIXME comment in the code.
 Posted in support for review of the OVS_ACTION_ATTR_DROP patch set.
 I can continue working on the proper implementation once I'm back
 from PTO.
]

Signed-off-by: Ilya Maximets <[email protected]>
---
 tests/dpif-netdev.at |  6 +++---
 vswitchd/bridge.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 85119fb81..a3e07895a 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -410,10 +410,10 @@ m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD],
       set bridge br0 datapath-type=dummy \
                      other-config:datapath-id=1234 fail-mode=secure], [], [],
       [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
-   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
    OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-ofctl del-flows br0])
    AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=IN_PORT])
@@ -473,10 +473,10 @@ m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS],
       set bridge br0 datapath-type=dummy \
                      other-config:datapath-id=1234 fail-mode=secure], [], [],
       [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
-   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
    OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-ofctl del-flows br0])
 
@@ -550,10 +550,10 @@ m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS_VID_ARP],
       set bridge br0 datapath-type=dummy \
                      other-config:datapath-id=1234 fail-mode=secure], [], [],
       [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
-   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
    OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg 
netdev_dummy:file:dbg])
 
    AT_CHECK([ovs-ofctl del-flows br0])
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e9110c1d8..b3ccdaa06 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1986,6 +1986,27 @@ port_is_bond_fake_iface(const struct port *port)
     return port->cfg->bond_fake_iface && !ovs_list_is_short(&port->ifaces);
 }
 
+static bool
+flow_api_status_changed(void)
+{
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+    static bool current_status = false;
+    bool new_status;
+
+    new_status = netdev_is_flow_api_enabled();
+
+    if (ovsthread_once_start(&once)) {
+        current_status = new_status;
+        ovsthread_once_done(&once);
+    }
+
+    if (current_status != new_status) {
+        current_status = new_status;
+        return true;
+    }
+    return false;
+}
+
 static void
 add_del_bridges(const struct ovsrec_open_vswitch *cfg)
 {
@@ -2023,6 +2044,22 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
         }
     }
 
+    if (flow_api_status_changed()) {
+        /* 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. */
+        HMAP_FOR_EACH_SAFE (br, node, &all_bridges) {
+            /* FIXME: This has to be called with 'false' in order to not
+             * destroy the datapath, but it requires re-working the
+             * 'iface_hints' mechanism by supplying hints every time
+             * ofproto is created, not only when the type is initialized.
+             * Oterwise, ports will be destroyed anyway.  For userspace
+             * datapath that will also mean complete removal of a bridge
+             * port without re-creation that we obviously do not want. */
+            bridge_destroy(br, true /* false! */);
+        }
+    }
+
     /* Add new bridges. */
     SHASH_FOR_EACH(node, &new_br) {
         const struct ovsrec_bridge *br_cfg = node->data;
-- 
2.41.0

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

Reply via email to