On Wed, May 8, 2013 at 10:18 AM, Ben Pfaff <b...@nicira.com> wrote: > Your return email address has a space in it. It appears as "Alex Wang > <al...@nicira.com >" (notice the space between ".com" and ">". I > guess there may be a typo in your .gitconfig? > >
Yes, that is a typo. I fixed it. Thx for pointing out. > On Mon, May 06, 2013 at 05:30:51PM -0700, Alex Wang <al...@nicira.com > > wrote: > > Subject: [ovs-dev] [PATCH 1/2] ofproto: Re-implement the ofproto/trace > > command, fix broken tests and man page > > You don't need to mention that you fixed broken tests or the manpage > in the subject, because it goes without saying. > > I see, thanks for the reminder. > > Therein, the bridge name is replaced by datapath name in the first > format, > > since the mapping between in_port and the corresponding datapath is > unique. > > The second format requires datapath name since it uses 'ovs-dpctl > dump-flows' > > output. And the thrid format requires bridge name since it uses > 'ovs-ofctl > > "thrid" =>" "third" > > Fixed this typo. > > Signed-off-by: Alex Wang <al...@nicira.com> > > "git am" reports some whitespace errors: > > /home/blp/ovs/.git/rebase-apply/patch:274: trailing whitespace. > Traces the path of an imaginary packet through \fiswitch\fR. The first > /home/blp/ovs/.git/rebase-apply/patch:275: trailing whitespace. > two forms require an optional \fIdatapath\fR argument, which is the > datapath > /home/blp/ovs/.git/rebase-apply/patch:276: trailing whitespace. > the \fiswitch\fR is on (can be found by \fBdpif/show\fR command). If > more > /home/blp/ovs/.git/rebase-apply/patch:277: trailing whitespace. > than one datapath exists, the \fIdatapath\fR argument is compulsory. > /home/blp/ovs/.git/rebase-apply/patch:278: trailing whitespace. > The third form requires a \fIswitch\fR argument (one of those listed by > warning: squelched 10 whitespace errors > warning: 15 lines add whitespace errors. > > I'll prevent this from happening. Forgot to run delete-trailing-whitespace. > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > > index 6ec1c23..6550af3 100644 > > --- a/ofproto/ofproto-dpif.c > > +++ b/ofproto/ofproto-dpif.c > > @@ -6073,7 +6073,7 @@ fix_sflow_action(struct action_xlate_ctx *ctx) > > } > > > > cookie = ofpbuf_at(ctx->odp_actions, ctx->user_cookie_offset, > > - sizeof cookie->sflow); > > + sizeof(*cookie)); > > The above appears to revert the bug fix from commit a93f1927f21f2 > (ofproto-dpif: Fix 'size' argument to fix_sflow_action().). Why does > it do that? > > Seemed that I haven't done 'git pull' to update my repo. Will prevent from happening. > The rest of the patch looks like it implements what we asked you to > do. Thank you. But, in retrospect, I am not sure that we asked you > to do something that makes much sense. The high-level idea was that > ofproto/trace should be easier to understand and to use, presumably by > having a more uniform interface, but I think that the changes we asked > you to make actually resulted in code and documentation that is harder > to understand. > > I think that it would be better to step back a little and reconsider > the syntax, so I did that. Here is a patch against the unmodified > "master" version of ofproto-unixctl.man that gives a revised > specification for ofproto/trace that appears, to me, to be easier to > understand and more uniform. If you agree, then would you please use > this patch instead for the documentation, and then update the code to > match this documentation? And I am of course happy to iterate on the > documentation in advance of updating the code, so that we end up with > something better. > > Thanks, > > Ben. > > diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man > index 8890343..de19281 100644 > *** a/ofproto/ofproto-unixctl.man > --- b/ofproto/ofproto-unixctl.man > *************** > *** 6,75 **** > Lists the names of the running ofproto instances. These are the names > that may be used on \fBofproto/trace\fR. > Thanks for the patch. I'd very much like to reconsider the design. Will study it and communicate with you more.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
