From: Jiri Pirko <jiri@resnulli.us>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org, jakub.kicinski@netronome.com
Subject: Re: [PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it
Date: Thu, 11 Jul 2019 15:18:40 +0200 [thread overview]
Message-ID: <20190711131840.GL2291@nanopsycho> (raw)
In-Reply-To: <20190711131300.ojbo5hxzvv6wi44t@salvia>
Thu, Jul 11, 2019 at 03:13:00PM CEST, pablo@netfilter.org wrote:
>On Thu, Jul 11, 2019 at 10:06:09AM +0200, Jiri Pirko wrote:
>> Thu, Jul 11, 2019 at 02:12:35AM CEST, pablo@netfilter.org wrote:
>> >This object stores the flow block callbacks that are attached to this
>> >block. This patch restores block sharing.
>> >
>> >Fixes: da3eeb904ff4 ("net: flow_offload: add list handling functions")
>> >Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>> >---
>> > include/net/flow_offload.h | 5 +++++
>> > include/net/netfilter/nf_tables.h | 5 +++--
>> > include/net/sch_generic.h | 2 +-
>> > net/core/flow_offload.c | 2 +-
>> > net/netfilter/nf_tables_api.c | 2 +-
>> > net/netfilter/nf_tables_offload.c | 5 +++--
>> > net/sched/cls_api.c | 10 +++++++---
>> > 7 files changed, 21 insertions(+), 10 deletions(-)
>> >
>> >diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>> >index 98bf3af5c84d..e50d94736829 100644
>> >--- a/include/net/flow_offload.h
>> >+++ b/include/net/flow_offload.h
>> >@@ -248,6 +248,10 @@ enum flow_block_binder_type {
>> > FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS,
>> > };
>> >
>> >+struct flow_block {
>> >+ struct list_head cb_list;
>> >+};
>> >+
>> > struct netlink_ext_ack;
>> >
>> > struct flow_block_offload {
>> >@@ -255,6 +259,7 @@ struct flow_block_offload {
>> > enum flow_block_binder_type binder_type;
>> > bool block_shared;
>> > struct net *net;
>> >+ struct flow_block *block;
>> > struct list_head cb_list;
>> > struct list_head *driver_block_list;
>> > struct netlink_ext_ack *extack;
>> >diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
>> >index 35dfdd9f69b3..00658462f89b 100644
>> >--- a/include/net/netfilter/nf_tables.h
>> >+++ b/include/net/netfilter/nf_tables.h
>> >@@ -11,6 +11,7 @@
>> > #include <linux/rhashtable.h>
>> > #include <net/netfilter/nf_flow_table.h>
>> > #include <net/netlink.h>
>> >+#include <net/flow_offload.h>
>> >
>> > struct module;
>> >
>> >@@ -951,7 +952,7 @@ struct nft_stats {
>> > * @stats: per-cpu chain stats
>> > * @chain: the chain
>> > * @dev_name: device name that this base chain is attached to (if any)
>> >- * @cb_list: list of flow block callbacks (for hardware offload)
>> >+ * @block: flow block (for hardware offload)
>> > */
>> > struct nft_base_chain {
>> > struct nf_hook_ops ops;
>> >@@ -961,7 +962,7 @@ struct nft_base_chain {
>> > struct nft_stats __percpu *stats;
>> > struct nft_chain chain;
>> > char dev_name[IFNAMSIZ];
>> >- struct list_head cb_list;
>> >+ struct flow_block block;
>> > };
>> >
>> > static inline struct nft_base_chain *nft_base_chain(const struct nft_chain *chain)
>> >diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>> >index 9482e060483b..58041cb0ce15 100644
>> >--- a/include/net/sch_generic.h
>> >+++ b/include/net/sch_generic.h
>> >@@ -399,7 +399,7 @@ struct tcf_block {
>> > refcount_t refcnt;
>> > struct net *net;
>> > struct Qdisc *q;
>> >- struct list_head cb_list;
>> >+ struct flow_block flow;
>>
>> It is not a "flow", that is confusing. It should be named "flow_block".
>
>Done.
>
>> > struct list_head owner_list;
>> > bool keep_dst;
>> > unsigned int offloadcnt; /* Number of oddloaded filters */
>> >diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>> >index a800fa78d96c..935c7f81a9ef 100644
>> >--- a/net/core/flow_offload.c
>> >+++ b/net/core/flow_offload.c
>> >@@ -198,7 +198,7 @@ struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
>> > {
>> > struct flow_block_cb *block_cb;
>> >
>> >- list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
>> >+ list_for_each_entry(block_cb, &f->block->cb_list, list) {
>>
>> Please made struct flow_block *block and argument of cb_lookup instead
>> of struct flow_block_offload *f (as it was previously).
>
>I can do so if you insist, this will make this fix larger.
Yes please. Thanks!
>
>> > if (block_cb->cb == cb &&
>> > block_cb->cb_ident == cb_ident)
>> > return block_cb;
>> >diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> >index ed17a7c29b86..c565f146435b 100644
>> >--- a/net/netfilter/nf_tables_api.c
>> >+++ b/net/netfilter/nf_tables_api.c
>> >@@ -1662,7 +1662,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
>> >
>> > chain->flags |= NFT_BASE_CHAIN | flags;
>> > basechain->policy = NF_ACCEPT;
>> >- INIT_LIST_HEAD(&basechain->cb_list);
>> >+ INIT_LIST_HEAD(&basechain->block.cb_list);
>> > } else {
>> > chain = kzalloc(sizeof(*chain), GFP_KERNEL);
>> > if (chain == NULL)
>> >diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
>> >index 2c3302845f67..2a184277ee58 100644
>> >--- a/net/netfilter/nf_tables_offload.c
>> >+++ b/net/netfilter/nf_tables_offload.c
>> >@@ -116,7 +116,7 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
>> > struct flow_block_cb *block_cb;
>> > int err;
>> >
>> >- list_for_each_entry(block_cb, &basechain->cb_list, list) {
>> >+ list_for_each_entry(block_cb, &basechain->block.cb_list, list) {
>> > err = block_cb->cb(type, type_data, block_cb->cb_priv);
>> > if (err < 0)
>> > return err;
>> >@@ -154,7 +154,7 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
>> > static int nft_flow_offload_bind(struct flow_block_offload *bo,
>> > struct nft_base_chain *basechain)
>> > {
>> >- list_splice(&bo->cb_list, &basechain->cb_list);
>> >+ list_splice(&bo->cb_list, &basechain->block.cb_list);
>> > return 0;
>> > }
>> >
>> >@@ -198,6 +198,7 @@ static int nft_flow_offload_chain(struct nft_trans *trans,
>> > return -EOPNOTSUPP;
>> >
>> > bo.command = cmd;
>> >+ bo.block = &basechain->block;
>> > bo.binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
>> > bo.extack = &extack;
>> > INIT_LIST_HEAD(&bo.cb_list);
>> >diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> >index 51fbe6e95a92..66181961ad6f 100644
>> >--- a/net/sched/cls_api.c
>> >+++ b/net/sched/cls_api.c
>> >@@ -691,6 +691,8 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
>> > if (!indr_dev->block)
>> > return;
>> >
>> >+ bo.block = &indr_dev->block->flow;
>> >+
>> > indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
>> > &bo);
>> > tcf_block_setup(indr_dev->block, &bo);
>> >@@ -775,6 +777,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>> > .command = command,
>> > .binder_type = ei->binder_type,
>> > .net = dev_net(dev),
>> >+ .block = &block->flow,
>> > .block_shared = tcf_block_shared(block),
>> > .extack = extack,
>> > };
>> >@@ -810,6 +813,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
>> > bo.net = dev_net(dev);
>> > bo.command = command;
>> > bo.binder_type = ei->binder_type;
>> >+ bo.block = &block->flow;
>> > bo.block_shared = tcf_block_shared(block);
>> > bo.extack = extack;
>> > INIT_LIST_HEAD(&bo.cb_list);
>> >@@ -988,7 +992,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
>> > }
>> > mutex_init(&block->lock);
>> > INIT_LIST_HEAD(&block->chain_list);
>> >- INIT_LIST_HEAD(&block->cb_list);
>> >+ INIT_LIST_HEAD(&block->flow.cb_list);
>>
>> With introduction of struct flow_block, please introduce also a helper
>> to init this struct. Does not look right to init it from user codes
>> (tc/nft).
>
>Done.
next prev parent reply other threads:[~2019-07-11 13:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-11 0:12 [PATCH net-next 1/3] net: flow_offload: remove netns parameter from flow_block_cb_alloc() Pablo Neira Ayuso
2019-07-11 0:12 ` [PATCH net-next 2/3] net: flow_offload: rename tc_setup_cb_t to flow_setup_cb_t Pablo Neira Ayuso
2019-07-11 8:07 ` Jiri Pirko
2019-07-11 0:12 ` [PATCH net-next 3/3] net: flow_offload: add flow_block structure and use it Pablo Neira Ayuso
2019-07-11 8:06 ` Jiri Pirko
2019-07-11 13:13 ` Pablo Neira Ayuso
2019-07-11 13:18 ` Jiri Pirko [this message]
2019-07-11 8:08 ` [PATCH net-next 1/3] net: flow_offload: remove netns parameter from flow_block_cb_alloc() Jiri Pirko
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=20190711131840.GL2291@nanopsycho \
--to=jiri@resnulli.us \
--cc=davem@davemloft.net \
--cc=jakub.kicinski@netronome.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.