Re: [PATCH net-next] net: sched: flower: insert filter to ht before offloading it to hw

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Wed, 10 Apr 2019 16:02:17 +0000, Vlad Buslov wrote:
> On Wed 10 Apr 2019 at 18:48, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> > On Wed, 10 Apr 2019 14:53:53 +0000, Vlad Buslov wrote:  
> >> >> For my next patch set that unlocks the offloads API I implemented the
> >> >> algorithm to track reoffload count for each tp that works like this:
> >> >>
> >> >> 1. struct tcf_proto is extended with reoffload_count counter that
> >> >>    incremented each time reoffload is called on particular tp instance.
> >> >>    Counter is protected by tp->lock.
> >> >>
> >> >> 2. struct cls_fl_filter is also extended with reoffload_count counter.
> >> >>    Its value is set to current tp->reoffload_count when offloading the
> >> >>    filter.
> >> >>
> >> >> 3. After offloading the filter, but before inserting it to idr,
> >> >>    f->reoffload_count is compared with tp->reoffload_count. If values
> >> >>    don't match, filter is deleted and -EAGAIN is returned. Cls API
> >> >>    retries filter insertion on -EAGAIN.  
> >> >
> >> > Sounds good for add.  Does this solve delete case as well?
> >> >
> >> >    CPU 0                       CPU 1
> >> >
> >> > __fl_delete
> >> >   IDR remove
> >> >                            cb unregister
> >> >                              hw delete all flows  <- doesn't see the
> >> >                                                      remove in progress
> >> >
> >> >   hw delete  <- doesn't see
> >> >                 the removed cb  
> >>
> >> Thanks for pointing that out! Looks like I need to move call to hw
> >> delete in __fl_delete() function to be executed before idr removal.  
> >
> > Ack, plus you need to do the same retry mechanism.  Save CB "count"/seq,
> > hw delete, remove from IDR, if CB "count"/seq changed hw delete again.
> > Right?  
> 
> Actually, I intended to modify fl_reoffload() to ignore filters with
> 'deleted' flag set when adding, but I guess reusing 'reoffload_count' to
> retry fl_hw_destroy_filter() would also work.

Yeah, I don't see how you can ignore deleted safely.  Perhaps lack of
coffee :)