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: [EMAIL PROTECTED]
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


_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to