Wed, Mar 27, 2019 at 08:45:38AM CET, j...@resnulli.us wrote:
>Tue, Mar 26, 2019 at 09:35:58PM CET, jakub.kicin...@netronome.com wrote:
>>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
>
>Nope. It does has access only to flavour. Not to port number for
>example. So I thought either expose that or the generated name.
To correct myself, all is exposed by kernel, just not implemented in
userspace. I thought that generated port name would be good to have.
Not really good idea to generate it again in userspace. Anyway, I'll
leave this patch out for now, I can send it separatelly later on.
>
>>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,
>>
