On Sat, Jan 03, 2026 at 06:44:33AM +0300, Vitaliy Makkoveev wrote: > On Sat, Jan 03, 2026 at 06:29:19AM +0300, Vitaliy Makkoveev wrote: > > I propose to take netlock status instead of `cmd' handrolling. This > > relocking is XXX in any case. > > > > Compile test only. > > > > Please drop previous. We can't know do we hold the shared netlock or > not, so handle SIOC*MEDIA and SIOCGIFSFFPAGE cases manually.
These locking dances are ugly and difficult to maintain. When we change ioctl(2) locking in the future, drivers may suddenly fail. Like in this case. And we do not have hardware for systematic testing. At Nov 14 I sent a diff to remove net lock from cad(4). https://marc.info/?l=openbsd-bugs&m=176314127606140&w=2 I don't know if it works, I cannot test. Is anyone on this list, who posseses the hardware, interested having it fixed? Please try my diff from Nov 14. Sending and receiving traffic while doing ifconfig cad up/down, ifconfig media change, plugging/unplugging may give interesting effects in this aerea. bluhm > Index: sys/dev/fdt/if_cad.c > =================================================================== > RCS file: /cvs/src/sys/dev/fdt/if_cad.c,v > retrieving revision 1.16 > diff -u -p -r1.16 if_cad.c > --- sys/dev/fdt/if_cad.c 17 Sep 2025 09:17:12 -0000 1.16 > +++ sys/dev/fdt/if_cad.c 3 Jan 2026 03:38:34 -0000 > @@ -589,22 +589,44 @@ cad_ioctl(struct ifnet *ifp, u_long cmd, > { > struct cad_softc *sc = ifp->if_softc; > struct ifreq *ifr = (struct ifreq *)data; > - int error = 0, netlock_held = 1; > + int error = 0, netlock_status; > int s; > > + /* > + * XXXSMP: We can't predict the netlock status. We can't > + * know, are we the holder of shared netlock. > + */ > switch (cmd) { > case SIOCGIFMEDIA: > case SIOCSIFMEDIA: > case SIOCGIFSFFPAGE: > - netlock_held = 0; > + netlock_status = 0; > + break; > + default: > + netlock_status = rw_status(&netlock); > break; > } > > - if (netlock_held) > + switch (netlock_status) { > + case RW_WRITE: > NET_UNLOCK(); > + break; > + case RW_READ: > + NET_UNLOCK_SHARED(); > + break; > + } > + > rw_enter_write(&sc->sc_cfg_lock); > - if (netlock_held) > + > + switch (netlock_status) { > + case RW_WRITE: > NET_LOCK(); > + break; > + case RW_READ: > + NET_LOCK_SHARED(); > + break; > + } > + > s = splnet(); > > switch (cmd) { > >
