Subject: Re: [PATCH 7/8 net] bnxt_tc: update indirect block support

From: Pablo Neira Ayuso <[email-masked]>
Date: Tue, 26 May 2020 23:59:53 +0200

Hi,

I overlook this email, sorry.

On Tue, May 19, 2020 at 02:23:54PM +0530, Sriharsha Basavapatna wrote:
[...]
> > @@ -2101,7 +2073,8 @@ void bnxt_shutdown_tc(struct bnxt *bp)
> >         if (!bnxt_tc_flower_enabled(bp))
> >                 return;
> >
> > -       unregister_netdevice_notifier(&bp->tc_netdev_nb);
> > +       flow_indr_dev_unregister(bnxt_tc_setup_indr_cb, bp,
> > +                                bnxt_tc_setup_indr_block_cb);
> 
> Why does the driver need to provide the "cb" again during unregister,
> since both "cb" and "cb_priv" are already provided during register() ?
> This interface could be simplified/improved if
> flow_indr_dev_register() returns an opaque handle to the object it
> creates (struct flow_indr_dev *) ?

Probably, at the expense to storing this in the netdev private area.

> The driver should just pass this
> handle during unregistration. Also, why do we need the extra (3rd)
> argument (flow_setup_cb_t / bnxt_tc_setup_indr_block_cb) during unreg
> ? It is handled internally by the driver as a part of FLOW_BLOCK_BIND
> / UNBIND ?

flow_indr_dev_unregister() needs bnxt_tc_setup_indr_block_cb to
identify what indirect flow_blocks need to be cleaned up before this
representor is gone.