On Fri, Apr 26, 2019 at 04:27:15PM +0200, Jiri Pirko wrote:
> Fri, Apr 26, 2019 at 02:33:44AM CEST, pablo@xxxxxxxxxxxxx wrote:
> >This allows us to register / unregister tcf_block_cb objects from the
> >core. The idea is to allocate the tcf_block_cb object from the driver,
> >attach it to the tc_block_offload->cb_list, then the core registers
> >them.
> >
> >Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> >---
>
> [...]
>
>
> >--- a/net/dsa/slave.c
> >+++ b/net/dsa/slave.c
> >@@ -902,6 +902,7 @@ static int dsa_slave_setup_tc_block_cb_eg(enum tc_setup_type type,
> > static int dsa_slave_setup_tc_block(struct net_device *dev,
> > struct tc_block_offload *f)
> > {
> >+ struct tcf_block_cb *block_cb;
> > tc_setup_cb_t *cb;
> >
> > if (f->binder_type == TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
> >@@ -913,9 +914,19 @@ static int dsa_slave_setup_tc_block(struct net_device *dev,
> >
> > switch (f->command) {
> > case TC_BLOCK_BIND:
> >- return tcf_block_cb_register(f->block, cb, dev, dev, f->extack);
> >+ block_cb = tcf_block_cb_alloc(f->block->index, cb, dev, dev,
> >+ NULL);
> >+ if (!block_cb)
> >+ return -ENOMEM;
> >+
> >+ tcf_block_cb_list_add(block_cb, &f->cb_list);
> >+ return 0;
> > case TC_BLOCK_UNBIND:
> >- tcf_block_cb_unregister(f->block, cb, dev);
> >+ block_cb = tcf_block_cb_lookup(f->block->index, cb, dev);
> >+ if (!block_cb)
> >+ return -ENOENT;
> >+
> >+ tcf_block_cb_list_move(block_cb, &f->cb_list);
>
> What you are trying to achieve with list_move here, wrapped by
> tcf_block_cb_list_move() for some reason is a mystery for me.
> You are moving to &f->cb_list, but you are already there...
>
> The whole work with lists here, including tcf_block_cb_list is very
> confusing for me.
The f->cb_list is a temporary list. The block_cb's are traveling back
to core through this list. Then, tcf_block_setup() takes these blocks
in this f->cb_list and place them in the right per-block and the
global block list.
So, instead of placing the block_cb's in the block list from the
driver, the driver just places block_cb in this temporary list, so the
core (either tc or netfilter) is responsible for attaching this to
either tcf_block or nft_chain.
