From a9e1b66f2717a73b35e7bfaebbdcef04e91dc7be Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 2 Mar 2016 18:04:46 +0000 Subject: [PATCH] ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action. 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_mark=1/1); ct_commit(ct_label=1); ct_commit(ct_label=1/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)) A future commit will make use of this feature. Signed-off-by: Russell Bryant Acked-by: Ben Pfaff Acked-by: Justin Pettit --- ovn/TODO | 6 ++ ovn/lib/actions.c | 151 +++++++++++++++++++++++++++++++++++++++++++++- ovn/ovn-sb.xml | 22 ++++++- tests/ovn.at | 6 ++ 4 files changed, 179 insertions(+), 6 deletions(-) diff --git a/ovn/TODO b/ovn/TODO index 0a6225dbb..4f134a4b3 100644 --- a/ovn/TODO +++ b/ovn/TODO @@ -38,6 +38,12 @@ ovn-sb.xml includes a tentative specification for this action. IPv6 will probably need an action or actions for ND that is similar to the "arp" action, and an action for generating +*** ct_label 128-bit support. + +We only support 64-bits for the ct_label argument to ct_commit(), but ct_label +is a 128-bit field. The OVN lexer only supports parsing 64-bit integers, but +we can use parse_int_string() to support larger integers. + ** IPv6 *** ND versus ARP diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 569970e2d..52c74e6e9 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -624,7 +624,9 @@ parse_put_dhcp_opts_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, int *ct_mark_mask, + ovs_be128 *ct_label, ovs_be128 *ct_label_mask) { struct ofpact_conntrack *ct = ofpact_put_CT(ctx->ofpacts); ct->flags |= commit ? NX_CT_F_COMMIT : 0; @@ -650,6 +652,149 @@ 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 = ct_mark_mask ? htonl(*ct_mark_mask) : 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 = ct_label_mask ? *ct_label_mask : 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=[/] + * ct_label=[/] + * + * If a comma separates the current argument from the next argument, this + * function will consume it. + * + * set_mark - This will be set to true if a value for ct_mark was successfully + * parsed. Otherwise, it will be unchanged. + * mark_value - If set_mark was set to true, this will contain the value + * parsed for ct_mark. + * mark_mask - If set_mark was set to true, this will contain the mask + * for ct_mark if one was found. Otherwise, it will be + * unchanged, so the caller should initialize this to an + * appropriate value. + * set_label - This will be set to true if a value for ct_label was successfully + * parsed. Otherwise, it will be unchanged. + * label_value - If set_label was set to true, this will contain the value + * parsed for ct_label. + * label_mask - If set_label was set to true, this will contain the mask + * for ct_label if one was found. Otherwise, it will be + * unchanged, so the caller should initialize this to an + * appropriate value. + * + * 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, int *mark_mask, + bool *set_label, ovs_be128 *label_value, + ovs_be128 *label_mask) +{ + 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 (ctx->lexer->token.type == LEX_T_INTEGER) { + *mark_value = ntohll(ctx->lexer->token.value.integer); + } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) { + *mark_value = ntohll(ctx->lexer->token.value.integer); + *mark_mask = ntohll(ctx->lexer->token.mask.integer); + } else { + action_error(ctx, "Expected integer after 'ct_mark='"); + return false; + } + lexer_get(ctx->lexer); + *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; + } + if (ctx->lexer->token.type == LEX_T_INTEGER) { + label_value->be64.lo = ctx->lexer->token.value.integer; + } else if (ctx->lexer->token.type == LEX_T_MASKED_INTEGER) { + /* XXX Technically, ct_label is a 128-bit field. The lexer + * only supports 64-bit integers, so that's all we support + * here. More work is needed to use parse_int_string() + * to support the full 128-bits. */ + label_value->be64.lo = ctx->lexer->token.value.integer; + label_mask->be64.hi = 0; + label_mask->be64.lo = ctx->lexer->token.mask.integer; + } else { + action_error(ctx, "Expected integer after 'ct_label='"); + return false; + } + lexer_get(ctx->lexer); + *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, NULL, NULL); + return; + } + + /* ct_commit(); + * 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; + int mark_mask = ~0; + ovs_be128 label_value = { .be32 = { 0, }, }; + ovs_be128 label_mask = OVS_BE128_MAX; + + while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { + if (!parse_ct_commit_arg(ctx, &set_mark, &mark_value, &mark_mask, + &set_label, &label_value, &label_mask)) { + return; + } + } + + emit_ct(ctx, false, true, + set_mark ? &mark_value : NULL, + set_mark ? &mark_mask : NULL, + set_label ? &label_value : NULL, + set_label ? &label_mask : NULL); } static void @@ -755,9 +900,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, 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, "ct_dnat")) { parse_ct_nat(ctx, false); } else if (lexer_match_id(ctx->lexer, "ct_snat")) { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index f33037432..3db846f25 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -939,15 +939,31 @@
ct_commit;
+
ct_commit(ct_mark=value[/mask]);
+
ct_commit(ct_label=value[/mask]);
+
ct_commit(ct_mark=value[/mask], ct_label=value[/mask]);

- Commit the flow to the connection tracking entry associated - with it by a previous call to ct_next. + Commit the flow to the connection tracking entry associated with it + by a previous call to ct_next. When + ct_mark=value[/mask] and/or + ct_label=value[/mask] are supplied, + ct_mark and/or ct_label will be set to the + values indicated by value[/mask] on the connection + tracking entry. ct_mark is a 32-bit field. + ct_label is technically a 128-bit field, though OVN + currently only supports 64-bits and will later be extended to + support the full 128-bits.

+

Note that if you want processing to continue in the next table, you must execute the next action after - ct_commit. + ct_commit. You may also leave out next + which will commit connection tracking state, and then drop the + packet. This could be useful for setting ct_mark + on a connection tracking entry before dropping a packet, + for example.

diff --git a/tests/ovn.at b/tests/ovn.at index 297070cb2..826c64b42 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -506,6 +506,12 @@ 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_mark=1/1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1/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_label=1/1); => actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1/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 # dnat ct_dnat; => actions=ct(table=27,zone=NXM_NX_REG3[0..15],nat), prereqs=ip -- 2.20.1