On Sat, Jan 03, 2026 at 02:34:38PM +0100, Alexander Bluhm wrote:
> 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
I did test your diff on my rpi5. It did survive rcctl start syncthing
(I could reliably reproduce the panic with that command before),
ifconfig up/down, ifconfig media change, and cable
unplugging/plugging.
Following the fixed diff; taskq_del() -> task_del().
Index: sys/dev/fdt/if_cad.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/if_cad.c,v
diff -u -p -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 20:00:22 -0000
@@ -589,22 +589,10 @@ 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;
int s;
- switch (cmd) {
- case SIOCGIFMEDIA:
- case SIOCSIFMEDIA:
- case SIOCGIFSFFPAGE:
- netlock_held = 0;
- break;
- }
-
- if (netlock_held)
- NET_UNLOCK();
rw_enter_write(&sc->sc_cfg_lock);
- if (netlock_held)
- NET_LOCK();
s = splnet();
switch (cmd) {
@@ -722,9 +710,6 @@ cad_up(struct cad_softc *sc)
rw_assert_wrlock(&sc->sc_cfg_lock);
- /* Release lock for memory allocation. */
- NET_UNLOCK();
-
if (sc->sc_dma64)
flags |= BUS_DMA_64BIT;
@@ -880,8 +865,6 @@ cad_up(struct cad_softc *sc)
}
}
- NET_LOCK();
-
/*
* Set MAC address filters.
*/
@@ -985,9 +968,6 @@ cad_down(struct cad_softc *sc)
ifq_clr_oactive(&ifp->if_snd);
ifp->if_timer = 0;
- /* Avoid lock order issues with barriers. */
- NET_UNLOCK();
-
timeout_del_barrier(&sc->sc_tick);
/* Disable data transfer. */
@@ -1011,7 +991,7 @@ cad_down(struct cad_softc *sc)
/* Wait for activity to cease. */
intr_barrier(sc->sc_ih);
ifq_barrier(&ifp->if_snd);
- taskq_del_barrier(systq, &sc->sc_statchg_task);
+ task_del(systq, &sc->sc_statchg_task);
/* Disable the packet clock as it is not needed any longer. */
clock_disable(sc->sc_node, GEM_CLK_TX);
@@ -1059,8 +1039,6 @@ cad_down(struct cad_softc *sc)
cad_dmamem_free(sc, sc->sc_rxring);
sc->sc_rxring = NULL;
sc->sc_rxdesc = NULL;
-
- NET_LOCK();
}
uint8_t
@@ -1683,8 +1661,14 @@ void
cad_statchg_task(void *arg)
{
struct cad_softc *sc = arg;
+ struct ifnet *ifp = &sc->sc_ac.ac_if;
- clock_set_frequency(sc->sc_node, GEM_CLK_TX, sc->sc_tx_freq);
+ rw_enter_write(&sc->sc_cfg_lock);
+
+ if ((ifp->if_flags & IFF_RUNNING))
+ clock_set_frequency(sc->sc_node, GEM_CLK_TX, sc->sc_tx_freq);
+
+ rw_exit_write(&sc->sc_cfg_lock);
}
struct cad_dmamem *
