| Message ID | 1469785785-6675-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Hans de Goede |
| Headers | show |
On Fri, 2016-07-29 at 11:49 +0200, Hans de Goede 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, and the other non constant words (1 and 2) > also > have quite a few bits which are the same for some boards, > > This commits switches to using words 1 - 3 ex-or-ed together to get a > more unique value for the mac-address / serialnr. > > 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> > --- > Changes in v2: > -ex-or all 3 words together instead of picking a different word > --- > board/sunxi/board.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index ef3fe26..e0734fb 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -623,6 +623,21 @@ static void setup_environment(const void *fdt) > > ret = sunxi_get_sid(sid); > if (ret == 0 && sid[0] != 0 && sid[3] != 0) { Could/should this check be removed now? Or should sid[1]/sid[2] be included? > +#ifdef CONFIG_MACH_SUN8I_H3 > > + /* > > + * The single words 1 - 3 of the SID have quite a few bits > > + * which are the same on many models on the H3, so we ex-or > > + * them together to get a bit more unique value. > > + * > > + * Note we only do this on the H3 as we cannot change the > > + * algorithm on other SoCs since those have been using > > + * fixed mac-addresses based on only using word 3 for a > > + * a long time and changing a fixed mac-address with an > > > + * u-boot update is not good. I wonder if we should invert this, i.e. make it not use this new method on all existing models. That would mean that all future models will automatically use this new (IMHO better) scheme instead of requiring it to be manually added each time (which we are sure to forget to do). Main downside is the ifdef is going to be pretty long, but at least it would be static. Maybe doing it with select in Kconfig (i.e. select SUNXI_LEGACY_MAC_SID_THINGY on the existing platforms) would be a more tollerable approach? > > > > > > + */ > > + sid[3] ^= sid[1] ^ sid[2]; > +#endif > + > > /* Ensure the NIC specific bytes of the mac are not all 0 */ > if ((sid[3] & 0xffffff) == 0) > sid[3] |= 0x800000;
On Fri, 29 Jul 2016 11:49:45 +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, and the other non constant words (1 and 2) also > have quite a few bits which are the same for some boards, > > This commits switches to using words 1 - 3 ex-or-ed together to get a > more unique value for the mac-address / serialnr. > > 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> > --- > Changes in v2: > -ex-or all 3 words together instead of picking a different word > --- > board/sunxi/board.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index ef3fe26..e0734fb 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -623,6 +623,21 @@ static void setup_environment(const void *fdt) > > ret = sunxi_get_sid(sid); > if (ret == 0 && sid[0] != 0 && sid[3] != 0) { > +#ifdef CONFIG_MACH_SUN8I_H3 > + /* > + * The single words 1 - 3 of the SID have quite a few bits > + * which are the same on many models on the H3, so we ex-or > + * them together to get a bit more unique value. > + * > + * Note we only do this on the H3 as we cannot change the > + * algorithm on other SoCs since those have been using > + * fixed mac-addresses based on only using word 3 for a > + * a long time and changing a fixed mac-address with an > + * u-boot update is not good. > + */ > + sid[3] ^= sid[1] ^ sid[2]; > +#endif > + > /* Ensure the NIC specific bytes of the mac are not all 0 */ > if ((sid[3] & 0xffffff) == 0) > sid[3] |= 0x800000; Using XOR is not a good idea, see my reply in the v1 patch discussion: http://lists.denx.de/pipermail/u-boot/2016-July/262298.html
Hi, On 29-07-16 12:28, Ian Campbell wrote: > On Fri, 2016-07-29 at 11:49 +0200, Hans de Goede 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, and the other non constant words (1 and 2) >> also >> have quite a few bits which are the same for some boards, >> >> This commits switches to using words 1 - 3 ex-or-ed together to get a >> more unique value for the mac-address / serialnr. >> >> 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> >> --- >> Changes in v2: >> -ex-or all 3 words together instead of picking a different word >> --- >> board/sunxi/board.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >> index ef3fe26..e0734fb 100644 >> --- a/board/sunxi/board.c >> +++ b/board/sunxi/board.c >> @@ -623,6 +623,21 @@ static void setup_environment(const void *fdt) >> >> ret = sunxi_get_sid(sid); >> if (ret == 0 && sid[0] != 0 && sid[3] != 0) { > > Could/should this check be removed now? Or should sid[1]/sid[2] be > included? Good point, the check is there because some A10 / A20 boards have an all 0 sid, now that we make sure that the NIC specific bytes of the mac are not all 0, only checking for sid[0] != 0 should be enough to capture non initialized sid-s. >> +#ifdef CONFIG_MACH_SUN8I_H3 >>> + /* >>> + * The single words 1 - 3 of the SID have quite a few bits >>> + * which are the same on many models on the H3, so we ex-or >>> + * them together to get a bit more unique value. >>> + * >>> + * Note we only do this on the H3 as we cannot change the >>> + * algorithm on other SoCs since those have been using >>> + * fixed mac-addresses based on only using word 3 for a >>> + * a long time and changing a fixed mac-address with an >>>> + * u-boot update is not good. > > I wonder if we should invert this, i.e. make it not use this new method > on all existing models. That would mean that all future models will > automatically use this new (IMHO better) scheme instead of requiring it > to be manually added each time (which we are sure to forget to do). Good point, I'll fix this and the above check for v3. Regards, Hans
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index ef3fe26..e0734fb 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -623,6 +623,21 @@ static void setup_environment(const void *fdt) ret = sunxi_get_sid(sid); if (ret == 0 && sid[0] != 0 && sid[3] != 0) { +#ifdef CONFIG_MACH_SUN8I_H3 + /* + * The single words 1 - 3 of the SID have quite a few bits + * which are the same on many models on the H3, so we ex-or + * them together to get a bit more unique value. + * + * Note we only do this on the H3 as we cannot change the + * algorithm on other SoCs since those have been using + * fixed mac-addresses based on only using word 3 for a + * a long time and changing a fixed mac-address with an + * u-boot update is not good. + */ + sid[3] ^= sid[1] ^ sid[2]; +#endif + /* Ensure the NIC specific bytes of the mac are not all 0 */ if ((sid[3] & 0xffffff) == 0) sid[3] |= 0x800000;
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, and the other non constant words (1 and 2) also have quite a few bits which are the same for some boards, This commits switches to using words 1 - 3 ex-or-ed together to get a more unique value for the mac-address / serialnr. 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> --- Changes in v2: -ex-or all 3 words together instead of picking a different word --- board/sunxi/board.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)