[patch net-next 0/5] net: call for phys_port_name into devlink directly is possible

[patch net-next 0/5] net: call for phys_port_name into devlink directly is possible

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

phys_port_name may be assembled by a helper in devlink. It is currently
the case only for mlxsw driver. Benefit from the get_devlink_port ndo
and call into devlink directly from dev_get_phys_port_name(). That saves
the trip to the driver, simplifies the code and makes it similar to
recently introduced ethtool-devlink compat helpers.

Jiri Pirko (5):
  net: replace ndo_get_devlink for ndo_get_devlink_port
  net: devlink: introduce devlink_compat_phys_port_name_get()
  mlxsw: spectrum: Implement ndo_get_devlink_port
  mlxsw: Remove ndo_get_phys_port_name implementation
  net: devlink: remove unused devlink_port_get_phys_port_name() function

 drivers/net/ethernet/mellanox/mlxsw/core.c         | 10 ++++---
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  5 ++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 22 +++++++-------
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c     | 11 -------
 drivers/net/ethernet/netronome/nfp/nfp_app.h       |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c   | 10 +++----
 .../net/ethernet/netronome/nfp/nfp_net_common.c    |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c  |  2 +-
 include/linux/netdevice.h                          |  6 ++--
 include/net/devlink.h                              | 34 ++++++++++++++--------
 net/core/dev.c                                     |  3 +-
 net/core/devlink.c                                 | 25 ++++++++++++++--
 12 files changed, 77 insertions(+), 55 deletions(-)

-- 
2.14.5

[patch net-next 1/5] net: replace ndo_get_devlink for ndo_get_devlink_port

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Follow-up patch is going to need a devlink port instance according to
a netdev. Devlink port instance should be always available when devlink
is used. So change the recently introduced ndo_get_devlink to
ndo_get_devlink_port. With that, adjust the wrapper for the only
user to get devlink pointer.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_app.h        |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_devlink.c    | 10 +++++-----
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  2 +-
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c   |  2 +-
 include/linux/netdevice.h                           |  6 +++---
 include/net/devlink.h                               | 16 +++++++++++++---
 6 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index f8d422713705..a6fda07fce43 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -433,6 +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);
+struct devlink_port *nfp_devlink_get_devlink_port(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 e9eca99cf493..f24a7fc11b9f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -377,13 +377,13 @@ 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 devlink_port *nfp_devlink_get_devlink_port(struct net_device *netdev)
 {
-	struct nfp_app *app;
+	struct nfp_port *port;
 
-	app = nfp_app_from_netdev(netdev);
-	if (!app)
+	port = nfp_port_from_netdev(netdev);
+	if (!port)
 		return NULL;
 
-	return priv_to_devlink(app->pf);
+	return &port->dl_port;
 }
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 6d1b8816552e..7dd6b3fbb394 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3531,7 +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,
+	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
 };
 
 /**
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index d2c803bb4e56..bf621674f583 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -273,7 +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,
+	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
 };
 
 void
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c10b60297d28..dc9d72b8932a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1251,8 +1251,8 @@ struct devlink;
  *	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.
+ * struct devlink_port *(*ndo_get_devlink_port)(struct net_device *dev);
+ *	Get devlink port instance associated with a given netdev.
  *	Called with a reference on the netdevice and devlink locks only,
  *	rtnl_lock is not held.
  */
@@ -1453,7 +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);
+	struct devlink_port *	(*ndo_get_devlink_port)(struct net_device *dev);
 };
 
 /**
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 7f5a0bdca228..f34107def48e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -538,15 +538,25 @@ 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)
+static inline struct devlink_port *
+netdev_to_devlink_port(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);
+	if (dev->netdev_ops->ndo_get_devlink_port)
+		return dev->netdev_ops->ndo_get_devlink_port(dev);
 #endif
 	return NULL;
 }
 
+static inline struct devlink *netdev_to_devlink(struct net_device *dev)
+{
+	struct devlink_port *devlink_port = netdev_to_devlink_port(dev);
+
+	if (devlink_port)
+		return devlink_port->devlink;
+	return NULL;
+}
+
 struct ib_device;
 
 #if IS_ENABLED(CONFIG_NET_DEVLINK)
-- 
2.14.5

Re: [patch net-next 1/5] net: replace ndo_get_devlink for ndo_get_devlink_port

[Michal Kubecek]

On Fri, Mar 01, 2019 at 09:13:58AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Follow-up patch is going to need a devlink port instance according to
> a netdev. Devlink port instance should be always available when devlink
> is used.

I noticed netdevsim has devlink interface but it does not register
devlink ports for its network devices. But that seems to be rather an
omission in netdevsim, right?

>          So change the recently introduced ndo_get_devlink to
> ndo_get_devlink_port. With that, adjust the wrapper for the only
> user to get devlink pointer.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

> ---
>  drivers/net/ethernet/netronome/nfp/nfp_app.h        |  2 +-
>  drivers/net/ethernet/netronome/nfp/nfp_devlink.c    | 10 +++++-----
>  drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  2 +-
>  drivers/net/ethernet/netronome/nfp/nfp_net_repr.c   |  2 +-
>  include/linux/netdevice.h                           |  6 +++---
>  include/net/devlink.h                               | 16 +++++++++++++---
>  6 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> index f8d422713705..a6fda07fce43 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> @@ -433,6 +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);
> +struct devlink_port *nfp_devlink_get_devlink_port(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 e9eca99cf493..f24a7fc11b9f 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> @@ -377,13 +377,13 @@ 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 devlink_port *nfp_devlink_get_devlink_port(struct net_device *netdev)
>  {
> -	struct nfp_app *app;
> +	struct nfp_port *port;
>  
> -	app = nfp_app_from_netdev(netdev);
> -	if (!app)
> +	port = nfp_port_from_netdev(netdev);
> +	if (!port)
>  		return NULL;
>  
> -	return priv_to_devlink(app->pf);
> +	return &port->dl_port;
>  }
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> index 6d1b8816552e..7dd6b3fbb394 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> @@ -3531,7 +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,
> +	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
>  };
>  
>  /**
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
> index d2c803bb4e56..bf621674f583 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
> @@ -273,7 +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,
> +	.ndo_get_devlink_port	= nfp_devlink_get_devlink_port,
>  };
>  
>  void
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c10b60297d28..dc9d72b8932a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1251,8 +1251,8 @@ struct devlink;
>   *	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.
> + * struct devlink_port *(*ndo_get_devlink_port)(struct net_device *dev);
> + *	Get devlink port instance associated with a given netdev.
>   *	Called with a reference on the netdevice and devlink locks only,
>   *	rtnl_lock is not held.
>   */
> @@ -1453,7 +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);
> +	struct devlink_port *	(*ndo_get_devlink_port)(struct net_device *dev);
>  };
>  
>  /**
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 7f5a0bdca228..f34107def48e 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -538,15 +538,25 @@ 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)
> +static inline struct devlink_port *
> +netdev_to_devlink_port(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);
> +	if (dev->netdev_ops->ndo_get_devlink_port)
> +		return dev->netdev_ops->ndo_get_devlink_port(dev);
>  #endif
>  	return NULL;
>  }
>  
> +static inline struct devlink *netdev_to_devlink(struct net_device *dev)
> +{
> +	struct devlink_port *devlink_port = netdev_to_devlink_port(dev);
> +
> +	if (devlink_port)
> +		return devlink_port->devlink;
> +	return NULL;
> +}
> +
>  struct ib_device;
>  
>  #if IS_ENABLED(CONFIG_NET_DEVLINK)
> -- 
> 2.14.5
> 

Re: [patch net-next 1/5] net: replace ndo_get_devlink for ndo_get_devlink_port

[Jiri Pirko]

Fri, Mar 01, 2019 at 01:45:26PM CET, mkubecek@suse.cz wrote:
>On Fri, Mar 01, 2019 at 09:13:58AM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Follow-up patch is going to need a devlink port instance according to
>> a netdev. Devlink port instance should be always available when devlink
>> is used.
>
>I noticed netdevsim has devlink interface but it does not register
>devlink ports for its network devices. But that seems to be rather an
>omission in netdevsim, right?

Yep.

>
>>          So change the recently introduced ndo_get_devlink to
>> ndo_get_devlink_port. With that, adjust the wrapper for the only
>> user to get devlink pointer.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

Thanks.

Re: [patch net-next 1/5] net: replace ndo_get_devlink for ndo_get_devlink_port

[Florian Fainelli]

On 3/1/2019 12:13 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Follow-up patch is going to need a devlink port instance according to
> a netdev. Devlink port instance should be always available when devlink
> is used. So change the recently introduced ndo_get_devlink to
> ndo_get_devlink_port. With that, adjust the wrapper for the only
> user to get devlink pointer.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

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

[patch net-next 2/5] net: devlink: introduce devlink_compat_phys_port_name_get()

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduce devlink_compat_phys_port_name_get() helper that
gets the physical port name for specified netdevice
according to devlink port attributes.
Call this helper from dev_get_phys_port_name()
in case ndo_get_phys_port_name is not defined.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h |  9 +++++++++
 net/core/dev.c        |  3 ++-
 net/core/devlink.c    | 30 ++++++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index f34107def48e..ae2cd34da99e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -729,6 +729,8 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
 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);
+int devlink_compat_phys_port_name_get(struct net_device *dev,
+				      char *name, size_t len);
 
 #else
 
@@ -1224,6 +1226,13 @@ devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int
+devlink_compat_phys_port_name_get(struct net_device *dev,
+				  char *name, size_t len)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b67f2aa59dd..a614b26b0842 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -146,6 +146,7 @@
 #include <net/udp_tunnel.h>
 #include <linux/net_namespace.h>
 #include <linux/indirect_call_wrapper.h>
+#include <net/devlink.h>
 
 #include "net-sysfs.h"
 
@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
 	const struct net_device_ops *ops = dev->netdev_ops;
 
 	if (!ops->ndo_get_phys_port_name)
-		return -EOPNOTSUPP;
+		return devlink_compat_phys_port_name_get(dev, name, len);
 	return ops->ndo_get_phys_port_name(dev, name, len);
 }
 EXPORT_SYMBOL(dev_get_phys_port_name);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index a49dee67e66f..4fd45d4a4818 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5419,8 +5419,8 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
 
-int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
-				    char *name, size_t len)
+static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
+					     char *name, size_t len)
 {
 	struct devlink_port_attrs *attrs = &devlink_port->attrs;
 	int n = 0;
@@ -5450,6 +5450,12 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
 
 	return 0;
 }
+
+int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
+				    char *name, size_t len)
+{
+	return __devlink_port_phys_port_name_get(devlink_port, name, len);
+}
 EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
 
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
@@ -6469,6 +6475,26 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 	return ret;
 }
 
+int devlink_compat_phys_port_name_get(struct net_device *dev,
+				      char *name, size_t len)
+{
+	struct devlink_port *devlink_port;
+	int err = -EOPNOTSUPP;
+
+	mutex_lock(&devlink_mutex);
+	devlink_port = netdev_to_devlink_port(dev);
+	if (!devlink_port)
+		goto unlock_list;
+
+	mutex_lock(&devlink_port->devlink->lock);
+	err = __devlink_port_phys_port_name_get(devlink_port, name, len);
+	mutex_unlock(&devlink_port->devlink->lock);
+unlock_list:
+	mutex_unlock(&devlink_mutex);
+
+	return err;
+}
+
 static int __init devlink_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
-- 
2.14.5

Re: [patch net-next 2/5] net: devlink: introduce devlink_compat_phys_port_name_get()

[Jakub Kicinski]

On Fri,  1 Mar 2019 09:13:59 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Introduce devlink_compat_phys_port_name_get() helper that
> gets the physical port name for specified netdevice
> according to devlink port attributes.
> Call this helper from dev_get_phys_port_name()
> in case ndo_get_phys_port_name is not defined.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/devlink.h |  9 +++++++++
>  net/core/dev.c        |  3 ++-
>  net/core/devlink.c    | 30 ++++++++++++++++++++++++++++--
>  3 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index f34107def48e..ae2cd34da99e 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -729,6 +729,8 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
>  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);
> +int devlink_compat_phys_port_name_get(struct net_device *dev,
> +				      char *name, size_t len);
>  
>  #else
>  
> @@ -1224,6 +1226,13 @@ devlink_compat_flash_update(struct net_device *dev, const char *file_name)
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static inline int
> +devlink_compat_phys_port_name_get(struct net_device *dev,
> +				  char *name, size_t len)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif
>  
>  #endif /* _NET_DEVLINK_H_ */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2b67f2aa59dd..a614b26b0842 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -146,6 +146,7 @@
>  #include <net/udp_tunnel.h>
>  #include <linux/net_namespace.h>
>  #include <linux/indirect_call_wrapper.h>
> +#include <net/devlink.h>
>  
>  #include "net-sysfs.h"
>  
> @@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
>  	const struct net_device_ops *ops = dev->netdev_ops;
>  
>  	if (!ops->ndo_get_phys_port_name)
> -		return -EOPNOTSUPP;
> +		return devlink_compat_phys_port_name_get(dev, name, len);
>  	return ops->ndo_get_phys_port_name(dev, name, len);
>  }
>  EXPORT_SYMBOL(dev_get_phys_port_name);
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index a49dee67e66f..4fd45d4a4818 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -5419,8 +5419,8 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
>  }
>  EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
>  
> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> -				    char *name, size_t len)
> +static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
> +					     char *name, size_t len)
>  {
>  	struct devlink_port_attrs *attrs = &devlink_port->attrs;
>  	int n = 0;
> @@ -5450,6 +5450,12 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>  
>  	return 0;
>  }
> +
> +int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> +				    char *name, size_t len)
> +{
> +	return __devlink_port_phys_port_name_get(devlink_port, name, len);
> +}
>  EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
>  
>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
> @@ -6469,6 +6475,26 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
>  	return ret;
>  }
>  
> +int devlink_compat_phys_port_name_get(struct net_device *dev,
> +				      char *name, size_t len)
> +{
> +	struct devlink_port *devlink_port;
> +	int err = -EOPNOTSUPP;
> +
> +	mutex_lock(&devlink_mutex);
> +	devlink_port = netdev_to_devlink_port(dev);
> +	if (!devlink_port)
> +		goto unlock_list;
> +
> +	mutex_lock(&devlink_port->devlink->lock);

I'm moderately confident you won't be able to take devlink locks from
RTNL code :(  I think the locking order is:

  devlink_mutex -> devlink->lock -> rtnl_lock

No? :(  At least NFP assumes that, but I think port splitting does, too.

> +	err = __devlink_port_phys_port_name_get(devlink_port, name, len);
> +	mutex_unlock(&devlink_port->devlink->lock);
> +unlock_list:
> +	mutex_unlock(&devlink_mutex);
> +
> +	return err;
> +}
> +
>  static int __init devlink_init(void)
>  {
>  	return genl_register_family(&devlink_nl_family);

Re: [patch net-next 2/5] net: devlink: introduce devlink_compat_phys_port_name_get()

[Jiri Pirko]

Fri, Mar 01, 2019 at 05:14:58PM CET, jakub.kicinski@netronome.com wrote:
>On Fri,  1 Mar 2019 09:13:59 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Introduce devlink_compat_phys_port_name_get() helper that
>> gets the physical port name for specified netdevice
>> according to devlink port attributes.
>> Call this helper from dev_get_phys_port_name()
>> in case ndo_get_phys_port_name is not defined.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>  include/net/devlink.h |  9 +++++++++
>>  net/core/dev.c        |  3 ++-
>>  net/core/devlink.c    | 30 ++++++++++++++++++++++++++++--
>>  3 files changed, 39 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index f34107def48e..ae2cd34da99e 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -729,6 +729,8 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
>>  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);
>> +int devlink_compat_phys_port_name_get(struct net_device *dev,
>> +				      char *name, size_t len);
>>  
>>  #else
>>  
>> @@ -1224,6 +1226,13 @@ devlink_compat_flash_update(struct net_device *dev, const char *file_name)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> +
>> +static inline int
>> +devlink_compat_phys_port_name_get(struct net_device *dev,
>> +				  char *name, size_t len)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>>  #endif
>>  
>>  #endif /* _NET_DEVLINK_H_ */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 2b67f2aa59dd..a614b26b0842 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -146,6 +146,7 @@
>>  #include <net/udp_tunnel.h>
>>  #include <linux/net_namespace.h>
>>  #include <linux/indirect_call_wrapper.h>
>> +#include <net/devlink.h>
>>  
>>  #include "net-sysfs.h"
>>  
>> @@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
>>  	const struct net_device_ops *ops = dev->netdev_ops;
>>  
>>  	if (!ops->ndo_get_phys_port_name)
>> -		return -EOPNOTSUPP;
>> +		return devlink_compat_phys_port_name_get(dev, name, len);
>>  	return ops->ndo_get_phys_port_name(dev, name, len);
>>  }
>>  EXPORT_SYMBOL(dev_get_phys_port_name);
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index a49dee67e66f..4fd45d4a4818 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -5419,8 +5419,8 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
>>  }
>>  EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
>>  
>> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>> -				    char *name, size_t len)
>> +static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
>> +					     char *name, size_t len)
>>  {
>>  	struct devlink_port_attrs *attrs = &devlink_port->attrs;
>>  	int n = 0;
>> @@ -5450,6 +5450,12 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>>  
>>  	return 0;
>>  }
>> +
>> +int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>> +				    char *name, size_t len)
>> +{
>> +	return __devlink_port_phys_port_name_get(devlink_port, name, len);
>> +}
>>  EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
>>  
>>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>> @@ -6469,6 +6475,26 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
>>  	return ret;
>>  }
>>  
>> +int devlink_compat_phys_port_name_get(struct net_device *dev,
>> +				      char *name, size_t len)
>> +{
>> +	struct devlink_port *devlink_port;
>> +	int err = -EOPNOTSUPP;
>> +
>> +	mutex_lock(&devlink_mutex);
>> +	devlink_port = netdev_to_devlink_port(dev);
>> +	if (!devlink_port)
>> +		goto unlock_list;
>> +
>> +	mutex_lock(&devlink_port->devlink->lock);
>
>I'm moderately confident you won't be able to take devlink locks from
>RTNL code :(  I think the locking order is:
>
>  devlink_mutex -> devlink->lock -> rtnl_lock
>
>No? :(  At least NFP assumes that, but I think port splitting does, too.

Oh right, thanks for spotting this. Will fix.


>
>> +	err = __devlink_port_phys_port_name_get(devlink_port, name, len);
>> +	mutex_unlock(&devlink_port->devlink->lock);
>> +unlock_list:
>> +	mutex_unlock(&devlink_mutex);
>> +
>> +	return err;
>> +}
>> +
>>  static int __init devlink_init(void)
>>  {
>>  	return genl_register_family(&devlink_nl_family);
>

[patch net-next 3/5] mlxsw: spectrum: Implement ndo_get_devlink_port

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

In order for devlink compat functions to work, implement
ndo_get_devlink_port.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 12 ++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h     |  3 +++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 11 +++++++++++
 3 files changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index b505d3858235..5d656310434d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1807,6 +1807,18 @@ int mlxsw_core_port_get_phys_port_name(struct mlxsw_core *mlxsw_core,
 }
 EXPORT_SYMBOL(mlxsw_core_port_get_phys_port_name);
 
+struct devlink_port *
+mlxsw_core_port_devlink_port_get(struct mlxsw_core *mlxsw_core,
+				 u8 local_port)
+{
+	struct mlxsw_core_port *mlxsw_core_port =
+					&mlxsw_core->ports[local_port];
+	struct devlink_port *devlink_port = &mlxsw_core_port->devlink_port;
+
+	return devlink_port;
+}
+EXPORT_SYMBOL(mlxsw_core_port_devlink_port_get);
+
 static void mlxsw_core_buf_dump_dbg(struct mlxsw_core *mlxsw_core,
 				    const char *buf, size_t size)
 {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index cd0c6aa0dff9..0592cd724ec3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -178,6 +178,9 @@ enum devlink_port_type mlxsw_core_port_type_get(struct mlxsw_core *mlxsw_core,
 						u8 local_port);
 int mlxsw_core_port_get_phys_port_name(struct mlxsw_core *mlxsw_core,
 				       u8 local_port, char *name, size_t len);
+struct devlink_port *
+mlxsw_core_port_devlink_port_get(struct mlxsw_core *mlxsw_core,
+				 u8 local_port);
 
 int mlxsw_core_schedule_dw(struct delayed_work *dwork, unsigned long delay);
 bool mlxsw_core_schedule_work(struct work_struct *work);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 6c797e322be8..7995e7b042a2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1725,6 +1725,16 @@ static int mlxsw_sp_port_get_port_parent_id(struct net_device *dev,
 	return 0;
 }
 
+static struct devlink_port *
+mlxsw_sp_port_get_devlink_port(struct net_device *dev)
+{
+	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
+	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+
+	return mlxsw_core_port_devlink_port_get(mlxsw_sp->core,
+						mlxsw_sp_port->local_port);
+}
+
 static const struct net_device_ops mlxsw_sp_port_netdev_ops = {
 	.ndo_open		= mlxsw_sp_port_open,
 	.ndo_stop		= mlxsw_sp_port_stop,
@@ -1741,6 +1751,7 @@ static const struct net_device_ops mlxsw_sp_port_netdev_ops = {
 	.ndo_get_phys_port_name	= mlxsw_sp_port_get_phys_port_name,
 	.ndo_set_features	= mlxsw_sp_set_features,
 	.ndo_get_port_parent_id	= mlxsw_sp_port_get_port_parent_id,
+	.ndo_get_devlink_port	= mlxsw_sp_port_get_devlink_port,
 };
 
 static void mlxsw_sp_port_get_drvinfo(struct net_device *dev,
-- 
2.14.5

[patch net-next 4/5] mlxsw: Remove ndo_get_phys_port_name implementation

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Rely on the previously introduce fallback and let the core directly call
devlink in order to get the physical port name.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c     | 10 ----------
 drivers/net/ethernet/mellanox/mlxsw/core.h     |  2 --
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 11 -----------
 drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 11 -----------
 4 files changed, 34 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 5d656310434d..d10108e44438 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1796,16 +1796,6 @@ enum devlink_port_type mlxsw_core_port_type_get(struct mlxsw_core *mlxsw_core,
 }
 EXPORT_SYMBOL(mlxsw_core_port_type_get);
 
-int mlxsw_core_port_get_phys_port_name(struct mlxsw_core *mlxsw_core,
-				       u8 local_port, char *name, size_t len)
-{
-	struct mlxsw_core_port *mlxsw_core_port =
-					&mlxsw_core->ports[local_port];
-	struct devlink_port *devlink_port = &mlxsw_core_port->devlink_port;
-
-	return devlink_port_get_phys_port_name(devlink_port, name, len);
-}
-EXPORT_SYMBOL(mlxsw_core_port_get_phys_port_name);
 
 struct devlink_port *
 mlxsw_core_port_devlink_port_get(struct mlxsw_core *mlxsw_core,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 0592cd724ec3..60b017a07cea 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -176,8 +176,6 @@ void mlxsw_core_port_clear(struct mlxsw_core *mlxsw_core, u8 local_port,
 			   void *port_driver_priv);
 enum devlink_port_type mlxsw_core_port_type_get(struct mlxsw_core *mlxsw_core,
 						u8 local_port);
-int mlxsw_core_port_get_phys_port_name(struct mlxsw_core *mlxsw_core,
-				       u8 local_port, char *name, size_t len);
 struct devlink_port *
 mlxsw_core_port_devlink_port_get(struct mlxsw_core *mlxsw_core,
 				 u8 local_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7995e7b042a2..5d0adec84858 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1253,16 +1253,6 @@ static int mlxsw_sp_port_kill_vid(struct net_device *dev,
 	return 0;
 }
 
-static int mlxsw_sp_port_get_phys_port_name(struct net_device *dev, char *name,
-					    size_t len)
-{
-	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
-
-	return mlxsw_core_port_get_phys_port_name(mlxsw_sp_port->mlxsw_sp->core,
-						  mlxsw_sp_port->local_port,
-						  name, len);
-}
-
 static struct mlxsw_sp_port_mall_tc_entry *
 mlxsw_sp_port_mall_tc_entry_find(struct mlxsw_sp_port *port,
 				 unsigned long cookie) {
@@ -1748,7 +1738,6 @@ static const struct net_device_ops mlxsw_sp_port_netdev_ops = {
 	.ndo_get_offload_stats	= mlxsw_sp_port_get_offload_stats,
 	.ndo_vlan_rx_add_vid	= mlxsw_sp_port_add_vid,
 	.ndo_vlan_rx_kill_vid	= mlxsw_sp_port_kill_vid,
-	.ndo_get_phys_port_name	= mlxsw_sp_port_get_phys_port_name,
 	.ndo_set_features	= mlxsw_sp_set_features,
 	.ndo_get_port_parent_id	= mlxsw_sp_port_get_port_parent_id,
 	.ndo_get_devlink_port	= mlxsw_sp_port_get_devlink_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 533fe6235b7c..9999d866f394 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -379,16 +379,6 @@ mlxsw_sx_port_get_stats64(struct net_device *dev,
 	stats->tx_dropped	= tx_dropped;
 }
 
-static int mlxsw_sx_port_get_phys_port_name(struct net_device *dev, char *name,
-					    size_t len)
-{
-	struct mlxsw_sx_port *mlxsw_sx_port = netdev_priv(dev);
-
-	return mlxsw_core_port_get_phys_port_name(mlxsw_sx_port->mlxsw_sx->core,
-						  mlxsw_sx_port->local_port,
-						  name, len);
-}
-
 static int mlxsw_sx_port_get_port_parent_id(struct net_device *dev,
 					    struct netdev_phys_item_id *ppid)
 {
@@ -407,7 +397,6 @@ static const struct net_device_ops mlxsw_sx_port_netdev_ops = {
 	.ndo_start_xmit		= mlxsw_sx_port_xmit,
 	.ndo_change_mtu		= mlxsw_sx_port_change_mtu,
 	.ndo_get_stats64	= mlxsw_sx_port_get_stats64,
-	.ndo_get_phys_port_name = mlxsw_sx_port_get_phys_port_name,
 	.ndo_get_port_parent_id	= mlxsw_sx_port_get_port_parent_id,
 };
 
-- 
2.14.5

[patch net-next 5/5] net: devlink: remove unused devlink_port_get_phys_port_name() function

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Now it is unused, remove it.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h | 9 ---------
 net/core/devlink.c    | 7 -------
 2 files changed, 16 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index ae2cd34da99e..227c453cc20e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    enum devlink_port_flavour flavour,
 			    u32 port_number, bool split,
 			    u32 split_subport_number);
-int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
-				    char *name, size_t len);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
@@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
 {
 }
 
-static inline int
-devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
-				char *name, size_t len)
-{
-	return -EOPNOTSUPP;
-}
-
 static inline int devlink_sb_register(struct devlink *devlink,
 				      unsigned int sb_index, u32 size,
 				      u16 ingress_pools_count,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4fd45d4a4818..d90a745d8258 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 	return 0;
 }
 
-int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
-				    char *name, size_t len)
-{
-	return __devlink_port_phys_port_name_get(devlink_port, name, len);
-}
-EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
-
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
-- 
2.14.5

Re: [patch net-next 5/5] net: devlink: remove unused devlink_port_get_phys_port_name() function

[Jakub Kicinski]

On Fri,  1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Now it is unused, remove it.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Oh, you nuke this completely, that's going to conflict hard with my
series :)

(Provided the locking is okay) I think it'd be good if we flipped the
logic in dev_get_phys_port_name().  Always try devlink:

@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
 	const struct net_device_ops *ops = dev->netdev_ops;
 	
+	if (!devlink_compat_phys_port_name_get(dev, name, len))
+		return 0;
 	if (!ops->ndo_get_phys_port_name)
		return -EOPNOTSUPP;
 	return ops->ndo_get_phys_port_name(dev, name, len);
 }

NFP is using a single set of NDOs for PFs and VFs (which don't have
devlink ports), so either it'd be good if I could call the devlink
helper for real ports, or we need the above.

> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index ae2cd34da99e..227c453cc20e 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
>  			    enum devlink_port_flavour flavour,
>  			    u32 port_number, bool split,
>  			    u32 split_subport_number);
> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> -				    char *name, size_t len);
>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>  			u32 size, u16 ingress_pools_count,
>  			u16 egress_pools_count, u16 ingress_tc_count,
> @@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
>  {
>  }
>  
> -static inline int
> -devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> -				char *name, size_t len)
> -{
> -	return -EOPNOTSUPP;
> -}
> -
>  static inline int devlink_sb_register(struct devlink *devlink,
>  				      unsigned int sb_index, u32 size,
>  				      u16 ingress_pools_count,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 4fd45d4a4818..d90a745d8258 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
>  	return 0;
>  }
>  
> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> -				    char *name, size_t len)
> -{
> -	return __devlink_port_phys_port_name_get(devlink_port, name, len);
> -}
> -EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
> -
>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>  			u32 size, u16 ingress_pools_count,
>  			u16 egress_pools_count, u16 ingress_tc_count,

Re: [patch net-next 5/5] net: devlink: remove unused devlink_port_get_phys_port_name() function

[Jiri Pirko]

Fri, Mar 01, 2019 at 05:25:29PM CET, jakub.kicinski@netronome.com wrote:
>On Fri,  1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Now it is unused, remove it.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>Oh, you nuke this completely, that's going to conflict hard with my
>series :)
>
>(Provided the locking is okay) I think it'd be good if we flipped the
>logic in dev_get_phys_port_name().  Always try devlink:
>
>@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
> 	const struct net_device_ops *ops = dev->netdev_ops;
> 	
>+	if (!devlink_compat_phys_port_name_get(dev, name, len))
>+		return 0;
> 	if (!ops->ndo_get_phys_port_name)
>		return -EOPNOTSUPP;
> 	return ops->ndo_get_phys_port_name(dev, name, len);
> }
>
>NFP is using a single set of NDOs for PFs and VFs (which don't have
>devlink ports), so either it'd be good if I could call the devlink
>helper for real ports, or we need the above.

That wouldn't give correct results now. Until you introduce PF/VF port
flavours, devlink_compat_phys_port_name_get() does not assemble correct
names for them. So we have to call ndo_get_phys_port_name until it knows
how to do that.


>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index ae2cd34da99e..227c453cc20e 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
>>  			    enum devlink_port_flavour flavour,
>>  			    u32 port_number, bool split,
>>  			    u32 split_subport_number);
>> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>> -				    char *name, size_t len);
>>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>>  			u32 size, u16 ingress_pools_count,
>>  			u16 egress_pools_count, u16 ingress_tc_count,
>> @@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
>>  {
>>  }
>>  
>> -static inline int
>> -devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>> -				char *name, size_t len)
>> -{
>> -	return -EOPNOTSUPP;
>> -}
>> -
>>  static inline int devlink_sb_register(struct devlink *devlink,
>>  				      unsigned int sb_index, u32 size,
>>  				      u16 ingress_pools_count,
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 4fd45d4a4818..d90a745d8258 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
>>  	return 0;
>>  }
>>  
>> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
>> -				    char *name, size_t len)
>> -{
>> -	return __devlink_port_phys_port_name_get(devlink_port, name, len);
>> -}
>> -EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name);
>> -
>>  int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>>  			u32 size, u16 ingress_pools_count,
>>  			u16 egress_pools_count, u16 ingress_tc_count,
>

Re: [patch net-next 5/5] net: devlink: remove unused devlink_port_get_phys_port_name() function

[Jakub Kicinski]

On Fri, 1 Mar 2019 17:27:20 +0100, Jiri Pirko wrote:
> Fri, Mar 01, 2019 at 05:25:29PM CET, jakub.kicinski@netronome.com wrote:
> >On Fri,  1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote:  
> >> From: Jiri Pirko <jiri@mellanox.com>
> >> 
> >> Now it is unused, remove it.
> >> 
> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>  
> >
> >Oh, you nuke this completely, that's going to conflict hard with my
> >series :)
> >
> >(Provided the locking is okay) I think it'd be good if we flipped the
> >logic in dev_get_phys_port_name().  Always try devlink:
> >
> >@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev,
> > 	const struct net_device_ops *ops = dev->netdev_ops;
> > 	
> >+	if (!devlink_compat_phys_port_name_get(dev, name, len))
> >+		return 0;
> > 	if (!ops->ndo_get_phys_port_name)
> >		return -EOPNOTSUPP;
> > 	return ops->ndo_get_phys_port_name(dev, name, len);
> > }
> >
> >NFP is using a single set of NDOs for PFs and VFs (which don't have
> >devlink ports), so either it'd be good if I could call the devlink
> >helper for real ports, or we need the above.  
> 
> That wouldn't give correct results now. Until you introduce PF/VF port
> flavours, devlink_compat_phys_port_name_get() does not assemble correct
> names for them. So we have to call ndo_get_phys_port_name until it knows
> how to do that.

Okay, yeah, this won't give correct results after my series though, so
please advise on which order you want these things merged?  I don't
think there were any changes or concerns with my series - one commit
message fix, split one helper into two, and wrap the peer into another
attr in the dump?  Can it go in first?