From: Vlad Buslov <vladbu@mellanox.com>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
jiri@resnulli.us, pablo@netfilter.org, kadlec@blackhole.kfki.hu,
fw@strlen.de, ast@kernel.org, daniel@iogearbox.net,
edumazet@google.com, vladbu@mellanox.com, keescook@chromium.org,
marcelo.leitner@gmail.com, kliteyn@mellanox.com
Subject: [PATCH net-next v5 00/11] Modify action API for implementing lockless actions
Date: Fri, 1 Jun 2018 18:15:07 +0300 [thread overview]
Message-ID: <1527866118-21312-1-git-send-email-vladbu@mellanox.com> (raw)
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 first 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 action API that
prevents it from being executed concurrently. This patch set does not
completely unlock rules or actions update path. Additional patch sets
are required to refactor individual actions and filters update for
parallel execution.
As a preparation for executing TC rules update handlers without rtnl
lock, action API code was audited to determine areas that assume
external synchronization with rtnl lock and must be changed to allow
safe concurrent access with following results:
1. Action idr is already protected with spinlock. However, some code
paths assume that idr state is not changes between several
consecutive tcf_idr_* function calls.
2. tc_action reference and bind counters are implemented as plain
integers. They purpose was to allow single actions to be shared
between multiple filters, not to provide means for concurrent
modification.
3. tc_action 'cookie' pointer field is not protected against
modification.
4. Action API functions, that work with set of actions, use intrusive
linked list, which cannot be used concurrently without additional
synchronization.
5. Action API functions don't take reference to actions while using
them, assuming external synchronization with rtnl lock.
Following solutions to these problems are implemented:
1. To remove assumption that idr state doesn't change between tcf_idr_*
calls, implement new functions that atomically perform several
operations on idr without releasing idr spinlock. (function to
atomically lookup and delete action by index, function to atomically
check if action exists and allocate new one if necessary, etc.)
2. Use atomic operations on counters to make them suitable for
concurrent get/put operations.
3. Data that 'cookie' points to is never modified, so it enough to
refactor it to rcu pointer to prevent concurrent de-allocation.
4. Action API doesn't actually use any linked list specific operations
on actions intrusive linked list, so it can be refactored to array in
straightforward manner.
5. Always take reference to action while accessing it in action API.
tcf_idr_search function modified to take reference to action before
returning it, so there is no way to lookup an action without
incrementing its reference counter. All users of this function are
modified to release the reference, after they done using action. With
all users using reference counting, it is now safe to concurrently
delete actions.
Additionally, actions init function signature was expanded with
'rtnl_held' argument, that allows actions that have internal dependency
on rtnl lock to take/release it when necessary.
Since only shared state in action API module are actions themselves and
action idr, these changes are sufficient to not to rely on global rtnl
lock for protection of internal action API data structures.
Changes from V4 to V5:
- Change action delete API to track actions that were deleted, to
prevent releasing them on error.
Changes from V3 to V4:
- Expand cover letter.
- Reduce actions array size in tcf_action_init_1.
- Rebase on latest net-next.
Changes from V2 to V3:
- Re-send with changelog copied to individual patches.
Changes from V1 to V2:
- Removed redundant actions ops lookup during delete.
- Merge action ops delete definition and implementation.
- Assume all actions have delete implemented and don't check for it
explicitly.
- Resplit action lookup/release code to prevent memory leaks in
individual patches.
- Make __tcf_idr_check function static
- Remove unique idr insertion function. Change original idr insert to do
the same thing.
- Merge changes that take reference to action when performing lookup and
changes that account for this additional reference when dumping action
to user space into single patch.
- Change convoluted commit message.
- Rename "unlocked" to "rtnl_held" for clarity.
- Remove estimator lock add patch.
- Refactor action check-alloc code into standalone function.
- Rename tcf_idr_find_delete to tcf_idr_delete_index.
- Rearrange variable definitions in tc_action_delete.
- Add patch that refactors action API code to use array of pointers to
actions instead of intrusive linked list.
- Expand cover letter.
Vlad Buslov (11):
net: sched: use rcu for action cookie update
net: sched: change type of reference and bind counters
net: sched: implement unlocked action init API
net: sched: always take reference to action
net: sched: implement action API that deletes action by index
net: sched: add 'delete' function to action ops
net: sched: implement reference counted action release
net: sched: don't release reference on action overwrite
net: sched: use reference counting action init
net: sched: atomically check-allocate action
net: sched: change action API to use array of pointers to actions
include/net/act_api.h | 25 ++-
include/net/pkt_cls.h | 1 +
net/sched/act_api.c | 414 +++++++++++++++++++++++++++++++--------------
net/sched/act_bpf.c | 34 ++--
net/sched/act_connmark.c | 29 +++-
net/sched/act_csum.c | 34 ++--
net/sched/act_gact.c | 31 +++-
net/sched/act_ife.c | 33 ++--
net/sched/act_ipt.c | 44 ++++-
net/sched/act_mirred.c | 38 +++--
net/sched/act_nat.c | 30 +++-
net/sched/act_pedit.c | 35 +++-
net/sched/act_police.c | 31 ++--
net/sched/act_sample.c | 34 ++--
net/sched/act_simple.c | 31 +++-
net/sched/act_skbedit.c | 31 +++-
net/sched/act_skbmod.c | 34 ++--
net/sched/act_tunnel_key.c | 32 ++--
net/sched/act_vlan.c | 40 +++--
net/sched/cls_api.c | 25 +--
20 files changed, 709 insertions(+), 297 deletions(-)
--
2.7.5
next reply other threads:[~2018-06-01 15:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-01 15:15 Vlad Buslov [this message]
2018-06-01 15:15 ` [PATCH net-next v5 01/11] net: sched: use rcu for action cookie update Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 02/11] net: sched: change type of reference and bind counters Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 03/11] net: sched: implement unlocked action init API Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 04/11] net: sched: always take reference to action Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 05/11] net: sched: implement action API that deletes action by index Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 06/11] net: sched: add 'delete' function to action ops Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 07/11] net: sched: implement reference counted action release Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 08/11] net: sched: don't release reference on action overwrite Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 09/11] net: sched: use reference counting action init Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 10/11] net: sched: atomically check-allocate action Vlad Buslov
2018-06-01 15:15 ` [PATCH net-next v5 11/11] net: sched: change action API to use array of pointers to actions Vlad Buslov
2018-06-04 15:28 ` [PATCH net-next v5 00/11] Modify action API for implementing lockless actions David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1527866118-21312-1-git-send-email-vladbu@mellanox.com \
--to=vladbu@mellanox.com \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kadlec@blackhole.kfki.hu \
--cc=keescook@chromium.org \
--cc=kliteyn@mellanox.com \
--cc=marcelo.leitner@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).