On 6/26/17, 10:18 AM, "Yi-Hung Wei" <[email protected]> wrote:
On Wed, Jun 21, 2017 at 11:19 AM, Darrell Ball <[email protected]> wrote:
>
>
> On 6/21/17, 11:06 AM, "[email protected] on behalf of
Yi-Hung Wei" <[email protected] on behalf of
[email protected]> wrote:
>
> This patch moves conntrack state parsing function from ovn-trace.c to
> lib/conntrack.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 <[email protected]>
> ---
> lib/conntrack.c | 40
++++++++++++++++++++++++++++++++++++++++
> lib/conntrack.h | 1 +
>
> This file is userspace datapath specific; the function being moved can
probably
> go to packet.c as we define the CT states already in packet.h.
Thanks for review. I agree that lib/conntrack.c is not that
appropriate to put this function. After further investigation, given
that ct_state_from_string() is in lib/flow.c, I move the ct state
parsing function to flow.c.
yes, flow.c looks more related too.
>
>
> ovn/utilities/ovn-trace.c | 30 ++----------------------------
> 3 files changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 90b154a87a74..901003ae0801 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2030,3 +2030,43 @@ conntrack_flush(struct conntrack *ct, const
uint16_t *zone)
> }
> return 0;
> }
> +
> +uint32_t
> +parse_ct_state(const char *state_s_, uint32_t default_state)
> +{
> + uint32_t state = default_state;
> +
> + 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);
> +
> + /* Check constraints listed in ovs-fields(7). */
> + if (state && !(state & CS_TRACKED)) {
> + VLOG_WARN("%s: invalid connection state: "
> + "If \"trk\" is unset, no other flags are set",
> + state_s_);
> + }
> + 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 (state & CS_NEW && state & CS_REPLY_DIR) {
> + VLOG_WARN("%s: invalid connection state: "
> + "\"new\" and \"rpy\" are mutually exclusive",
state_s_);
> + }
> +
> + return state;
> +}
> diff --git a/lib/conntrack.h b/lib/conntrack.h
> index 243aebb642f7..c623d79c1803 100644
> --- a/lib/conntrack.h
> +++ b/lib/conntrack.h
> @@ -113,6 +113,7 @@ int conntrack_dump_next(struct conntrack_dump *,
struct ct_dpif_entry *);
> int conntrack_dump_done(struct conntrack_dump *);
>
> int conntrack_flush(struct conntrack *, const uint16_t *zone);
> +uint32_t parse_ct_state(const char *ct_states, uint32_t
default_state);
> ?
> /* 'struct ct_lock' is a wrapper for an adaptive mutex. It's useful
to try
> * different types of locks (e.g. spinlocks) */
> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> index 8bdb7d8a304d..7bec26e27d2b 100644
> --- a/ovn/utilities/ovn-trace.c
> +++ b/ovn/utilities/ovn-trace.c
> @@ -20,6 +20,7 @@
>
> #include "command-line.h"
> #include "compiler.h"
> +#include "conntrack.h"
> #include "daemon.h"
> #include "dirs.h"
> #include "fatal-signal.h"
> @@ -169,34 +170,7 @@ 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);
> -
> - /* 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 (state & CS_NEW && state & CS_REPLY_DIR) {
> - VLOG_WARN("%s: invalid connection state: "
> - "\"new\" and \"rpy\" are mutually exclusive",
state_s_);
> - }
> + uint32_t state = parse_ct_state(state_s_, CS_TRACKED);
>
> ct_states = xrealloc(ct_states, (n_ct_states + 1) * sizeof
*ct_states);
> ct_states[n_ct_states++] = state;
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> [email protected]
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=re4JKD28cBRk_Jel2gAJThuouBDVdP6TphYh7zF7HvM&s=QR34jGwxJS3QONr4nmdwH-CKj9sWHAd5NJMoit_CqzQ&e=
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
