feature: rate limit

[SkohTV]

Fix #215
Currently a WIP POC

I've started getting into adding this features.
So far, I've managed to add the ratelimit [int]/[time_unit] keyword to the cli and put it's value into a bf_map.
However I'm not sure about implementing a "real" rate limiting as it seems to require EMIT (and that look scary).
Added (empty for now) elfstub to handle rate limiting

The idea of the implementation so far is pretty naive (only allow the first X requests to pass through), mostly because it was easier to implement and I hadn't strong feelings on which direction to go.

I'll keep this as a draft for now since it's still in its early stages.

[qdeslandes]

I went for an early review, mostly for the parsing part. On the BPF side, the map will contain the runtime values for a given rule's rate limit (current burst/allowance, limit, last update time...).

[SkohTV]

Here is a working POC
Not going to far, to make it easier to change/correct things if required

Things done:

• Added ratelimit XX/XX (ex: ratelimit 5/sec) to bfcli
• Created a map to store current (packets that passed in this unit of time) and last_time (last packet timestamp)
• Added an elfstub (and a bunch of emits) to allow only X packets to be ACCEPTED, packets over the rate limit will be DROPPED

Things remaining:

• Make the unit (sec) actually be used (atm we only do it per second)
• Handle cli syntax better (errors on negative or wrong syntax)

Testing:

$ sudo ./result/sbin/bpfilter --transient --verbose=bpf

$ sudo ./result/sbin/bfcli chain set --from-str \
	'chain my_chain BF_HOOK_XDP{ifindex=2} ACCEPT
	rule ip4.proto icmp log internet ratelimit 5/sec ACCEPT'

$ ping 8.8.8.8 -c 10 -i 0.02
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=120 time=24.7 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=120 time=18.9 ms
64 bytes from 8.8.8.8: icmp_seq=3 ttl=120 time=14.4 ms
64 bytes from 8.8.8.8: icmp_seq=4 ttl=120 time=17.1 ms
64 bytes from 8.8.8.8: icmp_seq=5 ttl=120 time=13.5 ms

--- 8.8.8.8 ping statistics ---
10 packets transmitted, 5 received, 50% packet loss, time 184ms
rtt min/avg/max/mdev = 13.462/17.706/24.692/3.982 ms, pipe 2

There's a bunch of doc to do and leftover comments to remove, but that should be enough to swi(t)ch this PR from draft to "ready-for-review" now that the feature is working !

[qdeslandes]

Variables are defined at the beginning of the scope.

[SkohTV]

Like this ?

if (rule->ratelimit) {
    EMIT_LOAD_RATELIMIT_FD_FIXUP(program, BPF_REG_1);
    EMIT(program, BPF_MOV32_IMM(BPF_REG_2, rule->index));
    EMIT(program, BPF_MOV32_IMM(BPF_REG_3, rule->ratelimit));
    EMIT_FIXUP_ELFSTUB(program, BF_ELFSTUB_RATELIMIT);

+   {
        _clean_bf_jmpctx_ struct bf_jmpctx ctx =
            bf_jmpctx_get(program, BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 0));

        EMIT(program,
             BPF_MOV64_IMM(BPF_REG_0,
                           program->runtime.ops->get_verdict(BF_VERDICT_DROP)));
        EMIT(program, BPF_EXIT_INSN());
+   }
}

[qdeslandes]

We need to think this through. With the current implementation: if the rate limit is reached, all extra packet matching the rule are dropped. What if the rule already drops the packet? Then you drop all the matching packets, meaning the rate limit actually matches because the outcome is the same.

Currently, actions (mark, log, counter) don't have any impact on whether the rule matches or not. This one does. Would this make more sense as a matcher instead? It would allow for negative matches:

• rule meta.ratelimit 4pkt/s ACCEPT: if the packet is within the limit, accept it
• rule meta.ratelimit not 4pkt/s DROP: packets within the limit are ignored, while packets above are matched and dropped (not or another keyword that fits better).

Seems like this would be more in line with nftables. What do you think? I'm open to alternatives here.

[SkohTV]

I'm not super familiar with nftables, but I really like the idea of "negative matches", I think it makes rate limiting very versatile and nice to use
Changing it to a matcher seems to make more sense