On 7/10/23 16:33, Eelco Chaudron wrote:


On 10 Jul 2023, at 16:26, Adrian Moreno wrote:

On 7/10/23 16:19, Adrian Moreno wrote:


On 7/6/23 14:50, Eelco Chaudron wrote:


On 2 Jun 2023, at 16:13, Adrian Moreno wrote:

Instead of relying on feature bits, use the speed value directly as
maximum rate for htb and hfsc classes.

There is still a limitation with the maximum rate that we can express
with a 32-bit number in bytes/s (~ 34.3Gbps), but using the actual link speed
instead of the feature bits, we can at least use an accurate maximum for
some link speeds (such as 25Gbps) which are not supported by netdev's feature
bits.

Hi Adrian,

See some comments below.

//Eelco


Signed-off-by: Adrian Moreno <[email protected]>
---
   lib/netdev-linux.c | 14 ++++++--------
   1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 3055f88d2..56b487eea 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -4746,11 +4746,10 @@ htb_parse_qdisc_details__(struct netdev *netdev_,

       hc->max_rate = smap_get_ullong(details, "max-rate", 0) / 8;
       if (!hc->max_rate) {
-        enum netdev_features current;
-
           netdev_linux_read_features(netdev);
-        current = !netdev->get_features_error ? netdev->current : 0;
-        hc->max_rate = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 8;

Re-reading my previous comments on fallback for to netdev_get_features() I 
guess the new API should replace this one.
So to make a statement I would say try to remove the netdev_features_to_bps() 
API, or move it to netdev_bsd_features_to_bps() for netdev_bsd_get_speed only 
(with a comment) so it’s clear that people should NOT use this API from now on.


We are still not deprecating the old API (netdev_get_features() and "enum 
netdev_features") since it is in line with OFP1.0 that does not have numeric speeds. 
e.g: lib/ofp-port uses netdev_get_features() to convert the maximum speed to kbps when 
decoding OFP1.0 port descriptions.

What I do think is a good idea is to add a comment pointing new users to the 
new API if what they are looking for is a more accurate speed. WDYT?



Or maybe we could even rename it and add "legacy" in the name so that it's more 
clear that it should not be used (for those who don't read the comments :-)).

What about a check patch warning when the API is used?


OK. I'll look into checkpatch.py. Thanks.

--
Adrián Moreno


--
Adrián Moreno

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

Reply via email to