this review refers to the webrev at:
http://cr.grommit.com/~zf162725/cr_0410_all/

usr/src/lib/libdladm/common/libdlwlan.c:

892:
you need to set DLADM_WLAN_LINKATTR_WLAN in la_valid
before goto done.

906:
why not add a new WL_ENC_WPA type?

1753:
rename to 'do_set_key'


2032-2086:
this code is still wrong.
you cannot blindly assume that the dladm_wlan_* structures are
identical to your wl_* ioctl structures.
at the very least, your code must do something like:

dladm_status_t
dladm_wlan_wpa_xxx(const char *link, dladm_wlan_xxx_t *xxx)
{
        wl_xxx_t wl_xxx;

        /*
         * for basic types (e.g. int)
         */
        wl_xxx.a = xxx->a;
        wl_xxx.b = xxx->b;

        /*
         * for types that require translation.
         */
        switch (xxx->c) {
        case DLADM_WLAN_XXX_D1:
                wl_xxx.c = WL_XXX_D1;
                break;
        case DLADM_WLAN_XXX_D2:
                wl_xxx.c = WL_XXX_D2;
                break;
        }

        ...

        return (ioctl_set(link, WL_XXX, &wl_xxx, sizeof (wl_xxx));

}
note that the fields a, b, c in dladm_wlan_xxx_t *must* refer to
either basic types or #defines in libdlwlan.h only.

also, it may be simpler to pass a, b, c as arguments rather than
using a structure.

similar changes are needed for dladm_wlan_wpa_getsr() as well.
(i.e. dladm_wlan_ess_t must not be assumed to be the same size as
 struct wpa_ess.


usr/src/cmd/cmd-inet/usr.lib/wpad/driver_wifi.c:

63: must check la_valid for DLADM_WLAN_LINKATTR_WLAN before
    using la_wlan_attr
64: must check wl_attrp->wa_valid for DLADM_WLAN_ATTR_BSSID before
    using wl_attrp->wa_bssid.

93-94: must check la_valid and wa_valid before using wa_essid.

114-116, 119-122: these lines could be moved into dladm_wlan_wpa_setie().
wpa_driver_wifi_set_wpa_ie() would become:

static int
wpa_driver_wifi_set_wpa_ie(const char *ifname,
    const char *wpa_ie, uint32_t wpa_ie_len)
{
        int     ret;

        (void) pthread_mutex_lock(&gbuf_1k);
        ret = dladm_wlan_wpa_setie(ifname, ie, wpa_ie_len);
        (void) pthread_mutex_unlock(&gbuf_1k);

        return (ret == DLADM_STATUS_OK ? 0 : -1);

}
btw, why do you need the gbuf_1k mutex?
remove it if it's not needed. your daemon should able to
call dladm_wlan_wpa_setie() directly.

144: enabled should be boolean_t
152: setting ret = -1 is useless here because of the assignment at 157.
157: just change dladm_wlan_wpa_setwpa() to:
     dladm_wlan_wpa_setwpa(const char *, boolean_t);
     this will let you delete 146,155-156.


177: dladm_wlan_wpa_delkey() should become:
     dladm_wlan_wpa_delkey(const char *, int, unsigned char *);
     this will let you delete 166,173-176.

217: this function needs to be modified according to my comment
     regarding libdlwlan.c:2032-2086 above.

     specifically:
     268-269,271: these fields must be a DLADM_WLAN_* type, not a IEEE80211 
type.
     280: you could remove the wk structure and pass individual arguments to
          dladm_wlan_set_wpa_setkey(), whose signature would become something 
like:

          dladm_wlan_set_wpa_setkey(const char *ifname, dladm_wpa_cipher_t 
cipher,
             unsigned char *addr, int key_idx, boolean_t set_tx, uint8_t *seq,
             uint32_t seq_len, uint8_t *key, uint32_t key_len);

306: change signature to:
     dladm_wlan_wpa_disassociate(const char *ifname, dladm_wpa_reason_t reason);
     and delete 304-305.

346: this seems useless.
354: change signature to:
     dladm_wlan_wpa_associate(const char *ifname, const char *bssid,
         const char *wpa_ie, uint32_t wpa_ie_len);
     and delete 349-353.
_______________________________________________
networking-discuss mailing list
[EMAIL PROTECTED]

Reply via email to