Page MenuHomeFreeBSD

icmp6: make icmp6_ratelimit() responsible to update the stats counter
ClosedPublic

Authored by glebius on Mar 22 2024, 9:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 21, 3:35 AM
Unknown Object (File)
Sat, Oct 19, 10:46 AM
Unknown Object (File)
Sat, Oct 19, 12:58 AM
Unknown Object (File)
Sat, Oct 19, 12:58 AM
Unknown Object (File)
Fri, Oct 18, 11:11 PM
Unknown Object (File)
Oct 2 2024, 3:12 PM
Unknown Object (File)
Sep 27 2024, 10:07 AM
Unknown Object (File)
Sep 21 2024, 2:33 PM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Mar 23 2024, 2:22 PM
zlei added inline comments.
sys/netinet6/icmp6.c
2745

There are consumers that do not increase statistic icp6s_toofreq such as

  1. icmp6_redirect_output() in sys/netinet6/icmp6.c
  2. pf_send_icmp() in sys/netpfil/pf/pf.c
  3. nat64_icmp6_reflect() in sys/netpfil/ipfw/nat64/nat64_translate.c

I'm not sure those are intended or not but I like the current implementation of icmp6_ratelimit() as it is simple and follows the single responsibility principle.

sys/netinet6/icmp6.c
2745

I'm convinced that consumers that didn't update icp6s_toofreq will now be fixed. Every return of a positive value from icmp6_ratelimit() means a dropped (or suppressed) packet. Such packets shall be counted. The "too frequent" counter is the right and the only counter that fits this purpose. Here is how it is visible to a user with netstat(1):

inet6.c:        p(icp6s_toofreq, "\t{:errors-discarded-by-rate-limitation/%ju} "

We can create a separate counter to count non-errors (e.g. echo replies) separately. I won't be against that, so if anyone makes a review - I'd approve. However, I see little value in additional counter, so I won't do that myself. But not counting at all is definitely a problem and must be fixed.