This is analogous to the split between rule and rule_actions in
ofproto. As there, it will allow retaining a reference to a rule's
actions, while processing them, without having to retain a reference
to the rule itself.
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
lib/dpif-netdev.c | 136 ++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 92 insertions(+), 44 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 62837b0..d0f6dab 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -138,10 +138,34 @@ struct dp_netdev_flow {
uint16_t tcp_flags; /* Bitwise-OR of seen tcp_flags values. */
/* Actions. */
- struct nlattr *actions;
- size_t actions_len;
+ struct dp_netdev_actions *actions;
};
+/* A set of datapath actions within a "struct dp_netdev_flow".
+ *
+ *
+ * Thread-safety
+ * =============
+ *
+ * A struct dp_netdev_actions 'actions' may be accessed without a risk of being
+ * freed by code that holds a read-lock or write-lock on 'flow->mutex' (where
+ * 'flow' is the dp_netdev_flow for which 'flow->actions == actions') or that
+ * owns a reference to 'actions->ref_cnt' (or both). */
+struct dp_netdev_actions {
+ struct ovs_refcount ref_cnt;
+
+ /* These members are immutable: they do not change during the struct's
+ * lifetime. */
+ struct nlattr *actions; /* Sequence of OVS_ACTION_ATTR_* attributes. */
+ unsigned int size; /* Size of 'actions', in bytes. */
+};
+
+struct dp_netdev_actions *dp_netdev_actions_create(const struct nlattr *,
+ size_t);
+struct dp_netdev_actions *dp_netdev_actions_ref(
+ const struct dp_netdev_actions *);
+void dp_netdev_actions_unref(struct dp_netdev_actions *);
+
/* Interface to netdev-based datapath. */
struct dpif_netdev {
struct dpif dpif;
@@ -901,8 +925,8 @@ dpif_netdev_flow_get(const struct dpif *dpif,
get_dpif_flow_stats(netdev_flow, stats);
}
if (actionsp) {
- *actionsp = ofpbuf_clone_data(netdev_flow->actions,
- netdev_flow->actions_len);
+ *actionsp = ofpbuf_clone_data(netdev_flow->actions->actions,
+ netdev_flow->actions->size);
}
} else {
error = ENOENT;
@@ -913,16 +937,6 @@ dpif_netdev_flow_get(const struct dpif *dpif,
}
static int
-set_flow_actions(struct dp_netdev_flow *netdev_flow,
- const struct nlattr *actions, size_t actions_len)
-{
- netdev_flow->actions = xrealloc(netdev_flow->actions, actions_len);
- netdev_flow->actions_len = actions_len;
- memcpy(netdev_flow->actions, actions, actions_len);
- return 0;
-}
-
-static int
dp_netdev_flow_add(struct dp_netdev *dp, const struct flow *flow,
const struct flow_wildcards *wc,
const struct nlattr *actions,
@@ -930,10 +944,10 @@ dp_netdev_flow_add(struct dp_netdev *dp, const struct
flow *flow,
{
struct dp_netdev_flow *netdev_flow;
struct match match;
- int error;
netdev_flow = xzalloc(sizeof *netdev_flow);
netdev_flow->flow = *flow;
+ netdev_flow->actions = dp_netdev_actions_create(actions, actions_len);
match_init(&match, flow, wc);
cls_rule_init(&netdev_flow->cr, &match, NETDEV_RULE_PRIORITY);
@@ -941,17 +955,6 @@ dp_netdev_flow_add(struct dp_netdev *dp, const struct flow
*flow,
classifier_insert(&dp->cls, &netdev_flow->cr);
ovs_rwlock_unlock(&dp->cls.rwlock);
- error = set_flow_actions(netdev_flow, actions, actions_len);
- if (error) {
- ovs_rwlock_wrlock(&dp->cls.rwlock);
- classifier_remove(&dp->cls, &netdev_flow->cr);
- ovs_rwlock_unlock(&dp->cls.rwlock);
- cls_rule_destroy(&netdev_flow->cr);
-
- free(netdev_flow);
- return error;
- }
-
hmap_insert(&dp->flow_table, &netdev_flow->node, flow_hash(flow, 0));
return 0;
}
@@ -1004,15 +1007,14 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct
dpif_flow_put *put)
} else {
if (put->flags & DPIF_FP_MODIFY
&& flow_equal(&flow, &netdev_flow->flow)) {
- error = set_flow_actions(netdev_flow, put->actions,
- put->actions_len);
- if (!error) {
- if (put->stats) {
- get_dpif_flow_stats(netdev_flow, put->stats);
- }
- if (put->flags & DPIF_FP_ZERO_STATS) {
- clear_stats(netdev_flow);
- }
+ dp_netdev_actions_unref(netdev_flow->actions);
+ netdev_flow->actions = dp_netdev_actions_create(put->actions,
+ put->actions_len);
+ if (put->stats) {
+ get_dpif_flow_stats(netdev_flow, put->stats);
+ }
+ if (put->flags & DPIF_FP_ZERO_STATS) {
+ clear_stats(netdev_flow);
}
} else if (put->flags & DPIF_FP_CREATE) {
error = EEXIST;
@@ -1057,7 +1059,7 @@ dpif_netdev_flow_del(struct dpif *dpif, const struct
dpif_flow_del *del)
struct dp_netdev_flow_state {
uint32_t bucket;
uint32_t offset;
- struct nlattr *actions;
+ struct dp_netdev_actions *actions;
struct odputil_keybuf keybuf;
struct odputil_keybuf maskbuf;
struct dpif_flow_stats stats;
@@ -1121,12 +1123,14 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif,
void *state_,
}
if (actions) {
- free(state->actions);
- state->actions = xmemdup(netdev_flow->actions,
- netdev_flow->actions_len);
+ dp_netdev_actions_unref(state->actions);
+ state->actions = NULL;
- *actions = state->actions;
- *actions_len = netdev_flow->actions_len;
+ if (actions) {
+ state->actions = dp_netdev_actions_ref(netdev_flow->actions);
+ *actions = state->actions->actions;
+ *actions_len = state->actions->size;
+ }
}
if (stats) {
@@ -1143,7 +1147,7 @@ dpif_netdev_flow_dump_done(const struct dpif *dpif
OVS_UNUSED, void *state_)
{
struct dp_netdev_flow_state *state = state_;
- free(state->actions);
+ dp_netdev_actions_unref(state->actions);
free(state);
return 0;
}
@@ -1256,6 +1260,46 @@ dpif_netdev_recv_purge(struct dpif *dpif)
ovs_mutex_unlock(&dp_netdev_mutex);
}
+/* Creates and returns a new 'struct dp_netdev_actions', with a reference count
+ * of 1, whose actions are a copy of from the 'ofpacts_len' bytes of
+ * 'ofpacts'. */
+struct dp_netdev_actions *
+dp_netdev_actions_create(const struct nlattr *actions, size_t size)
+{
+ struct dp_netdev_actions *netdev_actions;
+
+ netdev_actions = xmalloc(sizeof *netdev_actions);
+ ovs_refcount_init(&netdev_actions->ref_cnt);
+ netdev_actions->actions = xmemdup(actions, size);
+ netdev_actions->size = size;
+
+ return netdev_actions;
+}
+
+/* Increments 'actions''s refcount. */
+struct dp_netdev_actions *
+dp_netdev_actions_ref(const struct dp_netdev_actions *actions_)
+{
+ struct dp_netdev_actions *actions;
+
+ actions = CONST_CAST(struct dp_netdev_actions *, actions_);
+ if (actions) {
+ ovs_refcount_ref(&actions->ref_cnt);
+ }
+ return actions;
+}
+
+/* Decrements 'actions''s refcount and frees 'actions' if the refcount reaches
+ * 0. */
+void
+dp_netdev_actions_unref(struct dp_netdev_actions *actions)
+{
+ if (actions && ovs_refcount_unref(&actions->ref_cnt) == 1) {
+ free(actions->actions);
+ free(actions);
+ }
+}
+
static void
dp_netdev_flow_used(struct dp_netdev_flow *netdev_flow,
const struct ofpbuf *packet)
@@ -1281,10 +1325,14 @@ dp_netdev_port_input(struct dp_netdev *dp, struct
dp_netdev_port *port,
flow_extract(packet, 0, 0, NULL, &in_port_, &key);
netdev_flow = dp_netdev_lookup_flow(dp, &key);
if (netdev_flow) {
+ struct dp_netdev_actions *actions;
+
dp_netdev_flow_used(netdev_flow, packet);
+ actions = dp_netdev_actions_ref(netdev_flow->actions);
dp_netdev_execute_actions(dp, &key, packet,
- netdev_flow->actions,
- netdev_flow->actions_len);
+ actions->actions, actions->size);
+ dp_netdev_actions_unref(actions);
+
ovsthread_counter_inc(dp->n_hit, 1);
} else {
ovsthread_counter_inc(dp->n_missed, 1);
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
