On 2 Jun 2023, at 16:13, 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.
Hi Adrian,
This patch looks good, one suggestion to move some of the stuff to the previous
patch.
//Eelco
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2137619
> Signed-off-by: Adrian Moreno <[email protected]>
> ---
> acinclude.m4 | 10 +++++++
> lib/netdev-linux.c | 59 ++++++++++++++++++++++++++++-------------
> tests/atlocal.in | 1 +
> tests/system-traffic.at | 28 +++++++++++++++++++
> 4 files changed, 80 insertions(+), 18 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index ac1eab790..1e5a9872d 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -211,6 +211,16 @@ 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_SUBST(HAVE_TCA_HTB_RATE64,yes)
> + AC_DEFINE([HAVE_TCA_HTB_RATE64], [1],
> + [Define to 1 if TCA_HTB_RATE64 is available.])],
> + [AC_SUBST(HAVE_TCA_HTB_RATE64,no)]
> + )
> ])
>
> dnl OVS_CHECK_LINUX_SCTP_CT
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index bb81e7022..dff9d627f 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -484,9 +484,9 @@ static const struct tc_ops *const tcs[] = {
> NULL
> };
>
> -static unsigned int tc_ticks_to_bytes(unsigned int rate, unsigned int ticks);
> -static unsigned int tc_bytes_to_ticks(unsigned int rate, unsigned int size);
> -static unsigned int tc_buffer_per_jiffy(unsigned int rate);
> +static unsigned int tc_ticks_to_bytes(uint64_t rate, unsigned int ticks);
> +static unsigned int tc_bytes_to_ticks(uint64_t rate, unsigned int size);
> +static unsigned int tc_buffer_per_jiffy(uint64_t rate);
> static uint32_t tc_time_to_ticks(uint32_t time);
>
> static struct tcmsg *netdev_linux_tc_make_request(const struct netdev *,
> @@ -516,7 +516,7 @@ tc_put_rtab(struct ofpbuf *msg, uint16_t type, const
> struct tc_ratespec *rate,
> uint64_t rate64);
> static int tc_calc_cell_log(unsigned int mtu);
> static void tc_fill_rate(struct tc_ratespec *rate, uint64_t bps, int mtu);
> -static int tc_calc_buffer(unsigned int Bps, int mtu, uint64_t burst_bytes);
> +static int tc_calc_buffer(uint64_t Bps, int mtu, uint64_t burst_bytes);
These calculation changes (these 4 function types) I would move these to the
previous patch as they are general and not htb specific.
>
>
> /* This is set pretty low because we probably won't learn anything from the
> @@ -4560,13 +4560,13 @@ static const struct tc_ops tc_ops_netem = {
>
> struct htb {
> struct tc tc;
> - unsigned int max_rate; /* In bytes/s. */
> + uint64_t max_rate; /* In bytes/s. */
> };
>
> struct htb_class {
> struct tc_queue tc_queue;
> - unsigned int min_rate; /* In bytes/s. */
> - unsigned int max_rate; /* In bytes/s. */
> + uint64_t min_rate; /* In bytes/s. */
> + uint64_t max_rate; /* In bytes/s. */
> unsigned int burst; /* In bytes. */
> unsigned int priority; /* Lower values are higher priorities. */
> };
> @@ -4654,8 +4654,8 @@ htb_setup_class__(struct netdev *netdev, unsigned int
> handle,
> if ((class->min_rate / HTB_RATE2QUANTUM) < mtu) {
> opt.quantum = mtu;
> }
> - opt.buffer = tc_calc_buffer(opt.rate.rate, mtu, class->burst);
> - opt.cbuffer = tc_calc_buffer(opt.ceil.rate, mtu, class->burst);
> + opt.buffer = tc_calc_buffer(class->min_rate, mtu, class->burst);
> + opt.cbuffer = tc_calc_buffer(class->max_rate, mtu, class->burst);
> opt.prio = class->priority;
>
> tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
> @@ -4668,15 +4668,26 @@ htb_setup_class__(struct netdev *netdev, unsigned int
> handle,
>
> nl_msg_put_string(&request, TCA_KIND, "htb");
> opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
> +
> +#ifdef HAVE_TCA_HTB_RATE64
> + if (class->min_rate > UINT32_MAX) {
> + nl_msg_put_u64(&request, TCA_HTB_RATE64, class->min_rate);
> + }
> + if (class->max_rate > UINT32_MAX) {
> + nl_msg_put_u64(&request, TCA_HTB_CEIL64, class->max_rate);
> + }
> +#endif
> nl_msg_put_unspec(&request, TCA_HTB_PARMS, &opt, sizeof opt);
> - tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate, 0);
> - tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil, 0);
> +
> + tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate, class->min_rate);
> + tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil, class->max_rate);
> nl_msg_end_nested(&request, opt_offset);
>
> error = tc_transact(&request, NULL);
> if (error) {
> VLOG_WARN_RL(&rl, "failed to replace %s class %u:%u, parent %u:%u, "
> - "min_rate=%u max_rate=%u burst=%u prio=%u (%s)",
> + "min_rate=%"PRIu64" max_rate=%"PRIu64" burst=%u prio=%u
> "
> + "(%s)",
> netdev_get_name(netdev),
> tc_get_major(handle), tc_get_minor(handle),
> tc_get_major(parent), tc_get_minor(parent),
> @@ -4696,6 +4707,10 @@ htb_parse_tca_options__(struct nlattr *nl_options,
> struct htb_class *class)
> static const struct nl_policy tca_htb_policy[] = {
> [TCA_HTB_PARMS] = { .type = NL_A_UNSPEC, .optional = false,
> .min_len = sizeof(struct tc_htb_opt) },
> +#ifdef HAVE_TCA_HTB_RATE64
> + [TCA_HTB_RATE64] = { .type = NL_A_U64, .optional = true },
> + [TCA_HTB_CEIL64] = { .type = NL_A_U64, .optional = true },
> +#endif
> };
>
> struct nlattr *attrs[ARRAY_SIZE(tca_htb_policy)];
> @@ -4710,7 +4725,15 @@ htb_parse_tca_options__(struct nlattr *nl_options,
> struct htb_class *class)
> htb = nl_attr_get(attrs[TCA_HTB_PARMS]);
> class->min_rate = htb->rate.rate;
> class->max_rate = htb->ceil.rate;
> - class->burst = tc_ticks_to_bytes(htb->rate.rate, htb->buffer);
> +#ifdef HAVE_TCA_HTB_RATE64
> + if (attrs[TCA_HTB_RATE64]) {
> + class->min_rate = nl_attr_get_u64(attrs[TCA_HTB_RATE64]);
> + }
> + if (attrs[TCA_HTB_CEIL64]) {
> + class->max_rate = nl_attr_get_u64(attrs[TCA_HTB_CEIL64]);
> + }
> +#endif
> + class->burst = tc_ticks_to_bytes(class->min_rate, htb->buffer);
> class->priority = htb->prio;
> return 0;
> }
> @@ -5971,7 +5994,7 @@ exit:
> /* Returns the number of bytes that can be transmitted in 'ticks' ticks at a
> * rate of 'rate' bytes per second. */
> static unsigned int
> -tc_ticks_to_bytes(unsigned int rate, unsigned int ticks)
> +tc_ticks_to_bytes(uint64_t rate, unsigned int ticks)
> {
> read_psched();
> return (rate * ticks) / ticks_per_s;
> @@ -5980,7 +6003,7 @@ tc_ticks_to_bytes(unsigned int rate, unsigned int ticks)
> /* Returns the number of ticks that it would take to transmit 'size' bytes
> at a
> * rate of 'rate' bytes per second. */
> static unsigned int
> -tc_bytes_to_ticks(unsigned int rate, unsigned int size)
> +tc_bytes_to_ticks(uint64_t rate, unsigned int size)
> {
> read_psched();
> return rate ? ((unsigned long long int) ticks_per_s * size) / rate : 0;
> @@ -5989,7 +6012,7 @@ tc_bytes_to_ticks(unsigned int rate, unsigned int size)
> /* Returns the number of bytes that need to be reserved for qdisc buffering
> at
> * a transmission rate of 'rate' bytes per second. */
> static unsigned int
> -tc_buffer_per_jiffy(unsigned int rate)
> +tc_buffer_per_jiffy(uint64_t rate)
> {
> read_psched();
> return rate / buffer_hz;
> @@ -6346,7 +6369,7 @@ tc_fill_rate(struct tc_ratespec *rate, uint64_t Bps,
> int mtu)
> /* rate->overhead = 0; */ /* New in 2.6.24, not yet in some */
> /* rate->cell_align = 0; */ /* distro headers. */
> rate->mpu = ETH_TOTAL_MIN;
> - rate->rate = Bps;
> + rate->rate = MIN(UINT32_MAX, Bps);
> }
>
> /* Appends to 'msg' an "rtab" table for the specified 'rate' as a Netlink
> @@ -6378,7 +6401,7 @@ tc_put_rtab(struct ofpbuf *msg, uint16_t type, const
> struct tc_ratespec *rate,
> * burst size of 'burst_bytes'. (If no value was requested, a 'burst_bytes'
> of
> * 0 is fine.) */
> static int
> -tc_calc_buffer(unsigned int Bps, int mtu, uint64_t burst_bytes)
> +tc_calc_buffer(uint64_t Bps, int mtu, uint64_t burst_bytes)
> {
> unsigned int min_burst = tc_buffer_per_jiffy(Bps) + mtu;
> return tc_bytes_to_ticks(Bps, MAX(burst_bytes, min_burst));
> diff --git a/tests/atlocal.in b/tests/atlocal.in
> index 4909c9d08..e3ee2d48c 100644
> --- a/tests/atlocal.in
> +++ b/tests/atlocal.in
> @@ -5,6 +5,7 @@ HAVE_UNBOUND='@HAVE_UNBOUND@'
> EGREP='@EGREP@'
> PYTHON3='@PYTHON3@'
> CFLAGS='@CFLAGS@'
> +HAVE_TCA_HTB_RATE64='@HAVE_TCA_HTB_RATE64@'
>
> # PYTHONCOERCECLOCALE=0 disables the Unicode compatibility warning on
> # stderr that breaks almost any Python3 test (PEP 0538)
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 4c378e1d0..6200ec52e 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -2354,6 +2354,34 @@ 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])
> +AT_SKIP_IF([test $HAVE_TCA_HTB_RATE64 = no])
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +ADD_NAMESPACES(at_ns0, at_ns1)
> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> +
> +dnl Configure the QoS with rates that require 64bits, i.e: > 34Gbps.
> +AT_CHECK([ovs-vsctl set port ovs-p0 qos=@qos -- set port ovs-p1 qos=@qos dnl
> + -- --id=@qos create qos dnl
> + type=linux-htb other-config:max-rate=50000000000
> queues:0=@queue dnl
> + -- --id=@queue create queue dnl
> + other_config:min-rate=40000000000
> other_config:max-rate=50000000000 dnl
> + other_config:burst=50000000000],
> + [ignore], [ignore])
> +
> +OVS_WAIT_UNTIL([tc qdisc show dev ovs-p0 | grep -q htb])
> +OVS_WAIT_UNTIL([tc qdisc show dev ovs-p1 | grep -q htb])
> +
> +m4_define([HTB_CONF], [rate 40Gbit ceil 50Gbit burst 1955030000b cburst
> 1955031250b])
> +AT_CHECK([tc class show dev ovs-p0 | grep -q 'class htb .* HTB_CONF'])
> +AT_CHECK([tc class show dev ovs-p1 | grep -q 'class htb .* HTB_CONF'])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
> AT_BANNER([conntrack])
>
> AT_SETUP([conntrack - controller])
> --
> 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
