Commit 7eb4b1f1d "ofp-actions: Support OF1.5 (draft) masked Set-Field,
merge with reg_load." purportedly merged the set_field and reg_load
fields, including the commandline parsing to use the metafield syntax
like "load:192.168.0.1->OXM_OF_IPV4_SRC". However, the actual reg_load
parsing code was not updated.
This patch takes advantage of the newly refactored parse_set_field()
helpers to make parse_reg_load() conform to the documentation, which
allows more thorough validation of inputs, and allows reg_load to parse
fields that are wider than 64 bits, as well as masked fields.
Note that this disallows syntax that was previously allowed, such as:
load:0x0a000001->OXM_OF_IPV4_SRC
Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
Note that v2.4.0 includes the aforementioned commit; we should consider
either backporting this fix to branch-2.4, or reverting the
documentation modification on branch-2.4 to be consistent with the
previous syntax.
---
NEWS | 3 +++
lib/ofp-actions.c | 57 +++++++++++++++++++++++++++++++++++++--------------
tests/ofproto-dpif.at | 6 +++---
3 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/NEWS b/NEWS
index ca22c8e..ef0348b 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,9 @@ Post-v2.4.0
targets to run a new system testsuite. These tests can be run inside
a Vagrant box. See INSTALL.md for details
- Dropped support for GRE64 tunnel.
+ - ovs-ofctl syntax for the "reg_load" action now validates inputs to
+ ensure they are of the format that the field accepts. Simply loading
+ hexadecimal values into any field will no longer work.
v2.4.0 - 20 Aug 2015
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index d33e429..153c712 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -2571,36 +2571,63 @@ error:
}
static char * OVS_WARN_UNUSED_RESULT
-parse_reg_load(char *arg, struct ofpbuf *ofpacts)
+parse_reg_load__(char *arg, struct ofpbuf *ofpacts)
{
struct ofpact_set_field *sf = ofpact_put_reg_load(ofpacts);
- const char *full_arg = arg;
- uint64_t value = strtoull(arg, (char **) &arg, 0);
+ union mf_value immediate, mask;
+ const struct mf_field *mf;
+ char *key, *value, *delim;
struct mf_subfield dst;
- char *error;
+ char *error = NULL;
- if (strncmp(arg, "->", 2)) {
- return xasprintf("%s: missing `->' following value", full_arg);
+ error = set_field_split_str(arg, &key, &value, &delim);
+ if (error) {
+ return error;
}
- arg += 2;
- error = mf_parse_subfield(&dst, arg);
+
+ error = mf_parse_subfield(&dst, key);
if (error) {
return error;
}
+ delim[0] = '\0';
+ mf = dst.field;
- if (dst.n_bits < 64 && (value >> dst.n_bits) != 0) {
- return xasprintf("%s: value %"PRIu64" does not fit into %d bits",
- full_arg, value, dst.n_bits);
+ error = set_field_parse__(mf, key, value, &immediate, &mask);
+ if (error) {
+ return error;
}
- sf->field = dst.field;
+ if (!bitwise_is_all_zeros(&immediate, mf->n_bytes, dst.n_bits,
+ mf->n_bytes * 8 - dst.n_bits)) {
+ struct ds ds;
+
+ ds_init(&ds);
+ mf_format(mf, &immediate, &mask, &ds);
+ error = xasprintf("%s: value %s does not fit into %d bits",
+ arg, ds_cstr(&ds), dst.n_bits);
+ ds_destroy(&ds);
+ return error;
+ }
+
+ sf->field = mf;
memset(&sf->value, 0, sizeof sf->value);
- bitwise_put(value, &sf->value, dst.field->n_bytes, dst.ofs, dst.n_bits);
- bitwise_put(UINT64_MAX, &sf->mask,
- dst.field->n_bytes, dst.ofs, dst.n_bits);
+ bitwise_copy(&immediate, mf->n_bytes, 0, &sf->value, mf->n_bytes,
+ dst.ofs, dst.n_bits);
+ bitwise_copy(&mask, mf->n_bytes, 0, &sf->mask, mf->n_bytes,
+ dst.ofs, dst.n_bits);
+
return NULL;
}
+static char * OVS_WARN_UNUSED_RESULT
+parse_reg_load(char *arg, struct ofpbuf *ofpacts)
+{
+ char *copy = xstrdup(arg);
+ char *error = parse_reg_load__(copy, ofpacts);
+ free(copy);
+ return error;
+}
+
static void
format_SET_FIELD(const struct ofpact_set_field *a, struct ds *s)
{
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9609d2d..c211a50 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1292,7 +1292,7 @@ cookie=0x7 table=5 in_port=84
actions=load:5->NXM_NX_REG4[[]],load:6->NXM_NX_TUN
cookie=0x8 table=6 in_port=85 actions=mod_tp_src:85,controller,resubmit(86,7)
cookie=0x9 table=7 in_port=86 actions=mod_tp_dst:86,controller,controller
cookie=0xa dl_src=40:44:44:44:44:41
actions=mod_vlan_vid:99,mod_vlan_pcp:1,controller
-cookie=0xd dl_src=80:88:88:88:88:88 arp
actions=load:2->OXM_OF_ARP_OP[[]],controller,load:0xc0a88001->OXM_OF_ARP_SPA[[]],controller,load:0x404444444441->OXM_OF_ARP_THA[[]],load:0x01010101->OXM_OF_ARP_SPA[[]],load:0x02020202->OXM_OF_ARP_TPA[[]],controller
+cookie=0xd dl_src=80:88:88:88:88:88 arp
actions=load:2->OXM_OF_ARP_OP[[]],controller,load:192.168.128.1->OXM_OF_ARP_SPA[[]],controller,load:40:44:44:44:44:41->OXM_OF_ARP_THA[[]],load:1.1.1.1->OXM_OF_ARP_SPA[[]],load:2.2.2.2->OXM_OF_ARP_TPA[[]],controller
])
AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
@@ -1564,7 +1564,7 @@ cookie=0xd dl_src=60:66:66:66:66:66
actions=pop_mpls:0x0800,controller
cookie=0xc dl_src=70:77:77:77:77:77
actions=push_mpls:0x8848,load:1000->OXM_OF_MPLS_LABEL[[]],load:7->OXM_OF_MPLS_TC[[]],controller
cookie=0xd dl_src=60:66:66:66:00:01 actions=pop_mpls:0x0800,dec_ttl,controller
-cookie=0xd dl_src=60:66:66:66:00:02
actions=pop_mpls:0x0800,load:0xa000001->OXM_OF_IPV4_DST[[]],controller
+cookie=0xd dl_src=60:66:66:66:00:02
actions=pop_mpls:0x0800,load:10.0.0.1->OXM_OF_IPV4_DST[[]],controller
cookie=0xd dl_src=60:66:66:66:00:03
actions=pop_mpls:0x0800,move:OXM_OF_IPV4_DST[[]]->OXM_OF_IPV4_SRC[[]],controller
cookie=0xd dl_src=60:66:66:66:00:04
actions=pop_mpls:0x0800,push:OXM_OF_IPV4_DST[[]],pop:OXM_OF_IPV4_SRC[[]],controller
cookie=0xd dl_src=60:66:66:66:00:05
actions=pop_mpls:0x0800,multipath(eth_src,50,modulo_n,256,0,OXM_OF_IPV4_SRC[[0..7]]),controller
@@ -3020,7 +3020,7 @@ OVS_VSWITCHD_START
ADD_OF_PORTS([br0], [1], [2])
ovs-vsctl -- set Interface p2 type=dummy options:pcap=p2.pcap
-ovs-ofctl add-flow br0 'in_port=1,arp
actions=load:2->OXM_OF_ARP_OP[[]],2,load:0xc0a88001->OXM_OF_ARP_SPA[[]],2,load:0x404444444441->OXM_OF_ARP_THA[[]],2'
+ovs-ofctl add-flow br0 'in_port=1,arp
actions=load:2->OXM_OF_ARP_OP[[]],2,load:192.168.128.1->OXM_OF_ARP_SPA[[]],2,load:40:44:44:44:44:41->OXM_OF_ARP_THA[[]],2'
# Input some packets that should follow the arp modification slow-path.
for i in 1 2 3; do
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
