Re: Issue with /proc/sys/net/ipv4/tcp_mem

[ebiederm@xmission.com (Eric W. Biederman)]

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