netdev - Re: [RFC PATCH v2 net-next 7/8] net: dsa: ocelot: felix: add support for VSC75XX control over SPI

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: <20210710205205.blitrpvdwmf4au7z@skbuf>
Date:   Sat, 10 Jul 2021 23:52:05 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Colin Foster <colin.foster@...advantage.com>
Cc:     andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com,
        davem@...emloft.net, kuba@...nel.org, robh+dt@...nel.org,
        claudiu.manoil@....com, alexandre.belloni@...tlin.com,
        UNGLinuxDriver@...rochip.com, linux@...linux.org.uk,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 net-next 7/8] net: dsa: ocelot: felix: add support
 for VSC75XX control over SPI

On Sat, Jul 10, 2021 at 12:26:01PM -0700, Colin Foster wrote:
> +static const struct felix_info ocelot_spi_info = {
> +	.target_io_res			= vsc7512_target_io_res,
> +	.port_io_res			= vsc7512_port_io_res,
> +	.regfields			= vsc7512_regfields,
> +	.map				= vsc7512_regmap,
> +	.ops				= &vsc7512_ops,
> +	.stats_layout			= vsc7512_stats_layout,
> +	.num_stats			= ARRAY_SIZE(vsc7512_stats_layout),
> +	.vcap				= vsc7512_vcap_props,
> +	.num_mact_rows			= 1024,
> +
> +	/* The 7512 and 7514 both have support for up to 10 ports. The 7511 and
> +	 * 7513 have support for 4. Due to lack of hardware to test and
> +	 * validate external phys, this is currently limited to 4 ports.
> +	 * Expanding this to 10 for the 7512 and 7514 and defining the
> +	 * appropriate phy-handle values in the device tree should be possible.
> +	 */
> +	.num_ports			= 4,

Ouch, this was probably not a good move.
felix_setup() -> felix_init_structs sets ocelot->num_phys_ports based on
this value.
If you search for ocelot->num_phys_ports in ocelot and in felix, it is
widely used to denote "the index of the CPU port module within the
analyzer block", since the CPU port module's number is equal to the
number of the last physical port + 1. If VSC7512 has 10 ports, then the
CPU port module is port 10, and if you set num_ports to 4 you will cause
the driver to misbehave.

> +	.num_tx_queues			= OCELOT_NUM_TC,
> +	.mdio_bus_alloc			= felix_mdio_bus_alloc,
> +	.mdio_bus_free			= felix_mdio_bus_free,
> +	.phylink_validate		= vsc7512_phylink_validate,
> +	.prevalidate_phy_mode		= vsc7512_prevalidate_phy_mode,
> +	.port_setup_tc			= vsc7512_port_setup_tc,
> +	.init_regmap			= vsc7512_regmap_init,
> +};

> +	/* Not sure about this */
> +	ocelot->num_flooding_pgids = 1;

Why are you not sure? It's the same as ocelot.