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_label=1); ct_commit(ct_mark=1, ct_label=1); We would like to eventually also allow setting ct_mark and ct_label with a masked value. There are currently problems with this that Joe is working on, so that support will be added later. 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 <russ...@ovn.org> Acked-by: Ben Pfaff <b...@ovn.org> --- ovn/lib/actions.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- ovn/ovn-sb.xml | 13 +++++- tests/ovn.at | 4 ++ 3 files changed, 132 insertions(+), 5 deletions(-) diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 44957c7..9d6f557 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -412,7 +412,8 @@ parse_put_arp_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, ovs_be128 *ct_label) { struct ofpact_conntrack *ct = ofpact_put_CT(ctx->ofpacts); ct->flags |= commit ? NX_CT_F_COMMIT : 0; @@ -438,6 +439,119 @@ 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 = 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 = 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=0 + * ct_label=0 + * + * If a comma separates the current argument from the next argument, this + * function will consume it. + * + * 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, + bool *set_label, ovs_be128 *label_value) +{ + 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 (!action_get_int(ctx, mark_value)) { + return false; + } + *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; + } + /* XXX We only support a ct_label value specified as decimal. + * ct_label is 128-bit, so we should eventually also support specifying + * full 128-bit values as hex. Hex support isn't really needed until + * we need more than 32 bits. */ + int val; + if (!action_get_int(ctx, &val)) { + return false; + } + label_value->be32[3] = htonl(val); + *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); + return; + } + + if (lexer_match(ctx->lexer, LEX_T_RPAREN)) { + /* ct_commit(); */ + emit_ct(ctx, false, true, NULL, NULL); + return; + } + + /* 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; + ovs_be128 label_value = { .be32 = { 0, }, }; + + while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { + if (!parse_ct_commit_arg(ctx, &set_mark, &mark_value, + &set_label, &label_value)) { + return; + } + } + + emit_ct(ctx, false, true, + set_mark ? &mark_value : NULL, + set_label ? &label_value : NULL); } static bool @@ -464,9 +578,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); } 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, "arp")) { parse_arp_action(ctx); } else if (lexer_match_id(ctx->lexer, "get_arp")) { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index efd2f9a..aa614fc 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -939,15 +939,24 @@ </dd> <dt><code>ct_commit;</code></dt> + <dt><code>ct_commit(ct_mark=VALUE);</code></dt> <dd> <p> Commit the flow to the connection tracking entry associated - with it by a previous call to <code>ct_next</code>. + with it by a previous call to <code>ct_next</code>. When + the ct_mark=VALUE parameter is supplied, ct_mark will be set + to the 32-bit integer indicated by VALUE on the connection + tracking entry. </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 seting <code>ct_mark</code> + on a connection tracking entry before dropping a packet, + for example. </p> </dd> diff --git a/tests/ovn.at b/tests/ovn.at index 22121e1..1f57cd6 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -506,6 +506,10 @@ 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_label=1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field: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 # arp arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; => actions=controller(userdata=00.00.00.00.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00), prereqs=ip4 -- 2.5.5 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev