At some point in OVS history, some virtio features were announced as
supported (ECN and UFO virtio features).

The userspace TSO code, which has been added later, does not support
those features and tries to disable them.

This breaks OVS upgrades: if an existing VM already negotiated such
features, their lack on reconnection to an upgraded OVS triggers a
vhost socket disconnection by Qemu.
This results in an endless loop because Qemu then retries with the same
set of virtio features.

This patch proposes to try and detect those vhost socket disconnection
and fallback restoring the old virtio features (and disabling TSO for this
vhost port).

Signed-off-by: David Marchand <[email protected]>
---
Changelog since v1:
- added a note in the documentation,
- fixed vhost unregister trigger (so that both disabling and re-enabling
  TSO is handled),
- cleared netdev features when disabling TSO,
- changed level and ratelimited log message on vhost socket disconnect,

---
 Documentation/topics/userspace-tso.rst | 13 ++++-
 lib/netdev-dpdk.c                      | 71 ++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/Documentation/topics/userspace-tso.rst 
b/Documentation/topics/userspace-tso.rst
index 33a85965c1..e84770fe3a 100644
--- a/Documentation/topics/userspace-tso.rst
+++ b/Documentation/topics/userspace-tso.rst
@@ -68,7 +68,7 @@ as follows.
 connection is established, `TSO` is thus advertised to the guest as an
 available feature:
 
-QEMU Command Line Parameter::
+1. QEMU Command Line Parameter::
 
     $ sudo $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 \
     ...
@@ -83,6 +83,17 @@ used to enable same::
     $ ethtool -K eth0 tso on
     $ ethtool -k eth0
 
+**Note:** Enabling this feature impacts the virtio features exposed by the DPDK
+vHost User backend to a guest. If a guest was already connected to OvS before
+enabling TSO and restarting OvS, this guest ports won't have TSO available::
+
+  netdev_dpdk|WARN|Disabling TSO for vhost0.
+
+To restore TSO for this guest ports, this guest QEMU process must be stopped,
+then started again. OvS will then report::
+
+  netdev_dpdk|WARN|Re-enabling TSO for vhost0.
+
 ~~~~~~~~~~~
 Limitations
 ~~~~~~~~~~~
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 0dd655507b..5a9f79951d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -465,6 +465,15 @@ struct netdev_dpdk {
         /* True if vHost device is 'up' and has been reconfigured at least 
once */
         bool vhost_reconfigured;
 
+        /* Set on driver start (which means after a vHost connection is
+         * accepted), and cleared when the vHost device gets configured. */
+        bool vhost_initial_config;
+
+        /* Set on disconnection if an initial configuration did not finish.
+         * This triggers a workaround for Virtio features negotiation, that
+         * makes TSO unavailable. */
+        bool vhost_workaround_disable_tso;
+
         atomic_uint8_t vhost_tx_retries_max;
         /* 2 pad bytes here. */
     );
@@ -1293,6 +1302,7 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
     dev->requested_lsc_interrupt_mode = 0;
     ovsrcu_index_init(&dev->vid, -1);
     dev->vhost_reconfigured = false;
+    dev->vhost_initial_config = false;
     dev->attached = false;
     dev->started = false;
     dev->reset_needed = false;
@@ -3986,6 +3996,7 @@ new_device(int vid)
             } else {
                 /* Reconfiguration not required. */
                 dev->vhost_reconfigured = true;
+                dev->vhost_initial_config = false;
             }
 
             ovsrcu_index_set(&dev->vid, vid);
@@ -4141,6 +4152,7 @@ destroy_connection(int vid)
         ovs_mutex_lock(&dev->mutex);
         if (nullable_string_is_equal(ifname, dev->vhost_id)) {
             uint32_t qp_num = NR_QUEUE;
+            bool disable_tso;
 
             if (netdev_dpdk_get_vid(dev) >= 0) {
                 VLOG_ERR("Connection on socket '%s' destroyed while vhost "
@@ -4154,6 +4166,28 @@ destroy_connection(int vid)
                 dev->requested_n_txq = qp_num;
                 netdev_request_reconfigure(&dev->up);
             }
+
+            if (dev->vhost_initial_config) {
+                VLOG_WARN_RL(&rl, "Connection on socket '%s' closed during "
+                             "initialization.", dev->vhost_id);
+            }
+
+            disable_tso = dev->vhost_initial_config
+                && (dev->up.ol_flags & NETDEV_TX_OFFLOAD_TCP_TSO) != 0;
+            if (disable_tso != dev->vhost_workaround_disable_tso) {
+                /* Either an early disconnection was detected (and we can try
+                 * to disable TSO for a next connection) or the disconnection
+                 * does not seem incorrect (and we can try to enable TSO for a
+                 * next connection).
+                 *
+                 * In both cases, the netdev must be reconfigured to update its
+                 * ol_flags and so that the vhost library pushes the
+                 * corresponding virtio features
+                 * (see netdev_dpdk_vhost_client_reconfigure()). */
+                dev->vhost_workaround_disable_tso = disable_tso;
+                netdev_request_reconfigure(&dev->up);
+            }
+
             ovs_mutex_unlock(&dev->mutex);
             exists = true;
             break;
@@ -5058,6 +5092,7 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
 
         if (dev->vhost_reconfigured == false) {
             dev->vhost_reconfigured = true;
+            dev->vhost_initial_config = false;
             /* Carrier status may need updating. */
             netdev_change_seq_changed(&dev->up);
         }
@@ -5083,9 +5118,31 @@ static int
 netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev)
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-    int err;
-    uint64_t vhost_flags = 0;
     uint64_t vhost_unsup_flags;
+    uint64_t vhost_flags = 0;
+    bool unregister = false;
+    bool enable_tso;
+    char *vhost_id;
+    int err;
+
+    ovs_mutex_lock(&dev->mutex);
+
+    enable_tso = userspace_tso_enabled() && !dev->vhost_workaround_disable_tso;
+    if (enable_tso != ((netdev->ol_flags & NETDEV_TX_OFFLOAD_TCP_TSO) != 0)
+        && dev->vhost_id && dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT) {
+
+        dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
+        vhost_id = dev->vhost_id;
+        unregister = true;
+        VLOG_WARN("%sabling TSO for %s.", enable_tso ? "Re-en" : "Dis",
+                  dev->up.name);
+    }
+
+    ovs_mutex_unlock(&dev->mutex);
+
+    if (unregister) {
+        dpdk_vhost_driver_unregister(dev, vhost_id);
+    }
 
     ovs_mutex_lock(&dev->mutex);
 
@@ -5112,7 +5169,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
         }
 
         /* Enable External Buffers if TCP Segmentation Offload is enabled. */
-        if (userspace_tso_enabled()) {
+        if (enable_tso) {
             vhost_flags |= RTE_VHOST_USER_EXTBUF_SUPPORT;
         }
 
@@ -5137,7 +5194,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
             goto unlock;
         }
 
-        if (userspace_tso_enabled()) {
+        if (enable_tso) {
             netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
             netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CKSUM;
             netdev->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CKSUM;
@@ -5146,6 +5203,11 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
             vhost_unsup_flags = 1ULL << VIRTIO_NET_F_HOST_ECN
                                 | 1ULL << VIRTIO_NET_F_HOST_UFO;
         } else {
+            netdev->ol_flags &= ~NETDEV_TX_OFFLOAD_TCP_TSO;
+            netdev->ol_flags &= ~NETDEV_TX_OFFLOAD_TCP_CKSUM;
+            netdev->ol_flags &= ~NETDEV_TX_OFFLOAD_UDP_CKSUM;
+            netdev->ol_flags &= ~NETDEV_TX_OFFLOAD_SCTP_CKSUM;
+            netdev->ol_flags &= ~NETDEV_TX_OFFLOAD_IPV4_CKSUM;
             /* This disables checksum offloading and all the features
              * that depends on it (TSO, UFO, ECN) according to virtio
              * specification. */
@@ -5160,6 +5222,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
             goto unlock;
         }
 
+        dev->vhost_initial_config = true;
         err = rte_vhost_driver_start(dev->vhost_id);
         if (err) {
             VLOG_ERR("rte_vhost_driver_start failed for vhost user "
-- 
2.37.2

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

Reply via email to