netdev - Re: [PATCH net-next] net: dsa: mv88e6xxx: 6352: parse VTU data before loading STU data

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]

Message-ID: <20201111173953.7e194eab@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 11 Nov 2020 17:39:53 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Tobias Waldekranz" <tobias@...dekranz.com>
Cc:     <davem@...emloft.net>, <andrew@...n.ch>,
        <vivien.didelot@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: 6352: parse VTU data
 before loading STU data

On Thu, 12 Nov 2020 00:49:03 +0100 Tobias Waldekranz wrote:
> > I'm unclear what this fixes. What functionality is broken on 6097?  
> 
> VLAN configuration. As soon as you add the second port to a VLAN, all
> other port membership configuration is overwritten with zeroes. The HW
> interprets this as all ports being "unmodified members" of the VLAN.
> 
> I suspect that is why it has not been discovered. In the simple case
> when all ports belong to the same VLAN, switching will still work. But
> using multiple VLANs or trying to set multiple ports as tagged members
> will not work.

I see, this info would be good to include in the commit message.
User impact is what backporters care about the most.

> At the lowest level, the current implementation assumes that it can
> perform two consecutive operations where each op will load half of a
> register, and then read out the union of the information. This is true
> for some devices (6352), but not for others (6097).
> 
> 6352 pseudo-hdl-in-c:
> 
> stu_get_next()
> {
> 	*data |= stu_data & 0xf0f0;
> }
> 
> vtu_get_next()
> {
> 	*data |= vtu_data & 0x0f0f;
> }
> 
> 6097 pseudo-hdl-in-c:
> 
> stu_get_next()
> {
> 	*data = stu_data;
> }
> 
> vtu_get_next()
> {
> 	*data = vtu_data;
> }
> 
> > Can we identify the commit for a fixes tag?  
> 
> I will try to pinpoint it tomorrow. I suppose I should also rebase it
> against "net" since it is a bug.

Indeed, thanks :)