[ovs-dev] [PATCH 6/6] ofproto: Enable in-place modification for recirc actions.
Jarno Rajahalme
jrajahalme at nicira.com
Thu Oct 29 03:07:58 UTC 2015
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
ofproto/ofproto-dpif-rid.h | 63 +++++++++++++++++++++++++++++++
ofproto/ofproto-dpif-upcall.c | 88 +++++++++++++++++++++++++------------------
ofproto/ofproto-dpif-xlate.c | 38 ++++++-------------
ofproto/ofproto-dpif-xlate.h | 62 +-----------------------------
4 files changed, 129 insertions(+), 122 deletions(-)
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 6ac5fef..b5cbc73 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -21,6 +21,7 @@
#include <stdint.h>
#include "cmap.h"
+#include "flow.h"
#include "list.h"
#include "ofp-actions.h"
#include "ofproto-dpif-mirror.h"
@@ -196,4 +197,66 @@ void recirc_id_node_unref(const struct recirc_id_node *);
void recirc_run(void);
+/* Recirculation IDs on which references are held. */
+struct recirc_refs {
+ unsigned n_recircs;
+ union {
+ uint32_t recirc[2]; /* When n_recircs == 1 or 2 */
+ uint32_t *recircs; /* When 'n_recircs' > 2 */
+ };
+};
+
+#define RECIRC_REFS_EMPTY_INITIALIZER { 0, { { 0, 0 } } }
+/* Helpers to abstract the recirculation union away. */
+static inline void
+recirc_refs_init(struct recirc_refs *rr)
+{
+ memset(rr, 0, sizeof *rr);
+}
+
+static inline void
+recirc_refs_add(struct recirc_refs *rr, uint32_t id)
+{
+ if (OVS_LIKELY(rr->n_recircs < ARRAY_SIZE(rr->recirc))) {
+ rr->recirc[rr->n_recircs++] = id;
+ } else {
+ if (rr->n_recircs == ARRAY_SIZE(rr->recirc)) {
+ uint32_t *recircs = xmalloc(sizeof rr->recirc + sizeof id);
+
+ memcpy(recircs, rr->recirc, sizeof rr->recirc);
+ rr->recircs = recircs;
+ } else {
+ rr->recircs = xrealloc(rr->recircs,
+ (rr->n_recircs + 1) * sizeof id);
+ }
+ rr->recircs[rr->n_recircs++] = id;
+ }
+}
+
+static inline void
+recirc_refs_swap(struct recirc_refs *a, struct recirc_refs *b)
+{
+ struct recirc_refs tmp;
+
+ tmp = *a;
+ *a = *b;
+ *b = tmp;
+}
+
+static inline void
+recirc_refs_unref(struct recirc_refs *rr)
+{
+ if (OVS_LIKELY(rr->n_recircs <= ARRAY_SIZE(rr->recirc))) {
+ for (int i = 0; i < rr->n_recircs; i++) {
+ recirc_free_id(rr->recirc[i]);
+ }
+ } else {
+ for (int i = 0; i < rr->n_recircs; i++) {
+ recirc_free_id(rr->recircs[i]);
+ }
+ free(rr->recircs);
+ }
+ rr->n_recircs = 0;
+}
+
#endif
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index f749c66..b6c0b36 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -261,9 +261,8 @@ struct udpif_key {
struct nlattr nla;
} keybuf, maskbuf;
- /* Recirculation IDs with references held by the ukey. */
- unsigned n_recircs;
- uint32_t recircs[]; /* 'n_recircs' id's for which references are held. */
+ uint32_t key_recirc_id; /* Non-zero if reference is held by the ukey. */
+ struct recirc_refs recircs; /* Action recirc IDs with references held. */
};
/* Datapath operation with optional ukey attached. */
@@ -1453,12 +1452,10 @@ ukey_create__(const struct nlattr *key, size_t key_len,
bool ufid_present, const ovs_u128 *ufid,
const unsigned pmd_id, const struct ofpbuf *actions,
uint64_t dump_seq, uint64_t reval_seq, long long int used,
- const struct recirc_id_node *key_recirc, struct xlate_out *xout)
+ uint32_t key_recirc_id, struct xlate_out *xout)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
- unsigned n_recircs = (key_recirc ? 1 : 0) + (xout ? xout->n_recircs : 0);
- struct udpif_key *ukey = xmalloc(sizeof *ukey +
- n_recircs * sizeof *ukey->recircs);
+ struct udpif_key *ukey = xmalloc(sizeof *ukey);
memcpy(&ukey->keybuf, key, key_len);
ukey->key = &ukey->keybuf.nla;
@@ -1483,17 +1480,13 @@ ukey_create__(const struct nlattr *key, size_t key_len,
ukey->stats.used = used;
ukey->xcache = NULL;
- ukey->n_recircs = n_recircs;
- if (key_recirc) {
- ukey->recircs[0] = key_recirc->id;
+ ukey->key_recirc_id = key_recirc_id;
+ recirc_refs_init(&ukey->recircs);
+ if (xout) {
+ /* Take ownership of the action recirc id references. */
+ recirc_refs_swap(&ukey->recircs, &xout->recircs);
}
- if (xout && xout->n_recircs) {
- const uint32_t *act_recircs = xlate_out_get_recircs(xout);
- memcpy(ukey->recircs + (key_recirc ? 1 : 0), act_recircs,
- xout->n_recircs * sizeof *ukey->recircs);
- xlate_out_take_recircs(xout);
- }
return ukey;
}
@@ -1532,7 +1525,7 @@ ukey_create_from_upcall(struct upcall *upcall, struct flow_wildcards *wc)
true, upcall->ufid, upcall->pmd_id,
&upcall->put_actions, upcall->dump_seq,
upcall->reval_seq, 0,
- upcall->have_recirc_ref ? upcall->recirc : NULL,
+ upcall->have_recirc_ref ? upcall->recirc->id : 0,
&upcall->xout);
}
@@ -1585,7 +1578,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
*ukey = ukey_create__(flow->key, flow->key_len,
flow->mask, flow->mask_len, flow->ufid_present,
&flow->ufid, flow->pmd_id, &actions, dump_seq,
- reval_seq, flow->stats.used, NULL, NULL);
+ reval_seq, flow->stats.used, 0, NULL);
return 0;
}
@@ -1728,9 +1721,10 @@ ukey_delete__(struct udpif_key *ukey)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
if (ukey) {
- for (int i = 0; i < ukey->n_recircs; i++) {
- recirc_free_id(ukey->recircs[i]);
+ if (ukey->key_recirc_id) {
+ recirc_free_id(ukey->key_recirc_id);
}
+ recirc_refs_unref(&ukey->recircs);
xlate_cache_delete(ukey->xcache);
ofpbuf_delete(ovsrcu_get(struct ofpbuf *, &ukey->actions));
ovs_mutex_destroy(&ukey->mutex);
@@ -1787,11 +1781,14 @@ should_revalidate(const struct udpif *udpif, uint64_t packets,
* UKEY_KEEP The ukey is fine as is.
* UKEY_MODIFY The ukey's actions should be changed but is otherwise
* fine. Callers should change the actions to those found
- * in the caller supplied 'odp_actions' buffer. */
+ * in the caller supplied 'odp_actions' buffer. The
+ * recirculation references can be found in 'recircs' and
+ * must be handled by the caller. */
static enum reval_result
revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
const struct dpif_flow_stats *stats,
- struct ofpbuf *odp_actions, uint64_t reval_seq)
+ struct ofpbuf *odp_actions, uint64_t reval_seq,
+ struct recirc_refs *recircs)
OVS_REQUIRES(ukey->mutex)
{
struct xlate_out xout, *xoutp;
@@ -1810,6 +1807,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
result = UKEY_DELETE;
xoutp = NULL;
netflow = NULL;
+ recirc_refs_init(recircs);
ofpbuf_clear(odp_actions);
need_revalidate = (ukey->reval_seq != reval_seq);
@@ -1905,6 +1903,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
/* The datapath mask was OK, but the actions seem to have changed.
* Let's modify it in place. */
result = UKEY_MODIFY;
+ /* Transfer recirc action ID references to the caller. */
+ recirc_refs_swap(recircs, &xoutp->recircs);
goto exit;
}
@@ -2076,6 +2076,25 @@ log_unexpected_flow(const struct dpif_flow *flow, int error)
VLOG_WARN_RL(&rl, "%s", ds_cstr(&ds));
}
+static void reval_op_init(struct ukey_op *op, enum reval_result result,
+ struct udpif *udpif, struct udpif_key *ukey,
+ struct recirc_refs *recircs,
+ struct ofpbuf *odp_actions)
+{
+ if (result == UKEY_DELETE) {
+ delete_op_init(udpif, op, ukey);
+ } else if (result == UKEY_MODIFY) {
+ /* Store the new recircs. */
+ recirc_refs_swap(&ukey->recircs, recircs);
+ /* Release old recircs. */
+ recirc_refs_unref(recircs);
+ /* ukey->key_recirc_id remains, as the key is the same as before. */
+
+ ukey_set_actions(ukey, odp_actions);
+ modify_op_init(op, ukey);
+ }
+}
+
static void
revalidate(struct revalidator *revalidator)
{
@@ -2129,6 +2148,7 @@ revalidate(struct revalidator *revalidator)
for (f = flows; f < &flows[n_dumped]; f++) {
long long int used = f->stats.used;
+ struct recirc_refs recircs; /* Only used when UKEY_MODIFY */
enum reval_result result;
struct udpif_key *ukey;
bool already_dumped;
@@ -2168,16 +2188,14 @@ revalidate(struct revalidator *revalidator)
result = UKEY_DELETE;
} else {
result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
- reval_seq);
+ reval_seq, &recircs);
}
ukey->dump_seq = dump_seq;
ukey->flow_exists = result != UKEY_DELETE;
- if (result == UKEY_DELETE) {
- delete_op_init(udpif, &ops[n_ops++], ukey);
- } else if (result == UKEY_MODIFY) {
- ukey_set_actions(ukey, &odp_actions);
- modify_op_init(&ops[n_ops++], ukey);
+ if (result != UKEY_KEEP) {
+ reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,
+ &odp_actions);
}
ovs_mutex_unlock(&ukey->mutex);
}
@@ -2226,6 +2244,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
CMAP_FOR_EACH(ukey, cmap_node, &umap->cmap) {
bool flow_exists, seq_mismatch;
+ struct recirc_refs recircs; /* Only used when UKEY_MODIFY */
enum reval_result result;
/* Handler threads could be holding a ukey lock while it installs a
@@ -2246,16 +2265,13 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
COVERAGE_INC(revalidate_missed_dp_flow);
memset(&stats, 0, sizeof stats);
result = revalidate_ukey(udpif, ukey, &stats, &odp_actions,
- reval_seq);
+ reval_seq, &recircs);
}
- ovs_mutex_unlock(&ukey->mutex);
-
- if (result == UKEY_DELETE) {
- delete_op_init(udpif, &ops[n_ops++], ukey);
- } else if (result == UKEY_MODIFY) {
- ukey_set_actions(ukey, &odp_actions);
- modify_op_init(&ops[n_ops++], ukey);
+ if (result != UKEY_KEEP) {
+ reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,
+ &odp_actions);
}
+ ovs_mutex_unlock(&ukey->mutex);
if (n_ops == REVALIDATE_MAX_BATCH) {
push_ukey_ops(udpif, umap, ops, n_ops);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index fc76816..4d36b31 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3560,31 +3560,17 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table)
.ofpacts = ctx->action_set.data,
};
- /* Only allocate recirculation ID if we have a packet. */
- if (ctx->xin->packet) {
- /* Allocate a unique recirc id for the given metadata state in the
- * flow. The life-cycle of this recirc id is managed by associating it
- * with the udpif key ('ukey') created for each new datapath flow. */
- id = recirc_alloc_id_ctx(&state);
- if (!id) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
- VLOG_ERR_RL(&rl, "Failed to allocate recirculation id");
- ctx->error = ENODATA;
- return;
- }
- xlate_out_add_recirc(ctx->xout, id);
- } else {
- /* Look up an existing recirc id for the given metadata state in the
- * flow. No new reference is taken, as the ID is RCU protected and is
- * only required temporarily for verification.
- * If flow tables have changed sufficiently this can fail and we will
- * delete the old datapath flow. */
- id = recirc_find_id(&state);
- if (!id) {
- ctx->error = ENODATA;
- return;
- }
+ /* Allocate a unique recirc id for the given metadata state in the
+ * flow. The life-cycle of this recirc id is managed by associating it
+ * with the udpif key ('ukey') created for each new datapath flow. */
+ id = recirc_alloc_id_ctx(&state);
+ if (!id) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_ERR_RL(&rl, "Failed to allocate recirculation id");
+ ctx->error = ENODATA;
+ return;
}
+ recirc_refs_add(&ctx->xout->recircs, id);
nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC, id);
@@ -4683,7 +4669,7 @@ void
xlate_out_uninit(struct xlate_out *xout)
{
if (xout) {
- xlate_out_free_recircs(xout);
+ recirc_refs_unref(&xout->recircs);
}
}
@@ -4897,7 +4883,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
*xout = (struct xlate_out) {
.slow = 0,
.fail_open = false,
- .n_recircs = 0,
+ .recircs = RECIRC_REFS_EMPTY_INITIALIZER,
};
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 3b6b4a4..a442009 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -41,68 +41,10 @@ struct xlate_out {
enum slow_path_reason slow; /* 0 if fast path may be used. */
bool fail_open; /* Initial rule is fail open? */
- /* Recirculation IDs on which references are held. */
- unsigned n_recircs;
- union {
- uint32_t recirc[2]; /* When n_recircs == 1 or 2 */
- uint32_t *recircs; /* When 'n_recircs' > 2 */
- };
+ struct recirc_refs recircs; /* Recirc action IDs on which references are
+ * held. */
};
-/* Helpers to abstract the recirculation union away. */
-static inline void
-xlate_out_add_recirc(struct xlate_out *xout, uint32_t id)
-{
- if (OVS_LIKELY(xout->n_recircs < ARRAY_SIZE(xout->recirc))) {
- xout->recirc[xout->n_recircs++] = id;
- } else {
- if (xout->n_recircs == ARRAY_SIZE(xout->recirc)) {
- uint32_t *recircs = xmalloc(sizeof xout->recirc + sizeof id);
-
- memcpy(recircs, xout->recirc, sizeof xout->recirc);
- xout->recircs = recircs;
- } else {
- xout->recircs = xrealloc(xout->recircs,
- (xout->n_recircs + 1) * sizeof id);
- }
- xout->recircs[xout->n_recircs++] = id;
- }
-}
-
-static inline const uint32_t *
-xlate_out_get_recircs(const struct xlate_out *xout)
-{
- if (OVS_LIKELY(xout->n_recircs <= ARRAY_SIZE(xout->recirc))) {
- return xout->recirc;
- } else {
- return xout->recircs;
- }
-}
-
-static inline void
-xlate_out_take_recircs(struct xlate_out *xout)
-{
- if (OVS_UNLIKELY(xout->n_recircs > ARRAY_SIZE(xout->recirc))) {
- free(xout->recircs);
- }
- xout->n_recircs = 0;
-}
-
-static inline void
-xlate_out_free_recircs(struct xlate_out *xout)
-{
- if (OVS_LIKELY(xout->n_recircs <= ARRAY_SIZE(xout->recirc))) {
- for (int i = 0; i < xout->n_recircs; i++) {
- recirc_free_id(xout->recirc[i]);
- }
- } else {
- for (int i = 0; i < xout->n_recircs; i++) {
- recirc_free_id(xout->recircs[i]);
- }
- free(xout->recircs);
- }
-}
-
struct xlate_in {
struct ofproto_dpif *ofproto;
--
2.1.4
More information about the dev
mailing list