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  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]
Date:	Thu, 18 Dec 2014 11:41:45 +0000
From:	Thomas Graf <tgraf@...g.ch>
To:	"Varlese, Marco" <marco.varlese@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Fastabend, John R" <john.r.fastabend@...el.com>,
	Jiri Pirko <jiri@...nulli.us>,
	"roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
	"sfeldma@...il.com" <sfeldma@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port
 configuration

On 12/18/14 at 11:29am, Varlese, Marco wrote:
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index f7d0d2d..19cb51a 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -146,6 +146,7 @@ enum {
>  	IFLA_PHYS_PORT_ID,
>  	IFLA_CARRIER_CHANGES,
>  	IFLA_PHYS_SWITCH_ID,
> +	IFLA_SWITCH_PORT_CFG,
>  	__IFLA_MAX
>  };

Needs an entry in ifla_policy[]

        [IFLA_SWITCH_PORT_CFG] = { .type = NLA_NESTED },

> @@ -603,4 +604,18 @@ enum {
>  
>  #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)
>  
> +/* Switch Port Attributes section */
> +
> +enum {
> +	IFLA_ATTR_UNSPEC,
> +	IFLA_ATTR_LEARNING,
> +	IFLA_ATTR_LOOPBACK,
> +	IFLA_ATTR_BCAST_FLOODING,
> +	IFLA_ATTR_UCAST_FLOODING,
> +	IFLA_ATTR_MCAST_FLOODING,
> +	__IFLA_ATTR_MAX
> +};
> +
> +#define IFLA_ATTR_MAX (__IFLA_ATTR_MAX - 1)

Change the prefix to IFLA_SW_* since it's switch specific?

>  
> +#ifdef CONFIG_NET_SWITCHDEV
> +static int do_setswcfg(struct net_device *dev, struct nlattr *attr)
> +{
> +	int rem, err = -EINVAL;
> +	struct nlattr *v;
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	nla_for_each_nested(v, attr, rem) {
> +		u32 op = nla_type(v);
> +		u64 value = 0;
> +
> +		switch (op) {
> +		case IFLA_ATTR_LEARNING:
> +		case IFLA_ATTR_LOOPBACK:
> +		case IFLA_ATTR_BCAST_FLOODING:
> +		case IFLA_ATTR_UCAST_FLOODING:
> +		case IFLA_ATTR_MCAST_FLOODING: {
> +			if (nla_len(v) < sizeof(value)) {
> +				err = -EINVAL;
> +				break;
> +			}

You should validate the message before you start applying the
changes. Otherwise if the 3rd attribute is too short you've already
applied some changes and the user has not idea how much has been
applied.

nla_parse_nested() can help here.


> +
> +			value = nla_get_u64(v);
> +			err = ops->ndo_switch_port_set_cfg(dev,
> +							   op,
> +							   value);

This avoids having individual ndos but wastes a lot of space in the
Netlink message. Not a problem when setting configuration but you
likely want to dump these attributes as well and we need 12 bytes
for each attribute even though some are merely flags which could fit
in 4 bytes.

>  static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
>  {
>  	int rem, err = -EINVAL;
> @@ -1740,6 +1780,20 @@ static int do_setlink(const struct sk_buff *skb,
>  			status |= DO_SETLINK_NOTIFY;
>  		}
>  	}
> +#ifdef CONFIG_NET_SWITCHDEV
> +	if (tb[IFLA_SWITCH_PORT_CFG]) {
> +		err = -EOPNOTSUPP;
> +		if (!ops->ndo_switch_port_set_cfg)
> +			goto errout;
> +		if (!ops->ndo_switch_parent_id_get)
> +			goto errout;
> +		err = do_setswcfg(dev, tb[IFLA_SWITCH_PORT_CFG]);
> +		if (err < 0)
> +			goto errout;
> +
> +		status |= DO_SETLINK_NOTIFY;
> +	}
> +#endif

Should return -EOPNOTSUPP if IFLA_SWITCH_PORT_CFG is provided but
CONFIG_NET_SWITCHDEV is not set.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists