| Message ID | 20230712070259.531904-3-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Improve linux QoS for exotic and fast links | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 12 Jul 2023, at 9:02, 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. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> This patch looks good, just two coding style nits I would fix in case we need another revision. Cheers, Eelco > --- > lib/netdev-linux.c | 28 +++++++++++----------------- > 1 file changed, 11 insertions(+), 17 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index b5fd99874..4769894bb 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -4781,18 +4781,15 @@ htb_parse_tcmsg__(struct ofpbuf *tcmsg, unsigned int *queue_id, > } > > static void > -htb_parse_qdisc_details__(struct netdev *netdev_, > - const struct smap *details, struct htb_class *hc) > +htb_parse_qdisc_details__(struct netdev *netdev, const struct smap *details, > + struct htb_class *hc) > { > - struct netdev_linux *netdev = netdev_linux_cast(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; > + uint32_t current_speed; Newline here. > + netdev_get_speed(netdev, ¤t_speed, NULL); > + hc->max_rate = current_speed ? current_speed / 8 * 1000000ULL > + : NETDEV_DEFAULT_BPS / 8; > } > hc->min_rate = hc->max_rate; > hc->burst = 0; > @@ -5253,18 +5250,15 @@ hfsc_query_class__(const struct netdev *netdev, unsigned int handle, > } > > static void > -hfsc_parse_qdisc_details__(struct netdev *netdev_, const struct smap *details, > +hfsc_parse_qdisc_details__(struct netdev *netdev, const struct smap *details, > struct hfsc_class *class) > { > - struct netdev_linux *netdev = netdev_linux_cast(netdev_); > - > 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; > + uint32_t current_speed; Newline here. > + netdev_get_speed(netdev, ¤t_speed, NULL); > + max_rate = current_speed ? current_speed / 8 * 1000000ULL > + : NETDEV_DEFAULT_BPS / 8; > } > > class->min_rate = max_rate; > -- > 2.41.0
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index b5fd99874..4769894bb 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -4781,18 +4781,15 @@ htb_parse_tcmsg__(struct ofpbuf *tcmsg, unsigned int *queue_id, } static void -htb_parse_qdisc_details__(struct netdev *netdev_, - const struct smap *details, struct htb_class *hc) +htb_parse_qdisc_details__(struct netdev *netdev, const struct smap *details, + struct htb_class *hc) { - struct netdev_linux *netdev = netdev_linux_cast(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; + uint32_t current_speed; + netdev_get_speed(netdev, ¤t_speed, NULL); + hc->max_rate = current_speed ? current_speed / 8 * 1000000ULL + : NETDEV_DEFAULT_BPS / 8; } hc->min_rate = hc->max_rate; hc->burst = 0; @@ -5253,18 +5250,15 @@ hfsc_query_class__(const struct netdev *netdev, unsigned int handle, } static void -hfsc_parse_qdisc_details__(struct netdev *netdev_, const struct smap *details, +hfsc_parse_qdisc_details__(struct netdev *netdev, const struct smap *details, struct hfsc_class *class) { - struct netdev_linux *netdev = netdev_linux_cast(netdev_); - 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; + uint32_t current_speed; + netdev_get_speed(netdev, ¤t_speed, NULL); + max_rate = current_speed ? current_speed / 8 * 1000000ULL + : NETDEV_DEFAULT_BPS / 8; } class->min_rate = max_rate;
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. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- lib/netdev-linux.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)