On Tue, Dec 03, 2013 at 12:47:30AM +0200, Paul Irofti wrote:
> Could people doing actual networking with urndis(4) please test the
> following diff and report if any regressions are encountered?
>
> I plan on commiting it by the end of the week if I don't hear anything
> bad in regards to the patch.
>
To be more specific, this diff fixes a panic on detach when connecting
my phone to an OpenBSD machine.
The ctrl methods on attach only use control messages and don't need any
pipe besides the default pipe which is always present.
Thus the urndis_init() and, in turn, urndis_stop() funcalls are not
needed during attach.
Besides from that they're also poorly written as they can fail and that
is not checked nor reported. This driver is in really poor shape.
This diff just brings this awful driver closer to what other usb
ethernet drivers are doing.
Others diffs will probably follow in order to clean-up the current mess.
> Index: if_urndis.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_urndis.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 if_urndis.c
> --- if_urndis.c 21 Nov 2013 14:08:05 -0000 1.44
> +++ if_urndis.c 28 Nov 2013 10:22:42 -0000
> @@ -1363,6 +1363,7 @@ urndis_attach(struct device *parent, str
> sc = (void *)self;
> uaa = aux;
>
> + sc->sc_attached = 0;
> sc->sc_udev = uaa->device;
> id = usbd_get_interface_descriptor(uaa->iface);
> sc->sc_ifaceno_ctl = id->bInterfaceNumber;
> @@ -1447,14 +1448,11 @@ urndis_attach(struct device *parent, str
>
> IFQ_SET_READY(&ifp->if_snd);
>
> - urndis_init(sc);
> -
> s = splnet();
>
> if (urndis_ctrl_query(sc, OID_802_3_PERMANENT_ADDRESS, NULL, 0,
> &buf, &bufsz) != RNDIS_STATUS_SUCCESS) {
> printf(": unable to get hardware address\n");
> - urndis_stop(sc);
> splx(s);
> return;
> }
> @@ -1466,7 +1464,6 @@ urndis_attach(struct device *parent, str
> } else {
> printf(", invalid address\n");
> free(buf, M_TEMP);
> - urndis_stop(sc);
> splx(s);
> return;
> }
> @@ -1478,7 +1475,6 @@ urndis_attach(struct device *parent, str
> if (urndis_ctrl_set(sc, OID_GEN_CURRENT_PACKET_FILTER, &filter,
> sizeof(filter)) != RNDIS_STATUS_SUCCESS) {
> printf("%s: unable to set data filters\n", DEVNAME(sc));
> - urndis_stop(sc);
> splx(s);
> return;
> }
>
