On Fri, Apr 20, 2018 at 02:36:52AM +0000, Eric Dumazet wrote: > On Thu, Apr 19, 2018 at 7:02 PM Marcelo Ricardo Leitner < > marcelo.leit...@gmail.com> wrote: > > > Hi Eric, > > > As val may be changed to a smaller value by the line above, shouldn't > > it assign sk->sk_rcvlowat again? Otherwise it may still be bigger > > than sk_rcvbuf. > > > Say val = 512k, sysctl_tcp_rmem[2] = 256k > > val <<= 1 , val = 1M > > val = min() , val = 256k > > val > sk_rcvbuf > > sk_rcvbuf = 256k , at most, which is smaller than sk_rcvlowat > > > Without reassigning the application has to check how big is > > tcp_rmem[2] and be sure to not go above /2 of it to not trip on this > > again. > > I am not sure about that : > > Reporting an error might break existing applications that were not > expecting setsockopt() > to return an error, even if the value was 'probably too big to be okay'
I would argue that they are already broken but... > > > > Or, as you have added a return value here, it could return -EINVAL in > > such cases. Probably better, as then the application will not get a > > smaller buffer than wanted later. > > Note that maybe some applications might first set SO_RCVLOWAT, then > SO_RCVBUF, > we do not want to break them. ... yeah.. if they do it this way, they work today. Good point. > > > My patch really covers the case were autotuning should immediately grow the > sk_rcvbuf > for reasonable SO_RCVLOWAT values. That's not exactly what the comment above the function says, thus why my comments. Thanks.
