Re: [PATCH net-next v6 5/6] flow_offload: support get multi-subsystem block

Pablo Neira Ayuso
2019-08-06T16:10:14Z

On Sun, Aug 04, 2019 at 09:24:00PM +0800, wenxu@ucloud.cn wrote:
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index 8f1a7b8..6022dd0 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
[...]
> @@ -282,6 +282,8 @@ int flow_block_cb_setup_simple(struct flow_block_offload *f,
> }
> EXPORT_SYMBOL(flow_block_cb_setup_simple);
>
> +static LIST_HEAD(block_ing_cb_list);
> +
> static struct rhashtable indr_setup_block_ht;
>
> struct flow_indr_block_cb {
> @@ -295,7 +297,6 @@ struct flow_indr_block_dev {
> struct rhash_head ht_node;
> struct net_device *dev;
> unsigned int refcnt;
> - flow_indr_block_ing_cmd_t *block_ing_cmd_cb;
> struct list_head cb_list;
> };
>
> @@ -389,6 +390,22 @@ static void flow_indr_block_cb_del(struct flow_indr_block_cb *indr_block_cb)
> kfree(indr_block_cb);
> }
>
> +static void flow_block_ing_cmd(struct net_device *dev,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv,
> + enum flow_block_command command)
> +{
> + struct flow_indr_block_ing_entry *entry;
> +
> + rcu_read_lock();
> +

unnecessary empty line.

> + list_for_each_entry_rcu(entry, &block_ing_cb_list, list) {
> + entry->cb(dev, cb, cb_priv, command);
> + }
> +
> + rcu_read_unlock();

OK, there's rcu_read_lock here...

> +}
> +
> int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
> flow_indr_block_bind_cb_t *cb,
> void *cb_ident)
> @@ -406,10 +423,8 @@ int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
> if (err)
> goto err_dev_put;
>
> - if (indr_dev->block_ing_cmd_cb)
> - indr_dev->block_ing_cmd_cb(dev, indr_block_cb->cb,
> - indr_block_cb->cb_priv,
> - FLOW_BLOCK_BIND);
> + flow_block_ing_cmd(dev, indr_block_cb->cb, indr_block_cb->cb_priv,
> + FLOW_BLOCK_BIND);
>
> return 0;
>
> @@ -448,10 +463,8 @@ void __flow_indr_block_cb_unregister(struct net_device *dev,
> if (!indr_block_cb)
> return;
>
> - if (indr_dev->block_ing_cmd_cb)
> - indr_dev->block_ing_cmd_cb(dev, indr_block_cb->cb,
> - indr_block_cb->cb_priv,
> - FLOW_BLOCK_UNBIND);
> + flow_block_ing_cmd(dev, indr_block_cb->cb, indr_block_cb->cb_priv,
> + FLOW_BLOCK_UNBIND);
>
> flow_indr_block_cb_del(indr_block_cb);
> flow_indr_block_dev_put(indr_dev);
> @@ -469,7 +482,6 @@ void flow_indr_block_cb_unregister(struct net_device *dev,
> EXPORT_SYMBOL_GPL(flow_indr_block_cb_unregister);
>
> void flow_indr_block_call(struct net_device *dev,
> - flow_indr_block_ing_cmd_t cb,
> struct flow_block_offload *bo,
> enum flow_block_command command)
> {
> @@ -480,15 +492,24 @@ void flow_indr_block_call(struct net_device *dev,
> if (!indr_dev)
> return;
>
> - indr_dev->block_ing_cmd_cb = command == FLOW_BLOCK_BIND
> - ? cb : NULL;
> -
> list_for_each_entry(indr_block_cb, &indr_dev->cb_list, list)
> indr_block_cb->cb(dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
> bo);
> }
> EXPORT_SYMBOL_GPL(flow_indr_block_call);
>
> +void flow_indr_add_block_ing_cb(struct flow_indr_block_ing_entry *entry)
> +{

... but registration does not protect the list with a mutex.

> + list_add_tail_rcu(&entry->list, &block_ing_cb_list);
> +}
> +EXPORT_SYMBOL_GPL(flow_indr_add_block_ing_cb);