datapath: Change ODP_FLOW_GET to retrieve only a single flow at a time.

This brings the code closer to what the Netlink interface will need to
implement.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

9 files changed, 76 insertions, 232 deletions

diff --git a/datapath/datapath.c b/datapath/datapath.c
index da96e3ee..0f59e9ae 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -779,7 +779,6 @@ static void get_stats(struct sw_flow *flow, struct odp_flow_stats *stats)
 	stats->n_bytes = flow->byte_count;
 	stats->reserved = 0;
 	stats->tcp_flags = flow->tcp_flags;
-	stats->error = 0;
 }
 
 static void clear_stats(struct sw_flow *flow)
@@ -1017,55 +1016,25 @@ static int del_flow(struct datapath *dp, struct odp_flow __user *ufp)
 	return error;
 }
 
-static int do_query_flows(struct datapath *dp, const struct odp_flowvec *flowvec)
+static int query_flow(struct datapath *dp, struct odp_flow __user *uflow)
 {
 	struct tbl *table = get_table_protected(dp);
-	u32 i;
-
-	for (i = 0; i < flowvec->n_flows; i++) {
-		struct odp_flow __user *ufp = (struct odp_flow __user __force *)&flowvec->flows[i];
-		struct sw_flow_key key;
-		struct odp_flow uf;
-		struct tbl_node *flow_node;
-		int error;
-
-		if (copy_from_user(&uf, ufp, sizeof(uf)))
-			return -EFAULT;
-
-		error = flow_copy_from_user(&key, (const struct nlattr __force __user *)uf.key, uf.key_len);
-		if (error)
-			return error;
-
-		flow_node = tbl_lookup(table, &uf.key, flow_hash(&key), flow_cmp);
-		if (!flow_node)
-			error = put_user(ENOENT, &ufp->stats.error);
-		else
-			error = answer_query(dp, flow_cast(flow_node), uf.flags, ufp);
-		if (error)
-			return -EFAULT;
-	}
-	return flowvec->n_flows;
-}
-
-static int do_flowvec_ioctl(struct datapath *dp, unsigned long argp,
-			    int (*function)(struct datapath *,
-					    const struct odp_flowvec *))
-{
-	struct odp_flowvec __user *uflowvec;
-	struct odp_flowvec flowvec;
-	int retval;
+	struct tbl_node *flow_node;
+	struct sw_flow_key key;
+	struct odp_flow flow;
+	int error;
 
-	uflowvec = (struct odp_flowvec __user *)argp;
-	if (copy_from_user(&flowvec, uflowvec, sizeof(flowvec)))
+	if (copy_from_user(&flow, uflow, sizeof(flow)))
 		return -EFAULT;
 
-	if (flowvec.n_flows > INT_MAX / sizeof(struct odp_flow))
-		return -EINVAL;
+	error = flow_copy_from_user(&key, (const struct nlattr __force __user *)flow.key, flow.key_len);
+	if (error)
+		return error;
 
-	retval = function(dp, &flowvec);
-	return (retval < 0 ? retval
-		: retval == flowvec.n_flows ? 0
-		: put_user(retval, &uflowvec->n_flows));
+	flow_node = tbl_lookup(table, &flow.key, flow_hash(&key), flow_cmp);
+	if (!flow_node)
+		return -ENOENT;
+	return answer_query(dp, flow_cast(flow_node), flow.flags, uflow);
 }
 
 static struct sw_flow *do_dump_flow(struct datapath *dp, u32 __user *state)
@@ -1769,7 +1738,7 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
 		break;
 
 	case ODP_FLOW_GET:
-		err = do_flowvec_ioctl(dp, argp, do_query_flows);
+		err = query_flow(dp, (struct odp_flow __user *)argp);
 		break;
 
 	case ODP_FLOW_DUMP:
@@ -1870,37 +1839,25 @@ static int compat_del_flow(struct datapath *dp, struct compat_odp_flow __user *u
 	return error;
 }
 
-static int compat_query_flows(struct datapath *dp,
-			      struct compat_odp_flow __user *flows,
-			      u32 n_flows)
+static int compat_query_flow(struct datapath *dp, struct compat_odp_flow __user *uflow)
 {
 	struct tbl *table = get_table_protected(dp);
-	u32 i;
-
-	for (i = 0; i < n_flows; i++) {
-		struct compat_odp_flow __user *ufp = &flows[i];
-		struct odp_flow uf;
-		struct tbl_node *flow_node;
-		struct sw_flow_key key;
-		int error;
+	struct tbl_node *flow_node;
+	struct sw_flow_key key;
+	struct odp_flow flow;
+	int error;
 
-		if (compat_get_flow(&uf, ufp))
-			return -EFAULT;
+	if (compat_get_flow(&flow, uflow))
+		return -EFAULT;
 
-		error = flow_copy_from_user(&key, (const struct nlattr __force __user *) uf.key, uf.key_len);
-		if (error)
-			return error;
+	error = flow_copy_from_user(&key, (const struct nlattr __force __user *)flow.key, flow.key_len);
+	if (error)
+		return error;
 
-		flow_node = tbl_lookup(table, &key, flow_hash(&key), flow_cmp);
-		if (!flow_node)
-			error = put_user(ENOENT, &ufp->stats.error);
-		else
-			error = compat_answer_query(dp, flow_cast(flow_node),
-						    uf.flags, ufp);
-		if (error)
-			return -EFAULT;
-	}
-	return n_flows;
+	flow_node = tbl_lookup(table, &key, flow_hash(&key), flow_cmp);
+	if (!flow_node)
+		return -ENOENT;
+	return compat_answer_query(dp, flow_cast(flow_node), flow.flags, uflow);
 }
 
 static int compat_dump_flow(struct datapath *dp, struct compat_odp_flow_dump __user *udumpp)
@@ -1936,35 +1893,6 @@ static int compat_dump_flow(struct datapath *dp, struct compat_odp_flow_dump __u
 	return compat_answer_query(dp, flow, 0, uflowp);
 }
 
-static int compat_flowvec_ioctl(struct datapath *dp, unsigned long argp,
-				int (*function)(struct datapath *,
-						struct compat_odp_flow __user *,
-						u32 n_flows))
-{
-	struct compat_odp_flowvec __user *uflowvec;
-	struct compat_odp_flow __user *flows;
-	struct compat_odp_flowvec flowvec;
-	int retval;
-
-	uflowvec = compat_ptr(argp);
-	if (!access_ok(VERIFY_WRITE, uflowvec, sizeof(*uflowvec)) ||
-	    copy_from_user(&flowvec, uflowvec, sizeof(flowvec)))
-		return -EFAULT;
-
-	if (flowvec.n_flows > INT_MAX / sizeof(struct compat_odp_flow))
-		return -EINVAL;
-
-	flows = compat_ptr(flowvec.flows);
-	if (!access_ok(VERIFY_WRITE, flows,
-		       flowvec.n_flows * sizeof(struct compat_odp_flow)))
-		return -EFAULT;
-
-	retval = function(dp, flows, flowvec.n_flows);
-	return (retval < 0 ? retval
-		: retval == flowvec.n_flows ? 0
-		: put_user(retval, &uflowvec->n_flows));
-}
-
 static int compat_execute(struct datapath *dp, const struct compat_odp_execute __user *uexecute)
 {
 	struct odp_execute execute;
@@ -2028,7 +1956,7 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
 		break;
 
 	case ODP_FLOW_GET32:
-		err = compat_flowvec_ioctl(dp, argp, compat_query_flows);
+		err = compat_query_flow(dp, compat_ptr(argp));
 		break;
 
 	case ODP_FLOW_DUMP32:
diff --git a/datapath/odp-compat.h b/datapath/odp-compat.h
index 11089ddf..9e74a26d 100644
--- a/datapath/odp-compat.h
+++ b/datapath/odp-compat.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Nicira Networks.
+ * Copyright (c) 2010, 2011 Nicira Networks.
  * Distributed under the terms of the GNU GPL version 2.
  *
  * Significant portions of this file may be copied from parts of the Linux
@@ -15,7 +15,7 @@
 #include "openvswitch/datapath-protocol.h"
 #include <linux/compat.h>
 
-#define ODP_FLOW_GET32		_IOWR('O', 13, struct compat_odp_flowvec)
+#define ODP_FLOW_GET32		_IOWR('O', 13, struct compat_odp_flow)
 #define ODP_FLOW_PUT32		_IOWR('O', 14, struct compat_odp_flow)
 #define ODP_FLOW_DUMP32		_IOWR('O', 15, struct compat_odp_flow_dump)
 #define ODP_FLOW_DEL32		_IOWR('O', 17, struct compat_odp_flow)
@@ -41,11 +41,6 @@ struct compat_odp_flow_dump {
 	uint32_t state[2];
 };
 
-struct compat_odp_flowvec {
-	compat_uptr_t flows;
-	u32 n_flows;
-};
-
 struct compat_odp_execute {
 	compat_uptr_t actions;
 	u32 actions_len;
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index ec089bfe..54c39d1f 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -88,7 +88,7 @@
 #define ODP_VPORT_SET           _IOR('O', 22, struct odp_vport)
 #define ODP_VPORT_DUMP          _IOWR('O', 10, struct odp_vport)
 
-#define ODP_FLOW_GET            _IOWR('O', 13, struct odp_flowvec)
+#define ODP_FLOW_GET            _IOWR('O', 13, struct odp_flow)
 #define ODP_FLOW_PUT            _IOWR('O', 14, struct odp_flow)
 #define ODP_FLOW_DUMP           _IOWR('O', 15, struct odp_flow_dump)
 #define ODP_FLOW_FLUSH          _IO('O', 16)
@@ -214,7 +214,6 @@ struct odp_flow_stats {
     uint32_t used_nsec;
     uint8_t  tcp_flags;
     uint8_t  reserved;
-    uint16_t error;             /* Used by ODP_FLOW_GET. */
 };
 
 enum odp_key_type {
@@ -296,11 +295,6 @@ struct odp_flow_put {
     uint32_t flags;
 };
 
-struct odp_flowvec {
-    struct odp_flow *flows;
-    uint32_t n_flows;
-};
-
 /* ODP_FLOW_DUMP argument.
  *
  * This is used to iterate through the flow table flow-by-flow.  Each
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 6e85d61a..e0619b58 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -459,12 +459,9 @@ dpif_linux_port_poll_wait(const struct dpif *dpif_)
 }
 
 static int
-dpif_linux_flow_get(const struct dpif *dpif_, struct odp_flow flows[], int n)
+dpif_linux_flow_get(const struct dpif *dpif_, struct odp_flow *flow)
 {
-    struct odp_flowvec fv;
-    fv.flows = flows;
-    fv.n_flows = n;
-    return do_ioctl(dpif_, ODP_FLOW_GET, &fv);
+    return do_ioctl(dpif_, ODP_FLOW_GET, flow);
 }
 
 static int
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8852e9d5..8bb0ea4a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -625,26 +625,20 @@ static void
 answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags,
                   struct odp_flow *odp_flow)
 {
-    if (flow) {
-        odp_flow->stats.n_packets = flow->packet_count;
-        odp_flow->stats.n_bytes = flow->byte_count;
-        odp_flow->stats.used_sec = flow->used.tv_sec;
-        odp_flow->stats.used_nsec = flow->used.tv_nsec;
-        odp_flow->stats.tcp_flags = TCP_FLAGS(flow->tcp_ctl);
-        odp_flow->stats.reserved = 0;
-        odp_flow->stats.error = 0;
-        if (odp_flow->actions_len > 0) {
-            memcpy(odp_flow->actions, flow->actions,
-                   MIN(odp_flow->actions_len, flow->actions_len));
-            odp_flow->actions_len = flow->actions_len;
-        }
-
-        if (query_flags & ODPFF_ZERO_TCP_FLAGS) {
-            flow->tcp_ctl = 0;
-        }
+    odp_flow->stats.n_packets = flow->packet_count;
+    odp_flow->stats.n_bytes = flow->byte_count;
+    odp_flow->stats.used_sec = flow->used.tv_sec;
+    odp_flow->stats.used_nsec = flow->used.tv_nsec;
+    odp_flow->stats.tcp_flags = TCP_FLAGS(flow->tcp_ctl);
+    odp_flow->stats.reserved = 0;
+    if (odp_flow->actions_len > 0) {
+        memcpy(odp_flow->actions, flow->actions,
+               MIN(odp_flow->actions_len, flow->actions_len));
+        odp_flow->actions_len = flow->actions_len;
+    }
 
-    } else {
-        odp_flow->stats.error = ENOENT;
+    if (query_flags & ODPFF_ZERO_TCP_FLAGS) {
+        flow->tcp_ctl = 0;
     }
 }
 
@@ -675,25 +669,25 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
 }
 
 static int
-dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow flows[], int n)
+dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow *odp_flow)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
-    int i;
-
-    for (i = 0; i < n; i++) {
-        struct odp_flow *odp_flow = &flows[i];
-        struct flow key;
-        int error;
+    struct dp_netdev_flow *flow;
+    struct flow key;
+    int error;
 
-        error = dpif_netdev_flow_from_nlattrs(odp_flow->key, odp_flow->key_len,
-                                              &key);
-        if (error) {
-            return error;
-        }
+    error = dpif_netdev_flow_from_nlattrs(odp_flow->key, odp_flow->key_len,
+                                          &key);
+    if (error) {
+        return error;
+    }
 
-        answer_flow_query(dp_netdev_lookup_flow(dp, &key),
-                          odp_flow->flags, odp_flow);
+    flow = dp_netdev_lookup_flow(dp, &key);
+    if (!flow) {
+        return ENOENT;
     }
+
+    answer_flow_query(flow, odp_flow->flags, odp_flow);
     return 0;
 }
 
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index f6548b39..2218c112 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -207,34 +207,21 @@ struct dpif_class {
      * value other than EAGAIN. */
     void (*port_poll_wait)(const struct dpif *dpif);
 
-    /* For each flow 'flow' in the 'n' flows in 'flows':
+    /* Queries 'dpif' for a flow entry matching 'flow->key'.
      *
-     * - If a flow matching 'flow->key' exists in 'dpif':
+     * If a flow matching 'flow->key' exists in 'dpif', stores statistics for
+     * the flow into 'flow->stats'.  If 'flow->actions_len' is zero, then
+     * 'flow->actions' is ignored.  If 'flow->actions_len' is nonzero, then
+     * 'flow->actions' should point to an array of the specified number of
+     * bytes.  At most that many bytes of the flow's actions will be copied
+     * into that array.  'flow->actions_len' will be updated to the number of
+     * bytes of actions actually present in the flow, which may be greater than
+     * the amount stored if the flow has more actions than space available in
+     * the array.
      *
-     *     Stores 0 into 'flow->stats.error' and stores statistics for the flow
-     *     into 'flow->stats'.
-     *
-     *     If 'flow->n_actions' is zero, then 'flow->actions' is ignored.  If
-     *     'flow->n_actions' is nonzero, then 'flow->actions' should point to
-     *     an array of the specified number of actions.  At most that many of
-     *     the flow's actions will be copied into that array.
-     *     'flow->n_actions' will be updated to the number of actions actually
-     *     present in the flow, which may be greater than the number stored if
-     *     the flow has more actions than space available in the array.
-     *
-     * - Flow-specific errors are indicated by a positive errno value in
-     *   'flow->stats.error'.  In particular, ENOENT indicates that no flow
-     *   matching 'flow->key' exists in 'dpif'.  When an error value is stored,
-     *   the contents of 'flow->key' are preserved but other members of 'flow'
-     *   should be treated as indeterminate.
-     *
-     * Returns 0 if all 'n' flows in 'flows' were updated (whether they were
-     * individually successful or not is indicated by 'flow->stats.error',
-     * however).  Returns a positive errno value if an error that prevented
-     * this update occurred, in which the caller must not depend on any
-     * elements in 'flows' being updated or not updated.
-     */
-    int (*flow_get)(const struct dpif *dpif, struct odp_flow flows[], int n);
+     * If no flow matching 'flow->key' exists in 'dpif', returns ENOENT.  On
+     * other failure, returns a positive errno value. */
+    int (*flow_get)(const struct dpif *dpif, struct odp_flow *flow);
 
     /* Adds or modifies a flow in 'dpif' as specified in 'put':
      *
diff --git a/lib/dpif.c b/lib/dpif.c
index d7441967..463d4e7f 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -723,10 +723,7 @@ dpif_flow_get(const struct dpif *dpif, struct odp_flow *flow)
     COVERAGE_INC(dpif_flow_get);
 
     check_rw_flow_actions(flow);
-    error = dpif->dpif_class->flow_get(dpif, flow, 1);
-    if (!error) {
-        error = flow->stats.error;
-    }
+    error = dpif->dpif_class->flow_get(dpif, flow);
     if (error) {
         /* Make the results predictable on error. */
         memset(&flow->stats, 0, sizeof flow->stats);
@@ -738,51 +735,6 @@ dpif_flow_get(const struct dpif *dpif, struct odp_flow *flow)
     return error;
 }
 
-/* For each flow 'flow' in the 'n' flows in 'flows':
- *
- * - If a flow matching 'flow->key' exists in 'dpif':
- *
- *     Stores 0 into 'flow->stats.error' and stores statistics for the flow
- *     into 'flow->stats'.
- *
- *     If 'flow->actions_len' is zero, then 'flow->actions' is ignored.  If
- *     'flow->actions_len' is nonzero, then 'flow->actions' should point to an
- *     array of the specified number of bytes.  At most that amount of flow's
- *     actions will be copied into that array.  'flow->actions_len' will be
- *     updated to the number of bytes of actions actually present in the flow,
- *     which may be greater than the amount stored if the flow's actions are
- *     longer than the available space.
- *
- * - Flow-specific errors are indicated by a positive errno value in
- *   'flow->stats.error'.  In particular, ENOENT indicates that no flow
- *   matching 'flow->key' exists in 'dpif'.  When an error value is stored, the
- *   contents of 'flow->key' are preserved but other members of 'flow' should
- *   be treated as indeterminate.
- *
- * Returns 0 if all 'n' flows in 'flows' were updated (whether they were
- * individually successful or not is indicated by 'flow->stats.error',
- * however).  Returns a positive errno value if an error that prevented this
- * update occurred, in which the caller must not depend on any elements in
- * 'flows' being updated or not updated.
- */
-int
-dpif_flow_get_multiple(const struct dpif *dpif,
-                       struct odp_flow flows[], size_t n)
-{
-    int error;
-    size_t i;
-
-    COVERAGE_ADD(dpif_flow_get, n);
-
-    for (i = 0; i < n; i++) {
-        check_rw_flow_actions(&flows[i]);
-    }
-
-    error = dpif->dpif_class->flow_get(dpif, flows, n);
-    log_operation(dpif, "flow_get_multiple", error);
-    return error;
-}
-
 /* Adds or modifies a flow in 'dpif' as specified in 'put':
  *
  * - If the flow specified in 'put->flow' does not exist in 'dpif', then
diff --git a/lib/dpif.h b/lib/dpif.h
index 4efabd0a..3c376ec1 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -111,7 +111,6 @@ int dpif_flow_flush(struct dpif *);
 int dpif_flow_put(struct dpif *, struct odp_flow_put *);
 int dpif_flow_del(struct dpif *, struct odp_flow *);
 int dpif_flow_get(const struct dpif *, struct odp_flow *);
-int dpif_flow_get_multiple(const struct dpif *, struct odp_flow[], size_t n);
 
 struct dpif_flow_dump {
     const struct dpif *dpif;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ea869f1e..d928f686 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3487,9 +3487,7 @@ query_stats(struct ofproto *p, struct rule *rule,
     packet_count = rule->packet_count;
     byte_count = rule->byte_count;
 
-    /* Ask the datapath for statistics on all of the rule's facets.  (We could
-     * batch up statistics requests using dpif_flow_get_multiple(), but that is
-     * not yet implemented.)
+    /* Ask the datapath for statistics on all of the rule's facets.
      *
      * Also, add any statistics that are not tracked by the datapath for each
      * facet.  This includes, for example, statistics for packets that were