Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 22 Oct 2018 21:17:39 -0700
From:      "Kristof Provost" <kp@FreeBSD.org>
To:        "Andrey V. Elsukov" <ae@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r339554 - head/sys/net
Message-ID:  <5375183F-6F8B-4457-9CCD-511E214B5B16@FreeBSD.org>
In-Reply-To: <6FD6264C-06D6-40F4-8EED-B4B1AD950214@FreeBSD.org>
References:  <201810211824.w9LIOLuu094155@repo.freebsd.org> <6FD6264C-06D6-40F4-8EED-B4B1AD950214@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 22 Oct 2018, at 17:51, Kristof Provost wrote:
> On 21 Oct 2018, at 11:24, Andrey V. Elsukov wrote:
>> Author: ae
>> Date: Sun Oct 21 18:24:20 2018
>> New Revision: 339554
>> URL: https://svnweb.freebsd.org/changeset/base/339554
>>
>> Log:
>>   Rework if_ipsec(4) to use epoch(9) instead of rmlock.
>>
>>   * use CK_LIST and FNV hash to keep chains of softc;
>>   * read access to softc is protected by epoch();
>>   * write access is protected by ipsec_ioctl_sx. Changing of softc 
>> fields
>>     is allowed only when softc is unlinked from CK_LIST chains.
>>   * linking/unlinking of softc is allowed only when ipsec_ioctl_sx is
>>     exclusive locked.
>>   * the plain LIST of all softc is replaced by hash table that uses 
>> ingress
>>     address of tunnels as a key.
>>   * added support for appearing/disappearing of ingress address 
>> handling.
>>     Now it is allowed configure non-local ingress IP address, and 
>> thus the
>>     problem with if_ipsec(4) configuration that happens on boot, when
>>     ingress address is not yet configured, is solved.
>>
>>   MFC after:	1 month
>>   Sponsored by:	Yandex LLC
>>   Differential Revision:	https://reviews.freebsd.org/D17190
>>
> This panics during the pf tests.
I think I understand what’s happening here.

With this patch I see a different panic:

	diff --git a/sys/net/if_ipsec.c b/sys/net/if_ipsec.c
	index 91b11a455b3..146cb59aaaa 100644
	--- a/sys/net/if_ipsec.c
	+++ b/sys/net/if_ipsec.c
	@@ -134,6 +134,9 @@ ipsec_srchash(const struct sockaddr *sa)
	 {
	        uint32_t hval;

	+       KASSERT(V_ipsec4_srchtbl, ("NULL"));
	+       KASSERT(V_ipsec6_srchtbl, ("NULL (v6)"));
	+
	        switch (sa->sa_family) {
	 #ifdef INET
	        case AF_INET:
	@@ -265,17 +274,22 @@ static void
	 vnet_ipsec_uninit(const void *unused __unused)
	 {

	        if_clone_detach(V_ipsec_cloner);
	        free(V_ipsec_idhtbl, M_IPSEC);
	 #ifdef INET
	        if (IS_DEFAULT_VNET(curvnet))
	                ip_encap_unregister_srcaddr(ipsec4_srctab);
	        free(V_ipsec4_srchtbl, M_IPSEC);
	+       V_ipsec4_srchtbl = NULL;
	+
	 #endif
	 #ifdef INET6
	        if (IS_DEFAULT_VNET(curvnet))
	                ip6_encap_unregister_srcaddr(ipsec6_srctab);
	        free(V_ipsec6_srchtbl, M_IPSEC);
	+       V_ipsec4_srchtbl = NULL;
	 #endif
	 }
	 VNET_SYSUNINIT(vnet_ipsec_uninit, SI_SUB_PROTO_IFATTACHDOMAIN, 
SI_ORDER_ANY,

That trips the KASSERT() in ipsec_srchash(). Basically, the problem is 
that the V_ipsec4_srchtbl table gets freed in vnet_ipsec_uninit(), and 
later we get a srcaddr_change_event(), which tries to iterate the list 
(in ipsec_srcaddr()). Obviously iterating a freed list doesn’t go well 
for us (hence the 0xdeadc0dedeadc0de softc).

That also explains why this happens during the pf tests, even though 
they don’t actually use IPSec.

I’m not quite sure how to best fix this though. I suppose we could set 
V_ipsec4_srchtbl to NULL (well, copy the pointer first, set it to NULL 
and then free it so we don’t add a race condition) and then check for 
NULL in  ipsec_srcaddr().
It feels like the srcaddr_change_event needs to be per-vnet, so we can 
unregister before we free V_ipsec4_srchtbl.

Best regards,
Kristof
From owner-svn-src-head@freebsd.org  Tue Oct 23 04:31:51 2018
Return-Path: <owner-svn-src-head@freebsd.org>
Delivered-To: svn-src-head@mailman.ysv.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1])
 by mailman.ysv.freebsd.org (Postfix) with ESMTP id 829E7103627F;
 Tue, 23 Oct 2018 04:31:51 +0000 (UTC)
 (envelope-from nwhitehorn@freebsd.org)
Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80])
 (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
 (Client did not present a certificate)
 by mx1.freebsd.org (Postfix) with ESMTPS id 0A6E188D84;
 Tue, 23 Oct 2018 04:31:50 +0000 (UTC)
 (envelope-from nwhitehorn@freebsd.org)
Received: from comporellon.tachypleus.net (cpe-75-82-218-62.socal.res.rr.com
 [75.82.218.62]) (authenticated bits=0)
 by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id w9N4VgJ5025780
 (version=TLSv1.2 cipher=AES128-SHA bits=128 verify=NOT);
 Mon, 22 Oct 2018 21:31:43 -0700
Subject: Re: svn commit: r339523 - in head/sys: conf dev/amdgpio modules
 modules/amdgpio
To: Oleksandr Tymoshenko <gonzo@freebsd.org>
Cc: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
References: <201810210452.w9L4qbTc067553@repo.freebsd.org>
 <49432512-861b-47dd-4e25-08044b0b53fa@freebsd.org>
 <20181021201214.GA98172@bluezbox.com>
From: Nathan Whitehorn <nwhitehorn@freebsd.org>
Openpgp: preference=signencrypt
Autocrypt: addr=nwhitehorn@freebsd.org; keydata=
 xsFNBFuARN8BEADLKYsG3l1aq/M21R59I/5EsEfvtvd15ZJ9lDHcWPuxzIfGnu2LMpe5PrFP
 e/Y4bcsPrlB4S3I3ooIUDvoEEsDeqgqlZod3QevOK/RjLqiqx1i/4mKnobJ++3ppyVVIccgN
 sUrj786OYCFCI/W+uWw7cbKewNeaL//Z/TDKlHLkssiy6qmZbNQ0ZjcMLJKUesk4eVg2TtTD
 HNe42ZuxbUC9iLYieO4c7kQB4qiFhagDRiObXrLzvm2MQYeAaNVRqID+mfI75TWrQ+t98iVu
 mHvFu461eeteq59jg6H/IL07ACxL+HzEVM+D6tPtPrz7ppr3wiZL5Cu17yu0nAx0nhJTV8ZB
 qza1rOVun0x65S14L41XD2HkmBDxTaRlTg8ypnkLFo8kh+MEq4k67apL/DUGcaUjKy2TVUC7
 3igLO/DwQHrkWx2RrOmS3xS0TgGXVmB47nq2Zveo3fcjporQK63n2sbLkS70cfAJAJ9KHEIx
 u9am44iW5Ku3+mVLgQYybtcUxlk/Jw/BA5V6KUcDQMd5kTm0MyagziqMaT+57ceYxwRBK4HC
 DCLRpSOHV81/YzyL5vnwfHsxADm3091rd0uwr8uRCQn7wLvlcFyp/JKSFkVnE1oo7UE4QQJZ
 GbSJyvj7GdXu0LdghALcMj/thdb+js4D3UuCaAMecgVSscxEIQARAQABzS5OYXRoYW4gV2hp
 dGVob3JuIDxud2hpdGVob3JuQGljZWN1YmUud2lzYy5lZHU+wsGOBBMBCAA4FiEEPWQg+qgh
 ST6Avw1hOLZNlGaE6HcFAluAUl4CGwMFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQOLZN
 lGaE6HeHFw//fN2CzkiW1yedjLGEQ3uXRMgu6geRgWdtkgg/pOhn5OLSQI4R59kjvHNHHqln
 1QYdxe63lsbe+7CRsTKuke3/mgsQ1h6n7cCzsoXVP3eLtWjshAz7spwUcdRRFTSbwkMzKcRn
 plpr+ByZDw98vnpQo10J5xYmf6if6wcEpVlazwnC5G1gHktM4X0jrlAUKSgVx7MG8o4G6af9
 7MQJINAG6g6+BlBH3u5fmPunKi9qgHsZxKnTZneD2mO5u2x6p1qmqybvfvWI5UEvktPeEext
 JBeXXqdex+HWmAbLYznLUoBloBl+fW5Lo9VapkcGubvGC0WLr/gYuamAApwoFpa4/SBqgmHr
 JSsPqoDtheWt9oYzuYkYW5+tpJQoDVdG3KCOQJSYZIbNT8HyviFY32ZSn6gIn2qP/5E+rA8J
 +6/57XCZXPazE4FiPNbwY/OwZoi7w0yRKcdmJoSoC/GdjtQbjyeARbAaHIcudeU/bB3paGS2
 rHkoW4iR5TnK/V/QJvNT4KC6Dw3m6pfBIzsF+smcyjz+MzBFQNvSOtG4kJQooKcMsThas8oX
 VTx+WsqNAVOeQKTOU74jlUYKm5+w5aIOJbc8jg1LlTWJus1SxYtxT9lgNwOCUhE/j6Ueq9jG
 V9POWg31C44akWmnK8rS4PImUBsPKwtxUNM6BhfZRbtIjQ7OwU0EW4BFQQEQAOLKFtg6us0A
 LA7LtvjxIskIgqFJjHw2ka/UtdJ432P9kvmBq7z4v0+m/gkxCOOG0yDi2Cv/ALJobsyb56tb
 U6MU0SRjTio35S2jit369B1BDC2TLNF337sUquUx0l4wkEXEBefvLRYouF8BRbkgjveg7sA4
 NjsiduQx92vPJnBoaH2OWxqDbr5X6kF5cx9jPrKUJ4ZqH/raE/SSDhtow4aKO0nWbteVGck1
 5W1X/S8KziPXKazxCQ8qprQRTAehsdG/bSbWD95hp3TAlEbl4N4UqqS7n4jCZunCeii2TDZH
 Vvx/lpFAT2ezx646p2PUmH5hpiVMgbY5uHcyahwNf+eNOO7gotnNYoieoLw4fUeTYOq+s3IN
 isCB4iovQcZOCYSzmwRolQRggX0tBSenR6Pgp38YjVIkWvMHhxbVifAusjvVbm/GQeA2MaCt
 kog53Iyfo7ri9DeNpVuRc/47BxHi8JtdyyGgLO13Ajcwc6V7KLeDmw/SXJAMssuWQlXzs8Og
 spNvtymBh5rq4TlxAY65L3Yv/yh0izEztOJO3Ob9y3gLrp7TeDI0EO9SyGuFXbgWY/NXlDwW
 HWguMgO7DWM/KxeaMYyHfnffIeQ6uhM21y42I7NV11mWwycv/XJkID3fd7GWBecakdYnYI/7
 FYMDHmsUQPmSMkbqCqYcZe47ABEBAAHCwXYEGAEIACAWIQQ9ZCD6qCFJPoC/DWE4tk2UZoTo
 dwUCW4BFQQIbDAAKCRA4tk2UZoTod2RbEACbQ2bwJ3++bvqclErbekf7BXYja37/HxGE67q3
 9xf28hen8vWGtXwq4bWmZT5H8bBqXigA4bUU4nN4X3xEDfTyqkQMuDTnnwT7Y61B4QEqhi4a
 q4adf/KP0l1UCg4CJ0KS931Han+VbiuUcbadu1ZX37Ef6g/hG+mt59FeXDMU0rers2Bpr8zB
 8ywojAsVC92kvOHLsCQtdCsPzC+R6B1bY6/Re9slM1NBd+2k4BUVhYu8Fb8Ir37OmN0aGQzY
 uRczfrmR/OV5/1+g5XeYSFbq/0Q3KkFWLHfimff8lb9GRWrdvOUpYyGluv49b/G5o9lSxPwX
 yBfaoVi/WDDfJ/XJw9H90XK68TYxPfEQkeuLEEzg+Bz3Zeduyo2Zx4S5apLqAbv0RzduXgIG
 YZVPu8R4ya8nQWHeUpot17lt8SL7yFkMJaAXk27QqUAaxjqnGBLn70YMWXFGySfvjgaR1Ftu
 /S/HSKqH7m8aFYZftqs7ZojXNdqGHZKRrIx6hRUYuZQM8uxHDweF4jF+QIwYIUmtry5h8iti
 Sjt9KHjpkH3Wz5o1mk6cbFNN+wgpHplDl/iZMZjFskTAJfEsYHVSSm21zcYvvogrbqYvciMT
 ty65+0A8Gz9tMbcNx9ePaGoM+9jeFehrzTjdaiTiC+umSd/Y29DCW4OBMr1VfufVVKbfAQ==
Message-ID: <ad1867d0-a354-ea04-d099-46dd98f21ed9@freebsd.org>
Date: Mon, 22 Oct 2018 21:31:41 -0700
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181021201214.GA98172@bluezbox.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
X-Sonic-CAuth: UmFuZG9tSVbv89VbhpbGnXrlXTQsnzo5pN8qcX92vlB9PRhIarcHQpVr0wJmTV13t+IhlM/SKQcuhH6OGG+AVSmplmFDHQQMbKFhkN54Tlo=
X-Sonic-ID: C;9C+CinzW6BGH7OcbDaCztA== M;anbtinzW6BGH7OcbDaCztA==
X-Spam-Flag: No
X-Sonic-Spam-Details: 0.0/5.0 by cerberusd
X-BeenThere: svn-src-head@freebsd.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
 <svn-src-head.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>;
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 23 Oct 2018 04:31:51 -0000



On 10/21/18 1:12 PM, Oleksandr Tymoshenko wrote:
> Nathan Whitehorn (nwhitehorn@freebsd.org) wrote:
>>
>> On 10/20/18 9:52 PM, Oleksandr Tymoshenko wrote:
>>> Author: gonzo
>>> Date: Sun Oct 21 04:52:37 2018
>>> New Revision: 339523
>>> URL: https://svnweb.freebsd.org/changeset/base/339523
>>>
>>> Log:
>>>   Add amdgpio, driver for GPIO controller on AMD-based x86_64 platforms
>>>   
>>>   Submitted by:	Rajesh Kumar <rajbsd@gmail.com>
>>>   Differential Revision:	https://reviews.freebsd.org/D16865
>>>
>> [...]
>>> Modified: head/sys/modules/Makefile
>>> ==============================================================================
>>> --- head/sys/modules/Makefile	Sun Oct 21 02:39:13 2018	(r339522)
>>> +++ head/sys/modules/Makefile	Sun Oct 21 04:52:37 2018	(r339523)
>>> @@ -34,6 +34,7 @@ SUBDIR=	\
>>>  	ale \
>>>  	alq \
>>>  	${_amd_ecc_inject} \
>>> +	${_amdgpio} \
>>>  	${_amdsbwd} \
>>>  	${_amdsmn} \
>>>  	${_amdtemp} \
>>> @@ -717,6 +718,7 @@ _x86bios=	x86bios
>>>  .endif
>>>  
>>>  .if ${MACHINE_CPUARCH} == "amd64"
>>> +_amdgpio=	amdgpio
>>>  _ccp=		ccp
>>>  _efirt=		efirt
>>>  _iavf=		iavf
>>>
>> Does this not work on 64-bit AMD processors running i386 kernels?
> I see no reason why it wouldn't. Probably just haven't been tested.
>

It might be nice if we just added it to all architectures where it
conceivably applies. Essentially all of our code is more portable than
the platforms on which it is tested, so there doesn't seem to be a good
reason to limit such things.
-Nathan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5375183F-6F8B-4457-9CCD-511E214B5B16>