netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
Cc: corbet@lwn.net, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, linux-doc@vger.kernel.org,
	linux-kernel-mentees@lists.linux.dev,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	skhan@linuxfoundation.com, jacob.e.keller@intel.com,
	alok.a.tiwari@oracle.com
Subject: Re: [PATCH v2 2/2] docs: net: clarify sysctl value constraints
Date: Tue, 17 Jun 2025 19:12:19 +0100	[thread overview]
Message-ID: <20250617181219.GB2545@horms.kernel.org> (raw)
In-Reply-To: <20250614225324.82810-3-abdelrahmanfekry375@gmail.com>

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/

...

  reply	other threads:[~2025-06-17 18:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2025-06-20 21:51     ` Abdelrahman Fekry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250617181219.GB2545@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=abdelrahmanfekry375@gmail.com \
    --cc=alok.a.tiwari@oracle.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=skhan@linuxfoundation.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).