On Thu, May 04, 2023 at 04:15:31PM +0200, Adrian Moreno wrote: > > > On 5/4/23 09:09, Simon Horman wrote: > > On Wed, May 03, 2023 at 04:05:36PM +0200, Adrian Moreno wrote: > > > > > > > > > On 4/25/23 14:51, Simon Horman wrote: > > > > On Fri, Apr 21, 2023 at 05:16:48PM +0200, Adrian Moreno wrote: > > > > > Currently, htb rates are capped at ~34Gbps because they are internally > > > > > expressed as 32-bit fields. > > > > > > > > > > Move min and max rates to 64-bit fields and use TCA_HTB_RATE64 and > > > > > TCA_HTB_CEIL64 to configure HTC classes to break this barrier. > > > > > > > > > > In order to test this, create a dummy tuntap device and set it's > > > > > speed to a very high value so we can try adding a QoS queue with big > > > > > rates. > > > > > > > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2137619 > > > > > Signed-off-by: Adrian Moreno <amore...@redhat.com> > > > > > --- > > > > > acinclude.m4 | 7 +++++ > > > > > lib/netdev-linux.c | 61 > > > > > +++++++++++++++++++++++++++++------------ > > > > > tests/system-traffic.at | 31 +++++++++++++++++++++ > > > > > 3 files changed, 81 insertions(+), 18 deletions(-) > > > > > > > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > > > > index ac1eab790..3bce2a652 100644 > > > > > --- a/acinclude.m4 > > > > > +++ b/acinclude.m4 > > > > > @@ -211,6 +211,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [ > > > > > ])], > > > > > [AC_DEFINE([HAVE_TCA_STATS_PKT64], [1], > > > > > [Define to 1 if TCA_STATS_PKT64 is available.])]) > > > > > + > > > > > + AC_COMPILE_IFELSE([ > > > > > + AC_LANG_PROGRAM([#include <linux/pkt_sched.h>], [ > > > > > + int x = TCA_HTB_RATE64; > > > > > + ])], > > > > > + [AC_DEFINE([HAVE_TCA_HTB_RATE64], [1], > > > > > + [Define to 1 if TCA_HTB_RATE64 is available.])]) > > > > > ]) > > > > > dnl OVS_CHECK_LINUX_SCTP_CT > > > > > > > > ... > > > > > > > > > --- a/tests/system-traffic.at > > > > > +++ b/tests/system-traffic.at > > > > > @@ -2354,6 +2354,37 @@ AT_CHECK([tc class show dev ovs-p1 | grep -q > > > > > 'class htb .* HTB_CONF']) > > > > > OVS_TRAFFIC_VSWITCHD_STOP > > > > > AT_CLEANUP > > > > > +AT_SETUP([QoS - 64bit]) > > > > > +AT_SKIP_IF([test $HAVE_TC = no]) > > > > > +OVS_TRAFFIC_VSWITCHD_START() > > > > > > > > Hi Adrian, > > > > > > > > > Hi Simon, > > > Thanks for the review. > > > > > > > > > > > Should this test be conditional on HAVE_TCA_STATS_PKT64 and > > > > HAVE_TCA_HTB_RATE64? If so, perhaps something like > > > > CHECK_TC_INGRESS_PPS() is appropriate. > > > > > > > > > > Let me make sure I understand your proposal: You're suggesting adding a > > > function similar to CHECK_TC_INGRESS_PPS() that ensures that the running > > > kernel supports TCA_STATS_PKT64 and makes the test fail early if it > > > doesn't. > > > Is that it? > > > > Yes, with one minor modification: skip rather than fail the test. > > > > One question I have is, to counter my original suggestion, is do we really > > expect to see kernels without HAVE_TCA_STATS_PKT64 and HAVE_TCA_HTB_RATE64? > > > > I'm not sure when they were added. But perhaps it was such a long time ago > > that we won't be running this feature on kernels that old anyway - f.e. > > because some other prerequisite is missing or we just don't support Kernels > > that old. > > > > TCA_STATS_PKT64 was introduced 4 years ago[1] and TCA_HTB_RATE64 is even > older (10 years) [2], in kernel v3.13. I just wanted to be extra careful but > maybe you're right and it's not supported anyway.
Ok, I guess it is plausible that people will run the testsuite against a 4 year old kernel. So perhaps a check, along the lines of CHECK_TC_INGRESS_PPS() is a low cost way to guard against failures. > > [1] > https://github.com/torvalds/linux/commit/b33e699fe43aa63f29113311f69357e119ef5276 > [2] > https://github.com/torvalds/linux/commit/df62cdf348c91baac61b4cb19d19ea1ef87b271e > > -- > Adrián Moreno > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
