From 3a6ccc8c00697678b920af3d1563aa50891f03a5 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 11 Oct 2010 10:28:28 -0700 Subject: [PATCH] vswitchd: Support changing the type of a bridge port. Until now, if the type of a bridge port changed in the database, then ovs-vswitchd would report an error and keep it the same type. This commit changes the behavior to something more reasonable: the old datapath port is deleted and replaced by a new datapath port of the correct type. --- ofproto/ofproto.c | 36 ++++++++++++++++++++++++++ ofproto/ofproto.h | 2 ++ vswitchd/bridge.c | 66 ++++++++++++++++++++++++++++++++++++----------- 3 files changed, 89 insertions(+), 15 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 23ebae3b..c27dd8d4 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1247,6 +1247,42 @@ ofproto_is_alive(const struct ofproto *p) return !hmap_is_empty(&p->controllers); } +/* Deletes port number 'odp_port' from the datapath for 'ofproto'. + * + * This is almost the same as calling dpif_port_del() directly on the + * datapath, but it also makes 'ofproto' close its open netdev for the port + * (if any). This makes it possible to create a new netdev of a different + * type under the same name, which otherwise the netdev library would refuse + * to do because of the conflict. (The netdev would eventually get closed on + * the next trip through ofproto_run(), but this interface is more direct.) + * + * The caller must be prepared for a callback to its port_changed_cb hook + * function. + * + * Returns 0 if successful, otherwise a positive errno. */ +int +ofproto_port_del(struct ofproto *ofproto, uint16_t odp_port) +{ + struct ofport *ofport = get_port(ofproto, odp_port); + const char *name = ofport ? (char *) ofport->opp.name : ""; + int error; + + error = dpif_port_del(ofproto->dpif, odp_port); + if (error) { + VLOG_ERR("%s: failed to remove port %"PRIu16" (%s) interface (%s)", + dpif_name(ofproto->dpif), odp_port, name, strerror(error)); + } else if (ofport) { + /* 'name' is ofport->opp.name and update_port() is going to destroy + * 'ofport'. Just in case update_port() refers to 'name' after it + * destroys 'ofport', make a copy of it around the update_port() + * call. */ + char *devname = xstrdup(name); + update_port(ofproto, devname); + free(devname); + } + return error; +} + int ofproto_send_packet(struct ofproto *p, const flow_t *flow, const union ofp_action *actions, size_t n_actions, diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 22484519..e39c6c0d 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -99,6 +99,8 @@ int ofproto_run2(struct ofproto *, bool revalidate_all); void ofproto_wait(struct ofproto *); bool ofproto_is_alive(const struct ofproto *); +int ofproto_port_del(struct ofproto *, uint16_t odp_port); + /* Configuration. */ void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id); void ofproto_set_controllers(struct ofproto *, diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index e42d41f4..9a306ab4 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -667,38 +667,58 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) shash_init(&cur_ifaces); for (i = 0; i < n_dpif_ports; i++) { const char *name = dpif_ports[i].devname; - shash_add_once(&cur_ifaces, name, NULL); + shash_add_once(&cur_ifaces, name, &dpif_ports[i]); } - free(dpif_ports); /* Get the set of interfaces we want on this datapath. */ bridge_get_all_ifaces(br, &want_ifaces); + hmap_clear(&br->ifaces); SHASH_FOR_EACH (node, &want_ifaces) { const char *if_name = node->name; struct iface *iface = node->data; - - if (shash_find(&cur_ifaces, if_name)) { - /* Already exists on the datapath. If we have it open, - * reconfigure it; otherwise we'll open it later. */ - if (iface && iface->netdev) { - reconfigure_iface_netdev(iface); + bool internal = !iface || !strcmp(iface->type, "internal"); + struct odp_port *dpif_port = shash_find_data(&cur_ifaces, if_name); + int error; + + /* If we have a port or a netdev already, and it's not the type we + * want, then delete the port (if any) and close the netdev (if + * any). */ + if (internal + ? dpif_port && !(dpif_port->flags & ODP_PORT_INTERNAL) + : (iface->netdev + && strcmp(iface->type, netdev_get_type(iface->netdev)))) + { + if (dpif_port) { + error = ofproto_port_del(br->ofproto, dpif_port->port); + if (error) { + continue; + } + dpif_port = NULL; } - } else { - bool internal = !strcmp(iface->type, "internal"); - int error; + if (iface) { + netdev_close(iface->netdev); + iface->netdev = NULL; + } + } - /* Create interface if it doesn't already exist. */ - if (!internal) { + /* If it's not an internal port, open (possibly create) the + * netdev. */ + if (!internal) { + if (!iface->netdev) { error = create_iface_netdev(iface); if (error) { VLOG_WARN("could not create iface %s: %s", iface->name, strerror(error)); + continue; } - continue; + } else { + reconfigure_iface_netdev(iface); } + } - /* Add to datapath. */ + /* If it's not part of the datapath, add it. */ + if (!dpif_port) { error = dpif_port_add(br->dpif, if_name, internal ? ODP_PORT_INTERNAL : 0, NULL); if (error == EFBIG) { @@ -708,9 +728,25 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) } else if (error) { VLOG_ERR("failed to add %s interface to %s: %s", if_name, dpif_name(br->dpif), strerror(error)); + continue; } } + + /* If it's an internal port, open the netdev. */ + if (internal) { + if (iface && !iface->netdev) { + error = create_iface_netdev(iface); + if (error) { + VLOG_WARN("could not create iface %s: %s", iface->name, + strerror(error)); + continue; + } + } + } else { + assert(iface->netdev != NULL); + } } + free(dpif_ports); shash_destroy(&cur_ifaces); shash_destroy(&want_ifaces); } -- 2.30.2