Bug 1898015

Yiche,

This behaviour is per design of limit statement. It is consistent with iptables
in that regard:

| # iptables-nft -A FORWARD -m limit --limit 1/sec --limit-burst 4
| # iptables-nft -A FORWARD -m limit --limit 1/sec --limit-burst 5
| # iptables-nft -A FORWARD -m limit --limit 1/sec --limit-burst 6
| # iptables-nft -S FORWARD
| -P FORWARD ACCEPT
| -A FORWARD -m limit --limit 1/sec --limit-burst 4
| -A FORWARD -m limit --limit 1/sec
| -A FORWARD -m limit --limit 1/sec --limit-burst 6

Though, I have to admit iptables is not entirely consistent there:

| # iptables-nft -L FORWARD
| Chain FORWARD (policy ACCEPT)
| target     prot opt source               destination
|            all  --  anywhere             anywhere             limit: avg 1/sec burst 4
|            all  --  anywhere             anywhere             limit: avg 1/sec burst 5
|            all  --  anywhere             anywhere             limit: avg 1/sec burst 6

The problem with all these default values is we lose information of whether
they were explicitly chosen by the user or not in between commit to kernel and
retrieval. So basically whatever we do with them may be wrong/confusing or just
right: If the user didn't specify 'burst 5' in your example, it might be
confusing if we print it. In these questions, I tend to follow nftables' idea
of being simple in syntax and omit defaults. What do you think?

Thanks, Phil

When I test expression "{tcp dport limit rate over 1/minute drop}" with two tcp flow connect at same time,
it's strange that none of them dropped, because I didn't know the default burst value worked behind the scenes.
So, I think showing out what is affecting result is a better design. And if user explicitly chosen 'burst 5',
It can't be printed.


> I tend to follow nftables' idea of being simple in syntax and omit defaults.
I agree this is crazy:
tcp dport 2215 accept --> tcp sport * tcp dport 2215 ip saddr * ip daddr * ... accept
We don't want print all default value. But something really important default value should be print out.

(In reply to yiche from comment #2)
> When I test expression "{tcp dport limit rate over 1/minute drop}" with two
> tcp flow connect at same time,
> it's strange that none of them dropped, because I didn't know the default
> burst value worked behind the scenes.
> So, I think showing out what is affecting result is a better design. And if
> user explicitly chosen 'burst 5',
> It can't be printed.

It will always be a matter of deciding whether something should be printed even
though the user didn't specify it explicitly or it being omitted because the
user explicitly specified the default value, at least this is a common dilemma
with nftables output.

Thanks for elaborating on your concrete problem, it helps a lot! I even think
it's a bug to have an implicit burst larger than the specified rate.

And that leads me to a broader view: Maybe the default shouldn't be a fixed
value at all but a sane choice depending on the given rate.

> > I tend to follow nftables' idea of being simple in syntax and omit defaults.
> I agree this is crazy:
> tcp dport 2215 accept --> tcp sport * tcp dport 2215 ip saddr * ip daddr *
> ... accept
> We don't want print all default value. But something really important
> default value should be print out.

I agree. In this case though, I think the problem is on the other side: The
actual burst value should not be an important one, at least if the user didn't
choose a specific one! Let's see if I can implement a flexible burst value that
"just works" and is omitted from output. What do you think?

Thanks, Phil

> Maybe the default shouldn't be a fixed value at all but a sane choice depending on the given rate.
> Let's see if I can implement a flexible burst value that "just works" and is omitted from output.

Do you mean that nftables choose burst value automatically？That might be cool
I recommend showing it out instead of omitting from output.

I don't know if I got it right: "limit rate and burst" are like "token and bucket"
If we want {limit rate over 1/seconds} works exactly "over one packet per second",
burst value should always equal to 1, isn't it? So I am not quite understand "just works" here
or you have other plan?

(In reply to yiche from comment #4)
> > Maybe the default shouldn't be a fixed value at all but a sane choice depending on the given rate.
> > Let's see if I can implement a flexible burst value that "just works" and is omitted from output.
> 
> Do you mean that nftables choose burst value automatically？That might be cool
> I recommend showing it out instead of omitting from output.

I think it should be possible to calculate a sensible burst value based on the
given rate value.

> I don't know if I got it right: "limit rate and burst" are like "token and
> bucket"

I don't know what "token" and "bucket" are supposed to mean in this context.
But from my understanding, rate limiting works by averaging and that requires
to sample a certain amount of traffic. In practice, if we have a limit of say 5
packets/minute, it's not practical if we drop packets for 12s after the first
one arrived but instead accept a burst of say 3 packets which means three
packets are allowed in a row until we start dropping, but then we drop for 36s.
Right?

> If we want {limit rate over 1/seconds} works exactly "over one packet per
> second",
> burst value should always equal to 1, isn't it? So I am not quite understand
> "just works" here

Yes, that's the point. A default of 5 packets burst doesn't make sense if the
rate is less than 5 packets. If nftables chooses a better burst (1 packet in
this case), users don't need to know what the actual burst is. If they're into
it, they will choose it themselves. It's enough to illustrate the algorithm in
nft.8 in case someone wonders.

Cheers, Phil

> In practice, if we have a limit of say 5
> packets/minute, it's not practical if we drop packets for 12s after the first
> one arrived but instead accept a burst of say 3 packets which means three
> packets are allowed in a row until we start dropping, but then we drop for 36s.
> Right?
I don't know how the code does it. Thinking If burst is a bigger one, say 10 and a wave of packets come in, should we stop the service/stream for 2 minutes?
In my thought, “burst 5” is like a queue with length 5, dequeue rate is 5 p/min, if the queue is full, extra packets out of queue are dropped.
I still have a question that "limit rate 5/minutes" means take "dequeue 5 packets" action per minute or means take "dequeue 1 packet" action per 12 seconds?
In other words, we should use the time as denominator(packets/time) or packet as denominator(time/one packet).

> If nftables chooses a better burst (1 packet in
> this case), users don't need to know what the actual burst is. If they're into
> it, they will choose it themselves.
Yeah, Omitting is fine as long as it doesn't cause misunderstand.

Thanks, Yi

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Still relevant.

(In reply to Phil Sutter from comment #5)
[...]
> > If we want {limit rate over 1/seconds} works exactly "over one packet per
> > second",
> > burst value should always equal to 1, isn't it? So I am not quite understand
> > "just works" here
> 
> Yes, that's the point. A default of 5 packets burst doesn't make sense if the
> rate is less than 5 packets. If nftables chooses a better burst (1 packet in
> this case), users don't need to know what the actual burst is. If they're
> into
> it, they will choose it themselves. It's enough to illustrate the algorithm
> in
> nft.8 in case someone wonders.

Having educated myself about how the Token Bucket algorithm actually works, I
no longer think automatic burst value setting is needed or feasible in the
first place:

The burst value defines the bucket's size, i.e. how many tokens it may hold.
The rate value then defines how fast the bucket fills up with tokens if empty.
With a paket-based token, each paket passing the nftables 'limit' statement
removes a token from the bucket. If the latter is empty, the paket is exceeding
the defined rate (in our case, the nftables statement will not match anymore).

So in order to validate the defined limit rate is met, one has to either set
burst to a single paket or "drain" the bucket before starting the measurement. Take this sample ruleset:

| table ip t {
| 	chain c {
| 		type filter hook output priority filter; policy accept;
| 		oif "test0" limit rate over 1/second drop
| 	}
| }

Pinging a machine via test0 interface then looks like this:

| $ ping -i 0.1 10.0.0.2
| PING 10.0.0.2 (10.0.0.2) 56(84) bytes of data.
| 64 bytes from 10.0.0.2: icmp_seq=1 ttl=64 time=0.526 ms
| 64 bytes from 10.0.0.2: icmp_seq=2 ttl=64 time=0.207 ms
| 64 bytes from 10.0.0.2: icmp_seq=3 ttl=64 time=0.080 ms
| 64 bytes from 10.0.0.2: icmp_seq=4 ttl=64 time=0.193 ms
| 64 bytes from 10.0.0.2: icmp_seq=5 ttl=64 time=0.075 ms
| 64 bytes from 10.0.0.2: icmp_seq=11 ttl=64 time=0.121 ms
| 64 bytes from 10.0.0.2: icmp_seq=20 ttl=64 time=0.123 ms
| 64 bytes from 10.0.0.2: icmp_seq=29 ttl=64 time=0.099 ms
| 64 bytes from 10.0.0.2: icmp_seq=38 ttl=64 time=0.342 ms
| 64 bytes from 10.0.0.2: icmp_seq=47 ttl=64 time=0.402 ms
| 64 bytes from 10.0.0.2: icmp_seq=56 ttl=64 time=0.135 ms
| 64 bytes from 10.0.0.2: icmp_seq=65 ttl=64 time=0.391 ms

So you see, the first five packets pass despite exceeding the defined limit of
1pkt/s and thereby empty the bucket. No new tokens are added until the second
has passed, so pakets six to ten are dropped. Paket eleven gets the single
token added, so following pakets are dropped again until the next token
arrives.

So in summary, I understand rate and burst are not related at all (apart from
the latter allowing temporary exceeding of the further) and there is no
sensible way to determine a good default burst value from a given rate value.