Mon, Jul 08, 2019 at 06:06:07PM CEST, pablo@xxxxxxxxxxxxx wrote:
>This patch adds the list handling functions for the flow block API:
>
>* flow_block_cb_lookup() allows drivers to look up for existing flow blocks.
>* flow_block_cb_add() adds a flow block to the list to be registered by the
> core.
Per driver? You say "per driver" in the "remove" part.
>* flow_block_cb_remove() to remove a flow block from the list of existing
> flow blocks per driver and to request the core to unregister this.
>
>The flow block API also annotates the netns this flow block belongs to.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
>---
>v3: extracted from former patch "net: flow_offload: add flow_block_cb API".
>
> include/net/flow_offload.h | 20 ++++++++++++++++++++
> net/core/flow_offload.c | 18 ++++++++++++++++++
> net/sched/cls_api.c | 3 +++
> 3 files changed, 41 insertions(+)
>
>diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>index bcc4e2fef6ba..06acde2960fa 100644
>--- a/include/net/flow_offload.h
>+++ b/include/net/flow_offload.h
>@@ -251,12 +251,16 @@ struct flow_block_offload {
> enum flow_block_command command;
> enum flow_block_binder_type binder_type;
> struct tcf_block *block;
>+ struct net *net;
>+ struct list_head cb_list;
> struct list_head *driver_block_list;
> struct netlink_ext_ack *extack;
> };
>
> struct flow_block_cb {
>+ struct list_head driver_list;
> struct list_head list;
>+ struct net *net;
> tc_setup_cb_t *cb;
> void *cb_ident;
> void *cb_priv;
>@@ -269,6 +273,22 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> void (*release)(void *cb_priv));
> void flow_block_cb_free(struct flow_block_cb *block_cb);
>
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+ struct list_head *driver_flow_block_list,
>+ tc_setup_cb_t *cb, void *cb_ident);
>+
>+static inline void flow_block_cb_add(struct flow_block_cb *block_cb,
>+ struct flow_block_offload *offload)
>+{
>+ list_add_tail(&block_cb->driver_list, &offload->cb_list);
>+}
>+
>+static inline void flow_block_cb_remove(struct flow_block_cb *block_cb,
>+ struct flow_block_offload *offload)
>+{
>+ list_move(&block_cb->driver_list, &offload->cb_list);
>+}
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> struct list_head *driver_list, tc_setup_cb_t *cb,
> void *cb_ident, void *cb_priv, bool ingress_only);
>diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>index d08148cb6953..85fd5f4a1e0f 100644
>--- a/net/core/flow_offload.c
>+++ b/net/core/flow_offload.c
>@@ -176,6 +176,7 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> if (!block_cb)
> return ERR_PTR(-ENOMEM);
>
>+ block_cb->net = net;
> block_cb->cb = cb;
> block_cb->cb_ident = cb_ident;
> block_cb->cb_priv = cb_priv;
>@@ -194,6 +195,23 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
> }
> EXPORT_SYMBOL(flow_block_cb_free);
>
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+ struct list_head *driver_block_list,
In the header, you call this "driver_flow_block_list".
Where is this list coming from? In general, I don't think it is good to
have struct list_head as an arg of exported symbol. Should be contained
in some struct. Looks like this might be the "struct
flow_block_offload"?
Does this have anything to do with "struct list_head
*driver_block_list"? This is very confusing...
>+ tc_setup_cb_t *cb, void *cb_ident)
>+{
>+ struct flow_block_cb *block_cb;
>+
>+ list_for_each_entry(block_cb, driver_block_list, driver_list) {
>+ if (block_cb->net == net &&
>+ block_cb->cb == cb &&
>+ block_cb->cb_ident == cb_ident)
>+ return block_cb;
>+ }
>+
>+ return NULL;
>+}
>+EXPORT_SYMBOL(flow_block_cb_lookup);
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> struct list_head *driver_block_list,
> tc_setup_cb_t *cb, void *cb_ident, void *cb_priv,
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>index fa0c451aca59..72761b43ae41 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -679,6 +679,7 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
> struct tc_block_offload bo = {
> .command = command,
> .binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
>+ .net = dev_net(indr_dev->dev),
> .block = indr_dev->block,
> };
>
>@@ -767,6 +768,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
> struct tc_block_offload bo = {
> .command = command,
> .binder_type = ei->binder_type,
>+ .net = dev_net(dev),
> .block = block,
> .extack = extack,
> };
>@@ -795,6 +797,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> {
> struct tc_block_offload bo = {};
>
>+ bo.net = dev_net(dev);
> bo.command = command;
> bo.binder_type = ei->binder_type;
> bo.block = block;
>--
>2.11.0
>
