From owner-freebsd-stable@freebsd.org Wed Oct 12 15:17:51 2016 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4E9DBC0F7A6 for ; Wed, 12 Oct 2016 15:17:51 +0000 (UTC) (envelope-from julien.charbon@gmail.com) Received: from mail-lf0-f47.google.com (mail-lf0-f47.google.com [209.85.215.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id EE152CC1 for ; Wed, 12 Oct 2016 15:17:50 +0000 (UTC) (envelope-from julien.charbon@gmail.com) Received: by mail-lf0-f47.google.com with SMTP id l131so46686537lfl.2 for ; Wed, 12 Oct 2016 08:17:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=NCthbeygx3Gm7GeKSL/Dqm0XGzA2Cry3iKdPYiJVy7s=; b=lBXTBSp7WoDCUlOzQuIdu5sqmK3ts08mtPfNu8Ki0BQuxFaGjKOrrGFHr3ZmJ5vEf9 0mnHegAHyYTzGaoGq5o2mFIla1zVeUvHtSg5v2KE+JlvEIh9codPKaQUAOceG8MiymLs O5XZKHnnBM8aa4nL+D0EXz3oiPlem/tpRjJRpd07k93Y3x6vIVuZk3y7jUSM3bqjEgl3 iO4Sj1UdfGr8NvSW5wK+IymIvYWlw1CfHuC1lAsXr0MpLP2VtDiO1pjheQvjS64QoW4/ 6nrODUQzsx24ci8k9avKsz6nBVhODketAZ2VQpVz6cADn2r99D+VgXE3oQGl9uG03jYD hbYw== X-Gm-Message-State: AA6/9RmCCkg9A8AHvz6kd3S1OaW06sp1LxDO2939Z2mfzbZPDq2VJKDgaWNc+V6ML2lQmw== X-Received: by 10.25.29.1 with SMTP id d1mr2013538lfd.121.1476285462905; Wed, 12 Oct 2016 08:17:42 -0700 (PDT) Received: from [10.100.64.21] ([217.30.88.7]) by smtp.gmail.com with ESMTPSA id 87sm2393679lfs.0.2016.10.12.08.17.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Oct 2016 08:17:42 -0700 (PDT) Subject: Re: 11.0 stuck on high network load To: Slawa Olhovchenkov References: <20161011121145.GJ6177@zxy.spb.ru> <20161012084045.GA57714@zxy.spb.ru> <20161012092945.GB57714@zxy.spb.ru> <4b0d4b58-6d13-3cd5-6991-27163f27acca@freebsd.org> <20161012095233.GC57714@zxy.spb.ru> <20161012121322.GB57876@zxy.spb.ru> <62d8861c-673e-6d86-e96e-751399e505e5@freebsd.org> <20161012130103.GD57714@zxy.spb.ru> Cc: Konstantin Belousov , freebsd-stable@FreeBSD.org, hiren panchasara From: Julien Charbon Message-ID: Date: Wed, 12 Oct 2016 17:17:35 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161012130103.GD57714@zxy.spb.ru> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="9RACJdhco9AOMMH2HacDKhuumGKC9UBNd" X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Oct 2016 15:17:51 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9RACJdhco9AOMMH2HacDKhuumGKC9UBNd Content-Type: multipart/mixed; boundary="9bJTnMPbIV13a4h14mNLwvCpKSwoufKs7"; protected-headers="v1" From: Julien Charbon To: Slawa Olhovchenkov Cc: Konstantin Belousov , freebsd-stable@FreeBSD.org, hiren panchasara Message-ID: Subject: Re: 11.0 stuck on high network load References: <20161011121145.GJ6177@zxy.spb.ru> <20161012084045.GA57714@zxy.spb.ru> <20161012092945.GB57714@zxy.spb.ru> <4b0d4b58-6d13-3cd5-6991-27163f27acca@freebsd.org> <20161012095233.GC57714@zxy.spb.ru> <20161012121322.GB57876@zxy.spb.ru> <62d8861c-673e-6d86-e96e-751399e505e5@freebsd.org> <20161012130103.GD57714@zxy.spb.ru> In-Reply-To: <20161012130103.GD57714@zxy.spb.ru> --9bJTnMPbIV13a4h14mNLwvCpKSwoufKs7 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Hi Slawa, On 10/12/16 3:01 PM, Slawa Olhovchenkov wrote: > On Wed, Oct 12, 2016 at 02:35:11PM +0200, Julien Charbon wrote: >> On 10/12/16 2:13 PM, Slawa Olhovchenkov wrote: >>> On Wed, Oct 12, 2016 at 02:06:59PM +0200, Julien Charbon wrote: >>>>>>>>> sofree() call tcp_usr_detach() and in tcp_usr_detach() we have >>>>>>>>> unexpected INP_TIMEWAIT. >>>>>>>> >>>>>>>> I see, thus just for the context: The TCP stack in sys/dev/cxg= b* is a >>>>>>>> TOE (TCP Offload Engine?) TCP stack for Chelsio NICs, it is a >>>>>>>> separate/side TCP stack that is used only with TCP_OFFLOAD optio= n. >>>>>>>> >>>>>>>> This TOE TCP stack actually has its own set of detach()/input()= >>>>>>>> functions and seems to check INP_DROPPED flag properly. I guess= @np >>>>>>>> check fixes in socket TCP stack and decides which one can also i= mpact >>>>>>>> the Chelsio TOE TCP stack. Some bugs are only in socket TCP sta= ck, some >>>>>>>> are only in TOE TCP stack. >>>>>>> >>>>>>> I am fear about other direction -- setting INP_TIMEWAIT in Chelsi= o TOE >>>>>>> TCP stack and impact this to >>>>>>> tcp_timer_2msl()/tcp_close()/sofree()/tcp_usr_detach() path. >>>>>> >>>>>> I see, I expect no problem on this side as tcp_timer_2msl() check= s the >>>>>> INP_TIMEWAIT flag and do not call tcp_close() if set. >>>>> >>>>> I am about case when at time of first INP_WUNLOCK() tcp_timer_2msl(= ) >>>>> don't see INP_TIMEWAIT, call tcp_close(), tcp_close() do INP_WUNLOC= K() >>>>> and now Chelsio TOE take INP_WLOCK, do tcp_twstart() and set >>>>> INP_TIMEWAIT. After this tcp_timer_2msl resume and have unexpected >>>>> INP_TIMEWAIT in tcp_usr_detach(). >>>> >>>> Sure, basically the same bug that in classic TCP stack. If you thi= nk >>>> it can happen, send an email describing that to np@ and he will chec= k >>>> and fix that. He is a TOE TCP stack expert and I am not. In all ca= ses, >>>> if this issue is possible in TOE TCP stack context, the patch will b= e >>>> straightforward: If the INP_DROPPED flag is set do not call tcp_tws= tart(). >>>> >>>> The current patch focuses only on the classic TCP stack. >>> >>> May be current workaround (with logging) in tcp_usr_detach() is good >>> solutuion for preventing system lockout by similar bugs? >> >> Good question, the quick workaround in tcp_usr_detach() does not hand= le >> all the cases. If it reduces the number of crashes you can still find= >> scenarios where it can have unexpected side effect. >=20 > This is best then guaranted lockout. >=20 >> Long term solution is to enforce: If the inp has the INP_DROPPED fla= g >> just stop processing it and return. If you grep the INP_DROPPED flag = in >> kernel sources, you can see that this test is already done in almost a= ll >> tcp_*() processing functions but tcp_input(). >> >> I would say that even without this issue tcp_input() should check >> INP_DROPPED flags after INP_WLOCK anyway. Same for the TOE TCP stack,= >> you are simply not supposed to process a inp with INP_DROPPED flag. >=20 > Absolutly acceptant! > May point is: more check and good handling of check result is best for > stability. >=20 > I.e. AND check INP_DROPPED in tcp_input AND workaroud INP_TIMEWAIT in > tcp_usr_detach (with logging) and check of some posible cases in XXX TO= E. >=20 > Current TCP stack too complex and have many corner cases. This is need > additional guards where posible (not caused kernel panic). I see your point: Even if this issue is caught by this assert: KASSERT(tp =3D=3D NULL, ("tcp_detach: INP_TIMEWAIT && " "INP_DROPPED && tp !=3D NULL")); https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/tcp_us= rreq.c#L213 you might not have INVARIANT option, then you will get a lockout quite difficult to debug. Thus what we can do is: - If INVARIANT is set: kernel panic to get all the details in the core.= - If INVARIANT is not set: Log this error with an explicit kernel log(LOG_ERR) describing the issue, and then use the workaround to avoid the double-free to let the system to good enough state. Something like: tcp_detach() { ... if (inp->inp_flags & INP_TIMEWAIT) { ... if (inp->inp_flags & INP_DROPPED) { in_pcbdetach(inp); if (__predict_true(tp =3D=3D NULL)) { in_pcbfree(inp); } else { #ifdef INVARIANTS panic("tcp_detach: tp !=3D NULL, That's not good because 'blah'\n= "); #else log(LOG_ERR, "tcp_detach: tp !=3D NULL, That's not good because 'blah'\n"); #endif INP_WUNLOCK(inp); } } } =2E.. } -- Julien --9bJTnMPbIV13a4h14mNLwvCpKSwoufKs7-- --9RACJdhco9AOMMH2HacDKhuumGKC9UBNd Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQEcBAEBCgAGBQJX/lQVAAoJEKVlQ5Je6dhxjs4H/R2s88vWMX7pZf18nWtnvHhV bfSxX4ZTlwczbqsmzEhx8VdvwrbU1aZJsrBkFFqIV7ccxKxVdfQYxZajDqFLkShU a7VuqzYN5p+hNGkEgvt315KVRVl5ABTiFikKm2heMtvFnlrn3FO1HbuAyrVSdWlD QUw7+ecIU5RFpMJlc1VkRJPdSAKS+lCnZcfzvOdc5VHvwNSIW2atKXa3Wvw7nDcO XAACGSgXpeZRyi0+3iIhlc6+uwRIOFj9QdPso5vxx4Y9YTyI7scfdl1wxXi8AlOG fnhyBE6VhVf0DyIg9n6sddYFtwhR+eh4y501hNhKe20F8vSJbTEFVwTdznfupcs= =hbxA -----END PGP SIGNATURE----- --9RACJdhco9AOMMH2HacDKhuumGKC9UBNd--