mbox series

[net-next,00/12] Refactor flower classifier to remove dependency on rtnl lock

Message ID 20190214074712.17846-1-vladbu@mellanox.com
Headers show
Series Refactor flower classifier to remove dependency on rtnl lock | expand

Message

Vlad Buslov Feb. 14, 2019, 7:47 a.m. UTC
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.
TC rule update handlers (RTM_NEWTFILTER, RTM_DELTFILTER, etc.) are
already registered with this flag and only take rtnl lock when qdisc or
classifier requires it. Classifiers can indicate that their ops
callbacks don't require caller to hold rtnl lock by setting the
TCF_PROTO_OPS_DOIT_UNLOCKED flag. The goal of this change is to refactor
flower classifier to support unlocked execution and register it with
unlocked flag.

This patch set implements following changes to make flower classifier
concurrency-safe:

- Implement reference counting for individual filters. Change fl_get to
  take reference to filter. Implement tp->ops->put callback that was
  introduced in cls API patch set to release reference to flower filter.

- Use tp->lock spinlock to protect internal classifier data structures
  from concurrent modification.

- Handle concurrent tcf proto deletion by returning EAGAIN, which will
  cause cls API to retry and create new proto instance or return error
  to the user (depending on message type).

- Handle concurrent insertion of filter with same priority and handle by
  returning EAGAIN, which will cause cls API to lookup filter again and
  process it accordingly to netlink message flags.

- Extend flower mask with reference counting and protect masks list with
  masks_lock spinlock.

- Prevent concurrent mask insertion by inserting temporary value to
  masks hash table. This is necessary because mask initialization is a
  sleeping operation and cannot be done while holding tp->lock.

Tcf hw offloads API is not changed by this patch set and still requires
caller to hold rtnl lock. Refactored flower classifier tracks rtnl lock
state by means of 'rtnl_held' flag provided by cls API and obtains the
lock before calling hw offloads.

With these changes flower classifier is safely registered with
TCF_PROTO_OPS_DOIT_UNLOCKED flag in last patch.

Github: [https://github.com/vbuslov/linux/tree/unlocked_flower_cong_1]

Vlad Buslov (12):
  net: sched: flower: don't check for rtnl on head dereference
  net: sched: flower: refactor fl_change
  net: sched: flower: introduce reference counting for filters
  net: sched: flower: track filter deletion with flag
  net: sched: flower: add reference counter to flower mask
  net: sched: flower: handle concurrent mask insertion
  net: sched: flower: protect masks list with spinlock
  net: sched: flower: handle concurrent filter insertion in fl_change
  net: sched: flower: handle concurrent tcf proto deletion
  net: sched: flower: protect flower classifier state with spinlock
  net: sched: flower: track rtnl lock state
  net: sched: flower: set unlocked flag for flower proto ops

 net/sched/cls_flower.c | 424 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 321 insertions(+), 103 deletions(-)

Comments

Cong Wang Feb. 18, 2019, 7:15 p.m. UTC | #1
Hi,

>  net/sched/cls_flower.c | 424 +++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 321 insertions(+), 103 deletions(-)
>

Given you change cls_flower so much, please also add a test case for
verifying your changes, especially focusing on the atomicity of concurrent
modifications.

Thanks.
Vlad Buslov Feb. 19, 2019, 10 a.m. UTC | #2
On Mon 18 Feb 2019 at 19:15, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Hi,
>
>>  net/sched/cls_flower.c | 424 +++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 321 insertions(+), 103 deletions(-)
>>
>
> Given you change cls_flower so much, please also add a test case for
> verifying your changes, especially focusing on the atomicity of concurrent
> modifications.
>
> Thanks.

Will do.