From 81816a5fe8ddd96a296fabc8ea8069e9a19a2631 Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Tue, 2 Oct 2012 22:25:51 -0700 Subject: [PATCH] Allow the OpenFlow port to be requested for a port. A new "ofport_request" column makes it possible to request the OpenFlow port number when adding a port. Signed-off-by: Justin Pettit --- NEWS | 2 ++ ofproto/ofproto-dpif.c | 2 +- ofproto/ofproto-provider.h | 9 ++++++--- ofproto/ofproto.c | 14 +++++++++----- tests/ofproto.at | 27 +++++++++++++++++++++++++++ vswitchd/bridge.c | 32 +++++++++++++++++++------------- vswitchd/vswitch.ovsschema | 11 +++++++++-- vswitchd/vswitch.xml | 11 +++++++++++ 8 files changed, 84 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index 203a6623..b3f8f3c5 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,8 @@ v1.9.0 - xx xxx xxxx - Allow bitwise masking for SHA and THA fields in ARP, SLL and TLL fields in IPv6 neighbor discovery messages, and IPv6 flow label. - Adds support for writing to the metadata field for a flow. + - It is possible to request the OpenFlow port number with the + "ofport_request" column in the Interface table. - ovs-ofctl: - Commands and actions that accept port numbers now also accept keywords that represent those ports (such as LOCAL, NONE, and ALL). This is diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 97a94eb0..b8b144f9 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2531,7 +2531,7 @@ static int port_add(struct ofproto *ofproto_, struct netdev *netdev, uint16_t *ofp_portp) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); - uint32_t odp_port = UINT32_MAX; + uint32_t odp_port = *ofp_portp != OFPP_NONE ? *ofp_portp : UINT32_MAX; int error; error = dpif_port_add(ofproto->dpif, netdev, &odp_port); diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index a62473ba..05885d5c 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -583,9 +583,12 @@ 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'. 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'. 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. * * 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 diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 2fb2fc87..743db1f1 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1345,15 +1345,19 @@ ofproto_port_dump_done(struct ofproto_port_dump *dump) return dump->error == EOF ? 0 : dump->error; } -/* Attempts to add 'netdev' as a port on 'ofproto'. If successful, returns 0 - * and sets '*ofp_portp' to the new port's OpenFlow port number (if 'ofp_portp' - * is non-null). On failure, returns a positive errno value and sets - * '*ofp_portp' to OFPP_NONE (if 'ofp_portp' is non-null). */ +/* Attempts to add 'netdev' as a port on 'ofproto'. If 'ofp_portp' is + * non-null and '*ofp_portp' is not OFPP_NONE, attempts to use that as + * the port's OpenFlow port number. + * + * If successful, returns 0 and sets '*ofp_portp' to the new port's + * OpenFlow port number (if 'ofp_portp' is non-null). On failure, + * returns a positive errno value and sets '*ofp_portp' to OFPP_NONE (if + * 'ofp_portp' is non-null). */ int ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev, uint16_t *ofp_portp) { - uint16_t ofp_port; + uint16_t ofp_port = ofp_portp ? *ofp_portp : OFPP_NONE; int error; error = ofproto->ofproto_class->port_add(ofproto, netdev, &ofp_port); diff --git a/tests/ofproto.at b/tests/ofproto.at index cbf07bc0..affc702b 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -23,6 +23,33 @@ OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0 OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto - set OpenFlow port number]) +OVS_VSWITCHD_START( + [add-port br0 p1 -- set Interface p1 type=dummy --\ + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=99]) +AT_CHECK([ovs-ofctl -vwarn show br0], [0], [stdout]) +AT_CHECK([STRIP_XIDS stdout], [0], [dnl +OFPT_FEATURES_REPLY: dpid:fedcba9876543210 +n_tables:255, n_buffers:256 +capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP +actions: OUTPUT SET_VLAN_VID SET_VLAN_PCP STRIP_VLAN SET_DL_SRC SET_DL_DST SET_NW_SRC SET_NW_DST SET_NW_TOS SET_TP_SRC SET_TP_DST ENQUEUE + 1(p1): addr:aa:55:aa:55:00:01 + config: PORT_DOWN + state: LINK_DOWN + speed: 100 Mbps now, 100 Mbps max + 99(p2): addr:aa:55:aa:55:00:02 + config: PORT_DOWN + state: LINK_DOWN + speed: 100 Mbps now, 100 Mbps max + LOCAL(br0): addr:aa:55:aa:55:00:00 + config: PORT_DOWN + state: LINK_DOWN + speed: 100 Mbps now, 100 Mbps max +OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + dnl This is really bare-bones. dnl It at least checks request and reply serialization and deserialization. AT_SETUP([ofproto - port stats]) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index a481f061..8240d250 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -66,6 +66,7 @@ struct if_cfg { struct hmap_node hmap_node; /* Node in bridge's if_cfg_todo. */ const struct ovsrec_interface *cfg; /* Interface record. */ const struct ovsrec_port *parent; /* Parent port record. */ + int64_t ofport; /* Requested OpenFlow port number. */ }; /* OpenFlow port slated for removal from ofproto. */ @@ -1261,7 +1262,7 @@ bridge_refresh_ofp_port(struct bridge *br) } } -/* Opens a network device for 'iface_cfg' and configures it. If '*ofp_portp' +/* Opens a network device for 'if_cfg' and configures it. If '*ofp_portp' * is negative, adds the network device to br->ofproto and stores the OpenFlow * port number in '*ofp_portp'; otherwise leaves br->ofproto and '*ofp_portp' * untouched. @@ -1270,10 +1271,11 @@ bridge_refresh_ofp_port(struct bridge *br) * failure, returns a positive errno value and stores NULL in '*netdevp'. */ static int iface_do_create(const struct bridge *br, - const struct ovsrec_interface *iface_cfg, - const struct ovsrec_port *port_cfg, + const struct if_cfg *if_cfg, int *ofp_portp, struct netdev **netdevp) { + const struct ovsrec_interface *iface_cfg = if_cfg->cfg; + const struct ovsrec_port *port_cfg = if_cfg->parent; struct netdev *netdev; int error; @@ -1291,7 +1293,7 @@ iface_do_create(const struct bridge *br, } if (*ofp_portp < 0) { - uint16_t ofp_port; + uint16_t ofp_port = if_cfg->ofport; error = ofproto_port_add(br->ofproto, netdev, &ofp_port); if (error) { @@ -1335,11 +1337,7 @@ iface_create(struct bridge *br, struct if_cfg *if_cfg, int ofp_port) struct iface *iface; struct port *port; int error; - - /* Get rid of 'if_cfg' itself. We already copied out the interesting - * bits. */ - hmap_remove(&br->if_cfg_todo, &if_cfg->hmap_node); - free(if_cfg); + bool ok = true; /* Do the bits that can fail up front. * @@ -1348,11 +1346,12 @@ iface_create(struct bridge *br, struct if_cfg *if_cfg, int ofp_port) * additions and deletions are cheaper, these calls should be removed. */ bridge_run_fast(); assert(!iface_lookup(br, iface_cfg->name)); - error = iface_do_create(br, iface_cfg, port_cfg, &ofp_port, &netdev); + error = iface_do_create(br, if_cfg, &ofp_port, &netdev); bridge_run_fast(); if (error) { iface_clear_db_record(iface_cfg); - return false; + ok = false; + goto done; } /* Get or create the port structure. */ @@ -1390,7 +1389,9 @@ iface_create(struct bridge *br, struct if_cfg *if_cfg, int ofp_port) error = netdev_open(port->name, "internal", &netdev); if (!error) { - ofproto_port_add(br->ofproto, netdev, NULL); + uint16_t ofp_port = if_cfg->ofport; + + ofproto_port_add(br->ofproto, netdev, &ofp_port); netdev_close(netdev); } else { VLOG_WARN("could not open network device %s (%s)", @@ -1402,7 +1403,11 @@ iface_create(struct bridge *br, struct if_cfg *if_cfg, int ofp_port) } } - return true; +done: + hmap_remove(&br->if_cfg_todo, &if_cfg->hmap_node); + free(if_cfg); + + return ok; } /* Set Flow eviction threshold */ @@ -2451,6 +2456,7 @@ bridge_queue_if_cfg(struct bridge *br, if_cfg->cfg = cfg; if_cfg->parent = parent; + if_cfg->ofport = cfg->n_ofport_request ? *cfg->ofport_request : OFPP_NONE; hmap_insert(&br->if_cfg_todo, &if_cfg->hmap_node, hash_string(if_cfg->cfg->name, 0)); } diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema index bbfb01f2..12344889 100644 --- a/vswitchd/vswitch.ovsschema +++ b/vswitchd/vswitch.ovsschema @@ -1,6 +1,6 @@ {"name": "Open_vSwitch", - "version": "6.10.0", - "cksum": "3699312094 16958", + "version": "6.11.0", + "cksum": "3699219253 17163", "tables": { "Open_vSwitch": { "columns": { @@ -178,6 +178,13 @@ "ofport": { "type": {"key": "integer", "min": 0, "max": 1}, "ephemeral": true}, + "ofport_request": { + "type": { + "key": {"type": "integer", + "minInteger": 1, + "maxInteger": 65279}, + "min": 0, + "max": 1}}, "cfm_mpid": { "type": { "key": {"type": "integer"}, diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index f486d6af..c181b95a 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -1159,6 +1159,17 @@ cannot be added then Open vSwitch sets this column to -1.

+ + +

Requested OpenFlow port number for this interface. The port + number must be between 1 and 65279, inclusive. Some datapaths + cannot satisfy all requests for particular port numbers. When + this column is empty or the request cannot be fulfilled, the + system will choose a free port. The + column reports the assigned OpenFlow port number.

+

The port number must be requested in the same transaction + that creates the port.

+ -- 2.30.2