On Thu, May 02, 2013 at 06:06:35PM +0900, Simon Horman wrote:
> Do not perform validation of learn actions during parsing.
> I believe this is consistent with the handling of all other actions.
>
> Verification of all actions, including learn actions, occurs separately
> in ofpact_check__().
>
> This the code portion of this patch is larger than might otherwise be
> expected as the flow argument of learn_parse() is now unused and thus
> removed. This propagates up the call-chain some way.
>
> This was suggested by Jesse Gross in response to an enhancement
> I made to the validation performed during parsing learn actions
> to allow it to correctly account for changes to the dl_type
> due to MPLS push and pop actions.
>
> Tests have also been updated to use ovs-ofctl add-flow* instead
> of ovs-ofctl parse-flow to to allow verification occur using
> ofpact_check__().
>
> Cc: Jesse Gross <je...@nicira.com>
> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
The problem with this change is that it makes the error messages
impossible to read. Just look at the learn.at update from your diff.
It changes this error message:
ovs-ofctl: load:5->NXM_OF_IP_DST[]: cannot specify destination field
ip_dst because prerequisites are not satisfied
into this one:
OFPT_ERROR (xid=0x2): OFPBAC_BAD_ARGUMENT
OFPT_FLOW_MOD (xid=0x2):
(***truncated to 64 bytes from 120***)
00000000 01 0e 00 78 00 00 00 02-00 38 20 ff 00 00 00 00 |...x.....8 .....|
00000010 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 |................|
00000020 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 |................|
00000030 00 00 00 00 00 00 00 00-00 00 00 00 00 00 80 00 |................|
I know we'll get complaints about that.
Here's a change to ofp-parse.c that changes the error message back
into:
2013-05-03T16:34:21Z|00001|meta_flow|WARN|destination field ip_dst lacks
correct prerequisites
which is less specific than before but at least hints at the problem.
Can you please work that in?
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index fce0765..c55bf5f 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1009,6 +1009,8 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command,
const char *str_,
str_to_inst_ofpacts(act_str, &ofpacts);
fm->ofpacts_len = ofpacts.size;
fm->ofpacts = ofpbuf_steal_data(&ofpacts);
+
+ ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow, OFPP_MAX);
} else {
fm->ofpacts_len = 0;
fm->ofpacts = NULL;
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
