[ovs-dev] [PATCH] vswitch: allow to specify ofport when creating port

Isaku Yamahata yamahata at valinux.co.jp
Mon Oct 1 08:35:19 UTC 2012


For stable bridge configuration, sometimes it is desirable to be able to
add-port with port number specified.
This patch implements it and it can be done by setting ofport column of Interface
table like

  ovs-vsctl add-port s1 eth2 -- set Interface eth2 ofport=10

Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
---
 ofproto/ofproto-dpif.c |    2 +-
 ofproto/ofproto.c      |    2 +-
 tests/ovs-vsctl.at     |   29 +++++++++++++++++++++++++++++
 vswitchd/bridge.c      |   39 +++++++++++++++++++++++++++------------
 4 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index dcdd8f2..0bba0ec 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_);
-    uint16_t odp_port = UINT16_MAX;
+    uint16_t odp_port = ofp_port_to_odp_port(*ofp_portp);
     int error;
 
     error = dpif_port_add(ofproto->dpif, netdev, &odp_port);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b87b6e7..25b1824 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1353,7 +1353,7 @@ 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/ovs-vsctl.at b/tests/ovs-vsctl.at
index e903619..00e37cc 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1096,3 +1096,32 @@ AT_CHECK([RUN_OVS_VSCTL(
    [-- list Queue])], [0], [], [], [OVS_VSCTL_CLEANUP])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
+
+dnl ----------------------------------------------------------------------
+AT_BANNER([ovs-vsctl unit tests --- add-port with port no specified])
+
+AT_SETUP([add-br a, add-port a a1, set Interface a ofport=10])
+AT_KEYWORDS([ovs-vsctl port-no])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL([add-br a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+AT_CHECK([RUN_OVS_VSCTL([add-port a a0])], [0], [], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [a0])
+CHECK_IFACES([a], [a0])
+AT_CHECK([RUN_OVS_VSCTL(get Interface a0 ofport)], [0], [[[]
+]], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([del-port a a0])], [0], [], [], [OVS_VSCTL_CLEANUP])
+
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
+   [add-port a a1],
+   [set Interface a1 ofport=10])], [0], [
+
+], [], [OVS_VSCTL_CLEANUP])
+CHECK_PORTS([a], [a1])
+CHECK_IFACES([a], [a1])
+AT_CHECK([RUN_OVS_VSCTL(get Interface a1 ofport)], [0], [10
+], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([del-port a a1])], [0], [], [], [OVS_VSCTL_CLEANUP])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 940e5e7..e98ac57 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -65,6 +65,8 @@ COVERAGE_DEFINE(bridge_reconfigure);
 struct if_cfg {
     struct hmap_node hmap_node;         /* Node in bridge's if_cfg_todo. */
     const struct ovsrec_interface *cfg; /* Interface record. */
+    int64_t ofport;                     /* requested ofport. OFPP_NONE means
+                                           auto-allocate ofport */
     const struct ovsrec_port *parent;   /* Parent port record. */
 };
 
@@ -225,7 +227,8 @@ static bool mirror_configure(struct mirror *);
 static void mirror_refresh_stats(struct mirror *);
 
 static void iface_configure_lacp(struct iface *, struct lacp_slave_settings *);
-static bool iface_create(struct bridge *, struct if_cfg *, int ofp_port);
+static bool iface_create(struct bridge *, struct if_cfg *,
+                         int ofp_port, bool allocate_port);
 static const char *iface_get_type(const struct ovsrec_interface *,
                                   const struct ovsrec_bridge *);
 static void iface_destroy(struct iface *);
@@ -495,7 +498,7 @@ bridge_reconfigure_ofp(void)
         struct if_cfg *if_cfg, *next;
 
         HMAP_FOR_EACH_SAFE (if_cfg, next, hmap_node, &br->if_cfg_todo) {
-            iface_create(br, if_cfg, -1);
+            iface_create(br, if_cfg, if_cfg->ofport, true);
             time_refresh();
             if (time_msec() >= deadline) {
                 return false;
@@ -1205,9 +1208,14 @@ bridge_refresh_one_ofp_port(struct bridge *br,
          * and add it.  If that's successful, we'll keep it.  Otherwise, we'll
          * delete it and later try to re-add it. */
         struct if_cfg *if_cfg = if_cfg_lookup(br, name);
+        bool allocate_port = false;
+        if (ofp_port < 0) {
+            ofp_port = OFPP_NONE;
+            allocate_port = true;
+        }
         return (if_cfg
                 && !strcmp(type, iface_get_type(if_cfg->cfg, br->cfg))
-                && iface_create(br, if_cfg, ofp_port));
+                && iface_create(br, if_cfg, ofp_port, allocate_port));
     }
 }
 
@@ -1260,8 +1268,8 @@ bridge_refresh_ofp_port(struct bridge *br)
     }
 }
 
-/* Opens a network device for 'iface_cfg' and configures it.  If '*ofp_portp'
- * is negative, adds the network device to br->ofproto and stores the OpenFlow
+/* Opens a network device for 'iface_cfg' and configures it.  If allocate_port
+ * is true, adds the network device to br->ofproto and stores the OpenFlow
  * port number in '*ofp_portp'; otherwise leaves br->ofproto and '*ofp_portp'
  * untouched.
  *
@@ -1271,7 +1279,8 @@ static int
 iface_do_create(const struct bridge *br,
                 const struct ovsrec_interface *iface_cfg,
                 const struct ovsrec_port *port_cfg,
-                int *ofp_portp, struct netdev **netdevp)
+                int *ofp_portp, bool allocate_port,
+                struct netdev **netdevp)
 {
     struct netdev *netdev;
     int error;
@@ -1289,9 +1298,8 @@ iface_do_create(const struct bridge *br,
         goto error;
     }
 
-    if (*ofp_portp < 0) {
-        uint16_t ofp_port;
-
+    if (allocate_port) {
+        uint16_t ofp_port = *ofp_portp;
         error = ofproto_port_add(br->ofproto, netdev, &ofp_port);
         if (error) {
             goto error;
@@ -1319,13 +1327,14 @@ error:
 }
 
 /* Creates a new iface on 'br' based on 'if_cfg'.  The new iface has OpenFlow
- * port number 'ofp_port'.  If ofp_port is negative, an OpenFlow port is
+ * port number 'ofp_port'. If create is true, an OpenFlow port is
  * automatically allocated for the iface.  Takes ownership of and
  * deallocates 'if_cfg'.
  *
  * Return true if an iface is successfully created, false otherwise. */
 static bool
-iface_create(struct bridge *br, struct if_cfg *if_cfg, int ofp_port)
+iface_create(struct bridge *br, struct if_cfg *if_cfg,
+             int ofp_port, bool allocate_port)
 {
     const struct ovsrec_interface *iface_cfg = if_cfg->cfg;
     const struct ovsrec_port *port_cfg = if_cfg->parent;
@@ -1347,7 +1356,8 @@ 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, iface_cfg, port_cfg, &ofp_port, allocate_port,
+                            &netdev);
     bridge_run_fast();
     if (error) {
         iface_clear_db_record(iface_cfg);
@@ -2450,6 +2460,11 @@ bridge_queue_if_cfg(struct bridge *br,
 
     if_cfg->cfg = cfg;
     if_cfg->parent = parent;
+    if_cfg->ofport = OFPP_NONE;
+    if (cfg->n_ofport > 0 &&
+        (0 <= cfg->ofport[0] && cfg->ofport[0] <= OFPP_MAX)) {
+        if_cfg->ofport = cfg->ofport[0];
+    }
     hmap_insert(&br->if_cfg_todo, &if_cfg->hmap_node,
                 hash_string(if_cfg->cfg->name, 0));
 }
-- 
1.7.10.4




More information about the dev mailing list