Re: TBF man page: peakrate must be greater than rate

On Fri, 3 Sep 2021 16:48:45 +0100
Andy Walker [off-list ref] wrote:

Hi,

I was struggling with the following command line:
tc qdisc add dev eth1 parent 1:1 handle 10: tbf rate 10.0kbit maxburst
10kb latency 50ms peakrate 10kbit mtu 1600
RTNETLINK answers: Invalid argument

When I altered peakrate to be 10.1kbit, tbf accepted its parameters.

Looking in dmesg, I found the following:

sch_tbf: peakrate 1250 is lower than or equals to rate 1250

The man page does not specify that peakrate needs to be greater than
rate. This would probably be a useful addition, as it's easy to assume
that the two can/should be set to the same value for precise rate
limiting.

Thanks,
Andy

Try this:

From 20f4d28336fd7b1de2df1a7478319f44b3e258e2 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <redacted>
Date: Fri, 3 Sep 2021 09:24:52 -0700
Subject: [PATCH] net: sched: tbf replace log message with extack

The Token Bucket Filter qdisc should be using extack
to report errors, rather than forcing user to look in
dmesg.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 net/sched/sch_tbf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 78e79029dc63..33cd712caea5 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c

@@ -387,8 +387,7 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt,
 			prate64 = nla_get_u64(tb[TCA_TBF_PRATE64]);
 		psched_ratecfg_precompute(&peak, &qopt->peakrate, prate64);
 		if (peak.rate_bytes_ps <= rate.rate_bytes_ps) {
-			pr_warn_ratelimited("sch_tbf: peakrate %llu is lower than or equals to rate %llu !\n",
-					peak.rate_bytes_ps, rate.rate_bytes_ps);
+			NL_SET_ERR_MSG(extack, "peakrate is lower than or equals to rate");
 			err = -EINVAL;
 			goto done;
 		}

@@ -405,9 +404,7 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt,
 	}
 
 	if (max_size < psched_mtu(qdisc_dev(sch)))
-		pr_warn_ratelimited("sch_tbf: burst %llu is lower than device %s mtu (%u) !\n",
-				    max_size, qdisc_dev(sch)->name,
-				    psched_mtu(qdisc_dev(sch)));
+		NL_SET_ERR_MSG(extack, "burst is lower than device mtu");
 
 	if (!max_size) {
 		err = -EINVAL;

-- 
2.30.2