[ovs-dev] [PATCH] ofproto: Report 0 Mbps when speed not available instead of 100 Mbps.

Ben Pfaff blp at nicira.com
Wed Oct 24 20:44:37 UTC 2012


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 at nicira.com>
Signed-off-by: Ben Pfaff <blp at 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, &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 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, &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 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, &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");
@@ -1654,12 +1649,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 {
-- 
1.7.2.5




More information about the dev mailing list