All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
@ 2019-02-26  3:34 Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in Jakub Kicinski
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Hi!

This is a follow up to the series which added device flash
updates via devlink. I went with the approach of adding a
new NDO in the end. It seems to end up looking cleaner.

First patch removes the option to build devlink as a module.
Users can still decide to not build it, but the module option
ends up not being worth the maintenance cost.

Next two patches add a NDO which can be used to ask the driver
to return a devlink instance associated with a given netdev,
instead of iterating over devlink ports. Drivers which implement
this NDO must take into account the potential impact on the
visibility of the devlink instance.

With the new NDO in place we can remove NFP ethtool flash update
code.

Fifth patch makes sure we hold a reference to dev while
callbacks are active.

Last but not least the NULL-check of devlink->ops is moved
to instance allocation time.

Last but not least missing checks for devlink->ops are added.
There is currently no driver registering devlink without ops,
so can just fix this in -next.

v2 (Michal): add netdev_to_devlink() in patch 3.
v3 (Florian):
 - add missing checks for devlink->ops;
 - move locking/holding into devlink_compat_ functions.
v4 (Jiri):
 - hold devlink_mutex around callbacks (patch 2);
 - require non-NULL ops (patch 6).
 
Jakub Kicinski (6):
  net: devlink: turn devlink into a built-in
  devlink: create a special NDO for getting the devlink instance
  nfp: add .ndo_get_devlink
  nfp: remove ethtool flashing fallback
  devlink: hold a reference to the netdevice around ethtool compat
  devlink: require non-NULL ops for devlink instances

 drivers/infiniband/hw/bnxt_re/Kconfig         |   1 -
 drivers/infiniband/hw/mlx4/Kconfig            |   1 -
 drivers/net/Kconfig                           |   1 -
 drivers/net/ethernet/broadcom/Kconfig         |   1 -
 drivers/net/ethernet/cavium/Kconfig           |   1 -
 drivers/net/ethernet/mellanox/mlx4/Kconfig    |   1 -
 .../net/ethernet/mellanox/mlx5/core/Kconfig   |   1 -
 drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 -
 drivers/net/ethernet/netronome/Kconfig        |   1 -
 drivers/net/ethernet/netronome/nfp/nfp_app.h  |   2 +
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  11 ++
 .../ethernet/netronome/nfp/nfp_net_common.c   |   1 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  |  24 ----
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |   1 +
 include/linux/netdevice.h                     |   7 +
 include/net/devlink.h                         |  19 ++-
 net/Kconfig                                   |  11 +-
 net/core/devlink.c                            | 126 +++++++-----------
 net/core/ethtool.c                            |  13 +-
 net/dsa/Kconfig                               |   2 +-
 20 files changed, 90 insertions(+), 136 deletions(-)

-- 
2.19.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-28 15:01   ` Jiri Pirko
  2019-02-26  3:34 ` [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance Jakub Kicinski
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Being able to build devlink as a module causes growing pains.
First all drivers had to add a meta dependency to make sure
they are not built in when devlink is built as a module.  Now
we are struggling to invoke ethtool compat code reliably.

Make devlink code built-in, users can still not build it at
all but the dynamically loadable module option is removed.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/infiniband/hw/bnxt_re/Kconfig           |  1 -
 drivers/infiniband/hw/mlx4/Kconfig              |  1 -
 drivers/net/Kconfig                             |  1 -
 drivers/net/ethernet/broadcom/Kconfig           |  1 -
 drivers/net/ethernet/cavium/Kconfig             |  1 -
 drivers/net/ethernet/mellanox/mlx4/Kconfig      |  1 -
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig |  1 -
 drivers/net/ethernet/mellanox/mlxsw/Kconfig     |  1 -
 drivers/net/ethernet/netronome/Kconfig          |  1 -
 include/net/devlink.h                           | 10 ++++------
 net/Kconfig                                     | 11 +----------
 net/core/devlink.c                              | 15 ++-------------
 net/dsa/Kconfig                                 |  2 +-
 13 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
index 18f5ed082f41..19982a4a9bba 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,7 +1,6 @@
 config INFINIBAND_BNXT_RE
     tristate "Broadcom Netxtreme HCA support"
     depends on ETHERNET && NETDEVICES && PCI && INET && DCB
-    depends on MAY_USE_DEVLINK
     select NET_VENDOR_BROADCOM
     select BNXT
     ---help---
diff --git a/drivers/infiniband/hw/mlx4/Kconfig b/drivers/infiniband/hw/mlx4/Kconfig
index d1de3285fd88..4e9936731867 100644
--- a/drivers/infiniband/hw/mlx4/Kconfig
+++ b/drivers/infiniband/hw/mlx4/Kconfig
@@ -2,7 +2,6 @@ config MLX4_INFINIBAND
 	tristate "Mellanox ConnectX HCA support"
 	depends on NETDEVICES && ETHERNET && PCI && INET
 	depends on INFINIBAND_USER_ACCESS || !INFINIBAND_USER_ACCESS
-	depends on MAY_USE_DEVLINK
 	select NET_VENDOR_MELLANOX
 	select MLX4_CORE
 	---help---
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6210757772ed..5e4ca082cfcd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -505,7 +505,6 @@ source "drivers/net/hyperv/Kconfig"
 config NETDEVSIM
 	tristate "Simulated networking device"
 	depends on DEBUG_FS
-	depends on MAY_USE_DEVLINK
 	help
 	  This driver is a developer testing tool and software model that can
 	  be used to test various control path networking APIs, especially
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c1d3ee9baf7e..716bfbba59cf 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,7 +194,6 @@ config SYSTEMPORT
 config BNXT
 	tristate "Broadcom NetXtreme-C/E support"
 	depends on PCI
-	depends on MAY_USE_DEVLINK
 	select FW_LOADER
 	select LIBCRC32C
 	---help---
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 05f4a3b21e29..6650e2a5f171 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -64,7 +64,6 @@ config CAVIUM_PTP
 config LIQUIDIO
 	tristate "Cavium LiquidIO support"
 	depends on 64BIT && PCI
-	depends on MAY_USE_DEVLINK
 	depends on PCI
 	imply PTP_1588_CLOCK
 	select FW_LOADER
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index f200b8c420d5..ff8057ed97ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -4,7 +4,6 @@
 
 config MLX4_EN
 	tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
-	depends on MAY_USE_DEVLINK
 	depends on PCI && NETDEVICES && ETHERNET && INET
 	select MLX4_CORE
 	imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37a551436e4a..6debffb8336b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -4,7 +4,6 @@
 
 config MLX5_CORE
 	tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
-	depends on MAY_USE_DEVLINK
 	depends on PCI
 	imply PTP_1588_CLOCK
 	imply VXLAN
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9a25aed5d11..9c195dfed031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -4,7 +4,6 @@
 
 config MLXSW_CORE
 	tristate "Mellanox Technologies Switch ASICs support"
-	depends on MAY_USE_DEVLINK
 	---help---
 	  This driver supports Mellanox Technologies Switch ASICs family.
 
diff --git a/drivers/net/ethernet/netronome/Kconfig b/drivers/net/ethernet/netronome/Kconfig
index 66f15b05b65e..549898d5d450 100644
--- a/drivers/net/ethernet/netronome/Kconfig
+++ b/drivers/net/ethernet/netronome/Kconfig
@@ -19,7 +19,6 @@ config NFP
 	tristate "Netronome(R) NFP4000/NFP6000 NIC driver"
 	depends on PCI && PCI_MSI
 	depends on VXLAN || VXLAN=n
-	depends on MAY_USE_DEVLINK
 	---help---
 	  This driver supports the Netronome(R) NFP4000/NFP6000 based
 	  cards working as a advanced Ethernet NIC.  It works with both
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a2da49dd9147..f9f7fe974652 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -707,6 +707,10 @@ devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
 int devlink_health_report(struct devlink_health_reporter *reporter,
 			  const char *msg, void *priv_ctx);
 
+void devlink_compat_running_version(struct net_device *dev,
+				    char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
+
 #else
 
 static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -1190,13 +1194,7 @@ devlink_health_report(struct devlink_health_reporter *reporter,
 {
 	return 0;
 }
-#endif
 
-#if IS_REACHABLE(CONFIG_NET_DEVLINK)
-void devlink_compat_running_version(struct net_device *dev,
-				    char *buf, size_t len);
-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
-#else
 static inline void
 devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
 {
diff --git a/net/Kconfig b/net/Kconfig
index 62da6148e9f8..1efe1f9ee492 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,21 +429,12 @@ config NET_SOCK_MSG
 	  with the help of BPF programs.
 
 config NET_DEVLINK
-	tristate "Network physical/parent device Netlink interface"
+	bool "Network physical/parent device Netlink interface"
 	help
 	  Network physical/parent device Netlink interface provides
 	  infrastructure to support access to physical chip-wide config and
 	  monitoring.
 
-config MAY_USE_DEVLINK
-	tristate
-	default m if NET_DEVLINK=m
-	default y if NET_DEVLINK=y || NET_DEVLINK=n
-	help
-	  Drivers using the devlink infrastructure should have a dependency
-	  on MAY_USE_DEVLINK to ensure they do not cause link errors when
-	  devlink is a loadable module and the driver using it is built-in.
-
 config PAGE_POOL
        bool
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4f31ddc883e7..05e04ea0a5c7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6482,20 +6482,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 	return -EOPNOTSUPP;
 }
 
-static int __init devlink_module_init(void)
+static int __init devlink_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
 }
 
-static void __exit devlink_module_exit(void)
-{
-	genl_unregister_family(&devlink_nl_family);
-}
-
-module_init(devlink_module_init);
-module_exit(devlink_module_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
-MODULE_DESCRIPTION("Network physical device Netlink interface");
-MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
+subsys_initcall(devlink_init);
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 91e52973ee13..fab49132345f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -6,7 +6,7 @@ config HAVE_NET_DSA
 
 config NET_DSA
 	tristate "Distributed Switch Architecture"
-	depends on HAVE_NET_DSA && MAY_USE_DEVLINK
+	depends on HAVE_NET_DSA
 	depends on BRIDGE || BRIDGE=n
 	select NET_SWITCHDEV
 	select PHYLINK
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-26  8:56   ` Jiri Pirko
  2019-02-26  3:34 ` [PATCH net-next v4 3/6] nfp: add .ndo_get_devlink Jakub Kicinski
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Instead of iterating over all devlink ports add a NDO which
will return the devlink instance from the driver.

v2: add the netdev_to_devlink() helper (Michal)
v3: check that devlink has ops (Florian)
v4: hold devlink_mutex (Jiri)

Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/linux/netdevice.h |  7 +++++
 include/net/devlink.h     |  9 +++++++
 net/core/devlink.c        | 56 ++++++++++++---------------------------
 3 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ffbddd03242b..58e83bd7a861 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -941,6 +941,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
@@ -1249,6 +1251,10 @@ 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 netdevice and devlink locks only,
+ *	rtnl_lock is not held.
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);
@@ -1447,6 +1453,7 @@ struct net_device_ops {
 						u32 flags);
 	int			(*ndo_xsk_async_xmit)(struct net_device *dev,
 						      u32 queue_id);
+	struct devlink *	(*ndo_get_devlink)(struct net_device *dev);
 };
 
 /**
diff --git a/include/net/devlink.h b/include/net/devlink.h
index f9f7fe974652..7f5a0bdca228 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -538,6 +538,15 @@ static inline struct devlink *priv_to_devlink(void *priv)
 	return container_of(priv, struct devlink, priv);
 }
 
+static inline struct devlink *netdev_to_devlink(struct net_device *dev)
+{
+#if IS_ENABLED(CONFIG_NET_DEVLINK)
+	if (dev->netdev_ops->ndo_get_devlink)
+		return dev->netdev_ops->ndo_get_devlink(dev);
+#endif
+	return NULL;
+}
+
 struct ib_device;
 
 #if IS_ENABLED(CONFIG_NET_DEVLINK)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 05e04ea0a5c7..24bfbd2d71e7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6397,9 +6397,6 @@ static void __devlink_compat_running_version(struct devlink *devlink,
 	struct sk_buff *msg;
 	int rem, err;
 
-	if (!devlink->ops->info_get)
-		return;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return;
@@ -6431,55 +6428,36 @@ static void __devlink_compat_running_version(struct devlink *devlink,
 void devlink_compat_running_version(struct net_device *dev,
 				    char *buf, size_t len)
 {
-	struct devlink_port *devlink_port;
 	struct devlink *devlink;
 
 	mutex_lock(&devlink_mutex);
-	list_for_each_entry(devlink, &devlink_list, list) {
-		mutex_lock(&devlink->lock);
-		list_for_each_entry(devlink_port, &devlink->port_list, list) {
-			if (devlink_port->type == DEVLINK_PORT_TYPE_ETH &&
-			    devlink_port->type_dev == dev) {
-				__devlink_compat_running_version(devlink,
-								 buf, len);
-				mutex_unlock(&devlink->lock);
-				goto out;
-			}
-		}
-		mutex_unlock(&devlink->lock);
-	}
-out:
+	devlink = netdev_to_devlink(dev);
+	if (!devlink || !devlink->ops || !devlink->ops->info_get)
+		goto unlock_list;
+
+	mutex_lock(&devlink->lock);
+	__devlink_compat_running_version(devlink, buf, len);
+	mutex_unlock(&devlink->lock);
+unlock_list:
 	mutex_unlock(&devlink_mutex);
 }
 
 int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 {
-	struct devlink_port *devlink_port;
 	struct devlink *devlink;
+	int ret = -EOPNOTSUPP;
 
 	mutex_lock(&devlink_mutex);
-	list_for_each_entry(devlink, &devlink_list, list) {
-		mutex_lock(&devlink->lock);
-		list_for_each_entry(devlink_port, &devlink->port_list, list) {
-			int ret = -EOPNOTSUPP;
-
-			if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
-			    devlink_port->type_dev != dev)
-				continue;
+	devlink = netdev_to_devlink(dev);
+	if (!devlink || !devlink->ops || !devlink->ops->flash_update)
+		goto unlock_list;
 
-			mutex_unlock(&devlink_mutex);
-			if (devlink->ops->flash_update)
-				ret = devlink->ops->flash_update(devlink,
-								 file_name,
-								 NULL, NULL);
-			mutex_unlock(&devlink->lock);
-			return ret;
-		}
-		mutex_unlock(&devlink->lock);
-	}
+	mutex_lock(&devlink->lock);
+	ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
+	mutex_unlock(&devlink->lock);
+unlock_list:
 	mutex_unlock(&devlink_mutex);
-
-	return -EOPNOTSUPP;
+	return ret;
 }
 
 static int __init devlink_init(void)
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 3/6] nfp: add .ndo_get_devlink
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 4/6] nfp: remove ethtool flashing fallback Jakub Kicinski
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Support getting devlink instance from a new NDO.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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);
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 776f6c07701b..6d1b8816552e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3531,6 +3531,7 @@ const struct net_device_ops nfp_net_netdev_ops = {
 	.ndo_udp_tunnel_del	= nfp_net_del_vxlan_port,
 	.ndo_bpf		= nfp_net_xdp,
 	.ndo_get_port_parent_id	= nfp_port_get_port_parent_id,
+	.ndo_get_devlink	= nfp_devlink_get_devlink,
 };
 
 /**
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 62839807e21e..d2c803bb4e56 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -273,6 +273,7 @@ const struct net_device_ops nfp_repr_netdev_ops = {
 	.ndo_set_features	= nfp_port_set_features,
 	.ndo_set_mac_address    = eth_mac_addr,
 	.ndo_get_port_parent_id	= nfp_port_get_port_parent_id,
+	.ndo_get_devlink	= nfp_devlink_get_devlink,
 };
 
 void
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 4/6] nfp: remove ethtool flashing fallback
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
                   ` (2 preceding siblings ...)
  2019-02-26  3:34 ` [PATCH net-next v4 3/6] nfp: add .ndo_get_devlink Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-26  3:34 ` [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat Jakub Kicinski
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Now that devlink fallback will be called reliably, we can remove
the ethtool flashing code.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 24 -------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 8f189149efc5..690b62718dbb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1234,28 +1234,6 @@ static int nfp_net_set_channels(struct net_device *netdev,
 	return nfp_net_set_num_rings(nn, total_rx, total_tx);
 }
 
-static int
-nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
-{
-	struct nfp_app *app;
-	int ret;
-
-	if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
-		return -EOPNOTSUPP;
-
-	app = nfp_app_from_netdev(netdev);
-	if (!app)
-		return -EOPNOTSUPP;
-
-	dev_hold(netdev);
-	rtnl_unlock();
-	ret = nfp_flash_update_common(app->pf, flash->data, NULL);
-	rtnl_lock();
-	dev_put(netdev);
-
-	return ret;
-}
-
 static const struct ethtool_ops nfp_net_ethtool_ops = {
 	.get_drvinfo		= nfp_net_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
@@ -1266,7 +1244,6 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
 	.get_sset_count		= nfp_net_get_sset_count,
 	.get_rxnfc		= nfp_net_get_rxnfc,
 	.set_rxnfc		= nfp_net_set_rxnfc,
-	.flash_device		= nfp_net_flash_device,
 	.get_rxfh_indir_size	= nfp_net_get_rxfh_indir_size,
 	.get_rxfh_key_size	= nfp_net_get_rxfh_key_size,
 	.get_rxfh		= nfp_net_get_rxfh,
@@ -1292,7 +1269,6 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
 	.get_strings		= nfp_port_get_strings,
 	.get_ethtool_stats	= nfp_port_get_stats,
 	.get_sset_count		= nfp_port_get_sset_count,
-	.flash_device		= nfp_net_flash_device,
 	.set_dump		= nfp_app_set_dump,
 	.get_dump_flag		= nfp_app_get_dump_flag,
 	.get_dump_data		= nfp_app_get_dump_data,
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
                   ` (3 preceding siblings ...)
  2019-02-26  3:34 ` [PATCH net-next v4 4/6] nfp: remove ethtool flashing fallback Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-26  4:26   ` Florian Fainelli
  2019-02-26  8:57   ` Jiri Pirko
  2019-02-26  3:34 ` [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances Jakub Kicinski
  2019-02-26 16:50 ` [PATCH net-next v4 0/6] devlink: make ethtool compat reliable David Miller
  6 siblings, 2 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

When ethtool is calling into devlink compat code make sure we have
a reference on the netdevice on which the operation was invoked.

v3: move the hold/lock logic into devlink_compat_* functions (Florian)

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/core/devlink.c | 13 +++++++++++++
 net/core/ethtool.c | 13 ++-----------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 24bfbd2d71e7..ecff6e63bc4d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6430,6 +6430,9 @@ void devlink_compat_running_version(struct net_device *dev,
 {
 	struct devlink *devlink;
 
+	dev_hold(dev);
+	rtnl_unlock();
+
 	mutex_lock(&devlink_mutex);
 	devlink = netdev_to_devlink(dev);
 	if (!devlink || !devlink->ops || !devlink->ops->info_get)
@@ -6440,6 +6443,9 @@ void devlink_compat_running_version(struct net_device *dev,
 	mutex_unlock(&devlink->lock);
 unlock_list:
 	mutex_unlock(&devlink_mutex);
+
+	rtnl_lock();
+	dev_put(dev);
 }
 
 int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
@@ -6447,6 +6453,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 	struct devlink *devlink;
 	int ret = -EOPNOTSUPP;
 
+	dev_hold(dev);
+	rtnl_unlock();
+
 	mutex_lock(&devlink_mutex);
 	devlink = netdev_to_devlink(dev);
 	if (!devlink || !devlink->ops || !devlink->ops->flash_update)
@@ -6457,6 +6466,10 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 	mutex_unlock(&devlink->lock);
 unlock_list:
 	mutex_unlock(&devlink_mutex);
+
+	rtnl_lock();
+	dev_put(dev);
+
 	return ret;
 }
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1320e8dce559..47558ffae423 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -805,11 +805,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
 	if (ops->get_eeprom_len)
 		info.eedump_len = ops->get_eeprom_len(dev);
 
-	rtnl_unlock();
 	if (!info.fw_version[0])
 		devlink_compat_running_version(dev, info.fw_version,
 					       sizeof(info.fw_version));
-	rtnl_lock();
 
 	if (copy_to_user(useraddr, &info, sizeof(info)))
 		return -EFAULT;
@@ -2040,15 +2038,8 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
 		return -EFAULT;
 	efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
 
-	if (!dev->ethtool_ops->flash_device) {
-		int ret;
-
-		rtnl_unlock();
-		ret = devlink_compat_flash_update(dev, efl.data);
-		rtnl_lock();
-
-		return ret;
-	}
+	if (!dev->ethtool_ops->flash_device)
+		return devlink_compat_flash_update(dev, efl.data);
 
 	return dev->ethtool_ops->flash_device(dev, &efl);
 }
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
                   ` (4 preceding siblings ...)
  2019-02-26  3:34 ` [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat Jakub Kicinski
@ 2019-02-26  3:34 ` Jakub Kicinski
  2019-02-26  4:29   ` Florian Fainelli
  2019-02-26  8:59   ` Jiri Pirko
  2019-02-26 16:50 ` [PATCH net-next v4 0/6] devlink: make ethtool compat reliable David Miller
  6 siblings, 2 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Commit 76726ccb7f46 ("devlink: add flash update command") and
commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
access devlink ops without NULL-checking. There is, however, no
driver which would pass in NULL ops, so let's just make that
a requirement. Remove the now unnecessary NULL-checking.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/core/devlink.c | 48 +++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index ecff6e63bc4d..a49dee67e66f 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -723,7 +723,7 @@ static int devlink_port_type_set(struct devlink *devlink,
 {
 	int err;
 
-	if (devlink->ops && devlink->ops->port_type_set) {
+	if (devlink->ops->port_type_set) {
 		if (port_type == DEVLINK_PORT_TYPE_NOTSET)
 			return -EINVAL;
 		if (port_type == devlink_port->type)
@@ -760,7 +760,7 @@ static int devlink_port_split(struct devlink *devlink, u32 port_index,
 			      u32 count, struct netlink_ext_ack *extack)
 
 {
-	if (devlink->ops && devlink->ops->port_split)
+	if (devlink->ops->port_split)
 		return devlink->ops->port_split(devlink, port_index, count,
 						extack);
 	return -EOPNOTSUPP;
@@ -786,7 +786,7 @@ static int devlink_port_unsplit(struct devlink *devlink, u32 port_index,
 				struct netlink_ext_ack *extack)
 
 {
-	if (devlink->ops && devlink->ops->port_unsplit)
+	if (devlink->ops->port_unsplit)
 		return devlink->ops->port_unsplit(devlink, port_index, extack);
 	return -EOPNOTSUPP;
 }
@@ -961,7 +961,7 @@ static int devlink_nl_cmd_sb_pool_get_doit(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (!devlink->ops || !devlink->ops->sb_pool_get)
+	if (!devlink->ops->sb_pool_get)
 		return -EOPNOTSUPP;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1017,7 +1017,7 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
 	mutex_lock(&devlink_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
-		    !devlink->ops || !devlink->ops->sb_pool_get)
+		    !devlink->ops->sb_pool_get)
 			continue;
 		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
@@ -1046,7 +1046,7 @@ static int devlink_sb_pool_set(struct devlink *devlink, unsigned int sb_index,
 {
 	const struct devlink_ops *ops = devlink->ops;
 
-	if (ops && ops->sb_pool_set)
+	if (ops->sb_pool_set)
 		return ops->sb_pool_set(devlink, sb_index, pool_index,
 					size, threshold_type);
 	return -EOPNOTSUPP;
@@ -1151,7 +1151,7 @@ static int devlink_nl_cmd_sb_port_pool_get_doit(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (!devlink->ops || !devlink->ops->sb_port_pool_get)
+	if (!devlink->ops->sb_port_pool_get)
 		return -EOPNOTSUPP;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1213,7 +1213,7 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
 	mutex_lock(&devlink_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
-		    !devlink->ops || !devlink->ops->sb_port_pool_get)
+		    !devlink->ops->sb_port_pool_get)
 			continue;
 		mutex_lock(&devlink->lock);
 		list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
@@ -1242,7 +1242,7 @@ static int devlink_sb_port_pool_set(struct devlink_port *devlink_port,
 {
 	const struct devlink_ops *ops = devlink_port->devlink->ops;
 
-	if (ops && ops->sb_port_pool_set)
+	if (ops->sb_port_pool_set)
 		return ops->sb_port_pool_set(devlink_port, sb_index,
 					     pool_index, threshold);
 	return -EOPNOTSUPP;
@@ -1355,7 +1355,7 @@ static int devlink_nl_cmd_sb_tc_pool_bind_get_doit(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (!devlink->ops || !devlink->ops->sb_tc_pool_bind_get)
+	if (!devlink->ops->sb_tc_pool_bind_get)
 		return -EOPNOTSUPP;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -1439,7 +1439,7 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
 	mutex_lock(&devlink_mutex);
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
-		    !devlink->ops || !devlink->ops->sb_tc_pool_bind_get)
+		    !devlink->ops->sb_tc_pool_bind_get)
 			continue;
 
 		mutex_lock(&devlink->lock);
@@ -1471,7 +1471,7 @@ static int devlink_sb_tc_pool_bind_set(struct devlink_port *devlink_port,
 {
 	const struct devlink_ops *ops = devlink_port->devlink->ops;
 
-	if (ops && ops->sb_tc_pool_bind_set)
+	if (ops->sb_tc_pool_bind_set)
 		return ops->sb_tc_pool_bind_set(devlink_port, sb_index,
 						tc_index, pool_type,
 						pool_index, threshold);
@@ -1519,7 +1519,7 @@ static int devlink_nl_cmd_sb_occ_snapshot_doit(struct sk_buff *skb,
 	struct devlink_sb *devlink_sb = info->user_ptr[1];
 	const struct devlink_ops *ops = devlink->ops;
 
-	if (ops && ops->sb_occ_snapshot)
+	if (ops->sb_occ_snapshot)
 		return ops->sb_occ_snapshot(devlink, devlink_sb->index);
 	return -EOPNOTSUPP;
 }
@@ -1531,7 +1531,7 @@ static int devlink_nl_cmd_sb_occ_max_clear_doit(struct sk_buff *skb,
 	struct devlink_sb *devlink_sb = info->user_ptr[1];
 	const struct devlink_ops *ops = devlink->ops;
 
-	if (ops && ops->sb_occ_max_clear)
+	if (ops->sb_occ_max_clear)
 		return ops->sb_occ_max_clear(devlink, devlink_sb->index);
 	return -EOPNOTSUPP;
 }
@@ -1594,13 +1594,9 @@ static int devlink_nl_cmd_eswitch_get_doit(struct sk_buff *skb,
 					   struct genl_info *info)
 {
 	struct devlink *devlink = info->user_ptr[0];
-	const struct devlink_ops *ops = devlink->ops;
 	struct sk_buff *msg;
 	int err;
 
-	if (!ops)
-		return -EOPNOTSUPP;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -1625,9 +1621,6 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
 	int err = 0;
 	u16 mode;
 
-	if (!ops)
-		return -EOPNOTSUPP;
-
 	if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
 		if (!ops->eswitch_mode_set)
 			return -EOPNOTSUPP;
@@ -3869,7 +3862,7 @@ static int devlink_nl_cmd_info_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	if (!devlink->ops || !devlink->ops->info_get)
+	if (!devlink->ops->info_get)
 		return -EOPNOTSUPP;
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -5232,6 +5225,9 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
 {
 	struct devlink *devlink;
 
+	if (WARN_ON(!ops))
+		return NULL;
+
 	devlink = kzalloc(sizeof(*devlink) + priv_size, GFP_KERNEL);
 	if (!devlink)
 		return NULL;
@@ -6091,7 +6087,7 @@ __devlink_param_driverinit_value_set(struct devlink *devlink,
 int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 				       union devlink_param_value *init_val)
 {
-	if (!devlink->ops || !devlink->ops->reload)
+	if (!devlink->ops->reload)
 		return -EOPNOTSUPP;
 
 	return __devlink_param_driverinit_value_get(&devlink->param_list,
@@ -6138,7 +6134,7 @@ int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
 {
 	struct devlink *devlink = devlink_port->devlink;
 
-	if (!devlink->ops || !devlink->ops->reload)
+	if (!devlink->ops->reload)
 		return -EOPNOTSUPP;
 
 	return __devlink_param_driverinit_value_get(&devlink_port->param_list,
@@ -6435,7 +6431,7 @@ void devlink_compat_running_version(struct net_device *dev,
 
 	mutex_lock(&devlink_mutex);
 	devlink = netdev_to_devlink(dev);
-	if (!devlink || !devlink->ops || !devlink->ops->info_get)
+	if (!devlink || !devlink->ops->info_get)
 		goto unlock_list;
 
 	mutex_lock(&devlink->lock);
@@ -6458,7 +6454,7 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 
 	mutex_lock(&devlink_mutex);
 	devlink = netdev_to_devlink(dev);
-	if (!devlink || !devlink->ops || !devlink->ops->flash_update)
+	if (!devlink || !devlink->ops->flash_update)
 		goto unlock_list;
 
 	mutex_lock(&devlink->lock);
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
  2019-02-26  3:34 ` [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat Jakub Kicinski
@ 2019-02-26  4:26   ` Florian Fainelli
  2019-02-26  8:57   ` Jiri Pirko
  1 sibling, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2019-02-26  4:26 UTC (permalink / raw)
  To: Jakub Kicinski, davem, jiri; +Cc: mkubecek, andrew, netdev, oss-drivers



On 2/25/2019 7:34 PM, Jakub Kicinski wrote:
> When ethtool is calling into devlink compat code make sure we have
> a reference on the netdevice on which the operation was invoked.
> 
> v3: move the hold/lock logic into devlink_compat_* functions (Florian)
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
  2019-02-26  3:34 ` [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances Jakub Kicinski
@ 2019-02-26  4:29   ` Florian Fainelli
  2019-02-26  8:59   ` Jiri Pirko
  1 sibling, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2019-02-26  4:29 UTC (permalink / raw)
  To: Jakub Kicinski, davem, jiri; +Cc: mkubecek, andrew, netdev, oss-drivers



On 2/25/2019 7:34 PM, Jakub Kicinski wrote:
> Commit 76726ccb7f46 ("devlink: add flash update command") and
> commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
> access devlink ops without NULL-checking. There is, however, no
> driver which would pass in NULL ops, so let's just make that
> a requirement. Remove the now unnecessary NULL-checking.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance
  2019-02-26  3:34 ` [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance Jakub Kicinski
@ 2019-02-26  8:56   ` Jiri Pirko
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2019-02-26  8:56 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

Tue, Feb 26, 2019 at 04:34:03AM CET, jakub.kicinski@netronome.com wrote:
>Instead of iterating over all devlink ports add a NDO which
>will return the devlink instance from the driver.
>
>v2: add the netdev_to_devlink() helper (Michal)
>v3: check that devlink has ops (Florian)
>v4: hold devlink_mutex (Jiri)
>
>Suggested-by: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
  2019-02-26  3:34 ` [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat Jakub Kicinski
  2019-02-26  4:26   ` Florian Fainelli
@ 2019-02-26  8:57   ` Jiri Pirko
  1 sibling, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2019-02-26  8:57 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

Tue, Feb 26, 2019 at 04:34:06AM CET, jakub.kicinski@netronome.com wrote:
>When ethtool is calling into devlink compat code make sure we have
>a reference on the netdevice on which the operation was invoked.
>
>v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
  2019-02-26  3:34 ` [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances Jakub Kicinski
  2019-02-26  4:29   ` Florian Fainelli
@ 2019-02-26  8:59   ` Jiri Pirko
  1 sibling, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2019-02-26  8:59 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

Tue, Feb 26, 2019 at 04:34:07AM CET, jakub.kicinski@netronome.com wrote:
>Commit 76726ccb7f46 ("devlink: add flash update command") and
>commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
>access devlink ops without NULL-checking. There is, however, no
>driver which would pass in NULL ops, so let's just make that
>a requirement. Remove the now unnecessary NULL-checking.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Thanks Kuba!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
  2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
                   ` (5 preceding siblings ...)
  2019-02-26  3:34 ` [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances Jakub Kicinski
@ 2019-02-26 16:50 ` David Miller
  6 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2019-02-26 16:50 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: jiri, mkubecek, andrew, f.fainelli, netdev, oss-drivers

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 25 Feb 2019 19:34:01 -0800

> This is a follow up to the series which added device flash
> updates via devlink. I went with the approach of adding a
> new NDO in the end. It seems to end up looking cleaner.
> 
> First patch removes the option to build devlink as a module.
> Users can still decide to not build it, but the module option
> ends up not being worth the maintenance cost.
> 
> Next two patches add a NDO which can be used to ask the driver
> to return a devlink instance associated with a given netdev,
> instead of iterating over devlink ports. Drivers which implement
> this NDO must take into account the potential impact on the
> visibility of the devlink instance.
> 
> With the new NDO in place we can remove NFP ethtool flash update
> code.
> 
> Fifth patch makes sure we hold a reference to dev while
> callbacks are active.
> 
> Last but not least the NULL-check of devlink->ops is moved
> to instance allocation time.
> 
> Last but not least missing checks for devlink->ops are added.
> There is currently no driver registering devlink without ops,
> so can just fix this in -next.
> 
> v2 (Michal): add netdev_to_devlink() in patch 3.
> v3 (Florian):
>  - add missing checks for devlink->ops;
>  - move locking/holding into devlink_compat_ functions.
> v4 (Jiri):
>  - hold devlink_mutex around callbacks (patch 2);
>  - require non-NULL ops (patch 6).

Series applied, thanks Jakub.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
  2019-02-26  3:34 ` [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in Jakub Kicinski
@ 2019-02-28 15:01   ` Jiri Pirko
  2019-02-28 16:02     ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2019-02-28 15:01 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

Tue, Feb 26, 2019 at 04:34:02AM CET, jakub.kicinski@netronome.com wrote:

[...]


>diff --git a/net/Kconfig b/net/Kconfig
>index 62da6148e9f8..1efe1f9ee492 100644
>--- a/net/Kconfig
>+++ b/net/Kconfig
>@@ -429,21 +429,12 @@ config NET_SOCK_MSG
> 	  with the help of BPF programs.
> 
> config NET_DEVLINK
>-	tristate "Network physical/parent device Netlink interface"
>+	bool "Network physical/parent device Netlink interface"

Wouldn't "default y" be better?


> 	help
> 	  Network physical/parent device Netlink interface provides
> 	  infrastructure to support access to physical chip-wide config and
> 	  monitoring.
> 

[...]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
  2019-02-28 15:01   ` Jiri Pirko
@ 2019-02-28 16:02     ` Jakub Kicinski
  2019-03-01  7:23       ` Jiri Pirko
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2019-02-28 16:02 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

On Thu, 28 Feb 2019 16:01:25 +0100, Jiri Pirko wrote:
> Tue, Feb 26, 2019 at 04:34:02AM CET, jakub.kicinski@netronome.com wrote:
> >diff --git a/net/Kconfig b/net/Kconfig
> >index 62da6148e9f8..1efe1f9ee492 100644
> >--- a/net/Kconfig
> >+++ b/net/Kconfig
> >@@ -429,21 +429,12 @@ config NET_SOCK_MSG
> > 	  with the help of BPF programs.
> > 
> > config NET_DEVLINK
> >-	tristate "Network physical/parent device Netlink interface"
> >+	bool "Network physical/parent device Netlink interface"  
> 
> Wouldn't "default y" be better?

No opinion.  It used to be 'n', it didn't change here..  But we can
change if we write convincing enough commit message :)

Did you see cases where people "forget" to enable it?

> > 	help
> > 	  Network physical/parent device Netlink interface provides
> > 	  infrastructure to support access to physical chip-wide config and
> > 	  monitoring.
> >   



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
  2019-02-28 16:02     ` Jakub Kicinski
@ 2019-03-01  7:23       ` Jiri Pirko
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Pirko @ 2019-03-01  7:23 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers

Thu, Feb 28, 2019 at 05:02:56PM CET, jakub.kicinski@netronome.com wrote:
>On Thu, 28 Feb 2019 16:01:25 +0100, Jiri Pirko wrote:
>> Tue, Feb 26, 2019 at 04:34:02AM CET, jakub.kicinski@netronome.com wrote:
>> >diff --git a/net/Kconfig b/net/Kconfig
>> >index 62da6148e9f8..1efe1f9ee492 100644
>> >--- a/net/Kconfig
>> >+++ b/net/Kconfig
>> >@@ -429,21 +429,12 @@ config NET_SOCK_MSG
>> > 	  with the help of BPF programs.
>> > 
>> > config NET_DEVLINK
>> >-	tristate "Network physical/parent device Netlink interface"
>> >+	bool "Network physical/parent device Netlink interface"  
>> 
>> Wouldn't "default y" be better?
>
>No opinion.  It used to be 'n', it didn't change here..  But we can
>change if we write convincing enough commit message :)
>
>Did you see cases where people "forget" to enable it?

I just did :) But I believe that is what's going to happen in the future.

>
>> > 	help
>> > 	  Network physical/parent device Netlink interface provides
>> > 	  infrastructure to support access to physical chip-wide config and
>> > 	  monitoring.
>> >   
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-03-01  7:33 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26  3:34 [PATCH net-next v4 0/6] devlink: make ethtool compat reliable Jakub Kicinski
2019-02-26  3:34 ` [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in Jakub Kicinski
2019-02-28 15:01   ` Jiri Pirko
2019-02-28 16:02     ` Jakub Kicinski
2019-03-01  7:23       ` Jiri Pirko
2019-02-26  3:34 ` [PATCH net-next v4 2/6] devlink: create a special NDO for getting the devlink instance Jakub Kicinski
2019-02-26  8:56   ` Jiri Pirko
2019-02-26  3:34 ` [PATCH net-next v4 3/6] nfp: add .ndo_get_devlink Jakub Kicinski
2019-02-26  3:34 ` [PATCH net-next v4 4/6] nfp: remove ethtool flashing fallback Jakub Kicinski
2019-02-26  3:34 ` [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat Jakub Kicinski
2019-02-26  4:26   ` Florian Fainelli
2019-02-26  8:57   ` Jiri Pirko
2019-02-26  3:34 ` [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances Jakub Kicinski
2019-02-26  4:29   ` Florian Fainelli
2019-02-26  8:59   ` Jiri Pirko
2019-02-26 16:50 ` [PATCH net-next v4 0/6] devlink: make ethtool compat reliable David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.