Looks good.
--Justin
On Oct 24, 2012, at 1:44 PM, Ben Pfaff <b...@nicira.com> wrote:
> When a link is down, or when a link has no speed because it is not a
> physical interface, Open vSwitch previously reported that its rate is 100
> Mbps as a default. This is counterintuitive, however, so this commit
> changes Open vSwitch behavior to report 0 Mbps when a link is down or its
> speed is otherwise unavailable.
>
> Bug #13388.
> Reported-by: Hiroshi Tanaka <htan...@nicira.com>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/netdev-linux.c | 4 ++--
> lib/netdev.c | 7 ++++---
> lib/netdev.h | 3 ++-
> lib/ofp-util.c | 4 ++--
> ofproto/ofproto-dpif-sflow.c | 2 +-
> ofproto/ofproto.c | 4 ++--
> tests/ofp-print.at | 2 +-
> tests/ofproto.at | 10 +++++-----
> vswitchd/bridge.c | 18 ++++++------------
> 9 files changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 412a92d..0460c06 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2668,7 +2668,7 @@ htb_parse_qdisc_details__(struct netdev *netdev,
> enum netdev_features current;
>
> netdev_get_features(netdev, ¤t, NULL, NULL, NULL);
> - hc->max_rate = netdev_features_to_bps(current) / 8;
> + hc->max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) /
> 8;
> }
> hc->min_rate = hc->max_rate;
> hc->burst = 0;
> @@ -3147,7 +3147,7 @@ hfsc_parse_qdisc_details__(struct netdev *netdev, const
> struct smap *details,
> enum netdev_features current;
>
> netdev_get_features(netdev, ¤t, NULL, NULL, NULL);
> - max_rate = netdev_features_to_bps(current) / 8;
> + max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
> }
>
> class->min_rate = max_rate;
> diff --git a/lib/netdev.c b/lib/netdev.c
> index c135c6f..1921ac0 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -620,9 +620,10 @@ netdev_get_features(const struct netdev *netdev,
>
> /* Returns the maximum speed of a network connection that has the NETDEV_F_*
> * bits in 'features', in bits per second. If no bits that indicate a speed
> - * are set in 'features', assumes 100Mbps. */
> + * are set in 'features', returns 'default_bps'. */
> uint64_t
> -netdev_features_to_bps(enum netdev_features features)
> +netdev_features_to_bps(enum netdev_features features,
> + uint64_t default_bps)
> {
> enum {
> F_1000000MB = NETDEV_F_1TB_FD,
> @@ -641,7 +642,7 @@ netdev_features_to_bps(enum netdev_features features)
> : features & F_1000MB ? UINT64_C(1000000000)
> : features & F_100MB ? UINT64_C(100000000)
> : features & F_10MB ? UINT64_C(10000000)
> - : UINT64_C(100000000));
> + : default_bps);
> }
>
> /* Returns true if any of the NETDEV_F_* bits that indicate a full-duplex link
> diff --git a/lib/netdev.h b/lib/netdev.h
> index d2cc8b5..67122ee 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -146,7 +146,8 @@ int netdev_get_features(const struct netdev *,
> enum netdev_features *advertised,
> enum netdev_features *supported,
> enum netdev_features *peer);
> -uint64_t netdev_features_to_bps(enum netdev_features features);
> +uint64_t netdev_features_to_bps(enum netdev_features features,
> + uint64_t default_bps);
> bool netdev_features_is_full_duplex(enum netdev_features features);
> int netdev_set_advertisements(struct netdev *, enum netdev_features
> advertise);
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 34255da..ad59a34 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -2420,8 +2420,8 @@ ofputil_decode_ofp10_phy_port(struct ofputil_phy_port
> *pp,
> pp->supported = netdev_port_features_from_ofp10(opp->supported);
> pp->peer = netdev_port_features_from_ofp10(opp->peer);
>
> - pp->curr_speed = netdev_features_to_bps(pp->curr) / 1000;
> - pp->max_speed = netdev_features_to_bps(pp->supported) / 1000;
> + pp->curr_speed = netdev_features_to_bps(pp->curr, 0) / 1000;
> + pp->max_speed = netdev_features_to_bps(pp->supported, 0) / 1000;
>
> return 0;
> }
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index 23f5498..37b662c 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -179,7 +179,7 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller,
> if (!netdev_get_features(dsp->ofport->netdev, ¤t, NULL, NULL,
> NULL)) {
> /* The values of ifDirection come from MAU MIB (RFC 2668): 0 =
> unknown,
> 1 = full-duplex, 2 = half-duplex, 3 = in, 4=out */
> - counters->ifSpeed = netdev_features_to_bps(current);
> + counters->ifSpeed = netdev_features_to_bps(current, 0);
> counters->ifDirection = (netdev_features_is_full_duplex(current)
> ? 1 : 2);
> } else {
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 2fb2fc8..0979f70 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1549,8 +1549,8 @@ ofport_open(const struct ofproto *ofproto,
> pp->state = netdev_get_carrier(netdev) ? 0 : OFPUTIL_PS_LINK_DOWN;
> netdev_get_features(netdev, &pp->curr, &pp->advertised,
> &pp->supported, &pp->peer);
> - pp->curr_speed = netdev_features_to_bps(pp->curr);
> - pp->max_speed = netdev_features_to_bps(pp->supported);
> + pp->curr_speed = netdev_features_to_bps(pp->curr, 0);
> + pp->max_speed = netdev_features_to_bps(pp->supported, 0);
>
> return netdev;
> }
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index f1fdefb..82846d5 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -142,7 +142,7 @@ actions: OUTPUT SET_VLAN_VID SET_VLAN_PCP STRIP_VLAN
> SET_DL_SRC SET_DL_DST SET_N
> LOCAL(br0): addr:50:54:00:00:00:01
> config: PORT_DOWN
> state: LINK_DOWN
> - speed: 100 Mbps now, 100 Mbps max
> + speed: 0 Mbps now, 0 Mbps max
> ])
> AT_CLEANUP
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 1e5664a..51667e8 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -17,7 +17,7 @@ actions: OUTPUT SET_VLAN_VID SET_VLAN_PCP STRIP_VLAN
> SET_DL_SRC SET_DL_DST SET_N
> LOCAL(br0): addr:aa:55:aa:55:00:00
> config: PORT_DOWN
> state: LINK_DOWN
> - speed: 100 Mbps now, 100 Mbps max
> + speed: 0 Mbps now, 0 Mbps max
> OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
> ])
> OVS_VSWITCHD_STOP
> @@ -46,7 +46,7 @@ OFPST_PORT_DESC reply:
> LOCAL(br0): addr:aa:55:aa:55:00:00
> config: PORT_DOWN
> state: LINK_DOWN
> - speed: 100 Mbps now, 100 Mbps max
> + speed: 0 Mbps now, 0 Mbps max
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -97,7 +97,7 @@ actions: OUTPUT SET_VLAN_VID SET_VLAN_PCP STRIP_VLAN
> SET_DL_SRC SET_DL_DST SET_N
> LOCAL(br0): addr:aa:55:aa:55:00:00
> config: $config
> state: $state
> - speed: 100 Mbps now, 100 Mbps max
> + speed: 0 Mbps now, 0 Mbps max
> OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
> ])
> done
> @@ -654,7 +654,7 @@ priority:0,metadata:0,in_port:0000,tci(0)
> mac(00:26:b9:8c:b0:f9->00:25:83:df:b4:
> echo >>expout "OFPT_PORT_STATUS: ADD: 1(test): addr:aa:55:aa:55:00:0x
> config: PORT_DOWN
> state: LINK_DOWN
> - speed: 100 Mbps now, 100 Mbps max"
> + speed: 0 Mbps now, 0 Mbps max"
> fi
>
> # OFPT_PORT_STATUS, OFPPR_DELETE
> @@ -663,7 +663,7 @@ priority:0,metadata:0,in_port:0000,tci(0)
> mac(00:26:b9:8c:b0:f9->00:25:83:df:b4:
> echo >>expout "OFPT_PORT_STATUS: DEL: 1(test): addr:aa:55:aa:55:00:0x
> config: PORT_DOWN
> state: LINK_DOWN
> - speed: 100 Mbps now, 100 Mbps max"
> + speed: 0 Mbps now, 0 Mbps max"
> fi
>
> # OFPT_FLOW_REMOVED, OFPRR_DELETE
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index a481f06..79cd262 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -953,16 +953,11 @@ port_configure_stp(const struct ofproto *ofproto,
> struct port *port,
> port_s->path_cost = strtoul(config_str, NULL, 10);
> } else {
> enum netdev_features current;
> + unsigned int mbps;
>
> - if (netdev_get_features(iface->netdev, ¤t, NULL, NULL, NULL)) {
> - /* Couldn't get speed, so assume 100Mb/s. */
> - port_s->path_cost = 19;
> - } else {
> - unsigned int mbps;
> -
> - mbps = netdev_features_to_bps(current) / 1000000;
> - port_s->path_cost = stp_convert_speed_to_cost(mbps);
> - }
> + netdev_get_features(iface->netdev, ¤t, NULL, NULL, NULL);
> + mbps = netdev_features_to_bps(current, 100 * 1000 * 1000) / 1000000;
> + port_s->path_cost = stp_convert_speed_to_cost(mbps);
> }
>
> config_str = smap_get(&port->cfg->other_config, "stp-port-priority");
> @@ -1654,12 +1649,11 @@ iface_refresh_status(struct iface *iface)
> smap_destroy(&smap);
>
> error = netdev_get_features(iface->netdev, ¤t, NULL, NULL, NULL);
> - if (!error) {
> + bps = !error ? netdev_features_to_bps(current, 0) : 0;
> + if (bps) {
> ovsrec_interface_set_duplex(iface->cfg,
> netdev_features_is_full_duplex(current)
> ? "full" : "half");
> - /* warning: uint64_t -> int64_t conversion */
> - bps = netdev_features_to_bps(current);
> ovsrec_interface_set_link_speed(iface->cfg, &bps, 1);
> }
> else {
> --
> 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
