lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190326133558.40d5ff0e@cakuba.netronome.com>
Date:   Tue, 26 Mar 2019 13:35:58 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        idosch@...lanox.com, f.fainelli@...il.com, andrew@...n.ch,
        vivien.didelot@...il.com, michael.chan@...adcom.com
Subject: Re: [patch net-next v2 11/12] net: devlink: expose phys port name

On Tue, 26 Mar 2019 13:03:06 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...lanox.com>
> 
> Currently, it is exposed via rtnetlink. But it is relevant to devlink
> ports, so expose it here too.
> 
> Signed-off-by: Jiri Pirko <jiri@...lanox.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,

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ