On Fri, 22 Feb 2019 18:27:15 +0100, Michal Kubecek wrote:

quoted

quoted

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.