On 21 Apr 2021, at 15:48, Ilya Maximets wrote:
> Implementation of meters supposed to be a classic token bucket with 2
> typical parameters: rate and burst size.
>
> Burst size in this schema is the maximum number of bytes/packets that
> could pass without being rate limited.
>
> Recent changes to userspace datapath made meter implementation to be
> in line with the kernel one, and this uncovered several issues.
>
> The main problem is that maximum bucket size for unknown reason
> accounts not only burst size, but also the numerical value of rate.
> This creates a lot of confusion around behavior of meters.
>
> For example, if rate is configured as 1000 pps and burst size set to 1,
> this should mean that meter will tolerate bursts of 1 packet at most,
> i.e. not a single packet above the rate should pass the meter.
> However, current implementation calculates maximum bucket size as
> (rate + burst size), so the effective bucket size will be 1001. This
> means that first 1000 packets will not be rate limited and average
> rate might be twice as high as the configured rate. This also makes
> it practically impossible to configure meter that will have burst size
> lower than the rate, which might be a desirable configuration if the
> rate is high.
>
> Inability to configure low values of a burst size and overall inability
> for a user to predict what will be a maximum and average rate from the
> configured parameters of a meter without looking at the OVS and kernel
> code might be also classified as a security issue, because drop meters
> are frequently used as a way of protection from DoS attacks.
>
> This change removes rate from the calculation of a bucket size, making
> it in line with the classic token bucket algorithm and essentially
> making the rate and burst tolerance being predictable from a users'
> perspective.
>
> Same change will be proposed for the kernel implementation.
> Unit tests changed back to their correct version and enhanced.
>
> Signed-off-by: Ilya Maximets <[email protected]>
Reviewed and tested patch, looks good to me.
Acked-by: Eelco Chaudron <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
