netdev - Re: [PATCH net-next 3/5] devlink: create a special NDO for getting the devlink instance

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]

Message-ID: <20190222085630.6e9e764c@cakuba.netronome.com>
Date:   Fri, 22 Feb 2019 08:56:30 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     davem@...emloft.net, jiri@...nulli.us, andrew@...n.ch,
        f.fainelli@...il.com, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH net-next 3/5] devlink: create a special NDO for getting
 the devlink instance

On Fri, 22 Feb 2019 10:51:31 +0100, Michal Kubecek wrote:
> On Thu, Feb 21, 2019 at 09:46:18AM -0800, Jakub Kicinski wrote:
> > Instead of iterating over all devlink ports add a NDO which
> > will return the devlink instance from the driver.
> > 
> > Suggested-by: Jiri Pirko <jiri@...nulli.us>
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> > ---
> >  include/linux/netdevice.h |  6 ++++
> >  net/core/devlink.c        | 62 +++++++++++++--------------------------
> >  2 files changed, 26 insertions(+), 42 deletions(-)
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index aab4d9f6613d..eebcef6b8191 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -940,6 +940,8 @@ struct dev_ifalias {
> >  	char ifalias[];
> >  };
> >  
> > +struct devlink;
> > +
> >  /*
> >   * This structure defines the management hooks for network devices.
> >   * The following hooks can be defined; unless noted otherwise, they are
> > @@ -1248,6 +1250,9 @@ struct dev_ifalias {
> >   *	that got dropped are freed/returned via xdp_return_frame().
> >   *	Returns negative number, means general error invoking ndo, meaning
> >   *	no frames were xmit'ed and core-caller will free all frames.
> > + * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
> > + *	Get devlink instance associated with a given netdev.
> > + *	Called with a reference on the device only, rtnl_lock is not held.  
> 
> The formulation is a bit ambiguous. Do I understand correctly that what
> it says is that device reference is sufficient and rtnl_lock is not
> necessary (but there is no harm if caller holds rtnl_lock)?

Yes, only reference is held, the expectation is that drivers free
netdevs before they unregister devlink.  The devlink vs rtnl ordering
gets a bit tricky, drivers should definitely not expect rtnl to be held
in this NDO.  If the caller comes from rtnl path (like ethtool) it will
likely have to release rtnl before taking devlink->lock.