Correct three bugs in the rate limiter.

 * We should be dividing by the packet size, not multiplying, as we
   should be crediting fewer packets for larger packets, not more.
 * We should handle the packet count underflowing as it may have
   been some time since the last packet.
 * The bucket limit needs to be `<= 0xd00`, not `< 0xf00`, as
   `0xf00` + `0x100` overflows our counter and resets the bucket.

diff --git a/xdp.c b/xdp.c
index 122a9eb30656dcf84e015fe22069d44b1b2758e2..860fe2536bc3723f19272a0e72e0ada00161deda 100644 (file)
--- a/xdp.c
+++ b/xdp.c
@@ -246,14 +246,18 @@ if (rate) { \
                /* than packets, we can keep counting packets and simply adjust the ratelimit by the*/ \
                /* size of the packet we're looking at. */ \
                /* Thus, here, we simply reduce our packet counter by the */ \
-               /* time difference / our ns/packet limit times the size of the current packet. */ \
-               int64_t pkts_since_last = (time_diff << RATE_BUCKET_BITS) * ((uint64_t)amt_in_pkt) / ((uint64_t)limit_ns_per_pkt); \
-               bucket_pkts -= pkts_since_last; \
+               /* time difference / (our ns/packet limit * the size of the current packet). */ \
+               /* We shift by RATE_BUCKET_DECIMAL_BITS first since we're calculating whole packets. */ \
+               int64_t pkts_allowed_since_last_update = \
+                       (time_diff << RATE_BUCKET_BITS) / (((uint64_t)amt_in_pkt) * ((uint64_t)limit_ns_per_pkt)); \
+               bucket_pkts -= pkts_allowed_since_last_update; \
        } \
        /* Accept as long as we can add one to our bucket without overflow */ \
-       if (bucket_pkts < (((1 << RATE_BUCKET_INTEGER_BITS) - 1) << RATE_BUCKET_DECIMAL_BITS)) { \
+       const int64_t MAX_PACKETS = (1 << RATE_BUCKET_INTEGER_BITS) - 2; \
+       if (bucket_pkts <= (MAX_PACKETS << RATE_BUCKET_DECIMAL_BITS)) { \
                if (unlikely(bucket_pkts < 0)) bucket_pkts = 0; \
-               uint64_t new_packet_count = bucket_pkts + (1 << RATE_BUCKET_DECIMAL_BITS); \
+               int64_t new_packet_count = bucket_pkts + (1 << RATE_BUCKET_DECIMAL_BITS); \
+               if (new_packet_count < 0) { new_packet_count = 0; } \
                rate->pkts_and_time = time_masked | (new_packet_count << (64 - RATE_BUCKET_BITS)); \
                matchbool = 0; \
        } else { \