Re: [PATCH net 2/2] net/sched: add delete_empty() to filters and use it in cls_flower

[Vlad Buslov <vladbu@mellanox.com>]

On Tue 24 Dec 2019 at 16:53, Davide Caratti <dcaratti@redhat.com> wrote:
> hello Jamal and Vlad,
>
> thanks for looking at this.
>
> On Tue, 2019-12-24 at 11:48 +0000, Vlad Buslov wrote:
>> I guess we can reduce this code to just:
>>
>> spin_lock(&tp->lock);
>> tp->deleting = idr_is_empty(&head->handle_idr);
>> spin_unlock(&tp->lock);
>
> on the current version of delete_empty() for cls_flower, we are assuming
> an empty filter also when the IDR is allocated, but its refcount is zero:
>
> 1931         idr_for_each_entry_continue_ul(&head->handle_idr, f, tmp, id) {
> 1932                 /* don't return filters that are being deleted */
> 1933                 if (!refcount_inc_not_zero(&f->refcnt))
> 1934                         continue;
> 1935                 if (arg->fn(tp, f, arg) < 0) {
> 1936                         __fl_put(f);
> 1937                         arg->stop = 1;
> 1938                         break;
> 1939                 }
> 1940                 __fl_put(f);
> 1941                 arg->count++;
> 1942         }
>
> but probably this is relevant to dump(), not delete(). Correct?

I don't think that it is possible to get filter with refcnt==0 from idr
when holding the tp lock (filter is inserted with refcnt==1 and removed
from idr before releasing the last reference in fl_delete()). fl_walk()
doesn't take the lock by itself so it needs to deal with concurrent
removals.

>
>  # ./tdc.py -c flower -d enp2s0
>
> ^^ I'm running several loops of this, just to make sure. If I don't find
> anything relevant in few hours, I will send a v2.
> thanks!