On Fri, Nov 01, 2013 at 03:58:52PM -0700, Jarno Rajahalme wrote:
>
> On Nov 1, 2013, at 9:43 AM, Ben Pfaff <b...@nicira.com> wrote:
>
> > Jarno pointed out that modify_flows__() didn't really need to check every
> > instance of the flow separately. After some further investigation I
> > decided that this was even more of an improvement.
> >
>
> Indeed! Check the actions right after they are decoded rather than
> multiple places afterwards.
>
> One question below, otherwise:
>
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
Thanks!
> > @@ -1700,7 +1701,7 @@ ofpact_check__(const struct ofpact *a, struct flow
> > *flow, ofp_port_t max_ports,
> > case OFPACT_WRITE_ACTIONS: {
> > struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
> > return ofpacts_check(on->actions, ofpact_nest_get_action_len(on),
> > - flow, max_ports, table_id, false);
> > + flow, false, max_ports, table_id, n_tables);
> > }
> >
> > case OFPACT_WRITE_METADATA:
> > @@ -1714,11 +1715,14 @@ ofpact_check__(const struct ofpact *a, struct flow
> > *flow, ofp_port_t max_ports,
> > return 0;
> > }
> >
> > - case OFPACT_GOTO_TABLE:
> > - if (ofpact_get_GOTO_TABLE(a)->table_id <= table_id) {
> > + case OFPACT_GOTO_TABLE: {
> > + uint8_t goto_table = ofpact_get_GOTO_TABLE(a)->table_id;
> > + if ((table_id != 255 && goto_table <= table_id)
>
> Can the table id really be 255 at this point? I just learned that 255
> is not allowed in OF 1.1, and in OF1.2(+) it is allowed only on
> deletes, which should have no actions.
I think there's a bug elsewhere that might allow it to squeak through.
When parse_ofp_str__() parses a flow it can't always tell what version
of OpenFlow is going to be used in advance, so it can provide table ==
255 even with a flow that contains a Goto-Table.
(ofputil_encode_flow_mod() will later squash 255 into 0 for
transmission.) I guess we can or should fix that.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
