From patchwork Wed Sep 30 20:56:31 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Ben Pfaff
X-Patchwork-Id: 524580
Return-Path:
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Received: from archives.nicira.com (li376-54.members.linode.com
[96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id AEB8C1402B6
for ;
Thu, 1 Oct 2015 06:56:54 +1000 (AEST)
Received: from archives.nicira.com (localhost [127.0.0.1])
by archives.nicira.com (Postfix) with ESMTP id B22E322C3C3;
Wed, 30 Sep 2015 13:56:43 -0700 (PDT)
X-Original-To: dev@openvswitch.org
Delivered-To: dev@openvswitch.org
Received: from mx1e4.cudamail.com (mx1.cudamail.com [69.90.118.67])
by archives.nicira.com (Postfix) with ESMTPS id 143AF22C3BE
for ; Wed, 30 Sep 2015 13:56:42 -0700 (PDT)
Received: from bar2.cudamail.com (unknown [192.168.21.12])
by mx1e4.cudamail.com (Postfix) with ESMTPS id 8DE8B1E03D7
for ; Wed, 30 Sep 2015 14:56:41 -0600 (MDT)
X-ASG-Debug-ID: 1443646600-03dc537fe195d710001-byXFYA
Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar2.cudamail.com
with
ESMTP id qED7k2tkXf5N5N3m (version=TLSv1 cipher=DHE-RSA-AES256-SHA
bits=256 verify=NO) for ;
Wed, 30 Sep 2015 14:56:40 -0600 (MDT)
X-Barracuda-Envelope-From: blp@nicira.com
X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2
Received: from unknown (HELO mail-pa0-f54.google.com) (209.85.220.54)
by mx1-pf2.cudamail.com with ESMTPS (RC4-SHA encrypted);
30 Sep 2015 20:56:40 -0000
Received-SPF: unknown (mx1-pf2.cudamail.com: Multiple SPF records returned)
X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.54
Received: by pacfv12 with SMTP id fv12so52004409pac.2
for ; Wed, 30 Sep 2015 13:56:40 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20130820;
h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to
:references;
bh=lX9x54r2wiQL1hxVdaVcMddAlNm6RxZ1/5+dyuIRJSM=;
b=H5Z2y2y5WS6LftH1dunA0U8V+Omt+f75jQcbFBdOsESAZU+PzVxZctMI7r+yhE+SrY
1J2pldmOfNjIBvgbhqho+GaP3m5hPJBk0uOeTVQEB1ZUev+cQ7XMhJkOR68NPMwrmIc8
tZbMxqcpYaZ924e0ci21qtNrvt8XguBAyoeI3yHcTolSOBt6PhRTVzhHR35Yg2i4Ear9
x6pkT7MZeGt6HyKK0Yuj/EwpWAJJS6mvBTBK/ghtz2kIR32kiqNsQ/8tMMmiuCsyu9hg
uGzQo8nYk8uLAcq9cIQ4gndcmkl45ASPm+qRqwbAaAC2py7XFa8xoHmaqaeJFYjjr59l
e+4g==
X-Gm-Message-State:
ALoCoQk28ZOnZpMsqA4a4fKZgJl6qbyShrqAq+FHT5aubTObs1pEE46HUobZpHAJXEbUFnPJi4u7
X-Received: by 10.68.219.169 with SMTP id pp9mr7388439pbc.115.1443646600055;
Wed, 30 Sep 2015 13:56:40 -0700 (PDT)
Received: from sigabrt.benpfaff.org ([208.91.2.4])
by smtp.gmail.com with ESMTPSA id
qy5sm2421744pbb.16.2015.09.30.13.56.38
(version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
Wed, 30 Sep 2015 13:56:38 -0700 (PDT)
X-CudaMail-Envelope-Sender: blp@nicira.com
X-Barracuda-Apparent-Source-IP: 208.91.2.4
From: Ben Pfaff
To: dev@openvswitch.org
X-CudaMail-Whitelist-To: dev@openvswitch.org
X-CudaMail-MID: CM-E2-929096330
X-CudaMail-DTE: 093015
X-CudaMail-Originating-IP: 209.85.220.54
Date: Wed, 30 Sep 2015 13:56:31 -0700
X-ASG-Orig-Subj: [##CM-E2-929096330##][PATCH v2 2/3] expr: Implement action
to copy one field into another.
Message-Id: <1443646592-20290-3-git-send-email-blp@nicira.com>
X-Mailer: git-send-email 2.1.3
In-Reply-To: <1443646592-20290-1-git-send-email-blp@nicira.com>
References: <1443646592-20290-1-git-send-email-blp@nicira.com>
X-Barracuda-Connect: UNKNOWN[192.168.24.2]
X-Barracuda-Start-Time: 1443646600
X-Barracuda-Encrypted: DHE-RSA-AES256-SHA
X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi
X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?=
X-Virus-Scanned: by bsmtpd at cudamail.com
X-Barracuda-BRTS-Status: 1
Cc: Ben Pfaff
Subject: [ovs-dev] [PATCH v2 2/3] expr: Implement action to copy one field
into another.
X-BeenThere: dev@openvswitch.org
X-Mailman-Version: 2.1.16
Precedence: list
List-Id:
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
MIME-Version: 1.0
Errors-To: dev-bounces@openvswitch.org
Sender: "dev"
Signed-off-by: Ben Pfaff
Acked-by: Justin Pettit
---
ovn/TODO | 4 +-
ovn/lib/expr.c | 186 +++++++++++++++++++++++++++++++++++--------------------
ovn/ovn-sb.xml | 35 ++++++++---
tests/ovn.at | 14 ++++-
tests/test-ovn.c | 1 +
5 files changed, 159 insertions(+), 81 deletions(-)
diff --git a/ovn/TODO b/ovn/TODO
index c83a287..740ea17 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -132,8 +132,8 @@ the "arp" action, and an action for generating
*** Other actions.
-Possibly we'll need to implement "field1 = field2;" for copying
-between fields and "field1 <-> field2;" for swapping fields.
+Possibly we'll need to implement "field1 <-> field2;" for swapping
+fields.
*** ovn-controller translation to OpenFlow
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 71d817b..b7a3f7f 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2619,113 +2619,165 @@ expr_is_normalized(const struct expr *expr)
/* Action parsing helper. */
-static struct expr *
-parse_assignment(struct expr_context *ctx, const struct simap *ports,
- struct ofpbuf *ofpacts)
+static bool
+expand_symbol(struct expr_context *ctx, struct expr_field *f,
+ struct expr **prereqsp)
{
- struct expr *prereqs = NULL;
-
- struct expr_field f;
- if (!parse_field(ctx, &f)) {
- goto exit;
- }
- if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) {
- expr_syntax_error(ctx, "expecting `='.");
- goto exit;
- }
-
- if (f.symbol->expansion && f.symbol->level != EXPR_L_ORDINAL) {
- expr_error(ctx, "Can't assign to predicate symbol %s.",
- f.symbol->name);
- goto exit;
- }
-
- struct expr_constant_set cs;
- if (!parse_constant_set(ctx, &cs)) {
- goto exit;
- }
-
- if (!type_check(ctx, &f, &cs)) {
- goto exit_destroy_cs;
- }
- if (cs.in_curlies) {
- expr_error(ctx, "Assignments require a single value.");
- goto exit_destroy_cs;
+ if (f->symbol->expansion && f->symbol->level != EXPR_L_ORDINAL) {
+ expr_error(ctx, "Predicate symbol %s cannot be used in assignment.",
+ f->symbol->name);
+ return false;
}
- const struct expr_symbol *orig_symbol = f.symbol;
- union expr_constant *c = cs.values;
for (;;) {
/* Accumulate prerequisites. */
- if (f.symbol->prereqs) {
+ if (f->symbol->prereqs) {
struct ovs_list nesting = OVS_LIST_INITIALIZER(&nesting);
char *error;
struct expr *e;
- e = parse_and_annotate(f.symbol->prereqs, ctx->symtab, &nesting,
+ e = parse_and_annotate(f->symbol->prereqs, ctx->symtab, &nesting,
&error);
if (error) {
expr_error(ctx, "%s", error);
free(error);
- goto exit_destroy_cs;
+ return false;
}
- prereqs = expr_combine(EXPR_T_AND, prereqs, e);
+ *prereqsp = expr_combine(EXPR_T_AND, *prereqsp, e);
}
/* If there's no expansion, we're done. */
- if (!f.symbol->expansion) {
+ if (!f->symbol->expansion) {
break;
}
/* Expand. */
struct expr_field expansion;
char *error;
- if (!parse_field_from_string(f.symbol->expansion, ctx->symtab,
+ if (!parse_field_from_string(f->symbol->expansion, ctx->symtab,
&expansion, &error)) {
expr_error(ctx, "%s", error);
free(error);
- goto exit_destroy_cs;
+ return false;
}
- f.symbol = expansion.symbol;
- f.ofs += expansion.ofs;
+ f->symbol = expansion.symbol;
+ f->ofs += expansion.ofs;
}
- if (!f.symbol->field->writable) {
- expr_error(ctx, "Field %s is not modifiable.", orig_symbol->name);
- goto exit_destroy_cs;
+ return true;
+}
+
+static struct expr *
+parse_assignment(struct expr_context *ctx, const struct simap *ports,
+ struct ofpbuf *ofpacts)
+{
+ struct expr *prereqs = NULL;
+
+ /* Parse destination and do basic checking. */
+ struct expr_field dst;
+ if (!parse_field(ctx, &dst)) {
+ goto exit;
+ }
+ if (!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);
+ goto exit;
}
- struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
- sf->field = f.symbol->field;
- if (f.symbol->width) {
- mf_subvalue_shift(&c->value, f.ofs);
- if (!c->masked) {
- memset(&c->mask, 0, sizeof c->mask);
- bitwise_one(&c->mask, sizeof c->mask, f.ofs, f.n_bits);
- } else {
- mf_subvalue_shift(&c->mask, f.ofs);
+ if (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)) {
+ 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);
+ 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);
+ 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);
+ goto exit;
}
- memcpy(&sf->value, &c->value.u8[sizeof c->value - sf->field->n_bytes],
- sf->field->n_bytes);
- memcpy(&sf->mask, &c->mask.u8[sizeof c->mask - sf->field->n_bytes],
- sf->field->n_bytes);
+ 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;
} else {
- uint32_t port = simap_get(ports, c->string);
- bitwise_put(port, &sf->value,
- sf->field->n_bytes, 0, sf->field->n_bits);
- bitwise_put(UINT64_MAX, &sf->mask,
- sf->field->n_bytes, 0, sf->field->n_bits);
+ struct expr_constant_set cs;
+ if (!parse_constant_set(ctx, &cs)) {
+ goto exit;
+ }
+
+ if (!type_check(ctx, &dst, &cs)) {
+ goto exit_destroy_cs;
+ }
+ if (cs.in_curlies) {
+ expr_error(ctx, "Assignments require a single value.");
+ goto exit_destroy_cs;
+ }
+
+ union expr_constant *c = cs.values;
+ struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts);
+ sf->field = dst.symbol->field;
+ if (dst.symbol->width) {
+ mf_subvalue_shift(&c->value, dst.ofs);
+ if (!c->masked) {
+ memset(&c->mask, 0, sizeof c->mask);
+ bitwise_one(&c->mask, sizeof c->mask, dst.ofs, dst.n_bits);
+ } else {
+ mf_subvalue_shift(&c->mask, dst.ofs);
+ }
+
+ memcpy(&sf->value,
+ &c->value.u8[sizeof c->value - sf->field->n_bytes],
+ sf->field->n_bytes);
+ memcpy(&sf->mask,
+ &c->mask.u8[sizeof c->mask - sf->field->n_bytes],
+ sf->field->n_bytes);
+ } else {
+ uint32_t port = simap_get(ports, c->string);
+ bitwise_put(port, &sf->value,
+ sf->field->n_bytes, 0, sf->field->n_bits);
+ bitwise_put(UINT64_MAX, &sf->mask,
+ sf->field->n_bytes, 0, sf->field->n_bits);
+ }
+
+ exit_destroy_cs:
+ expr_constant_set_destroy(&cs);
}
-exit_destroy_cs:
- expr_constant_set_destroy(&cs);
exit:
return prereqs;
}
/* A helper for actions_parse(), to parse an OVN assignment action in the form
- * "field = value" into 'ofpacts'. The parameters and return value match those
- * for actions_parse(). */
+ * "field = value" or "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/ovn-sb.xml b/ovn/ovn-sb.xml
index da14fe4..28b0d2c 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -800,26 +800,41 @@
pipeline.
-
-
- The following actions will likely be useful later, but they have not
- been thought out carefully.
-
-
-
field1 = field2;
-
- Extends the assignment action to allow copying between fields.
+ Sets data or metadata field field1 to the value of data
+ or metadata field field2, e.g. reg0 =
+ ip4.src;
to copy ip4.src
into
+ reg0
. To modify only a subset of a field's bits,
+ specify a subfield for field1 or field2 or
+ both, e.g. vlan.pcp = reg0[0..2];
to set the VLAN PCP
+ from the least-significant bits of reg0
.
- An assignment adds prerequisites from the source and the
- destination fields.
+ field1 and field2 must be the same type,
+ either both string or both integer fields. If they are both
+ integer fields, they must have the same width.
+
+
+
+ If field1 or field2 has prerequisites, they
+ are added implicitly to . It is possible to
+ write an assignment with contradictory prerequisites, such as
+ ip4.src = ip6.src[0..31];
, but the contradiction means
+ that a logical flow with such an assignment will never be matched.
+
+
+
+ The following actions will likely be useful later, but they have not
+ been thought out carefully.
+
+
ip.ttl--;
-
diff --git a/tests/ovn.at b/tests/ovn.at
index 70fc086..96f190d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -444,6 +444,12 @@ tcp.dst=80; => actions=set_field:80->tcp_dst, prereqs=ip.proto == 0x6 && (eth.ty
eth.dst[40] = 1; => actions=set_field:01:00:00:00:00:00/01:00:00:00:00:00->eth_dst, prereqs=1
vlan.pcp = 2; => actions=set_field:0x4000/0xe000->vlan_tci, prereqs=vlan.tci[12]
vlan.tci[13..15] = 2; => actions=set_field:0x4000/0xe000->vlan_tci, prereqs=1
+reg0 = reg1; => actions=move:OXM_OF_PKT_REG0[0..31]->OXM_OF_PKT_REG0[32..63], prereqs=1
+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
+# 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
## Negative tests.
@@ -462,13 +468,17 @@ next => Syntax error at end of input expecting ';'.
inport[1] = 1; => Cannot select subfield of string field inport.
ip.proto[1] = 1; => Cannot select subfield of nominal field ip.proto.
eth.dst[40] == 1; => Syntax error at `==' expecting `='.
-ip = 1; => Can't assign to predicate symbol ip.
+ip = 1; => Predicate symbol ip cannot be used in assignment.
ip.proto = 6; => Field ip.proto is not modifiable.
inport = {"a", "b"}; => Assignments require a single value.
inport = {}; => Syntax error at `}' expecting constant.
bad_prereq = 123; => Error parsing expression `xyzzy' encountered as prerequisite or predicate of initial expression: Syntax error at `xyzzy' expecting field name.
self_recurse = 123; => Error parsing expression `self_recurse != 0' encountered as prerequisite or predicate of initial expression: Error parsing expression `self_recurse != 0' encountered as prerequisite or predicate of initial expression: Recursive expansion of symbol `self_recurse'.
-vlan.present = 0; => Can't assign to predicate symbol vlan.present.
+vlan.present = 0; => Predicate symbol vlan.present cannot be used in assignment.
+reg0[0] = vlan.present; => Predicate symbol vlan.present cannot be used in assignment.
+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.
]])
sed 's/ =>.*//' test-cases.txt > input.txt
sed 's/.* => //' test-cases.txt > expout
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index ecb3b5c..c806068 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -235,6 +235,7 @@ create_symtab(struct shash *symtab)
"mutual_recurse_2 != 0", false);
expr_symtab_add_field(symtab, "mutual_recurse_2", MFF_XREG0,
"mutual_recurse_1 != 0", false);
+ expr_symtab_add_string(symtab, "big_string", MFF_XREG0, NULL);
}
static void