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? > -- > Adrián Moreno _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
