From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Edward Cree <ecree@solarflare.com>
Cc: netdev <netdev@vger.kernel.org>, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH net-next,v4 07/12] net: sched: use flow block API
Date: Fri, 16 Aug 2019 03:10:33 +0200 [thread overview]
Message-ID: <20190816011033.sw34b3ckfd636afg@salvia> (raw)
In-Reply-To: <b45709c7-38b5-2dcb-3db1-0c2fca1840be@solarflare.com>
On Wed, Aug 14, 2019 at 05:32:34PM +0100, Edward Cree wrote:
> On 09/07/2019 21:55, Pablo Neira Ayuso wrote:
> > This patch adds tcf_block_setup() which uses the flow block API.
> >
> > This infrastructure takes the flow block callbacks coming from the
> > driver and register/unregister to/from the cls_api core.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > <snip>
> > @@ -796,13 +804,20 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> > struct netlink_ext_ack *extack)
> > {
> > struct tc_block_offload bo = {};
> > + int err;
> >
> > bo.net = dev_net(dev);
> > bo.command = command;
> > bo.binder_type = ei->binder_type;
> > bo.block = block;
> > bo.extack = extack;
> > - return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_BLOCK, &bo);
> > + INIT_LIST_HEAD(&bo.cb_list);
> > +
> > + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_BLOCK, &bo);
> > + if (err < 0)
> > + return err;
> > +
> > + return tcf_block_setup(block, &bo);
> > }
> >
> > static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
> > @@ -1636,6 +1651,77 @@ void tcf_block_cb_unregister(struct tcf_block *block,
> > }
> > EXPORT_SYMBOL(tcf_block_cb_unregister);
> >
> > +static int tcf_block_bind(struct tcf_block *block,
> > + struct flow_block_offload *bo)
> > +{
> > + struct flow_block_cb *block_cb, *next;
> > + int err, i = 0;
> > +
> > + list_for_each_entry(block_cb, &bo->cb_list, list) {
> > + err = tcf_block_playback_offloads(block, block_cb->cb,
> > + block_cb->cb_priv, true,
> > + tcf_block_offload_in_use(block),
> > + bo->extack);
> > + if (err)
> > + goto err_unroll;
> > +
> > + i++;
> > + }
> > + list_splice(&bo->cb_list, &block->cb_list);
> > +
> > + return 0;
> > +
> > +err_unroll:
> > + list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) {
> > + if (i-- > 0) {
> > + list_del(&block_cb->list);
> > + tcf_block_playback_offloads(block, block_cb->cb,
> > + block_cb->cb_priv, false,
> > + tcf_block_offload_in_use(block),
> > + NULL);
> > + }
> > + flow_block_cb_free(block_cb);
> > + }
> > +
> > + return err;
> > +}
> >
> Why has the replay been moved from the function called by the driver
> (__tcf_block_cb_register()) to work done by the driver's caller based on
> what the driver has left on this flow_block_offload.cb_list? This makes
> it impossible for the driver to (say) unregister a block outside of an
> explicit request from ndo_setup_tc().
> In my under-development driver, I have a teardown path called on PCI
> remove, which calls tcf_block_cb_unregister() on all my block bindings
> (of which the driver keeps track), to ensure that no flow rules are still
> in place when unregister_netdev() is called;
It's the subsystem that has to release resources when
unregister_netdev() event happens. At least in netfilter, when the
device is going away, the filtering policy is removed, hence the
FLOW_BLOCK_UNBIND is called to release the blocks and, hence, the
offload resources. I remember tc ingress qdisc works like this too.
> this is needed because some of the driver's state for certain
> rules involves taking a reference on the netdevice (dev_hold()).
> Your structural changes here make that impossible; is there any
> reason why they're necessary?
May I have access to your driver code?
This would make it easier for me to understand your requirements, and
to discuss changes with you.
next prev parent reply other threads:[~2019-08-16 1:10 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-09 20:55 [PATCH net-next,v4 00/11] netfilter: add hardware offload infrastructure Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 01/12] net: flow_offload: add flow_block_cb_setup_simple() Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 02/12] net: flow_offload: rename TC_BLOCK_{UN}BIND to FLOW_BLOCK_{UN}BIND Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 03/12] net: flow_offload: rename TCF_BLOCK_BINDER_TYPE_* to FLOW_BLOCK_BINDER_TYPE_* Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 04/12] net: flow_offload: add flow_block_cb_alloc() and flow_block_cb_free() Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 05/12] net: flow_offload: add list handling functions Pablo Neira Ayuso
2019-07-10 7:36 ` Jiri Pirko
2019-07-10 9:06 ` Jiri Pirko
2019-07-09 20:55 ` [PATCH net-next,v4 06/12] net: flow_offload: add flow_block_cb_{priv,incref,decref}() Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 07/12] net: sched: use flow block API Pablo Neira Ayuso
2019-08-14 16:32 ` Edward Cree
2019-08-16 1:10 ` Pablo Neira Ayuso [this message]
2019-07-09 20:55 ` [PATCH net-next,v4 08/12] drivers: net: " Pablo Neira Ayuso
2019-07-10 8:01 ` Jiri Pirko
2019-08-12 17:50 ` Edward Cree
2019-08-13 19:51 ` Pablo Neira Ayuso
2019-08-14 16:17 ` Edward Cree
2019-08-16 1:04 ` Pablo Neira Ayuso
2019-08-16 17:00 ` Edward Cree
2019-07-09 20:55 ` [PATCH net-next,v4 09/12] net: sched: remove tcf " Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 10/12] net: flow_offload: add flow_block_cb_is_busy() and use it Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 11/12] net: flow_offload: rename tc_cls_flower_offload to flow_cls_offload Pablo Neira Ayuso
2019-07-09 20:55 ` [PATCH net-next,v4 12/12] netfilter: nf_tables: add hardware offload support Pablo Neira Ayuso
2019-07-10 7:52 ` Jiri Pirko
2019-07-11 0:18 ` Pablo Neira Ayuso
2019-07-09 21:43 ` [PATCH net-next,v4 00/11] netfilter: add hardware offload infrastructure David Miller
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=20190816011033.sw34b3ckfd636afg@salvia \
--to=pablo@netfilter.org \
--cc=ecree@solarflare.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.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).