[ovs-dev] [PATCH v2 2/3] ofproto/trace: Add support for tracing conntrack recirculation

Yi-Hung Wei yihung.wei at gmail.com
Mon Jun 26 17:25:23 UTC 2017


Previously, a user need to run ofproto/trace multiple times to derive the
final datapath actions if a flow hit conntrack actions that involves
recirculation. To improve the usability of ofproto/trace, in this patch,
we keep track of the conntrack actions, and automatically run the
recirculation process so that a user only need to execute the ofproto/trace
command once. Currently, this patch sets the default ct_state as
trk and est in the automatic recirculation process. A following patch
will provide an option to customize ct_state.

Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
---
 ofproto/ofproto-dpif-rid.c   | 13 +++++++
 ofproto/ofproto-dpif-rid.h   |  1 +
 ofproto/ofproto-dpif-trace.c | 88 +++++++++++++++++++++++++++++++++++++-------
 ofproto/ofproto-dpif-trace.h | 22 +++++++++++
 ofproto/ofproto-dpif-xlate.c | 29 +++++++++++++--
 ofproto/ofproto-dpif-xlate.h |  4 ++
 tests/ofproto-dpif.at        | 35 ++++++++++++++++++
 7 files changed, 175 insertions(+), 17 deletions(-)

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index 9381dee61404..d546b150b938 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -107,6 +107,19 @@ recirc_id_node_find(uint32_t id)
         : NULL;
 }
 
+bool
+recirc_id_node_find_and_ref(uint32_t id)
+{
+    struct recirc_id_node *rid_node =
+        CONST_CAST(struct recirc_id_node *, recirc_id_node_find(id));
+
+    if (!rid_node) {
+        return false;
+    }
+
+    return ovs_refcount_try_ref_rcu(&rid_node->refcount);
+}
+
 static uint32_t
 frozen_state_hash(const struct frozen_state *state)
 {
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index ab9b1b7e5222..c6743a133ed5 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -186,6 +186,7 @@ void recirc_free_id(uint32_t recirc_id);
 void recirc_free_ofproto(struct ofproto_dpif *, const char *ofproto_name);
 
 const struct recirc_id_node *recirc_id_node_find(uint32_t recirc_id);
+bool recirc_id_node_find_and_ref(uint32_t id);
 
 static inline struct recirc_id_node *
 recirc_id_node_from_state(const struct frozen_state *state)
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index c7f0e48caa16..1e4290e8fe0b 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -86,6 +86,38 @@ oftrace_node_destroy(struct oftrace_node *node)
     }
 }
 
+bool
+oftrace_add_recirc_node(struct ovs_list *recirc_queue,
+                        enum oftrace_recirc_type type, struct flow *flow,
+                        uint32_t recirc_id)
+{
+    if (type != OFT_RECIRC_INIT) {
+        if (!recirc_id_node_find_and_ref(recirc_id)) {
+            return false;
+        }
+    }
+
+    struct oftrace_recirc_node *node = xmalloc(sizeof *node);
+    ovs_list_push_back(recirc_queue, &node->node);
+
+    node->type = type;
+    node->recirc_id = recirc_id;
+    node->flow = xmemdup(flow, sizeof *flow);
+    node->flow->recirc_id = recirc_id;
+
+    return true;
+}
+
+static void
+oftrace_recirc_node_destroy(struct oftrace_recirc_node *node)
+{
+    if (node->type != OFT_RECIRC_INIT) {
+        recirc_free_id(node->recirc_id);
+    }
+    free(node->flow);
+    free(node);
+}
+
 static void
 oftrace_node_print_details(struct ds *output,
                            const struct ovs_list *nodes, int level)
@@ -419,20 +451,11 @@ exit:
     ofpbuf_uninit(&ofpacts);
 }
 
-/* Implements a "trace" through 'ofproto''s flow table, appending a textual
- * description of the results to 'output'.
- *
- * The trace follows a packet with the specified 'flow' through the flow
- * table.  'packet' may be nonnull to trace an actual packet, with consequent
- * side effects (if it is nonnull then its flow must be 'flow').
- *
- * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to
- * trace, otherwise the actions are determined by a flow table lookup. */
 static void
-ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
-              const struct dp_packet *packet,
-              const struct ofpact ofpacts[], size_t ofpacts_len,
-              struct ds *output)
+ofproto_trace__(struct ofproto_dpif *ofproto, struct flow *flow,
+                const struct dp_packet *packet, struct ovs_list *recirc_queue,
+                const struct ofpact ofpacts[], size_t ofpacts_len,
+                struct ds *output)
 {
     struct ofpbuf odp_actions;
     ofpbuf_init(&odp_actions, 0);
@@ -447,6 +470,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
     xin.ofpacts = ofpacts;
     xin.ofpacts_len = ofpacts_len;
     xin.trace = &trace;
+    xin.recirc_queue = recirc_queue;
 
     /* Copy initial flow out of xin.flow.  It differs from '*flow' because
      * xlate_in_init() initializes actset_output to OFPP_UNSET. */
@@ -503,6 +527,44 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
     oftrace_node_list_destroy(&trace);
 }
 
+/* Implements a "trace" through 'ofproto''s flow table, appending a textual
+ * description of the results to 'output'.
+ *
+ * The trace follows a packet with the specified 'flow' through the flow
+ * table.  'packet' may be nonnull to trace an actual packet, with consequent
+ * side effects (if it is nonnull then its flow must be 'flow').
+ *
+ * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to
+ * trace, otherwise the actions are determined by a flow table lookup. */
+static void
+ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
+              const struct dp_packet *packet,
+              const struct ofpact ofpacts[], size_t ofpacts_len,
+              struct ds *output)
+{
+    struct ovs_list recirc_queue = OVS_LIST_INITIALIZER(&recirc_queue);
+    oftrace_add_recirc_node(&recirc_queue, OFT_RECIRC_INIT, flow,
+                            flow->recirc_id);
+
+    while (!ovs_list_is_empty(&recirc_queue)) {
+        struct ovs_list *node = ovs_list_pop_front(&recirc_queue);
+        struct oftrace_recirc_node *recirc_node;
+
+        ASSIGN_CONTAINER(recirc_node, node, node);
+
+        if (recirc_node->type == OFT_RECIRC_CONNTRACK) {
+            recirc_node->flow->ct_state = CS_ESTABLISHED | CS_TRACKED;
+            ds_put_cstr(output, "\n\nResume conntrack prcessing with "
+                                "default ct_state=trk|est.\n");
+            ds_put_char_multiple(output, '-', 79);
+            ds_put_char(output, '\n');
+        }
+        ofproto_trace__(ofproto, recirc_node->flow, packet, &recirc_queue,
+                        ofpacts, ofpacts_len, output);
+        oftrace_recirc_node_destroy(recirc_node);
+    }
+}
+
 void
 ofproto_dpif_trace_init(void)
 {
diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
index e3d3b393cec8..db3be290e477 100644
--- a/ofproto/ofproto-dpif-trace.h
+++ b/ofproto/ofproto-dpif-trace.h
@@ -45,6 +45,16 @@ enum oftrace_node_type {
     OFT_ERROR,                  /* An erroneous situation, worth logging. */
 };
 
+/* Type of a node within a recirculation queue. */
+enum oftrace_recirc_type {
+    /* The initial flow to be traced. */
+    OFT_RECIRC_INIT,
+    /* A flow recirculates because of the following actions. */
+    OFT_RECIRC_CONNTRACK,
+    OFT_RECIRC_MPLS,
+    OFT_RECIRC_BOND,
+};
+
 /* A node within a trace. */
 struct oftrace_node {
     struct ovs_list node;       /* In parent. */
@@ -54,9 +64,21 @@ struct oftrace_node {
     char *text;
 };
 
+/* A node within a recirculation queue. */
+struct oftrace_recirc_node {
+    struct ovs_list node;       /* In recirc_queue. */
+
+    enum oftrace_recirc_type type;
+    uint32_t recirc_id;
+    struct flow *flow;
+};
+
 void ofproto_dpif_trace_init(void);
 
 struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type,
                                     const char *text);
+bool oftrace_add_recirc_node(struct ovs_list *recirc_queue,
+                             enum oftrace_recirc_type, struct flow *,
+                             uint32_t recirc_id);
 
 #endif /* ofproto-dpif-trace.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 48c4bad4ac0b..c29d37e3187c 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4356,9 +4356,14 @@ emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state)
     }
 }
 
-static void
+/* Creates a frozen state, and allocates a unique recirc id for the given
+ * state.  Returns a non-zero recirc id if it is allocated successfully.
+ * Returns 0 otherwise.
+ **/
+static uint32_t
 finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
 {
+    uint32_t id = 0;
     ovs_assert(ctx->freezing);
 
     struct frozen_state state = {
@@ -4385,11 +4390,11 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
          * recirculation context, will be returned if possible.
          * The life-cycle of this recirc id is managed by associating it
          * with the udpif key ('ukey') created for each new datapath flow. */
-        uint32_t id = recirc_alloc_id_ctx(&state);
+        id = recirc_alloc_id_ctx(&state);
         if (!id) {
             xlate_report_error(ctx, "Failed to allocate recirculation id");
             ctx->error = XLATE_NO_RECIRCULATION_CONTEXT;
-            return;
+            return 0;
         }
         recirc_refs_add(&ctx->xout->recircs, id);
 
@@ -4408,6 +4413,7 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
 
     /* Undo changes done by freezing. */
     ctx_cancel_freeze(ctx);
+    return id;
 }
 
 /* Called only when we're freezing. */
@@ -4425,8 +4431,22 @@ finish_freezing(struct xlate_ctx *ctx)
 static void
 compose_recirculate_and_fork(struct xlate_ctx *ctx, uint8_t table)
 {
+    uint32_t recirc_id;
     ctx->freezing = true;
-    finish_freezing__(ctx, table);
+    recirc_id = finish_freezing__(ctx, table);
+
+    if (OVS_UNLIKELY(ctx->xin->trace) && recirc_id) {
+        if (oftrace_add_recirc_node(ctx->xin->recirc_queue,
+                                    OFT_RECIRC_CONNTRACK, &ctx->xin->flow,
+                                    recirc_id)) {
+            xlate_report(ctx, OFT_DETAIL, "A clone of the packet is forked to "
+                         "recirculate. The forked pipeline will be resumed at "
+                         "table %u.", table);
+        } else {
+            xlate_report(ctx, OFT_DETAIL, "Failed to trace the conntrack "
+                        "forked pipeline with recirc_id = %d.", recirc_id);
+        }
+    }
 }
 
 static void
@@ -5970,6 +5990,7 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
     xin->wc = wc;
     xin->odp_actions = odp_actions;
     xin->in_packet_out = false;
+    xin->recirc_queue = NULL;
 
     /* Do recirc lookup. */
     xin->frozen_state = NULL;
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 68e114afb9ae..3de7dec8765d 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -158,6 +158,10 @@ struct xlate_in {
 
     /* If true, the packet to be translated is from a packet_out msg. */
     bool in_packet_out;
+
+    /* ofproto/trace maintains this queue to trace flows that require
+     * recirculation. */
+    struct ovs_list *recirc_queue;
 };
 
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index e2228661fd09..c51c689b9f46 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -9710,6 +9710,41 @@ udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - conntrack - ofproto/trace])
+OVS_VSWITCHD_START
+
+add_of_ports br0 1 2
+
+AT_DATA([flows.txt], [dnl
+dnl Table 0
+dnl
+table=0,priority=100,arp,action=normal
+table=0,priority=10,udp,action=ct(table=1,zone=0)
+table=0,priority=1,action=drop
+dnl
+dnl Table 1
+dnl
+table=1,priority=10,in_port=1,ct_state=+trk+new,udp,action=ct(commit,zone=0),2
+table=1,priority=10,in_port=1,ct_state=+trk+est,udp,action=2
+table=1,priority=10,in_port=2,ct_state=+trk+est,udp,action=1
+table=1,priority=1,action=drop
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=2,udp'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 1
+])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,udp'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto - set mtu])
 OVS_VSWITCHD_START
 
-- 
2.7.4



More information about the dev mailing list