On Tue, Apr 12, 2016 at 09:38:53AM +0200, Martin Pieuchot wrote:
> On 12/04/16(Tue) 17:16, Jonathan Gray wrote:
> > On Thu, Apr 07, 2016 at 04:45:43PM +0200, Martin Pieuchot wrote:
> > > On 07/04/16(Thu) 22:29, Jonathan Gray wrote:
> > > > On Thu, Apr 07, 2016 at 01:47:13PM +0200, Martin Pieuchot wrote:
> > > > > If we need a random value to fake an Ethernet address, let's use
> > > > > arc4random(9).
> > > > >
> > > > > ok?
> > > >
> > > > You missed if_cdce.c which uses 0x2acb as well but uses microuptime.
> > >
> > > Updated diff below.
> > >
> > > > Why can't these all just use ether_fakeaddr() ?
> > >
> > > Why not?
> >
> > I can't find any reason for the mac addresses to be a particular
> > value, but your diff needs some more tweaks:
> >
> > > @@ -310,13 +307,7 @@ found:
> > >
> > > if (!ethd || usbd_get_string_desc(sc->cdce_udev, ethd->iMacAddress, 0,
> > > &eaddr_str, &len)) {
> > > - macaddr_hi = htons(0x2acb);
> > > - bcopy(&macaddr_hi, &sc->cdce_arpcom.ac_enaddr[0],
> > > - sizeof(u_int16_t));
> > > - getmicrotime(&now);
> > > - macaddr_lo = htonl(now.tv_usec << 8);
> > > - bcopy(&macaddr_lo, &sc->cdce_arpcom.ac_enaddr[2],
> > > sizeof(u_int32_t));
> > > - sc->cdce_arpcom.ac_enaddr[5] = (u_int8_t)(sc->cdce_dev.dv_unit);
> > > + ether_fakeaddr(ifp);
> >
> > The other case should be changed to write to ((struct arpcom
> > *)ifp)->ac_enaddr
> >
> > > } else {
> > > for (i = 0; i < ETHER_ADDR_LEN * 2; i++) {
> > > int c = UGETW(eaddr_str.bString[i]);
> > > @@ -337,7 +328,6 @@ found:
> > > printf("%s: address %s\n", sc->cdce_dev.dv_xname,
> > > ether_sprintf(sc->cdce_arpcom.ac_enaddr));
> >
> > and use ((struct arpcom *)ifp)->ac_enaddr to print it here?
> >
> > Or should the addr in the ifp be copied to sc->cdce_arpcom.ac_enaddr ?
>
> ifp points to sc->cdce_arpcom.ac_if which is the first member of the
> "struct arpcom" so there's no need to copy anything. I admit it is
> confusing but the two following writing point to the same location:
>
> - ((struct arpcom *)ifp)->ac_enaddr
> - sc->cdce_arpcom.ac_enaddr
>
Ah right, GET_IFP is taken from the arpcom in the softc.
ok jsg@
