On Wed 10 Apr 2019 at 18:48, Jakub Kicinski <jakub.kicin...@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.
