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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).