netdev: Assume default link speed to be 10 Gbps instead of 100 Mbps.

100 Mbps was a fair assumption 13 years ago. Modern days 10 Gbps seems like a good value in case no information is available otherwise. The change mainly affects QoS which is currently limited to 100 Mbps if the user didn't specify 'max-rate' and the card doesn't report the speed or OVS doesn't have a predefined enumeration for the speed reported by the NIC. Calculation of the path cost for STP/RSTP is also affected if OVS is unable to determine the link speed. Lower link speed adapters are typically good at reporting their speed, so chances for overshoot should be low. But newer high-speed adapters, for which there is no speed enumeration or if there are some other issues, will not suffer that much. Acked-by: Mike Pattrick <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
vakabus · Nov 30, 2022 · b22c4d8 · b22c4d8
1 parent d240f72
commit b22c4d8
Show file tree

Hide file tree

Showing 10 changed files with 27 additions and 18 deletions.
diff --git a/NEWS b/NEWS
@@ -23,6 +23,10 @@ Post-v3.0.0
        bug and CVE fixes addressed since its release.
        If a user wishes to benefit from these fixes it is recommended to use
        DPDK 21.11.2.
+   - For the QoS max-rate and STP/RSTP path-cost configuration OVS now assumes
+     10 Gbps link speed by default in case the actual link speed cannot be
+     determined.  Previously it was 10 Mbps.  Values can still be overridden
+     by specifying 'max-rate' or '[r]stp-path-cost' accordingly.
 
 
 v3.0.0 - 15 Aug 2022

diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
@@ -121,6 +121,8 @@ enum netdev_features {
     NETDEV_F_PAUSE_ASYM = 1 << 15, /* Asymmetric pause. */
 };
 
+#define NETDEV_DEFAULT_BPS UINT64_C(10 * 1000 * 1000 * 1000)
+
 int netdev_get_features(const struct netdev *,
                         enum netdev_features *current,
                         enum netdev_features *advertised,

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
@@ -4710,7 +4710,7 @@ htb_parse_qdisc_details__(struct netdev *netdev_,
 
         netdev_linux_read_features(netdev);
         current = !netdev->get_features_error ? netdev->current : 0;
-        hc->max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
+        hc->max_rate = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 8;
     }
     hc->min_rate = hc->max_rate;
     hc->burst = 0;
@@ -5182,7 +5182,7 @@ hfsc_parse_qdisc_details__(struct netdev *netdev_, const struct smap *details,
 
         netdev_linux_read_features(netdev);
         current = !netdev->get_features_error ? netdev->current : 0;
-        max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
+        max_rate = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 8;
     }
 
     class->min_rate = max_rate;

diff --git a/lib/rstp.c b/lib/rstp.c
@@ -784,7 +784,7 @@ rstp_convert_speed_to_cost(unsigned int speed)
           : speed >= 100 ? 200000 /* 100 Mb/s. */
           : speed >= 10 ? 2000000 /* 10 Mb/s. */
           : speed >= 1 ? 20000000 /* 1 Mb/s. */
-          : RSTP_DEFAULT_PORT_PATH_COST; /* 100 Mb/s. */
+          : RSTP_DEFAULT_PORT_PATH_COST; /* 10 Gb/s. */
 
     return value;
 }

diff --git a/lib/rstp.h b/lib/rstp.h
@@ -84,7 +84,7 @@ struct dp_packet;
 /* Port path cost [Table 17-3] */
 #define RSTP_MIN_PORT_PATH_COST 1
 #define RSTP_MAX_PORT_PATH_COST 200000000
-#define RSTP_DEFAULT_PORT_PATH_COST 200000
+#define RSTP_DEFAULT_PORT_PATH_COST 2000
 
 /* RSTP Bridge identifier [9.2.5].  Top four most significant bits are a
  * priority value. The next most significant twelve bits are a locally

diff --git a/lib/stp.c b/lib/stp.c
@@ -313,7 +313,7 @@ stp_create(const char *name, stp_identifier bridge_id,
     for (p = stp->ports; p < &stp->ports[ARRAY_SIZE(stp->ports)]; p++) {
         p->stp = stp;
         p->port_id = (stp_port_no(p) + 1) | (STP_DEFAULT_PORT_PRIORITY << 8);
-        p->path_cost = 19;      /* Recommended default for 100 Mb/s link. */
+        p->path_cost = 2;      /* Recommended default for 10 Gb/s link. */
         stp_initialize_port(p, STP_DISABLED);
     }
     ovs_refcount_init(&stp->ref_cnt);
@@ -989,7 +989,7 @@ stp_convert_speed_to_cost(unsigned int speed)
         : speed >= 16 ? 62  /* 16 Mb/s. */
         : speed >= 10 ? 100 /* 10 Mb/s. */
         : speed >= 4 ? 250  /* 4 Mb/s. */
-        : 19;             /* 100 Mb/s (guess). */
+        : 2;             /* 10 Gb/s (guess). */
     ovs_mutex_unlock(&mutex);
     return ret;
 }

diff --git a/tests/stp.at b/tests/stp.at
@@ -620,10 +620,10 @@ ovs-appctl time/stop
 ovs-appctl time/warp 31000 1000
 
 AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
-  p1         designated forwarding 19    128.1
+  p1         designated forwarding 2     128.1
 ])
 AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
-  p2         designated forwarding 19    128.2
+  p2         designated forwarding 2     128.2
 ])
 
 # add a stp port
@@ -637,24 +637,24 @@ ovs-appctl netdev-dummy/set-admin-state p3 down
 
 # We should not show the p3 because its link-state is down
 AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
-  p1         designated forwarding 19    128.1
+  p1         designated forwarding 2     128.1
 ])
 AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
-  p2         designated forwarding 19    128.2
+  p2         designated forwarding 2     128.2
 ])
 AT_CHECK([ovs-appctl stp/show br0 | grep p3], [1], [dnl
 ])
 
 ovs-appctl netdev-dummy/set-admin-state p3 up
 
 AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
-  p1         designated forwarding 19    128.1
+  p1         designated forwarding 2     128.1
 ])
 AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
-  p2         designated forwarding 19    128.2
+  p2         designated forwarding 2     128.2
 ])
 AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl
-  p3         designated listening  19    128.3
+  p3         designated listening  2     128.3
 ])
 
 

diff --git a/tests/test-rstp.c b/tests/test-rstp.c
@@ -107,6 +107,8 @@ send_bpdu(struct dp_packet *pkt, void *port_, void *b_)
     dp_packet_delete(pkt);
 }
 
+#define RSTP_PORT_PATH_COST_100M 200000
+
 static struct bridge *
 new_bridge(struct test_case *tc, int id)
 {
@@ -122,6 +124,7 @@ new_bridge(struct test_case *tc, int id)
     for (i = 1; i < MAX_PORTS; i++) {
         p = rstp_add_port(b->rstp);
         rstp_port_set_aux(p, p);
+        rstp_port_set_path_cost(p, RSTP_PORT_PATH_COST_100M);
         rstp_port_set_state(p, RSTP_DISABLED);
         rstp_port_set_mac_operational(p, true);
     }
@@ -544,8 +547,8 @@ test_rstp_main(int argc, char *argv[])
                         }
                         get_token();
 
-                        path_cost = match(":") ? must_get_int() :
-                                                 RSTP_DEFAULT_PORT_PATH_COST;
+                        path_cost = match(":") ? must_get_int()
+                                               : RSTP_PORT_PATH_COST_100M;
                         if (port_no < bridge->n_ports) {
                             /* Enable port. */
                             reinitialize_port(p);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
@@ -1678,7 +1678,7 @@ port_configure_stp(const struct ofproto *ofproto, struct port *port,
         unsigned int mbps;
 
         netdev_get_features(iface->netdev, &current, NULL, NULL, NULL);
-        mbps = netdev_features_to_bps(current, 100 * 1000 * 1000) / 1000000;
+        mbps = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 1000000;
         port_s->path_cost = stp_convert_speed_to_cost(mbps);
     }
 
@@ -1761,7 +1761,7 @@ port_configure_rstp(const struct ofproto *ofproto, struct port *port,
         unsigned int mbps;
 
         netdev_get_features(iface->netdev, &current, NULL, NULL, NULL);
-        mbps = netdev_features_to_bps(current, 100 * 1000 * 1000) / 1000000;
+        mbps = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 1000000;
         port_s->path_cost = rstp_convert_speed_to_cost(mbps);
     }
 

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
@@ -4776,7 +4776,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
         Maximum rate shared by all queued traffic, in bit/s.  Optional.  If not
         specified, for physical interfaces, the default is the link rate.  For
         other interfaces or if the link rate cannot be determined, the default
-        is currently 100 Mbps.
+        is currently 10 Gbps.
       </column>
     </group>