netdev - Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration

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: <20141218114145.GH28766@casper.infradead.org>
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