Tue, Mar 26, 2019 at 09:35:58PM CET, [email protected] wrote:
>On Tue, 26 Mar 2019 13:03:06 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <[email protected]>
>>
>> Currently, it is exposed via rtnetlink. But it is relevant to devlink
>> ports, so expose it here too.
>>
>> Signed-off-by: Jiri Pirko <[email protected]>
>
>Why? User space has access to all the components which are used to
>generate the name in a typed, structured form. That's strictly
Nope. It does has access only to flavour. Not to port number for
example. So I thought either expose that or the generated name.
>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,
>
