linux-kernel - Re: [Aspeed, v2 2/2] net: ftgmac100: Change the order of getting MAC address

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: <20201228140115.6af4d510@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 28 Dec 2020 14:01:15 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Hongwei Zhang <hongweiz@....com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        openbmc@...ts.ozlabs.org, David S Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>
Subject: Re: [Aspeed, v2 2/2] net: ftgmac100: Change the order of getting
 MAC address

On Tue, 22 Dec 2020 22:00:34 +0100 Andrew Lunn wrote:
> On Tue, Dec 22, 2020 at 09:46:52PM +0100, Heiner Kallweit wrote:
> > On 22.12.2020 21:14, Hongwei Zhang wrote:  
> > > Dear Reviewer,
> > > 
> > > Use native MAC address is preferred over other choices, thus change the order
> > > of reading MAC address, try to read it from MAC chip first, if it's not
> > >  availabe, then try to read it from device tree.
> > > 
> > > Hi Heiner,
> > >   
> > >> From:	Heiner Kallweit <hkallweit1@...il.com>
> > >> Sent:	Monday, December 21, 2020 4:37 PM  
> > >>> Change the order of reading MAC address, try to read it from MAC chip 
> > >>> first, if it's not availabe, then try to read it from device tree.
> > >>>  
> > >> This commit message leaves a number of questions. It seems the change isn't related at all to the 
> > >> change that it's supposed to fix.
> > >>
> > >> - What is the issue that you're trying to fix?
> > >> - And what is wrong with the original change?  
> > > 
> > > There is no bug or something wrong with the original code. This patch is for
> > > improving the code. We thought if the native MAC address is available, then
> > > it's preferred over MAC address from dts (assuming both sources are available).
> > > 
> > > One possible scenario, a MAC address is set in dts and the BMC image is 
> > > compiled and loaded into more than one platform, then the platforms will
> > > have network issue due to the same MAC address they read.
> > >   
> > 
> > Typically the DTS MAC address is overwritten by the boot loader, e.g. uboot.
> > And the boot loader can read it from chip registers. There are more drivers
> > trying to read the MAC address from DTS first. Eventually, I think, the code
> > here will read the same MAC address from chip registers as uboot did before.  
> 
> Do we need to worry about, the chip contains random junk, which passes
> the validitiy test? Before this patch the value from DT would be used,
> and the random junk is ignored. Is this change possibly going to cause
> a regression?

Hongwei, please address Andrew's questions.

Once the discussion is over please repost the patches as
git-format-patch would generate them. The patch 2/2 of this 
series is not really a patch, which confuses all patch handling 
systems.

It also appears that 35c54922dc97 ("ARM: dts: tacoma: Add reserved
memory for ramoops") does not exist upstream.