Date: Wed, 12 Oct 2016 17:17:35 +0200 From: Julien Charbon <jch@freebsd.org> To: Slawa Olhovchenkov <slw@zxy.spb.ru> Cc: Konstantin Belousov <kostikbel@gmail.com>, freebsd-stable@FreeBSD.org, hiren panchasara <hiren@strugglingcoder.info> Subject: Re: 11.0 stuck on high network load Message-ID: <e8a46471-576d-e074-8a50-5c316fb98bce@freebsd.org> In-Reply-To: <20161012130103.GD57714@zxy.spb.ru> References: <20161011121145.GJ6177@zxy.spb.ru> <f1d9e34e-3d85-bd02-e660-6d647e4343fb@freebsd.org> <20161012084045.GA57714@zxy.spb.ru> <f3c0e73a-5e6e-2190-aed3-499250c1764c@freebsd.org> <20161012092945.GB57714@zxy.spb.ru> <4b0d4b58-6d13-3cd5-6991-27163f27acca@freebsd.org> <20161012095233.GC57714@zxy.spb.ru> <e4f1343c-636a-0970-856b-e65955f79e1a@freebsd.org> <20161012121322.GB57876@zxy.spb.ru> <62d8861c-673e-6d86-e96e-751399e505e5@freebsd.org> <20161012130103.GD57714@zxy.spb.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--9RACJdhco9AOMMH2HacDKhuumGKC9UBNd
Content-Type: multipart/mixed; boundary="9bJTnMPbIV13a4h14mNLwvCpKSwoufKs7";
protected-headers="v1"
From: Julien Charbon <jch@freebsd.org>
To: Slawa Olhovchenkov <slw@zxy.spb.ru>
Cc: Konstantin Belousov <kostikbel@gmail.com>, freebsd-stable@FreeBSD.org,
hiren panchasara <hiren@strugglingcoder.info>
Message-ID: <e8a46471-576d-e074-8a50-5c316fb98bce@freebsd.org>
Subject: Re: 11.0 stuck on high network load
References: <20161011121145.GJ6177@zxy.spb.ru>
<f1d9e34e-3d85-bd02-e660-6d647e4343fb@freebsd.org>
<20161012084045.GA57714@zxy.spb.ru>
<f3c0e73a-5e6e-2190-aed3-499250c1764c@freebsd.org>
<20161012092945.GB57714@zxy.spb.ru>
<4b0d4b58-6d13-3cd5-6991-27163f27acca@freebsd.org>
<20161012095233.GC57714@zxy.spb.ru>
<e4f1343c-636a-0970-856b-e65955f79e1a@freebsd.org>
<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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?e8a46471-576d-e074-8a50-5c316fb98bce>