netdev - Re: [net-next PATCH v2] net: dummy: Introduce dummy virtual functions

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]

Date:   Fri, 18 Nov 2016 16:59:47 +0100
From:   Phil Sutter <phil@....cc>
To:     Or Gerlitz <gerlitz.or@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Netdev List <netdev@...r.kernel.org>,
        Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [net-next PATCH v2] net: dummy: Introduce dummy virtual functions

Hi,

On Fri, Nov 18, 2016 at 12:04:14AM +0200, Or Gerlitz wrote:
> On Mon, Nov 14, 2016 at 3:02 PM, Phil Sutter <phil@....cc> wrote:
> 
> > Due to the assumption that all PFs are PCI devices, this implementation
> > is not completely straightforward: In order to allow for
> > rtnl_fill_ifinfo() to see the dummy VFs, a fake PCI parent device is
> > attached to the dummy netdev. This has to happen at the right spot so
> > register_netdevice() does not get confused. This patch abuses
> > ndo_fix_features callback for that. In ndo_uninit callback, the fake
> > parent is removed again for the same purpose.
> 
> So you did some mimic-ing of PCI interface, how do you let the user to
> config the number of VFs? though a module param? why? if the module
> param only serves to say how many VF the device supports, maybe
> support the maximum possible by PCI spec and skip the module param?

Yes, this is controlled via module parameter. But it doesn't say how
much is supported but rather how many dummy VFs are to be created for
each dummy interface.

> > +module_param(num_vfs, int, 0);
> > +MODULE_PARM_DESC(num_vfs, "Number of dummy VFs per dummy device");
> > +
> 
> > @@ -190,6 +382,7 @@ static int __init dummy_init_one(void)
> >         err = register_netdevice(dev_dummy);
> >         if (err < 0)
> >                 goto err;
> > +
> >         return 0;
> 
> nit, remove this added blank line..

Oh yes, thanks. Spontaneous reviewer's blindness. :)

The implementation is problematic in another aspect though: Upon reboot,
it seems like no netdev ops are being called but the fake PCI parent's
kobject is being freed which does not work (and leads to an oops).

Cheers, Phil