Groups | Search | Server Info | Login | Register

Groups > muc.lists.netbsd.tech.net > #5909

Breaking a usbnet deadlock with async multicast filter updates

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

- usbnet core lock
- kpause / running callouts
- softnet_lock

The deadlock manifests through SIOCADDMULTI/SIOCDELMULTI, which unlike
all other ioctls sometimes run under the short-term packet-processing
lock softnet_lock and not the long-term configuration lock IFNET_LOCK:

- ure detaches and holds the usbnet core lock over kpause in ure_reset
  which awaits softint softclock `lock' to wake callouts
- callout holds the softint softclock `lock' and awaits softnet_lock
  (e.g., in frag6_fasttimo or any various other network callouts)
- mdnsd holds softnet_lock and awaits usbnet core lock in
  ure_ioctl(SIOCDELMULTI)

The attached patch adapts usbnet(9) to split SIOCADDMULTI/SIOCDELMULTI
handling into two stages:

1. Synchronously update the ethercom's multicast address list with
   ether_ioctl/addmulti/delmulti.

2. Defer updating the hardware's multicast filter to an asynchronous
   USB task.

This breaks the deadlock by making usbnet's SIOCADDMULTI/SIOCDELMULTI
never take the usbnet core lock.

However, it may delay updating the hardware's multicast filter a
little bit -- the filter won't be updated synchronously by the time
if_mcast_op returns.

Is this delay a problem?

I would assume no -- it seems like the worst possible outcome is that
we might lose a race with a few packets that get dropped on the floor
because the hardware's multicast filter wasn't told to accept them
yet, or we might lose a race and get a few packets that the hardware
was supposed to ignore, but neither of these seems particularly bad to
me.

But I don't understand this stuff very well so perhaps my assessment
is wrong.


Another approach would be to use a different lock to exclude
SIOCADDMULTI/SIOCDELMULTI -- maybe a short-term lock (never held over
kpause, safe to take under softnet_lock) together with a flag that
says `skip updating the hardware filter because reset (init or stop)
is in progress and init will do it before bringing the interface back
up anyway'.

Perhaps the usbnet core lock is the right lock for that -- after all,
we already have IFNET_LOCK as a long-term configuration change lock --
but it will require some more substantial changes to all the usbnet
drivers so they _don't_ hold the usbnet core lock over device reset
waits.  Certainly the asynchronous update approach will make pullups
to netbsd-9 much easier.

Back to muc.lists.netbsd.tech.net | Previous | Next — Next in thread | Find similar

Thread

Breaking a usbnet deadlock with async multicast filter updates Taylor R Campbell <riastradh@NetBSD.org> - 2021-12-30 21:03 +0000
  Re: Breaking a usbnet deadlock with async multicast filter updates Greg Troxel <gdt@lexort.com> - 2021-12-30 19:19 -0500

csiph-web