Hi Folks, Here are my comments:
usr/src/cmd/dlstat/dlstat.c usr/src/cmd/flowstat/flowstat.c * General: These work in non-global zones, correct? usr/src/uts/common/io/dld/dld_drv.c * 436: You removed the zoneid checks here, why? This means that any zone can see every other zones' datalinks' ring grouping information, which is a leakage of information across zone boundaries is it not? usr/src/uts/common/io/mac/mac_sched.c * 464: Tracking down where this hint comes from is quite difficult. Some more comments about where this 64-bit value comes from would be helpful. It looks like in most cases, it comes from CONN_TO_XMIT_HINT() or ILL_RING_TO_XMIT_HINT(), which both go out of their way to cast the pointers used as hints to 32-bit values. Why? Also, given the HASH_HINT() algorithm, I don't see how those macros' bit-shifting magic does anything useful. I have the same question for the similar code in aggr_send.c. * 482, 507, 534: The MAC name isn't the same as the link name. I suspect that things may be broken with renamed links. usr/src/uts/common/io/mac/mac.c * 323: Comments are needed to explain what this taskq is used for. * 2844: It looks like this function should be checking the size of all public properties (given how it's used in dld_drv.c), but this isn't the complete set of public properties... For example, you removed the property size checks out of iptun.c, but did not replace that code with any checks for those properties anywhere else. What about WiFi properties etc...? * 3526: typo, remove "be". * 3905: among * 6294: close parens * 6502: Need blank line between variable decl. and code. * 7324: What prevents the mip from being destroyed right before you call i_mac_perim_enter() here? Comment please. usr/src/uts/common/io/mac/mac_stat.c * 312: Using kstat_create() means that this stat will be visible by all zones, which is likely not what is intended for statistics associated with objects that only exist in one particular zone. See kstat_create_zone(). There is also a pre-existing bug of the same nature at line 795. usr/src/uts/common/io/mac/mac_protect.c * General: It may be a good idea to assert that appropriate mutexes are held in the various routines that directly access the avl trees (e.g. insert_dhcpv4_pending_txn() and similar functions) since they don't have any locking themselves. * 136: Remove extra empty comment line. * 144: This is trivial to implement (increment a statistic when the limit is hit), why not do that now? * 297: Can valid DHCP packets be fragmented? If so, then why would you reject them? If you reject such packets, then there could be a security hole as link protection will silently be inactive. * 748: In the IPv4 case (at line 297), you reject IP fragments but you don't do anything equivalent here for IPv6. If there's a valid reason to reject fragments (which is the subject of my previous comment), then you should do so consistently. usr/src/uts/common/io/mac/mac_util.c * 473: What are these diffs against? The onnv version of this function has two more arguments (ip_fragmented and ip_frag_ident), neither of which are in the left side of the diffs... Color me confused. -Seb
