Date: Wed, 25 Jun 2025 17:56:37 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 1cd5c35d136e - main - counter(9): rate limit periods may be more than 1 second Message-ID: <202506251756.55PHubhT035284@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=1cd5c35d136e2c8605b9ba23e6a5879539411947 commit 1cd5c35d136e2c8605b9ba23e6a5879539411947 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-06-03 07:07:14 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-06-25 17:56:22 +0000 counter(9): rate limit periods may be more than 1 second 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. Reviewed by: glebius Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D50796 --- share/man/man9/counter.9 | 27 +++++++++++++++++++---- sys/kern/subr_counter.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++-- sys/netinet/ip_icmp.c | 9 ++++---- sys/netinet6/icmp6.c | 9 ++++---- sys/sys/counter.h | 13 ++++------- 5 files changed, 90 insertions(+), 25 deletions(-) diff --git a/share/man/man9/counter.9 b/share/man/man9/counter.9 index 1d3f3281ac0b..05af87e8263e 100644 --- a/share/man/man9/counter.9 +++ b/share/man/man9/counter.9 @@ -23,7 +23,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd March 11, 2021 +.Dd June 19, 2025 .Dt COUNTER 9 .Os .Sh NAME @@ -49,8 +49,14 @@ .Fn counter_u64_fetch "counter_u64_t c" .Ft void .Fn counter_u64_zero "counter_u64_t c" +.Ft struct counter_rate * +.Fn counter_rate_alloc "int flags" "int period" .Ft int64_t .Fn counter_ratecheck "struct counter_rate *cr" "int64_t limit" +.Ft uint64_t +.Fn counter_rate_get "struct counter_rate *cr" +.Ft void +.Fn counter_rate_free "struct counter_rate *cr" .Fn COUNTER_U64_SYSINIT "counter_u64_t c" .Fn COUNTER_U64_DEFINE_EARLY "counter_u64_t c" .In sys/sysctl.h @@ -133,6 +139,13 @@ value for any moment. Clear the counter .Fa c and set it to zero. +.It Fn counter_rate_alloc flags period +Allocate a new struct counter_rate. +.Fa flags +is passed to +.Xr malloc 9 . +.Fa period +is the time over which the rate is checked. .It Fn counter_ratecheck cr limit The function is a multiprocessor-friendly version of .Fn ppsratecheck @@ -140,11 +153,17 @@ which uses .Nm internally. Returns non-negative value if the rate is not yet reached during the current -second, and a negative value otherwise. -If the limit was reached on previous second, but was just reset back to zero, -then +period, and a negative value otherwise. +If the limit was reached during the previous period, but was just reset back +to zero, then .Fn counter_ratecheck returns number of events since previous reset. +.It Fn counter_rate_get cr +The number of hits to this check within the current period. +.It Fn counter_rate_free cr +Free the +.Fa cr +counter. .It Fn COUNTER_U64_SYSINIT c Define a .Xr SYSINIT 9 diff --git a/sys/kern/subr_counter.c b/sys/kern/subr_counter.c index 2cb987cb67f9..b629abc99315 100644 --- a/sys/kern/subr_counter.c +++ b/sys/kern/subr_counter.c @@ -40,6 +40,8 @@ #define IN_SUBR_COUNTER_C #include <sys/counter.h> +static MALLOC_DEFINE(M_COUNTER_RATE, "counter_rate", "counter rate allocations"); + void counter_u64_zero(counter_u64_t c) { @@ -114,6 +116,57 @@ sysctl_handle_counter_u64_array(SYSCTL_HANDLER_ARGS) return (0); } +/* + * counter(9) based rate checking. + */ +struct counter_rate { + counter_u64_t cr_rate; /* Events since last second */ + volatile int cr_lock; /* Lock to clean the struct */ + int cr_ticks; /* Ticks on last clean */ + int cr_over; /* Over limit since cr_ticks? */ + int cr_period; /* Allow cr_rate per cr_period seconds. */ +}; + +struct counter_rate * +counter_rate_alloc(int flags, int period) +{ + struct counter_rate *new; + + new = malloc(sizeof(struct counter_rate), M_COUNTER_RATE, + flags | M_ZERO); + if (new == NULL) + return (NULL); + + new->cr_rate = counter_u64_alloc(flags); + if (new->cr_rate == NULL) { + free(new, M_COUNTER_RATE); + return (NULL); + } + new->cr_ticks = ticks; + new->cr_period = period; + + return (new); +} + +void +counter_rate_free(struct counter_rate *rate) +{ + if (rate == NULL) + return; + + counter_u64_free(rate->cr_rate); + free(rate, M_COUNTER_RATE); +} + +uint64_t +counter_rate_get(struct counter_rate *cr) +{ + if (cr->cr_ticks < (tick - (hz * cr->cr_period))) + return (0); + + return (counter_u64_fetch(cr->cr_rate)); +} + /* * MP-friendly version of ppsratecheck(). * @@ -132,7 +185,7 @@ counter_ratecheck(struct counter_rate *cr, int64_t limit) val = cr->cr_over; now = ticks; - if ((u_int)(now - cr->cr_ticks) >= hz) { + if ((u_int)(now - cr->cr_ticks) >= (hz * cr->cr_period)) { /* * Time to clear the structure, we are in the next second. * First try unlocked read, and then proceed with atomic. @@ -143,7 +196,7 @@ counter_ratecheck(struct counter_rate *cr, int64_t limit) * Check if other thread has just went through the * reset sequence before us. */ - if ((u_int)(now - cr->cr_ticks) >= hz) { + if ((u_int)(now - cr->cr_ticks) >= (hz * cr->cr_period)) { val = counter_u64_fetch(cr->cr_rate); counter_u64_zero(cr->cr_rate); cr->cr_over = 0; diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c index 17d15d7d9629..cb4b6df57c57 100644 --- a/sys/netinet/ip_icmp.c +++ b/sys/netinet/ip_icmp.c @@ -1090,7 +1090,7 @@ ip_next_mtu(int mtu, int dir) * the 'final' error, but it doesn't make sense to solve the printing * delay with more complex code. */ -VNET_DEFINE_STATIC(struct counter_rate, icmp_rates[BANDLIM_MAX]); +VNET_DEFINE_STATIC(struct counter_rate *, icmp_rates[BANDLIM_MAX]); #define V_icmp_rates VNET(icmp_rates) static const char *icmp_rate_descrs[BANDLIM_MAX] = { @@ -1158,8 +1158,7 @@ icmp_bandlimit_init(void) { for (int i = 0; i < BANDLIM_MAX; i++) { - V_icmp_rates[i].cr_rate = counter_u64_alloc(M_WAITOK); - V_icmp_rates[i].cr_ticks = ticks; + V_icmp_rates[i] = counter_rate_alloc(M_WAITOK, 1); icmplim_new_jitter(i); } } @@ -1172,7 +1171,7 @@ icmp_bandlimit_uninit(void) { for (int i = 0; i < BANDLIM_MAX; i++) - counter_u64_free(V_icmp_rates[i].cr_rate); + counter_rate_free(V_icmp_rates[i]); } VNET_SYSUNINIT(icmp_bandlimit, SI_SUB_PROTO_DOMAIN, SI_ORDER_THIRD, icmp_bandlimit_uninit, NULL); @@ -1189,7 +1188,7 @@ badport_bandlim(int which) KASSERT(which >= 0 && which < BANDLIM_MAX, ("%s: which %d", __func__, which)); - pps = counter_ratecheck(&V_icmp_rates[which], V_icmplim + + pps = counter_ratecheck(V_icmp_rates[which], V_icmplim + V_icmplim_curr_jitter[which]); if (pps > 0) { if (V_icmplim_output) diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c index 9ea640fd24c8..d89515d7eda5 100644 --- a/sys/netinet6/icmp6.c +++ b/sys/netinet6/icmp6.c @@ -2844,7 +2844,7 @@ sysctl_icmp6lim_and_jitter(SYSCTL_HANDLER_ARGS) } -VNET_DEFINE_STATIC(struct counter_rate, icmp6_rates[RATELIM_MAX]); +VNET_DEFINE_STATIC(struct counter_rate *, icmp6_rates[RATELIM_MAX]); #define V_icmp6_rates VNET(icmp6_rates) static void @@ -2852,8 +2852,7 @@ icmp6_ratelimit_init(void) { for (int i = 0; i < RATELIM_MAX; i++) { - V_icmp6_rates[i].cr_rate = counter_u64_alloc(M_WAITOK); - V_icmp6_rates[i].cr_ticks = ticks; + V_icmp6_rates[i] = counter_rate_alloc(M_WAITOK, 1); icmp6lim_new_jitter(i); } } @@ -2866,7 +2865,7 @@ icmp6_ratelimit_uninit(void) { for (int i = 0; i < RATELIM_MAX; i++) - counter_u64_free(V_icmp6_rates[i].cr_rate); + counter_rate_free(V_icmp6_rates[i]); } VNET_SYSUNINIT(icmp6_ratelimit, SI_SUB_PROTO_DOMAIN, SI_ORDER_THIRD, icmp6_ratelimit_uninit, NULL); @@ -2916,7 +2915,7 @@ icmp6_ratelimit(const struct in6_addr *dst, const int type, const int code) break; }; - pps = counter_ratecheck(&V_icmp6_rates[which], V_icmp6errppslim + + pps = counter_ratecheck(V_icmp6_rates[which], V_icmp6errppslim + V_icmp6lim_curr_jitter[which]); if (pps > 0) { if (V_icmp6lim_output) diff --git a/sys/sys/counter.h b/sys/sys/counter.h index 65d6a54b47a6..e303fc4ecbc2 100644 --- a/sys/sys/counter.h +++ b/sys/sys/counter.h @@ -60,17 +60,12 @@ uint64_t counter_u64_fetch(counter_u64_t); counter_u64_zero((a)[_i]); \ } while (0) -/* - * counter(9) based rate checking. - */ -struct counter_rate { - counter_u64_t cr_rate; /* Events since last second */ - volatile int cr_lock; /* Lock to clean the struct */ - int cr_ticks; /* Ticks on last clean */ - int cr_over; /* Over limit since cr_ticks? */ -}; +struct counter_rate; +struct counter_rate *counter_rate_alloc(int flags, int period); +void counter_rate_free(struct counter_rate *); int64_t counter_ratecheck(struct counter_rate *, int64_t); +uint64_t counter_rate_get(struct counter_rate *); #define COUNTER_U64_SYSINIT(c) \ SYSINIT(c##_counter_sysinit, SI_SUB_COUNTER, \
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202506251756.55PHubhT035284>