On Mon, 29 Oct 2018 17:12:27 +0200, Or Gerlitz wrote: > >> 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.. > > > 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 > > > 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.