Hi Seb, I reviewed the mac related changes and glanced over iptun.c. I only have a few comments:
http://cr.opensolaris.org/~seb/clearview-iptun-cr2/ aggr_grp.c: 1428: you could change aggr_grp_delete() to pass in a cred_t so you don't have to use CRED(). same issue with vnic_dev.c:589. dld_drv.c: 1272: is this missing something? dld_proto.c: 843: why is the initialization needed? 868: why is there no (void) cast in front of this call? (since this returns a boolean_t). dls_mgmt.c: 786: is this because the zone holds an extra reference so dls_devnet_rename() can't reach here? 813: shouldn't the last arg be (mod_hash_val_t *)&ddp? 886: the ddp could've been freed at 877 so this could access freed memory. 1148: this assert doesn't seem valid all the time. (e.g. if the daemon dies and couldn't handle the upcall) 1105: I don't quite understand why you need to clear this flag. i_dls_devnet_destroy_iptun() will call dls_devnet_destroy()-> dls_devnet_unset() which does not re-enter dls_devnet_rele() so iptun_delete() won't be called again. the comments also refers to a non-existent function dls_devnet_unset_common(). 1758: why doesn't this use CRED() like iptun_create()? mac_ipv4.c: 195,196: do you need to sanity check these lengths? iptun.c: 214: why not return an error if iptun_task_dispatch() fails? 1046: why is IPTUN_SRC/DST always cleared? if iptun already has a valid addr, shouldn't that be kept even after an attempt to set an invalid one? 1488: the done label seems unused. 2017: add check for MBLKL(mp) > (ipha_t + icmp_t) or (ip6_t + icmp6_t)? 2059: add check for first_mblkl is > ipha_t? 2424: possibly more header size checks needed. 2571: may be better to break up the chain to get the actual drop count. eric
