On Wed, Nov 24, 2021 at 05:12:36PM +0300, Vitaliy Makkoveev wrote:
> Understood. But his means we encoded double unref when we calling
> tdb_unref() just after tdb_delete(tdb). To me it looks better to avoid
> this and rework handlers like below:
I have tried this before. It creates a tdb leak. The double unref
is correct. tdb_delete() must only be called once.
> tdb_timeout(void *v)
> {
> struct tdb *tdb = v;
>
> NET_LOCK();
> if (tdb->tdb_flags & TDBF_TIMER) {
> /* If it's an "invalid" TDB do a silent expiration. */
> if (!(tdb->tdb_flags & TDBF_INVALID)) {
> ipsecstat_inc(ipsec_exctdb);
> pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
> }
> /* tdb_delete() calls tdb_unref() */
> tdb_delete(tdb);
> } else {
> /* decrement refcount of the timeout argument */
> tdb_unref(tdb);
> }
> NET_UNLOCK();
> }
>
> Also we could rework tdb_delete() to set the 'TDBF_DELETED' flag and do
> not tdb_unref() passed `tdb'. It's assumed that caller should unref this
> `tdb'. I like this because we will not kill `tdb' passed to (*xf_input)()
> and (*xf_output)().
If the caller needs a tdb ref, he has to refcount it. The tdb_delete()
decrements the refcount that was added during allocation.
My next diff will add a refcount in ip_output_ipsec_lookup() to
address this. The current diff is complicated enough. I want to
commit something that does neither leak nor crash. After that I
can add more refcounts. Finally I will run in parallel.
I cannot put everything in one gigantic diff.
bluhm
