[PATCH] supplicant: add dbus getter method for nl80211 iftype

[Jouni Malinen]

On Thu, Nov 06, 2014 at 12:44:14PM +0530, Avinash Patil wrote:
> This patch adds dbus getter method for nl80211 iftype.
> This is required by certain applications which intend to start
> AP operations only if current interface type is AP.
> Getter method for capabilities cannot be used for this purpose as
> this enumarates all the supported interface types.
> 
> Patch also adds notification handlers for interface type change
> events.

This has significant whitespace damage in the inline patch.. Could you
please resend with those fixed? For example:

checking file src/drivers/driver_nl80211.c
patch: **** malformed patch at line 196: *priv, u8 *bssid)

> diff --git a/src/common/defs.h b/src/common/defs.h
> @@ -297,6 +297,24 @@ enum wpa_ctrl_req_type {
>         NUM_WPA_CTRL_REQS
>  };
> 
> +enum wpa_nl80211_iftype {

nl80211 should not really show up in any way in defs.h. In addition, it
is a bit strange to see interface types defined here, i.e., I would have
expected to see this in src/drivers/driver.h.

> +       WPA_IFTYPE_UNSPECIFIED,
..

> +       /* keep last */
> +       WPA_NL80211_IFTYPES,
> +       WPA_IFTYPE_MAX = WPA_NL80211_IFTYPES - 1

What would these be used for? There was nothing using these in the
patch..


> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> @@ -1427,6 +1427,18 @@ struct wpa_driver_ops {
>         /**
> +        * get_iftype - Get the current NL80211 iftype

driver.h is supposed to define a driver independent interface. This
should not be documented as being specific to nl80211.

> +       int (*get_iftype)(void *priv, u8 *iftype);

u8 * ??

Shouldn't this be that enum wpa_nl80211_iftype (or well, renamed to
something not nl80211 specific) and the enum defined in this file?

> +static int wpa_driver_nl80211_get_nl80211_iftype(void *priv, u8 *iftype)
> +{
> +       struct i802_bss *bss = priv;
> +       if (!bss)
> +               return -1;
> +
> +       *iftype = (u8)nl80211_get_ifmode(bss);
> +
> +       return 0;
> +}

This would need to map driver interface (nl80211) specific values to
generic values defined in driver.h. That said, I'm not sure I really
understand why we would expose every nl80211 iftype to external programs
through wpa_supplicant. What is the real need for that information? Is
it just something like "is this interface configured in AP mode"? I'd
assume something much simpler would cover that need..

> +       if(wpa_drv_get_iftype(wpa_s, &iftype)) {

"if (wpa_drv..."

> +       char_iftype =  wpa_supplicant_iftype_txt(iftype);

"char_iftype = wpa_..."

> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index 6761c1a..3a075c8 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -182,9 +182,12 @@ void wpa_supplicant_stop_countermeasures(void
> *eloop_ctx, void *sock_ctx)
>  void wpa_supplicant_mark_disassoc(struct wpa_supplicant *wpa_s)
>  {
>         int bssid_changed;
> -
> +       u8 old_iftype = 0, new_iftype = 0;
>         wnm_bss_keep_alive_deinit(wpa_s);

Should not move that empty line in that way.. And why use u8 for
something that seemed to get an enum defined for it?

> +       if(wpa_drv_get_iftype(wpa_s, &new_iftype)) {
> +               wpa_dbg(wpa_s, MSG_DEBUG, "Failed to get new iftype!");
> +       }
> +       if (new_iftype != old_iftype)
> +               wpas_notify_interface_type_changed(wpa_s->global);

Why would interface type be reported to change if wpa_drv_get_iftype()
failed?

-- 
Jouni Malinen                                            PGP id EFC895FA