ofproto: Report 0 Mbps when speed not available instead of 100 Mbps.

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 <htanaka@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 412a92debf0d0758ffcdb2fe0e7fa70c1ee0e7d3..0460c069cc3b8ef25b452dc7784d014abc44e1c5 100644 (file)
--- 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, &current, 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, &current, 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 c135c6f291210af316c657b80f086620b412790d..1921ac0da0df5328769dc9889420df0295e2c42f 100644 (file)
--- 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 d2cc8b5d9b66b57c03c7f09d10e50bb13a24f731..67122eec1dd11126e0759a4bae76cf56e05b10cf 100644 (file)
--- 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 b81476869610ce16a050e047d517bec9b96c89d1..20306e4835bec39ef55dea2482957fc0d83e0722 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2421,8 +2421,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 d43cb39e7171ef6e557157a0a9741be020f231a2..69362ab84ed8060a208bbf331692e6ecfa08985c 100644 (file)
--- 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, &current, 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 83fd46efacff733fc36fc7ee03c9b5fdac04b4c8..1f34bbaeda98aaf6c92bb0e32abec33b10998f1a 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1707,8 +1707,8 @@ ofport_open(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 963f13c9acae830a97d9ced6eb0bca360e60c130..db972a9e48c4bb8658e6cf795c60e812f6a4e4fe 100644 (file)
--- 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 01205c72521a63a9bff1c338b281c881cdde7ff2..69a170ecb2784377151ef2c6dcd2a7e15323cb4d 100644 (file)
--- 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
@@ -39,15 +39,15 @@ actions: OUTPUT SET_VLAN_VID SET_VLAN_PCP STRIP_VLAN SET_DL_SRC SET_DL_DST SET_N
  1(p1): 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
  99(p2): 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
  LOCAL(br0): 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
 OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
 ])
 
@@ -77,7 +77,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
@@ -128,7 +128,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
@@ -683,7 +683,7 @@ priority=0,udp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_
         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
@@ -692,7 +692,7 @@ priority=0,udp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_
         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 27d40a87519f252d3ac4f5525bf94195f9af77b8..9fcc97098129b026a7a9f240c3872c490b69127f 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -996,16 +996,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, &current, 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, &current, 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");
@@ -1701,12 +1696,11 @@ iface_refresh_status(struct iface *iface)
     smap_destroy(&smap);
 
     error = netdev_get_features(iface->netdev, &current, 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 {