netdev - Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Feb 2019 19:20:02 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>, davem@...emloft.net,
        netdev@...r.kernel.org, oss-drivers@...ronome.com,
        mkubecek@...e.cz, andrew@...n.ch
Subject: Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device
 via devlink

On Wed, 20 Feb 2019 18:59:05 -0800, Florian Fainelli wrote:
> On 2/19/2019 4:49 PM, Jakub Kicinski wrote:
> > On Tue, 19 Feb 2019 10:19:42 +0100, Jiri Pirko wrote:  
> >> Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@...ronome.com wrote:  
> >>> On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote:    
> >>>>> static const struct ethtool_ops nfp_net_ethtool_ops = {      
> >>>>
> >>>> Why don't you use the compat fallback? I think you should.    
> >>>
> >>> You and Michal both asked the same so let me answer the first to ask :)
> >>> - if devlink is built as a module the fallback is not reachable.    
> >>
> >> So the fallback is not really good as you can't use it for real drivers
> >> anyway. Odd. Maybe we should compile devlink in without possibility to
> >> have it as module.  
> > 
> > Ack, I'll make devlink a bool.  
> 
> Meh how about those poor and memory constrained embedded systems?
> Ideally ethtool should/could have been modular as well, but that ship
> has now sailed.

To be clear - I'm not going to add a dependency.  You'll still be able
to use ethtool without devlink (granted, you won't be able to flash the
device).

It's not immediately clear to me how devlink being modular saves memory.
The way I see it you either:

 (a) not have networking or need any devlink related functionality, and
     therefore do DEVLINK=n;

or:

 (b) have a networking device driver built and loaded which will depend
     on devlink, so devlink will practically speaking always be loaded,
     even if its =m.

> > I need a little extra time, I forgot that nfp's flower offload still
> > doesn't register all ports (using your port flavour infrastructure).
> 
> We have had similar issues with PHYLIB before where we wanted
> net/core/ethtool.c to be able to call into generic PHYLIB functions to
> obtain PHY statistics, an inline helper that de-references the PHY
> device's driver function pointers solved that (look for
> phy_ethtool_get_{strings,sset,stats}) while letting PHYLIB remain modular.
> 
> devlink_compat_flash_update() is a bit big to be inlined, but why not?
> 
> If we make sure we always provide a devlink_mutex and devlink_list that
> symbols such that this builds wheter CONFIG_DEVLINK=y|m then everything
> else can be determined at runtime whether devlink.ko is loaded or not.
> 
> Does that make sense?

I think so, we'd have to have a little object that would always be
built-in when DEVLINK=m, and therefore accessible?