[ovs-dev] [PATCH 1/2] dpif: Allow the port number to be requested when adding an interface.

[Justin Pettit]

On Jul 30, 2012, at 4:02 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Sat, Jul 28, 2012 at 11:54:09AM -0700, Justin Pettit wrote:
>> The datapath allows requesting a specific port number for a port, but
>> the dpif interface didn't expose it.  This commit adds that support.
>> 
>> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> 
> One oddity is that dpif_port_add() uses 0 as an out-of-band value for
> input and UINT16_MAX as an out-of-band value for output.  I think we
> could use UINT16_MAX (or even 0) in both places, for a possibly less
> confusing interface.

Good point.  I'll stick with UINT16_MAX.

> In dpif_port_add() don't we need to initialize port_no in the case
> where port_nop is NULL?

Whoops.  Thanks.

> Otherwise looks good, thanks.


Thanks.  I've appended an incremental.

--Justin


-=-=-=-=-=-=-=-=-=-=-

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index f1824c2..cc00536 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -429,7 +429,8 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netde
     do {
         uint32_t upcall_pid;
 
-        request.port_no = *port_nop ? *port_nop : ++dpif->alloc_port_no;
+        request.port_no = *port_nop != UINT16_MAX ? *port_nop
+                          : ++dpif->alloc_port_no;
         upcall_pid = dpif_linux_port_get_pid(dpif_, request.port_no);
         request.upcall_pid = &upcall_pid;
         error = dpif_linux_vport_transact(&request, &reply, &buf);
@@ -442,13 +443,13 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *net
             /* Older datapath has lower limit. */
             max_ports = dpif->alloc_port_no;
             dpif->alloc_port_no = 0;
-        } else if (error == EBUSY && *port_nop) {
+        } else if (error == EBUSY && *port_nop != UINT16_MAX) {
             VLOG_INFO("%s: requested port %"PRIu16" is in use",
                      dpif_name(dpif_), *port_nop);
         }
 
         ofpbuf_delete(buf);
-    } while ((!*port_nop) && (i++ < max_ports)
+    } while ((*port_nop != UINT16_MAX) && (i++ < max_ports)
              && (error == EBUSY || error == EFBIG));
 
     return error;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 94b8d36..60fae5f 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -407,7 +407,7 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netde
     struct dp_netdev *dp = get_dp_netdev(dpif);
     int port_no;
 
-    if (*port_nop) {
+    if (*port_nop != UINT16_MAX) {
         if (*port_nop >= MAX_PORTS) {
             return EFBIG;
         } else if (dp->ports[*port_nop]) {
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 8c6b1e6..317e617 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -110,8 +110,8 @@ struct dpif_class {
     /* Retrieves statistics for 'dpif' into 'stats'. */
     int (*get_stats)(const struct dpif *dpif, struct dpif_dp_stats *stats);
 
-    /* Adds 'netdev' as a new port in 'dpif'.  If '*port_no' is
-     * non-zero, attempts to use that as the port's port number.
+    /* Adds 'netdev' as a new port in 'dpif'.  If '*port_no' is not
+     * UINT16_MAX, attempts to use that as the port's port number.
      *
      * If port is successfully added, sets '*port_no' to the new port's
      * port number.  Returns EBUSY if caller attempted to choose a port
diff --git a/lib/dpif.c b/lib/dpif.c
index 6b661bb..667e07c 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -411,8 +411,8 @@ dpif_get_dp_stats(const struct dpif *dpif, struct dpif_dp_st
 }
 
 /* Attempts to add 'netdev' as a port on 'dpif'.  If 'port_nop' is
- * non-null and dereferences to a non-zero value, then attempts to use
- * the value as the port number.
+ * non-null and its value is not UINT16_MAX, then attempts to use the
+ * value as the port number.
  *
  * If successful, returns 0 and sets '*port_nop' to the new port's port
  * number (if 'port_nop' is non-null).  On failure, returns a positive
@@ -422,7 +422,7 @@ int
 dpif_port_add(struct dpif *dpif, struct netdev *netdev, uint16_t *port_nop)
 {
     const char *netdev_name = netdev_get_name(netdev);
-    uint16_t port_no;
+    uint16_t port_no = UINT16_MAX;
     int error;
 
     COVERAGE_INC(dpif_port_add);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 99105b2..76addfb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2514,7 +2514,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 = 0;
+    uint16_t odp_port = UINT16_MAX;
     int error;
 
     error = dpif_port_add(ofproto->dpif, netdev, &odp_port);