Date:   Fri, 26 Apr 2019 11:29:56 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     Jiri Pirko <jiri@...nulli.us>, netfilter-devel@...r.kernel.org,
        davem@...emloft.net, netdev@...r.kernel.org, jiri@...lanox.com,
        john.hurley@...ronome.com, ogerlitz@...lanox.com
Subject: Re: [PATCH net-next,RFC 0/9] net: sched: prepare to reuse per-block
 callbacks from netfilter

On Fri, 26 Apr 2019 18:28:36 +0200, Pablo Neira Ayuso wrote:
> On Fri, Apr 26, 2019 at 04:32:58PM +0200, Jiri Pirko wrote:
> > Fri, Apr 26, 2019 at 02:33:37AM CEST, pablo@...filter.org wrote:  
> > >Hi,
> > >
> > >This patchset aims to introduce changes to reuse the existing .ndo_setup_tc
> > >netdev operations from netfilter.
> > >
> > >The idea is to move tcf_block_cb to net/core/flow_offload.c and rename
> > >it to flow_block_cb. This object provides the minimal infrastructure to
> > >set up per-block callbacks that are called to offload policies to
> > >hardware.
> > >
> > >The tcf_block object is specific for TC to share policies between
> > >ingress devices. This object has a list of tcf_block_cb objects that are
> > >called to offload the policies to hardware. In netfilter, the idea is to
> > >store the list of tcf_block_cb objects in a chain that would be bound to
> > >several devices, eg.
> > >
> > >  chain x {
> > >	type filter hook ingress devices = { eth0, eth1 } priority 0;
> > >	...
> > >  }
> > >  
> > 
> > Do you have the follow-up patchset somewhere? I'm curius about your
> > goal. Without that, it is hard to understand what you are getting at.  
> 
> Goal is to use the TC_SETUP_BLOCK logic in the existing drivers from
> netfilter. So Netfilter calls TC_SETUP_BLOCK by when a chain is set up
> to configure the driver, hence reuse your whole logic with minimal
> changes.
> 
> Currently, the tcf_block_cb_register() call assumes there's a
> tcf_block object in place and it internally invokes the tc
> .reoffload() callback. This tcf_block corresponds to the nft_chain
> object in netfilter, and I need to add my own .reoffload() callback
> for the nft_chain object. This patch uses the block_index instead from
> the driver, instead of exposing tcf_block.

block_index is non-0 only for shared blocks, right?  Did you change
that?

> This patchset updates the TC_SETUP_BLOCK path to only configure the
> block_cb objects. The registration is done from the core, by iterating
> the list of block_cb's that the driver offers in the temporary
> tc_block_offload->cb_list, and then iterate over that list and
> register them from the core.
> 
> My patchset moves the tcf_block_cb object to net/core/flow_offload.c
> (it renames it to flow_block_cb) so it can be used both by tc and
> netfilter.
> 
> Follow up patchset in netfilter calls TC_SETUP_BLOCK when the offloadi
> flag is set on. Then, it has its own version of tc_setup_cb_call(),
> which iterates over the block_cb() in this chain to reuse existing
> driver codebase.
> 
> > >Hence, this emulates the shared blocks available in TC that Jiri made.
> > >
> > >Note that the list of tcf_block_cb objects will be called to offload
> > >policies in this chain.  
> > 
> > So you are going to use chain_id (if there is anything like that) as
> > block_index during offload, right?  
> 
> Yes. But I don't need to expose this chain_index to userspace though,
> I can internally allocate it, I only need to make sure it does not
> overlap with any of the existing tc block_indexed. I can just use a
> different index space which does not overlap with the tc block index
> space.

How will the association between a block and a device work for
netfilter?

Powered by blists - more mailing lists