Re: [PATCH] IPSec fixes

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
> _at__at_ -1958,7 +1958,6 _at__at_ 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);
>
> _at__at_ -7591,9 +7590,10 _at__at_ 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/