Re: [PATCH v2 net-next] rds-tcp: Add sysctl tunables for sndbuf/rcvbuf on rds-tcp socket

On Tue, 2016-03-15 at 13:30 -0400, Sowmini Varadhan wrote:

On (03/15/16 10:18), Eric Dumazet wrote:

quoted

Look at SO_SNDBUF and SO_RCVBUF implementation.

sk->sk_sndbuf = max_t(u32, val * 2, SOCK_MIN_SNDBUF);

sk->sk_rcvbuf = max_t(u32, val * 2, SOCK_MIN_RCVBUF);

kernel definitely has some logic here.

Ok, I can do the same thing (and we do this consistently across
all drivers?)

quoted

If you believe SOCK_MIN_SNDBUF and/or SOCK_MIN_RCVBUF are wrong, please
elaborate.

I dont recall suggesting that.

You said "just as user-space SO_SNDBUF allows ridiculous values
for buffer size.."

So I understood you believe SOCK_MIN_SNDBUF and/or SOCK_MIN_RCVBUF are
ridiculous ;)

BTW, when I tried it, doing a SO_SNDBUF of 1 from uspace does not return
an error. It merely sets the buffer size to 4608 (as reported by
getsockopt in my env. I think the getsockopt value is impacted by
many factors).

I pointed to you the actual code.

sk->sk_sndbuf = max_t(u32, val * 2, SOCK_MIN_SNDBUF);


No error is returned. kernel enforces a minimal value.

#define SOCK_MIN_SNDBUF         (TCP_SKB_MIN_TRUESIZE * 2)
#define TCP_SKB_MIN_TRUESIZE    (2048 + SKB_DATA_ALIGN(sizeof(struct
sk_buff)))

-> 2 * (2048 + 256) = 4608  given current sk_buff overhead (that might
change in linux 5.4 ... )

But again if your sysctl allows to set a value below SOCK_MIN_SNDBUF,
that might be a problem, because stack could have a hidden bug for very
small values of sndbuf/rcvbuf.