Thanks for the review


On Tue, Jun 17, 2025 at 9:31 PM Simon Horman [off-list ref] wrote:

On Sun, Jun 15, 2025 at 01:53:23AM +0300, Abdelrahman Fekry wrote:

quoted

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.

quoted

Thanks for the review.

On Thu, 12 Jun 2025, Jacob Keller wrote:

quoted

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:

quoted

for consistency
remove extra space before colon
Default: 1 (enabled)

Fixed.

On Sat, 14 Jun 2025 10:46:29 -0700, Jakub Kicinski wrote:

quoted

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 <redacted>
---

Noted, Thanks.

quoted

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.

quoted

-     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.

quoted

 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.