On 21 March 2016 at 07:54, Russell Bryant <russ...@ovn.org> wrote:
> 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);
>
> 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))
>
I think this feature makes it tricky to share zones with other stateful
additions. If you want to commit only once for all stateful services, then
set-field for ct_mark and ct_label will need to be loaded to registers in
advance, which I guess would mean that you loose 2 registers for this
purpose.
>
> A future commit will make use of this feature.
>
> Signed-off-by: Russell Bryant <russ...@ovn.org>
> ---
> ovn/lib/actions.c | 128
> ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> ovn/ovn-sb.xml | 13 +++++-
> 2 files changed, 136 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 6f67b93..a6b1988 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,127 @@ 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");
> +
> + if (ct_mark) {
> + size_t set_field_offset = ctx->ofpacts->size;
> + ofpbuf_pull(ctx->ofpacts, set_field_offset);
> +
> + 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;
> +
> + ctx->ofpacts->header = ofpbuf_push_uninit(ctx->ofpacts,
> set_field_offset);
> + ct = ctx->ofpacts->header;
> + ofpact_finish(ctx->ofpacts, &ct->ofpact);
> + }
> +
> + if (ct_label) {
> + size_t set_field_offset = ctx->ofpacts->size;
> + ofpbuf_pull(ctx->ofpacts, set_field_offset);
> +
> + 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 +586,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 31af8dc..467f889 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -915,15 +915,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>
>
> --
> 2.5.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
