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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
