Re: [ovs-dev] [PATCH] ofp-parse: Fix match parsing with [x..y]=z format.

2017-04-17 Thread Jarno Rajahalme

> On Apr 14, 2017, at 8:58 PM, Ben Pfaff  wrote:
> 
> On Thu, Apr 13, 2017 at 06:31:06PM -0700, Jarno Rajahalme wrote:
>> Commit 21b2fa617126 ("ofp-parse: Allow match field names in actions
>> and brackets in matches.") added support for matching a consecutive
>> set of bits with the [x..y]=z format, but the copying of the parsed
>> value ('z') to the match was done from a wrong offset, so that the
>> actual value matched would be incorrect.
>> 
>> Fix this and add a test case preventing regression in future.
>> 
>> Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and 
>> brackets in matches.")
>> Signed-off-by: Jarno Rajahalme 
> 
> Oops, thanks for the fix!
> 
> Should the test try a multibit match too, for completeness?
> 

Pushed to master and branch-2.7 with an additional multibit match. Thanks for 
the review!

  Jarno

> Acked-by: Ben Pfaff 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofp-parse: Fix match parsing with [x..y]=z format.

2017-04-14 Thread Ben Pfaff
On Thu, Apr 13, 2017 at 06:31:06PM -0700, Jarno Rajahalme wrote:
> Commit 21b2fa617126 ("ofp-parse: Allow match field names in actions
> and brackets in matches.") added support for matching a consecutive
> set of bits with the [x..y]=z format, but the copying of the parsed
> value ('z') to the match was done from a wrong offset, so that the
> actual value matched would be incorrect.
> 
> Fix this and add a test case preventing regression in future.
> 
> Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and 
> brackets in matches.")
> Signed-off-by: Jarno Rajahalme 

Oops, thanks for the fix!

Should the test try a multibit match too, for completeness?

Acked-by: Ben Pfaff 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofp-parse: Fix match parsing with [x..y]=z format.

2017-04-13 Thread Jarno Rajahalme
Commit 21b2fa617126 ("ofp-parse: Allow match field names in actions
and brackets in matches.") added support for matching a consecutive
set of bits with the [x..y]=z format, but the copying of the parsed
value ('z') to the match was done from a wrong offset, so that the
actual value matched would be incorrect.

Fix this and add a test case preventing regression in future.

Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and 
brackets in matches.")
Signed-off-by: Jarno Rajahalme 
---
 lib/ofp-parse.c| 6 +++---
 tests/ovs-ofctl.at | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 7826bc5..c8cac5b 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -290,11 +290,11 @@ parse_subfield(const char *name, const char *str_value, 
struct match *match,
 
 const struct mf_field *field = sf.field;
 union mf_value value, mask;
-unsigned int size = DIV_ROUND_UP(sf.n_bits, 8);
+unsigned int size = field->n_bytes;
 
 mf_get(field, match, &value, &mask);
-bitwise_copy(&val, size, 0, &value, field->n_bytes, sf.ofs, sf.n_bits);
-bitwise_one (   &mask,  field->n_bytes, sf.ofs, sf.n_bits);
+bitwise_copy(&val, size, 0, &value, size, sf.ofs, sf.n_bits);
+bitwise_one (   &mask,  size, sf.ofs, sf.n_bits);
 *usable_protocols &= mf_set(field, &value, &mask, match, &error);
 }
 return error;
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 737f609..18ab788 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -285,6 +285,7 @@ AT_CLEANUP
 AT_SETUP([ovs-ofctl parse-flows (OpenFlow 1.2)])
 AT_DATA([flows.txt], [[
 # comment
+tcp,tp_src[5]=1,actions=flood
 tcp,tp_src=123,actions=flood
 in_port=LOCAL dl_vlan=9 dl_src=00:0A:E4:25:6B:B0 
actions=mod_vlan_vid:7,mod_vlan_pcp:2
 udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0
@@ -309,6 +310,7 @@ AT_CHECK([ovs-ofctl --protocols OpenFlow12 parse-flows 
flows.txt
 AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0],
 [[usable protocols: NXM,OXM
 chosen protocol: OXM-OpenFlow12
+OFPT_FLOW_MOD (OF1.2): ADD tcp,tp_src=0x20/0x20 actions=FLOOD
 OFPT_FLOW_MOD (OF1.2): ADD tcp,tp_src=123 actions=FLOOD
 OFPT_FLOW_MOD (OF1.2): ADD in_port=LOCAL,dl_vlan=9,dl_src=00:0a:e4:25:6b:b0 
actions=set_field:4103->vlan_vid,set_field:2->vlan_pcp
 OFPT_FLOW_MOD (OF1.2): ADD udp,dl_vlan_pcp=7 idle:5 actions=pop_vlan,output:0
-- 
2.1.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev