This cleans up the dpif interface to make it more consistent with the
other dpif operations, and allows flows to be fetched in batches.
Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
v2: Require callers to pass down an initialized buffer.
Remove NLM_F_ECHO flag from get request.
Rebase against master.
RFC: Initial post under "dpif: Allow batching flow_get."
---
lib/dpif-linux.c | 85 +++++++++++++++++++++-------------------
lib/dpif-netdev.c | 60 ++++++++++++++--------------
lib/dpif-provider.h | 31 ---------------
lib/dpif.c | 87 ++++++++++++++++++-----------------------
lib/dpif.h | 33 +++++++++++++++-
ofproto/ofproto-dpif-upcall.c | 6 ++-
6 files changed, 148 insertions(+), 154 deletions(-)
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 6d461b2..e767d9f 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -126,6 +126,8 @@ static int dpif_linux_flow_transact(struct dpif_linux_flow
*request,
struct ofpbuf **bufp);
static void dpif_linux_flow_get_stats(const struct dpif_linux_flow *,
struct dpif_flow_stats *);
+static void dpif_linux_flow_to_dpif_flow(struct dpif_flow *,
+ const struct dpif_linux_flow *);
/* One of the dpif channels between the kernel and userspace. */
struct dpif_channel {
@@ -1046,48 +1048,27 @@ dpif_linux_port_poll_wait(const struct dpif *dpif_)
}
}
-static int
-dpif_linux_flow_get__(const struct dpif_linux *dpif,
- const struct nlattr *key, size_t key_len,
- struct dpif_linux_flow *reply, struct ofpbuf **bufp)
+static void
+dpif_linux_init_flow_get(const struct dpif_linux *dpif,
+ const struct nlattr *key, size_t key_len,
+ struct dpif_linux_flow *request)
{
- struct dpif_linux_flow request;
-
- dpif_linux_flow_init(&request);
- request.cmd = OVS_FLOW_CMD_GET;
- request.dp_ifindex = dpif->dp_ifindex;
- request.key = key;
- request.key_len = key_len;
- return dpif_linux_flow_transact(&request, reply, bufp);
+ dpif_linux_flow_init(request);
+ request->cmd = OVS_FLOW_CMD_GET;
+ request->dp_ifindex = dpif->dp_ifindex;
+ request->key = key;
+ request->key_len = key_len;
}
static int
-dpif_linux_flow_get(const struct dpif *dpif_,
+dpif_linux_flow_get(const struct dpif_linux *dpif,
const struct nlattr *key, size_t key_len,
- struct ofpbuf **bufp,
- struct nlattr **maskp, size_t *mask_len,
- struct nlattr **actionsp, size_t *actions_len,
- struct dpif_flow_stats *stats)
+ struct dpif_linux_flow *reply, struct ofpbuf **bufp)
{
- const struct dpif_linux *dpif = dpif_linux_cast(dpif_);
- struct dpif_linux_flow reply;
- int error;
+ struct dpif_linux_flow request;
- error = dpif_linux_flow_get__(dpif, key, key_len, &reply, bufp);
- if (!error) {
- if (maskp) {
- *maskp = CONST_CAST(struct nlattr *, reply.mask);
- *mask_len = reply.mask_len;
- }
- if (actionsp) {
- *actionsp = CONST_CAST(struct nlattr *, reply.actions);
- *actions_len = reply.actions_len;
- }
- if (stats) {
- dpif_linux_flow_get_stats(&reply, stats);
- }
- }
- return error;
+ dpif_linux_init_flow_get(dpif, key, key_len, &request);
+ return dpif_linux_flow_transact(&request, reply, bufp);
}
static void
@@ -1217,7 +1198,7 @@ dpif_linux_flow_dump_thread_destroy(struct
dpif_flow_dump_thread *thread_)
static void
dpif_linux_flow_to_dpif_flow(struct dpif_flow *dpif_flow,
- struct dpif_linux_flow *linux_flow)
+ const struct dpif_linux_flow *linux_flow)
{
dpif_flow->key = linux_flow->key;
dpif_flow->key_len = linux_flow->key_len;
@@ -1266,9 +1247,9 @@ dpif_linux_flow_dump_next(struct dpif_flow_dump_thread
*thread_,
} else {
/* Rare case: the flow does not include actions. Retrieve this
* individual flow again to get the actions. */
- error = dpif_linux_flow_get__(dpif, linux_flow.key,
- linux_flow.key_len, &linux_flow,
- &thread->nl_actions);
+ error = dpif_linux_flow_get(dpif, linux_flow.key,
+ linux_flow.key_len, &linux_flow,
+ &thread->nl_actions);
if (error == ENOENT) {
VLOG_DBG("dumped flow disappeared on get");
continue;
@@ -1344,6 +1325,7 @@ dpif_linux_operate__(struct dpif_linux *dpif,
struct dpif_flow_put *put;
struct dpif_flow_del *del;
struct dpif_execute *execute;
+ struct dpif_flow_get *get;
struct dpif_linux_flow flow;
ofpbuf_use_stub(&aux->request,
@@ -1380,6 +1362,13 @@ dpif_linux_operate__(struct dpif_linux *dpif,
&aux->request);
break;
+ case DPIF_OP_FLOW_GET:
+ get = &op->u.flow_get;
+ dpif_linux_init_flow_get(dpif, get->key, get->key_len, &flow);
+ aux->txn.reply = get->buffer;
+ dpif_linux_flow_to_ofpbuf(&flow, &aux->request);
+ break;
+
default:
OVS_NOT_REACHED();
}
@@ -1396,6 +1385,7 @@ dpif_linux_operate__(struct dpif_linux *dpif,
struct dpif_op *op = ops[i];
struct dpif_flow_put *put;
struct dpif_flow_del *del;
+ struct dpif_flow_get *get;
op->error = txn->error;
@@ -1441,6 +1431,22 @@ dpif_linux_operate__(struct dpif_linux *dpif,
case DPIF_OP_EXECUTE:
break;
+ case DPIF_OP_FLOW_GET:
+ get = &op->u.flow_get;
+ if (!op->error) {
+ struct dpif_linux_flow reply;
+
+ op->error = dpif_linux_flow_from_ofpbuf(&reply, txn->reply);
+ if (!op->error) {
+ dpif_linux_flow_to_dpif_flow(get->flow, &reply);
+ }
+ }
+
+ if (op->error) {
+ memset(get->flow, 0, sizeof *get->flow);
+ }
+ break;
+
default:
OVS_NOT_REACHED();
}
@@ -1867,7 +1873,6 @@ const struct dpif_class dpif_linux_class = {
dpif_linux_port_dump_done,
dpif_linux_port_poll,
dpif_linux_port_poll_wait,
- dpif_linux_flow_get,
dpif_linux_flow_flush,
dpif_linux_flow_dump_create,
dpif_linux_flow_dump_destroy,
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0eea3ab..ba54e3b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1050,7 +1050,7 @@ dp_netdev_find_flow(const struct dp_netdev *dp, const
struct flow *flow)
}
static void
-get_dpif_flow_stats(struct dp_netdev_flow *netdev_flow,
+get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow,
struct dpif_flow_stats *stats)
{
struct dp_netdev_flow_stats *bucket;
@@ -1067,6 +1067,27 @@ get_dpif_flow_stats(struct dp_netdev_flow *netdev_flow,
}
}
+static void
+dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
+ struct ofpbuf *buffer, struct dpif_flow *flow)
+{
+ struct flow_wildcards wc;
+ struct dp_netdev_actions *actions;
+
+ minimask_expand(&netdev_flow->cr.match.mask, &wc);
+ odp_flow_key_from_mask(buffer, &wc.masks, &netdev_flow->flow,
+ odp_to_u32(wc.masks.in_port.odp_port),
+ SIZE_MAX, true);
+ flow->mask = ofpbuf_data(buffer);
+ flow->mask_len = ofpbuf_size(buffer);
+
+ actions = dp_netdev_flow_get_actions(netdev_flow);
+ flow->actions = actions->actions;
+ flow->actions_len = actions->size;
+
+ get_dpif_flow_stats(netdev_flow, &flow->stats);
+}
+
static int
dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
const struct nlattr *mask_key,
@@ -1160,19 +1181,14 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key,
uint32_t key_len,
}
static int
-dpif_netdev_flow_get(const struct dpif *dpif,
- const struct nlattr *nl_key, size_t nl_key_len,
- struct ofpbuf **bufp,
- struct nlattr **maskp, size_t *mask_len,
- struct nlattr **actionsp, size_t *actions_len,
- struct dpif_flow_stats *stats)
+dpif_netdev_flow_get(const struct dpif *dpif, const struct dpif_flow_get *get)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
struct dp_netdev_flow *netdev_flow;
struct flow key;
int error;
- error = dpif_netdev_flow_from_nlattrs(nl_key, nl_key_len, &key);
+ error = dpif_netdev_flow_from_nlattrs(get->key, get->key_len, &key);
if (error) {
return error;
}
@@ -1180,28 +1196,7 @@ dpif_netdev_flow_get(const struct dpif *dpif,
netdev_flow = dp_netdev_find_flow(dp, &key);
if (netdev_flow) {
- if (stats) {
- get_dpif_flow_stats(netdev_flow, stats);
- }
-
- if (maskp) {
- struct flow_wildcards wc;
-
- *bufp = ofpbuf_new(sizeof(struct odputil_keybuf));
- minimask_expand(&netdev_flow->cr.match.mask, &wc);
- odp_flow_key_from_mask(*bufp, &wc.masks, &netdev_flow->flow,
- odp_to_u32(wc.masks.in_port.odp_port),
- SIZE_MAX, true);
- *maskp = ofpbuf_data(*bufp);
- *mask_len = ofpbuf_size(*bufp);
- }
- if (actionsp) {
- struct dp_netdev_actions *actions;
-
- actions = dp_netdev_flow_get_actions(netdev_flow);
- *actionsp = actions->actions;
- *actions_len = actions->size;
- }
+ dp_netdev_flow_to_dpif_flow(netdev_flow, get->buffer, get->flow);
} else {
error = ENOENT;
}
@@ -1534,6 +1529,10 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op
**ops, size_t n_ops)
case DPIF_OP_EXECUTE:
op->error = dpif_netdev_execute(dpif, &op->u.execute);
break;
+
+ case DPIF_OP_FLOW_GET:
+ op->error = dpif_netdev_flow_get(dpif, &op->u.flow_get);
+ break;
}
}
}
@@ -2231,7 +2230,6 @@ const struct dpif_class dpif_netdev_class = {
dpif_netdev_port_dump_done,
dpif_netdev_port_poll,
dpif_netdev_port_poll_wait,
- dpif_netdev_flow_get,
dpif_netdev_flow_flush,
dpif_netdev_flow_dump_create,
dpif_netdev_flow_dump_destroy,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 9f6ef04..7966b83 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -239,37 +239,6 @@ struct dpif_class {
* value other than EAGAIN. */
void (*port_poll_wait)(const struct dpif *dpif);
- /* Queries 'dpif' for a flow entry. The flow is specified by the Netlink
- * attributes with types OVS_KEY_ATTR_* in the 'key_len' bytes starting at
- * 'key'.
- *
- * Returns 0 if successful. If no flow matches, returns ENOENT. On other
- * failure, returns a positive errno value.
- *
- * On success, '*bufp' will be set to an ofpbuf owned by the caller that
- * contains the response for 'maskp' and/or 'actionsp'. The caller must
- * supply a valid pointer, and must free the ofpbuf (with ofpbuf_delete())
- * when it is no longer needed.
- *
- * If 'maskp' is nonnull, then on success '*maskp' will point to the
- * Netlink attributes for the flow's mask. '*mask_len' will be set to the
- * length of the mask attributes. Implementations may opt to point 'maskp'
- * at RCU-protected data rather than making a copy in '*bufp'.
- *
- * If 'actionsp' is nonnull, then on success '*actionsp' will point to the
- * Netlink attributes for the flow's actions. '*actions_len' will be set to
- * the length of the actions attributes. Implementations may opt to point
- * 'actionsp' at RCU-protected data rather than making a copy in '*bufp'.
- *
- * If 'stats' is nonnull, then on success it must be updated with the
- * flow's statistics. */
- int (*flow_get)(const struct dpif *dpif,
- const struct nlattr *key, size_t key_len,
- struct ofpbuf **bufp,
- struct nlattr **maskp, size_t *mask_len,
- struct nlattr **actionsp, size_t *acts_len,
- struct dpif_flow_stats *stats);
-
/* Deletes all flows from 'dpif' and clears all of its queues of received
* packets. */
int (*flow_flush)(struct dpif *dpif);
diff --git a/lib/dpif.c b/lib/dpif.c
index 1f15840..8ba889f 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -98,6 +98,8 @@ static void log_flow_del_message(struct dpif *, const struct
dpif_flow_del *,
int error);
static void log_execute_message(struct dpif *, const struct dpif_execute *,
bool subexecute, int error);
+static void log_flow_get_message(const struct dpif *,
+ const struct dpif_flow_get *, int error);
static void
dp_initialize(void)
@@ -829,59 +831,27 @@ dpif_flow_flush(struct dpif *dpif)
return error;
}
-/* Queries 'dpif' for a flow entry. The flow is specified by the Netlink
- * attributes with types OVS_KEY_ATTR_* in the 'key_len' bytes starting at
- * 'key'.
- *
- * Returns 0 if successful. If no flow matches, returns ENOENT. On other
- * failure, returns a positive errno value.
- *
- * On success, '*bufp' will be set to an ofpbuf owned by the caller that
- * contains the response for 'flow->mask' and 'flow->actions'. The caller must
- * supply a valid pointer, and must free the ofpbuf (with ofpbuf_delete()) when
- * it is no longer needed.
- *
- * On success, 'flow' will be populated with the mask, actions and stats for
- * the datapath flow corresponding to 'key'. The mask and actions will point
- * within '*bufp'.
- *
- * Implementations may opt to point 'flow->mask' and/or 'flow->actions' at
- * RCU-protected data rather than making a copy of them. Therefore, callers
- * that wish to hold these over quiescent periods must make a copy of these
- * fields before quiescing. */
+/* A dpif_operate() wrapper for performing a single DPIF_OP_FLOW_GET. */
int
-dpif_flow_get(const struct dpif *dpif,
+dpif_flow_get(struct dpif *dpif,
const struct nlattr *key, size_t key_len,
- struct ofpbuf **bufp, struct dpif_flow *flow)
+ struct ofpbuf *buf, struct dpif_flow *flow)
{
- int error;
- struct nlattr *mask, *actions;
- size_t mask_len, actions_len;
- struct dpif_flow_stats stats;
+ struct dpif_op *opp;
+ struct dpif_op op;
- COVERAGE_INC(dpif_flow_get);
+ op.type = DPIF_OP_FLOW_GET;
+ op.u.flow_get.key = key;
+ op.u.flow_get.key_len = key_len;
+ op.u.flow_get.buffer = buf;
+ op.u.flow_get.flow = flow;
+ op.u.flow_get.flow->key = key;
+ op.u.flow_get.flow->key_len = key_len;
- *bufp = NULL;
- error = dpif->dpif_class->flow_get(dpif, key, key_len, bufp,
- &mask, &mask_len,
- &actions, &actions_len, &stats);
- if (error) {
- memset(flow, 0, sizeof *flow);
- ofpbuf_delete(*bufp);
- *bufp = NULL;
- } else {
- flow->mask = mask;
- flow->mask_len = mask_len;
- flow->actions = actions;
- flow->actions_len = actions_len;
- flow->stats = stats;
- }
- if (should_log_flow_message(error)) {
- log_flow_message(dpif, error, "flow_get", key, key_len,
- NULL, 0, &flow->stats,
- flow->actions, flow->actions_len);
- }
- return error;
+ opp = &op;
+ dpif_operate(dpif, &opp, 1);
+
+ return op.error;
}
/* A dpif_operate() wrapper for performing a single DPIF_OP_FLOW_PUT. */
@@ -1177,6 +1147,14 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops,
size_t n_ops)
break;
}
+ case DPIF_OP_FLOW_GET: {
+ struct dpif_flow_get *get = &op->u.flow_get;
+
+ COVERAGE_INC(dpif_flow_get);
+ log_flow_get_message(dpif, get, error);
+ break;
+ }
+
case DPIF_OP_FLOW_DEL: {
struct dpif_flow_del *del = &op->u.flow_del;
@@ -1574,3 +1552,16 @@ log_execute_message(struct dpif *dpif, const struct
dpif_execute *execute,
free(packet);
}
}
+
+static void
+log_flow_get_message(const struct dpif *dpif, const struct dpif_flow_get *get,
+ int error)
+{
+ if (should_log_flow_message(error)) {
+ log_flow_message(dpif, error, "flow_get",
+ get->key, get->key_len,
+ get->flow->mask, get->flow->mask_len,
+ &get->flow->stats,
+ get->flow->actions, get->flow->actions_len);
+ }
+}
diff --git a/lib/dpif.h b/lib/dpif.h
index 64fe110..4a21a59 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -522,9 +522,9 @@ int dpif_flow_put(struct dpif *, enum dpif_flow_put_flags,
int dpif_flow_del(struct dpif *,
const struct nlattr *key, size_t key_len,
struct dpif_flow_stats *);
-int dpif_flow_get(const struct dpif *,
+int dpif_flow_get(struct dpif *,
const struct nlattr *key, size_t key_len,
- struct ofpbuf **, struct dpif_flow *);
+ struct ofpbuf *, struct dpif_flow *);
/* Flow dumping interface
* ======================
@@ -573,6 +573,8 @@ struct dpif_flow {
};
int dpif_flow_dump_next(struct dpif_flow_dump_thread *,
struct dpif_flow *flows, int max_flows);
+
+#define DPIF_FLOW_BUFSIZE 2048
/* Operation batching interface.
*
@@ -584,6 +586,7 @@ enum dpif_op_type {
DPIF_OP_FLOW_PUT = 1,
DPIF_OP_FLOW_DEL,
DPIF_OP_EXECUTE,
+ DPIF_OP_FLOW_GET,
};
/* Add or modify a flow.
@@ -664,6 +667,31 @@ struct dpif_execute {
struct pkt_metadata md; /* Packet metadata. */
};
+/* Queries the dpif for a flow entry.
+ *
+ * The flow is specified by the Netlink attributes with types OVS_KEY_ATTR_* in
+ * the 'key_len' bytes starting at 'key'. 'buffer' must point to an initialized
+ * buffer of size DPIF_FLOW_BUFSIZE bytes.
+ *
+ * On success, 'flow' will be populated with the mask, actions and stats for
+ * the datapath flow corresponding to 'key'. The mask and actions may point
+ * within '*buffer', or may point at RCU-protected data. Therefore, callers
+ * that wish to hold these over quiescent periods must make a copy of these
+ * fields before quiescing.
+ *
+ * Succeeds with status 0 if the flow is fetched, or fails with ENOENT if no
+ * such flow exists. Other failures are indicated with a positive errno value.
+ */
+struct dpif_flow_get {
+ /* Input. */
+ const struct nlattr *key; /* Flow to get. */
+ size_t key_len; /* Length of 'key' in bytes. */
+ struct ofpbuf *buffer; /* Storage for output parameters. */
+
+ /* Output. */
+ struct dpif_flow *flow; /* Resulting flow from datapath. */
+};
+
int dpif_execute(struct dpif *, struct dpif_execute *);
struct dpif_op {
@@ -673,6 +701,7 @@ struct dpif_op {
struct dpif_flow_put flow_put;
struct dpif_flow_del flow_del;
struct dpif_execute execute;
+ struct dpif_flow_get flow_get;
} u;
};
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 66728e6..851aed7 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1523,15 +1523,17 @@ handle_missed_revalidation(struct revalidator
*revalidator,
{
struct udpif *udpif = revalidator->udpif;
struct dpif_flow flow;
- struct ofpbuf *buf;
+ struct ofpbuf buf;
+ uint64_t stub[DPIF_FLOW_BUFSIZE / 8];
bool keep = false;
COVERAGE_INC(revalidate_missed_dp_flow);
+ ofpbuf_use_stub(&buf, &stub, sizeof stub);
if (!dpif_flow_get(udpif->dpif, ukey->key, ukey->key_len, &buf, &flow)) {
keep = revalidate_ukey(udpif, ukey, &flow);
- ofpbuf_delete(buf);
}
+ ofpbuf_uninit(&buf);
return keep;
}
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
