From mboxrd@z Thu Jan 1 00:00:00 1970 From: Li Yu Subject: Re: v3 for tcp friends? Date: Wed, 23 Jan 2013 17:52:18 +0800 Message-ID: <50FFB2D2.4050603@gmail.com> References: <20120903.154833.1547153833820955116.davem@davemloft.net> <20120904.125841.2293649688957878987.davem@davemloft.net> <50FCEE64.80203@gmail.com> <50FCEEC5.9010404@gmail.com> <50FF5592.60008@gmail.com> <50FF7F64.2060902@gmail.com> <1358923606.12374.746.camel@edumazet-glaptop> <50FF8F75.4000903@gmail.com> <50FF9836.4060106@gmail.com> <50FFAFE7.5030707@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: David Miller , Eric Dumazet , Bruce Curtis To: netdev Return-path: Received: from mail-da0-f44.google.com ([209.85.210.44]:52866 "EHLO mail-da0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754670Ab3AWJwe (ORCPT ); Wed, 23 Jan 2013 04:52:34 -0500 Received: by mail-da0-f44.google.com with SMTP id z20so3726087dae.17 for ; Wed, 23 Jan 2013 01:52:32 -0800 (PST) In-Reply-To: <50FFAFE7.5030707@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: =E4=BA=8E 2013=E5=B9=B401=E6=9C=8823=E6=97=A5 17:39, Li Yu =E5=86=99=E9= =81=93: > =E4=BA=8E 2013=E5=B9=B401=E6=9C=8823=E6=97=A5 15:58, Li Yu =E5=86=99=E9= =81=93: >> =E4=BA=8E 2013=E5=B9=B401=E6=9C=8823=E6=97=A5 15:21, Li Yu =E5=86=99= =E9=81=93: >>> =E4=BA=8E 2013=E5=B9=B401=E6=9C=8823=E6=97=A5 14:46, Eric Dumazet =E5= =86=99=E9=81=93: >>>> On Wed, 2013-01-23 at 14:12 +0800, Li Yu wrote: >>>>> Oops, this hang is not since TCP friends patch! >>>>> >>>>> sk_sndbuf_get() is broken by 32 bits integer overflow >>>>> because of so large value in net.ipv4.tcp_{rmem,wmem}. >>>>> >>>>> but this hang also can be found in net-next.git >>>>> (3.8.0-rc3+), if we run below commands, then all new >>>>> TCP connections stop working! >>>>> >>>>> # when TCP friends is disabled >>>>> sysctl -w net.ipv4.tcp_rmem=3D"4096 4294967296 4294967296" # 4GB >>>>> sysctl -w net.ipv4.tcp_wmem=3D"4096 4294967296 4294967296" >>>> >>>> Right we need to make sure we dont overflow. >>>> >>>> Try the following fix : >>>> >>>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4= =2Ec >>>> index a25e1d2..1459145 100644 >>>> --- a/net/ipv4/sysctl_net_ipv4.c >>>> +++ b/net/ipv4/sysctl_net_ipv4.c >>>> @@ -549,14 +549,16 @@ static struct ctl_table ipv4_table[] =3D { >>>> .data =3D &sysctl_tcp_wmem, >>>> .maxlen =3D sizeof(sysctl_tcp_wmem), >>>> .mode =3D 0644, >>>> - .proc_handler =3D proc_dointvec >>>> + .extra1 =3D &zero, > > If we added below: > > +static int one =3D 1; > +static int int_max =3D INT_MAX; > .... > + .extra1 =3D &one, > + .extra2 =3D &int_max, > The "int_max" may be unnecessary here :) > The bug is fixed without TCP friends. > > Maybe, TCP friends need to use long integer to record result > of "sk->sk_sndbuf + sk->sk_friend->sk_rcvbuf" ? > > BTW: This overflow problem also breaks UDP sockets. Unix domain sockets is broken too, and any other ? thanks Yu > > Thanks > > Yu > >>>> + .proc_handler =3D proc_dointvec_minmax >>>> }, >>>> { >>>> .procname =3D "tcp_rmem", >>>> .data =3D &sysctl_tcp_rmem, >>>> .maxlen =3D sizeof(sysctl_tcp_rmem), >>>> .mode =3D 0644, >>>> - .proc_handler =3D proc_dointvec >>>> + .extra1 =3D &zero, >>>> + .proc_handler =3D proc_dointvec_minmax >>>> }, >>>> { >>>> .procname =3D "tcp_app_win", >>>> >>>> >>>> >>> Thanks for so quick reply, I will test it soon. >>> >>> however I suspect whether this patch could fix overflow if we merge= d TCP >>> friends patch in future. >>> >> >> Tested on 3.7.0-rc1+, but bug is still alive >> with disabled TCP friends ... >> >> Thanks >> >> Yu >> >>> With TCP friends, we have source like below, or TCP friends should = care >>> this ? >>> >>> static inline int sk_sndbuf_get(const struct sock *sk) >>> { >>> if (sk->sk_friend) >>> return sk->sk_sndbuf + sk->sk_friend->sk_rcvbuf; >>> else >>> return sk->sk_sndbuf; >>> } >>> >>> static inline bool sk_stream_memory_free(const struct sock *sk) >>> { >>> return sk_wmem_queued_get(sk) < sk_sndbuf_get(sk); >>> } >>> >>> So sk_sndbuf_get() still may be overflow when we have right value i= n >>> net.ipv4.tcp_{rmem,wmem}. >>> >>> Thanks >> >