| Message ID | 1503344333-9953-1-git-send-email-azhou@ovn.org |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On 21 August 2017 at 12:38, Andy Zhou <azhou@ovn.org> wrote: > Fixes: 96518518cc41 ("netfilter: add nftables") > > Current implementation treats the burst configuration the same as > rate configuration. This can cause the per packet cost to be lower > than configured. In effect, this bug causes the token bucket to be > refilled at a higher rate than what user has specified. > > This patch changes the implementation so that the token bucket size > is controlled by "rate + burst", while maintain the token bucket > refill rate the same as user specified. > > Signed-off-by: Andy Zhou <azhou@ovn.org> Usually "Fixes" tag appears immediately above the signoff lines. This is the bug that we brought up during NFWS this year in Faro, how the burst was not acting as a burst but rather it just added to the rate. Acked-by: Joe Stringer <joe@ovn.org> -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 21, 2017 at 02:29:13PM -0700, Joe Stringer wrote: > On 21 August 2017 at 12:38, Andy Zhou <azhou@ovn.org> wrote: > > Fixes: 96518518cc41 ("netfilter: add nftables") > > > > Current implementation treats the burst configuration the same as > > rate configuration. This can cause the per packet cost to be lower > > than configured. In effect, this bug causes the token bucket to be > > refilled at a higher rate than what user has specified. > > > > This patch changes the implementation so that the token bucket size > > is controlled by "rate + burst", while maintain the token bucket > > refill rate the same as user specified. > > > > Signed-off-by: Andy Zhou <azhou@ovn.org> > > Usually "Fixes" tag appears immediately above the signoff lines. > > This is the bug that we brought up during NFWS this year in Faro, how > the burst was not acting as a burst but rather it just added to the > rate. > > Acked-by: Joe Stringer <joe@ovn.org> Applied, thanks a lot for this fix. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nft_limit.c b/net/netfilter/nft_limit.c index 18dd57a52651..14538b1d4d11 100644 --- a/net/netfilter/nft_limit.c +++ b/net/netfilter/nft_limit.c @@ -65,19 +65,23 @@ static int nft_limit_init(struct nft_limit *limit, limit->nsecs = unit * NSEC_PER_SEC; if (limit->rate == 0 || limit->nsecs < unit) return -EOVERFLOW; - limit->tokens = limit->tokens_max = limit->nsecs; - - if (tb[NFTA_LIMIT_BURST]) { - u64 rate; + if (tb[NFTA_LIMIT_BURST]) limit->burst = ntohl(nla_get_be32(tb[NFTA_LIMIT_BURST])); + else + limit->burst = 0; + + if (limit->rate + limit->burst < limit->rate) + return -EOVERFLOW; - rate = limit->rate + limit->burst; - if (rate < limit->rate) - return -EOVERFLOW; + /* The token bucket size limits the number of tokens can be + * accumulated. tokens_max specifies the bucket size. + * tokens_max = unit * (rate + burst) / rate. + */ + limit->tokens = div_u64(limit->nsecs * (limit->rate + limit->burst), + limit->rate); + limit->tokens_max = limit->tokens; - limit->rate = rate; - } if (tb[NFTA_LIMIT_FLAGS]) { u32 flags = ntohl(nla_get_be32(tb[NFTA_LIMIT_FLAGS])); @@ -95,9 +99,8 @@ static int nft_limit_dump(struct sk_buff *skb, const struct nft_limit *limit, { u32 flags = limit->invert ? NFT_LIMIT_F_INV : 0; u64 secs = div_u64(limit->nsecs, NSEC_PER_SEC); - u64 rate = limit->rate - limit->burst; - if (nla_put_be64(skb, NFTA_LIMIT_RATE, cpu_to_be64(rate), + if (nla_put_be64(skb, NFTA_LIMIT_RATE, cpu_to_be64(limit->rate), NFTA_LIMIT_PAD) || nla_put_be64(skb, NFTA_LIMIT_UNIT, cpu_to_be64(secs), NFTA_LIMIT_PAD) ||
Fixes: 96518518cc41 ("netfilter: add nftables") Current implementation treats the burst configuration the same as rate configuration. This can cause the per packet cost to be lower than configured. In effect, this bug causes the token bucket to be refilled at a higher rate than what user has specified. This patch changes the implementation so that the token bucket size is controlled by "rate + burst", while maintain the token bucket refill rate the same as user specified. Signed-off-by: Andy Zhou <azhou@ovn.org> --- net/netfilter/nft_limit.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)