counter(9): rate limit periods may be more than 1 second
ClosedPublic
Actions

Authored by kp on Wed, Jun 11, 7:40 PM.

Details

Reviewers

glebius
jtl

Group Reviewers

pfsense

Commits

rG1cd5c35d136e: counter(9): rate limit periods may be more than 1 second

Summary

Teach counter_rate() to deal with periods of more than 1 second, so we can
express 'at most 100 in 10 seconds', which is different from 'at most 10 in
1 second'.
While here move the struct counter_rate definition into subr_counter.c so users
cannot mess with its internals. Add allocation and free functions.

Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 64976
Build 61859: arc lint + arc unit

Event Timeline

kp created this revision.Wed, Jun 11, 7:40 PM

Herald added subscribers: olce, glebius, melifaro and 2 others. · View Herald TranscriptWed, Jun 11, 7:40 PM

kp requested review of this revision.Wed, Jun 11, 7:40 PM

Harbormaster completed remote builds in B64764: Diff 156836.Wed, Jun 11, 7:40 PM

kp added a child revision: D50797: pf: use counter_rate() for rate checking.Wed, Jun 11, 7:40 PM

May I ask to wait for my review until next week?

In D50796#1159287, @glebius wrote:

May I ask to wait for my review until next week?

Sure. This isn't particularly urgent. It's a preparatory commit for the max-pkt-rate feature in pf.

zlei added a subscriber: zlei.Fri, Jun 13, 10:01 AM

I totally agree with the period feature. Thanks!

I'm not sure about making the structure opaque and thus only dynamically allocated. In general I prefer hiding internals, but for some reason when we did this, we decided to make it exposed. There definitely was some reason for that, otherwise I would make it opaque. Adding Jonathan, maybe he can recall.

Manual page counter.9 also needs to be updated.

man page

Herald added a subscriber: ziaee. · View Herald TranscriptThu, Jun 19, 8:46 AM

Harbormaster completed remote builds in B64976: Diff 157302.Thu, Jun 19, 8:46 AM

In D50796#1162059, @glebius wrote:

I'm not sure about making the structure opaque and thus only dynamically allocated. In general I prefer hiding internals, but for some reason when we did this, we decided to make it exposed. There definitely was some reason for that, otherwise I would make it opaque. Adding Jonathan, maybe he can recall.

Given that we already had to counter_u64_alloc() I don't think this actually makes a difference.

Unless Jonathan remembers something I'd like to go with this version.