[ovs-dev] [PATCH v3 3/3] ofproto/trace: Add --ct-next option to ofproto/trace

[Ben Pfaff]

On Tue, Jun 27, 2017 at 11:11:34AM -0700, Yi-Hung Wei wrote:
> Previous patch enables ofproto/trace to automatically trace a flow
> that involves multiple recirculation on conntrack. However, it always
> sets the ct_state to trk|est when it processes recirculated conntrack flows.
> With this patch, users can customize the expected next ct_state in the
> aforementioned use case.
> 
> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>

Thanks a lot!

I simplified oftrace_pop_ct_state() a bit, and I fixed a memory leak in
the case where not all the next_ct_states were used, and applied this to
master.  I'm appending what I folded in.

--8<--------------------------cut here-------------------------->8--

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 9df39a110089..b3f3cbc6a4ef 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -131,14 +131,11 @@ oftrace_push_ct_state(struct ovs_list *next_ct_states, uint32_t ct_state)
 static uint32_t
 oftrace_pop_ct_state(struct ovs_list *next_ct_states)
 {
-    struct ovs_list *list_node = ovs_list_pop_front(next_ct_states);
-    struct oftrace_next_ct_state *next_ct_state;
-
-    ASSIGN_CONTAINER(next_ct_state, list_node, node);
-    uint32_t ct_state = next_ct_state->state;
-    free(next_ct_state);
-
-    return ct_state;
+    struct oftrace_next_ct_state *s;
+    LIST_FOR_EACH_POP (s, node, next_ct_states) {
+        return s->state;
+    }
+    OVS_NOT_REACHED();
 }
 
 static void
@@ -404,6 +401,14 @@ exit:
 }
 
 static void
+free_ct_states(struct ovs_list *ct_states)
+{
+    while (!ovs_list_is_empty(ct_states)) {
+        oftrace_pop_ct_state(ct_states);
+    }
+}
+
+static void
 ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
                       void *aux OVS_UNUSED)
 {
@@ -428,6 +433,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
         unixctl_command_reply_error(conn, error);
         free(error);
     }
+    free_ct_states(&next_ct_states);
 }
 
 static void
@@ -528,6 +534,7 @@ exit:
     ds_destroy(&result);
     dp_packet_delete(packet);
     ofpbuf_uninit(&ofpacts);
+    free_ct_states(&next_ct_states);
 }
 
 static void