Subject: Re: [PATCH][NET_SCHED] cls_u32: refcounting fix for u32_delete()

From: Patrick McHardy <[email-masked]>
Date: Fri, 11 Apr 2008 15:22:51 +0200

Jarek Poplawski wrote:
> On Fri, Apr 11, 2008 at 02:55:27PM +0200, Patrick McHardy wrote:
>> Jarek Poplawski wrote:
>>> [NET_SCHED] cls_u32: refcounting fix for u32_delete()
>>>
>>> @@ -441,8 +443,10 @@ static int u32_delete(struct tcf_proto *tp, unsigned long arg)
>>>  	if (tp->root == ht)
>>>  		return -EINVAL;
>>>  -	if (--ht->refcnt == 0)
>>> +	if (ht->refcnt == 1) {
>>> +		ht->refcnt--;
>>>  		u32_destroy_hnode(tp, ht);
>>> +	}
>>>  
>> Shouldn't the refcount be decremented unconditionally?
>> Otherwise I'd suggest to reject the removal if refcnt > 1,
>> but silently doing nothing doesn't seem right.
> 
> If someone tries deleting and the refcnt is e.g. 3 (2 links)
> unconditional deleting could decrease refcnt without deleting
> and then unlinking would make it below 0. I think it's OK now:
> we can't delete hnode until it's referenced, but I could add
> a warning if you like?

I'd suggest to behave similar to qdiscs when trying to
removing classes that are still referenced, return -EBUSY.