On 5/5/23 10:04, Adrian Moreno wrote: > > > On 5/4/23 17:52, Ilya Maximets wrote: >> On 4/21/23 17:16, 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. >> >> Hi, Adrian. Thanks for the set! I didn't test it, but I have a few >> comments inline. >> >> Best regards, Ilya Maximets. >> >>> >>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2137567 >> >> I'm not sure about this link. The BZ talks about switching to >> ETHTOOL_GLINKSETTINGS interface to get speeds that do not have >> predefined values. Current patch is a step forward to be able >> to support other link speeds, but IIUC, we will still not able >> to detect 25 and 50 Gbps links in netdev-linux. > > It's true that we will not be able to report a bit in the feature bitmask > that indicates 25 or 50 Gbps. I think this is not a huge deal because we > don't have those flags in the ofproto layer anyway. > > However, with this patch netdev-linux should be able to read the speed > directly from ethtool_cmd_speed() which uses a 32-bit field in Mbps. Using > this speed directly (and not the bitmask) to calculate QoS maximum rates and > even to report the speed in ofproto port information (OFP >= 1.1) should, > IIUC, address the limitations reported in the BZ.
Oh, I see. I missed the fact that ETHTOOL_GSET reports a numerical value of the speed. Is it possible to add a system test for this functionality? e.g. can we change the speed of a tap or veth interface with ethtool to some currently unsupported value (50G) ? OTOH, ETHTOOL_GSET is still a deprecated interface, so we'll need to eventually move to ETHTOOL_GLINKSETTINGS. But that can be a separate change. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
