Thanks, I pushed this to master.
On Sat, Nov 03, 2012 at 01:50:10PM -0700, Justin Pettit wrote:
> 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
