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

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 3 Sep 2021 16:48:45 +0100
Andy Walker <camtarn@camtarn.org> 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 <sthemmin@microsoft.com>
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