netdev - Re: [RFC net-next v2 1/8] net: sched: register callbacks for indirect tc block binds

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20181029113653.04fc3d2b@cakuba.netronome.com>
Date:   Mon, 29 Oct 2018 11:36:53 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Or Gerlitz <gerlitz.or@...il.com>
Cc:     John Hurley <john.hurley@...ronome.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com, Jiri Pirko <jiri@...nulli.us>,
        Oz Shlomo <ozsh@...lanox.com>,
        Simon Horman <simon.horman@...ronome.com>,
        Aviv Heller <avivh@...lanox.com>
Subject: Re: [RFC net-next v2 1/8] net: sched: register callbacks for
 indirect tc block binds

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.