ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.
authorRussell Bryant <russell@ovn.org>
Wed, 2 Mar 2016 18:04:46 +0000 (18:04 +0000)
committerRussell Bryant <russell@ovn.org>
Thu, 30 Jun 2016 20:08:34 +0000 (16:08 -0400)
Update the "ct_commit;" logical flow action to optionally take
one or two parameters, setting the value of "ct_mark" or "ct_label".
Supported ct_commit syntax now includes:

    ct_commit;
    ct_commit();
    ct_commit(ct_mark=1);
    ct_commit(ct_mark=1/1);
    ct_commit(ct_label=1);
    ct_commit(ct_label=1/1);
    ct_commit(ct_mark=1, ct_label=1);

Setting ct_mark via this type of logical flow results in an OpenFlow
flow that looks like:

    actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_mark))

Similarly, setting ct_label results in:

    actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_label))

A future commit will make use of this feature.

Signed-off-by: Russell Bryant <russell@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
ovn/TODO
ovn/lib/actions.c
ovn/ovn-sb.xml
tests/ovn.at

index 0a6225d..4f134a4 100644 (file)
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -38,6 +38,12 @@ ovn-sb.xml includes a tentative specification for this action.
 IPv6 will probably need an action or actions for ND that is similar to
 the "arp" action, and an action for generating
 
+*** ct_label 128-bit support.
+
+We only support 64-bits for the ct_label argument to ct_commit(), but ct_label
+is a 128-bit field.  The OVN lexer only supports parsing 64-bit integers, but
+we can use parse_int_string() to support larger integers.
+
 ** IPv6
 
 *** ND versus ARP
index 569970e..52c74e6 100644 (file)
@@ -624,7 +624,9 @@ parse_put_dhcp_opts_action(struct action_context *ctx,
 }
 
 static void
-emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
+emit_ct(struct action_context *ctx, bool recirc_next, bool commit,
+        int *ct_mark, int *ct_mark_mask,
+        ovs_be128 *ct_label, ovs_be128 *ct_label_mask)
 {
     struct ofpact_conntrack *ct = ofpact_put_CT(ctx->ofpacts);
     ct->flags |= commit ? NX_CT_F_COMMIT : 0;
@@ -650,6 +652,149 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
 
     /* CT only works with IP, so set up a prerequisite. */
     add_prerequisite(ctx, "ip");
+
+    size_t set_field_offset = ctx->ofpacts->size;
+    ofpbuf_pull(ctx->ofpacts, set_field_offset);
+
+    if (ct_mark) {
+        struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
+        sf->field = mf_from_id(MFF_CT_MARK);
+        sf->value.be32 = htonl(*ct_mark);
+        sf->mask.be32 = ct_mark_mask ? htonl(*ct_mark_mask) : OVS_BE32_MAX;
+    }
+
+    if (ct_label) {
+        struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
+        sf->field = mf_from_id(MFF_CT_LABEL);
+        sf->value.be128 = *ct_label;
+        sf->mask.be128 = ct_label_mask ? *ct_label_mask : OVS_BE128_MAX;
+    }
+
+    ctx->ofpacts->header = ofpbuf_push_uninit(ctx->ofpacts, set_field_offset);
+    ct = ctx->ofpacts->header;
+    ofpact_finish(ctx->ofpacts, &ct->ofpact);
+}
+
+/* Parse an argument to the ct_commit(); action.  Supported arguments include:
+ *
+ *      ct_mark=<value>[/<mask>]
+ *      ct_label=<value>[/<mask>]
+ *
+ * If a comma separates the current argument from the next argument, this
+ * function will consume it.
+ *
+ * set_mark - This will be set to true if a value for ct_mark was successfully
+ *            parsed. Otherwise, it will be unchanged.
+ * mark_value - If set_mark was set to true, this will contain the value
+ *              parsed for ct_mark.
+ * mark_mask - If set_mark was set to true, this will contain the mask
+ *             for ct_mark if one was found.  Otherwise, it will be
+ *             unchanged, so the caller should initialize this to an
+ *             appropriate value.
+ * set_label - This will be set to true if a value for ct_label was successfully
+ *             parsed. Otherwise, it will be unchanged.
+ * label_value - If set_label was set to true, this will contain the value
+ *               parsed for ct_label.
+ * label_mask - If set_label was set to true, this will contain the mask
+ *              for ct_label if one was found.  Otherwise, it will be
+ *              unchanged, so the caller should initialize this to an
+ *              appropriate value.
+ *
+ * Return true after successfully parsing an argument.  false on failure. */
+static bool
+parse_ct_commit_arg(struct action_context *ctx,
+                    bool *set_mark, int *mark_value, int *mark_mask,
+                    bool *set_label, ovs_be128 *label_value,
+                    ovs_be128 *label_mask)
+{
+    if (lexer_match_id(ctx->lexer, "ct_mark")) {
+        if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) {
+            action_error(ctx, "Expected '=' after argument to ct_commit");
+            return false;
+        }
+        if (ctx->lexer->token.type == LEX_T_INTEGER) {
+            *mark_value = ntohll(ctx->lexer->token.value.integer);
+        } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
+            *mark_value = ntohll(ctx->lexer->token.value.integer);
+            *mark_mask = ntohll(ctx->lexer->token.mask.integer);
+        } else {
+            action_error(ctx, "Expected integer after 'ct_mark='");
+            return false;
+        }
+        lexer_get(ctx->lexer);
+        *set_mark = true;
+    } else if (lexer_match_id(ctx->lexer, "ct_label")) {
+        if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) {
+            action_error(ctx, "Expected '=' after argument to ct_commit");
+            return false;
+        }
+        if (ctx->lexer->token.type == LEX_T_INTEGER) {
+            label_value->be64.lo = ctx->lexer->token.value.integer;
+        } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
+            /* XXX Technically, ct_label is a 128-bit field.  The lexer
+             * only supports 64-bit integers, so that's all we support
+             * here.  More work is needed to use parse_int_string()
+             * to support the full 128-bits. */
+            label_value->be64.lo = ctx->lexer->token.value.integer;
+            label_mask->be64.hi = 0;
+            label_mask->be64.lo = ctx->lexer->token.mask.integer;
+        } else {
+            action_error(ctx, "Expected integer after 'ct_label='");
+            return false;
+        }
+        lexer_get(ctx->lexer);
+        *set_label = true;
+    } else {
+        action_error(ctx, "Expected argument to ct_commit()");
+        return false;
+    }
+
+    if (lexer_match(ctx->lexer, LEX_T_COMMA)) {
+        /* A comma is valid after an argument, but only if another
+         * argument is present (not a closing paren) */
+        if (lexer_lookahead(ctx->lexer) == LEX_T_RPAREN) {
+            action_error(ctx, "Another argument to ct_commit() expected "
+                              "after comma.");
+            return false;
+        }
+    }
+
+    return true;
+}
+
+static void
+parse_ct_commit_action(struct action_context *ctx)
+{
+    if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+        /* ct_commit; */
+        emit_ct(ctx, false, true, NULL, NULL, NULL, NULL);
+        return;
+    }
+
+    /* ct_commit();
+     * ct_commit(ct_mark=0);
+     * ct_commit(ct_label=0);
+     * ct_commit(ct_mark=0, ct_label=0); */
+
+    bool set_mark = false;
+    bool set_label = false;
+    int mark_value = 0;
+    int mark_mask = ~0;
+    ovs_be128 label_value = { .be32 = { 0, }, };
+    ovs_be128 label_mask = OVS_BE128_MAX;
+
+    while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+        if (!parse_ct_commit_arg(ctx, &set_mark, &mark_value, &mark_mask,
+                                 &set_label, &label_value, &label_mask)) {
+            return;
+        }
+    }
+
+    emit_ct(ctx, false, true,
+            set_mark ? &mark_value : NULL,
+            set_mark ? &mark_mask : NULL,
+            set_label ? &label_value : NULL,
+            set_label ? &label_mask : NULL);
 }
 
 static void
@@ -755,9 +900,9 @@ parse_action(struct action_context *ctx)
             action_syntax_error(ctx, "expecting `--'");
         }
     } else if (lexer_match_id(ctx->lexer, "ct_next")) {
-        emit_ct(ctx, true, false);
+        emit_ct(ctx, true, false, NULL, NULL, NULL, NULL);
     } else if (lexer_match_id(ctx->lexer, "ct_commit")) {
-        emit_ct(ctx, false, true);
+        parse_ct_commit_action(ctx);
     } else if (lexer_match_id(ctx->lexer, "ct_dnat")) {
         parse_ct_nat(ctx, false);
     } else if (lexer_match_id(ctx->lexer, "ct_snat")) {
index f330374..3db846f 100644 (file)
         </dd>
 
         <dt><code>ct_commit;</code></dt>
+        <dt><code>ct_commit(ct_mark=<var>value[/mask]</var>);</code></dt>
+        <dt><code>ct_commit(ct_label=<var>value[/mask]</var>);</code></dt>
+        <dt><code>ct_commit(ct_mark=<var>value[/mask]</var>, ct_label=<var>value[/mask]</var>);</code></dt>
         <dd>
           <p>
-            Commit the flow to the connection tracking entry associated
-            with it by a previous call to <code>ct_next</code>.
+            Commit the flow to the connection tracking entry associated with it
+            by a previous call to <code>ct_next</code>.  When
+            <code>ct_mark=<var>value[/mask]</var></code> and/or
+            <code>ct_label=<var>value[/mask]</var></code> are supplied,
+            <code>ct_mark</code> and/or <code>ct_label</code> will be set to the
+            values indicated by <var>value[/mask]</var> on the connection
+            tracking entry. <code>ct_mark</code> is a 32-bit field.
+            <code>ct_label</code> is technically a 128-bit field, though OVN
+            currently only supports 64-bits and will later be extended to
+            support the full 128-bits.
           </p>
+
           <p>
             Note that if you want processing to continue in the next table,
             you must execute the <code>next</code> action after
-            <code>ct_commit</code>.
+            <code>ct_commit</code>.  You may also leave out <code>next</code>
+            which will commit connection tracking state, and then drop the
+            packet.  This could be useful for setting <code>ct_mark</code>
+            on a connection tracking entry before dropping a packet,
+            for example.
           </p>
         </dd>
 
index 297070c..826c64b 100644 (file)
@@ -506,6 +506,12 @@ ip.ttl => Syntax error at end of input expecting `--'.
 # conntrack
 ct_next; => actions=ct(table=27,zone=NXM_NX_REG5[0..15]), prereqs=ip
 ct_commit; => actions=ct(commit,zone=NXM_NX_REG5[0..15]), prereqs=ip
+ct_commit(); => actions=ct(commit,zone=NXM_NX_REG5[0..15]), prereqs=ip
+ct_commit(ct_mark=1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_mark)), prereqs=ip
+ct_commit(ct_mark=1/1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1/0x1->ct_mark)), prereqs=ip
+ct_commit(ct_label=1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_label)), prereqs=ip
+ct_commit(ct_label=1/1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1/0x1->ct_label)), prereqs=ip
+ct_commit(ct_mark=1, ct_label=2); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_mark,set_field:0x2->ct_label)), prereqs=ip
 
 # dnat
 ct_dnat; => actions=ct(table=27,zone=NXM_NX_REG3[0..15],nat), prereqs=ip