public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/4] sun8i: On H3 Use word 1 instead of word 3 from the SID
Date: Wed, 27 Jul 2016 22:14:29 +0300	[thread overview]
Message-ID: <20160727221429.2321e2da@i7> (raw)
In-Reply-To: <1469635835-2063-3-git-send-email-hdegoede@redhat.com>

Hello Hans,

On Wed, 27 Jul 2016 18:10:34 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> It seems that bytes 13-14 of the SID / bytes 1-2 from word 3 of the SID
> are always 0 on H3 making it a poor candidate to use as source for the
> serialnr / mac-address, switch to word1 which seems to be more random.
> 
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Corentin LABBE <clabbe.montjoie@gmail.com>
> Cc: Amit Singh Tomar <amittomer25@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  board/sunxi/board.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index ef3fe26..bbe5340 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -620,12 +620,17 @@ static void setup_environment(const void *fdt)
>  	uint8_t mac_addr[6];
>  	char ethaddr[16];
>  	int i, ret;
> +#ifdef CONFIG_MACH_SUN8I_H3
> +	const int idx0 = 0, idx1 = 1;
> +#else
> +	const int idx0 = 0, idx1 = 3;
> +#endif
>  
>  	ret = sunxi_get_sid(sid);
> -	if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
> +	if (ret == 0 && sid[idx0] != 0 && sid[idx1] != 0) {
>  		/* Ensure the NIC specific bytes of the mac are not all 0 */
> -		if ((sid[3] & 0xffffff) == 0)
> -			sid[3] |= 0x800000;
> +		if ((sid[idx1] & 0xffffff) == 0)
> +			sid[idx1] |= 0x800000;
>  
>  		for (i = 0; i < 4; i++) {
>  			sprintf(ethaddr, "ethernet%d", i);
> @@ -642,18 +647,18 @@ static void setup_environment(const void *fdt)
>  
>  			/* Non OUI / registered MAC address */
>  			mac_addr[0] = (i << 4) | 0x02;
> -			mac_addr[1] = (sid[0] >>  0) & 0xff;
> -			mac_addr[2] = (sid[3] >> 24) & 0xff;
> -			mac_addr[3] = (sid[3] >> 16) & 0xff;
> -			mac_addr[4] = (sid[3] >>  8) & 0xff;
> -			mac_addr[5] = (sid[3] >>  0) & 0xff;
> +			mac_addr[1] = (sid[idx0] >>  0) & 0xff;
> +			mac_addr[2] = (sid[idx1] >> 24) & 0xff;
> +			mac_addr[3] = (sid[idx1] >> 16) & 0xff;
> +			mac_addr[4] = (sid[idx1] >>  8) & 0xff;
> +			mac_addr[5] = (sid[idx1] >>  0) & 0xff;
>  
>  			eth_setenv_enetaddr(ethaddr, mac_addr);
>  		}
>  
>  		if (!getenv("serial#")) {
>  			snprintf(serial_string, sizeof(serial_string),
> -				"%08x%08x", sid[0], sid[3]);
> +				"%08x%08x", sid[idx0], sid[idx1]);
>  
>  			setenv("serial#", serial_string);
>  		}

Is it really a good idea trying to guess which parts of the SID are
sufficiently unique, depending on the SoC generation?

We can calculate some kind of hash (CRC32? SHA256?) over the whole
SID. This will ensure that all the SID bits are accounted for.

Also changing the MAC address generation algorithm is an invasive
change, which is affecting the end users. So IMHO it's best to have
it implemented properly right from the start, rather than evolving
via a trial and error method. What if it turns out that word1
from the SID is actually not sufficiently random on H3? How large
was your sample set?

-- 
Best regards,
Siarhei Siamashka

  parent reply	other threads:[~2016-07-27 19:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-27 16:10 [U-Boot] [PATCH 1/4] net: sun8i_emac: Fix DMA alignment issues with the rx / tx buffers Hans de Goede
2016-07-27 16:10 ` [U-Boot] [PATCH 2/4] sunxi: Ensure that the NIC specific bytes of the mac are not all 0 Hans de Goede
2016-07-27 18:24   ` Ian Campbell
2016-07-27 16:10 ` [U-Boot] [PATCH 3/4] sun8i: On H3 Use word 1 instead of word 3 from the SID Hans de Goede
2016-07-27 18:25   ` Ian Campbell
2016-07-27 19:14   ` Siarhei Siamashka [this message]
2016-07-28  3:13     ` Chen-Yu Tsai
2016-07-28 18:35       ` Hans de Goede
2016-07-29 11:12         ` Siarhei Siamashka
2016-07-29  9:35     ` Hans de Goede
2016-07-27 16:10 ` [U-Boot] [PATCH 4/4] sunxi: Re-enable h3 emac support Hans de Goede
2016-07-27 18:25   ` Ian Campbell
2016-07-28 18:01     ` Jagan Teki
2016-07-27 17:11 ` [U-Boot] [PATCH 1/4] net: sun8i_emac: Fix DMA alignment issues with the rx / tx buffers Amit Tomer
2016-07-28 18:37   ` Hans de Goede
2016-07-27 18:20 ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160727221429.2321e2da@i7 \
    --to=siarhei.siamashka@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox