Thread (12 messages) 12 messages, 2 authors, 2019-02-22

Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink

From: Michal Kubecek <hidden>
Date: 2019-02-22 10:04:53

On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
quoted hunk ↗ jump to hunk
Support getting devlink instance from a new NDO.

Signed-off-by: Jakub Kicinski <redacted>
---
 drivers/net/ethernet/netronome/nfp/nfp_app.h        |  2 ++
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c    | 11 +++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  1 +
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c   |  1 +
 4 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index d578d856a009..f8d422713705 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
 int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
 				   struct nfp_net *nn, unsigned int id);
 
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
+
 #endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index db2da99f6aa7..e9eca99cf493 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
 {
 	devlink_port_unregister(&port->dl_port);
 }
+
+struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
+{
+	struct nfp_app *app;
+
+	app = nfp_app_from_netdev(netdev);
+	if (!app)
+		return NULL;
+
+	return priv_to_devlink(app->pf);
+}
AFAICS this would return a pointer to zero initialized struct devlink
when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
would execute

	if (!dev->netdev_ops->ndo_get_devlink)
		return;

	devlink = dev->netdev_ops->ndo_get_devlink(dev);
	if (!devlink || !devlink->ops->info_get)
		return;

with non-null devlink but null devlink->ops so that it dereferences null
pointer (and so does devlink_compat_flash_update()).

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.

Michal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help