From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [RFC net-next v2 1/8] net: sched: register callbacks for indirect tc block binds Date: Mon, 29 Oct 2018 11:36:53 -0700 Message-ID: <20181029113653.04fc3d2b@cakuba.netronome.com> References: <1540470417-14803-1-git-send-email-john.hurley@netronome.com> <1540470417-14803-2-git-send-email-john.hurley@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: John Hurley , Linux Netdev List , oss-drivers@netronome.com, Jiri Pirko , Oz Shlomo , Simon Horman , Aviv Heller To: Or Gerlitz Return-path: Received: from mail-ed1-f68.google.com ([209.85.208.68]:44488 "EHLO mail-ed1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728488AbeJ3D0y (ORCPT ); Mon, 29 Oct 2018 23:26:54 -0400 Received: by mail-ed1-f68.google.com with SMTP id z21-v6so8216083edb.11 for ; Mon, 29 Oct 2018 11:37:02 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: 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.