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.

> +        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.

>      }
>      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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Reply via email to