netdev - Re: [PATCH net-next 15/17] net: dsa: Add new binding implementation

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]

Date:	Fri, 3 Jun 2016 14:19:48 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>,
	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 15/17] net: dsa: Add new binding implementation

On 06/03/2016 09:44 AM, Andrew Lunn wrote:
> The existing DSA binding has a number of limitations and problems. The
> main problem is that it cannot represent a switch as a linux device,
> hanging off some bus. It is limited to one CPU port. The DSA platform
> device is artificial, and does not really represent hardware.
> 
> Implement a new binding which can be embedded into any type of node on
> a bus to represent one switch device, and its links to other switches.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>

Just a few nits that I had not seen before...


> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 6c314f300424..d8cb2acd4f0a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -294,6 +294,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
>  			}
>  			dst->cpu_switch = index;
>  			dst->cpu_port = i;
> +			ds->cpu_port_mask |= 1 << i;
>  		} else if (!strcmp(name, "dsa")) {
>  			ds->dsa_port_mask |= 1 << i;
>  		} else {

We might want to undo setting the cpu_port_mask bit in
dsa_cpu_dsa_destroy()?

[snip]

> +static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
> +{
> +	struct device_node *port;
> +	u32 index;
> +	int err;
> +
> +	for (index = 0; index < DSA_MAX_PORTS; index++) {
> +		port = ds->ports[index].dn;
> +		if (!port)
> +			continue;
> +
> +		if (!dsa_port_is_dsa(port))
> +			continue;
> +
> +		ds->dsa_port_mask |= 1 << index;
> +
> +		err = dsa_port_complete(dst, ds, port, index);
> +		if (err != 0)

Should we move ds->dsa_port_mask |= 1 << index into dsa_port_complete,
for a) symetry with code undoing this, and b) avoid letting this bit be
set in case dsa_port_complete() returns an error?
-- 
Florian