* [PATCH] net: Update window_clamp if SOCK_RCVBUF is set @ 2020-11-09 9:20 Mao Wenan 2020-11-09 9:33 ` [PATCH net v2] " Mao Wenan 0 siblings, 1 reply; 16+ messages in thread From: Mao Wenan @ 2020-11-09 9:20 UTC (permalink / raw) To: edumazet, davem, kuznet, yoshfuji, kuba Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan When net.ipv4.tcp_syncookies=1 and syn flood is happened, cookie_v4_check tries to redo what tcp_v4_send_synack did, rsk_window_clamp will be changed if SOCK_RCVBUF is set by user, which will make rcv_wscale is different, the client still operates with initial window scale and can overshot granted window, the client use the initial scale but local server use new scale to advertise window value, and session work abnormally. Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> --- net/ipv4/syncookies.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b..57ce317 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) /* Try to redo what tcp_v4_send_synack did. */ req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = tcp_full_space(sk); tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 9:20 [PATCH] net: Update window_clamp if SOCK_RCVBUF is set Mao Wenan @ 2020-11-09 9:33 ` Mao Wenan 2020-11-09 9:56 ` Eric Dumazet 0 siblings, 1 reply; 16+ messages in thread From: Mao Wenan @ 2020-11-09 9:33 UTC (permalink / raw) To: edumazet, davem, kuznet, yoshfuji, kuba Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan When net.ipv4.tcp_syncookies=1 and syn flood is happened, cookie_v4_check or cookie_v6_check tries to redo what tcp_v4_send_synack or tcp_v6_send_synack did, rsk_window_clamp will be changed if SOCK_RCVBUF is set, which will make rcv_wscale is different, the client still operates with initial window scale and can overshot granted window, the client use the initial scale but local server use new scale to advertise window value, and session work abnormally. Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> --- v2: fix for ipv6. net/ipv4/syncookies.c | 4 ++++ net/ipv6/syncookies.c | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b..57ce317 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) /* Try to redo what tcp_v4_send_synack did. */ req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = tcp_full_space(sk); tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index e796a64..c041360 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -241,6 +241,11 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) } req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = tcp_full_space(sk); + tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 9:33 ` [PATCH net v2] " Mao Wenan @ 2020-11-09 9:56 ` Eric Dumazet 2020-11-09 10:12 ` Mao Wenan 0 siblings, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 9:56 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > When net.ipv4.tcp_syncookies=1 and syn flood is happened, > cookie_v4_check or cookie_v6_check tries to redo what > tcp_v4_send_synack or tcp_v6_send_synack did, > rsk_window_clamp will be changed if SOCK_RCVBUF is set, > which will make rcv_wscale is different, the client > still operates with initial window scale and can overshot > granted window, the client use the initial scale but local > server use new scale to advertise window value, and session > work abnormally. What is not working exactly ? Sending a 'big wscale' should not really matter, unless perhaps there is a buggy stack at the remote end ? > > Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> > --- > v2: fix for ipv6. > net/ipv4/syncookies.c | 4 ++++ > net/ipv6/syncookies.c | 5 +++++ > 2 files changed, 9 insertions(+) > > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c > index 6ac473b..57ce317 100644 > --- a/net/ipv4/syncookies.c > +++ b/net/ipv4/syncookies.c > @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) > > /* Try to redo what tcp_v4_send_synack did. */ > req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); > + /* limit the window selection if the user enforce a smaller rx buffer */ > + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && > + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0)) > + req->rsk_window_clamp = tcp_full_space(sk); This seems not needed to me. We call tcp_select_initial_window() with tcp_full_space(sk) passed as the 2nd parameter. tcp_full_space(sk) will then apply : space = min(*window_clamp, space); Please cook a packetdrill test to demonstrate what you are seeing ? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 9:56 ` Eric Dumazet @ 2020-11-09 10:12 ` Mao Wenan 2020-11-09 10:19 ` Mao Wenan 2020-11-09 11:02 ` Eric Dumazet 0 siblings, 2 replies; 16+ messages in thread From: Mao Wenan @ 2020-11-09 10:12 UTC (permalink / raw) To: Eric Dumazet Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors 在 2020/11/9 下午5:56, Eric Dumazet 写道: > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: >> >> When net.ipv4.tcp_syncookies=1 and syn flood is happened, >> cookie_v4_check or cookie_v6_check tries to redo what >> tcp_v4_send_synack or tcp_v6_send_synack did, >> rsk_window_clamp will be changed if SOCK_RCVBUF is set, >> which will make rcv_wscale is different, the client >> still operates with initial window scale and can overshot >> granted window, the client use the initial scale but local >> server use new scale to advertise window value, and session >> work abnormally. > > What is not working exactly ? > > Sending a 'big wscale' should not really matter, unless perhaps there > is a buggy stack at the remote end ? 1)in tcp_v4_send_synack, if SO_RCVBUF is set and tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to tcp_select_initial_window, rcv_wscale will be zero, and send to client, the client consider wscale is 0; 2)when ack is back from client, if there is no this patch, req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, wscale will be 7, this new rcv_wscale is no way to advertise to client. 3)if server send rcv_wind to client with window=63, it consider the real window is 63*2^7=8064, but client consider the server window is only 63*2^0=63, it can't send big packet to server, and the send-q of client is full. > >> >> Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> >> --- >> v2: fix for ipv6. >> net/ipv4/syncookies.c | 4 ++++ >> net/ipv6/syncookies.c | 5 +++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c >> index 6ac473b..57ce317 100644 >> --- a/net/ipv4/syncookies.c >> +++ b/net/ipv4/syncookies.c >> @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) >> >> /* Try to redo what tcp_v4_send_synack did. */ >> req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); >> + /* limit the window selection if the user enforce a smaller rx buffer */ >> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && >> + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0)) >> + req->rsk_window_clamp = tcp_full_space(sk); > > This seems not needed to me. > > We call tcp_select_initial_window() with tcp_full_space(sk) passed as > the 2nd parameter. > > tcp_full_space(sk) will then apply : > > space = min(*window_clamp, space); if cookie_v4_check pass window_clamp=0 to tcp_select_initial_window, it will set window_clamp to max value. (*window_clamp) = (U16_MAX << TCP_MAX_WSCALE); but space will fetch from sysctl_rmem_max and sysctl_tcp_rmem[2] which is also big value. space = max_t(u32, space, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]); space = max_t(u32, space, sysctl_rmem_max); Then,space = min(*window_clamp, space) is a big value, lead wscale to 7, is different from tcp_v4_send_synack. > > Please cook a packetdrill test to demonstrate what you are seeing ? > I have real environment and reproduce this case, this patch can fix that, i will try to use packetdrill with syn cookies and syn flood happen. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 10:12 ` Mao Wenan @ 2020-11-09 10:19 ` Mao Wenan 2020-11-09 11:02 ` Eric Dumazet 1 sibling, 0 replies; 16+ messages in thread From: Mao Wenan @ 2020-11-09 10:19 UTC (permalink / raw) To: Eric Dumazet Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors 在 2020/11/9 下午6:12, Mao Wenan 写道: > > > 在 2020/11/9 下午5:56, Eric Dumazet 写道: >> On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan >> <wenan.mao@linux.alibaba.com> wrote: >>> >>> When net.ipv4.tcp_syncookies=1 and syn flood is happened, >>> cookie_v4_check or cookie_v6_check tries to redo what >>> tcp_v4_send_synack or tcp_v6_send_synack did, >>> rsk_window_clamp will be changed if SOCK_RCVBUF is set, >>> which will make rcv_wscale is different, the client >>> still operates with initial window scale and can overshot >>> granted window, the client use the initial scale but local >>> server use new scale to advertise window value, and session >>> work abnormally. >> >> What is not working exactly ? >> >> Sending a 'big wscale' should not really matter, unless perhaps there >> is a buggy stack at the remote end ? > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to > tcp_select_initial_window, rcv_wscale will be zero, and send to client, > the client consider wscale is 0; > 2)when ack is back from client, if there is no this patch, > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, > wscale will be 7, this new rcv_wscale is no way to advertise to client. > 3)if server send rcv_wind to client with window=63, it consider the real > window is 63*2^7=8064, but client consider the server window is only > 63*2^0=63, it can't send big packet to server, and the send-q of client > is full. > > >> >>> >>> Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> >>> --- >>> v2: fix for ipv6. >>> net/ipv4/syncookies.c | 4 ++++ >>> net/ipv6/syncookies.c | 5 +++++ >>> 2 files changed, 9 insertions(+) >>> >>> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c >>> index 6ac473b..57ce317 100644 >>> --- a/net/ipv4/syncookies.c >>> +++ b/net/ipv4/syncookies.c >>> @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, >>> struct sk_buff *skb) >>> >>> /* Try to redo what tcp_v4_send_synack did. */ >>> req->rsk_window_clamp = tp->window_clamp ? >>> :dst_metric(&rt->dst, RTAX_WINDOW); >>> + /* limit the window selection if the user enforce a smaller >>> rx buffer */ >>> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && >>> + (req->rsk_window_clamp > tcp_full_space(sk) || >>> req->rsk_window_clamp == 0)) >>> + req->rsk_window_clamp = tcp_full_space(sk); >> >> This seems not needed to me. >> >> We call tcp_select_initial_window() with tcp_full_space(sk) passed as >> the 2nd parameter. >> >> tcp_full_space(sk) will then apply : >> >> space = min(*window_clamp, space); > > if cookie_v4_check pass window_clamp=0 to tcp_select_initial_window, it > will set window_clamp to max value. > (*window_clamp) = (U16_MAX << TCP_MAX_WSCALE); window_clamp=0 is from req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); and if SO_RCVBUF is set and equal to 65535,req->rsk_window_clamp will be 65535. req->rsk_window_clamp = tcp_full_space(sk); > > but space will fetch from sysctl_rmem_max and sysctl_tcp_rmem[2] which > is also big value. > space = max_t(u32, space, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]); > space = max_t(u32, space, sysctl_rmem_max); > > Then,space = min(*window_clamp, space) is a big value, lead wscale to 7, > is different from tcp_v4_send_synack. > > >> >> Please cook a packetdrill test to demonstrate what you are seeing ? >> > I have real environment and reproduce this case, this patch can fix > that, i will try to use packetdrill with syn cookies and syn flood happen. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 10:12 ` Mao Wenan 2020-11-09 10:19 ` Mao Wenan @ 2020-11-09 11:02 ` Eric Dumazet 2020-11-09 11:41 ` Eric Dumazet 1 sibling, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 11:02 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > > > 在 2020/11/9 下午5:56, Eric Dumazet 写道: > > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > >> > >> When net.ipv4.tcp_syncookies=1 and syn flood is happened, > >> cookie_v4_check or cookie_v6_check tries to redo what > >> tcp_v4_send_synack or tcp_v6_send_synack did, > >> rsk_window_clamp will be changed if SOCK_RCVBUF is set, > >> which will make rcv_wscale is different, the client > >> still operates with initial window scale and can overshot > >> granted window, the client use the initial scale but local > >> server use new scale to advertise window value, and session > >> work abnormally. > > > > What is not working exactly ? > > > > Sending a 'big wscale' should not really matter, unless perhaps there > > is a buggy stack at the remote end ? > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to > tcp_select_initial_window, rcv_wscale will be zero, and send to client, > the client consider wscale is 0; > 2)when ack is back from client, if there is no this patch, > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, > wscale will be 7, this new rcv_wscale is no way to advertise to client. > 3)if server send rcv_wind to client with window=63, it consider the real > window is 63*2^7=8064, but client consider the server window is only > 63*2^0=63, it can't send big packet to server, and the send-q of client > is full. > I see, please change your patches so that tcp_full_space() is used _once_ listener sk_rcvbuf can change under us. I really have no idea how window can be set to 63, so please send us the packetdrill test once you have it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 11:02 ` Eric Dumazet @ 2020-11-09 11:41 ` Eric Dumazet 2020-11-09 14:01 ` Eric Dumazet 0 siblings, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 11:41 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors Packetdrill test would be : // Force syncookies `sysctl -q net.ipv4.tcp_syncookies=2` 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0 +0 bind(3, ..., ...) = 0 +0 listen(3, 1) = 0 +0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7> +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0> +.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000> +0 accept(3, ..., ...) = 4 +0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }% On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > > > > > > > 在 2020/11/9 下午5:56, Eric Dumazet 写道: > > > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > >> > > >> When net.ipv4.tcp_syncookies=1 and syn flood is happened, > > >> cookie_v4_check or cookie_v6_check tries to redo what > > >> tcp_v4_send_synack or tcp_v6_send_synack did, > > >> rsk_window_clamp will be changed if SOCK_RCVBUF is set, > > >> which will make rcv_wscale is different, the client > > >> still operates with initial window scale and can overshot > > >> granted window, the client use the initial scale but local > > >> server use new scale to advertise window value, and session > > >> work abnormally. > > > > > > What is not working exactly ? > > > > > > Sending a 'big wscale' should not really matter, unless perhaps there > > > is a buggy stack at the remote end ? > > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and > > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to > > tcp_select_initial_window, rcv_wscale will be zero, and send to client, > > the client consider wscale is 0; > > 2)when ack is back from client, if there is no this patch, > > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, > > wscale will be 7, this new rcv_wscale is no way to advertise to client. > > 3)if server send rcv_wind to client with window=63, it consider the real > > window is 63*2^7=8064, but client consider the server window is only > > 63*2^0=63, it can't send big packet to server, and the send-q of client > > is full. > > > > I see, please change your patches so that tcp_full_space() is used _once_ > > listener sk_rcvbuf can change under us. > > I really have no idea how window can be set to 63, so please send us > the packetdrill test once you have it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 11:41 ` Eric Dumazet @ 2020-11-09 14:01 ` Eric Dumazet 2020-11-09 16:26 ` Mao Wenan 2020-11-09 16:53 ` [PATCH net v3] " Mao Wenan 0 siblings, 2 replies; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 14:01 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Mon, Nov 9, 2020 at 12:41 PM Eric Dumazet <edumazet@google.com> wrote: > > Packetdrill test would be : > > // Force syncookies > `sysctl -q net.ipv4.tcp_syncookies=2` > > 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 > +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 > +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0 > +0 bind(3, ..., ...) = 0 > +0 listen(3, 1) = 0 > > +0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7> > +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0> > +.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000> > +0 accept(3, ..., ...) = 4 > +0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }% > Also, please add to your next submission an appropriate Fixes: tag : Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") > On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <edumazet@google.com> wrote: > > > > On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > > > > > > > > > > > 在 2020/11/9 下午5:56, Eric Dumazet 写道: > > > > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > > >> > > > >> When net.ipv4.tcp_syncookies=1 and syn flood is happened, > > > >> cookie_v4_check or cookie_v6_check tries to redo what > > > >> tcp_v4_send_synack or tcp_v6_send_synack did, > > > >> rsk_window_clamp will be changed if SOCK_RCVBUF is set, > > > >> which will make rcv_wscale is different, the client > > > >> still operates with initial window scale and can overshot > > > >> granted window, the client use the initial scale but local > > > >> server use new scale to advertise window value, and session > > > >> work abnormally. > > > > > > > > What is not working exactly ? > > > > > > > > Sending a 'big wscale' should not really matter, unless perhaps there > > > > is a buggy stack at the remote end ? > > > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and > > > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to > > > tcp_select_initial_window, rcv_wscale will be zero, and send to client, > > > the client consider wscale is 0; > > > 2)when ack is back from client, if there is no this patch, > > > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, > > > wscale will be 7, this new rcv_wscale is no way to advertise to client. > > > 3)if server send rcv_wind to client with window=63, it consider the real > > > window is 63*2^7=8064, but client consider the server window is only > > > 63*2^0=63, it can't send big packet to server, and the send-q of client > > > is full. > > > > > > > I see, please change your patches so that tcp_full_space() is used _once_ > > > > listener sk_rcvbuf can change under us. > > > > I really have no idea how window can be set to 63, so please send us > > the packetdrill test once you have it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v2] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 14:01 ` Eric Dumazet @ 2020-11-09 16:26 ` Mao Wenan 2020-11-09 16:53 ` [PATCH net v3] " Mao Wenan 1 sibling, 0 replies; 16+ messages in thread From: Mao Wenan @ 2020-11-09 16:26 UTC (permalink / raw) To: Eric Dumazet Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors 在 2020/11/9 下午10:01, Eric Dumazet 写道: > On Mon, Nov 9, 2020 at 12:41 PM Eric Dumazet <edumazet@google.com> wrote: >> >> Packetdrill test would be : >> >> // Force syncookies >> `sysctl -q net.ipv4.tcp_syncookies=2` >> >> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 >> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 >> +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0 >> +0 bind(3, ..., ...) = 0 >> +0 listen(3, 1) = 0 >> >> +0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7> >> +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0> >> +.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000> >> +0 accept(3, ..., ...) = 4 >> +0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }% >> > > Also, please add to your next submission an appropriate Fixes: tag : > > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's > rcv-buffer via setsockopt") OK, thanks, I can reproduce wscale=0 with your packetdrill, and I will send v3 with the fixes tag. > >> On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <edumazet@google.com> wrote: >>> >>> On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: >>>> >>>> >>>> >>>> 在 2020/11/9 下午5:56, Eric Dumazet 写道: >>>>> On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: >>>>>> >>>>>> When net.ipv4.tcp_syncookies=1 and syn flood is happened, >>>>>> cookie_v4_check or cookie_v6_check tries to redo what >>>>>> tcp_v4_send_synack or tcp_v6_send_synack did, >>>>>> rsk_window_clamp will be changed if SOCK_RCVBUF is set, >>>>>> which will make rcv_wscale is different, the client >>>>>> still operates with initial window scale and can overshot >>>>>> granted window, the client use the initial scale but local >>>>>> server use new scale to advertise window value, and session >>>>>> work abnormally. >>>>> >>>>> What is not working exactly ? >>>>> >>>>> Sending a 'big wscale' should not really matter, unless perhaps there >>>>> is a buggy stack at the remote end ? >>>> 1)in tcp_v4_send_synack, if SO_RCVBUF is set and >>>> tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to >>>> tcp_select_initial_window, rcv_wscale will be zero, and send to client, >>>> the client consider wscale is 0; >>>> 2)when ack is back from client, if there is no this patch, >>>> req->rsk_window_clamp is 0, and pass to tcp_select_initial_window, >>>> wscale will be 7, this new rcv_wscale is no way to advertise to client. >>>> 3)if server send rcv_wind to client with window=63, it consider the real >>>> window is 63*2^7=8064, but client consider the server window is only >>>> 63*2^0=63, it can't send big packet to server, and the send-q of client >>>> is full. >>>> >>> >>> I see, please change your patches so that tcp_full_space() is used _once_ >>> >>> listener sk_rcvbuf can change under us. >>> >>> I really have no idea how window can be set to 63, so please send us >>> the packetdrill test once you have it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net v3] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 14:01 ` Eric Dumazet 2020-11-09 16:26 ` Mao Wenan @ 2020-11-09 16:53 ` Mao Wenan 2020-11-09 16:59 ` Eric Dumazet 1 sibling, 1 reply; 16+ messages in thread From: Mao Wenan @ 2020-11-09 16:53 UTC (permalink / raw) To: edumazet, davem, kuznet, yoshfuji, kuba Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan When net.ipv4.tcp_syncookies=1 and syn flood is happened, cookie_v4_check or cookie_v6_check tries to redo what tcp_v4_send_synack or tcp_v6_send_synack did, rsk_window_clamp will be changed if SOCK_RCVBUF is set, which will make rcv_wscale is different, the client still operates with initial window scale and can overshot granted window, the client use the initial scale but local server use new scale to advertise window value, and session work abnormally. Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> --- v3: add local variable full_space, add fixes tag. v2: fix for ipv6. net/ipv4/syncookies.c | 7 ++++++- net/ipv6/syncookies.c | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b..eea4698 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) struct inet_request_sock *ireq; struct tcp_request_sock *treq; struct tcp_sock *tp = tcp_sk(sk); + int full_space = tcp_full_space(sk); const struct tcphdr *th = tcp_hdr(skb); __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; @@ -427,8 +428,12 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) /* Try to redo what tcp_v4_send_synack did. */ req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(&rt->dst, RTAX_INITRWND)); diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index e796a64..5b09bb6 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) struct tcp_request_sock *treq; struct ipv6_pinfo *np = inet6_sk(sk); struct tcp_sock *tp = tcp_sk(sk); + int full_space = tcp_full_space(sk); const struct tcphdr *th = tcp_hdr(skb); __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; @@ -241,7 +242,12 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) } req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + /* limit the window selection if the user enforce a smaller rx buffer */ + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; + + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(dst, RTAX_INITRWND)); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net v3] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 16:53 ` [PATCH net v3] " Mao Wenan @ 2020-11-09 16:59 ` Eric Dumazet 2020-11-09 17:17 ` [PATCH net v4] " Mao Wenan 0 siblings, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 16:59 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Mon, Nov 9, 2020 at 5:54 PM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > When net.ipv4.tcp_syncookies=1 and syn flood is happened, > cookie_v4_check or cookie_v6_check tries to redo what > tcp_v4_send_synack or tcp_v6_send_synack did, > rsk_window_clamp will be changed if SOCK_RCVBUF is set, > which will make rcv_wscale is different, the client > still operates with initial window scale and can overshot > granted window, the client use the initial scale but local > server use new scale to advertise window value, and session > work abnormally. > > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's > rcv-buffer via setsockopt") Please put this tag in a single line (no line wrap) And do not add an empty line after it. The Fixes: tag is part of the official tags, all grouped together. > > Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> > --- > v3: add local variable full_space, add fixes tag. > v2: fix for ipv6. > net/ipv4/syncookies.c | 7 ++++++- > net/ipv6/syncookies.c | 8 +++++++- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c > index 6ac473b..eea4698 100644 > --- a/net/ipv4/syncookies.c > +++ b/net/ipv4/syncookies.c > @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) > struct inet_request_sock *ireq; > struct tcp_request_sock *treq; > struct tcp_sock *tp = tcp_sk(sk); > + int full_space = tcp_full_space(sk); Please delay the actual call to tcp_full_space() until we need it. If a packet does not validate the cookie, no need to init @full_space. > const struct tcphdr *th = tcp_hdr(skb); > __u32 cookie = ntohl(th->ack_seq) - 1; > struct sock *ret = sk; > @@ -427,8 +428,12 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) > > /* Try to redo what tcp_v4_send_synack did. */ > req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); > + /* limit the window selection if the user enforce a smaller rx buffer */ eg: full_space = tcp_full_space(sk); > + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && > + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) > + req->rsk_window_clamp = full_space; > > - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, > + tcp_select_initial_window(sk, full_space, req->mss, > &req->rsk_rcv_wnd, &req->rsk_window_clamp, > ireq->wscale_ok, &rcv_wscale, > dst_metric(&rt->dst, RTAX_INITRWND)); > diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c > index e796a64..5b09bb6 100644 > --- a/net/ipv6/syncookies.c > +++ b/net/ipv6/syncookies.c > @@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) > struct tcp_request_sock *treq; > struct ipv6_pinfo *np = inet6_sk(sk); > struct tcp_sock *tp = tcp_sk(sk); > + int full_space = tcp_full_space(sk); Same remark here. > const struct tcphdr *th = tcp_hdr(skb); > __u32 cookie = ntohl(th->ack_seq) - 1; > struct sock *ret = sk; > @@ -241,7 +242,12 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) > } > > req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); > - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, > + /* limit the window selection if the user enforce a smaller rx buffer */ > + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && > + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) > + req->rsk_window_clamp = full_space; > + > + tcp_select_initial_window(sk, full_space, req->mss, > &req->rsk_rcv_wnd, &req->rsk_window_clamp, > ireq->wscale_ok, &rcv_wscale, > dst_metric(dst, RTAX_INITRWND)); > -- > 1.8.3.1 > Thanks ! ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net v4] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 16:59 ` Eric Dumazet @ 2020-11-09 17:17 ` Mao Wenan 2020-11-09 17:28 ` Eric Dumazet 0 siblings, 1 reply; 16+ messages in thread From: Mao Wenan @ 2020-11-09 17:17 UTC (permalink / raw) To: edumazet, davem, kuznet, yoshfuji, kuba Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan When net.ipv4.tcp_syncookies=1 and syn flood is happened, cookie_v4_check or cookie_v6_check tries to redo what tcp_v4_send_synack or tcp_v6_send_synack did, rsk_window_clamp will be changed if SOCK_RCVBUF is set, which will make rcv_wscale is different, the client still operates with initial window scale and can overshot granted window, the client use the initial scale but local server use new scale to advertise window value, and session work abnormally. Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> --- v4: change fixes tag format, and delay the actual call to tcp_full_space(). v3: add local variable full_space, add fixes tag. v2: fix for ipv6. net/ipv4/syncookies.c | 8 +++++++- net/ipv6/syncookies.c | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b..8784e1f 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) struct inet_request_sock *ireq; struct tcp_request_sock *treq; struct tcp_sock *tp = tcp_sk(sk); + int full_space; const struct tcphdr *th = tcp_hdr(skb); __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; @@ -427,8 +428,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) /* Try to redo what tcp_v4_send_synack did. */ req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + full_space = tcp_full_space(sk); + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(&rt->dst, RTAX_INITRWND)); diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index e796a64..798ede8 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) struct tcp_request_sock *treq; struct ipv6_pinfo *np = inet6_sk(sk); struct tcp_sock *tp = tcp_sk(sk); + int full_space; const struct tcphdr *th = tcp_hdr(skb); __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; @@ -241,7 +242,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) } req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + /* limit the window selection if the user enforce a smaller rx buffer */ + full_space = tcp_full_space(sk); + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; + + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(dst, RTAX_INITRWND)); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net v4] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 17:17 ` [PATCH net v4] " Mao Wenan @ 2020-11-09 17:28 ` Eric Dumazet 2020-11-10 0:16 ` [PATCH net v5] " Mao Wenan 0 siblings, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-09 17:28 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Mon, Nov 9, 2020 at 6:18 PM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > When net.ipv4.tcp_syncookies=1 and syn flood is happened, > cookie_v4_check or cookie_v6_check tries to redo what > tcp_v4_send_synack or tcp_v6_send_synack did, > rsk_window_clamp will be changed if SOCK_RCVBUF is set, > which will make rcv_wscale is different, the client > still operates with initial window scale and can overshot > granted window, the client use the initial scale but local > server use new scale to advertise window value, and session > work abnormally. > > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") > Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> > --- > v4: change fixes tag format, and delay the actual call to > tcp_full_space(). > v3: add local variable full_space, add fixes tag. > v2: fix for ipv6. > net/ipv4/syncookies.c | 8 +++++++- > net/ipv6/syncookies.c | 9 ++++++++- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c > index 6ac473b..8784e1f 100644 > --- a/net/ipv4/syncookies.c > +++ b/net/ipv4/syncookies.c > @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) > struct inet_request_sock *ireq; > struct tcp_request_sock *treq; > struct tcp_sock *tp = tcp_sk(sk); > + int full_space; SGTM. although you could have avoided adding a variable breaking the almost correct reverse Christmas tree that some of us prefer. Something like this would look better : diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b47f30d4d5e5e9455424b1a91d84e649ee..78af720f3e2c6dcdc7298178c5d2f02f0e425e04 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -331,7 +331,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; struct request_sock *req; - int mss; + int full_space, mss; struct rtable *rt; __u8 rcv_wscale; struct flowi4 fl4; ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net v5] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-09 17:28 ` Eric Dumazet @ 2020-11-10 0:16 ` Mao Wenan 2020-11-10 7:32 ` Eric Dumazet 0 siblings, 1 reply; 16+ messages in thread From: Mao Wenan @ 2020-11-10 0:16 UTC (permalink / raw) To: edumazet, davem, kuznet, yoshfuji, kuba Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan When net.ipv4.tcp_syncookies=1 and syn flood is happened, cookie_v4_check or cookie_v6_check tries to redo what tcp_v4_send_synack or tcp_v6_send_synack did, rsk_window_clamp will be changed if SOCK_RCVBUF is set, which will make rcv_wscale is different, the client still operates with initial window scale and can overshot granted window, the client use the initial scale but local server use new scale to advertise window value, and session work abnormally. Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> --- v5: fix variable to adapat to Christmas tree format. v4: change fixes tag format, and delay the actual call to tcp_full_space(). v3: add local variable full_space, add fixes tag. v2: fix for ipv6. net/ipv4/syncookies.c | 9 +++++++-- net/ipv6/syncookies.c | 10 ++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index 6ac473b..00dc3f9 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -331,7 +331,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; struct request_sock *req; - int mss; + int full_space, mss; struct rtable *rt; __u8 rcv_wscale; struct flowi4 fl4; @@ -427,8 +427,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) /* Try to redo what tcp_v4_send_synack did. */ req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW); + /* limit the window selection if the user enforce a smaller rx buffer */ + full_space = tcp_full_space(sk); + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(&rt->dst, RTAX_INITRWND)); diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c index e796a64..9b6cae1 100644 --- a/net/ipv6/syncookies.c +++ b/net/ipv6/syncookies.c @@ -136,7 +136,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) __u32 cookie = ntohl(th->ack_seq) - 1; struct sock *ret = sk; struct request_sock *req; - int mss; + int full_space, mss; struct dst_entry *dst; __u8 rcv_wscale; u32 tsoff = 0; @@ -241,7 +241,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) } req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW); - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss, + /* limit the window selection if the user enforce a smaller rx buffer */ + full_space = tcp_full_space(sk); + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK && + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0)) + req->rsk_window_clamp = full_space; + + tcp_select_initial_window(sk, full_space, req->mss, &req->rsk_rcv_wnd, &req->rsk_window_clamp, ireq->wscale_ok, &rcv_wscale, dst_metric(dst, RTAX_INITRWND)); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net v5] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-10 0:16 ` [PATCH net v5] " Mao Wenan @ 2020-11-10 7:32 ` Eric Dumazet 2020-11-11 1:45 ` Jakub Kicinski 0 siblings, 1 reply; 16+ messages in thread From: Eric Dumazet @ 2020-11-10 7:32 UTC (permalink / raw) To: Mao Wenan Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev, LKML, kernel-janitors On Tue, Nov 10, 2020 at 1:16 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > When net.ipv4.tcp_syncookies=1 and syn flood is happened, > cookie_v4_check or cookie_v6_check tries to redo what > tcp_v4_send_synack or tcp_v6_send_synack did, > rsk_window_clamp will be changed if SOCK_RCVBUF is set, > which will make rcv_wscale is different, the client > still operates with initial window scale and can overshot > granted window, the client use the initial scale but local > server use new scale to advertise window value, and session > work abnormally. > > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") > Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Thanks ! ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net v5] net: Update window_clamp if SOCK_RCVBUF is set 2020-11-10 7:32 ` Eric Dumazet @ 2020-11-11 1:45 ` Jakub Kicinski 0 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2020-11-11 1:45 UTC (permalink / raw) To: Eric Dumazet Cc: Mao Wenan, David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, LKML, kernel-janitors On Tue, 10 Nov 2020 08:32:52 +0100 Eric Dumazet wrote: > On Tue, Nov 10, 2020 at 1:16 AM Mao Wenan <wenan.mao@linux.alibaba.com> wrote: > > When net.ipv4.tcp_syncookies=1 and syn flood is happened, > > cookie_v4_check or cookie_v6_check tries to redo what > > tcp_v4_send_synack or tcp_v6_send_synack did, > > rsk_window_clamp will be changed if SOCK_RCVBUF is set, > > which will make rcv_wscale is different, the client > > still operates with initial window scale and can overshot > > granted window, the client use the initial scale but local > > server use new scale to advertise window value, and session > > work abnormally. > > > > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt") > > Signed-off-by: Mao Wenan <wenan.mao@linux.alibaba.com> > > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied, thanks! ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-11-11 1:45 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-09 9:20 [PATCH] net: Update window_clamp if SOCK_RCVBUF is set Mao Wenan 2020-11-09 9:33 ` [PATCH net v2] " Mao Wenan 2020-11-09 9:56 ` Eric Dumazet 2020-11-09 10:12 ` Mao Wenan 2020-11-09 10:19 ` Mao Wenan 2020-11-09 11:02 ` Eric Dumazet 2020-11-09 11:41 ` Eric Dumazet 2020-11-09 14:01 ` Eric Dumazet 2020-11-09 16:26 ` Mao Wenan 2020-11-09 16:53 ` [PATCH net v3] " Mao Wenan 2020-11-09 16:59 ` Eric Dumazet 2020-11-09 17:17 ` [PATCH net v4] " Mao Wenan 2020-11-09 17:28 ` Eric Dumazet 2020-11-10 0:16 ` [PATCH net v5] " Mao Wenan 2020-11-10 7:32 ` Eric Dumazet 2020-11-11 1:45 ` Jakub Kicinski
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).