[ovs-dev] [PATCH v3 05/18] netdev-dpdk: Start also dpdkr devices only once on port-add.

Daniele Di Proietto diproiettod at vmware.com
Mon Jan 9 03:15:03 UTC 2017


Since commit 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming"),
we don't call rte_eth_start() from netdev_open() anymore, we only call
it from netdev_reconfigure().  This commit does that also for 'dpdkr'
devices, and remove some useless code.

Calling rte_eth_start() also from netdev_open() was unnecessary and
wasteful. Not doing it reduces code duplication and makes adding a port
faster (~900ms before the patch, ~400ms after).

Another reason why this is useful is that some DPDK driver might have
problems with reconfiguration. For example, until DPDK commit
8618d19b52b1("net/vmxnet3: reallocate shared memzone on re-config"),
vmxnet3 didn't support being restarted with a different number of
queues.

Technically, the netdev interface changed because before opening rxqs or
calling netdev_send() the user must check if reconfiguration is
required.  This patch also documents that, even though no change to the
userspace datapath (the only user) is required.

Lastly, this patch makes sure the errors returned by ofproto_port_add
(which includes the first port reconfiguration) are reported back to the
database.

Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
---
 lib/netdev-dpdk.c | 70 ++++++++++++++++++++++++-------------------------------
 lib/netdev.c      |  6 ++++-
 vswitchd/bridge.c |  2 ++
 3 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2df3e220c..d6315357b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -746,10 +746,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
     int diag;
     int n_rxq, n_txq;
 
-    if (!rte_eth_dev_is_valid_port(dev->port_id)) {
-        return ENODEV;
-    }
-
     rte_eth_dev_info_get(dev->port_id, &info);
 
     n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
@@ -858,30 +854,23 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int port_no,
     dev->port_id = port_no;
     dev->type = type;
     dev->flags = 0;
-    dev->requested_mtu = dev->mtu = ETHER_MTU;
+    dev->requested_mtu = ETHER_MTU;
     dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
     ovsrcu_index_init(&dev->vid, -1);
     dev->vhost_reconfigured = false;
 
-    err = netdev_dpdk_mempool_configure(dev);
-    if (err) {
-        goto unlock;
-    }
-
     ovsrcu_init(&dev->qos_conf, NULL);
 
     ovsrcu_init(&dev->ingress_policer, NULL);
     dev->policer_rate = 0;
     dev->policer_burst = 0;
 
-    netdev->n_rxq = NR_QUEUE;
-    netdev->n_txq = NR_QUEUE;
-    dev->requested_n_rxq = netdev->n_rxq;
-    dev->requested_n_txq = netdev->n_txq;
-    dev->rxq_size = NIC_PORT_DEFAULT_RXQ_SIZE;
-    dev->txq_size = NIC_PORT_DEFAULT_TXQ_SIZE;
-    dev->requested_rxq_size = dev->rxq_size;
-    dev->requested_txq_size = dev->txq_size;
+    netdev->n_rxq = 0;
+    netdev->n_txq = 0;
+    dev->requested_n_rxq = NR_QUEUE;
+    dev->requested_n_txq = NR_QUEUE;
+    dev->requested_rxq_size = NIC_PORT_DEFAULT_RXQ_SIZE;
+    dev->requested_txq_size = NIC_PORT_DEFAULT_TXQ_SIZE;
 
     /* Initialize the flow control to NULL */
     memset(&dev->fc_conf, 0, sizeof dev->fc_conf);
@@ -891,25 +880,18 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int port_no,
 
     dev->flags = NETDEV_UP | NETDEV_PROMISC;
 
-    if (type == DPDK_DEV_ETH) {
-        if (rte_eth_dev_is_valid_port(dev->port_id)) {
-            err = dpdk_eth_dev_init(dev);
-            if (err) {
-                goto unlock;
-            }
-        }
-        dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
-    } else {
+    if (type == DPDK_DEV_VHOST) {
         dev->tx_q = netdev_dpdk_alloc_txq(OVS_VHOST_MAX_QUEUE_NUM);
-    }
-
-    if (!dev->tx_q) {
-        err = ENOMEM;
-        goto unlock;
+        if (!dev->tx_q) {
+            err = ENOMEM;
+            goto unlock;
+        }
     }
 
     ovs_list_push_back(&dpdk_list, &dev->list_node);
 
+    netdev_request_reconfigure(netdev);
+
 unlock:
     ovs_mutex_unlock(&dev->mutex);
     return err;
@@ -3168,7 +3150,7 @@ out:
     return err;
 }
 
-static void
+static int
 dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
     OVS_REQUIRES(dev->mutex)
 {
@@ -3189,32 +3171,38 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
         }
     }
 
+    if (!dev->dpdk_mp) {
+        return ENOMEM;
+    }
+
     if (netdev_dpdk_get_vid(dev) >= 0) {
         dev->vhost_reconfigured = true;
     }
+
+    return 0;
 }
 
 static int
 netdev_dpdk_vhost_reconfigure(struct netdev *netdev)
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+    int err;
 
     ovs_mutex_lock(&dev->mutex);
-    dpdk_vhost_reconfigure_helper(dev);
+    err = dpdk_vhost_reconfigure_helper(dev);
     ovs_mutex_unlock(&dev->mutex);
-    return 0;
+
+    return err;
 }
 
 static int
 netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev)
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-    int err = 0;
+    int err;
 
     ovs_mutex_lock(&dev->mutex);
 
-    dpdk_vhost_reconfigure_helper(dev);
-
     /* Configure vHost client mode if requested and if the following criteria
      * are met:
      *  1. Device hasn't been registered yet.
@@ -3228,6 +3216,7 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev)
         if (err) {
             VLOG_ERR("vhost-user device setup failure for device %s\n",
                      dev->vhost_id);
+            goto unlock;
         } else {
             /* Configuration successful */
             dev->vhost_driver_flags |= RTE_VHOST_USER_CLIENT;
@@ -3237,9 +3226,12 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev *netdev)
         }
     }
 
+    err = dpdk_vhost_reconfigure_helper(dev);
+
+unlock:
     ovs_mutex_unlock(&dev->mutex);
 
-    return 0;
+    return err;
 }
 
 #define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT,    \
diff --git a/lib/netdev.c b/lib/netdev.c
index 100af0359..f9dba0324 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -331,7 +331,11 @@ netdev_is_reserved_name(const char *name)
  * null.
  *
  * Some network devices may need to be configured (with netdev_set_config())
- * before they can be used. */
+ * before they can be used.
+ *
+ * Before opening rxqs or sending packets, '*netdevp' may need to be
+ * reconfigured (with netdev_is_reconf_required() and netdev_reconfigure()).
+ * */
 int
 netdev_open(const char *name, const char *type, struct netdev **netdevp)
     OVS_EXCLUDED(netdev_mutex)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 7f330706c..4122b5c79 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1762,6 +1762,8 @@ iface_do_create(const struct bridge *br,
     *ofp_portp = iface_pick_ofport(iface_cfg);
     error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
     if (error) {
+        VLOG_WARN_BUF(errp, "could not add network device %s to ofproto (%s)",
+                      iface_cfg->name, ovs_strerror(error));
         goto error;
     }
 
-- 
2.11.0



More information about the dev mailing list