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-all@freebsd.org Tue Oct 23 04:31:51 2018 Return-Path: <owner-svn-src-all@freebsd.org> Delivered-To: svn-src-all@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-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" <svn-src-all.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-all>, <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>; List-Post: <mailto:svn-src-all@freebsd.org> List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-all>, <mailto:svn-src-all-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>