On 5/5/23 11:56, Ilya Maximets wrote:
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.
That's true. However, there is a compatibility layer so even if new drivers
don't implement the old driver API, it'll still be usable from uapi. What the
new API definitely improves the amount (and extensibility) of feature bits.
But as long as the compatibility layer properly reports the numeric speed, we
will be accurate in reporting the link speed (as well as calculating QoS max
rates, etc).
Now, if we evaluate the benefit OVS will take from the new API, I think it won't
be very big since OFP layer will not support the new feature bits. The only real
benefit would be increased accuracy in the numeric maximum speed reported since
the only way is to calculate this based on the feature bits. My feeling is that
it's not that big of a benefit compared with the complexity of having to
maintain both APIs, what do you think?
Best regards, Ilya Maximets.
--
Adrián Moreno
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
