It doesn't seem like a big deal to me one way or another. Anyway, if
it's a problem I'm sure we'll get a report.
I applied this to master and branch-1.10. Thanks for the review.
On Fri, Mar 08, 2013 at 10:20:24AM -0800, Justin Pettit wrote:
> I wonder if it would be better to only print the bridge name when
> there was a mismatch along with a warning. I'm not a heavy user of
> "ofproto/trace", so I don't have a good sense for which way would be
> better.
>
> Looks good, though.
>
> --Justin
>
>
> On Mar 5, 2013, at 4:49 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > If there is more than one bridge, then it's easy to specify the wrong one
> > on an ofproto/trace command. Previously, this would produce surprising
> > results. With this commit, "ofproto/trace" should silently fix up the
> > problem.
> >
> > It would be nice to not require the user to specify a bridge at all, but
> > it's theoretically possible to have more than one backer, in which case we
> > need some way to distinguish, and a bridge name is as good an identifier
> > as we have. We could ask the user to specify the datapath_type, I guess,
> > but that's a less familiar name to most users and it would be a somewhat
> > gratuitous change in synatx for ofproto/trace.
> >
> > Bug #15419.
> > Reported-by: Paul Ingram <p...@nicira.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > ofproto/ofproto-dpif.c | 10 +++++-----
> > 1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 7035530..1087d72 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -7626,16 +7626,16 @@ ofproto_unixctl_trace(struct unixctl_conn *conn,
> > int argc, const char *argv[],
> > goto exit;
> > }
> >
> > - /* XXX: Since we allow the user to specify an ofproto, it's
> > - * possible they will specify a different ofproto than the one
> > the
> > - * port actually belongs too. Ideally we should simply remove
> > the
> > - * ability to specify the ofproto. */
> > + /* The user might have specified the wrong ofproto but within
> > the
> > + * same backer. That's OK, ofproto_receive() can find the
> > right
> > + * one for us. */
> > if (ofproto_receive(ofproto->backer, NULL, odp_key.data,
> > - odp_key.size, &flow, NULL, NULL, NULL,
> > + odp_key.size, &flow, NULL, &ofproto, NULL,
> > &initial_tci)) {
> > unixctl_command_reply_error(conn, "Invalid flow");
> > goto exit;
> > }
> > + ds_put_format(&result, "Bridge: %s\n", ofproto->up.name);
> > } else {
> > char *error_s;
> >
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
