From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C3822C433F5 for ; Thu, 18 Nov 2021 19:04:37 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 702B4611F2 for ; Thu, 18 Nov 2021 19:04:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 702B4611F2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id C068882F97; Thu, 18 Nov 2021 20:04:33 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1637262274; bh=e74l+AZB8cU54yxAHF3UMUDISVsm7bOJEC3NU1TE1Iw=; h=To:cc:From:Subject:In-reply-to:References:Date:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=DGbWB76evb47sZEis1DCh2Dr/rV0gnV/rDLVAb7v5SNHKHPVz3osrVCpiFKGmgpzy r+X0f+d/mVr3QS21ZUwclniea/nSiXG1E7ryBjFS6KFuW8q6X5J9u/r2NDmKzb28LA jLuddScZ4dzDtDbw28SxjRBpQYutXQ/gKR3Wf+A2Q/s8C1tPjisNnWn+P/lMskYfCX r8oKSyMRZIjb8Xh95vaQ2+vLjpNQlO4RZVo0Tc8lX5oSI43R8hSpGa9jlO+s7id1hZ l5zxmmJnuMoz8n7vC/t76tS0kBymbA5bEt4N2OjJeCTK2n7aF3c7RU9eoezlqS10o6 dciLnnVtWTvPg== Received: from janitor.denx.de (unknown [62.91.23.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: noc@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 116D682F8D for ; Thu, 18 Nov 2021 20:04:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1637262272; bh=e74l+AZB8cU54yxAHF3UMUDISVsm7bOJEC3NU1TE1Iw=; h=To:cc:From:Subject:In-reply-to:References:Date:From; b=VmCPwWEp+2mcGzSSNmUAlJkQRAHNi0HR1mcWGJHuTE/m698uCUH36m/sdb1a8Lag1 P6bpQbtV86deod0gRZrE2Ng7IvQD9U2/aJevf12QRR48H9j/v9eAXgDxsIUvA1YUMd 54nbSXBe+ASFUwL/FeSgqkSFWq2wgNiWjmCgxF2NaHVCofo8GlujnQUGHgDNpBIe/i weK1HvdNYihE25K0CxiAxZ7qAfCGxcqXmyhT1RziqmzoYKWo0JovMmXuWdLqCep+LF XTkbVKGGB8mVSS/WIdTLwea48uKerO0Cu8+IX+iMa9BpSdsyuPR/55jghbjFsDeL8h oUgf/NToGqORg== Received: by janitor.denx.de (Postfix, from userid 108) id 79660A02FC; Thu, 18 Nov 2021 20:04:31 +0100 (CET) Received: from gemini.denx.de (gemini.denx.de [10.4.0.2]) by janitor.denx.de (Postfix) with ESMTPS id 042C7A011E; Thu, 18 Nov 2021 20:04:22 +0100 (CET) Received: from gemini.denx.de (localhost [IPv6:::1]) by gemini.denx.de (Postfix) with ESMTP id 903DD1E0CB7; Thu, 18 Nov 2021 20:04:22 +0100 (CET) To: Tom Rini cc: Michael Walle , Ramon Fried , Michal Simek , Grygorii Strashko , git , Joe Hershberger , Simon Glass , U-Boot Mailing List From: Wolfgang Denk Subject: Re: [PATCH] net: uclass: Save ethernet MAC address when generated MIME-Version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 8bit In-reply-to: <20211118162920.GH24579@bill-the-cat> References: <20211116141855.GD24579@bill-the-cat> <1739526.1637074605@gemini.denx.de> <20211116184146.GF24579@bill-the-cat> <1797888.1637135092@gemini.denx.de> <20211117115025.GV24579@bill-the-cat> <1820750.1637151898@gemini.denx.de> <20211117123548.GX24579@bill-the-cat> <1836596.1637164587@gemini.denx.de> <20211117161545.GA24579@bill-the-cat> <1889944.1637219294@gemini.denx.de> <20211118162920.GH24579@bill-the-cat> Comments: In-reply-to Tom Rini message dated "Thu, 18 Nov 2021 11:29:20 -0500." Date: Thu, 18 Nov 2021 20:04:22 +0100 Message-ID: <1944509.1637262262@gemini.denx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.35 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Dear Tom, In message <20211118162920.GH24579@bill-the-cat> you wrote: > > > It is perfectly OK for U-Boot to start with a random MAC address, > > use this for a while, and change it so something else later. This > > is what may happen at production: say the MAC address is stored in > > some EEPROM or fuses, which are initially empty, so U-Boot will use > > a random MAC address, download it's board specific date (serial#, > > MAC address, ...) over network, programm it into the respective > > storage devices, and switch to using the new "official" MAC address. > > Yes. And up until this patch saying I want to use this random MAC with > Linux required user intervention. Correct - this is a bug in the implementation of thispatch, and apparently the few people that ever used it did not notice it or care enough about it to submit fixes. > And I dare say that half the time or > more that was probably just not noticed (everything comes up with > dynamic host names/dns these days, noticing the IP changed between > U-Boot and Linux is easy to miss in those cases). Agreed. > > CONFIG_NET_RANDOM_ETHADDR is the attempt to do the same > > automatically, except that it falls short of setting the "ethaddr" > > environment variable. I consider this a bug. > > Since the code isn't that old, it shouldn't be hard to pull up the > thread / patch on introducing it. So, lets: > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-1-git-sen= > d-email-joe.hershberger@ni.com/ > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-2-git-sen= > d-email-joe.hershberger@ni.com/ > https://patchwork.ozlabs.org/project/uboot/patch/1430769315-27109-3-git-sen= > d-email-joe.hershberger@ni.com/ > > And from there we can take away I think two important things: > 1: Not setting ethaddr with NET_RANDOM_ETHADDR is intentional Ummm... From which part of the patches or the comments do you take this conclusion? Not a single line of code, or comments in the code or commit messages, nor any of the review comments refer to the "ethadddr" environment variable. I thing it was just an oversight to set it here, which was not detected during testing as everything appeared to work. I actually see the opposite - Joes initial commit messages speaks about "Implement the random *ethaddr* fallback", and as his following patches clean up the use of hard codes definitions of the "ethaddr" environment variable, I see it more likely that he means the environment variable and not MAC address. > 2: It was widely inconsistent before! Lots of platforms were generating > a random MAC and then setting ethaddr. I think in fact most were and > it's just a few (which coincidentally are some of Michael's other > platforms) that were not. Agreed. > > Yes, and it should be clear that this is not a reasonable approach. > > It thwarts the original intent of the NET_RANDOM_ETHADDR patch to do > > thing in an automated, scriptable way. I see this actually as a > > manifestation of the bug that ethaddr does not get set. At this > > point the problem was recognized, but instead of properly fixing it, > > a poor workaround was documented. > > Well, for the record, with the above patch links, NET_RANDOM_ETHADDR > _is_ working as intended. That said.. No, it works only half. It fails to set the "ethaddr" environment variable which is mandatory for correct funktion (query by the user, passing to Linux). > So, yes, OK, this is a bug fix, in that NET_RANDOM_ETHADDR was incorrect > at introduction. That means we do need a v2 of this patch that updates > the Kconfig help text as that currently says it will not update the > environment. This makes no sense to me. Instead of documenting the bug we should fix it and add the missing eth_env_set_enetaddr(). If I prepare such patches, will you accept these? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Microsoft Multitasking: several applications can crash at the same time.