Signed-off-by: Ben Pfaff <b...@nicira.com> --- ovn/TODO | 5 --- ovn/lib/actions.c | 4 ++- ovn/lib/expr.c | 97 ++++++++++++++++++++++++++++++++++++++----------------- ovn/lib/lex.c | 6 ++++ ovn/lib/lex.h | 1 + ovn/ovn-sb.xml | 19 ++++++++--- tests/ovn.at | 14 +++++++- 7 files changed, 105 insertions(+), 41 deletions(-)
diff --git a/ovn/TODO b/ovn/TODO index 740ea17..b29df75 100644 --- a/ovn/TODO +++ b/ovn/TODO @@ -130,11 +130,6 @@ 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 -*** Other actions. - -Possibly we'll need to implement "field1 <-> field2;" for swapping -fields. - *** ovn-controller translation to OpenFlow The following two translation strategies come to mind. Some of the diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 0a0158a..935706bc 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -105,6 +105,7 @@ action_syntax_error(struct action_context *ctx, const char *message, ...) ctx->error = ds_steal_cstr(&s); } +/* Parses an assignment or swap action. */ static void parse_set_action(struct action_context *ctx) { @@ -153,7 +154,8 @@ parse_actions(struct action_context *ctx) } enum lex_type lookahead = lexer_lookahead(ctx->lexer); - if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_LSQUARE) { + if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_SWAP + || lookahead == LEX_T_LSQUARE) { parse_set_action(ctx); } else if (lexer_match_id(ctx->lexer, "next")) { if (ctx->next_table_id) { diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index b7a3f7f..41da0fe 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -2620,12 +2620,14 @@ expr_is_normalized(const struct expr *expr) /* Action parsing helper. */ static bool -expand_symbol(struct expr_context *ctx, struct expr_field *f, - struct expr **prereqsp) +expand_symbol(struct expr_context *ctx, bool rw, bool swap, + struct expr_field *f, struct expr **prereqsp) { + const struct expr_symbol *orig_symbol = f->symbol; + if (f->symbol->expansion && f->symbol->level != EXPR_L_ORDINAL) { - expr_error(ctx, "Predicate symbol %s cannot be used in assignment.", - f->symbol->name); + expr_error(ctx, "Predicate symbol %s cannot be used in %s.", + f->symbol->name, swap ? "exchange" : "assignment"); return false; } @@ -2663,9 +2665,28 @@ expand_symbol(struct expr_context *ctx, struct expr_field *f, f->ofs += expansion.ofs; } + if (rw && !f->symbol->field->writable) { + expr_error(ctx, "Field %s is not modifiable.", orig_symbol->name); + return false; + } + return true; } +static void +mf_subfield_from_expr_field(const struct expr_field *f, struct mf_subfield *sf) +{ + sf->field = f->symbol->field; + sf->ofs = f->ofs; + sf->n_bits = f->n_bits ? f->n_bits : f->symbol->field->n_bits; +} + +static void +init_stack_action(const struct expr_field *f, struct ofpact_stack *stack) +{ + mf_subfield_from_expr_field(f, &stack->subfield); +} + static struct expr * parse_assignment(struct expr_context *ctx, const struct simap *ports, struct ofpbuf *ofpacts) @@ -2677,56 +2698,73 @@ parse_assignment(struct expr_context *ctx, const struct simap *ports, if (!parse_field(ctx, &dst)) { goto exit; } - if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) { + bool swap = lexer_match(ctx->lexer, LEX_T_SWAP); + if (!swap && !lexer_match(ctx->lexer, LEX_T_EQUALS)) { expr_syntax_error(ctx, "expecting `='."); goto exit; } const struct expr_symbol *orig_dst = dst.symbol; - if (!expand_symbol(ctx, &dst, &prereqs)) { - goto exit; - } - if (!dst.symbol->field->writable) { - expr_error(ctx, "Field %s is not modifiable.", orig_dst->name); + if (!expand_symbol(ctx, true, swap, &dst, &prereqs)) { goto exit; } - if (ctx->lexer->token.type == LEX_T_ID) { + if (swap || ctx->lexer->token.type == LEX_T_ID) { struct expr_field src; if (!parse_field(ctx, &src)) { goto exit; } const struct expr_symbol *orig_src = src.symbol; - if (!expand_symbol(ctx, &src, &prereqs)) { + if (!expand_symbol(ctx, swap, swap, &src, &prereqs)) { goto exit; } if ((dst.symbol->width != 0) != (src.symbol->width != 0)) { - expr_error(ctx, "Can't assign %s field (%s) to %s field (%s).", - orig_src->width ? "integer" : "string", - orig_src->name, - orig_dst->width ? "integer" : "string", - orig_dst->name); + if (swap) { + expr_error(ctx, + "Can't exchange %s field (%s) with %s field (%s).", + orig_dst->width ? "integer" : "string", + orig_dst->name, + orig_src->width ? "integer" : "string", + orig_src->name); + } else { + expr_error(ctx, "Can't assign %s field (%s) to %s field (%s).", + orig_src->width ? "integer" : "string", + orig_src->name, + orig_dst->width ? "integer" : "string", + orig_dst->name); + } goto exit; } if (dst.n_bits != src.n_bits) { - expr_error(ctx, "Can't assign %d-bit value to %d-bit destination.", - src.n_bits, dst.n_bits); + if (swap) { + expr_error(ctx, + "Can't exchange %d-bit field with %d-bit field.", + dst.n_bits, src.n_bits); + } else { + expr_error(ctx, + "Can't assign %d-bit value to %d-bit destination.", + src.n_bits, dst.n_bits); + } goto exit; } else if (!dst.n_bits && dst.symbol->field->n_bits != src.symbol->field->n_bits) { expr_error(ctx, "String fields %s and %s are incompatible for " - "assignment.", orig_dst->name, orig_src->name); + "%s.", orig_dst->name, orig_src->name, + swap ? "exchange" : "assignment"); goto exit; } - struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts); - move->src.field = src.symbol->field; - move->src.ofs = src.ofs; - move->src.n_bits = src.n_bits ? src.n_bits : src.symbol->field->n_bits; - move->dst.field = dst.symbol->field; - move->dst.ofs = dst.ofs; - move->dst.n_bits = dst.n_bits ? dst.n_bits : dst.symbol->field->n_bits; + if (swap) { + init_stack_action(&src, ofpact_put_STACK_PUSH(ofpacts)); + init_stack_action(&dst, ofpact_put_STACK_PUSH(ofpacts)); + init_stack_action(&src, ofpact_put_STACK_POP(ofpacts)); + init_stack_action(&dst, ofpact_put_STACK_POP(ofpacts)); + } else { + struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts); + mf_subfield_from_expr_field(&src, &move->src); + mf_subfield_from_expr_field(&dst, &move->dst); + } } else { struct expr_constant_set cs; if (!parse_constant_set(ctx, &cs)) { @@ -2776,8 +2814,9 @@ exit: } /* A helper for actions_parse(), to parse an OVN assignment action in the form - * "field = value" or "field1 = field2" into 'ofpacts'. The parameters and - * return value match those for actions_parse(). */ + * "field = value" or "field1 = field2", or a "swap" action in the form "field1 + * <-> field2", into 'ofpacts'. The parameters and return value match those + * for actions_parse(). */ char * expr_parse_assignment(struct lexer *lexer, const struct shash *symtab, const struct simap *ports, diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c index 46e86c2..332802e 100644 --- a/ovn/lib/lex.c +++ b/ovn/lib/lex.c @@ -238,6 +238,9 @@ lex_token_format(const struct lex_token *token, struct ds *s) case LEX_T_EQUALS: ds_put_cstr(s, "="); break; + case LEX_T_SWAP: + ds_put_cstr(s, "<->"); + break; default: OVS_NOT_REACHED(); } @@ -599,6 +602,9 @@ next: if (*p == '=') { token->type = LEX_T_LE; p++; + } else if (*p == '-' && p[1] == '>') { + token->type = LEX_T_SWAP; + p += 2; } else { token->type = LEX_T_LT; } diff --git a/ovn/lib/lex.h b/ovn/lib/lex.h index df4db2d..c27b189 100644 --- a/ovn/lib/lex.h +++ b/ovn/lib/lex.h @@ -58,6 +58,7 @@ enum lex_type { LEX_T_COMMA, /* , */ LEX_T_SEMICOLON, /* ; */ LEX_T_EQUALS, /* = */ + LEX_T_SWAP, /* <-> */ }; /* Subtype for LEX_T_INTEGER and LEX_T_MASKED_INTEGER tokens. diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 28b0d2c..3ae99b3 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -806,11 +806,11 @@ <p> Sets data or metadata field <var>field1</var> to the value of data or metadata field <var>field2</var>, e.g. <code>reg0 = - ip4.src;</code> to copy <code>ip4.src</code> into - <code>reg0</code>. To modify only a subset of a field's bits, - specify a subfield for <var>field1</var> or <var>field2</var> or - both, e.g. <code>vlan.pcp = reg0[0..2];</code> to set the VLAN PCP - from the least-significant bits of <code>reg0</code>. + ip4.src;</code> copies <code>ip4.src</code> into <code>reg0</code>. + To modify only a subset of a field's bits, specify a subfield for + <var>field1</var> or <var>field2</var> or both, e.g. <code>vlan.pcp + = reg0[0..2];</code> copies the least-significant bits of + <code>reg0</code> into the VLAN PCP. </p> <p> @@ -827,6 +827,15 @@ that a logical flow with such an assignment will never be matched. </p> </dd> + + <dt><code><var>field1</var> <-> <var>field2</var>;</code></dt> + <dd> + <p> + Similar to <code><var>field1</var> = <var>field2</var>;</code> + except that the two values are exchanged instead of copied. Both + <var>field1</var> and <var>field2</var> must modifiable. + </p> + </dd> </dl> <p> diff --git a/tests/ovn.at b/tests/ovn.at index 96f190d..4e9e1be 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -81,7 +81,7 @@ ff:ff:ff:ff:ff:ff/fe:ff:ff:ff:ff:ff => error("Value contains unmasked 1-bits.") fe:x => error("Invalid numeric constant.") 00:01:02:03:04:x => error("Invalid numeric constant.") -(){}[[]]==!=<<=>>=!&&||..,;= => ( ) { } [[ ]] == != < <= > >= ! && || .. , ; = +(){}[[]]==!=<<=>>=!&&||..,;=<-> => ( ) { } [[ ]] == != < <= > >= ! && || .. , ; = <-> & => error("`&' is only valid as part of `&&'.") | => error("`|' is only valid as part of `||'.") @@ -448,8 +448,13 @@ reg0 = reg1; => actions=move:OXM_OF_PKT_REG0[0..31]->OXM_OF_PKT_REG0[32..63], pr vlan.pcp = reg0[0..2]; => actions=move:OXM_OF_PKT_REG0[32..34]->NXM_OF_VLAN_TCI[13..15], prereqs=vlan.tci[12] reg0[10] = vlan.pcp[1]; => actions=move:NXM_OF_VLAN_TCI[14]->OXM_OF_PKT_REG0[42], prereqs=vlan.tci[12] outport = inport; => actions=move:NXM_NX_REG6[]->NXM_NX_REG7[], prereqs=1 +reg0 <-> reg1; => actions=push:OXM_OF_PKT_REG0[0..31],push:OXM_OF_PKT_REG0[32..63],pop:OXM_OF_PKT_REG0[0..31],pop:OXM_OF_PKT_REG0[32..63], prereqs=1 +vlan.pcp <-> reg0[0..2]; => actions=push:OXM_OF_PKT_REG0[32..34],push:NXM_OF_VLAN_TCI[13..15],pop:OXM_OF_PKT_REG0[32..34],pop:NXM_OF_VLAN_TCI[13..15], prereqs=vlan.tci[12] +reg0[10] <-> vlan.pcp[1]; => actions=push:NXM_OF_VLAN_TCI[14],push:OXM_OF_PKT_REG0[42],pop:NXM_OF_VLAN_TCI[14],pop:OXM_OF_PKT_REG0[42], prereqs=vlan.tci[12] +outport <-> inport; => actions=push:NXM_NX_REG6[],push:NXM_NX_REG7[],pop:NXM_NX_REG6[],pop:NXM_NX_REG7[], prereqs=1 # Contradictionary prerequisites (allowed but not useful): ip4.src = ip6.src[0..31]; => actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd +ip4.src <-> ip6.src[0..31]; => actions=push:NXM_NX_IPV6_SRC[0..31],push:NXM_OF_IP_SRC[],pop:NXM_NX_IPV6_SRC[0..31],pop:NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd ## Negative tests. @@ -479,6 +484,13 @@ reg0[0] = vlan.present; => Predicate symbol vlan.present cannot be used in assig reg0 = reg1[0..10]; => Can't assign 11-bit value to 32-bit destination. inport = reg0; => Can't assign integer field (reg0) to string field (inport). inport = big_string; => String fields inport and big_string are incompatible for assignment. +ip.proto = reg0[0..7]; => Field ip.proto is not modifiable. +reg0[0] <-> vlan.present; => Predicate symbol vlan.present cannot be used in exchange. +reg0 <-> reg1[0..10]; => Can't exchange 32-bit field with 11-bit field. +inport <-> reg0; => Can't exchange string field (inport) with integer field (reg0). +inport <-> big_string; => String fields inport and big_string are incompatible for exchange. +ip.proto <-> reg0[0..7]; => Field ip.proto is not modifiable. +reg0[0..7] <-> ip.proto; => Field ip.proto is not modifiable. ]]) sed 's/ =>.*//' test-cases.txt > input.txt sed 's/.* => //' test-cases.txt > expout -- 2.1.3 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev