Fri, Mar 01, 2019 at 05:14:58PM CET, jakub.kicin...@netronome.com wrote:
>On Fri, 1 Mar 2019 09:13:59 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <j...@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 <j...@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);
>
