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 <jpettit@nicira.com>

8 files changed, 84 insertions, 24 deletions

diff --git a/NEWS b/NEWS
index 203a66239..b3f8f3c52 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 97a94eb00..b8b144f9f 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 a62473ba7..05885d5cc 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 2fb2fc87b..743db1f10 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 cbf07bc05..affc702bd 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 a481f061a..8240d2503 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 bbfb01f29..12344889d 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 f486d6afd..c181b95ae 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1159,6 +1159,17 @@
         cannot be added then Open vSwitch sets this column
         to -1.</p>
       </column>
+
+      <column name="ofport_request">
+        <p>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 <ref column="ofport"/>
+        column reports the assigned OpenFlow port number.</p>
+        <p>The port number must be requested in the same transaction
+        that creates the port.</p>
+      </column>
     </group>
 
     <group title="System-Specific Details">