[ovs-dev] [PATCH 01/11] dpif-netdev: Proper error handling in do_add_port().

Daniele Di Proietto diproiettod at vmware.com
Sun Feb 28 20:13:13 UTC 2016


This fixes multiple error path mistakes in do_add_port, none of which
has been a problem in practice so far. This change will make it easier
for a following commit to return in case of error.

Also, this removes an unneeded special case for tunnel ports.

Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
---
 lib/dpif-netdev.c | 50 +++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a7e224a..914579d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1106,27 +1106,28 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
     struct netdev *netdev;
     enum netdev_flags flags;
     const char *open_type;
-    int error;
-    int i;
+    int error = 0;
+    int i, n_open_rxqs;
 
     /* Reject devices already in 'dp'. */
     if (!get_port_by_name(dp, devname, &port)) {
-        return EEXIST;
+        error = EEXIST;
+        goto out;
     }
 
     /* Open and validate network device. */
     open_type = dpif_netdev_port_open_type(dp->class, type);
     error = netdev_open(devname, open_type, &netdev);
     if (error) {
-        return error;
+        goto out;
     }
     /* XXX reject non-Ethernet devices */
 
     netdev_get_flags(netdev, &flags);
     if (flags & NETDEV_LOOPBACK) {
         VLOG_ERR("%s: cannot add a loopback device", devname);
-        netdev_close(netdev);
-        return EINVAL;
+        error = EINVAL;
+        goto out_close;
     }
 
     if (netdev_is_pmd(netdev)) {
@@ -1134,7 +1135,8 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
 
         if (n_cores == OVS_CORE_UNSPEC) {
             VLOG_ERR("%s, cannot get cpu core info", devname);
-            return ENOENT;
+            error = ENOENT;
+            goto out_close;
         }
         /* There can only be ovs_numa_get_n_cores() pmd threads,
          * so creates a txq for each, and one extra for the non
@@ -1143,7 +1145,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
                                   netdev_requested_n_rxq(netdev));
         if (error && (error != EOPNOTSUPP)) {
             VLOG_ERR("%s, cannot set multiq", devname);
-            return errno;
+            goto out_close;
         }
     }
     port = xzalloc(sizeof *port);
@@ -1152,30 +1154,20 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
     port->rxq = xmalloc(sizeof *port->rxq * netdev_n_rxq(netdev));
     port->type = xstrdup(type);
     port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
+    n_open_rxqs = 0;
     for (i = 0; i < netdev_n_rxq(netdev); i++) {
         error = netdev_rxq_open(netdev, &port->rxq[i], i);
-        if (error
-            && !(error == EOPNOTSUPP && dpif_netdev_class_is_dummy(dp->class))) {
+        if (error) {
             VLOG_ERR("%s: cannot receive packets on this network device (%s)",
                      devname, ovs_strerror(errno));
-            netdev_close(netdev);
-            free(port->type);
-            free(port->rxq);
-            free(port);
-            return error;
+            goto out_rxq_close;
         }
+        n_open_rxqs++;
     }
 
     error = netdev_turn_flags_on(netdev, NETDEV_PROMISC, &sf);
     if (error) {
-        for (i = 0; i < netdev_n_rxq(netdev); i++) {
-            netdev_rxq_close(port->rxq[i]);
-        }
-        netdev_close(netdev);
-        free(port->type);
-        free(port->rxq);
-        free(port);
-        return error;
+        goto out_rxq_close;
     }
     port->sf = sf;
 
@@ -1188,6 +1180,18 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
     seq_change(dp->port_seq);
 
     return 0;
+
+out_rxq_close:
+    for (i = 0; i < n_open_rxqs; i++) {
+        netdev_rxq_close(port->rxq[i]);
+    }
+    free(port->type);
+    free(port->rxq);
+    free(port);
+out_close:
+    netdev_close(netdev);
+out:
+    return error;
 }
 
 static int
-- 
2.1.4




More information about the dev mailing list