All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.