On Tue, 26 Mar 2019 13:03:06 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <j...@mellanox.com>
>
> Currently, it is exposed via rtnetlink. But it is relevant to devlink
> ports, so expose it here too.
>
> Signed-off-by: Jiri Pirko <j...@mellanox.com>
Why? User space has access to all the components which are used to
generate the name in a typed, structured form. That's strictly
superior to the string netdev string.
Do you have some use in mind for the new attribute?
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 5bb4ea67d84f..2d0365e45141 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -332,6 +332,8 @@ enum devlink_attr {
> DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
> DEVLINK_ATTR_FLASH_UPDATE_COMPONENT, /* string */
>
> + DEVLINK_ATTR_PORT_PHYS_NAME, /* string */
> +
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 6bbd07e3861e..356d7ee7c404 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -508,6 +508,57 @@ static void devlink_notify(struct devlink *devlink, enum
> devlink_command cmd)
> msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
> }
>
> +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;
> +
> + if (!attrs->set)
> + return -EOPNOTSUPP;
> +
> + switch (attrs->flavour) {
> + case DEVLINK_PORT_FLAVOUR_PHYSICAL:
> + if (!attrs->split)
> + n = snprintf(name, len, "p%u", attrs->port_number);
> + else
> + n = snprintf(name, len, "p%us%u", attrs->port_number,
> + attrs->split_subport_number);
> + break;
> + case DEVLINK_PORT_FLAVOUR_CPU:
> + case DEVLINK_PORT_FLAVOUR_DSA:
> + /* As CPU and DSA ports do not have a netdevice associated
> + * case should not ever happen.
> + */
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + if (n >= len)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int devlink_nl_port_phys_port_name_put(struct sk_buff *msg,
> + struct devlink_port *devlink_port)
> +{
> + struct devlink_port_attrs *attrs = &devlink_port->attrs;
> + char phys_name[IFNAMSIZ];
> + int err;
> +
> + if (attrs->flavour != DEVLINK_PORT_FLAVOUR_PHYSICAL)
> + return 0;
> +
> + err = __devlink_port_phys_port_name_get(devlink_port,
> + phys_name, sizeof(phys_name));
> + if (err)
> + return err;
> + if (nla_put_string(msg, DEVLINK_ATTR_PORT_PHYS_NAME, phys_name))
> + return -EMSGSIZE;
> + return 0;
> +}
> +
> static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> struct devlink_port *devlink_port)
> {
> @@ -526,6 +577,9 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
> attrs->split_subport_number))
> return -EMSGSIZE;
> + if (devlink_nl_port_phys_port_name_put(msg, devlink_port))
> + return -EMSGSIZE;
> +
> return 0;
> }
>
> @@ -5414,38 +5468,6 @@ void devlink_port_attrs_set(struct devlink_port
> *devlink_port,
> }
> EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
>
> -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;
> -
> - if (!attrs->set)
> - return -EOPNOTSUPP;
> -
> - switch (attrs->flavour) {
> - case DEVLINK_PORT_FLAVOUR_PHYSICAL:
> - if (!attrs->split)
> - n = snprintf(name, len, "p%u", attrs->port_number);
> - else
> - n = snprintf(name, len, "p%us%u", attrs->port_number,
> - attrs->split_subport_number);
> - break;
> - case DEVLINK_PORT_FLAVOUR_CPU:
> - case DEVLINK_PORT_FLAVOUR_DSA:
> - /* As CPU and DSA ports do not have a netdevice associated
> - * case should not ever happen.
> - */
> - WARN_ON(1);
> - return -EINVAL;
> - }
> -
> - if (n >= len)
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> 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,
