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-Tag: v0.90.3~24 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=9ee3ae3e0d89fcd67d04d8a890734a5fbee218a5;p=sliver-openvswitch.git 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 ceaed1537..27bc5ce47 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 09a47d2fc..5558631ec 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 c4619c9be..88bd15dcf 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 c43d4ffa0..0a1d67029 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 9f45b3add..270281832 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 da80eed7e..f0aa9c03c 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);