[ovs-dev] [PATCH] ovs-vswitchd: Log datapath ID in a more user-friendly way.

[Justin Pettit]

Looks good to me.

--Justin


On Jun 26, 2012, at 2:43 PM, Ben Pfaff wrote:

> The layering between ofproto and ovs-vswitchd caused the datapath ID to be
> logged in a needlessly confusing way.  First, ofproto would log its
> default datapath ID:
> 
>     using datapath ID 0000505400000004
> 
> then the bridge code would immediately determine the datapath ID that it
> wanted and call ofproto_set_datapath_id(), which would log the change
> 
>     datapath ID changed to 0000111122223333
> 
> This commit stops logging the default datapath ID, which is never actually
> visible in OpenFlow.  This should make the log files easier to understand.
> 
> Bug #12164.
> Reported-by: Jacob Cherkas <jcherkas at nicira.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> ofproto/ofproto.c |   11 ++++++-----
> ofproto/ofproto.h |    1 +
> vswitchd/bridge.c |    5 ++++-
> 3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 68bd485..dfc8db4 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -404,8 +404,6 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
>     assert(ofproto->n_tables);
> 
>     ofproto->datapath_id = pick_datapath_id(ofproto);
> -    VLOG_INFO("%s: using datapath ID %016"PRIx64,
> -              ofproto->name, ofproto->datapath_id);
>     init_ports(ofproto);
> 
>     *ofprotop = ofproto;
> @@ -427,15 +425,18 @@ ofproto_init_tables(struct ofproto *ofproto, int n_tables)
>     }
> }
> 
> +uint64_t
> +ofproto_get_datapath_id(const struct ofproto *ofproto)
> +{
> +    return ofproto->datapath_id;
> +}
> +
> void
> ofproto_set_datapath_id(struct ofproto *p, uint64_t datapath_id)
> {
>     uint64_t old_dpid = p->datapath_id;
>     p->datapath_id = datapath_id ? datapath_id : pick_datapath_id(p);
>     if (p->datapath_id != old_dpid) {
> -        VLOG_INFO("%s: datapath ID changed to %016"PRIx64,
> -                  p->name, p->datapath_id);
> -
>         /* Force all active connections to reconnect, since there is no way to
>          * notify a controller that the datapath ID has changed. */
>         ofproto_reconnect_controllers(p);
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index ea988e7..0919d81 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -201,6 +201,7 @@ int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
>                                struct ofproto_port *);
> 
> /* Top-level configuration. */
> +uint64_t ofproto_get_datapath_id(const struct ofproto *);
> void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
> void ofproto_set_controllers(struct ofproto *,
>                              const struct ofproto_controller *, size_t n);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 67e29d2..3a3e58b 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -754,7 +754,10 @@ bridge_configure_datapath_id(struct bridge *br)
>     memcpy(br->ea, ea, ETH_ADDR_LEN);
> 
>     dpid = bridge_pick_datapath_id(br, ea, hw_addr_iface);
> -    ofproto_set_datapath_id(br->ofproto, dpid);
> +    if (dpid != ofproto_get_datapath_id(br->ofproto)) {
> +        VLOG_INFO("bridge %s: using datapath ID %016"PRIx64, br->name, dpid);
> +        ofproto_set_datapath_id(br->ofproto, dpid);
> +    }
> 
>     dpid_string = xasprintf("%016"PRIx64, dpid);
>     ovsrec_bridge_set_datapath_id(br->cfg, dpid_string);
> -- 
> 1.7.2.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev