diff mbox series

[ovs-dev,2/5] actions: Add action ct_lb_mark.

Message ID 20220313195532.1141490-3-hzhou@ovn.org
State Superseded
Headers show
Series Use ct_mark for masked access to make flows HW-offloading friendly. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Han Zhou March 13, 2022, 7:55 p.m. UTC 
Add a new action ct_lb_mark, which is the same as ct_lb except that it
internally uses ct_mark to store the NAT flag, while ct_lb uses ct_label
for the same purpose. This will be used later to move the masked access
of ct_label to ct_mark while keeping the backward compatibility.

Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 include/ovn/actions.h |  3 ++-
 lib/actions.c         | 55 ++++++++++++++++++++++++++++++++++++-------
 ovn-sb.xml            | 10 ++++++++
 tests/ovn.at          | 24 +++++++++++--------
 utilities/ovn-trace.c |  8 ++++++-
 5 files changed, 79 insertions(+), 21 deletions(-)

Comments

0-day Robot March 13, 2022, 9:20 p.m. UTC | #1 
Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 144 characters long (recommended limit is 79)
#161 FILE: ovn-sb.xml:1992:
        <dt><code>ct_lb_mark(backends=<var>ip</var>[:<var>port</var>][,...][; hash_fields=<var>field1</var>,<var>field2</var>,...]);</code></dt>

WARNING: Line is 80 characters long (recommended limit is 79)
#164 FILE: ovn-sb.xml:1995:
              Same as <code>ct_lb</code>, except that it internally uses ct_mark

Lines checked: 262, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0641b927e..8dfb6fdc5 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -69,6 +69,7 @@  struct ovn_extend_table;
     OVNACT(CT_DNAT_IN_CZONE,  ovnact_ct_nat)          \
     OVNACT(CT_SNAT_IN_CZONE,  ovnact_ct_nat)          \
     OVNACT(CT_LB,             ovnact_ct_lb)           \
+    OVNACT(CT_LB_MARK,        ovnact_ct_lb)           \
     OVNACT(SELECT,            ovnact_select)          \
     OVNACT(CT_CLEAR,          ovnact_null)            \
     OVNACT(CLONE,             ovnact_nest)            \
@@ -273,7 +274,7 @@  struct ovnact_ct_lb_dst {
     uint16_t port;
 };
 
-/* OVNACT_CT_LB. */
+/* OVNACT_CT_LB/OVNACT_CT_LB_MARK. */
 struct ovnact_ct_lb {
     struct ovnact ovnact;
     struct ovnact_ct_lb_dst *dsts;
diff --git a/lib/actions.c b/lib/actions.c
index 5d3caaf2b..1c328f88d 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1079,7 +1079,7 @@  ovnact_ct_nat_free(struct ovnact_ct_nat *ct_nat OVS_UNUSED)
 }
 
 static void
-parse_ct_lb_action(struct action_context *ctx)
+parse_ct_lb_action(struct action_context *ctx, bool ct_lb_mark)
 {
     if (ctx->pp->cur_ltable >= ctx->pp->n_tables) {
         lexer_error(ctx->lexer, "\"ct_lb\" action not allowed in last table.");
@@ -1185,7 +1185,8 @@  parse_ct_lb_action(struct action_context *ctx)
         }
     }
 
-    struct ovnact_ct_lb *cl = ovnact_put_CT_LB(ctx->ovnacts);
+    struct ovnact_ct_lb *cl = ct_lb_mark ? ovnact_put_CT_LB_MARK(ctx->ovnacts)
+                                         : ovnact_put_CT_LB(ctx->ovnacts);
     cl->ltable = ctx->pp->cur_ltable + 1;
     cl->dsts = dsts;
     cl->n_dsts = n_dsts;
@@ -1193,9 +1194,13 @@  parse_ct_lb_action(struct action_context *ctx)
 }
 
 static void
-format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s)
+format_ct_lb(const struct ovnact_ct_lb *cl, struct ds *s, bool ct_lb_mark)
 {
-    ds_put_cstr(s, "ct_lb");
+    if (ct_lb_mark) {
+        ds_put_cstr(s, "ct_lb_mark");
+    } else {
+        ds_put_cstr(s, "ct_lb");
+    }
     if (cl->n_dsts) {
         ds_put_cstr(s, "(backends=");
         for (size_t i = 0; i < cl->n_dsts; i++) {
@@ -1231,9 +1236,22 @@  format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s)
 }
 
 static void
-encode_CT_LB(const struct ovnact_ct_lb *cl,
+format_CT_LB(const struct ovnact_ct_lb *cl, struct ds *s)
+{
+    format_ct_lb(cl, s, false);
+}
+
+static void
+format_CT_LB_MARK(const struct ovnact_ct_lb *cl, struct ds *s)
+{
+    format_ct_lb(cl, s, true);
+}
+
+static void
+encode_ct_lb(const struct ovnact_ct_lb *cl,
              const struct ovnact_encode_params *ep,
-             struct ofpbuf *ofpacts)
+             struct ofpbuf *ofpacts,
+             bool ct_lb_mark)
 {
     uint8_t recirc_table = cl->ltable + first_ptable(ep, ep->pipeline);
     if (!cl->n_dsts) {
@@ -1302,8 +1320,9 @@  encode_CT_LB(const struct ovnact_ct_lb *cl,
         ds_put_format(&ds, "),commit,table=%d,zone=NXM_NX_REG%d[0..15],"
                       "exec(set_field:"
                         OVN_CT_MASKED_STR(OVN_CT_NATTED)
-                      "->ct_label))",
-                      recirc_table, zone_reg);
+                      "->%s))",
+                      recirc_table, zone_reg,
+                      ct_lb_mark ? "ct_mark" : "ct_label");
     }
 
     table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
@@ -1318,6 +1337,22 @@  encode_CT_LB(const struct ovnact_ct_lb *cl,
     og->group_id = table_id;
 }
 
+static void
+encode_CT_LB(const struct ovnact_ct_lb *cl,
+             const struct ovnact_encode_params *ep,
+             struct ofpbuf *ofpacts)
+{
+    encode_ct_lb(cl, ep, ofpacts, false);
+}
+
+static void
+encode_CT_LB_MARK(const struct ovnact_ct_lb *cl,
+                  const struct ovnact_encode_params *ep,
+                  struct ofpbuf *ofpacts)
+{
+    encode_ct_lb(cl, ep, ofpacts, true);
+}
+
 static void
 ovnact_ct_lb_free(struct ovnact_ct_lb *ct_lb)
 {
@@ -4219,7 +4254,9 @@  parse_action(struct action_context *ctx)
     } else if (lexer_match_id(ctx->lexer, "ct_snat_in_czone")) {
         parse_CT_SNAT_IN_CZONE(ctx);
     } else if (lexer_match_id(ctx->lexer, "ct_lb")) {
-        parse_ct_lb_action(ctx);
+        parse_ct_lb_action(ctx, false);
+    } else if (lexer_match_id(ctx->lexer, "ct_lb_mark")) {
+        parse_ct_lb_action(ctx, true);
     } else if (lexer_match_id(ctx->lexer, "ct_clear")) {
         ovnact_put_CT_CLEAR(ctx->ovnacts);
     } else if (lexer_match_id(ctx->lexer, "clone")) {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 3afea4ed4..aa9ee0ff5 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -1988,6 +1988,16 @@ 
           </p>
         </dd>
 
+        <dt><code>ct_lb_mark;</code></dt>
+        <dt><code>ct_lb_mark(backends=<var>ip</var>[:<var>port</var>][,...][; hash_fields=<var>field1</var>,<var>field2</var>,...]);</code></dt>
+        <dd>
+          <p>
+              Same as <code>ct_lb</code>, except that it internally uses ct_mark
+              to store the NAT flag, while <code>ct_lb</code> uses ct_label for
+              the same purpose.
+          </p>
+        </dd>
+
         <dt>
           <code><var>R</var> = dns_lookup();</code>
         </dt>
diff --git a/tests/ovn.at b/tests/ovn.at
index 9b13a980d..691f74a43 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1075,6 +1075,10 @@  ct_lb(backends=fd0f::2,fd0f::3; hash_fields="eth_src,eth_dst,ip_src,ip_dst,sctp_
     uses group: id(8), name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,sctp_src,sctp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15],exec(set_field:2/2->ct_label)),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15],exec(set_field:2/2->ct_label)))
     has prereqs ip
 
+ct_lb_mark(backends=192.168.1.2:80,192.168.1.3:80);
+    encodes as group:9
+    uses group: id(9), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=ct(nat(dst=192.168.1.2:80),commit,table=19,zone=NXM_NX_REG13[0..15],exec(set_field:2/2->ct_mark)),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=192.168.1.3:80),commit,table=19,zone=NXM_NX_REG13[0..15],exec(set_field:2/2->ct_mark)))
+    has prereqs ip
 # ct_next
 ct_next;
     encodes as ct(table=19,zone=NXM_NX_REG13[0..15])
@@ -1827,13 +1831,13 @@  handle_svc_check(reg0);
 # select
 reg9[16..31] = select(1=50, 2=100, 3, );
     formats as reg9[16..31] = select(1=50, 2=100, 3=100);
-    encodes as group:9
-    uses group: id(9), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:50,actions=load:1->xreg4[16..31],resubmit(,19),bucket=bucket_id=1,weight:100,actions=load:2->xreg4[16..31],resubmit(,19),bucket=bucket_id=2,weight:100,actions=load:3->xreg4[16..31],resubmit(,19))
+    encodes as group:10
+    uses group: id(10), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:50,actions=load:1->xreg4[16..31],resubmit(,19),bucket=bucket_id=1,weight:100,actions=load:2->xreg4[16..31],resubmit(,19),bucket=bucket_id=2,weight:100,actions=load:3->xreg4[16..31],resubmit(,19))
 
 reg0 = select(1, 2);
     formats as reg0 = select(1=100, 2=100);
-    encodes as group:10
-    uses group: id(10), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:1->xxreg0[96..127],resubmit(,19),bucket=bucket_id=1,weight:100,actions=load:2->xxreg0[96..127],resubmit(,19))
+    encodes as group:11
+    uses group: id(11), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:1->xxreg0[96..127],resubmit(,19),bucket=bucket_id=1,weight:100,actions=load:2->xxreg0[96..127],resubmit(,19))
 
 reg0 = select(1=, 2);
     Syntax error at `,' expecting weight.
@@ -1850,12 +1854,12 @@  reg0[0..14] = select(1, 2, 3);
 
 fwd_group(liveness=true, childports="eth0", "lsp1");
     formats as fwd_group(liveness="true", childports="eth0", "lsp1");
-    encodes as group:11
-    uses group: id(11), name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=watch_port:17,load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
+    encodes as group:12
+    uses group: id(12), name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=watch_port:17,load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
 
 fwd_group(childports="eth0", "lsp1");
-    encodes as group:12
-    uses group: id(12), name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
+    encodes as group:13
+    uses group: id(13), name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
 
 fwd_group(childports=eth0);
     Syntax error at `eth0' expecting logical switch port.
@@ -1864,8 +1868,8 @@  fwd_group();
     Syntax error at `)' expecting `;'.
 
 fwd_group(childports="eth0", "lsp1");
-    encodes as group:12
-    uses group: id(12), name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
+    encodes as group:13
+    uses group: id(13), name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
 
 fwd_group(liveness=xyzzy, childports="eth0", "lsp1");
     Syntax error at `xyzzy' expecting true or false.
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index ece5803f2..d6ff75886 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -2409,7 +2409,8 @@  execute_ct_lb(const struct ovnact_ct_lb *ct_lb,
     }
 
     struct ovntrace_node *node = ovntrace_node_append(
-        super, OVNTRACE_NODE_TRANSFORMATION, "ct_lb%s",
+        super, OVNTRACE_NODE_TRANSFORMATION, "%s%s",
+        ct_lb->ovnact.type == OVNACT_CT_LB_MARK ? "ct_lb_mark" : "ct_lb",
         ds_cstr_ro(&comment));
     ds_destroy(&comment);
     trace__(dp, &ct_lb_flow, ct_lb->ltable, pipeline, &node->subs);
@@ -2634,6 +2635,11 @@  trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
             execute_ct_lb(ovnact_get_CT_LB(a), dp, uflow, pipeline, super);
             break;
 
+        case OVNACT_CT_LB_MARK:
+            execute_ct_lb(ovnact_get_CT_LB_MARK(a), dp, uflow, pipeline,
+                          super);
+            break;
+
         case OVNACT_SELECT:
             execute_select(ovnact_get_SELECT(a), dp, uflow,
                                    pipeline, super);