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:
>
> Index: if_cdce.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 if_cdce.c
> --- if_cdce.c 25 Nov 2015 03:10:00 -0000 1.68
> +++ if_cdce.c 7 Apr 2016 14:43:42 -0000
> @@ -151,7 +151,7 @@ cdce_attach(struct device *parent, struc
> struct cdce_softc *sc = (struct cdce_softc *)self;
> struct usb_attach_arg *uaa = aux;
> int s;
> - struct ifnet *ifp;
> + struct ifnet *ifp = GET_IFP(sc);
> struct usbd_device *dev = uaa->device;
> const struct cdce_type *t;
> usb_interface_descriptor_t *id;
> @@ -162,9 +162,6 @@ cdce_attach(struct device *parent, struc
> const usb_descriptor_t *desc;
> struct usbd_desc_iter iter;
> usb_string_descriptor_t eaddr_str;
> - struct timeval now;
> - u_int32_t macaddr_lo;
> - u_int16_t macaddr_hi;
> int i, j, numalts, len;
> int ctl_ifcno = -1;
> int data_ifcno = -1;
> @@ -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 ?
Likewise for cdcef/ugl.
>
> - ifp = GET_IFP(sc);
> ifp->if_softc = sc;
> ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> ifp->if_ioctl = cdce_ioctl;
> Index: if_cdcef.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_cdcef.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 if_cdcef.c
> --- if_cdcef.c 25 Nov 2015 03:10:00 -0000 1.41
> +++ if_cdcef.c 7 Apr 2016 14:44:00 -0000
> @@ -121,8 +121,6 @@ struct usbf_function_methods cdcef_metho
>
> #define DEVNAME(sc) ((sc)->sc_dev.bdev.dv_xname)
>
> -extern int ticks;
> -
> /*
> * USB function match/attach/detach
> */
> @@ -139,11 +137,10 @@ cdcef_attach(struct device *parent, stru
> struct cdcef_softc *sc = (struct cdcef_softc *)self;
> struct usbf_attach_arg *uaa = aux;
> struct usbf_device *dev = uaa->device;
> - struct ifnet *ifp;
> + struct ifnet *ifp = GET_IFP(sc);
> usbf_status err;
> struct usb_cdc_union_descriptor udesc;
> int s;
> - u_int16_t macaddr_hi;
>
>
> /* Set the device identification according to the function. */
> @@ -237,14 +234,9 @@ cdcef_attach(struct device *parent, stru
>
> s = splnet();
>
> - macaddr_hi = htons(0x2acb);
> - bcopy(&macaddr_hi, &sc->sc_arpcom.ac_enaddr[0], sizeof(u_int16_t));
> - bcopy(&ticks, &sc->sc_arpcom.ac_enaddr[2], sizeof(u_int32_t));
> - sc->sc_arpcom.ac_enaddr[5] = (u_int8_t)(sc->sc_dev.bdev.dv_unit);
> -
> + ether_fakeaddr(ifp);
> printf(": address %s\n", ether_sprintf(sc->sc_arpcom.ac_enaddr));
>
> - ifp = GET_IFP(sc);
> ifp->if_softc = sc;
> ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> ifp->if_ioctl = cdcef_ioctl;
> Index: if_ugl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_ugl.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 if_ugl.c
> --- if_ugl.c 25 Nov 2015 11:20:38 -0000 1.18
> +++ if_ugl.c 7 Apr 2016 14:44:04 -0000
> @@ -152,8 +152,6 @@ int ugldebug = 0;
> #define DPRINTFN(n,x)
> #endif
>
> -extern int ticks;
> -
> /*
> * Various supported device vendors/products.
> */
> @@ -210,11 +208,10 @@ ugl_attach(struct device *parent, struct
> int s;
> struct usbd_device *dev = uaa->device;
> struct usbd_interface *iface = uaa->iface;
> - struct ifnet *ifp;
> + struct ifnet *ifp = GET_IFP(sc);
> usb_interface_descriptor_t *id;
> usb_endpoint_descriptor_t *ed;
> int i;
> - u_int16_t macaddr_hi;
>
> DPRINTFN(5,(" : ugl_attach: sc=%p, dev=%p", sc, dev));
>
> @@ -251,16 +248,11 @@ ugl_attach(struct device *parent, struct
>
> s = splnet();
>
> - macaddr_hi = htons(0x2acb);
> - bcopy(&macaddr_hi, &sc->sc_arpcom.ac_enaddr[0], sizeof(u_int16_t));
> - bcopy(&ticks, &sc->sc_arpcom.ac_enaddr[2], sizeof(u_int32_t));
> - sc->sc_arpcom.ac_enaddr[5] = (u_int8_t)(sc->sc_dev.dv_unit);
> -
> + ether_fakeaddr(ifp);
> printf("%s: address %s\n",
> sc->sc_dev.dv_xname, ether_sprintf(sc->sc_arpcom.ac_enaddr));
>
> /* Initialize interface info.*/
> - ifp = GET_IFP(sc);
> ifp->if_softc = sc;
> ifp->if_hardmtu = UGL_MAX_MTU;
> ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
>
