On Thu, Feb 11, 2016 at 12:00:46PM -0800, Jarno Rajahalme wrote:
> With the (few) comments below:
>
> Acked-by: Jarno Rajahalme <ja...@ovn.org>
Thanks. I'm going to post a v4 of this series in a few minutes.
It has major revisions, so only some of these comments are relevant
now. However, important ones are, and I'm answering them below.
> > + struct recirc_state recirc = {
> > + .table_id = 0, /* Not the table where NXAST_PAUSE was
> > executed. */
> > + .ofproto_uuid = closure->bridge,
> > + .stack = closure->stack,
> > + .n_stack = closure->n_stack,
> > + .mirrors = closure->mirrors,
> > + .conntracked = closure->conntracked,
> > +
> > + /* When there are no actions, xlate_actions() will search the flow
> > + * table. We don't want it to do that (we want it to resume), so
> > + * supply a no-op action if there aren't any.
> > + *
> > + * (We can't necessarily avoid translating actions entirely if
> > there
> > + * aren't any actions, because there might be some finishing-up to
> > do
> > + * at the end of the pipeline, and we don't check for those
> > + * conditions.) */
> > + .ofpacts = any_actions ? closure->actions : &noop.ofpact,
> > + .ofpacts_len = any_actions ? closure->actions_len : sizeof noop,
> > +
> > + .action_set = closure->action_set,
> > + .action_set_len = closure->action_set_len,
> > + };
> > + recirc_metadata_from_flow(&recirc.metadata,
> > + &closure->public.metadata.flow);
> > + xin.recirc = &recirc;
>
> Maybe this could be called ‘xin.frozen_state’ now? Also the 'struct
> recirc_state' could be renamed as 'struct frozen_state’.
Yes, those are better names.
I also renamed recirc_metadata to frozen_metadata.
> > +# Check that multiple levels of resubmit continue following resume.
> > +#
> > +# The "resubmit:5", which is relative to the current table, is
>
> Should be “:55"
Thanks, fixed.
> > +# Check that hops across patch ports resume the entire pipeline.
>
> This seems to be in conflict with the comment about pausing at the
> peer bridge above. The pause at the peer bridge should only resume the
> pipeline at the peer bridge (here br1), not the original bridge (here
> br0).
Thanks, I guess I wrote that comment before I actually understood what
was happening. I changed the comment to:
# Check that pause works in the presence of patch ports.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
