ovn-controller: A new action "select".

Support a new logical flow action "select", which can be used to
implement features such as ECMP.  The action uses OpenFlow group
action to select an integer (uint16_t) from a list of integers,
and assign it to specified field, e.g.:
    reg0 = select(1, 2, 3)
A weight can be specified for each member as well, e.g.:
    reg0 = select(1=20, 2=30, 3=50)

Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>

include/ovn/actions.h

@@ -61,6 +61,7 @@ struct ovn_extend_table;
     OVNACT(CT_DNAT,           ovnact_ct_nat)          \
     OVNACT(CT_SNAT,           ovnact_ct_nat)          \
     OVNACT(CT_LB,             ovnact_ct_lb)           \
+    OVNACT(SELECT,            ovnact_select)          \
     OVNACT(CT_CLEAR,          ovnact_null)            \
     OVNACT(CLONE,             ovnact_nest)            \
     OVNACT(ARP,               ovnact_nest)            \
@@ -251,6 +252,20 @@ struct ovnact_ct_lb {
     uint8_t ltable;             /* Logical table ID of next table. */
 };
 
+struct ovnact_select_dst {
+    uint16_t id;
+    uint16_t weight;
+};
+
+/* OVNACT_SELECT. */
+struct ovnact_select {
+    struct ovnact ovnact;
+    struct ovnact_select_dst *dsts;
+    size_t n_dsts;
+    uint8_t ltable;             /* Logical table ID of next table. */
+    struct expr_field res_field;
+};
+
 /* OVNACT_ARP, OVNACT_ND_NA, OVNACT_CLONE. */
 struct ovnact_nest {
     struct ovnact ovnact;

lib/actions.c

@@ -218,17 +218,18 @@ action_parse_field(struct action_context *ctx,
 }
 
 static bool
-action_parse_port(struct action_context *ctx, uint16_t *port)
+action_parse_uint16(struct action_context *ctx, uint16_t *_value,
+                    const char *msg)
 {
     if (lexer_is_int(ctx->lexer)) {
         int value = ntohll(ctx->lexer->token.value.integer);
         if (value <= UINT16_MAX) {
-            *port = value;
+            *_value = value;
             lexer_get(ctx->lexer);
             return true;
         }
     }
-    lexer_syntax_error(ctx->lexer, "expecting port number");
+    lexer_syntax_error(ctx->lexer, "expecting %s", msg);
     return false;
 }
 
@@ -927,7 +928,7 @@ parse_ct_lb_action(struct action_context *ctx)
                 }
                 dst.port = 0;
                 if (lexer_match(ctx->lexer, LEX_T_COLON)
-                    && !action_parse_port(ctx, &dst.port)) {
+                    && !action_parse_uint16(ctx, &dst.port, "port number")) {
                     free(dsts);
                     return;
                 }
@@ -957,7 +958,8 @@ parse_ct_lb_action(struct action_context *ctx)
                         lexer_syntax_error(ctx->lexer, "IPv6 address needs "
                                 "square brackets if port is included");
                         return;
-                    } else if (!action_parse_port(ctx, &dst.port)) {
+                    } else if (!action_parse_uint16(ctx, &dst.port,
+                                                    "port number")) {
                         free(dsts);
                         return;
                     }
@@ -1099,6 +1101,138 @@ ovnact_ct_lb_free(struct ovnact_ct_lb *ct_lb)
 }
 
 static void
+parse_select_action(struct action_context *ctx, struct expr_field *res_field)
+{
+    /* Check if the result field is modifiable. */
+    char *error = expr_type_check(res_field, res_field->n_bits, true);
+    if (error) {
+        lexer_error(ctx->lexer, "%s", error);
+        free(error);
+        return;
+    }
+
+    if (res_field->n_bits < 16) {
+        lexer_error(ctx->lexer, "cannot use %d-bit field %s[%d..%d] "
+                    "for \"select\", which requires at least 16 bits.",
+                    res_field->n_bits, res_field->symbol->name,
+                    res_field->ofs,
+                    res_field->ofs + res_field->n_bits - 1);
+        return;
+    }
+
+    if (ctx->pp->cur_ltable >= ctx->pp->n_tables) {
+        lexer_error(ctx->lexer,
+                    "\"select\" action not allowed in last table.");
+        return;
+    }
+
+    struct ovnact_select_dst *dsts = NULL;
+    size_t allocated_dsts = 0;
+    size_t n_dsts = 0;
+
+    lexer_get(ctx->lexer); /* Skip "select". */
+    lexer_get(ctx->lexer); /* Skip '('. */
+
+    while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+        struct ovnact_select_dst dst;
+        if (!action_parse_uint16(ctx, &dst.id, "id")) {
+            free(dsts);
+            return;
+        }
+
+        dst.weight = 0;
+        if (lexer_match(ctx->lexer, LEX_T_EQUALS)) {
+            if (!action_parse_uint16(ctx, &dst.weight, "weight")) {
+                free(dsts);
+                return;
+            }
+            if (dst.weight == 0) {
+                lexer_syntax_error(ctx->lexer, "weight can't be 0");
+            }
+        }
+        lexer_match(ctx->lexer, LEX_T_COMMA);
+
+        /* Append to dsts. */
+        if (n_dsts >= allocated_dsts) {
+            dsts = x2nrealloc(dsts, &allocated_dsts, sizeof *dsts);
+        }
+        dsts[n_dsts++] = dst;
+    }
+    if (n_dsts <= 1) {
+        lexer_syntax_error(ctx->lexer, "expecting at least 2 group members");
+        return;
+    }
+
+    struct ovnact_select *select = ovnact_put_SELECT(ctx->ovnacts);
+    select->ltable = ctx->pp->cur_ltable + 1;
+    select->dsts = dsts;
+    select->n_dsts = n_dsts;
+    select->res_field = *res_field;
+}
+
+static void
+format_SELECT(const struct ovnact_select *select, struct ds *s)
+{
+    expr_field_format(&select->res_field, s);
+    ds_put_cstr(s, " = ");
+    ds_put_cstr(s, "select");
+    ds_put_char(s, '(');
+    for (size_t i = 0; i < select->n_dsts; i++) {
+        if (i) {
+            ds_put_cstr(s, ", ");
+        }
+
+        const struct ovnact_select_dst *dst = &select->dsts[i];
+        ds_put_format(s, "%"PRIu16, dst->id);
+        ds_put_format(s, "=%"PRIu16, dst->weight ? dst->weight : 100);
+    }
+    ds_put_char(s, ')');
+    ds_put_char(s, ';');
+}
+
+static void
+encode_SELECT(const struct ovnact_select *select,
+             const struct ovnact_encode_params *ep,
+             struct ofpbuf *ofpacts)
+{
+    ovs_assert(select->n_dsts >= 1);
+    uint8_t resubmit_table = select->ltable + first_ptable(ep, ep->pipeline);
+    uint32_t table_id = 0;
+    struct ofpact_group *og;
+
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    ds_put_format(&ds, "type=select,selection_method=dp_hash");
+
+    struct mf_subfield sf = expr_resolve_field(&select->res_field);
+
+    for (size_t bucket_id = 0; bucket_id < select->n_dsts; bucket_id++) {
+        const struct ovnact_select_dst *dst = &select->dsts[bucket_id];
+        ds_put_format(&ds, ",bucket=bucket_id=%"PRIuSIZE",weight:%"PRIu16
+                      ",actions=", bucket_id, dst->weight ? dst->weight : 100);
+        ds_put_format(&ds, "load:%u->%s[%u..%u],", dst->id, sf.field->name,
+                      sf.ofs, sf.ofs + sf.n_bits - 1);
+        ds_put_format(&ds, "resubmit(,%d)", resubmit_table);
+    }
+
+    table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
+                                          ep->lflow_uuid);
+    ds_destroy(&ds);
+    if (table_id == EXT_TABLE_ID_INVALID) {
+        return;
+    }
+
+    /* Create an action to set the group. */
+    og = ofpact_put_GROUP(ofpacts);
+    og->group_id = table_id;
+}
+
+static void
+ovnact_select_free(struct ovnact_select *select)
+{
+    free(select->dsts);
+}
+
+static void
 format_CT_CLEAR(const struct ovnact_null *null OVS_UNUSED, struct ds *s)
 {
     ds_put_cstr(s, "ct_clear;");
@@ -2868,6 +3002,9 @@ parse_set_action(struct action_context *ctx)
     } else if (lexer_match(ctx->lexer, LEX_T_EQUALS)) {
         if (ctx->lexer->token.type != LEX_T_ID) {
             parse_LOAD(ctx, &lhs);
+        } else if (!strcmp(ctx->lexer->token.s, "select")
+                   && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
+            parse_select_action(ctx, &lhs);
         } else if (!strcmp(ctx->lexer->token.s, "put_dhcp_opts")
                    && lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
             parse_put_dhcp_opts(ctx, &lhs, ovnact_put_PUT_DHCPV4_OPTS(

ovn-sb.xml

@@ -2114,6 +2114,40 @@ tcp.flags = RST;
 
           <p><b>Example:</b> <code>handle_svc_check(inport);</code></p>
         </dd>
+
+        <dt><code><var>R</var> = select(<var>N1</var>[=<var>W1</var>], <var>N2</var>[=<var>W2</var>], ...);</code></dt>
+        <dd>
+          <p>
+            <b>Parameters</b>: Integer <var>N1</var>, <var>N2</var>..., with
+            optional weight <var>W1</var>, <var>W2</var>, ...
+          </p>
+
+          <p>
+            <b>Result</b>: stored to a logical field or subfield <var>R</var>.
+          </p>
+
+          <p>
+            Select from a list of integers <var>N1</var>, <var>N2</var>...,
+            each within the range 0 ~ 65535, and store the selected one in the
+            field <var>R</var>.  There must be 2 or more integers listed, each
+            with an optional weight, which is an integer within the range 1 ~
+            65535.  If weight is not specified, it defaults to 100. The
+            selection method is based on the 5-tuple hash of packet header.
+          </p>
+
+          <p>
+            Processing automatically moves on to the next table, as if
+            <code>next;</code> were specified.  The <code>select</code> action
+            must be put as the last action of the logical flow when there are
+            multiple actions (actions put after <code>select</code> will not
+            take effect).
+          </p>
+
+          <p>
+            <b>Example:</b> <code>reg8[16..31] = select(1=20, 2=30, 3=50);</code>
+          </p>
+        </dd>
+
       </dl>
     </column>
 

tests/ovn.at

@@ -965,14 +965,17 @@ ct_lb();
     has prereqs ip
 ct_lb(192.168.1.2:80, 192.168.1.3:80);
     encodes as group:1
+    uses group: id(1), 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]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=192.168.1.3:80),commit,table=19,zone=NXM_NX_REG13[0..15]))
     has prereqs ip
 ct_lb(192.168.1.2, 192.168.1.3, );
     formats as ct_lb(192.168.1.2, 192.168.1.3);
     encodes as group:2
+    uses group: id(2), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=ct(nat(dst=192.168.1.2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=192.168.1.3),commit,table=19,zone=NXM_NX_REG13[0..15]))
     has prereqs ip
 ct_lb(fd0f::2, fd0f::3, );
     formats as ct_lb(fd0f::2, fd0f::3);
     encodes as group:3
+    uses group: id(3), name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
     has prereqs ip
 
 ct_lb(192.168.1.2:);
@@ -1481,6 +1484,30 @@ handle_svc_check();
 handle_svc_check(reg0);
     Cannot use numeric field reg0 where string field is required.
 
+# 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:4
+    uses group: id(4), 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:5
+    uses group: id(5), 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.
+reg0 = select(1=0, 2);
+    Syntax error at `,' weight can't be 0.
+reg0 = select(1=123456, 2);
+    Syntax error at `123456' expecting weight.
+reg0 = select(123);
+    Syntax error at `;' expecting at least 2 group members.
+ip.proto = select(1, 2, 3);
+    Field ip.proto is not modifiable.
+reg0[0..14] = select(1, 2, 3);
+    cannot use 15-bit field reg0[0..14] for "select", which requires at least 16 bits.
+
 # Miscellaneous negative tests.
 ;
     Syntax error at `;'.

tests/test-ovn.c

@@ -1228,6 +1228,28 @@ test_expr_to_packets(struct ovs_cmdl_context *ctx OVS_UNUSED)
 
 /* Actions. */
 
+static void
+print_group_info(struct ovn_extend_table *group_table, const char *action)
+{
+    struct ovn_extend_table_info *g;
+    HMAP_FOR_EACH (g, hmap_node, &group_table->desired) {
+        char buf[64];
+        sprintf(buf, "group:%"PRIu32, g->table_id);
+        char *match = strstr(buf, action);
+        if (match) {
+            if (match[strlen(buf)] != '\0') {
+                /* Add ',' and match again. */
+                sprintf(buf, "group:%"PRIu32",", g->table_id);
+                match = strstr(buf, action);
+            }
+        }
+        if (match) {
+            printf("    uses group: id(%"PRIu32"), name(%s)\n",
+                   g->table_id, g->name);
+        }
+    }
+}
+
 static void
 test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
 {
@@ -1307,7 +1329,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED)
             struct ds ofpacts_s = DS_EMPTY_INITIALIZER;
             struct ofpact_format_params fp = { .s = &ofpacts_s };
             ofpacts_format(ofpacts.data, ofpacts.size, &fp);
-            printf("    encodes as %s\n", ds_cstr(&ofpacts_s));
+            char *ofpacts_cstr = ds_cstr(&ofpacts_s);
+            printf("    encodes as %s\n", ofpacts_cstr);
+            print_group_info(&group_table, ofpacts_cstr);
             ds_destroy(&ofpacts_s);
             ofpbuf_uninit(&ofpacts);
 

utilities/ovn-trace.8.xml

@@ -438,6 +438,15 @@
       <code>--lb-dst</code> is not available in daemon mode.
     </dd>
 
+    <dt><code>--select-id=</code><var>id</var></dt>
+    <dd>
+      Specify the <var>id</var> to be selected by the <code>select</code>
+      action. <var>id</var> must be one of the values listed in the
+      <code>select</code> action.  Otherwise, a random id is selected from
+      the list, as if <code>--select-id</code> were not specified.
+      <code>--select-id</code> is not available in daemon mode.
+    </dd>
+
     <dt><code>--friendly-names</code></dt>
     <dt><code>--no-friendly-names</code></dt>
     <dd>