On Thu 09 Aug 2018 at 19:38, Cong Wang [off-list ref] wrote:

On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov [off-list ref] wrote:

quoted

Extend action ops with 'delete' function. Each action type to implements
its own delete function that doesn't depend on rtnl lock.

Implement delete function that is required to delete actions without
holding rtnl lock. Use action API function that atomically deletes action
only if it is still in action idr. This implementation prevents concurrent
threads from deleting same action twice.

I fail to understand why you introduce ops->delete(), it seems all
you want is getting the tn->idrinfo, but you already have tc_action
before calling ops->delete(), and tc_action has ->idrinfo...

Each type of action does the same too, that is, just calling
tcf_idr_delete_index()...

I agree with your assessment. Should have implemented it by just calling
tcf_idr_delete_index() directly.

This changelog sucks again, it claims for skipping rtnl lock,
but you can skip rtnl lock by just calling tcf_idr_delete_index()
directly too, it is not the reason for adding ops->delete().

My intention was to implement some generic and parallel-safe API that
could be used to implement delete for all actions. It turns out that, as
you noted, just calling tcf_idr_delete_index() is enough because any
special per-action delete code is already implemented by ops->cleanup().