All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Wang Yufen <wangyufen@huawei.com>
Cc: <davem@davemloft.net>, <netdev@vger.kernel.org>,
	<guohanjun@huawei.com>, <zhangdianfang@huawei.com>,
	<dingtianhong@huawei.com>, <huxinwei@huawei.com>,
	Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH net] net:sysctl fix the confusing corner of tcp_mem
Date: Wed, 09 Dec 2015 10:38:01 -0600	[thread overview]
Message-ID: <877fknbms6.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <1449626509-3736-1-git-send-email-wangyufen@huawei.com> (Wang Yufen's message of "Wed, 9 Dec 2015 10:01:49 +0800")

Wang Yufen <wangyufen@huawei.com> writes:

> From: Yufen Wang <wangyufen@huawei.com>
>
> I tried on linux-4.1:
>     linux:~# cat /proc/sys/net/ipv4/tcp_mem 
>     8388608	12582912	16777216
>     linux:~# echo 1234 >/proc/sys/net/ipv4/tcp_mem 
>     -bash: echo: write error: Invalid argument
>     linux:~# cat /proc/sys/net/ipv4/tcp_mem 
>     1234	12582912	16777216
>
> the echo operation got error, but value already written to tcp_mem.
> If a write() returns an error like EINVAL, we expect no change occurred.
> This patch fix the confusing corner and makes __do_proc_doulongvec_minmax 
> works the same as __do_proc_dointvec

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Except for possibly breaking your muscle memory this does not explain
why this is a problem.

Further you are changing a whole lot more than tcp_mem, without a word
of justification in your description.

I do not think changing every integer use of sysctl for some unknown
reason.  Is justified in this case.

Eric



> Signed-off-by: Yufen Wang <wangyufen@huawei.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  kernel/sysctl.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index c3eee4c..e3ee4be 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2318,6 +2318,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
>  			bool neg;
>  
>  			left -= proc_skip_spaces(&kbuf);
> +                        if (!left)
> +                                break;
>  
>  			err = proc_get_long(&kbuf, &left, &val, &neg,
>  					     proc_wspace_sep,

  parent reply	other threads:[~2015-12-09 16:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-09  2:01 [PATCH net] net:sysctl fix the confusing corner of tcp_mem Wang Yufen
2015-12-09 13:47 ` Sergei Shtylyov
2015-12-09 16:38 ` Eric W. Biederman [this message]
2015-12-21  1:02   ` Hanjun Guo

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=877fknbms6.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=eric.dumazet@gmail.com \
    --cc=guohanjun@huawei.com \
    --cc=huxinwei@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=wangyufen@huawei.com \
    --cc=zhangdianfang@huawei.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.