From: Ben Pfaff Date: Wed, 17 Jun 2009 21:26:19 +0000 (-0700) Subject: datapath: Make the datapath responsible for choosing port numbers. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ee3ae3e0d89fcd67d04d8a890734a5fbee218a5;p=openvswitch 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 ceaed153..27bc5ce4 100644 --- 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 09a47d2f..5558631e 100644 --- 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 c4619c9b..88bd15dc 100644 --- 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 c43d4ffa..0a1d6702 100644 --- 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 9f45b3ad..27028183 100644 --- 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 da80eed7..f0aa9c03 100644 --- 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);