On Mon, Sep 24, 2018 at 9:23 AM Vlad Buslov <vla...@mellanox.com> wrote: > > Currently, all netlink protocol handlers for updating rules, actions and > qdiscs are protected with single global rtnl lock which removes any > possibility for parallelism. This patch set is a third step to remove > rtnl lock dependency from TC rules update path. > > Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added. > Handlers registered with this flag are called without RTNL taken. End > goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER, > etc.) to be registered with UNLOCKED flag to allow parallel execution. > However, there is no intention to completely remove or split rtnl lock > itself. This patch set addresses specific problems in implementation of > classifiers API that prevent its control path from being executed > concurrently. Additional changes are required to refactor classifiers > API and individual classifiers for parallel execution. This patch set > lays groundwork to eventually register rule update handlers as > rtnl-unlocked by modifying code in cls API that works with Qdiscs and > blocks. Following patch set does the same for chains and classifiers. > > The goal of this change is to refactor tcf_block_find() and its > dependencies to allow concurrent execution: > - Extend Qdisc API with rcu to lookup and take reference to Qdisc > without relying on rtnl lock. > - Extend tcf_block with atomic reference counting and rcu. > - Always take reference to tcf_block while working with it. > - Implement tcf_block_release() to release resources obtained by > tcf_block_find() > - Create infrastructure to allow registering Qdiscs with class ops that > do not require the caller to hold rtnl lock. > > All three netlink rule update handlers use tcf_block_find() to lookup > Qdisc and block, and this patch set introduces additional means of > synchronization to substitute rtnl lock in cls API. > > Some functions in cls and sch APIs have historic names that no longer > clearly describe their intent. In order not make this code even more > confusing when introducing their concurrency-friendly versions, rename > these functions to describe actual implementation. > > Changes from V2 to V3: > - Patch 1: > - Explicitly include refcount.h in rtnetlink.h. > - Patch 3: > - Move rcu_head field to the end of struct Qdisc. > - Rearrange local variable declarations in qdisc_lookup_rcu(). > - Patch 5: > - Remove tcf_qdisc_put() and inline its content to callers.
Overall looks good to me, Acked-by: Cong Wang <xiyou.wangc...@gmail.com> There are some minor issues like qdisc_free_cb() can be static, please send follow-up patch to address it. Thanks.
