datapath: Make the datapath responsible for choosing port numbers.

Soon we will allow for multiple datapath implementations.  By allowing
the datapath to choose the port numbers, we possibly simplify some datapath
implementations, and the datapath's clients don't have to guess (or to
check) what port numbers are free, so this seems like a better way to go.

diff --git a/datapath/datapath.c b/datapath/datapath.c
index ceaed15..27bc5ce 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -370,11 +370,6 @@ static int add_port(int dp_idx, struct odp_port __user *portp)
        if (copy_from_user(&port, portp, sizeof port))
                goto out;
        port.devname[IFNAMSIZ - 1] = '\0';
-       port_no = port.port;
-
-       err = -EINVAL;
-       if (port_no < 0 || port_no >= DP_MAX_PORTS)
-               goto out;
 
        rtnl_lock();
        dp = get_dp_locked(dp_idx);
@@ -382,10 +377,13 @@ static int add_port(int dp_idx, struct odp_port __user *portp)
        if (!dp)
                goto out_unlock_rtnl;
 
-       err = -EEXIST;
-       if (dp->ports[port_no])
-               goto out_unlock_dp;
+       for (port_no = 1; port_no < DP_MAX_PORTS; port_no++)
+               if (!dp->ports[port_no])
+                       goto got_port_no;
+       err = -EXFULL;
+       goto out_unlock_dp;
 
+got_port_no:
        if (!(port.flags & ODP_PORT_INTERNAL)) {
                err = -ENODEV;
                dev = dev_get_by_name(&init_net, port.devname);
@@ -411,6 +409,8 @@ static int add_port(int dp_idx, struct odp_port __user *portp)
        if (dp_add_if_hook)
                dp_add_if_hook(dp->ports[port_no]);
 
+       err = __put_user(port_no, &port.port);
+
 out_put:
        dev_put(dev);
 out_unlock_dp:

diff --git a/lib/dpif.c b/lib/dpif.c
index 09a47d2..5558631 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -243,25 +243,33 @@ dpif_recv_purge(struct dpif *dpif)
 }
 
 int
-dpif_port_add(struct dpif *dpif, const char *devname, uint16_t port_no,
-              uint16_t flags)
+dpif_port_add(struct dpif *dpif, const char *devname, uint16_t flags,
+              uint16_t *port_nop)
 {
     struct odp_port port;
+    uint16_t port_no;
+    int error;
 
     COVERAGE_INC(dpif_port_add);
+
     memset(&port, 0, sizeof port);
     strncpy(port.devname, devname, sizeof port.devname);
-    port.port = port_no;
     port.flags = flags;
-    if (!ioctl(dpif->fd, ODP_PORT_ADD, &port)) {
+
+    error = do_ioctl(dpif, ODP_PORT_ADD, NULL, &port);
+    if (!error) {
+        port_no = port.port;
         VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu16,
                     dpif_name(dpif), devname, port_no);
-        return 0;
     } else {
-        VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port %"PRIu16": %s",
-                     dpif_name(dpif), devname, port_no, strerror(errno));
-        return errno;
+        port_no = UINT16_MAX;
+        VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
+                     dpif_name(dpif), devname, strerror(errno));
     }
+    if (port_nop) {
+        *port_nop = port_no;
+    }
+    return error;
 }
 
 int

diff --git a/lib/dpif.h b/lib/dpif.h
index c4619c9..88bd15d 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -42,8 +42,8 @@ int dpif_get_dp_stats(const struct dpif *, struct odp_stats *);
 int dpif_get_drop_frags(const struct dpif *, bool *drop_frags);
 int dpif_set_drop_frags(struct dpif *, bool drop_frags);
 
-int dpif_port_add(struct dpif *, const char *devname, uint16_t port_no,
-                  uint16_t flags);
+int dpif_port_add(struct dpif *, const char *devname, uint16_t flags,
+                  uint16_t *port_no);
 int dpif_port_del(struct dpif *, uint16_t port_no);
 int dpif_port_query_by_number(const struct dpif *, uint16_t port_no,
                               struct odp_port *);

diff --git a/utilities/ovs-dpctl.8.in b/utilities/ovs-dpctl.8.in
index c43d4ff..0a1d670 100644 (file)
--- a/utilities/ovs-dpctl.8.in
+++ b/utilities/ovs-dpctl.8.in
@@ -70,11 +70,6 @@ A \fInetdev\fR may be followed by a comma-separated list of options.
 The following options are currently supported:
 
 .RS
-.IP "\fBport=\fIportno\fR"
-Specifies \fIportno\fR (a number between 1 and 255) as the port number
-at which \fInetdev\fR will be attached.  By default, \fBadd\-if\fR
-automatically selects the lowest available port number.
-
 .IP "\fBinternal\fR"
 Instead of attaching an existing \fInetdev\fR, creates an internal
 port (analogous to the local port) with that name.

diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 9f45b3a..2702818 100644 (file)
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -243,29 +243,6 @@ query_ports(struct dpif *dpif, struct odp_port **ports, size_t *n_ports)
     qsort(*ports, *n_ports, sizeof **ports, compare_ports);
 }
 
-static uint16_t
-get_free_port(struct dpif *dpif)
-{
-    struct odp_port *ports;
-    size_t n_ports;
-    int port_no;
-
-    query_ports(dpif, &ports, &n_ports);
-    for (port_no = 0; port_no <= UINT16_MAX; port_no++) {
-        size_t i;
-        for (i = 0; i < n_ports; i++) {
-            if (ports[i].port == port_no) {
-                goto next_portno;
-            }
-        }
-        free(ports);
-        return port_no;
-
-    next_portno: ;
-    }
-    ovs_fatal(0, "no free datapath ports");
-}
-
 static void
 do_add_if(int argc UNUSED, char *argv[])
 {
@@ -277,7 +254,6 @@ do_add_if(int argc UNUSED, char *argv[])
     for (i = 2; i < argc; i++) {
         char *save_ptr = NULL;
         char *devname, *suboptions;
-        int port = -1;
         int flags = 0;
         int error;
 
@@ -290,11 +266,9 @@ do_add_if(int argc UNUSED, char *argv[])
         suboptions = strtok_r(NULL, "", &save_ptr);
         if (suboptions) {
             enum {
-                AP_PORT,
                 AP_INTERNAL
             };
             static char *options[] = {
-                "port",
                 "internal"
             };
 
@@ -302,13 +276,6 @@ do_add_if(int argc UNUSED, char *argv[])
                 char *value;
 
                 switch (getsubopt(&suboptions, options, &value)) {
-                case AP_PORT:
-                    if (!value) {
-                        ovs_error(0, "'port' suboption requires a value");
-                    }
-                    port = atoi(value);
-                    break;
-
                 case AP_INTERNAL:
                     flags |= ODP_PORT_INTERNAL;
                     break;
@@ -319,14 +286,10 @@ do_add_if(int argc UNUSED, char *argv[])
                 }
             }
         }
-        if (port < 0) {
-            port = get_free_port(dpif);
-        }
 
-        error = dpif_port_add(dpif, devname, port, flags);
+        error = dpif_port_add(dpif, devname, flags, NULL);
         if (error) {
-            ovs_error(error, "adding %s as port %"PRIu16" of %s failed",
-                      devname, port, argv[1]);
+            ovs_error(error, "adding %s to %s failed", devname, argv[1]);
             failure = true;
         } else if (if_up(devname)) {
             failure = true;

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index da80eed..f0aa9c0 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -438,7 +438,6 @@ bridge_reconfigure(void)
         struct odp_port *dpif_ports;
         size_t n_dpif_ports;
         struct svec cur_ifaces, want_ifaces, add_ifaces;
-        int next_port_no;
 
         dpif_port_list(br->dpif, &dpif_ports, &n_dpif_ports);
         svec_init(&cur_ifaces);
@@ -450,29 +449,20 @@ bridge_reconfigure(void)
         bridge_get_all_ifaces(br, &want_ifaces);
         svec_diff(&want_ifaces, &cur_ifaces, &add_ifaces, NULL, NULL);
 
-        next_port_no = 1;
         for (i = 0; i < add_ifaces.n; i++) {
             const char *if_name = add_ifaces.names[i];
-            for (;;) {
-                int internal = cfg_get_bool(0, "iface.%s.internal", if_name);
-                int error = dpif_port_add(br->dpif, if_name, next_port_no++,
-                                          internal ? ODP_PORT_INTERNAL : 0);
-                if (error != EEXIST) {
-                    if (next_port_no >= 256) {
-                        VLOG_ERR("ran out of valid port numbers on %s",
-                                 dpif_name(br->dpif));
-                        goto out;
-                    }
-                    if (error) {
-                        VLOG_ERR("failed to add %s interface to %s: %s",
-                                 if_name, dpif_name(br->dpif),
-                                 strerror(error));
-                    }
-                    break;
-                }
+            int internal = cfg_get_bool(0, "iface.%s.internal", if_name);
+            int flags = internal ? ODP_PORT_INTERNAL : 0;
+            int error = dpif_port_add(br->dpif, if_name, flags, NULL);
+            if (error == EXFULL) {
+                VLOG_ERR("ran out of valid port numbers on %s",
+                         dpif_name(br->dpif));
+                break;
+            } else if (error) {
+                VLOG_ERR("failed to add %s interface to %s: %s",
+                         if_name, dpif_name(br->dpif), strerror(error));
             }
         }
-    out:
         svec_destroy(&cur_ifaces);
         svec_destroy(&want_ifaces);
         svec_destroy(&add_ifaces);