On Fri, 22 Feb 2019 18:27:15 +0100, Michal Kubecek wrote:
> > > Maybe it would be safer not to call ndo_get_devlink directly and have
> > > an inline wrapper like
> > >
> > > #if IS_ENABLED(CONFIG_NET_DEVLINK)
> > > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > > {
> > > if (dev->netdev_ops->ndo_get_devlink)
> > > return dev->netdev_ops->ndo_get_devlink();
> > > else
> > > retrurn NULL;
> > > }
> > > #else
> > > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > > {
> > > return NULL;
> > > }
> > > #endif
> > >
> > > so that one can simply call the wrapper and check return value for NULL.
> >
> > Only devlink code can call this ndo, and it doesn't exist with
> > DEVLINK=n. I don't dislike wrappers for NDOs, but I'll defer to Jiri
> > to decide if we want a wrapper here (without the #if/#else, just the
> > first part for code clarity) :)
>
> If the NDO is only supposed to be called from devlink code (or, more
> precisely, code built only with CONFIG_DEVLINK=y), it should be IMHO
> mentioned in its description. Another option would be enforcing it by
> adding #ifdef around the ndo_get_devlink entry in struct net_device_ops
> but that would require using ifdefs also in each driver providing the
> NDO which seems inconvenient.
Yes, let's just go with your first proposal of a static inline helper.
I think it's the cleanest solution.
