netdev - Re: [PATCH RFC net-next 1/3] net: dsa: allow for multiple CPU ports

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: <20190824154302.GB8251@lunn.ch>
Date:   Sat, 24 Aug 2019 17:43:02 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marek Behún <marek.behun@....cz>
Cc:     netdev@...r.kernel.org, Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        David Ahern <dsahern@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH RFC net-next 1/3] net: dsa: allow for multiple CPU ports

> +static int dsa_tree_setup_default_cpus(struct dsa_switch_tree *dst)
>  {
>  	struct dsa_switch *ds;
>  	struct dsa_port *dp;
> -	int device, port;
> +	int device, port, i;
>  
> -	/* DSA currently only supports a single CPU port */
> -	dst->cpu_dp = dsa_tree_find_first_cpu(dst);
> -	if (!dst->cpu_dp) {
> +	dsa_tree_fill_cpu_ports(dst);
> +	if (!dst->num_cpu_dps) {
>  		pr_warn("Tree has no master device\n");
>  		return -EINVAL;
>  	}
>  
> -	/* Assign the default CPU port to all ports of the fabric */
> +	/* Assign the default CPU port to all ports of the fabric in a round
> +	 * robin way. This should work nicely for all sane switch tree designs.
> +	 */
> +	i = 0;
> +
>  	for (device = 0; device < DSA_MAX_SWITCHES; device++) {
>  		ds = dst->ds[device];
>  		if (!ds)
> @@ -238,18 +249,20 @@ static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
>  		for (port = 0; port < ds->num_ports; port++) {
>  			dp = &ds->ports[port];
>  
> -			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
> -				dp->cpu_dp = dst->cpu_dp;
> +			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp)) {
> +				dp->cpu_dp = dst->cpu_dps[i++];
> +				if (i == dst->num_cpu_dps)
> +					i = 0;
> +			}

Hi Marek

For a single switch, i think this is O.K, but when you have a cluster,
maybe a different allocation should be considered? If this switch has
a local CPU port, use it. Only round robing between remote CPU ports
when there is no local CPU port?

For a two switch setup and each switch having its own CPU port, your
allocation will cause half the CPU traffic to go across the DSA link
between the two switches. But we really want to keep the DSA link for
traffic between user ports on different switches.

But i don't know if it is worth the effort. I've never seen a D in DSA
setup with multiple CPUs ports. I've only ever seen an single switch
with multiple CPU ports.