On Tue, Nov 05, 2013 at 03:27:16PM -0800, Gurucharan Shetty wrote: > On Wed, Oct 30, 2013 at 2:13 PM, Ben Pfaff <b...@nicira.com> wrote: > > Feature #20543. > > Requested-by: Ronghua Zhang <rzh...@vmware.com> > > Signed-off-by: Ben Pfaff <b...@nicira.com> > > --- > > ofproto/ofproto-dpif.c | 123 > > ++++++++++++++++++++++++++++++++++++++----- > > ofproto/ofproto-unixctl.man | 57 +++++++++++++++----- > > tests/ofproto-dpif.at | 19 +++++++ > > 3 files changed, 172 insertions(+), 27 deletions(-) > > > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > > index d735507..c79570e 100644 > > --- a/ofproto/ofproto-dpif.c > > +++ b/ofproto/ofproto-dpif.c > > @@ -515,7 +515,9 @@ ofproto_dpif_cast(const struct ofproto *ofproto) > > static struct ofport_dpif *get_ofp_port(const struct ofproto_dpif *ofproto, > > ofp_port_t ofp_port); > > static void ofproto_trace(struct ofproto_dpif *, const struct flow *, > > - const struct ofpbuf *packet, struct ds *); > > + const struct ofpbuf *packet, > > + const struct ofpact[], size_t ofpacts_len, > > + struct ds *); > I think the above change in ofproto_trace definition should be in patch 1.
I think it's actually in the correct place. This change isn't needed until ofproto/trace-packet-out is implemented. Patch 1 did have a misplaced change to call the new form of ofproto_trace() added in this patch (you pointed it out on patch 1), so I moved that change to this patch. > > + retval = ofpacts_check(ofpacts.data, ofpacts.size, &flow, > > + u16_to_ofp(ofproto->up.max_ports), 0, > > + enforce_consistency); > This patch needs a rebase as ofpacts_check interface has changed by a > later commit. Yes, thanks, I fixed it up here. > Otherrwise, This looks good to me. Thanks. > > +static void > > ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow, > > - const struct ofpbuf *packet, struct ds *ds) > > + const struct ofpbuf *packet, > > + const struct ofpact ofpacts[], size_t ofpacts_len, > > + struct ds *ds) > It will be nice if this function has a comment at the top. Done, I added: /* Implements a "trace" through 'ofproto''s flow table, appending a textual * description of the results to 'ds'. * * The trace follows a packet with the specified 'flow' through the flow * table. 'packet' may be nonnull to trace an actual packet, with consequent * side effects (if it is nonnull then its flow must be 'flow'). * * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to * trace, otherwise the actions are determined by a flow table lookup. */ _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
