Bug 216813

Summary: HFSC: "tc class change" can't remove rt service curve
Product: Networking Reporter: Karel Řeřicha (karel)
Component: OtherAssignee: Stephen Hemminger (stephen)
Status: NEW ---    
Severity: normal    
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.15.76 Subsystem:
Regression: No Bisected commit-id:

Description Karel Řeřicha 2022-12-16 09:29:19 UTC
When trying to change HFSC class with rt, ls and ul service curves "tc class change" is unable to remove rt service curve.

System:

# uname -a
Linux karel2 5.15.76-gentoo #1 SMP PREEMPT Mon Oct 31 08:56:41 CET 2022 x86_64 AMD Ryzen 5 3600 6-Core Processor AuthenticAMD GNU/Linux
# tc -V
tc utility, iproute2-6.0.0

Let us create HFSC qdisc with one class having rt, ls, ul service curves:

# tc qdisc add dev lo root handle 1:0 hfsc
# tc class add dev lo parent 1:0 classid 1:1 hfsc ls m2 1Gbit ul m2 1Gbit
# tc class add dev lo parent 1:1 classid 1:2 hfsc rt m2 100Mbit ls m2 200Mbit ul m2 300Mbit
# tc class show dev lo classid 1:2
class hfsc 1:2 parent 1:1 rt m1 0bit d 0us m2 100Mbit ls m1 0bit d 0us m2 200Mbit ul m1 0bit d 0us m2 300Mbit
```
Now when trying to remove rt from 1:2 class:

# tc class change dev lo parent 1:1 classid 1:2 hfsc rt m2 0kbit ls m2 201Mbit ul m2 301Mbit
HFSC: Service Curve has two zero slopes
HFSC: Illegal "rt"

Ok, it doesn't accept empty service curve. Let us try to not specify rt:

# tc class change dev lo parent 1:1 classid 1:2 hfsc ls m2 202Mbit ul m2 302Mbit
# tc class show dev lo classid 1:2
class hfsc 1:2 parent 1:1 rt m1 0bit d 0us m2 100Mbit ls m1 0bit d 0us m2 202Mbit ul m1 0bit d 0us m2 302Mbit

Command is accepted but realtime service curve is still there. This is apparently inconsistent behaviour. I would expect "tc class change" will remove realtime curve if only link sharing and upper limit are passed.

If class has both rt and ls service curves, "tc class change" should be able to remove one of them from class.

I would say there are two ways to achieve it:

1. allow to specify one of rt and ls to be "m2 0" and remove such curve from class

2. respect passed parameters and when one of rt or ls is not specified, then remove it from class if it is there

Second method seems to me more consistent.

I have looked at the source of iproute2 and check for two zero slopes is same for adding and changing class:

https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/tc/q_hfsc.c#n410

It would possible to rewrite this check to allow "tc class change" to pass one of rt and ls with zero m2. But I am not sure if this is enough and what will happen if two zero slopes are passed to HFSC.