Date: Fri, 16 Jan 2004 09:31:25 +0000 (UTC) From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: Hajimu UMEMOTO <ume@FreeBSD.org> Cc: Jun-ichiro itojun Hagino <itojun@itojun.org> Subject: Re: [PATCH] IPSec fixes Message-ID: <Pine.BSF.4.53.0401160829530.66397@e0-0.zab2.int.zabbadoz.net> In-Reply-To: <ygeeku0uxas.wl%ume@FreeBSD.org> References: <Pine.BSF.4.53.0401140934230.30149@e0-0.zab2.int.zabbadoz.net> <20040115041435.7B739A6@coconut.itojun.org> <Pine.BSF.4.53.0401152212230.66397@e0-0.zab2.int.zabbadoz.net> <ygeeku0uxas.wl%ume@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 16 Jan 2004, Hajimu UMEMOTO wrote:
Hi,
> Do you mean that following patch itojun offered doesn't help for you?
I never applied it as going through the code showed that it will not
do the right thing(TM). See older mail.
> It seems fix the problem here.
ahh - this isn't exactly the diff itojun offered.
the newsp->refcnt++; is missing. Is it also missing in your kernel ?
I am currently offsite but could you please try to add this
- shouldn't give too much debugging output; depending on number of
SPs ypu have - do some setkey spdadd and sdpflush and
restart your ike daemon possibly running and mail me the output ?
> Index: sys/netkey/key.c
> diff -up sys/netkey/key.c.orig sys/netkey/key.c
> --- sys/netkey/key.c.orig Fri Jan 16 17:06:26 2004
> +++ sys/netkey/key.c Fri Jan 16 17:07:38 2004
> @@ -1958,7 +1958,6 @@ key_spdadd(so, m, mhp)
> newsp->lifetime = lft ? lft->sadb_lifetime_addtime : 0;
> newsp->validtime = lft ? lft->sadb_lifetime_usetime : 0;
>
> - newsp->refcnt = 1; /* do not reclaim until I say I do */
> newsp->state = IPSEC_SPSTATE_ALIVE;
> LIST_INSERT_TAIL(&sptree[newsp->dir], newsp, secpolicy, chain);
>
> @@ -7591,9 +7590,10 @@ key_sp_unlink(sp)
> {
>
> /* remove from SP index */
> - if (__LIST_CHAINED(sp))
> + if (__LIST_CHAINED(sp)) {
> LIST_REMOVE(sp, chain);
> - key_freesp(sp);
+ /* you should only see this once */
+ printf("%s:%d key_sp_unlink(%p) called\n",
+ __func__, __LINE__, sp);
+ printf("+ id=%u, refcnt=%d\n", sp->id, sp->refcnt);
+ printf("+ le_next=%p, le_prev=%p\n",
+ sp->chain.le_next, sp->chain.le_prev);
> + key_freesp(sp);
> + }
+ else {
+ /* you should never see this */
+ printf("%s:%d key_sp_unlink(%p) called another time\n",
+ __func__, __LINE__, sp);
+ printf("+ id=%u, refcnt=%d\n", sp->id, sp->refcnt);
+ printf("+ le_next=%p, le_prev=%p\n",
+ sp->chain.le_next, sp->chain.le_prev);
+ }
> }
>
> /* XXX too much? */
PS: do you have other patches in your local tree that are not yet
in HEAD ? I got another patch from you once in Nov or Dec but I think
it was renaming functions and passing pcb instead of so only (removing
some unnecessary function calls) [more like FATS_IPSEC]. I have a more
improved version of this in my private patchset but currently
suspended while debugging.
--
Greetings
Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT
56 69 73 69 74 http://www.zabbadoz.net/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.53.0401160829530.66397>