[ovs-dev] [actions 3/5] ofp-util: Simplify iteration through OpenFlow actions.

Ben Pfaff blp at nicira.com
Thu Jun 30 17:03:58 UTC 2011


On Tue, Jun 28, 2011 at 01:31:42PM -0700, Andrew Evans wrote:
> On Fri, 2011-06-24 at 15:02 -0700, Ben Pfaff wrote:
> > The existing actions_first() and actions_next() iterator functions are not
> > much like the other iteration constructs found throughout the Open vSwitch
> > tree.  Also, they only work with actions that have already been validated,
> > so there are cases where they cannot be used.
> > 
> > This commit adds new macros for iterating through OpenFlow actions, one
> > for actions that have been validated and one for actions that have not, and
> > adapts the existing users.  The following commit will further refine action
> > parsing and add more users.
> 
> Thanks, this looks good. I only have one minor comment:
>  
> > @@ -527,7 +501,11 @@ ofp_print_packet_out(struct ds *string, const struct ofp_packet_out *opo,
> >          ds_put_format(string, "***packet too short for action length***\n");
> >          return;
> >      }
> > -    ofp_print_actions(string, opo->actions, actions_len);
> > +    if (actions_len % sizeof(union ofp_action)) {
> > +        ds_put_format(string, "***action length not a multiple of 8***\n");
> 
> Can we replace "8" with "%zu", sizeof(union ofp_action) here?

OK, done.

> Looks good to me otherwise. I can't say I closely scrutinized the use of
> safe vs. unsafe iteration macros, though.

Thanks for looking it over.



More information about the dev mailing list