On Jan 11, 2015, at 1:29 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Jan 09, 2015 at 04:54:42PM -0800, Jarno Rajahalme wrote: >> With the small nits below: >> >> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com> > > Thanks. I fixed up everything you mentioned and applied this to master. > > I gave details below; the only bit where I think you might want followup > is on the treatment of OFPACT_CONJUNCTION in action translation (see > below).
(snip) >>> @@ -4055,6 +4056,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t >>> ofpacts_len, >>> xlate_learn_action(ctx, ofpact_get_LEARN(a)); >>> break; >>> >>> + case OFPACT_CONJUNCTION: >>> + break; >>> + >> >> This should never happen, do you consider OVS_NOT_REACHED() too risky? > > I think it's too risky because not every action execution comes as a > result of a classifier lookup. We don't prevent "conjunction" from > appearing in the actions in a "packet-out", for example. It's useless > there. We could ignore it, or go to some trouble to give some kind of > error. I decided to just ignore it. Maybe we should log it? > I hadn’t thought of the actions in packet out messages. A rate limited log message would be nice, just in case. (snip) >>> +priority=2,ip,ip_src=10.0.0.1,ip_dst=10.0.0.5,actions=4 >>> + >> >> It would be good to also have an identical lower priority rule that is a >> hard match. > > You're right. I added one and thereby found a bug. Incremental > follows: > Cool, sorry for not catching the bug in the code review, though :-) Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
