Hi meem, Thanks for the review. I summarized your comments. Below is my response to your comments. Please let me know if you have any additional questions.
-- Quaker ===================================================== REVIEWER: Peter.Memishian at Sun.COM WEBREV: http://cr.grommit.com/~zf162725/cr_0308/ http://cr.grommit.com/~zf162725/cr_0307/ FILES: GLDv3 Userland + GLDv3 Kernel NOTES: Description of feedbacks: ACCEPT Request accepted REJECT Request rejected EXPLAIN Explanation given DISCUSS Request requires further discussion to resolve DEFER Request deferred (e.g. because work is out-of-scope) ===================================================== usr/src/uts/common/net/wpa.h: * Place in HDRS in uts/common/net/Makefile. This will cause it to be installed into $ROOT/usr/include/net during the nightly build. | ACCEPT * Add exceptions for usr/include/net/wpa.h into pkgdefs/etc/exception_list_{i386,sparc}. Put them near the PPP exceptions since those are also for usr/include/net. | ACCEPT * Remove change to CPPFLAGS in libwladm/Makefile.com. | ACCEPT usr/src/uts/common/io/dld/dld_drv.c: * Remove #include of <cmn_err.h>. | ACCEPT usr/src/uts/common/io/mac/plugins/mac_wifi.c: * 245: Add blank line. | ACCEPT * 292-299: Seems slightly simpler as: if (wh->i_fc[1] & IEEE80211_FC1_WEP) { llcp += IEEE80211_WEP_IVLEN + IEEE80211_WEP_KIDLEN; if (wdp->wd_secalloc == WIFI_SEC_WPA) llcp += IEEE80211_WEP_EXTIVLEN; } | ACCEPT usr/src/lib/libdladm/common/secobj.c: * Changes at the end of secobj.c seem needless. | ACCEPT | removed the needless changes. usr/src/lib/libwladm/common/libwladm.h: * might be safer to put wk_class at the end of the struct, in case there are unbundled things (from opensolaris.org?) that know about the existing struct. | ACCEPT * 192: s/IOCLT/IOCTL/ | ACCEPT usr/src/cmd/dladm/dladm.c: * Change to require -s with -k (lines 2230-2238) doesn't seem right, and I think it will break existing usage of dladm connect-wifi. | ACCEPT | Remove the requirement of -s with -k. * 2783-2793: What does this have to do with WPA? Seems like a bugfix. Need to file a bug for it (though it can be putback with WPA if that's convenient). | ACCEPT | It's the temporary code to test another problem, removed. * convert_secobj() changes seem like they could be simpler -- e.g., just add: if (class == DLADM_SECOBJ_CLASS_WPA) { if (len < 8 || len > 63) return (EINVAL); (void) memcpy(obj_val, buf, len); *obj_lenp = len; return (0); } ... before the existing `if (class != DLADM_SECOBJ_CLASS_WEP)' | ACCEPT uts/common/sys/ethernet.h: * 97-99: Please merge these constants in with the rest of the ETHERTYPE_* values. (They're all sorted in numeric order.) | ACCEPT cmd/cmd-inet/usr.lib/wpad/l2_packet.c: * 53: Comment says that DL_PROMISC_PHYS needs to be enabled, but the code doesn't do that. (But why would DL_PROMISC_PHYS be needed? Is the comment just wrong?) | ACCEPT | The comments is wrong, removed. * 106: This will massively over-allocate: uint64_t buf[IEEE80211_MTU_MAX]; /* aligned on 64-bit boundary */ You need: uint64_t buf[IEEE80211_MTU_MAX / sizeof (uint64_t)]; | ACCEPT
