On Fri, Oct 04, 2019 at 03:24:45PM -0700, John Baldwin wrote: > On 10/4/19 1:48 PM, Kyle Evans wrote: > > On Fri, Oct 4, 2019 at 2:12 PM John Baldwin <j...@freebsd.org> wrote: > >> > >> On 10/4/19 6:43 AM, Kyle Evans wrote: > >>> Author: kevans > >>> Date: Fri Oct 4 13:43:07 2019 > >>> New Revision: 353103 > >>> URL: https://svnweb.freebsd.org/changeset/base/353103 > >>> > >>> Log: > >>> tuntap(4): loosen up tunclose restrictions > >>> > >>> Realistically, this cannot work. We don't allow the tun to be opened > >>> twice, > >>> so it must be done via fd passing, fork, dup, some mechanism like these. > >>> Applications demonstrably do not enforce strict ordering when they're > >>> handing off tun devices, so the parent closing before the child will > >>> easily > >>> leave the tun/tap device in a bad state where it can't be destroyed and > >>> a > >>> confused user because they did nothing wrong. > >>> > >>> Concede that we can't leave the tun/tap device in this kind of state > >>> because > >>> of software not playing the TUNSIFPID game, but it is still good to > >>> find and > >>> fix this kind of thing to keep ifconfig(8) up-to-date and help ensure > >>> good > >>> discipline in tun handling. > >> > >> Why are you using d_close for last close anyway? It's not really reliable > >> compared > >> to using cdevpriv and a cdevpriv dtor. > > Sorry, right after I sent this I realized that is probably just the old code. > > > This decision predates me by a long time, I'm afraid. =-) > > > > If you have time to elaborate on the comparable reliability point, I'd > > be interested in hearing it. A little bit of searching didn't seem to > > turn up much there, I'm afraid. > > There are certain edge cases (e.g. if d_open() fails part-way through IIRC, > but > I think at least one other) where d_close() may not get invoked. OTOH, once > the > cdevpriv dtor is installed, it will always be called when the reference count > on > the associated struct file drops to zero. If you need to reliably free > resources created during open(), then the cdevpriv dtor is the best way to > manage that. > > d_close() can still be useful for dealing with conditions that aren't "this > file > descriptor has completely gone away" since close() can have defined semantics > on > ttys as Bruce has noted. Most non-tty devices don't honor revoke(), and it's > not > fully clear to me if revoke() really makes sense on non-tty devices. (E.g. > what > does it mean to revoke a swap partition, /dev/null, or /dev/random) Without > revoke() I think you avoid many of the complexities from close semantics. Unfortunately not. The same complications are created by multi-mounts of devfs and forced unmount.
You never know that d_close() is really last, and D_TRACKCLOSE is unable to provide the guarantee that each open is matched. > > > I did otherwise spend a little bit of time diving into the path taken > > to get to d_close and the trade-offs between cdevpriv vs. what tuntap > > does now. I think I'm convinced either way that cdevpriv is a good way > > to go- it seems to have the advantage that with a little refactoring > > we could actually set the softc atomically on the device cdevpriv > > instead of cdev->si_drv1 and I can axe this rwatson@ comment about the > > non-atomic test and set. > > So the si_drv1 thing doesn't require cdevpriv, that is just a matter of using > make_dev_s() instead of make_dev(). Any driver that uses si_drv1 should > really > be using make_dev_s() to close the race of setting si_drv1 during cdev > creation. > > -- > John Baldwin _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
