netdev - Re: [PATCH v2 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477

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] [day] [month] [year] [list]

Date:   Mon, 15 May 2017 14:20:53 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Woojung.Huh@...rochip.com, andrew@...n.ch
Cc:     vivien.didelot@...oirfairelinux.com,
        sergei.shtylyov@...entembedded.com, netdev@...r.kernel.org,
        davem@...emloft.net, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v2 net-next 3/5] dsa: add DSA switch driver for Microchip
 KSZ9477

On 05/15/2017 02:01 PM, Woojung.Huh@...rochip.com wrote:
>>> +static const struct ksz_chip_data ksz_switch_chips[] = {
>>> +	{
>>> +		.chip_id = 0x00947700,
>>> +		.dev_name = "KSZ9477",
>>> +		.num_vlans = 4096,
>>> +		.num_alus = 4096,
>>> +		.num_statics = 16,
>>> +		.enabled_ports = 0x1F,	/* port0-4 */
>>> +		.cpu_port = 5,		/* port5 (RGMII) */
>>> +		.port_cnt = 7,
>>> +		.phy_port_cnt = 5,
>>> +	},
>>> +};
>>
>> Hi Woojung
>>
>> Do we need cpu_port in this table? Can any port be used as a CPU port?
>> From the code in ksz_config_cpu_port() it seems like it can.
>>
>> And do we need enabled_ports? This seems to suggest only ports 0-4 can
>> be user ports?
>>
> Andrew,
> 
> Intention of cpu_port is for default cpu_port when devicetree doesn't have it.
> However, it won't get back to dst, so it won't be needed.
> Will remove it.
> 
> Enabled_ports was to configure physically connected ports. (For instance, 7 ports switch but board only uses 4 ports.)
> This code path is not working as expected. Will update at next version of patch.

FWIW, the b53 driver has a similar set of constructs (which is probably
where you got this from), and this needs fixing too. cpu_port is a good
indication of which ports can support tags, whereas enabled_ports is
completely redundant with ds->enabled_port_mask.

Once net-next opens again, the plan is to stop using these private
constructs and switch to using what DSA provides.

BTW, there are only a few things that make sense to be represented as a
true number, most attributes of a switch (port number, locations etc.)
are truly bitmasks.
-- 
Florian