On Jul 30, 2012, at 4:02 PM, Ben Pfaff <b...@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 <jpet...@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);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
