Re: [ovs-dev] [PATCH] dpif-linux: Choose port numbers more prudently.

[Ben Pfaff]

On Tue, Apr 05, 2011 at 06:09:04PM -0700, Ethan Jackson wrote:
> Before this patch the kernel chose the lowest available number for
> newly created datapath ports.  This patch moves the port number
> choosing responsibility to user space, and implements a least
> recently used port number queue in an attempt to avoid reuse.
> 
> Bug #2140.

I think you should add a check for port > ODPP_LOCAL && port <
LRU_MAX_PORTS in dpif_linux_push_port().

Alternatively, here's an incremental that adds a bitmap:

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 464ac88..10ddd51 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -32,6 +32,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "bitmap.h"
 #include "dpif-provider.h"
 #include "netdev.h"
 #include "netdev-vport.h"
@@ -134,6 +135,7 @@ struct dpif_linux {
     bool change_error;
 
     /* Queue of unused ports. */
+    unsigned long *lru_bitmap;
     uint16_t lru_ports[LRU_MAX_PORTS];
     size_t lru_head;
     size_t lru_tail;
@@ -170,31 +172,24 @@ dpif_linux_cast(const struct dpif *dpif)
 static void
 dpif_linux_push_port(struct dpif_linux *dp, uint16_t port)
 {
-    size_t i;
-
-    if (dp->lru_head - dp->lru_tail >= LRU_MAX_PORTS) {
-        return;
-    }
-
-    /* XXX: Replace this loop with a bitmap indicating which ports are in the
-     * queue. */
-    for (i = dp->lru_tail; i != dp->lru_head; i++) {
-        if (dp->lru_ports[i & LRU_MASK] == port) {
-            return;
-        }
+    if (port < LRU_MAX_PORTS && !bitmap_is_set(dp->lru_bitmap, port)) {
+        bitmap_set1(dp->lru_bitmap, port);
+        dp->lru_ports[dp->lru_head++ & LRU_MASK] = port;
     }
-
-    dp->lru_ports[dp->lru_head++ & LRU_MASK] = port;
 }
 
 static uint32_t
 dpif_linux_pop_port(struct dpif_linux *dp)
 {
+    uint16_t port;
+
     if (dp->lru_head == dp->lru_tail) {
         return UINT32_MAX;
     }
 
-    return dp->lru_ports[dp->lru_tail++ & LRU_MASK];
+    port = dp->lru_ports[dp->lru_tail++ & LRU_MASK];
+    bitmap_set0(dp->lru_bitmap, port);
+    return port;
 }
 
 static int
@@ -275,6 +270,8 @@ open_dpif(const struct dpif_linux_dp *dp, struct dpif 
**dpifp)
     *dpifp = &dpif->dpif;
 
     dpif->lru_head = dpif->lru_tail = 0;
+    dpif->lru_bitmap = bitmap_allocate(LRU_MAX_PORTS);
+    bitmap_set1(dpif->lru_bitmap, ODPP_LOCAL);
     for (i = 1; i < LRU_MAX_PORTS; i++) {
         dpif_linux_push_port(dpif, i);
     }
@@ -291,6 +288,7 @@ dpif_linux_close(struct dpif *dpif_)
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     rtnetlink_link_notifier_unregister(&dpif->port_notifier);
     sset_destroy(&dpif->changed_ports);
+    free(dpif->lru_bitmap);
     free(dpif);
 }
 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev