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