[prev in list] [next in list] [prev in thread] [next in thread]
List: openvswitch-dev
Subject: [ovs-dev] [PATCH v2 3/3] ovn: Implement action to exchange two fields.
From: blp () nicira ! com (Ben Pfaff)
Date: 2015-09-30 20:56:32
Message-ID: 1443646592-20290-4-git-send-email-blp () nicira ! com
[Download RAW message or body]
Signed-off-by: Ben Pfaff <blp at 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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic