diff mbox

[ovs-dev,v3,1/3] conntrack: Move ct_state parsing to lib/flow.c

Message ID 1498587094-107805-2-git-send-email-yihung.wei@gmail.com
State Accepted
Headers show

Commit Message

Yi-Hung Wei June 27, 2017, 6:11 p.m. UTC
This patch moves conntrack state parsing function from ovn-trace.c to
lib/flow.c, because it will be used by ofproto/trace unixctl command
later on. It also updates the ct_state checking logic, since we no longer
assume CS_TRACKED is enable by default.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
 lib/flow.c                | 68 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/flow.h                |  3 +++
 ovn/utilities/ovn-trace.c | 32 +++++-----------------
 3 files changed, 78 insertions(+), 25 deletions(-)

Comments

Ben Pfaff July 12, 2017, 4:56 p.m. UTC | #1
On Tue, Jun 27, 2017 at 11:11:32AM -0700, Yi-Hung Wei wrote:
> This patch moves conntrack state parsing function from ovn-trace.c to
> lib/flow.c, because it will be used by ofproto/trace unixctl command
> later on. It also updates the ct_state checking logic, since we no longer
> assume CS_TRACKED is enable by default.
> 
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>

Thanks, applied to master.
diff mbox

Patch

diff --git a/lib/flow.c b/lib/flow.c
index d73e796a2f45..119ce1a7f20b 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1047,6 +1047,74 @@  ct_state_from_string(const char *s)
     return 0;
 }
 
+/* Parses conntrack state from 'state_str'.  If it is parsed successfully,
+ * stores the parsed ct_state in 'ct_state', and returns true.  Otherwise,
+ * returns false, and reports error message in 'ds'. */
+bool
+parse_ct_state(const char *state_str, uint32_t default_state,
+               uint32_t *ct_state, struct ds *ds)
+{
+    uint32_t state = default_state;
+    char *state_s = xstrdup(state_str);
+    char *save_ptr = NULL;
+
+    for (char *cs = strtok_r(state_s, ", ", &save_ptr); cs;
+         cs = strtok_r(NULL, ", ", &save_ptr)) {
+        uint32_t bit = ct_state_from_string(cs);
+        if (!bit) {
+            ds_put_format(ds, "%s: unknown connection tracking state flag",
+                          cs);
+            return false;
+        }
+        state |= bit;
+    }
+
+    *ct_state = state;
+    free(state_s);
+
+    return true;
+}
+
+/* Checks the given conntrack state 'state' according to the constraints
+ * listed in ovs-fields (7).  Returns true if it is valid.  Otherwise, returns
+ * false, and reports error in 'ds'. */
+bool
+validate_ct_state(uint32_t state, struct ds *ds)
+{
+    bool valid_ct_state = true;
+    struct ds d_str = DS_EMPTY_INITIALIZER;
+
+    format_flags(&d_str, ct_state_to_string, state, '|');
+
+    if (state && !(state & CS_TRACKED)) {
+        ds_put_format(ds, "%s: invalid connection state: "
+                      "If \"trk\" is unset, no other flags are set\n",
+                      ds_cstr(&d_str));
+        valid_ct_state = false;
+    }
+    if (state & CS_INVALID && state & ~(CS_TRACKED | CS_INVALID)) {
+        ds_put_format(ds, "%s: invalid connection state: "
+                      "when \"inv\" is set, only \"trk\" may also be set\n",
+                      ds_cstr(&d_str));
+        valid_ct_state = false;
+    }
+    if (state & CS_NEW && state & CS_ESTABLISHED) {
+        ds_put_format(ds, "%s: invalid connection state: "
+                      "\"new\" and \"est\" are mutually exclusive\n",
+                      ds_cstr(&d_str));
+        valid_ct_state = false;
+    }
+    if (state & CS_NEW && state & CS_REPLY_DIR) {
+        ds_put_format(ds, "%s: invalid connection state: "
+                      "\"new\" and \"rpy\" are mutually exclusive\n",
+                      ds_cstr(&d_str));
+        valid_ct_state = false;
+    }
+
+    ds_destroy(&d_str);
+    return valid_ct_state;
+}
+
 /* Clears the fields in 'flow' associated with connection tracking. */
 void
 flow_clear_conntrack(struct flow *flow)
diff --git a/lib/flow.h b/lib/flow.h
index 68bd4f3c5149..5889c4decaaa 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -75,6 +75,9 @@  void flow_get_metadata(const struct flow *, struct match *flow_metadata);
 
 const char *ct_state_to_string(uint32_t state);
 uint32_t ct_state_from_string(const char *);
+bool parse_ct_state(const char *state_str, uint32_t default_state,
+                    uint32_t *ct_state, struct ds *);
+bool validate_ct_state(uint32_t state, struct ds *);
 void flow_clear_conntrack(struct flow *);
 
 char *flow_to_string(const struct flow *, const struct ofputil_port_map *);
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 8bdb7d8a304d..ab56221d7829 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -169,34 +169,16 @@  default_ovs(void)
 static void
 parse_ct_option(const char *state_s_)
 {
-    uint32_t state = CS_TRACKED;
-
-    char *state_s = xstrdup(state_s_);
-    char *save_ptr = NULL;
-    for (char *cs = strtok_r(state_s, ", ", &save_ptr); cs;
-         cs = strtok_r(NULL, ", ", &save_ptr)) {
-        uint32_t bit = ct_state_from_string(cs);
-        if (!bit) {
-            ovs_fatal(0, "%s: unknown connection tracking state flag", cs);
-        }
-        state |= bit;
-    }
-    free(state_s);
+    uint32_t state;
+    struct ds ds = DS_EMPTY_INITIALIZER;
 
-    /* Check constraints listed in ovs-fields(7). */
-    if (state & CS_INVALID && state & ~(CS_TRACKED | CS_INVALID)) {
-        VLOG_WARN("%s: invalid connection state: "
-                  "when \"inv\" is set, only \"trk\" may also be set",
-                  state_s_);
-    }
-    if (state & CS_NEW && state & CS_ESTABLISHED) {
-        VLOG_WARN("%s: invalid connection state: "
-                  "\"new\" and \"est\" are mutually exclusive", state_s_);
+    if (!parse_ct_state(state_s_, CS_TRACKED, &state, &ds)) {
+        ovs_fatal(0, "%s", ds_cstr(&ds));
     }
-    if (state & CS_NEW && state & CS_REPLY_DIR) {
-        VLOG_WARN("%s: invalid connection state: "
-                  "\"new\" and \"rpy\" are mutually exclusive", state_s_);
+    if (!validate_ct_state(state, &ds)) {
+        VLOG_WARN("%s", ds_cstr(&ds));
     }
+    ds_destroy(&ds);
 
     ct_states = xrealloc(ct_states, (n_ct_states + 1) * sizeof *ct_states);
     ct_states[n_ct_states++] = state;