On 4/25/23 14:47, Simon Horman wrote:
On Fri, Apr 21, 2023 at 05:16:45PM +0200, Adrian Moreno wrote:
Currently, the netdev's speed is being calculated by taking the link's
feature bits (using netdev_get_features()) and transforming them into
bps.

This mechanism can be both inaccurate and difficult to maintain, mainly
because we currently use the feature bits supported by OpenFlow which
would have to be extended to support all new feature bits of all netdev
implementations while keeping the OpenFlow API intact.

In order to expose the link speed accurately for all current and future
hardware, add a new netdev API call that allows the implementations to
provide the current and maximum link speeds in Mbps.

Internally, the logic to get the maximum supported speed still relies on
feature bits so it might still get out of sync in the future. However,
the maximum configurable speed is not used as much as the current speed
and these feature bits are not exposed through the netdev interface so
it should be easier to add more.

Use this new function instead of netdev_get_features() where the link
speed is needed.

As a consecuence of this patch, link speeds of cards is properly
reported (internally in OVSDB) even if not supported by OpenFlow.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2137567
Signed-off-by: Adrian Moreno <[email protected]>

...

@@ -2456,14 +2459,19 @@ iface_refresh_netdev_status(struct iface *iface)
      ovsrec_interface_set_link_resets(iface->cfg, &link_resets, 1);
error = netdev_get_features(iface->netdev, &current, NULL, NULL, NULL);
-    bps = !error ? netdev_features_to_bps(current, 0) : 0;
-    if (bps) {
+    if (!error) {
          ovsrec_interface_set_duplex(iface->cfg,
                                      netdev_features_is_full_duplex(current)
                                      ? "full" : "half");
-        ovsrec_interface_set_link_speed(iface->cfg, &bps, 1);
      } else {
          ovsrec_interface_set_duplex(iface->cfg, NULL);
+    }
+
+    error = netdev_get_speed(iface->netdev, &mbps, NULL);
+    if (!error) {
+        bps = (uint64_t) mbps * 1000000;

Hi Adrian,

nit: Can 1000000ULL be used to avoid the need for a cast?
      Likewise in patch 2.


Sure! Thanks for the suggestion.

+        ovsrec_interface_set_link_speed(iface->cfg, &bps, 1);
+    } else {
          ovsrec_interface_set_link_speed(iface->cfg, NULL, 0);
      }

...


--
Adrián Moreno

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to