From: Krishna Kondaka <kkond...@vmware.com>

alloc_ofp_port() does not allocate the port number correctly if the port
number passed initially is already in use. The following if block

if (ofp_port >= ofproto->max_ports
            || bitmap_is_set(ofproto->ofp_port_ids, ofp_port)) {

is entered when either of the two conditions is true but the while block
after this is not entered if the second condition above is true and the
first condition is false.

This results in an existing port_number to be re-assigned!

    Signed-off-by: Krishna Kondaka <kkond...@vmware.com>
---
 AUTHORS           |    1 +
 ofproto/ofproto.c |   35 +++++++++++------------------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 0639f7e..b34287e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -44,6 +44,7 @@ Joe Stringer            j...@wand.net.nz
 Jun Nakajima            jun.nakaj...@intel.com
 Justin Pettit           jpet...@nicira.com
 Keith Amidon            ke...@nicira.com
+Krishna Kondaka         kkond...@vmware.com
 Kyle Mestery            kmest...@cisco.com
 Leo Alterman            lalter...@nicira.com
 Luca Giraudo            lgira...@nicira.com
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 98515c2..514a311 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1613,38 +1613,25 @@ static uint16_t
 alloc_ofp_port(struct ofproto *ofproto, const char *netdev_name)
 {
     uint16_t ofp_port;
+    uint16_t end_port_no = ofproto->alloc_port_no;
 
     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;
-                 ofproto->alloc_port_no++) {
-                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;
-                }
+        do {
+            if (++ofproto->alloc_port_no == ofproto->max_ports)
+                ofproto->alloc_port_no = 0;
+            if (ofproto->alloc_port_no == end_port_no)
+                return OFPP_NONE;
+            if (!bitmap_is_set(ofproto->ofp_port_ids,
+                               ofproto->alloc_port_no)) {
+                ofp_port = ofproto->alloc_port_no;
+                break;
             }
-        }
+        } while (1);
     }
-
     bitmap_set1(ofproto->ofp_port_ids, ofp_port);
     return ofp_port;
 }
-- 
1.7.4.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to