netdev - Re: [PATCH net-next 1/2] net: dsa: Add ability to elect CPU port

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: <20191029013938.GG15259@lunn.ch>
Date:   Tue, 29 Oct 2019 02:39:38 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, vivien.didelot@...il.com,
        davem@...emloft.net, Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: Re: [PATCH net-next 1/2] net: dsa: Add ability to elect CPU port

On Mon, Oct 28, 2019 at 03:32:35PM -0700, Florian Fainelli wrote:
> In a configuration where multiple CPU ports are declared within the
> platform configuration, it may be desirable to make sure that a
> particular CPU port gets used. This is particularly true for Broadcom
> switch that are fairly flexible to some extent in which port can be the
> CPU port, yet will be more featureful if port 8 is elected.

> -static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
> +static struct dsa_port *dsa_tree_find_cpu(struct dsa_switch_tree *dst)
>  {
> +	struct dsa_switch *ds;
>  	struct dsa_port *dp;
> +	int err;
>  
> -	list_for_each_entry(dp, &dst->ports, list)
> -		if (dsa_port_is_cpu(dp))
> +	list_for_each_entry(dp, &dst->ports, list) {
> +		ds = dp->ds;
> +		if (!dsa_port_is_cpu(dp))
> +			continue;
> +
> +		if (!ds->ops->elect_cpu_port)
>  			return dp;
>  
> +		err = ds->ops->elect_cpu_port(ds, dp->index);
> +		if (err == 0)
> +			return dp;
> +	}
> +
>  	return NULL;
>  }

Hi Florian

I think is_preferred_cpu_port() would be a better name, and maybe a
bool?

Also, i don't think we should be returning NULL at the end like
this. If the device tree does not have the preferred port as CPU port,
we should use dsa_tree_find_cpu() to pick one of the actually offered
CPU ports in DT? It sounds like your hardware will still work if any
port is used as the CPU port.

And maybe we need a is_valid_cpu_port()? Some of the chipsets only
have a subset which can be CPU ports, the hardware is not as flexible.
The core can then validate the CPU port really is valid, rather than
the driver, e.g. qca8k, validating the CPU port in setup() and
returning an error.

    Andrew