netdev - Re: [PATCH net-next 1/2] net: dsa: add a dsa_port_is

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: <20200319151714.GB3446043@t480s.localdomain>
Date:   Thu, 19 Mar 2020 15:17:14 -0400
From:   Vivien Didelot <vivien.didelot@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch,
        f.fainelli@...il.com
Subject: Re: [PATCH net-next 1/2] net: dsa: add a dsa_port_is_enabled helper
 function

Hi Vladimir,

On Thu, 19 Mar 2020 20:56:19 +0200, Vladimir Oltean <olteanv@...il.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> Sometimes drivers need to do per-port operation outside the port DSA
> methods, and in that case they typically iterate through their port list
> themselves.
> 
> Give them an aid to skip ports that are disabled in the device tree
> (which the DSA core already skips).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  include/net/dsa.h |  2 ++
>  net/dsa/dsa2.c    | 29 +++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index beeb81a532e3..813792e6f0be 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -376,6 +376,8 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
>  		return dp->vlan_filtering;
>  }
>  
> +bool dsa_port_is_enabled(struct dsa_switch *ds, int port);
> +
>  typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
>  			      bool is_static, void *data);
>  struct dsa_switch_ops {
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index e7c30b472034..752f21273bd6 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -727,6 +727,35 @@ static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
>  	return err;
>  }
>  
> +bool dsa_port_is_enabled(struct dsa_switch *ds, int port)
> +{
> +	struct device_node *dn = ds->dev->of_node;
> +	struct device_node *ports, *port_node;
> +	bool found = false;
> +	int reg, err;
> +
> +	ports = of_get_child_by_name(dn, "ports");
> +	if (!ports) {
> +		dev_err(ds->dev, "no ports child node found\n");
> +		return false;
> +	}
> +
> +	for_each_available_child_of_node(ports, port_node) {
> +		err = of_property_read_u32(port_node, "reg", &reg);
> +		if (err)
> +			goto out_put_node;
> +
> +		if (reg == port) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +out_put_node:
> +	of_node_put(ports);
> +	return found;
> +}

Why do you need to iterate on the device tree? Ideally we could make
abstraction of that (basic platform data is supported too) and use pure DSA
helpers, given that DSA core has already peeled all that for us.

Your helper above doesn't even look at the port's state, only if it is declared
or not. Looking at patch 2, Using !dsa_is_unused_port(ds, port) seems enough.


Thanks,

	Vivien