* Issue with /proc/sys/net/ipv4/tcp_mem
@ 2015-10-12 9:38 wangyufen
2015-10-12 15:24 ` Eric W. Biederman
2015-10-12 16:37 ` Eric W. Biederman
0 siblings, 2 replies; 6+ messages in thread
From: wangyufen @ 2015-10-12 9:38 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev
Hi,
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.
I checked, patch f594d63199688ad568fb caused the issue.
thanks,
Wang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Issue with /proc/sys/net/ipv4/tcp_mem 2015-10-12 9:38 Issue with /proc/sys/net/ipv4/tcp_mem wangyufen @ 2015-10-12 15:24 ` Eric W. Biederman 2015-10-12 16:37 ` Eric W. Biederman 1 sibling, 0 replies; 6+ messages in thread From: Eric W. Biederman @ 2015-10-12 15:24 UTC (permalink / raw) To: wangyufen; +Cc: netdev wangyufen <wangyufen@huawei.com> writes: > Hi, > > 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. > > I checked, patch f594d63199688ad568fb caused the issue. Use the cgroup interface if you want per cgroup control. Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Issue with /proc/sys/net/ipv4/tcp_mem 2015-10-12 9:38 Issue with /proc/sys/net/ipv4/tcp_mem wangyufen 2015-10-12 15:24 ` Eric W. Biederman @ 2015-10-12 16:37 ` Eric W. Biederman 2015-10-13 1:44 ` Eric Dumazet 1 sibling, 1 reply; 6+ messages in thread From: Eric W. Biederman @ 2015-10-12 16:37 UTC (permalink / raw) To: wangyufen; +Cc: netdev wangyufen <wangyufen@huawei.com> writes: > Hi, > > 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. > > I checked, patch f594d63199688ad568fb caused the issue. If your problem is that you can not write a single value and instead have to write all three values I don't know what to tell you. I don't see how that could have ever worked. Certainly the commit you pointed at did not change that behavior. Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Issue with /proc/sys/net/ipv4/tcp_mem 2015-10-12 16:37 ` Eric W. Biederman @ 2015-10-13 1:44 ` Eric Dumazet 2015-10-13 5:07 ` Eric W. Biederman 0 siblings, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2015-10-13 1:44 UTC (permalink / raw) To: Eric W. Biederman; +Cc: wangyufen, netdev On Mon, 2015-10-12 at 11:37 -0500, Eric W. Biederman wrote: > wangyufen <wangyufen@huawei.com> writes: > > > Hi, > > > > 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. > > > > I checked, patch f594d63199688ad568fb caused the issue. > > > If your problem is that you can not write a single value and instead > have to write all three values I don't know what to tell you. I don't > see how that could have ever worked. > > Certainly the commit you pointed at did not change that behavior. I would not be so sure. Above commit added a regression for partial writes. If a write() returns an error like EINVAL, we expect no change occurred. Prior code was calling proc_doulongvec_minmax() using a temporary array, and updated tcp_mem[0 .. 2] only of proc_doulongvec_minmax() returned 0 ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos); if (ret) return ret; #ifdef CONFIG_MEMCG_KMEM // deleted for clarity #endif net->ipv4.sysctl_tcp_mem[0] = vec[0]; net->ipv4.sysctl_tcp_mem[1] = vec[1]; net->ipv4.sysctl_tcp_mem[2] = vec[2]; return 0; We could argue it is a bug in proc_doulongvec_minmax(). This helper probably should allocate a temp buffer, as we have the same issue with udp_mem[]. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Issue with /proc/sys/net/ipv4/tcp_mem 2015-10-13 1:44 ` Eric Dumazet @ 2015-10-13 5:07 ` Eric W. Biederman 2015-11-28 8:13 ` wangyufen 0 siblings, 1 reply; 6+ messages in thread From: Eric W. Biederman @ 2015-10-13 5:07 UTC (permalink / raw) To: Eric Dumazet; +Cc: wangyufen, netdev Eric Dumazet <eric.dumazet@gmail.com> writes: > On Mon, 2015-10-12 at 11:37 -0500, Eric W. Biederman wrote: >> wangyufen <wangyufen@huawei.com> writes: >> >> > Hi, >> > >> > 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. >> > >> > I checked, patch f594d63199688ad568fb caused the issue. >> >> >> If your problem is that you can not write a single value and instead >> have to write all three values I don't know what to tell you. I don't >> see how that could have ever worked. >> >> Certainly the commit you pointed at did not change that behavior. > > I would not be so sure. > Above commit added a regression for partial writes. > If a write() returns an error like EINVAL, we expect no change occurred. > > Prior code was calling proc_doulongvec_minmax() using a temporary array, > and updated tcp_mem[0 .. 2] only of proc_doulongvec_minmax() returned 0 > > ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos); > if (ret) > return ret; > #ifdef CONFIG_MEMCG_KMEM > // deleted for clarity > #endif > > net->ipv4.sysctl_tcp_mem[0] = vec[0]; > net->ipv4.sysctl_tcp_mem[1] = vec[1]; > net->ipv4.sysctl_tcp_mem[2] = vec[2]; > > return 0; > > We could argue it is a bug in proc_doulongvec_minmax(). > This helper probably should allocate a temp buffer, > as we have the same issue with udp_mem[]. Point. We do store the value on partial writes when before we did not. That is weird. Clearly someone noticed. I agree this is a confusing corner case in proc_doulongvec_minmax that it may be worth addressing. Does this cause a regression in a real application? I definitely would like to know what in the world a real application is doing that causes it to break with this difference in behavior before doing anything, because I am dense enough not to see how an application could meaningfully care about this difference in behavior. Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Issue with /proc/sys/net/ipv4/tcp_mem 2015-10-13 5:07 ` Eric W. Biederman @ 2015-11-28 8:13 ` wangyufen 0 siblings, 0 replies; 6+ messages in thread From: wangyufen @ 2015-11-28 8:13 UTC (permalink / raw) To: Eric W. Biederman, Eric Dumazet Cc: netdev, Hanjun Guo, Dingtianhong, Dianfang Zhang, Xinwei Hu On 2015/10/13 13:07, Eric W. Biederman wrote: > Eric Dumazet <eric.dumazet@gmail.com> writes: > >> On Mon, 2015-10-12 at 11:37 -0500, Eric W. Biederman wrote: >>> wangyufen <wangyufen@huawei.com> writes: >>> >>>> Hi, >>>> >>>> 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. >>>> >>>> I checked, patch f594d63199688ad568fb caused the issue. >>> >>> >>> If your problem is that you can not write a single value and instead >>> have to write all three values I don't know what to tell you. I don't >>> see how that could have ever worked. >>> >>> Certainly the commit you pointed at did not change that behavior. >> >> I would not be so sure. >> Above commit added a regression for partial writes. >> If a write() returns an error like EINVAL, we expect no change occurred. >> >> Prior code was calling proc_doulongvec_minmax() using a temporary array, >> and updated tcp_mem[0 .. 2] only of proc_doulongvec_minmax() returned 0 >> >> ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos); >> if (ret) >> return ret; >> #ifdef CONFIG_MEMCG_KMEM >> // deleted for clarity >> #endif >> >> net->ipv4.sysctl_tcp_mem[0] = vec[0]; >> net->ipv4.sysctl_tcp_mem[1] = vec[1]; >> net->ipv4.sysctl_tcp_mem[2] = vec[2]; >> >> return 0; >> >> We could argue it is a bug in proc_doulongvec_minmax(). >> This helper probably should allocate a temp buffer, >> as we have the same issue with udp_mem[]. > > Point. We do store the value on partial writes when before we did not. > > That is weird. Clearly someone noticed. I agree this is a confusing > corner case in proc_doulongvec_minmax that it may be worth addressing. > I think maybe we can fix the confusing corner with that patch: --- 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, -- 2.5.0 ~ The patch makes __do_proc_doulongvec_minmax works the same as __do_proc_dointvec, but I'm not sure this change will not break something. thanks, Wang > Does this cause a regression in a real application? I definitely would > like to know what in the world a real application is doing that causes > it to break with this difference in behavior before doing anything, > because I am dense enough not to see how an application could > meaningfully care about this difference in behavior. > > Eric > > > ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-28 8:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-12 9:38 Issue with /proc/sys/net/ipv4/tcp_mem wangyufen 2015-10-12 15:24 ` Eric W. Biederman 2015-10-12 16:37 ` Eric W. Biederman 2015-10-13 1:44 ` Eric Dumazet 2015-10-13 5:07 ` Eric W. Biederman 2015-11-28 8:13 ` wangyufen
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).