Bug 150917 - icmp(4): Incorrect sysctls descriptions for icmplim and icmplim_output
Summary: icmp(4): Incorrect sysctls descriptions for icmplim and icmplim_output
Status: Closed FIXED
Alias: None
Product: Documentation
Classification: Unclassified
Component: Manual Pages (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Eitan Adler
URL:
Keywords:
Depends on: 163623
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-24 14:50 UTC by Nikos Vassiliadis
Modified: 2021-07-21 02:09 UTC (History)
3 users (show)

See Also:


Attachments
file.diff (782 bytes, patch)
2010-09-24 14:50 UTC, Nikos Vassiliadis
no flags Details | Diff
ip_icmp.c.patch (1.02 KB, patch)
2010-09-24 16:32 UTC, Nikos Vassiliadis
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikos Vassiliadis 2010-09-24 14:50:01 UTC
The icmp manual page describes icmplim and icmplim_output wrongly.

     icmplim	     (integer) Bandwidth limit for ICMP replies in pack-
		     ets/second.  Used when icmplim_output is non-zero.
		     Defaults to 200.

     icmplim_output  (boolean) Enable/disable bandwidth limiting of ICMP
		     replies.  Defaults to true.

The correct description [1] is this:
If you do not want to see messages about this in your log files, but you still want the kernel to do response limiting, you can use the net.inet.icmp.icmplim_output sysctl variable to disable the output like this:
# sysctl -w net.inet.icmp.icmplim_output=0

Finally, if you want to disable response limiting, you can set the net.inet.icmp.icmplim sysctl variable (see above for an example) to 0. Disabling response limiting is discouraged for the reasons listed above.

[1] http://www.freebsd.org/doc/en/books/faq/networking.html#ICMP-RESPONSE-BW-LIMIT

Fix: Use the attached patch

Patch attached with submission follows:
How-To-Repeat: n/a
Comment 1 Glen Barber freebsd_committer freebsd_triage 2010-09-24 14:58:14 UTC
Responsible Changed
From-To: freebsd-doc->gjb

I'll take this.
Comment 2 Nikos Vassiliadis 2010-09-24 16:32:18 UTC
Consider the following patch as well
Comment 3 Glen Barber freebsd_committer freebsd_triage 2010-10-01 12:51:59 UTC
Responsible Changed
From-To: gjb->freebsd-doc

Back to the pool for now due to lack of time.
Comment 4 Giorgos Keramidas 2011-02-15 07:47:18 UTC
On 2010-09-24 13:42, Nikos Vassiliadis <nvass9573@gmx.com> wrote:
> The icmp manual page describes icmplim and icmplim_output wrongly.
>
>      icmplim	     (integer) Bandwidth limit for ICMP replies in pack-
> 		     ets/second.  Used when icmplim_output is non-zero.
> 		     Defaults to 200.
>
>      icmplim_output  (boolean) Enable/disable bandwidth limiting of ICMP
> 		     replies.  Defaults to true.
>
> The correct description [1] is this:
>
> If you do not want to see messages about this in your log files, but
> you still want the kernel to do response limiting, you can use the
> net.inet.icmp.icmplim_output sysctl variable to disable the output
> like this:
>
> # sysctl -w net.inet.icmp.icmplim_output=0
>
> Finally, if you want to disable response limiting, you can set the
> net.inet.icmp.icmplim sysctl variable (see above for an example) to
> 0. Disabling response limiting is discouraged for the reasons listed
> above.

> Index: src/share/man/man4/icmp.4
> ===================================================================
> --- src/share/man/man4/icmp.4   (revision 213086)
> +++ src/share/man/man4/icmp.4   (working copy)
> @@ -179,15 +179,16 @@
>  the system replies to an ICMP Address Mask Request packet.
>  Defaults to 0.
>  .It Va icmplim
> -.Pq Vt integer
> -Bandwidth limit for ICMP replies in packets/second.
> -Used when
> -.Va icmplim_output
> -is non-zero.
> +.Pq Vt "unsigned integer"
> +Limit for ICMP or TCP RST responses in packets per second.
> +Response limiting is disabled by setting
> +.Va icmplim
> +to 0.

I am not sure I like the idea of mentioning all the other protocols that
may send ICMP replies too, so this probably needs a bit of reworking.
For instance, UDP may send ICMP unreachable errors too.  SCTP also.

Are we going to list *all* the possible protocols by name?

>  Defaults to 200.
>  .It Va icmplim_output
>  .Pq Vt boolean
> -Enable/disable bandwidth limiting of ICMP replies.
> +Enable/disable logging of
> +.Va icmplim .

"logging of rate-limiting messages from icmplim" ?

>  Index: src/sys/netinet/ip_icmp.c
>  ===================================================================
>  --- src/sys/netinet/ip_icmp.c	(revision 213086)
>  +++ src/sys/netinet/ip_icmp.c	(working copy)
>  @@ -106,15 +106,15 @@
>
>   static VNET_DEFINE(int, icmplim) = 200;
>   #define	V_icmplim			VNET(icmplim)
>  -SYSCTL_VNET_INT(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLFLAG_RW,
>  +SYSCTL_VNET_UINT(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLFLAG_RW,
>   	&VNET_NAME(icmplim), 0,
>  -	"Maximum number of ICMP responses per second");
>  +	"Maximum number of ICMP or TCP RST responses per second");
>
>   static VNET_DEFINE(int, icmplim_output) = 1;
>   #define	V_icmplim_output		VNET(icmplim_output)
>  -SYSCTL_VNET_INT(_net_inet_icmp, OID_AUTO, icmplim_output, CTLFLAG_RW,
>  +SYSCTL_VNET_UINT(_net_inet_icmp, OID_AUTO, icmplim_output, CTLFLAG_RW,
>   	&VNET_NAME(icmplim_output), 0,
>  -	"Enable rate limiting of ICMP responses");
>  +	"Enable logging of enforced limit on ICMP or TCP RST responses");

This also has the problem that it does not mention UDP or SCTP replies
for port-unreachable, host-unreachable, etc.
Comment 5 nvass 2011-02-15 12:27:47 UTC
On 2/15/2011 9:47 AM, Giorgos Keramidas wrote:
> On 2010-09-24 13:42, Nikos Vassiliadis<nvass9573@gmx.com>  wrote:
>> The icmp manual page describes icmplim and icmplim_output wrongly.
>>
>>       icmplim	     (integer) Bandwidth limit for ICMP replies in pack-
>> 		     ets/second.  Used when icmplim_output is non-zero.
>> 		     Defaults to 200.
>>
>>       icmplim_output  (boolean) Enable/disable bandwidth limiting of ICMP
>> 		     replies.  Defaults to true.
>>
>> The correct description [1] is this:
>>
>> If you do not want to see messages about this in your log files, but
>> you still want the kernel to do response limiting, you can use the
>> net.inet.icmp.icmplim_output sysctl variable to disable the output
>> like this:
>>
>> # sysctl -w net.inet.icmp.icmplim_output=0
>>
>> Finally, if you want to disable response limiting, you can set the
>> net.inet.icmp.icmplim sysctl variable (see above for an example) to
>> 0. Disabling response limiting is discouraged for the reasons listed
>> above.
>
>> Index: src/share/man/man4/icmp.4
>> ===================================================================
>> --- src/share/man/man4/icmp.4   (revision 213086)
>> +++ src/share/man/man4/icmp.4   (working copy)
>> @@ -179,15 +179,16 @@
>>   the system replies to an ICMP Address Mask Request packet.
>>   Defaults to 0.
>>   .It Va icmplim
>> -.Pq Vt integer
>> -Bandwidth limit for ICMP replies in packets/second.
>> -Used when
>> -.Va icmplim_output
>> -is non-zero.
>> +.Pq Vt "unsigned integer"
>> +Limit for ICMP or TCP RST responses in packets per second.
>> +Response limiting is disabled by setting
>> +.Va icmplim
>> +to 0.
>
> I am not sure I like the idea of mentioning all the other protocols that
> may send ICMP replies too, so this probably needs a bit of reworking.
> For instance, UDP may send ICMP unreachable errors too.  SCTP also.
>
> Are we going to list *all* the possible protocols by name?

We don't list the protocols that initiated the response.
As you mentioned, TCP|UDP|SCTP can initiate this rate-limiting function.
Yet, all possible replies that this code handles are either ICMP or TCP.
For example the current code does not handle SCTP ABORT, it *does* handle
the case of an ICMP response generated by SCTP.

The replies are defined here:
http://fxr.watson.org/fxr/source/netinet/icmp_var.h#L99

>
>>   Defaults to 200.
>>   .It Va icmplim_output
>>   .Pq Vt boolean
>> -Enable/disable bandwidth limiting of ICMP replies.
>> +Enable/disable logging of
>> +.Va icmplim .
>
> "logging of rate-limiting messages from icmplim" ?

Yes, that's better.

>
>>   Index: src/sys/netinet/ip_icmp.c
>>   ===================================================================
>>   --- src/sys/netinet/ip_icmp.c	(revision 213086)
>>   +++ src/sys/netinet/ip_icmp.c	(working copy)
>>   @@ -106,15 +106,15 @@
>>
>>    static VNET_DEFINE(int, icmplim) = 200;
>>    #define	V_icmplim			VNET(icmplim)
>>   -SYSCTL_VNET_INT(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLFLAG_RW,
>>   +SYSCTL_VNET_UINT(_net_inet_icmp, ICMPCTL_ICMPLIM, icmplim, CTLFLAG_RW,
>>    	&VNET_NAME(icmplim), 0,
>>   -	"Maximum number of ICMP responses per second");
>>   +	"Maximum number of ICMP or TCP RST responses per second");
>>
>>    static VNET_DEFINE(int, icmplim_output) = 1;
>>    #define	V_icmplim_output		VNET(icmplim_output)
>>   -SYSCTL_VNET_INT(_net_inet_icmp, OID_AUTO, icmplim_output, CTLFLAG_RW,
>>   +SYSCTL_VNET_UINT(_net_inet_icmp, OID_AUTO, icmplim_output, CTLFLAG_RW,
>>    	&VNET_NAME(icmplim_output), 0,
>>   -	"Enable rate limiting of ICMP responses");
>>   +	"Enable logging of enforced limit on ICMP or TCP RST responses");
>
> This also has the problem that it does not mention UDP or SCTP replies
> for port-unreachable, host-unreachable, etc.

These are ICMP messages generated by UDP|SCTP. But... the sysctl's 
description
doesn't have to be that accurate, if you ask me. I would like the manual 
page
to be as accurate as possible. I find particularly unattractive the term
bandwidth there!

Thanks for handling this, Nikos
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:01 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-19 03:32:02 UTC
^Triage: Re-triage
Comment 8 Eugene Grosbein freebsd_committer freebsd_triage 2021-07-20 05:13:14 UTC
Fixed 6 years ago with r280688.
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-21 02:09:04 UTC
^Triage: Assign to committer that resolved

Thanks Eugene!