> > http://cr.opensolaris.org/~seb/clearview-iptun-cr3/ > > > > uts/common/inet/iptun/iptundummy.c: > > > > * 99-100: Not due to your wad, but it seems like this should be > > done by ipcl_conn_create(). > > DEFER: I started to move this into ipcl_conn_create() and this turned > out to be a bit more complicated than a simple code refactor. In some > cases where ipcl_conn_create(), a cred isn't handy and isn't obtained > until later (e.g. tcp_get_conn()), which makes this a bit more involved. > I'd rather this bit of strangeness be refactored some other time.
Sure. > > * 233-234: Is this just paranoia? If we're really dependent > > on iptun_count(), then what prevents iptun_tunnelcount changing > > immediately after the check, given that we're not holding any > > locks? > > This isn't paranoia, but prevents the pseudo device from detaching (and > the driver from unloading) when there are tunnels configured. We don't > need to hold a lock here because the tunnel count is only changed when a > tunnel is created or deleted, which can't happen while the detach > routine is running (the ioctl path calls ddi_hold_devi_by_instance() in > drv_ioctl(), and the /dev/net implicit path has the device open). I see. I think this warrants a comment. > > uts/common/inet/iptun/iptun_ctl.c: > > > > * 27-31: Comment seems only tangentially related to the source. > > ACCEPT: The iptun.c comments describe the module at a high-level. > Modified the comment to: > /* > * This file implements the ioctl control path for the iptun driver. The > * GLDv3 dld_ioc_register() mechanism is used to register iptun ioctls with > * the dld module. > */ Looks good. -- meem
