linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] docs: net: sysctl documentation cleanup
  2025-06-12 16:29 [PATCH " Abdelrahman Fekry
@ 2025-06-14  9:25 ` Abdelrahman Fekry
  2025-06-14 17:46   ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-14  9:25 UTC (permalink / raw)
  To: corbet, davem, edumazet, horms, kuba, pabeni
  Cc: abdelrahmanfekry375, linux-doc, linux-kernel-mentees,
	linux-kernel, netdev, skhan

Changes in v2:
- Deleted space before colon for consistency
- Standardized more boolean representation (0/1 with enabled/disabled)

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
 Documentation/networking/ip-sysctl.rst | 47 ++++++++++++++++++++------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 0f1251cce314..68778532faa5 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -8,14 +8,16 @@ IP Sysctl
 ==============================
 
 ip_forward - BOOLEAN
-	- 0 - disabled (default)
-	- not 0 - enabled
+	- 0 (disabled)
+	- not 0 (enabled)
 
 	Forward Packets between interfaces.
 
 	This variable is special, its change resets all configuration
 	parameters to their default state (RFC1122 for hosts, RFC1812
 	for routers)
+
+	Default: 0 (disabled)
 
 ip_default_ttl - INTEGER
 	Default value of TTL field (Time To Live) for outgoing (but not
@@ -75,7 +77,7 @@ fwmark_reflect - BOOLEAN
 	If unset, these packets have a fwmark of zero. If set, they have the
 	fwmark of the packet they are replying to.
 
-	Default: 0
+	Default: 0 (disabled)
 
 fib_multipath_use_neigh - BOOLEAN
 	Use status of existing neighbor entry when determining nexthop for
@@ -368,7 +370,7 @@ tcp_autocorking - BOOLEAN
 	queue. Applications can still use TCP_CORK for optimal behavior
 	when they know how/when to uncork their sockets.
 
-	Default : 1
+	Default: 1 (enabled)
 
 tcp_available_congestion_control - STRING
 	Shows the available congestion control choices that are registered.
@@ -407,6 +409,12 @@ tcp_congestion_control - STRING
 
 tcp_dsack - BOOLEAN
 	Allows TCP to send "duplicate" SACKs.
+
+	Possible values:
+		- 0 (disabled)
+		- 1 (enabled)
+
+	Default: 1 (enabled)
 
 tcp_early_retrans - INTEGER
 	Tail loss probe (TLP) converts RTOs occurring due to tail
@@ -623,6 +631,8 @@ tcp_no_metrics_save - BOOLEAN
 	increases overall performance, but may sometimes cause performance
 	degradation.  If set, TCP will not cache metrics on closing
 	connections.
+
+	Default: 0 (disabled)
 
 tcp_no_ssthresh_metrics_save - BOOLEAN
 	Controls whether TCP saves ssthresh metrics in the route cache.
@@ -684,6 +694,8 @@ tcp_retrans_collapse - BOOLEAN
 	Bug-to-bug compatibility with some broken printers.
 	On retransmit try to send bigger packets to work around bugs in
 	certain TCP stacks.
+
+	Default: 1 (enabled)
 
 tcp_retries1 - INTEGER
 	This value influences the time, after which TCP decides, that
@@ -739,6 +751,8 @@ tcp_rmem - vector of 3 INTEGERs: min, default, max
 
 tcp_sack - BOOLEAN
 	Enable select acknowledgments (SACKS).
+
+	Default: 1 (enabled)
 
 tcp_comp_sack_delay_ns - LONG INTEGER
 	TCP tries to reduce number of SACK sent, using a timer
@@ -766,7 +780,7 @@ tcp_backlog_ack_defer - BOOLEAN
 	one ACK for the whole queue. This helps to avoid potential
 	long latencies at end of a TCP socket syscall.
 
-	Default : true
+	Default: 1 (enabled)
 
 tcp_slow_start_after_idle - BOOLEAN
 	If set, provide RFC2861 behavior and time out the congestion
@@ -781,7 +795,7 @@ tcp_stdurg - BOOLEAN
 	Most hosts use the older BSD interpretation, so if you turn this on
 	Linux might not communicate correctly with them.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_synack_retries - INTEGER
 	Number of times SYNACKs for a passive TCP connection attempt will
@@ -1018,6 +1032,10 @@ tcp_tw_reuse_delay - UNSIGNED INTEGER
 
 tcp_window_scaling - BOOLEAN
 	Enable window scaling as defined in RFC1323.
+	- 0 (disabled)
+	- 1 (enabled)
+
+	Default: 1 (enabled)
 
 tcp_shrink_window - BOOLEAN
 	This changes how the TCP receive window is calculated.
@@ -1160,7 +1178,7 @@ tcp_plb_enabled - BOOLEAN
 	congestion measure (e.g. ce_ratio). PLB needs a congestion measure to
 	make repathing decisions.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_plb_idle_rehash_rounds - INTEGER
 	Number of consecutive congested rounds (RTT) seen after which
@@ -1352,7 +1370,7 @@ cipso_rbm_optfmt - BOOLEAN
 
 	Default: 0
 
-cipso_rbm_structvalid - BOOLEAN
+cipso_rbm_strictvalid - BOOLEAN
 	If set, do a very strict check of the CIPSO option when
 	ip_options_compile() is called.  If unset, relax the checks done during
 	ip_options_compile().  Either way is "safe" as errors are caught else
@@ -1543,7 +1561,7 @@ icmp_ignore_bogus_error_responses - BOOLEAN
 	If this is set to TRUE, the kernel will not give such warnings, which
 	will avoid log file clutter.
 
-	Default: 1
+	Default: 1 (enabled)
 
 icmp_errors_use_inbound_ifaddr - BOOLEAN
 
@@ -1560,7 +1578,7 @@ icmp_errors_use_inbound_ifaddr - BOOLEAN
 	then the primary address of the first non-loopback interface that
 	has one will be used regardless of this setting.
 
-	Default: 0
+	Default: 0 (disabled)
 
 igmp_max_memberships - INTEGER
 	Change the maximum number of multicast groups we can subscribe to.
@@ -1933,10 +1951,15 @@ mcast_resolicit - INTEGER
 
 disable_policy - BOOLEAN
 	Disable IPSEC policy (SPD) for this interface
+
+	Default: 0
+
 
 disable_xfrm - BOOLEAN
 	Disable IPSEC encryption on this interface, whatever the policy
 
+	Default: 0
+
 igmpv2_unsolicited_report_interval - INTEGER
 	The interval in milliseconds in which the next unsolicited
 	IGMPv1 or IGMPv2 report retransmit will take place.
@@ -1951,11 +1974,15 @@ igmpv3_unsolicited_report_interval - INTEGER
 
 ignore_routes_with_linkdown - BOOLEAN
         Ignore routes whose link is down when performing a FIB lookup.
+
+        Default: 0 (disabled)
 
 promote_secondaries - BOOLEAN
 	When a primary IP address is removed from this interface
 	promote a corresponding secondary IP address instead of
 	removing all the corresponding secondary IP addresses.
+
+	Default: 0 (disabled)
 
 drop_unicast_in_l2_multicast - BOOLEAN
 	Drop any unicast IP packets that are received in link-layer
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] docs: net: sysctl documentation cleanup
@ 2025-06-14  9:33 Abdelrahman Fekry
  0 siblings, 0 replies; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-14  9:33 UTC (permalink / raw)
  To: corbet, davem, edumazet, horms, kuba, pabeni
  Cc: linux-doc, linux-kernel-mentees, linux-kernel, netdev, skhan,
	Abdelrahman Fekry

Changes in v2:
- Deleted space before colon for consistency
- Standardized more boolean representation (0/1 with enabled/disabled)

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
 Documentation/networking/ip-sysctl.rst | 47 ++++++++++++++++++++------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 0f1251cce314..68778532faa5 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -8,14 +8,16 @@ IP Sysctl
 ==============================
 
 ip_forward - BOOLEAN
-	- 0 - disabled (default)
-	- not 0 - enabled
+	- 0 (disabled)
+	- not 0 (enabled)
 
 	Forward Packets between interfaces.
 
 	This variable is special, its change resets all configuration
 	parameters to their default state (RFC1122 for hosts, RFC1812
 	for routers)
+
+	Default: 0 (disabled)
 
 ip_default_ttl - INTEGER
 	Default value of TTL field (Time To Live) for outgoing (but not
@@ -75,7 +77,7 @@ fwmark_reflect - BOOLEAN
 	If unset, these packets have a fwmark of zero. If set, they have the
 	fwmark of the packet they are replying to.
 
-	Default: 0
+	Default: 0 (disabled)
 
 fib_multipath_use_neigh - BOOLEAN
 	Use status of existing neighbor entry when determining nexthop for
@@ -368,7 +370,7 @@ tcp_autocorking - BOOLEAN
 	queue. Applications can still use TCP_CORK for optimal behavior
 	when they know how/when to uncork their sockets.
 
-	Default : 1
+	Default: 1 (enabled)
 
 tcp_available_congestion_control - STRING
 	Shows the available congestion control choices that are registered.
@@ -407,6 +409,12 @@ tcp_congestion_control - STRING
 
 tcp_dsack - BOOLEAN
 	Allows TCP to send "duplicate" SACKs.
+
+	Possible values:
+		- 0 (disabled)
+		- 1 (enabled)
+
+	Default: 1 (enabled)
 
 tcp_early_retrans - INTEGER
 	Tail loss probe (TLP) converts RTOs occurring due to tail
@@ -623,6 +631,8 @@ tcp_no_metrics_save - BOOLEAN
 	increases overall performance, but may sometimes cause performance
 	degradation.  If set, TCP will not cache metrics on closing
 	connections.
+
+	Default: 0 (disabled)
 
 tcp_no_ssthresh_metrics_save - BOOLEAN
 	Controls whether TCP saves ssthresh metrics in the route cache.
@@ -684,6 +694,8 @@ tcp_retrans_collapse - BOOLEAN
 	Bug-to-bug compatibility with some broken printers.
 	On retransmit try to send bigger packets to work around bugs in
 	certain TCP stacks.
+
+	Default: 1 (enabled)
 
 tcp_retries1 - INTEGER
 	This value influences the time, after which TCP decides, that
@@ -739,6 +751,8 @@ tcp_rmem - vector of 3 INTEGERs: min, default, max
 
 tcp_sack - BOOLEAN
 	Enable select acknowledgments (SACKS).
+
+	Default: 1 (enabled)
 
 tcp_comp_sack_delay_ns - LONG INTEGER
 	TCP tries to reduce number of SACK sent, using a timer
@@ -766,7 +780,7 @@ tcp_backlog_ack_defer - BOOLEAN
 	one ACK for the whole queue. This helps to avoid potential
 	long latencies at end of a TCP socket syscall.
 
-	Default : true
+	Default: 1 (enabled)
 
 tcp_slow_start_after_idle - BOOLEAN
 	If set, provide RFC2861 behavior and time out the congestion
@@ -781,7 +795,7 @@ tcp_stdurg - BOOLEAN
 	Most hosts use the older BSD interpretation, so if you turn this on
 	Linux might not communicate correctly with them.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_synack_retries - INTEGER
 	Number of times SYNACKs for a passive TCP connection attempt will
@@ -1018,6 +1032,10 @@ tcp_tw_reuse_delay - UNSIGNED INTEGER
 
 tcp_window_scaling - BOOLEAN
 	Enable window scaling as defined in RFC1323.
+	- 0 (disabled)
+	- 1 (enabled)
+
+	Default: 1 (enabled)
 
 tcp_shrink_window - BOOLEAN
 	This changes how the TCP receive window is calculated.
@@ -1160,7 +1178,7 @@ tcp_plb_enabled - BOOLEAN
 	congestion measure (e.g. ce_ratio). PLB needs a congestion measure to
 	make repathing decisions.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_plb_idle_rehash_rounds - INTEGER
 	Number of consecutive congested rounds (RTT) seen after which
@@ -1352,7 +1370,7 @@ cipso_rbm_optfmt - BOOLEAN
 
 	Default: 0
 
-cipso_rbm_structvalid - BOOLEAN
+cipso_rbm_strictvalid - BOOLEAN
 	If set, do a very strict check of the CIPSO option when
 	ip_options_compile() is called.  If unset, relax the checks done during
 	ip_options_compile().  Either way is "safe" as errors are caught else
@@ -1543,7 +1561,7 @@ icmp_ignore_bogus_error_responses - BOOLEAN
 	If this is set to TRUE, the kernel will not give such warnings, which
 	will avoid log file clutter.
 
-	Default: 1
+	Default: 1 (enabled)
 
 icmp_errors_use_inbound_ifaddr - BOOLEAN
 
@@ -1560,7 +1578,7 @@ icmp_errors_use_inbound_ifaddr - BOOLEAN
 	then the primary address of the first non-loopback interface that
 	has one will be used regardless of this setting.
 
-	Default: 0
+	Default: 0 (disabled)
 
 igmp_max_memberships - INTEGER
 	Change the maximum number of multicast groups we can subscribe to.
@@ -1933,10 +1951,15 @@ mcast_resolicit - INTEGER
 
 disable_policy - BOOLEAN
 	Disable IPSEC policy (SPD) for this interface
+
+	Default: 0
+
 
 disable_xfrm - BOOLEAN
 	Disable IPSEC encryption on this interface, whatever the policy
 
+	Default: 0
+
 igmpv2_unsolicited_report_interval - INTEGER
 	The interval in milliseconds in which the next unsolicited
 	IGMPv1 or IGMPv2 report retransmit will take place.
@@ -1951,11 +1974,15 @@ igmpv3_unsolicited_report_interval - INTEGER
 
 ignore_routes_with_linkdown - BOOLEAN
         Ignore routes whose link is down when performing a FIB lookup.
+
+        Default: 0 (disabled)
 
 promote_secondaries - BOOLEAN
 	When a primary IP address is removed from this interface
 	promote a corresponding secondary IP address instead of
 	removing all the corresponding secondary IP addresses.
+
+	Default: 0 (disabled)
 
 drop_unicast_in_l2_multicast - BOOLEAN
 	Drop any unicast IP packets that are received in link-layer
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] docs: net: sysctl documentation cleanup
  2025-06-14  9:25 ` [PATCH v2 " Abdelrahman Fekry
@ 2025-06-14 17:46   ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-06-14 17:46 UTC (permalink / raw)
  To: Abdelrahman Fekry
  Cc: corbet, davem, edumazet, horms, pabeni, linux-doc,
	linux-kernel-mentees, linux-kernel, netdev, skhan

On Sat, 14 Jun 2025 12:25:42 +0300 Abdelrahman Fekry wrote:
> Changes in v2:
> - Deleted space before colon for consistency
> - Standardized more boolean representation (0/1 with enabled/disabled)

You need to repost the entire series. Make sure you read:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
before you do.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 0/2] docs: net: sysctl documentation improvements
@ 2025-06-14 22:53 Abdelrahman Fekry
  2025-06-14 22:53 ` [PATCH v2 1/2] docs: net: sysctl documentation cleanup Abdelrahman Fekry
  2025-06-14 22:53 ` [PATCH v2 2/2] docs: net: clarify sysctl value constraints Abdelrahman Fekry
  0 siblings, 2 replies; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-14 22:53 UTC (permalink / raw)
  To: corbet, davem, edumazet, horms, kuba, pabeni
  Cc: linux-doc, linux-kernel-mentees, linux-kernel, netdev, skhan,
	jacob.e.keller, alok.a.tiwari, Abdelrahman Fekry

This series cleans up and clarifies networking sysctl documentation,
specially the boolean parameters:

Patch 1: Fixes typos, adds missing defaults, and standardizes 
	boolean representation.
         
Patch 2: Adds value constraint clarifications for parameters with u8
	implementation details , fixes typos.
	
In this version 2 , Modifications were made to be more consistent 
and more boolean representations are fixed.

---
v2:
- Standarize more boolean representation.
- Removed inconsistent white spaces.

v1: https://lore.kernel.org/all/20250612162954.55843-1-abdelrahmanfekry375@gmail.com/
- Fix typos
- Add missing defaults
- standarize boolean representation.
Abdelrahman Fekry (2):
  docs: net: sysctl documentation cleanup
  docs: net: clarify sysctl value constraints

 Documentation/networking/ip-sysctl.rst | 117 +++++++++++++++++++------
 1 file changed, 92 insertions(+), 25 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] docs: net: sysctl documentation cleanup
  2025-06-14 22:53 [PATCH v2 0/2] docs: net: sysctl documentation improvements Abdelrahman Fekry
@ 2025-06-14 22:53 ` Abdelrahman Fekry
  2025-06-17 18:31   ` Simon Horman
  2025-06-14 22:53 ` [PATCH v2 2/2] docs: net: clarify sysctl value constraints Abdelrahman Fekry
  1 sibling, 1 reply; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-14 22:53 UTC (permalink / raw)
  To: corbet, davem, edumazet, horms, kuba, pabeni
  Cc: linux-doc, linux-kernel-mentees, linux-kernel, netdev, skhan,
	jacob.e.keller, alok.a.tiwari, Abdelrahman Fekry

I noticed that some boolean parameters have missing default values
(enabled/disabled) in the documentation so i checked the initialization
functions to get their default values, also there was some inconsistency
in the representation. During the process , i stumbled upon a typo in
cipso_rbm_struct_valid instead of cipso_rbm_struct_valid.

Thanks for the review.

On Thu, 12 Jun 2025, Jacob Keller wrote:
> Would it make sense to use "0 (disabled)" and "1 (enabled)" with
> parenthesis for consistency with the default value?

Used as suggested.

On Fri, 13 Jun 2025, ALOK TIWARI wrote:
> for consistency
> remove extra space before colon
> Default: 1 (enabled)

Fixed. 

On Sat, 14 Jun 2025 10:46:29 -0700, Jakub Kicinski wrote:
> You need to repost the entire series. Make sure you read:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
> before you do.

Reposted the entire series, Thanks for you patiency.

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
v2:
- Deleted space before colon for consistency
- Standardized more boolean representation (0/1 with enabled/disabled)

v1: https://lore.kernel.org/all/20250612162954.55843-2-abdelrahmanfekry375@gmail.com/
- Fixed typo in cipso_rbm_struct_valid
- Added missing default value declarations
- Standardized boolean representation (0/1 with enabled/disabled)
 Documentation/networking/ip-sysctl.rst | 47 ++++++++++++++++++++------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 0f1251cce314..68778532faa5 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -8,14 +8,16 @@ IP Sysctl
 ==============================
 
 ip_forward - BOOLEAN
-	- 0 - disabled (default)
-	- not 0 - enabled
+	- 0 (disabled)
+	- not 0 (enabled)
 
 	Forward Packets between interfaces.
 
 	This variable is special, its change resets all configuration
 	parameters to their default state (RFC1122 for hosts, RFC1812
 	for routers)
+
+	Default: 0 (disabled)
 
 ip_default_ttl - INTEGER
 	Default value of TTL field (Time To Live) for outgoing (but not
@@ -75,7 +77,7 @@ fwmark_reflect - BOOLEAN
 	If unset, these packets have a fwmark of zero. If set, they have the
 	fwmark of the packet they are replying to.
 
-	Default: 0
+	Default: 0 (disabled)
 
 fib_multipath_use_neigh - BOOLEAN
 	Use status of existing neighbor entry when determining nexthop for
@@ -368,7 +370,7 @@ tcp_autocorking - BOOLEAN
 	queue. Applications can still use TCP_CORK for optimal behavior
 	when they know how/when to uncork their sockets.
 
-	Default : 1
+	Default: 1 (enabled)
 
 tcp_available_congestion_control - STRING
 	Shows the available congestion control choices that are registered.
@@ -407,6 +409,12 @@ tcp_congestion_control - STRING
 
 tcp_dsack - BOOLEAN
 	Allows TCP to send "duplicate" SACKs.
+
+	Possible values:
+		- 0 (disabled)
+		- 1 (enabled)
+
+	Default: 1 (enabled)
 
 tcp_early_retrans - INTEGER
 	Tail loss probe (TLP) converts RTOs occurring due to tail
@@ -623,6 +631,8 @@ tcp_no_metrics_save - BOOLEAN
 	increases overall performance, but may sometimes cause performance
 	degradation.  If set, TCP will not cache metrics on closing
 	connections.
+
+	Default: 0 (disabled)
 
 tcp_no_ssthresh_metrics_save - BOOLEAN
 	Controls whether TCP saves ssthresh metrics in the route cache.
@@ -684,6 +694,8 @@ tcp_retrans_collapse - BOOLEAN
 	Bug-to-bug compatibility with some broken printers.
 	On retransmit try to send bigger packets to work around bugs in
 	certain TCP stacks.
+
+	Default: 1 (enabled)
 
 tcp_retries1 - INTEGER
 	This value influences the time, after which TCP decides, that
@@ -739,6 +751,8 @@ tcp_rmem - vector of 3 INTEGERs: min, default, max
 
 tcp_sack - BOOLEAN
 	Enable select acknowledgments (SACKS).
+
+	Default: 1 (enabled)
 
 tcp_comp_sack_delay_ns - LONG INTEGER
 	TCP tries to reduce number of SACK sent, using a timer
@@ -766,7 +780,7 @@ tcp_backlog_ack_defer - BOOLEAN
 	one ACK for the whole queue. This helps to avoid potential
 	long latencies at end of a TCP socket syscall.
 
-	Default : true
+	Default: 1 (enabled)
 
 tcp_slow_start_after_idle - BOOLEAN
 	If set, provide RFC2861 behavior and time out the congestion
@@ -781,7 +795,7 @@ tcp_stdurg - BOOLEAN
 	Most hosts use the older BSD interpretation, so if you turn this on
 	Linux might not communicate correctly with them.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_synack_retries - INTEGER
 	Number of times SYNACKs for a passive TCP connection attempt will
@@ -1018,6 +1032,10 @@ tcp_tw_reuse_delay - UNSIGNED INTEGER
 
 tcp_window_scaling - BOOLEAN
 	Enable window scaling as defined in RFC1323.
+	- 0 (disabled).
+	- 1 (enabled).
+
+	Default: 1 (enabled)
 
 tcp_shrink_window - BOOLEAN
 	This changes how the TCP receive window is calculated.
@@ -1160,7 +1178,7 @@ tcp_plb_enabled - BOOLEAN
 	congestion measure (e.g. ce_ratio). PLB needs a congestion measure to
 	make repathing decisions.
 
-	Default: FALSE
+	Default: 0 (disabled)
 
 tcp_plb_idle_rehash_rounds - INTEGER
 	Number of consecutive congested rounds (RTT) seen after which
@@ -1352,7 +1370,7 @@ cipso_rbm_optfmt - BOOLEAN
 
 	Default: 0
 
-cipso_rbm_structvalid - BOOLEAN
+cipso_rbm_strictvalid - BOOLEAN
 	If set, do a very strict check of the CIPSO option when
 	ip_options_compile() is called.  If unset, relax the checks done during
 	ip_options_compile().  Either way is "safe" as errors are caught else
@@ -1543,7 +1561,7 @@ icmp_ignore_bogus_error_responses - BOOLEAN
 	If this is set to TRUE, the kernel will not give such warnings, which
 	will avoid log file clutter.
 
-	Default: 1
+	Default: 1 (enabled)
 
 icmp_errors_use_inbound_ifaddr - BOOLEAN
 
@@ -1560,7 +1578,7 @@ icmp_errors_use_inbound_ifaddr - BOOLEAN
 	then the primary address of the first non-loopback interface that
 	has one will be used regardless of this setting.
 
-	Default: 0
+	Default: 0 (disabled)
 
 igmp_max_memberships - INTEGER
 	Change the maximum number of multicast groups we can subscribe to.
@@ -1933,10 +1951,15 @@ mcast_resolicit - INTEGER
 
 disable_policy - BOOLEAN
 	Disable IPSEC policy (SPD) for this interface
+
+	Default: 0
+
 
 disable_xfrm - BOOLEAN
 	Disable IPSEC encryption on this interface, whatever the policy
 
+	Default: 0
+
 igmpv2_unsolicited_report_interval - INTEGER
 	The interval in milliseconds in which the next unsolicited
 	IGMPv1 or IGMPv2 report retransmit will take place.
@@ -1951,11 +1974,15 @@ igmpv3_unsolicited_report_interval - INTEGER
 
 ignore_routes_with_linkdown - BOOLEAN
         Ignore routes whose link is down when performing a FIB lookup.
+
+        Default: 0 (disabled)
 
 promote_secondaries - BOOLEAN
 	When a primary IP address is removed from this interface
 	promote a corresponding secondary IP address instead of
 	removing all the corresponding secondary IP addresses.
+
+	Default: 0 (disabled)
 
 drop_unicast_in_l2_multicast - BOOLEAN
 	Drop any unicast IP packets that are received in link-layer
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] docs: net: clarify sysctl value constraints
  2025-06-14 22:53 [PATCH v2 0/2] docs: net: sysctl documentation improvements Abdelrahman Fekry
  2025-06-14 22:53 ` [PATCH v2 1/2] docs: net: sysctl documentation cleanup Abdelrahman Fekry
@ 2025-06-14 22:53 ` Abdelrahman Fekry
  2025-06-17 18:12   ` Simon Horman
  1 sibling, 1 reply; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-14 22:53 UTC (permalink / raw)
  To: corbet, davem, edumazet, horms, kuba, pabeni
  Cc: linux-doc, linux-kernel-mentees, linux-kernel, netdev, skhan,
	jacob.e.keller, alok.a.tiwari, Abdelrahman Fekry

So, i also noticed that some of the parameters represented
as boolean have no value constrain checks and accept integer
values due to u8 implementation, so i wrote a note for every
boolean parameter that have no constrain checks in code. and
fixed a typo in fmwark instead of fwmark.

Added notes for 19 confirmed parameters,
Verified by code inspection and runtime testing.

- No changes for v2 in this patch , still waiting to be reviewed.
Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
v2:
- No change.
v1:
- Added notes for booleans that accept 0-255 not only 0/1.
 Documentation/networking/ip-sysctl.rst | 70 ++++++++++++++++++++------
 1 file changed, 55 insertions(+), 15 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 68778532faa5..38f2981290d6 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -70,6 +70,8 @@ ip_forward_use_pmtu - BOOLEAN
 
 	- 0 - disabled
 	- 1 - enabled
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 fwmark_reflect - BOOLEAN
 	Controls the fwmark of kernel-generated IPv4 reply packets that are not
@@ -91,6 +93,8 @@ fib_multipath_use_neigh - BOOLEAN
 
 	- 0 - disabled
 	- 1 - enabled
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 fib_multipath_hash_policy - INTEGER
 	Controls which hash policy to use for multipath routes. Only valid
@@ -490,7 +494,9 @@ tcp_fwmark_accept - BOOLEAN
 	have a fwmark set via setsockopt(SOL_SOCKET, SO_MARK, ...) are
 	unaffected.
 
-	Default: 0
+	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_invalid_ratelimit - INTEGER
 	Limit the maximal rate for sending duplicate acknowledgments
@@ -605,6 +611,8 @@ tcp_moderate_rcvbuf - BOOLEAN
 	automatically size the buffer (no greater than tcp_rmem[2]) to
 	match the size required by the path for full throughput.  Enabled by
 	default.
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_mtu_probing - INTEGER
 	Controls TCP Packetization-Layer Path MTU Discovery.  Takes three
@@ -638,6 +646,8 @@ tcp_no_ssthresh_metrics_save - BOOLEAN
 	Controls whether TCP saves ssthresh metrics in the route cache.
 
 	Default is 1, which disables ssthresh metrics.
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_orphan_retries - INTEGER
 	This value influences the timeout of a locally closed TCP connection,
@@ -705,7 +715,9 @@ tcp_retries1 - INTEGER
 
 	RFC 1122 recommends at least 3 retransmissions, which is the
 	default.
-
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 tcp_retries2 - INTEGER
 	This value influences the timeout of an alive TCP connection,
 	when RTO retransmissions remain unacknowledged.
@@ -728,7 +740,7 @@ tcp_rfc1337 - BOOLEAN
 	we are not conforming to RFC, but prevent TCP TIME_WAIT
 	assassination.
 
-	Default: 0
+	Default: 0 (disabled)
 
 tcp_rmem - vector of 3 INTEGERs: min, default, max
 	min: Minimal size of receive buffer used by TCP sockets.
@@ -753,6 +765,8 @@ tcp_sack - BOOLEAN
 	Enable select acknowledgments (SACKS).
 	
 	Default: 1 (enabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_comp_sack_delay_ns - LONG INTEGER
 	TCP tries to reduce number of SACK sent, using a timer
@@ -788,7 +802,9 @@ tcp_slow_start_after_idle - BOOLEAN
 	the current RTO.  If unset, the congestion window will not
 	be timed out after an idle period.
 
-	Default: 1
+	Default: 1 (enabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_stdurg - BOOLEAN
 	Use the Host requirements interpretation of the TCP urgent pointer field.
@@ -796,6 +812,8 @@ tcp_stdurg - BOOLEAN
 	Linux might not communicate correctly with them.
 
 	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_synack_retries - INTEGER
 	Number of times SYNACKs for a passive TCP connection attempt will
@@ -852,7 +870,7 @@ tcp_migrate_req - BOOLEAN
 	migration by returning SK_DROP in the type of eBPF program, or
 	disable this option.
 
-	Default: 0
+	Default: 0 (disabled)
 
 tcp_fastopen - INTEGER
 	Enable TCP Fast Open (RFC7413) to send and accept data in the opening
@@ -1036,6 +1054,8 @@ tcp_window_scaling - BOOLEAN
 	- 1 - Enabled.
 	
 	Default: 1 (enabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_shrink_window - BOOLEAN
 	This changes how the TCP receive window is calculated.
@@ -1050,7 +1070,9 @@ tcp_shrink_window - BOOLEAN
 			This only occurs if a non-zero receive window
 			scaling factor is also in effect.
 
-	Default: 0
+	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
 
 tcp_wmem - vector of 3 INTEGERs: min, default, max
 	min: Amount of memory reserved for send buffers for TCP sockets.
@@ -1092,7 +1114,7 @@ tcp_workaround_signed_windows - BOOLEAN
 	If unset, assume the remote TCP is not broken even if we do
 	not receive a window scaling option from them.
 
-	Default: 0
+	Default: 0 (disabled)
 
 tcp_thin_linear_timeouts - BOOLEAN
 	Enable dynamic triggering of linear timeouts for thin streams.
@@ -1105,8 +1127,10 @@ tcp_thin_linear_timeouts - BOOLEAN
 	For more information on thin streams, see
 	Documentation/networking/tcp-thin.rst
 
-	Default: 0
-
+	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 tcp_limit_output_bytes - INTEGER
 	Controls TCP Small Queue limit per tcp socket.
 	TCP bulk sender tends to increase packets in flight until it
@@ -1350,7 +1374,7 @@ cipso_cache_enable - BOOLEAN
 	invalidated when required when means you can safely toggle this on and
 	off and the cache will always be "safe".
 
-	Default: 1
+	Default: 1 (enabled)
 
 cipso_cache_bucket_size - INTEGER
 	The CIPSO label cache consists of a fixed size hash table with each
@@ -1368,7 +1392,10 @@ cipso_rbm_optfmt - BOOLEAN
 	This means that when set the CIPSO tag will be padded with empty
 	categories in order to make the packet data 32-bit aligned.
 
-	Default: 0
+	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 
 cipso_rbm_strictvalid - BOOLEAN
 	If set, do a very strict check of the CIPSO option when
@@ -1378,7 +1405,10 @@ cipso_rbm_strictvalid - BOOLEAN
 	result in less work (i.e. it should be faster) but could cause problems
 	with other implementations that require strict checking.
 
-	Default: 0
+	Default: 0 (disabled)
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 
 IP Variables
 ============
@@ -1439,6 +1469,9 @@ ip_nonlocal_bind - BOOLEAN
 	which can be quite useful - but may break some applications.
 
 	Default: 0
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 
 ip_autobind_reuse - BOOLEAN
 	By default, bind() does not select the ports automatically even if
@@ -1449,6 +1482,8 @@ ip_autobind_reuse - BOOLEAN
 	option should only be set by experts.
 	Default: 0
 
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 ip_dynaddr - INTEGER
 	If set non-zero, enables support for dynamic addresses.
 	If set to a non-zero value larger than 1, a kernel log
@@ -1478,13 +1513,16 @@ tcp_early_demux - BOOLEAN
 	Enable early demux for established TCP sockets.
 
 	Default: 1
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 
 udp_early_demux - BOOLEAN
 	Enable early demux for connected UDP sockets. Disable this if
 	your system could experience more unconnected load.
 
 	Default: 1
-
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 icmp_echo_ignore_all - BOOLEAN
 	If set non-zero, then the kernel will ignore all ICMP ECHO
 	requests sent to it.
@@ -1817,7 +1855,7 @@ src_valid_mark - BOOLEAN
 	  lookup.  This permits rp_filter to function when the fwmark is
 	  used for routing traffic in both directions.
 
-	This setting also affects the utilization of fmwark when
+	This setting also affects the utilization of fwmark when
 	performing source address selection for ICMP replies, or
 	determining addresses stored for the IPOPT_TS_TSANDADDR and
 	IPOPT_RR IP options.
@@ -2326,7 +2364,9 @@ fwmark_reflect - BOOLEAN
 	fwmark of the packet they are replying to.
 
 	Default: 0
-
+
+	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.
+
 ``conf/interface/*``:
 	Change special settings per interface.
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] docs: net: clarify sysctl value constraints
  2025-06-14 22:53 ` [PATCH v2 2/2] docs: net: clarify sysctl value constraints Abdelrahman Fekry
@ 2025-06-17 18:12   ` Simon Horman
  2025-06-20 21:51     ` Abdelrahman Fekry
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2025-06-17 18:12 UTC (permalink / raw)
  To: Abdelrahman Fekry
  Cc: corbet, davem, edumazet, kuba, pabeni, linux-doc,
	linux-kernel-mentees, linux-kernel, netdev, skhan, jacob.e.keller,
	alok.a.tiwari

On Sun, Jun 15, 2025 at 01:53:24AM +0300, Abdelrahman Fekry wrote:
> So, i also noticed that some of the parameters represented
> as boolean have no value constrain checks and accept integer
> values due to u8 implementation, so i wrote a note for every
> boolean parameter that have no constrain checks in code. and
> fixed a typo in fmwark instead of fwmark.
> 
> Added notes for 19 confirmed parameters,
> Verified by code inspection and runtime testing.

Please consider using imperative mode in patch descriptions.

> - No changes for v2 in this patch , still waiting to be reviewed.

The text on the line above would fit better along
side the "No change." below the scissors ("---") a few lines below.

> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> ---
> v2:
> - No change.
> v1:
> - Added notes for booleans that accept 0-255 not only 0/1.
>  Documentation/networking/ip-sysctl.rst | 70 ++++++++++++++++++++------
>  1 file changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 68778532faa5..38f2981290d6 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -70,6 +70,8 @@ ip_forward_use_pmtu - BOOLEAN
>  
>  	- 0 - disabled
>  	- 1 - enabled
> +
> +	note: Accepts integer values (0-255) but only 0/1 have defined behaviour.

In his review of v1 [*] Jacob said:

  "Hm. In many cases any non-zero value might be interpreted as "enabled" I
   suppose that is simply "undefined behavior"?

Looking over the parsing and use of ip_forward_use_pmtu (I did not check
the other parameters whose documentation this patch updates) I would take
Jacob's remark a few steps further.

It seems to me that values of 0-255 are accepted and while 0 means
disabled, all the other values mean enabled. That is because that
what the code does. And being part of the UAPI it can't be changed.

So I don't think it is correct to describe only values 0/1 having defined
behaviour. Because the code defines behaviour for all the values in the
range 0-255.

[*] https://lore.kernel.org/netdev/8b53b5be-82eb-458c-8269-d296bffcef33@intel.com/

...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] docs: net: sysctl documentation cleanup
  2025-06-14 22:53 ` [PATCH v2 1/2] docs: net: sysctl documentation cleanup Abdelrahman Fekry
@ 2025-06-17 18:31   ` Simon Horman
  2025-06-20 21:48     ` Abdelrahman Fekry
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2025-06-17 18:31 UTC (permalink / raw)
  To: Abdelrahman Fekry
  Cc: corbet, davem, edumazet, kuba, pabeni, linux-doc,
	linux-kernel-mentees, linux-kernel, netdev, skhan, jacob.e.keller,
	alok.a.tiwari

On Sun, Jun 15, 2025 at 01:53:23AM +0300, Abdelrahman Fekry wrote:
> I noticed that some boolean parameters have missing default values
> (enabled/disabled) in the documentation so i checked the initialization
> functions to get their default values, also there was some inconsistency
> in the representation. During the process , i stumbled upon a typo in
> cipso_rbm_struct_valid instead of cipso_rbm_struct_valid.

Please consider using the imperative mood in patch discriptions.

As per [*] please denote the target tree for Networking patches.
In this case net-next seems appropriate.

  [PATCH net-next v3 1/2] ...

[*] https://docs.kernel.org/process/maintainer-netdev.html

And please make sure the patches apply cleanly, without fuzz, on
top of the target tree: this series seems to apply cleanly neither
on net or net-next.

The text below, up to (but not including your Signed-off-by line)
doesn't belong in the patch description. If you wish to include
notes or commentary of this nature then please do so below the
scissors ("---"). But I think the brief summary you already
have there is sufficient in this case - we can follow
the link to v1 for more information.

> 
> Thanks for the review.
> 
> On Thu, 12 Jun 2025, Jacob Keller wrote:
> > Would it make sense to use "0 (disabled)" and "1 (enabled)" with
> > parenthesis for consistency with the default value?
> 
> Used as suggested.
> 
> On Fri, 13 Jun 2025, ALOK TIWARI wrote:
> > for consistency
> > remove extra space before colon
> > Default: 1 (enabled)
> 
> Fixed. 
> 
> On Sat, 14 Jun 2025 10:46:29 -0700, Jakub Kicinski wrote:
> > You need to repost the entire series. Make sure you read:
> > https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
> > before you do.
> 
> Reposted the entire series, Thanks for you patiency.
> 
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> ---
> v2:
> - Deleted space before colon for consistency
> - Standardized more boolean representation (0/1 with enabled/disabled)
> 
> v1: https://lore.kernel.org/all/20250612162954.55843-2-abdelrahmanfekry375@gmail.com/
> - Fixed typo in cipso_rbm_struct_valid
> - Added missing default value declarations
> - Standardized boolean representation (0/1 with enabled/disabled)
>  Documentation/networking/ip-sysctl.rst | 47 ++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 0f1251cce314..68778532faa5 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -8,14 +8,16 @@ IP Sysctl
>  ==============================
>  
>  ip_forward - BOOLEAN
> -	- 0 - disabled (default)
> -	- not 0 - enabled
> +	- 0 (disabled)
> +	- not 0 (enabled)
>  
>  	Forward Packets between interfaces.
>  
>  	This variable is special, its change resets all configuration
>  	parameters to their default state (RFC1122 for hosts, RFC1812
>  	for routers)
> +
> +	Default: 0 (disabled)
>  
>  ip_default_ttl - INTEGER
>  	Default value of TTL field (Time To Live) for outgoing (but not
> @@ -75,7 +77,7 @@ fwmark_reflect - BOOLEAN
>  	If unset, these packets have a fwmark of zero. If set, they have the
>  	fwmark of the packet they are replying to.

Maybe it would be more consistent to describe this in terms
of enabled / disabled rather than set / unset.

>  
> -	Default: 0
> +	Default: 0 (disabled)
>  
>  fib_multipath_use_neigh - BOOLEAN
>  	Use status of existing neighbor entry when determining nexthop for
> @@ -368,7 +370,7 @@ tcp_autocorking - BOOLEAN
>  	queue. Applications can still use TCP_CORK for optimal behavior
>  	when they know how/when to uncork their sockets.
>  
> -	Default : 1
> +	Default: 1 (enabled)

For consistency, would it make sense to document the possible values here.

>  
>  tcp_available_congestion_control - STRING
>  	Shows the available congestion control choices that are registered.
> @@ -407,6 +409,12 @@ tcp_congestion_control - STRING
>  
>  tcp_dsack - BOOLEAN
>  	Allows TCP to send "duplicate" SACKs.
> +
> +	Possible values:
> +		- 0 (disabled)
> +		- 1 (enabled)

In the case of ip_forward, the possible values are not explicitly named
as such and appear at the top of the documentation for the parameter.

Here they are explicitly named possible values and appear below the
description of the parameter, but before documentation of the Default.
Elsewhere, e.g. ip_forward_use_pmtu, they appear after the documentation of
the Default. And sometimes, e.g. ip_default_ttl, the possible values are
documented at all.

Likewise, indentation and use of blank lines seems inconsistent.

Is there a value in cleaning this up too?

> +
> +	Default: 1 (enabled)
>  

...

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] docs: net: sysctl documentation cleanup
  2025-06-17 18:31   ` Simon Horman
@ 2025-06-20 21:48     ` Abdelrahman Fekry
  0 siblings, 0 replies; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-20 21:48 UTC (permalink / raw)
  To: Simon Horman
  Cc: corbet, davem, edumazet, kuba, pabeni, linux-doc,
	linux-kernel-mentees, linux-kernel, netdev, skhan, jacob.e.keller,
	alok.a.tiwari

Thanks for the review


On Tue, Jun 17, 2025 at 9:31 PM Simon Horman <horms@kernel.org> wrote:
>
> On Sun, Jun 15, 2025 at 01:53:23AM +0300, Abdelrahman Fekry wrote:
> > I noticed that some boolean parameters have missing default values
> > (enabled/disabled) in the documentation so i checked the initialization
> > functions to get their default values, also there was some inconsistency
> > in the representation. During the process , i stumbled upon a typo in
> > cipso_rbm_struct_valid instead of cipso_rbm_struct_valid.
>
> Please consider using the imperative mood in patch discriptions.

Noted , will be used in v3.

> As per [*] please denote the target tree for Networking patches.
> In this case net-next seems appropriate.
>
>   [PATCH net-next v3 1/2] ...
>
> [*] https://docs.kernel.org/process/maintainer-netdev.html
>
> And please make sure the patches apply cleanly, without fuzz, on
> top of the target tree: this series seems to apply cleanly neither
> on net or net-next.

Noted, will make sure to denote the target tree and to test it first.

> The text below, up to (but not including your Signed-off-by line)
> doesn't belong in the patch description. If you wish to include
> notes or commentary of this nature then please do so below the
> scissors ("---"). But I think the brief summary you already
> have there is sufficient in this case - we can follow
> the link to v1 for more information.
>
> >
> > Thanks for the review.
> >
> > On Thu, 12 Jun 2025, Jacob Keller wrote:
> > > Would it make sense to use "0 (disabled)" and "1 (enabled)" with
> > > parenthesis for consistency with the default value?
> >
> > Used as suggested.
> >
> > On Fri, 13 Jun 2025, ALOK TIWARI wrote:
> > > for consistency
> > > remove extra space before colon
> > > Default: 1 (enabled)
> >
> > Fixed.
> >
> > On Sat, 14 Jun 2025 10:46:29 -0700, Jakub Kicinski wrote:
> > > You need to repost the entire series. Make sure you read:
> > > https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
> > > before you do.
> >
> > Reposted the entire series, Thanks for you patiency.
> >
> > Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> > ---

Noted, Thanks.

> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index 0f1251cce314..68778532faa5 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -8,14 +8,16 @@ IP Sysctl
> >  ==============================
> >
> >  ip_forward - BOOLEAN
> > -     - 0 - disabled (default)
> > -     - not 0 - enabled
> > +     - 0 (disabled)
> > +     - not 0 (enabled)
> >
> >       Forward Packets between interfaces.
> >
> >       This variable is special, its change resets all configuration
> >       parameters to their default state (RFC1122 for hosts, RFC1812
> >       for routers)
> > +
> > +     Default: 0 (disabled)
> >
> >  ip_default_ttl - INTEGER
> >       Default value of TTL field (Time To Live) for outgoing (but not
> > @@ -75,7 +77,7 @@ fwmark_reflect - BOOLEAN
> >       If unset, these packets have a fwmark of zero. If set, they have the
> >       fwmark of the packet they are replying to.
>
> Maybe it would be more consistent to describe this in terms
> of enabled / disabled rather than set / unset.

Will do this  here and in other parameters to ensure consistency.

>
> >
> > -     Default: 0
> > +     Default: 0 (disabled)
> >
> >  fib_multipath_use_neigh - BOOLEAN
> >       Use status of existing neighbor entry when determining nexthop for
> > @@ -368,7 +370,7 @@ tcp_autocorking - BOOLEAN
> >       queue. Applications can still use TCP_CORK for optimal behavior
> >       when they know how/when to uncork their sockets.
> >
> > -     Default : 1
> > +     Default: 1 (enabled)
>
> For consistency, would it make sense to document the possible values here.

Noted, will document possible values here and in other parameters for
consistency.

>
> >
> >  tcp_available_congestion_control - STRING
> >       Shows the available congestion control choices that are registered.
> > @@ -407,6 +409,12 @@ tcp_congestion_control - STRING
> >
> >  tcp_dsack - BOOLEAN
> >       Allows TCP to send "duplicate" SACKs.
> > +
> > +     Possible values:
> > +             - 0 (disabled)
> > +             - 1 (enabled)
>
> In the case of ip_forward, the possible values are not explicitly named
> as such and appear at the top of the documentation for the parameter.
>
> Here they are explicitly named possible values and appear below the
> description of the parameter, but before documentation of the Default.
> Elsewhere, e.g. ip_forward_use_pmtu, they appear after the documentation of
> the Default. And sometimes, e.g. ip_default_ttl, the possible values are
> documented at all.
>

Noted, will make sure that all representation follow the same appearance,
first the description then possible values then default.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] docs: net: clarify sysctl value constraints
  2025-06-17 18:12   ` Simon Horman
@ 2025-06-20 21:51     ` Abdelrahman Fekry
  0 siblings, 0 replies; 10+ messages in thread
From: Abdelrahman Fekry @ 2025-06-20 21:51 UTC (permalink / raw)
  To: Simon Horman
  Cc: corbet, davem, edumazet, kuba, pabeni, linux-doc,
	linux-kernel-mentees, linux-kernel, netdev, skhan, jacob.e.keller,
	alok.a.tiwari

On Tue, Jun 17, 2025 at 9:12 PM Simon Horman <horms@kernel.org> wrote:
> In his review of v1 [*] Jacob said:
>
>   "Hm. In many cases any non-zero value might be interpreted as "enabled" I
>    suppose that is simply "undefined behavior"?
>
> Looking over the parsing and use of ip_forward_use_pmtu (I did not check
> the other parameters whose documentation this patch updates) I would take
> Jacob's remark a few steps further.
>
> It seems to me that values of 0-255 are accepted and while 0 means
> disabled, all the other values mean enabled. That is because that
> what the code does. And being part of the UAPI it can't be changed.
>
> So I don't think it is correct to describe only values 0/1 having defined
> behaviour. Because the code defines behaviour for all the values in the
> range 0-255.
>
> [*] https://lore.kernel.org/netdev/8b53b5be-82eb-458c-8269-d296bffcef33@intel.com/
>
> ...

well , Thanks for the clarification , i will only keep patch 1/2 in
the next post

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-06-20 21:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-14 22:53 [PATCH v2 0/2] docs: net: sysctl documentation improvements Abdelrahman Fekry
2025-06-14 22:53 ` [PATCH v2 1/2] docs: net: sysctl documentation cleanup Abdelrahman Fekry
2025-06-17 18:31   ` Simon Horman
2025-06-20 21:48     ` Abdelrahman Fekry
2025-06-14 22:53 ` [PATCH v2 2/2] docs: net: clarify sysctl value constraints Abdelrahman Fekry
2025-06-17 18:12   ` Simon Horman
2025-06-20 21:51     ` Abdelrahman Fekry
  -- strict thread matches above, loose matches on Subject: below --
2025-06-14  9:33 [PATCH v2 1/2] docs: net: sysctl documentation cleanup Abdelrahman Fekry
2025-06-12 16:29 [PATCH " Abdelrahman Fekry
2025-06-14  9:25 ` [PATCH v2 " Abdelrahman Fekry
2025-06-14 17:46   ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).