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
