On Thu, May 23, 2013 at 11:10:37AM -0700, Ben Pfaff wrote:
> On Wed, May 22, 2013 at 04:08:10PM +0900, Simon Horman wrote:
> > diff --git a/lib/execute-actions.c b/lib/execute-actions.c
> > index 5e05dc5..1036ede 100644
> > --- a/lib/execute-actions.c
> > +++ b/lib/execute-actions.c
> > @@ -170,7 +170,8 @@ execute_actions(void *dp, struct ofpbuf *packet, struct
> > flow *key,
> >
> > switch ((enum ovs_action_attr) type) {
> > case OVS_ACTION_ATTR_OUTPUT:
> > - output(dp, packet, nl_attr_get_u32(a));
> > + if (output)
> > + output(dp, packet, nl_attr_get_u32(a));
>
> Needs {}.
Thanks, fixed.
> > + ovs_assert(!execute_actions(NULL, packet, &key, ctx->odp_actions->data,
> > + ctx->odp_actions->size, NULL, NULL));
>
> Please don't put required side effects in the condition for
> ovs_assert(). (Even though there's no way to turn off ovs_assert()
> yet, I don't want to rule that out for the future.)
Thanks, I have removed the assert as I'm not entirely convinced of its value.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
