Commit 4f8116c8 authored by Vlad Buslov's avatar Vlad Buslov Committed by David S. Miller

net: sched: protect block offload-related fields with rw_semaphore

In order to remove dependency on rtnl lock, extend tcf_block with 'cb_lock'
rwsem and use it to protect flow_block->cb_list and related counters from
concurrent modification. The lock is taken in read mode for read-only
traversal of cb_list in tc_setup_cb_call() and write mode in all other
cases. This approach ensures that:

- cb_list is not changed concurrently while filters is being offloaded on
  block.

- block->nooffloaddevcnt is checked while holding the lock in read mode,
  but is only changed by bind/unbind code when holding the cb_lock in write
  mode to prevent concurrent modification.
Signed-off-by: default avatarVlad Buslov <vladbu@mellanox.com>
Acked-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 0846e161
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <linux/refcount.h> #include <linux/refcount.h>
#include <linux/workqueue.h> #include <linux/workqueue.h>
#include <linux/mutex.h> #include <linux/mutex.h>
#include <linux/rwsem.h>
#include <net/gen_stats.h> #include <net/gen_stats.h>
#include <net/rtnetlink.h> #include <net/rtnetlink.h>
#include <net/flow_offload.h> #include <net/flow_offload.h>
...@@ -396,6 +397,7 @@ struct tcf_block { ...@@ -396,6 +397,7 @@ struct tcf_block {
refcount_t refcnt; refcount_t refcnt;
struct net *net; struct net *net;
struct Qdisc *q; struct Qdisc *q;
struct rw_semaphore cb_lock; /* protects cb_list and offload counters */
struct flow_block flow_block; struct flow_block flow_block;
struct list_head owner_list; struct list_head owner_list;
bool keep_dst; bool keep_dst;
......
...@@ -568,9 +568,11 @@ static void tc_indr_block_ing_cmd(struct net_device *dev, ...@@ -568,9 +568,11 @@ static void tc_indr_block_ing_cmd(struct net_device *dev,
bo.block = &block->flow_block; bo.block = &block->flow_block;
down_write(&block->cb_lock);
cb(dev, cb_priv, TC_SETUP_BLOCK, &bo); cb(dev, cb_priv, TC_SETUP_BLOCK, &bo);
tcf_block_setup(block, &bo); tcf_block_setup(block, &bo);
up_write(&block->cb_lock);
} }
static struct tcf_block *tc_dev_ingress_block(struct net_device *dev) static struct tcf_block *tc_dev_ingress_block(struct net_device *dev)
...@@ -661,6 +663,7 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q, ...@@ -661,6 +663,7 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
struct net_device *dev = q->dev_queue->dev; struct net_device *dev = q->dev_queue->dev;
int err; int err;
down_write(&block->cb_lock);
if (!dev->netdev_ops->ndo_setup_tc) if (!dev->netdev_ops->ndo_setup_tc)
goto no_offload_dev_inc; goto no_offload_dev_inc;
...@@ -669,24 +672,31 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q, ...@@ -669,24 +672,31 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
*/ */
if (!tc_can_offload(dev) && tcf_block_offload_in_use(block)) { if (!tc_can_offload(dev) && tcf_block_offload_in_use(block)) {
NL_SET_ERR_MSG(extack, "Bind to offloaded block failed as dev has offload disabled"); NL_SET_ERR_MSG(extack, "Bind to offloaded block failed as dev has offload disabled");
return -EOPNOTSUPP; err = -EOPNOTSUPP;
goto err_unlock;
} }
err = tcf_block_offload_cmd(block, dev, ei, FLOW_BLOCK_BIND, extack); err = tcf_block_offload_cmd(block, dev, ei, FLOW_BLOCK_BIND, extack);
if (err == -EOPNOTSUPP) if (err == -EOPNOTSUPP)
goto no_offload_dev_inc; goto no_offload_dev_inc;
if (err) if (err)
return err; goto err_unlock;
tc_indr_block_call(block, dev, ei, FLOW_BLOCK_BIND, extack); tc_indr_block_call(block, dev, ei, FLOW_BLOCK_BIND, extack);
up_write(&block->cb_lock);
return 0; return 0;
no_offload_dev_inc: no_offload_dev_inc:
if (tcf_block_offload_in_use(block)) if (tcf_block_offload_in_use(block)) {
return -EOPNOTSUPP; err = -EOPNOTSUPP;
goto err_unlock;
}
err = 0;
block->nooffloaddevcnt++; block->nooffloaddevcnt++;
tc_indr_block_call(block, dev, ei, FLOW_BLOCK_BIND, extack); tc_indr_block_call(block, dev, ei, FLOW_BLOCK_BIND, extack);
return 0; err_unlock:
up_write(&block->cb_lock);
return err;
} }
static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q, static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
...@@ -695,6 +705,7 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q, ...@@ -695,6 +705,7 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
struct net_device *dev = q->dev_queue->dev; struct net_device *dev = q->dev_queue->dev;
int err; int err;
down_write(&block->cb_lock);
tc_indr_block_call(block, dev, ei, FLOW_BLOCK_UNBIND, NULL); tc_indr_block_call(block, dev, ei, FLOW_BLOCK_UNBIND, NULL);
if (!dev->netdev_ops->ndo_setup_tc) if (!dev->netdev_ops->ndo_setup_tc)
...@@ -702,10 +713,12 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q, ...@@ -702,10 +713,12 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
err = tcf_block_offload_cmd(block, dev, ei, FLOW_BLOCK_UNBIND, NULL); err = tcf_block_offload_cmd(block, dev, ei, FLOW_BLOCK_UNBIND, NULL);
if (err == -EOPNOTSUPP) if (err == -EOPNOTSUPP)
goto no_offload_dev_dec; goto no_offload_dev_dec;
up_write(&block->cb_lock);
return; return;
no_offload_dev_dec: no_offload_dev_dec:
WARN_ON(block->nooffloaddevcnt-- == 0); WARN_ON(block->nooffloaddevcnt-- == 0);
up_write(&block->cb_lock);
} }
static int static int
...@@ -820,6 +833,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q, ...@@ -820,6 +833,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
} }
mutex_init(&block->lock); mutex_init(&block->lock);
init_rwsem(&block->cb_lock);
flow_block_init(&block->flow_block); flow_block_init(&block->flow_block);
INIT_LIST_HEAD(&block->chain_list); INIT_LIST_HEAD(&block->chain_list);
INIT_LIST_HEAD(&block->owner_list); INIT_LIST_HEAD(&block->owner_list);
...@@ -1355,6 +1369,8 @@ tcf_block_playback_offloads(struct tcf_block *block, flow_setup_cb_t *cb, ...@@ -1355,6 +1369,8 @@ tcf_block_playback_offloads(struct tcf_block *block, flow_setup_cb_t *cb,
struct tcf_proto *tp, *tp_prev; struct tcf_proto *tp, *tp_prev;
int err; int err;
lockdep_assert_held(&block->cb_lock);
for (chain = __tcf_get_next_chain(block, NULL); for (chain = __tcf_get_next_chain(block, NULL);
chain; chain;
chain_prev = chain, chain_prev = chain,
...@@ -1393,6 +1409,8 @@ static int tcf_block_bind(struct tcf_block *block, ...@@ -1393,6 +1409,8 @@ static int tcf_block_bind(struct tcf_block *block,
struct flow_block_cb *block_cb, *next; struct flow_block_cb *block_cb, *next;
int err, i = 0; int err, i = 0;
lockdep_assert_held(&block->cb_lock);
list_for_each_entry(block_cb, &bo->cb_list, list) { list_for_each_entry(block_cb, &bo->cb_list, list) {
err = tcf_block_playback_offloads(block, block_cb->cb, err = tcf_block_playback_offloads(block, block_cb->cb,
block_cb->cb_priv, true, block_cb->cb_priv, true,
...@@ -1427,6 +1445,8 @@ static void tcf_block_unbind(struct tcf_block *block, ...@@ -1427,6 +1445,8 @@ static void tcf_block_unbind(struct tcf_block *block,
{ {
struct flow_block_cb *block_cb, *next; struct flow_block_cb *block_cb, *next;
lockdep_assert_held(&block->cb_lock);
list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) { list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) {
tcf_block_playback_offloads(block, block_cb->cb, tcf_block_playback_offloads(block, block_cb->cb,
block_cb->cb_priv, false, block_cb->cb_priv, false,
...@@ -2987,19 +3007,26 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type, ...@@ -2987,19 +3007,26 @@ int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type,
int ok_count = 0; int ok_count = 0;
int err; int err;
down_read(&block->cb_lock);
/* Make sure all netdevs sharing this block are offload-capable. */ /* Make sure all netdevs sharing this block are offload-capable. */
if (block->nooffloaddevcnt && err_stop) if (block->nooffloaddevcnt && err_stop) {
return -EOPNOTSUPP; ok_count = -EOPNOTSUPP;
goto err_unlock;
}
list_for_each_entry(block_cb, &block->flow_block.cb_list, list) { list_for_each_entry(block_cb, &block->flow_block.cb_list, list) {
err = block_cb->cb(type, type_data, block_cb->cb_priv); err = block_cb->cb(type, type_data, block_cb->cb_priv);
if (err) { if (err) {
if (err_stop) if (err_stop) {
return err; ok_count = err;
goto err_unlock;
}
} else { } else {
ok_count++; ok_count++;
} }
} }
err_unlock:
up_read(&block->cb_lock);
return ok_count; return ok_count;
} }
EXPORT_SYMBOL(tc_setup_cb_call); EXPORT_SYMBOL(tc_setup_cb_call);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment