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 <amore...@redhat.com>
---
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?
+ hc->max_rate = !netdev->get_features_error
+ ? netdev->current_speed / 8 * 1000000ULL
+ : NETDEV_DEFAULT_BPS / 8;
What if for some reason netdev->current_speed equals zero, it no longer reports
NETDEV_DEFAULT_BPS / 8.
Although it's likely this never happens with the current implementation, this
could change and your proposal is more robust. I'll update it.
}
hc->min_rate = hc->max_rate;
hc->burst = 0;
@@ -5218,11 +5217,10 @@ hfsc_parse_qdisc_details__(struct netdev *netdev_,
const struct smap *details,
uint32_t max_rate = smap_get_ullong(details, "max-rate", 0) / 8;
if (!max_rate) {
- enum netdev_features current;
-
netdev_linux_read_features(netdev);
- current = !netdev->get_features_error ? netdev->current : 0;
- max_rate = netdev_features_to_bps(current, NETDEV_DEFAULT_BPS) / 8;
+ max_rate = !netdev->get_features_error
+ ? netdev->current_speed / 8 * 1000000ULL
+ : NETDEV_DEFAULT_BPS / 8;
Same as above when netdev->current_speed == 0.
}
class->min_rate = max_rate;
--
2.40.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
Adrián Moreno
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
