[ovs-dev] [PATCHv2] ovn: Add support for ACL logging.

Ben Pfaff blp at ovn.org
Fri Jul 28 23:57:04 UTC 2017


On Fri, Jul 28, 2017 at 03:48:38PM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit <jpettit at ovn.org>
> Acked-by: Han Zhou <zhouhan at gmail.com>
> ---
> v1->v2: Incorporate Ben and Han's feedback.
>         Improve output of "ovn-nbctl acl-list".

Thank you!

I'm appending a few final suggestions.  The only important one is the
one that moves the free() call, which I believe fixes a double-free on
the error path: the old name is freed but not set to NULL, which means
that ovnact_log_free() eventually frees it again.

Acked-by: Ben Pfaff <blp at ovn.org>

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

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index b88effee7437..0a04af7aaf79 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -415,7 +415,6 @@ enum action_opcode {
      * Arguments are as follows:
      *   - An 8-bit verdict.
      *   - An 8-bit severity.
-     *   - An 16-bit string length for the name.
      *   - A variable length string containing the name.
      */
     ACTION_OPCODE_LOG,
diff --git a/ovn/lib/acl-log.c b/ovn/lib/acl-log.c
index 0f5e43aff720..f47b0af437da 100644
--- a/ovn/lib/acl-log.c
+++ b/ovn/lib/acl-log.c
@@ -78,7 +78,7 @@ log_severity_from_string(const char *name)
 void
 handle_acl_log(const struct flow *headers, struct ofpbuf *userdata)
 {
-    if(!VLOG_IS_INFO_ENABLED()) {
+    if (!VLOG_IS_INFO_ENABLED()) {
         return;
     }
 
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index f010b05a1f48..d0d73b69c32d 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1781,9 +1781,6 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log)
             return;
         }
         /* If multiple names are given, use the most recent. */
-        if (log->name) {
-            free(log->name);
-        }
         if (ctx->lexer->token.type == LEX_T_STRING) {
             /* Arbitrarily limit the name length to 64 bytes, since
              * these will be encoded in datapath actions. */
@@ -1792,6 +1789,7 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log)
                                                "than 64 characters");
                 return;
             }
+            free(log->name);
             log->name = xstrdup(ctx->lexer->token.s);
         } else {
             lexer_syntax_error(ctx->lexer, "expecting string");


More information about the dev mailing list