netdev - Re: [PATCH v3 net-next 4/8] octeontx2-vf: Ethtool support

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:   Thu, 19 Mar 2020 15:42:11 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     sunil.kovvuri@...il.com, netdev@...r.kernel.org,
        davem@...emloft.net, leon@...nel.org,
        Tomasz Duszynski <tduszynski@...vell.com>,
        Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [PATCH v3 net-next 4/8] octeontx2-vf: Ethtool support

On Thu, 19 Mar 2020 16:56:31 +0100 Andrew Lunn wrote:
> On Thu, Mar 19, 2020 at 06:37:24PM +0530, sunil.kovvuri@...il.com wrote:
> > From: Tomasz Duszynski <tduszynski@...vell.com>
> > 
> > Added ethtool support for VF devices for
> >  - Driver stats, Tx/Rx perqueue stats
> >  - Set/show Rx/Tx queue count
> >  - Set/show Rx/Tx ring sizes
> >  - Set/show IRQ coalescing parameters
> >  - RSS configuration etc
> > 
> > It's the PF which owns the interface, hence VF
> > cannot display underlying CGX interface stats.
> > Except for this rest ethtool support reuses PF's
> > APIs.
> > 
> > Signed-off-by: Tomasz Duszynski <tduszynski@...vell.com>
> > Signed-off-by: Sunil Goutham <sgoutham@...vell.com>  
> 
> Reviewed-by: Andrew Lunn <andrew@...n.ch>

But they didn't add static inlines, no? Don't the dependencies 
look strange?

VF depends on PF code, but ethtool code (part of PF) also needs 
symbols from the VF..

Are the kconfig options really needed? If the dependencies are
circular, this will either not work well or have all the modules 
loaded together. So let's just have everything under one kconfig 
knob and not bother breaking it up at module boundaries..

Note that it's perfectly fine to have multiple drivers in one module
(it used to be harder long time ago).