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
