[ovs-dev] [PATCH v3 2/3] OVN ACL: Allow ct_mark and ct_label values to be set from register as well

Ankur Sharma ankur.sharma at nutanix.com
Tue Apr 16 23:58:08 UTC 2019


OVN allows only an integer (or masked integer) to be assigned to
ct_mark and ct_label.

This patch, enhances the parser code to allow ct_mark and ct_label
to be assigned from 32 bit registers (MFF_REG0 - MFF_REG15) and  128
bit registers (MFF_XXREG0 - MFF_XXREG3) respectively.

Signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>
---
 include/ovn/actions.h |  3 ++
 ovn/lib/actions.c     | 77 +++++++++++++++++++++++++++++++++++++++++++++------
 ovn/ovn-sb.xml        | 20 +++++++------
 tests/ovn.at          | 16 +++++++++++
 4 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 1c0c67c..58b96a1 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -24,6 +24,7 @@
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/uuid.h"
+#include "openvswitch/meta-flow.h"
 #include "util.h"
 
 struct expr;
@@ -196,8 +197,10 @@ struct ovnact_ct_next {
 /* OVNACT_CT_COMMIT. */
 struct ovnact_ct_commit {
     struct ovnact ovnact;
+    bool is_ct_mark_reg, is_ct_label_reg; /* If the value is from a register */
     uint32_t ct_mark, ct_mark_mask;
     ovs_be128 ct_label, ct_label_mask;
+    enum mf_field_id ct_mark_reg, ct_label_reg;
 };
 
 /* OVNACT_CT_DNAT, OVNACT_CT_SNAT. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index eb7e5ba..d8b86dc 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -627,8 +627,28 @@ parse_ct_commit_arg(struct action_context *ctx,
         } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
             cc->ct_mark = ntohll(ctx->lexer->token.value.integer);
             cc->ct_mark_mask = ntohll(ctx->lexer->token.mask.integer);
+        } else if (ctx->lexer->token.type == LEX_T_ID) {
+
+           cc->ct_mark_mask = UINT32_MAX;
+
+           const struct mf_field *mf = mf_from_name(ctx->lexer->token.s);
+           if (mf) {
+
+              if (mf->id >= MFF_REG0 && mf->id <= MFF_REG15) {
+                 cc->is_ct_mark_reg = true;
+                 cc->ct_mark_reg = mf->id;
+              } else {
+                 lexer_syntax_error(ctx->lexer, "input: %s,  not a 32 bit "
+                                    "register", mf->name);
+                 return;
+              }
+           } else {
+              lexer_syntax_error(ctx->lexer, "invalid field name: %s",
+                                 ctx->lexer->token.s);
+              return;
+           }
         } else {
-            lexer_syntax_error(ctx->lexer, "expecting integer");
+            lexer_syntax_error(ctx->lexer, "invalid token type");
             return;
         }
         lexer_get(ctx->lexer);
@@ -642,9 +662,28 @@ parse_ct_commit_arg(struct action_context *ctx,
         } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) {
             cc->ct_label = ctx->lexer->token.value.be128_int;
             cc->ct_label_mask = ctx->lexer->token.mask.be128_int;
+        } else if (ctx->lexer->token.type == LEX_T_ID) {
+
+           cc->ct_label_mask = OVS_BE128_MAX;
+           const struct mf_field *mf = mf_from_name(ctx->lexer->token.s);
+           if (mf) {
+              if (mf->id >= MFF_XXREG0 && mf->id <= MFF_XXREG3) {
+                 cc->is_ct_label_reg = true;
+                 cc->ct_label_reg = mf->id;
+              } else {
+                 lexer_syntax_error(ctx->lexer, "input: %s,  not a 128 bit "
+                                    "register", mf->name);
+                 return;
+              }
+           } else {
+              lexer_syntax_error(ctx->lexer, "invalid field name: %s",
+                                 ctx->lexer->token.s);
+              return;
+           }
+
         } else {
-            lexer_syntax_error(ctx->lexer, "expecting integer");
-            return;
+           lexer_syntax_error(ctx->lexer, "invalid token type");
+           return;
         }
         lexer_get(ctx->lexer);
     } else {
@@ -713,14 +752,36 @@ encode_CT_COMMIT(const struct ovnact_ct_commit *cc,
     ofpbuf_pull(ofpacts, set_field_offset);
 
     if (cc->ct_mark_mask) {
-        const ovs_be32 value = htonl(cc->ct_mark);
-        const ovs_be32 mask = htonl(cc->ct_mark_mask);
-        ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), &value, &mask);
+       if (cc->is_ct_mark_reg) {
+          struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
+
+          move->src.field = mf_from_id(cc->ct_mark_reg);
+          move->src.ofs = 0;
+          move->src.n_bits = 32;
+          move->dst.field = mf_from_id(MFF_CT_MARK);
+          move->dst.ofs = 0;
+          move->dst.n_bits = 32;
+       } else {
+          const ovs_be32 value = htonl(cc->ct_mark);
+          const ovs_be32 mask = htonl(cc->ct_mark_mask);
+          ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_MARK), &value, &mask);
+       }
     }
 
     if (!ovs_be128_is_zero(cc->ct_label_mask)) {
-        ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL), &cc->ct_label,
-                             &cc->ct_label_mask);
+       if (cc->is_ct_label_reg) {
+          struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts);
+
+          move->src.field = mf_from_id(cc->ct_label_reg);
+          move->src.ofs = 0;
+          move->src.n_bits = 128;
+          move->dst.field = mf_from_id(MFF_CT_LABEL);
+          move->dst.ofs = 0;
+          move->dst.n_bits = 128;
+       } else {
+          ofpact_put_set_field(ofpacts, mf_from_id(MFF_CT_LABEL), &cc->ct_label,
+                               &cc->ct_label_mask);
+       }
     }
 
     ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 5c4a852..35719c1 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1180,19 +1180,21 @@
         </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>
+        <dt><code>ct_commit(ct_mark=(<var>value[/mask]</var> OR <var>regX</var>));</code></dt>
+        <dt><code>ct_commit(ct_label=(<var>value[/mask]</var> OR <var>xxregX</var>));</code></dt>
+        <dt><code>ct_commit(ct_mark=(<var>value[/mask]</var> OR <var>regX</var>), ct_label=(<var>value[/mask]</var> OR <var>xxregX</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>.  When
-            <code>ct_mark=<var>value[/mask]</var></code> and/or
-            <code>ct_label=<var>value[/mask]</var></code> are supplied,
+            by a previous call to <code>ct_next</code>. When
+            <code>ct_mark=<var>value[/mask]</var> OR <var>xxregX</var></code> and/or
+            <code>ct_label=<var>value[/mask]</var> OR <var>xxregX</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 a 128-bit field. The <var>value[/mask]</var>
+            values indicated by <var>value[/mask]</var> or 32 bit/128 bit registers
+            on the connection tracking entry. <code>ct_mark</code> is a 32-bit field
+            and hence will read value only from a 32 bit register (reg0 - reg9).
+            <code>ct_label</code> is a 128-bit field and hence will read value only
+            from a 128 bit register (xxreg0 - xxreg1). The <var>value[/mask]</var>
             should be specified in hex string if more than 64bits are to be used.
           </p>
 
diff --git a/tests/ovn.at b/tests/ovn.at
index b546e9a..f4e3650 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1021,6 +1021,22 @@ ct_commit(ct_label=18446744073709551615);
 ct_commit(ct_label=18446744073709551616);
     Decimal constants must be less than 2**64.
 
+ct_commit(ct_label=xxreg1);
+    formats as ct_commit(ct_label=0);
+    encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(move:NXM_NX_XXREG1[]->NXM_NX_CT_LABEL[]))
+    has prereqs ip
+
+ct_commit(ct_mark=reg1);
+    formats as ct_commit(ct_mark=0);
+    encodes as ct(commit,zone=NXM_NX_REG13[0..15],exec(move:NXM_NX_REG1[]->NXM_NX_CT_MARK[]))
+    has prereqs ip
+
+ct_commit(ct_label=reg1);
+    Syntax error at `reg1' input: reg1,  not a 128 bit register.
+
+ct_commit(ct_mark=xxreg1);
+    Syntax error at `xxreg1' input: xxreg1,  not a 32 bit register.
+
 # ct_dnat
 ct_dnat;
     encodes as ct(table=19,zone=NXM_NX_REG11[0..15],nat)
-- 
1.8.3.1



More information about the dev mailing list