netdev - Re: [RFC net-next 3/6] ethernet: prestera: use eth_hw_addr_set

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: <YW2wBJ7yoUaLkYVv@shredder>
Date:   Mon, 18 Oct 2021 20:33:56 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        olteanv@...il.com, andrew@...n.ch, f.fainelli@...il.com,
        vkochan@...vell.com, tchornyi@...vell.com
Subject: Re: [RFC net-next 3/6] ethernet: prestera: use eth_hw_addr_set_port()

On Mon, Oct 18, 2021 at 09:26:21AM -0700, Shannon Nelson wrote:
> On 10/18/21 7:19 AM, Jakub Kicinski wrote:
> > On Sat, 16 Oct 2021 14:19:18 -0700 Shannon Nelson wrote:
> > > As a potential consumer of these helpers, I'd rather do my own mac
> > > address byte twiddling and then use eth_hw_addr_set() to put it into place.
> > This is disproved by many upstream drivers, I only converted the ones
> > that jumped out at me on Friday, but I'm sure there is more. If your
> > driver is _really_ doing something questionable^W I mean "special"
> > nothing is stopping you from open coding it. For others the helper will
> > be useful.
> > 
> > IOW I don't understand your comment.
> 
> To try to answer your RFC more clearly: I feel that this particular helper
> obfuscates the operation more than it helps.

FWIW, it at least helped me realize that we are going to have a bug with
Spectrum-4. Currently we have:

ether_addr_copy(addr, mlxsw_sp->base_mac);
addr[ETH_ALEN - 1] += mlxsw_sp_port->local_port;

As a preparation for Spectrum-4 we are promoting 'local_port' to u16
since at least 257 and 258 are valid local port values.

With the current code, the netdev corresponding to local port 1 will
have the same MAC as the netdev corresponding to local port 257.

After Jakub's conversion and changing the 'id' argument to 'unsigned
int' [1], it should work correctly.

[1] https://lore.kernel.org/netdev/20211018070845.68ba815d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/