I propose to avoid netlock relocking for SIOCGIFSFFPAGE too. Also, since the relock should be avoided for many commands, I prefer to have something like below:
switch(cmd){ case SIOCGIFMEDIA: case ...: relock = 1; break; } if (relock) NET_UNLOCK(); > On 14 Aug 2022, at 22:09, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: > > On Sun, Aug 14 2022, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: >> On Thu, Jul 28 2022, Alexander Bluhm <alexander.bl...@gmx.net> wrote: >>> Hi, >>> >>> The netlock for SIOCSIFMEDIA and SIOCGIFMEDIA ioctl is not necessary. >>> Legacy drivers run with kernel lock, interface media is MP safe or >>> has kernel lock. >>> >>> ixl(4) talks about net lock but in fact has kernel lock. >>> >>> Problem is that smtpd(8) periodically checks media status. This >>> stops network if netlock was taken. >>> >>> ok? >> >> I'm seeing fallout with cad(4) in the RAMDISK and GENERIC.MP kernels >> (didn't try GENERIC). RAMDISK after >> >> OpenBSD 7.2-beta (RAMDISK) #140: Sat Aug 6 02:06:57 MDT 2022 >> >> hangs some time after mounting root. On GENERIC.MP I get a panic: >> >> --8<-- >> Automatic boot in progress: starting file system checks. >> /dev/sd0a (c92bac01c037a788.a): file system is clean; not checking >> /dev/sd0m (c92bac01c037a788.m): file system is clean; not checking >> /dev/sd0j (c92bac01c037a788.j): file system is clean; not checking >> /dev/sd0d (c92bac01c037a788.d): file system is clean; not checking >> /dev/sd0e (c92bac01c037a788.e): file system is clean; not checking >> /dev/sd0f (c92bac01c037a788.f): file system is clean; not checking >> /dev/sd0g (c92bac01c037a788.g): file system is clean; not checking >> /dev/sd0h (c92bac01c037a788.h): file system is clean; not checking >> /dev/sd0k (c92bac01c037a788.k): file system is clean; not checking >> /dev/sd0l (c92bac01c037a788.l): file system is clean; not checking >> /dev/sd0o (c92bac01c037a788.o): file system is clean; not checking >> /dev/sd0n (c92bac01c037a788.n): file system is clean; not checking >> /dev/sd0p (c92bac01c037a788.p): file system is clean; not checking >> pf enabled >> kern.bufcachepercent: 20 -> 80 >> sysctl: kern.allowdt: value is not available >> ddb.console: 0 -> 1 >> starting network >> panic: netlock: lock not held >> Stopped at panic+0x106: addi a0,zero,256 TID PID UID >> PR >> FLAGS PFLAGS CPU COMMAND >> *464996 1081 0 0x3 0 0 ifconfig >> 492269 29052 0 0x14000 0x200 1 zerothread >> panic() at panic+0x106 >> rw_exit_write() at rw_exit_write+0x94 >> cad_ioctl() at cad_ioctl+0x34 >> ifioctl() at ifioctl+0xaaa >> soo_ioctl() at soo_ioctl+0x152 >> sys_ioctl() at sys_ioctl+0x230 >> svc_handler() at svc_handler+0x284 >> do_trap_user() at do_trap_user+0x15c >> cpu_exception_handler_user() at cpu_exception_handler_user+0x7c >> end of kernel >> end trace frame: 0x5408b2ae8, count: 6 >> https://www.openbsd.org/ddb.html describes the minimum info required in bug >> reports. Insufficient info makes it difficult to find and fix bugs. >> >> -->8-- >> >> The diff below fixes GENERIC.MP, I have yet to build and try RAMDISK. > > Done, the diff fixes bsd.rd too. > >> I'm not completely sure this is the best way to solve it but I can't >> spot other uses of ifp->if_ioctl() not protected by the NET_LOCK() in >> ifioctl(). Except SIOCGIFSFFPAGE but that seems fine... > > Thoughts? ok? > >> Index: dev/fdt/if_cad.c >> =================================================================== >> RCS file: /home/cvs/src/sys/dev/fdt/if_cad.c,v >> retrieving revision 1.11 >> diff -u -p -r1.11 if_cad.c >> --- dev/fdt/if_cad.c 8 Mar 2022 16:13:08 -0000 1.11 >> +++ dev/fdt/if_cad.c 14 Aug 2022 13:04:41 -0000 >> @@ -553,9 +553,11 @@ cad_ioctl(struct ifnet *ifp, u_long cmd, >> int error = 0; >> int s; >> >> - NET_UNLOCK(); >> + if (cmd != SIOCGIFMEDIA && cmd != SIOCSIFMEDIA) >> + NET_UNLOCK(); >> rw_enter_write(&sc->sc_cfg_lock); >> - NET_LOCK(); >> + if (cmd != SIOCGIFMEDIA && cmd != SIOCSIFMEDIA) >> + NET_LOCK(); >> s = splnet(); >> >> switch (cmd) { > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE