On Mon, 29 Oct 2018 17:12:27 +0200, Or Gerlitz wrote:

quoted

quoted

Maybe it would be better to follow the trusted environment model of the kernel
and not protect the core from driver bugs? If the driver does things right they
will unregister before bailing out and if not, they will have to fix..  

quoted

The owner stuff just makes it easier for a driver to track the blocks
it has registered for and, in turn, release these when exiting.
We could just leave this up to the driver to ensure it properly cleans
up after itself.  

If it makes the life of the driver easier and doesn't add notable complexity,
then I think I am good to leave it

quoted

I don't feel that strongly either way.  

m2

So lets see if other comment here, if not, we can just leave it, I guess

To be honest big part of why we retained this mechanism was to keep the
per-driver core structure in existence (struct tcf_indr_block_owner).
In my experience it is way easier to move common functionality into the
core if there is a place where core can track offload-related state.

Growing core structures just for offloads is not super advisable, so
unless there is a separate structure core allocates - all state lands in
the drivers.  This lesson comes from BPF offload, which started off as
mostly stateless from core's perspective where all operations were muxed
via a single NDO, but that became increasingly awkward to use.  We are
gradually moving to a "offload device + ops" form.

I'm not 100% sure the indirect callbacks are a good place for a core
structure, given we didn't seem to need such a thing for normal TC
blocks.  So yes, perhaps we should drop that code.

Hope that explanation makes sense.