[ovs-dev] [PATCH v5 10/12] dpif-netdev: Fix reconfigure_pmd_threads().

Kavanagh, Mark B mark.b.kavanagh at intel.com
Thu Mar 24 16:53:57 UTC 2016


Hi Daniele,

One minor comment below.

Cheers,
Mark

>
>This commit changes reconfigure_pmd_threads() to interact with the ports
>cmap using RCU semantics (the content of the port structure is not
>altered while concurrent readers might access it) and to fail more
>gracefully in case of a set_multiq fail (now we remove the port from the
>datapath, instead of returning prematurely from the function without
>restarting the pmd threads).
>
>Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>Tested-by: Ilya Maximets <i.maximets at samsung.com>
>Acked-by: Ilya Maximets <i.maximets at samsung.com>
>---
> lib/dpif-netdev.c | 78 ++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 54 insertions(+), 24 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index c66bf29..456cda4 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -2591,6 +2591,11 @@ static void
> reconfigure_pmd_threads(struct dp_netdev *dp)
> {
>     struct dp_netdev_port *port;
>+    struct hmapx to_reconfigure = HMAPX_INITIALIZER(&to_reconfigure);
>+    struct hmapx_node *node;
>+    bool failed_config = false;
>+
>+    ovs_mutex_lock(&dp->port_mutex);
>
>     dp_netdev_destroy_all_pmds(dp);
>
>@@ -2599,33 +2604,58 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>         int requested_n_rxq = netdev_requested_n_rxq(netdev);
>         if (netdev_is_pmd(port->netdev)
>             && port->latest_requested_n_rxq != requested_n_rxq) {
>-            int i, err;
>+            cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no));
>+            hmapx_add(&to_reconfigure, port);
>+        }
>+    }
>+    ovs_mutex_unlock(&dp->port_mutex);
>
>-            /* Closes the existing 'rxq's. */
>-            for (i = 0; i < port->n_rxq; i++) {
>-                netdev_rxq_close(port->rxq[i]);
>-                port->rxq[i] = NULL;
>-            }
>-            port->n_rxq = 0;
>-
>-            /* Sets the new rx queue config. */
>-            err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() + 1,
>-                                    requested_n_rxq);
>-            if (err && (err != EOPNOTSUPP)) {
>-                VLOG_ERR("Failed to set dpdk interface %s rx_queue to: %u",
>-                         netdev_get_name(port->netdev),
>-                         requested_n_rxq);
>-                return;
>-            }
>-            port->latest_requested_n_rxq = requested_n_rxq;
>-            /* If the set_multiq() above succeeds, reopens the 'rxq's. */
>-            port->n_rxq = netdev_n_rxq(port->netdev);
>-            port->rxq = xrealloc(port->rxq, sizeof *port->rxq * port->n_rxq);
>-            for (i = 0; i < port->n_rxq; i++) {
>-                netdev_rxq_open(port->netdev, &port->rxq[i], i);
>-            }
>+    /* Waits for the other threads to see the ports removed from the cmap,
>+     * otherwise we are not allowed to alter them. */
>+    ovsrcu_synchronize();
>+
>+    ovs_mutex_lock(&dp->port_mutex);
>+    HMAPX_FOR_EACH (node, &to_reconfigure) {
>+        int requested_n_rxq, i, err;
>+
>+        port = node->data;
>+        requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>+        /* Closes the existing 'rxq's. */
>+        for (i = 0; i < port->n_rxq; i++) {
>+            netdev_rxq_close(port->rxq[i]);
>+            port->rxq[i] = NULL;
>+        }
>+        port->n_rxq = 0;
>+
>+        /* Sets the new rx queue config. */
>+        err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() + 1,
>+                                requested_n_rxq);
>+        if (err && (err != EOPNOTSUPP)) {
>+            VLOG_ERR("Failed to set dpdk interface %s rx_queue to: %u",
>+                     netdev_get_name(port->netdev),
>+                     requested_n_rxq);
>+            do_destroy_port(port);
>+            failed_config = true;
>+            continue;
>+        }
>+        port->latest_requested_n_rxq = requested_n_rxq;
>+        /* If the netdev_reconfigure() above succeeds, reopens the 'rxq's and

Shouldn't this be 'If the netdev_set_multiq() above succeeds"?

>+         * inserts the port back in the cmap, to allow transmitting packets. */
>+        port->n_rxq = netdev_n_rxq(port->netdev);
>+        port->rxq = xrealloc(port->rxq, sizeof *port->rxq * port->n_rxq);
>+        for (i = 0; i < port->n_rxq; i++) {
>+            netdev_rxq_open(port->netdev, &port->rxq[i], i);
>         }
>+        cmap_insert(&dp->ports, &port->node, hash_port_no(port->port_no));
>     }
>+    ovs_mutex_unlock(&dp->port_mutex);
>+
>+    hmapx_destroy(&to_reconfigure);
>+
>+    if (failed_config) {
>+        seq_change(dp->port_seq);
>+    }
>+
>     /* Reconfigures the cpu mask. */
>     ovs_numa_set_cpu_mask(dp->requested_pmd_cmask);
>     free(dp->pmd_cmask);
>--
>2.1.4




More information about the dev mailing list