From e1b1d06afdee407ccfa3c4c257b8dcfbcdd004ff Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Fri, 28 Sep 2012 17:56:07 -0700 Subject: [PATCH] Separate OpenFlow port numbers from datapath ones. In a future commit, we will make multiple bridges share a single backing datapath. Our simple mapping from datapath to OpenFlow port numbers won't work, since we'll want the same OpenFlow port numbers on different bridges. For example, the OFPP_LOCAL port must be the same on all bridges, but will have to be a different datapath port on the converged datapath. This commit makes it the responsibility of ofproto to assign the OpenFlow port numbers instead of doing a simple translation from the datapath ones. Signed-off-by: Justin Pettit --- NEWS | 3 + lib/odp-util.h | 27 ------- ofproto/ofproto-dpif-sflow.c | 15 ++-- ofproto/ofproto-dpif-sflow.h | 4 +- ofproto/ofproto-dpif.c | 129 +++++++++++++++++++++++++-------- ofproto/ofproto-provider.h | 20 +++--- ofproto/ofproto.c | 133 +++++++++++++++++++++++++++++++---- tests/learn.at | 3 +- tests/ofproto.at | 2 +- 9 files changed, 248 insertions(+), 88 deletions(-) diff --git a/NEWS b/NEWS index b3f8f3c52..a052f225a 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,9 @@ v1.9.0 - xx xxx xxxx - ovsdb-server now enforces the immutability of immutable columns. This was not enforced in earlier versions due to an oversight. - New support for a nonstandard form of GRE that supports a 64-bit key. + - The ofproto library is now responsible for assigning OpenFlow port + numbers. An ofproto implementation should assign them when + port_construct() is called. - The following features are now deprecated. They will be removed no earlier than February 2013. Please email dev@openvswitch.org with concerns. diff --git a/lib/odp-util.h b/lib/odp-util.h index 358096e73..5cdb20410 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -34,33 +34,6 @@ struct simap; #define OVSP_NONE UINT32_MAX -static inline uint32_t -ofp_port_to_odp_port(uint16_t ofp_port) -{ - switch (ofp_port) { - case OFPP_LOCAL: - return OVSP_LOCAL; - case OFPP_NONE: - case OFPP_CONTROLLER: - return OVSP_NONE; - default: - return ofp_port; - } -} - -static inline uint16_t -odp_port_to_ofp_port(uint32_t odp_port) -{ - switch (odp_port) { - case OVSP_LOCAL: - return OFPP_LOCAL; - case OVSP_NONE: - return OFPP_NONE; - default: - return odp_port; - } -} - void format_odp_actions(struct ds *, const struct nlattr *odp_actions, size_t actions_len); int odp_actions_from_string(const char *, const struct simap *port_names, diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c index 999e55b91..aacb3e81c 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -46,6 +46,7 @@ struct dpif_sflow_port { struct hmap_node hmap_node; /* In struct dpif_sflow's "ports" hmap. */ SFLDataSource_instance dsi; /* sFlow library's notion of port number. */ struct ofport *ofport; /* To retrive port stats. */ + uint32_t odp_port; }; struct dpif_sflow { @@ -148,7 +149,7 @@ dpif_sflow_find_port(const struct dpif_sflow *ds, uint32_t odp_port) HMAP_FOR_EACH_IN_BUCKET (dsp, hmap_node, hash_int(odp_port, 0), &ds->ports) { - if (ofp_port_to_odp_port(dsp->ofport->ofp_port) == odp_port) { + if (dsp->odp_port == odp_port) { return dsp; } } @@ -339,8 +340,7 @@ dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) sflow_agent_get_counters); sfl_poller_set_sFlowCpInterval(poller, ds->options->polling_interval); sfl_poller_set_sFlowCpReceiver(poller, RECEIVER_INDEX); - sfl_poller_set_bridgePort(poller, - ofp_port_to_odp_port(dsp->ofport->ofp_port)); + sfl_poller_set_bridgePort(poller, dsp->odp_port); } static void @@ -353,10 +353,10 @@ dpif_sflow_add_sampler(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) } void -dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport) +dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, + uint32_t odp_port) { struct dpif_sflow_port *dsp; - uint32_t odp_port = ofp_port_to_odp_port(ofport->ofp_port); uint32_t ifindex; dpif_sflow_del_port(ds, odp_port); @@ -368,6 +368,7 @@ dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport) ifindex = (ds->sflow_agent->subId << 16) + odp_port; } dsp->ofport = ofport; + dsp->odp_port = odp_port; SFL_DS_SET(dsp->dsi, 0, ifindex, 0); hmap_insert(&ds->ports, &dsp->hmap_node, hash_int(odp_port, 0)); @@ -491,7 +492,7 @@ dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *ds, void dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, - const struct flow *flow, + const struct flow *flow, uint32_t odp_in_port, const union user_action_cookie *cookie) { SFL_FLOW_SAMPLE_TYPE fs; @@ -507,7 +508,7 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, /* Build a flow sample */ memset(&fs, 0, sizeof fs); - in_dsp = dpif_sflow_find_port(ds, ofp_port_to_odp_port(flow->in_port)); + in_dsp = dpif_sflow_find_port(ds, odp_in_port); if (!in_dsp) { return; } diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h index 3555bc5b7..c7dc872c3 100644 --- a/ofproto/ofproto-dpif-sflow.h +++ b/ofproto/ofproto-dpif-sflow.h @@ -37,7 +37,8 @@ void dpif_sflow_set_options(struct dpif_sflow *, void dpif_sflow_clear(struct dpif_sflow *); bool dpif_sflow_is_enabled(const struct dpif_sflow *); -void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport); +void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, + uint32_t odp_port); void dpif_sflow_del_port(struct dpif_sflow *, uint32_t odp_port); void dpif_sflow_run(struct dpif_sflow *); @@ -46,6 +47,7 @@ void dpif_sflow_wait(struct dpif_sflow *); void dpif_sflow_received(struct dpif_sflow *, struct ofpbuf *, const struct flow *, + uint32_t odp_port, const union user_action_cookie *); int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *, uint32_t); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 6f63f831e..e5767169c 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -485,6 +485,7 @@ static void facet_account(struct facet *); static bool facet_is_controller_flow(struct facet *); struct ofport_dpif { + struct hmap_node odp_port_node; /* In ofproto-dpif's "odp_to_ofport_map". */ struct ofport up; uint32_t odp_port; @@ -543,6 +544,11 @@ static bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *); static void vsp_remove(struct ofport_dpif *); static void vsp_add(struct ofport_dpif *, uint16_t realdev_ofp_port, int vid); +static uint32_t ofp_port_to_odp_port(const struct ofproto_dpif *, + uint16_t ofp_port); +static uint16_t odp_port_to_ofp_port(const struct ofproto_dpif *, + uint32_t odp_port); + static struct ofport_dpif * ofport_dpif_cast(const struct ofport *ofport) { @@ -641,6 +647,9 @@ struct ofproto_dpif { /* VLAN splinters. */ struct hmap realdev_vid_map; /* (realdev,vid) -> vlandev. */ struct hmap vlandev_map; /* vlandev -> (realdev,vid). */ + + /* ODP port to ofp_port mapping. */ + struct hmap odp_to_ofport_map; }; /* Defer flow mod completion until "ovs-appctl ofproto/unclog"? (Useful only @@ -808,6 +817,8 @@ construct(struct ofproto *ofproto_) hmap_init(&ofproto->vlandev_map); hmap_init(&ofproto->realdev_vid_map); + hmap_init(&ofproto->odp_to_ofport_map); + hmap_insert(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node, hash_string(ofproto->up.name, 0)); memset(&ofproto->stats, 0, sizeof ofproto->stats); @@ -932,6 +943,8 @@ destruct(struct ofproto *ofproto_) hmap_destroy(&ofproto->vlandev_map); hmap_destroy(&ofproto->realdev_vid_map); + hmap_destroy(&ofproto->odp_to_ofport_map); + dpif_close(ofproto->dpif); } @@ -1194,9 +1207,10 @@ port_construct(struct ofport *port_) { struct ofport_dpif *port = ofport_dpif_cast(port_); struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto); + struct dpif_port dpif_port; + int error; ofproto->need_revalidate = REV_RECONFIGURE; - port->odp_port = ofp_port_to_odp_port(port->up.ofp_port); port->bundle = NULL; port->cfm = NULL; port->tag = tag_create_random(); @@ -1208,8 +1222,28 @@ port_construct(struct ofport *port_) port->vlandev_vid = 0; port->carrier_seq = netdev_get_carrier_resets(port->up.netdev); + error = dpif_port_query_by_name(ofproto->dpif, + netdev_get_name(port->up.netdev), + &dpif_port); + if (error) { + return error; + } + + port->odp_port = dpif_port.port_no; + + /* Sanity-check that a mapping doesn't already exist. This + * shouldn't happen. */ + if (odp_port_to_ofp_port(ofproto, port->odp_port) != OFPP_NONE) { + VLOG_ERR("port %s already has an OpenFlow port number\n", + dpif_port.name); + return EBUSY; + } + + hmap_insert(&ofproto->odp_to_ofport_map, &port->odp_port_node, + hash_int(port->odp_port, 0)); + if (ofproto->sflow) { - dpif_sflow_add_port(ofproto->sflow, port_); + dpif_sflow_add_port(ofproto->sflow, port_, port->odp_port); } return 0; @@ -1221,6 +1255,7 @@ port_destruct(struct ofport *port_) struct ofport_dpif *port = ofport_dpif_cast(port_); struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto); + hmap_remove(&ofproto->odp_to_ofport_map, &port->odp_port_node); ofproto->need_revalidate = REV_RECONFIGURE; bundle_remove(port_); set_cfm(port_, NULL); @@ -1273,7 +1308,7 @@ set_sflow(struct ofproto *ofproto_, ds = ofproto->sflow = dpif_sflow_create(ofproto->dpif); HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) { - dpif_sflow_add_port(ds, &ofport->up); + dpif_sflow_add_port(ds, &ofport->up, ofport->odp_port); } ofproto->need_revalidate = REV_RECONFIGURE; } @@ -2445,16 +2480,17 @@ get_ofp_port(const struct ofproto_dpif *ofproto, uint16_t ofp_port) static struct ofport_dpif * get_odp_port(const struct ofproto_dpif *ofproto, uint32_t odp_port) { - return get_ofp_port(ofproto, odp_port_to_ofp_port(odp_port)); + return get_ofp_port(ofproto, odp_port_to_ofp_port(ofproto, odp_port)); } static void -ofproto_port_from_dpif_port(struct ofproto_port *ofproto_port, +ofproto_port_from_dpif_port(struct ofproto_dpif *ofproto, + struct ofproto_port *ofproto_port, struct dpif_port *dpif_port) { ofproto_port->name = dpif_port->name; ofproto_port->type = dpif_port->type; - ofproto_port->ofp_port = odp_port_to_ofp_port(dpif_port->port_no); + ofproto_port->ofp_port = odp_port_to_ofp_port(ofproto, dpif_port->port_no); } static void @@ -2527,32 +2563,30 @@ port_query_by_name(const struct ofproto *ofproto_, const char *devname, error = dpif_port_query_by_name(ofproto->dpif, devname, &dpif_port); if (!error) { - ofproto_port_from_dpif_port(ofproto_port, &dpif_port); + ofproto_port_from_dpif_port(ofproto, ofproto_port, &dpif_port); } return error; } static int -port_add(struct ofproto *ofproto_, struct netdev *netdev, uint16_t *ofp_portp) +port_add(struct ofproto *ofproto_, struct netdev *netdev) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); - uint32_t odp_port = *ofp_portp != OFPP_NONE ? *ofp_portp : UINT32_MAX; - int error; + uint32_t odp_port = UINT32_MAX; - error = dpif_port_add(ofproto->dpif, netdev, &odp_port); - if (!error) { - *ofp_portp = odp_port_to_ofp_port(odp_port); - } - return error; + return dpif_port_add(ofproto->dpif, netdev, &odp_port); } static int port_del(struct ofproto *ofproto_, uint16_t ofp_port) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); - int error; + uint32_t odp_port = ofp_port_to_odp_port(ofproto, ofp_port); + int error = 0; - error = dpif_port_del(ofproto->dpif, ofp_port_to_odp_port(ofp_port)); + if (odp_port != OFPP_NONE) { + error = dpif_port_del(ofproto->dpif, odp_port); + } if (!error) { struct ofport_dpif *ofport = get_ofp_port(ofproto, ofp_port); if (ofport) { @@ -2644,11 +2678,12 @@ static int port_dump_next(const struct ofproto *ofproto_ OVS_UNUSED, void *state_, struct ofproto_port *port) { + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); struct port_dump_state *state = state_; struct dpif_port dpif_port; if (dpif_port_dump_next(&state->dump, &dpif_port)) { - ofproto_port_from_dpif_port(port, &dpif_port); + ofproto_port_from_dpif_port(ofproto, port, &dpif_port); return 0; } else { int error = dpif_port_dump_done(&state->dump); @@ -3050,7 +3085,7 @@ ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto, enum odp_key_fitness fitness; fitness = odp_flow_key_to_flow(key, key_len, flow); - flow->in_port = odp_port_to_ofp_port(flow->in_port); + flow->in_port = odp_port_to_ofp_port(ofproto, flow->in_port); if (fitness == ODP_FIT_ERROR) { return fitness; } @@ -3223,6 +3258,7 @@ handle_sflow_upcall(struct ofproto_dpif *ofproto, enum odp_key_fitness fitness; ovs_be16 initial_tci; struct flow flow; + uint32_t odp_in_port; fitness = ofproto_dpif_extract_flow_key(ofproto, upcall->key, upcall->key_len, &flow, @@ -3232,7 +3268,9 @@ handle_sflow_upcall(struct ofproto_dpif *ofproto, } memcpy(&cookie, &upcall->userdata, sizeof(cookie)); - dpif_sflow_received(ofproto->sflow, upcall->packet, &flow, &cookie); + odp_in_port = ofp_port_to_odp_port(ofproto, flow.in_port); + dpif_sflow_received(ofproto->sflow, upcall->packet, &flow, + odp_in_port, &cookie); } static int @@ -3683,7 +3721,8 @@ execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow, int error; ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); - odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(flow->in_port)); + odp_flow_key_from_flow(&key, flow, + ofp_port_to_odp_port(ofproto, flow->in_port)); error = dpif_execute(ofproto->dpif, key.data, key.size, odp_actions, actions_len, packet); @@ -4332,7 +4371,7 @@ subfacet_find(struct ofproto_dpif *ofproto, struct flow flow; fitness = odp_flow_key_to_flow(key, key_len, &flow); - flow.in_port = odp_port_to_ofp_port(flow.in_port); + flow.in_port = odp_port_to_ofp_port(ofproto, flow.in_port); if (fitness == ODP_FIT_ERROR) { return NULL; } @@ -4380,11 +4419,15 @@ static void subfacet_get_key(struct subfacet *subfacet, struct odputil_keybuf *keybuf, struct ofpbuf *key) { + if (!subfacet->key) { + struct ofproto_dpif *ofproto; struct flow *flow = &subfacet->facet->flow; ofpbuf_use_stack(key, keybuf, sizeof *keybuf); - odp_flow_key_from_flow(key, flow, ofp_port_to_odp_port(flow->in_port)); + ofproto = ofproto_dpif_cast(subfacet->facet->rule->up.ofproto); + odp_flow_key_from_flow(key, flow, + ofp_port_to_odp_port(ofproto, flow->in_port)); } else { ofpbuf_use_const(key, subfacet->key, subfacet->key_len); } @@ -4782,7 +4825,8 @@ send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet) } ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); - odp_flow_key_from_flow(&key, &flow, ofp_port_to_odp_port(flow.in_port)); + odp_flow_key_from_flow(&key, &flow, + ofp_port_to_odp_port(ofproto, flow.in_port)); ofpbuf_init(&odp_actions, 32); compose_sflow_action(ofproto, &odp_actions, &flow, odp_port); @@ -4851,7 +4895,7 @@ put_userspace_action(const struct ofproto_dpif *ofproto, uint32_t pid; pid = dpif_port_get_pid(ofproto->dpif, - ofp_port_to_odp_port(flow->in_port)); + ofp_port_to_odp_port(ofproto, flow->in_port)); return odp_put_userspace_action(pid, cookie, odp_actions); } @@ -4959,7 +5003,7 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port, bool check_stp) { const struct ofport_dpif *ofport = get_ofp_port(ctx->ofproto, ofp_port); - uint32_t odp_port = ofp_port_to_odp_port(ofp_port); + uint32_t odp_port = ofp_port_to_odp_port(ctx->ofproto, ofp_port); ovs_be16 flow_vlan_tci = ctx->flow.vlan_tci; uint8_t flow_nw_tos = ctx->flow.nw_tos; uint16_t out_port; @@ -6486,7 +6530,8 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet, struct ofpbuf odp_actions; ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); - odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(flow->in_port)); + odp_flow_key_from_flow(&key, flow, + ofp_port_to_odp_port(ofproto, flow->in_port)); dpif_flow_stats_extract(flow, packet, time_msec(), &stats); @@ -7080,16 +7125,17 @@ vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto, uint32_t realdev_odp_port, ovs_be16 vlan_tci) { if (!hmap_is_empty(&ofproto->realdev_vid_map)) { - uint16_t realdev_ofp_port = odp_port_to_ofp_port(realdev_odp_port); + uint16_t realdev_ofp_port; int vid = vlan_tci_to_vid(vlan_tci); const struct vlan_splinter *vsp; + realdev_ofp_port = odp_port_to_ofp_port(ofproto, realdev_odp_port); HMAP_FOR_EACH_WITH_HASH (vsp, realdev_vid_node, hash_realdev_vid(realdev_ofp_port, vid), &ofproto->realdev_vid_map) { if (vsp->realdev_ofp_port == realdev_ofp_port && vsp->vid == vid) { - return ofp_port_to_odp_port(vsp->vlandev_ofp_port); + return ofp_port_to_odp_port(ofproto, vsp->vlandev_ofp_port); } } } @@ -7204,7 +7250,30 @@ vsp_add(struct ofport_dpif *port, uint16_t realdev_ofp_port, int vid) VLOG_ERR("duplicate vlan device record"); } } - + +static uint32_t +ofp_port_to_odp_port(const struct ofproto_dpif *ofproto, uint16_t ofp_port) +{ + const struct ofport_dpif *ofport = get_ofp_port(ofproto, ofp_port); + return ofport ? ofport->odp_port : OVSP_NONE; +} + +static uint16_t +odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, uint32_t odp_port) +{ + struct ofport_dpif *port; + + HMAP_FOR_EACH_IN_BUCKET (port, odp_port_node, + hash_int(odp_port, 0), + &ofproto->odp_to_ofport_map) { + if (port->odp_port == odp_port) { + return port->up.ofp_port; + } + } + + return OFPP_NONE; +} + const struct ofproto_class ofproto_dpif_class = { init, enumerate_types, diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 8c01b979d..6286d8197 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -27,12 +27,12 @@ #include "ofp-errors.h" #include "ofp-util.h" #include "shash.h" +#include "simap.h" #include "timeval.h" struct match; struct ofpact; struct ofputil_flow_mod; -struct simap; /* An OpenFlow switch. * @@ -62,6 +62,9 @@ struct ofproto { /* Datapath. */ struct hmap ports; /* Contains "struct ofport"s. */ struct shash port_by_name; + unsigned long *ofp_port_ids;/* Bitmap of used OpenFlow port numbers. */ + struct simap ofp_requests; /* OpenFlow port number requests. */ + uint16_t alloc_port_no; /* Last allocated OpenFlow port number. */ uint16_t max_ports; /* Max possible OpenFlow port num, plus one. */ /* Flow tables. */ @@ -536,6 +539,8 @@ struct ofproto_class { /* Life-cycle functions for a "struct ofport" (see "Life Cycle" above). * * ->port_construct() should not modify any base members of the ofport. + * An ofproto implementation should use the 'ofp_port' member of + * "struct ofport" as the OpenFlow port number. * * ofports are managed by the base ofproto code. The ofproto * implementation should only create and destroy them in response to calls @@ -593,18 +598,15 @@ struct ofproto_class { int (*port_query_by_name)(const struct ofproto *ofproto, const char *devname, struct ofproto_port *port); - /* Attempts to add 'netdev' as a port on 'ofproto'. If '*ofp_portp' - * is not OFPP_NONE, attempts to use that as the port's OpenFlow - * port number. - * - * Returns 0 if successful, otherwise a positive errno value. If - * successful, sets '*ofp_portp' to the new port's port number. + /* Attempts to add 'netdev' as a port on 'ofproto'. Returns 0 if + * successful, otherwise a positive errno value. The caller should + * inform the implementation of the OpenFlow port through the + * ->port_construct() method. * * It doesn't matter whether the new port will be returned by a later call * to ->port_poll(); the implementation may do whatever is more * convenient. */ - int (*port_add)(struct ofproto *ofproto, struct netdev *netdev, - uint16_t *ofp_portp); + int (*port_add)(struct ofproto *ofproto, struct netdev *netdev); /* Deletes port number 'ofp_port' from the datapath for 'ofproto'. Returns * 0 if successful, otherwise a positive errno value. diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index ae01c57d9..cd09bbd11 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -221,6 +221,9 @@ static size_t allocated_ofproto_classes; /* Map from datapath name to struct ofproto, for use by unixctl commands. */ static struct hmap all_ofprotos = HMAP_INITIALIZER(&all_ofprotos); +/* Initial mappings of port to OpenFlow number mappings. */ +static struct shash init_ofp_ports = SHASH_INITIALIZER(&init_ofp_ports); + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); /* Must be called to initialize the ofproto library. @@ -235,12 +238,26 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); void ofproto_init(const struct shash *iface_hints) { + struct shash_node *node; size_t i; ofproto_class_register(&ofproto_dpif_class); + /* Make a local copy, since we don't own 'iface_hints' elements. */ + SHASH_FOR_EACH(node, iface_hints) { + const struct iface_hint *orig_hint = node->data; + struct iface_hint *new_hint = xmalloc(sizeof *new_hint); + const char *br_type = ofproto_normalize_type(orig_hint->br_type); + + new_hint->br_name = xstrdup(orig_hint->br_name); + new_hint->br_type = xstrdup(br_type); + new_hint->ofp_port = orig_hint->ofp_port; + + shash_add(&init_ofp_ports, node->name, new_hint); + } + for (i = 0; i < n_ofproto_classes; i++) { - ofproto_classes[i]->init(iface_hints); + ofproto_classes[i]->init(&init_ofp_ports); } } @@ -397,6 +414,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ofproto->frag_handling = OFPC_FRAG_NORMAL; hmap_init(&ofproto->ports); shash_init(&ofproto->port_by_name); + simap_init(&ofproto->ofp_requests); ofproto->max_ports = OFPP_MAX; ofproto->tables = NULL; ofproto->n_tables = 0; @@ -421,6 +439,11 @@ ofproto_create(const char *datapath_name, const char *datapath_type, return error; } + /* The "max_ports" member should have been set by ->construct(ofproto). + * Port 0 is not a valid OpenFlow port, so mark that as unavailable. */ + ofproto->ofp_port_ids = bitmap_allocate(ofproto->max_ports); + bitmap_set1(ofproto->ofp_port_ids, 0); + assert(ofproto->n_tables); ofproto->datapath_id = pick_datapath_id(ofproto); @@ -1015,6 +1038,8 @@ ofproto_destroy__(struct ofproto *ofproto) free(ofproto->dp_desc); hmap_destroy(&ofproto->ports); shash_destroy(&ofproto->port_by_name); + bitmap_free(ofproto->ofp_port_ids); + simap_destroy(&ofproto->ofp_requests); OFPROTO_FOR_EACH_TABLE (table, ofproto) { oftable_destroy(table); @@ -1367,12 +1392,20 @@ ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev, uint16_t ofp_port = ofp_portp ? *ofp_portp : OFPP_NONE; int error; - error = ofproto->ofproto_class->port_add(ofproto, netdev, &ofp_port); + error = ofproto->ofproto_class->port_add(ofproto, netdev); if (!error) { - update_port(ofproto, netdev_get_name(netdev)); + const char *netdev_name = netdev_get_name(netdev); + + simap_put(&ofproto->ofp_requests, netdev_name, ofp_port); + update_port(ofproto, netdev_name); } if (ofp_portp) { - *ofp_portp = error ? OFPP_NONE : ofp_port; + struct ofproto_port ofproto_port; + + ofproto_port_query_by_name(ofproto, netdev_get_name(netdev), + &ofproto_port); + *ofp_portp = error ? OFPP_NONE : ofproto_port.ofp_port; + ofproto_port_destroy(&ofproto_port); } return error; } @@ -1403,8 +1436,14 @@ ofproto_port_del(struct ofproto *ofproto, uint16_t ofp_port) { struct ofport *ofport = ofproto_get_port(ofproto, ofp_port); const char *name = ofport ? netdev_get_name(ofport->netdev) : ""; + struct simap_node *ofp_request_node; int error; + ofp_request_node = simap_find(&ofproto->ofp_requests, name); + if (ofp_request_node) { + simap_delete(&ofproto->ofp_requests, ofp_request_node); + } + error = ofproto->ofproto_class->port_del(ofproto, ofp_port); if (!error && ofport) { /* 'name' is the netdev's name and update_port() is going to close the @@ -1530,12 +1569,57 @@ reinit_ports(struct ofproto *p) sset_destroy(&devnames); } +static uint16_t +alloc_ofp_port(struct ofproto *ofproto, const char *netdev_name) +{ + uint16_t ofp_port; + + ofp_port = simap_get(&ofproto->ofp_requests, netdev_name); + ofp_port = ofp_port ? ofp_port : OFPP_NONE; + + if (ofp_port >= ofproto->max_ports + || bitmap_is_set(ofproto->ofp_port_ids, ofp_port)) { + bool retry = ofproto->alloc_port_no ? true : false; + + /* Search for a free OpenFlow port number. We try not to + * immediately reuse them to prevent problems due to old + * flows. */ + while (ofp_port >= ofproto->max_ports) { + for (ofproto->alloc_port_no++; + ofproto->alloc_port_no < ofproto->max_ports; ) { + if (!bitmap_is_set(ofproto->ofp_port_ids, + ofproto->alloc_port_no)) { + ofp_port = ofproto->alloc_port_no; + break; + } + } + if (ofproto->alloc_port_no >= ofproto->max_ports) { + if (retry) { + ofproto->alloc_port_no = 0; + retry = false; + } else { + return OFPP_NONE; + } + } + } + } + + bitmap_set1(ofproto->ofp_port_ids, ofp_port); + return ofp_port; +} + +static void +dealloc_ofp_port(const struct ofproto *ofproto, uint16_t ofp_port) +{ + bitmap_set0(ofproto->ofp_port_ids, ofp_port); +} + /* Opens and returns a netdev for 'ofproto_port' in 'ofproto', or a null * pointer if the netdev cannot be opened. On success, also fills in * 'opp'. */ static struct netdev * -ofport_open(const struct ofproto *ofproto, - const struct ofproto_port *ofproto_port, +ofport_open(struct ofproto *ofproto, + struct ofproto_port *ofproto_port, struct ofputil_phy_port *pp) { enum netdev_flags flags; @@ -1552,6 +1636,14 @@ ofport_open(const struct ofproto *ofproto, return NULL; } + if (ofproto_port->ofp_port == OFPP_NONE) { + if (!strcmp(ofproto->name, ofproto_port->name)) { + ofproto_port->ofp_port = OFPP_LOCAL; + } else { + ofproto_port->ofp_port = alloc_ofp_port(ofproto, + ofproto_port->name); + } + } pp->port_no = ofproto_port->ofp_port; netdev_get_etheraddr(netdev, pp->hw_addr); ovs_strlcpy(pp->name, ofproto_port->name, sizeof pp->name); @@ -1721,6 +1813,7 @@ static void ofport_destroy(struct ofport *port) { if (port) { + dealloc_ofp_port(port->ofproto, port->ofp_port); port->ofproto->ofproto_class->port_destruct(port); ofport_destroy__(port); } @@ -1814,19 +1907,25 @@ init_ports(struct ofproto *p) { struct ofproto_port_dump dump; struct ofproto_port ofproto_port; + struct shash_node *node, *next; OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, p) { - uint16_t ofp_port = ofproto_port.ofp_port; - if (ofproto_get_port(p, ofp_port)) { - VLOG_WARN_RL(&rl, "%s: ignoring duplicate port %"PRIu16" " - "in datapath", p->name, ofp_port); - } else if (shash_find(&p->port_by_name, ofproto_port.name)) { + const char *name = ofproto_port.name; + + if (shash_find(&p->port_by_name, name)) { VLOG_WARN_RL(&rl, "%s: ignoring duplicate device %s in datapath", - p->name, ofproto_port.name); + p->name, name); } else { struct ofputil_phy_port pp; struct netdev *netdev; + /* Check if an OpenFlow port number had been requested. */ + node = shash_find(&init_ofp_ports, name); + if (node) { + const struct iface_hint *iface_hint = node->data; + simap_put(&p->ofp_requests, name, iface_hint->ofp_port); + } + netdev = ofport_open(p, &ofproto_port, &pp); if (netdev) { ofport_install(p, netdev, &pp); @@ -1834,6 +1933,16 @@ init_ports(struct ofproto *p) } } + SHASH_FOR_EACH_SAFE(node, next, &init_ofp_ports) { + const struct iface_hint *iface_hint = node->data; + + if (!strcmp(iface_hint->br_name, p->name)) { + free(iface_hint->br_name); + free(iface_hint->br_type); + shash_delete(&init_ofp_ports, node); + } + } + return 0; } diff --git a/tests/learn.at b/tests/learn.at index da82f51f2..b2bec0255 100644 --- a/tests/learn.at +++ b/tests/learn.at @@ -161,7 +161,8 @@ AT_SETUP([learning action - fin_timeout feature]) # This is a totally artificial use of the "learn" action. The only purpose # is to check that specifying fin_idle_timeout or fin_hard_timeout causes # a corresponding fin_timeout action to end up in the learned flows. -OVS_VSWITCHD_START +OVS_VSWITCHD_START( + [add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1]) AT_CHECK([[ovs-ofctl add-flow br0 'actions=learn(fin_hard_timeout=10, fin_idle_timeout=5, NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], output:NXM_OF_IN_PORT[])']]) AT_CHECK([ovs-appctl ofproto/trace br0 'in_port(1),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=192.168.0.1,tip=192.168.0.2,op=1,sha=50:54:00:00:00:05,tha=00:00:00:00:00:00)' -generate], [0], [ignore]) AT_CHECK([ovs-ofctl dump-flows br0 table=1 | ofctl_strip], [0], diff --git a/tests/ofproto.at b/tests/ofproto.at index affc702bd..3c270d514 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -676,7 +676,7 @@ priority=0,udp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_ fi # OFPT_PORT_STATUS, OFPPR_ADD - ovs-vsctl add-port br0 test -- set Interface test type=dummy + ovs-vsctl add-port br0 test -- set Interface test type=dummy ofport_request=1 if test X"$1" = X"OFPPR_ADD"; then shift; echo >>expout "OFPT_PORT_STATUS: ADD: 1(test): addr:aa:55:aa:55:00:0x config: PORT_DOWN -- 2.20.1