Adding DisassociateReason as dbus property
Avichal Agarwal
avichal.a
Sun Jun 14 23:21:27 PDT 2015
Thanks Dan
I will apply these changes and make this patch again
and reply.
Avichal Agarwal
------- Original Message -------
Sender : Dan Williams<dcbw at redhat.com>
Date : Jun 13, 2015 03:10 (GMT+05:30)
Title : Re: Adding DisassociateReason as dbus property
On Tue, 2015-06-02 at 09:49 +0000, Avichal Agarwal wrote:
> From 8e90e66d3497b959906f28e7e75d80cf94f7e44b Mon Sep 17 00:00:00 2001
> From: Avichal Agarwal
> Date: Tue, 2 Jun 2015 15:00:50 +0530
> Subject: [PATCH] Adding disassociate reason property in dbus
> it gives bssid and status as string
> implemented for both wlan and p2p
It looks like whitespace has been collapsed in this patch; make sure you
send them as "preformatted" or if that just never works, as a text file
attachment.
> Signed-off-by: Avichal Agarwal
> ---
> wpa_supplicant/dbus/dbus_new.c | 25 +++++++++++++++++++++++++
> wpa_supplicant/dbus/dbus_new.h | 2 ++
> wpa_supplicant/dbus/dbus_new_handlers.c | 22 ++++++++++++++++++++++
> wpa_supplicant/dbus/dbus_new_handlers.h | 3 +++
> wpa_supplicant/events.c | 16 +++++++++++++---
> wpa_supplicant/notify.c | 11 +++++++++++
> wpa_supplicant/notify.h | 1 +
> 7 files changed, 77 insertions(+), 3 deletions(-)
>
> diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
> index 6382d77..0aaceea 100644
> --- a/wpa_supplicant/dbus/dbus_new.c
> +++ b/wpa_supplicant/dbus/dbus_new.c
> @@ -1852,15 +1852,32 @@ void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
> prop = "DisconnectReason";
> flush = TRUE;
> break;
> + case WPAS_DBUS_PROP_DISASSOCIATE_REASON:
> + prop = "DisassociateReason";
> + flush = TRUE;
> + break;
> + case WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON:
> + prop = "DisassociateReasonP2P";
> + flush = TRUE;
> + break;
> default:
> wpa_printf(MSG_ERROR, "dbus: %s: Unknown Property value %d",
> __func__, property);
> return;
> }
>
> + if(wpa_s->p2p_mgmt)
> + {
> + wpa_dbus_mark_property_changed(wpa_s->global->dbus,
> + wpa_s->dbus_new_path,
> + WPAS_DBUS_NEW_IFACE_P2PDEVICE, prop);
> + }
> + else
> + {
> wpa_dbus_mark_property_changed(wpa_s->global->dbus,
> wpa_s->dbus_new_path,
> WPAS_DBUS_NEW_IFACE_INTERFACE, prop);
> + }
> if (flush) {
> wpa_dbus_flush_object_changed_properties(
> wpa_s->global->dbus->con, wpa_s->dbus_new_path);
> @@ -2950,11 +2967,19 @@ static const struct wpa_dbus_property_desc wpas_dbus_interface_properties[] = {
> wpas_dbus_getter_persistent_groups,
> NULL
> },
> + { "DisassociateReasonP2P", WPAS_DBUS_NEW_IFACE_P2PDEVICE, "s",
> + wpas_dbus_getter_disassociate_reason,
> + NULL
> + },
I think these should be a struct '(si)' to better match the Disconnect
reason. The 's' part would be the BSSID of the rejector. The 'i' would
be the 802.11 status code. The point here is to make this as easy for
clients to process as possible, so that they don't have to do a bunch of
string manipulation. D-Bus has strongly typed arguments and we should
use them :)
> #endif /* CONFIG_P2P */
> { "DisconnectReason", WPAS_DBUS_NEW_IFACE_INTERFACE, "i",
> wpas_dbus_getter_disconnect_reason,
> NULL
> },
> + { "DisassociateReason", WPAS_DBUS_NEW_IFACE_INTERFACE, "s",
> + wpas_dbus_getter_disassociate_reason,
> + NULL
> + },
Same here.
> { NULL, NULL, NULL, NULL, NULL }
> };
>
> diff --git a/wpa_supplicant/dbus/dbus_new.h b/wpa_supplicant/dbus/dbus_new.h
> index 31db8d4..3fe0160 100644
> --- a/wpa_supplicant/dbus/dbus_new.h
> +++ b/wpa_supplicant/dbus/dbus_new.h
> @@ -29,6 +29,8 @@ enum wpas_dbus_prop {
> WPAS_DBUS_PROP_CURRENT_AUTH_MODE,
> WPAS_DBUS_PROP_BSSS,
> WPAS_DBUS_PROP_DISCONNECT_REASON,
> + WPAS_DBUS_PROP_DISASSOCIATE_REASON,
> + WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON,
> };
>
> enum wpas_dbus_bss_prop {
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
> index 97db9a8..1c3dda8 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> @@ -2771,6 +2771,28 @@ dbus_bool_t wpas_dbus_getter_disconnect_reason(DBusMessageIter *iter,
> &reason, error);
> }
>
> +/**
> + * wpas_dbus_getter_disassociate_reason -
> + *Get bssid with status code in string format (bssid="" status_code="") for ASSOC_REJECT
> + * @iter: Pointer to incoming dbus message iter
> + * @error: Location to store error on failure
> + * @user_data: Function specific data
> + * Returns: TRUE on success, FALSE on failure
> + *
> + * Getter for "disassociateReason" property.
> + */
> +dbus_bool_t wpas_dbus_getter_disassociate_reason(DBusMessageIter *iter,
> + DBusError *error,
> + void *user_data)
> +{
> + struct wpa_supplicant *wpa_s = user_data;
> + char* reason = wpa_s->diassoc_reason_buff;
> +
> + return wpas_dbus_simple_property_getter(iter, DBUS_TYPE_STRING,
> + &reason, error);
Since the property would be "(si)" now it's actually a struct instead of
a simple property. That's fine, but requires a bit more work in the
getter. I think this is the right way to do it:
DBusMessageIter variant_iter;
DBusMessageIter struct_iter;
if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
"(si)", &variant_iter) ||
!dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT,
NULL, &struct_iter) ||
!dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_INT32,
&wpa_s->disassoc_reason) ||
!dbus_message_iter_append_basic(&struct_iter, DBUS_TYPE_STRING,
&wpa_s->disassoc_bssid) ||
!dbus_message_iter_close_container(iter, &struct_iter) ||
!dbus_message_iter_close_container(iter, &variant_iter)) {
dbus_set_error(error, DBUS_ERROR_FAILED,
"%s: error constructing reply variant", _func__);
return FALSE;
}
return TRUE;
see below for more comments on this.
> +}
> +
> +
>
> /**
> * wpas_dbus_getter_bss_expire_age - Get BSS entry expiration age
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.h b/wpa_supplicant/dbus/dbus_new_handlers.h
> index 9afdc05..fc67a4e 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.h
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.h
> @@ -169,6 +169,9 @@ dbus_bool_t wpas_dbus_setter_fast_reauth(DBusMessageIter *iter,
> dbus_bool_t wpas_dbus_getter_disconnect_reason(DBusMessageIter *iter,
> DBusError *error,
> void *user_data);
> +dbus_bool_t wpas_dbus_getter_disassociate_reason(DBusMessageIter *iter,
> + DBusError *error,
> + void *user_data);
>
> dbus_bool_t wpas_dbus_getter_bss_expire_age(DBusMessageIter *iter,
> DBusError *error, void *user_data);
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index d086624..1d09d08 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -3294,15 +3294,25 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
> break;
> #endif /* CONFIG_IBSS_RSN */
> case EVENT_ASSOC_REJECT:
> + os_memset(wpa_s->diassoc_reason_buff ,0,100);
> if (data->assoc_reject.bssid)
> + {
> + os_snprintf(wpa_s->diassoc_reason_buff ,50,"bssid="MACSTR "status_code=%u",MAC2STR(data->assoc_reject.bssid),data->assoc_reject.status_code);
> wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_ASSOC_REJECT
> - "bssid=" MACSTR " status_code=%u",
> - MAC2STR(data->assoc_reject.bssid),
> - data->assoc_reject.status_code);
> + "bssid=" MACSTR " status_code=%u",
> + MAC2STR(data->assoc_reject.bssid),
> + data->assoc_reject.status_code);
> + }
> else
> + {
> + os_snprintf(wpa_s->diassoc_reason_buff ,15,"status_code=%u",data->assoc_reject.status_code);
> wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_ASSOC_REJECT
> "status_code=%u",
> data->assoc_reject.status_code);
> + }
So instead of building up a string here in wpa_s->disassoc_reason_buff
you'd have two items in wpa_s: (1) disassoc_reason (the 802.11 status
code) and (2) disassoc_bssid[20] (holds string format of BSSID).
Dan
> +
> + wpas_notify_diassociate_reason(wpa_s);
> +
> if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_SME)
> sme_event_assoc_reject(wpa_s, data);
> else {
> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
> index 4df9d90..cd80b96 100644
> --- a/wpa_supplicant/notify.c
> +++ b/wpa_supplicant/notify.c
> @@ -116,6 +116,17 @@ void wpas_notify_disconnect_reason(struct wpa_supplicant *wpa_s)
> wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_DISCONNECT_REASON);
> }
>
> +void wpas_notify_diassociate_reason(struct wpa_supplicant *wpa_s)
> +{
> + if (wpa_s->p2p_mgmt)
> + {
> + wpas_dbus_signal_prop_changed(wpa_s, WPAS_P2P_DBUS_PROP_DISASSOCIATE_REASON);
> + }
> + else
> + {
> + wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_DISASSOCIATE_REASON);
> + }
> +}
>
> void wpas_notify_network_changed(struct wpa_supplicant *wpa_s)
> {
> diff --git a/wpa_supplicant/notify.h b/wpa_supplicant/notify.h
> index 1025ca8..5e2d340 100644
> --- a/wpa_supplicant/notify.h
> +++ b/wpa_supplicant/notify.h
> @@ -23,6 +23,7 @@ void wpas_notify_state_changed(struct wpa_supplicant *wpa_s,
> enum wpa_states new_state,
> enum wpa_states old_state);
> void wpas_notify_disconnect_reason(struct wpa_supplicant *wpa_s);
> +void wpas_notify_diassociate_reason(struct wpa_supplicant *wpa_s);
> void wpas_notify_network_changed(struct wpa_supplicant *wpa_s);
> void wpas_notify_ap_scan_changed(struct wpa_supplicant *wpa_s);
> void wpas_notify_bssid_changed(struct wpa_supplicant *wpa_s);
> --
> 1.7.9.5
> > ------- Original Message -------
> > Sender : Avichal Agarwal Lead Engineer (2)/SRI-Delhi-Advanced SW Team/Samsung Electronics
> > Date : Jun 01, 2015 11:51 (GMT+05:30)
> > Title : [PATCH] Adding some important wlan cotnrol events as dbus signals
> >
> >
> > From 51363ead405042ca466a418b4676086ec060de4c Mon Sep 17 00:00:00 2001
> >
> > Following events are added as dbus signals , this patch will help application
> > to make quick decisions based on events , as happens in wpa ctrl_socket
> > based application. This make dbus interface fast.
> > Now no more waithing for state change notifications.
>
> If making things 'fast' is your goal, then instead of adding new events
> lets talk about how to make 'state' changes faster. Currently they are
> queued and coalesced in wpas_dbus_signal_prop_changed(), so instead of
> adding new/duplicate events I would test adding a "flush = TRUE" for the
> "State" property and see if that fixes your speed issues.
>
> But in general, the D-Bus interface is not supposed to be a direct
> mirror of the control socket interface, especially with names. The
> D-Bus interface uses conventional D-Bus naming, which is StudlyCaps
> format without "-" dividers.
>
> > 1. WPA-CONNECTED
> > 2. WPA-DISCONNECTED
>
> These two are already communicated by the "state" property, and making
> them "faster" (if slow is the problem) is worthwhile.
>
> > 3. WPA-ASSOCREJECT
>
> This one doesn't have a corresponding event yet, and would be useful.
> However, I don't think having a "WPA-ASSOCREJECT" signal is the right
> way to do it. However, since the state property doesn't indicate a
> reason, we've worked around this with the "DisconnectReason" property.
> Perhaps we could add a "DisassociateReason" that includes the 802.11
> status code instead.
>
> > 4. WPA-TERMINATING
>
> For D-Bus a TERMINATING event is not relevant, as supplicant termination
> is indicated already by D-Bus and the NameOwnerChanged signal.
>
> > 5. WPA-SCAN-STARTED
> > 6. WPA-SCAN-RESULTS
>
> SCAN-STARTED/RESULTS is already indicated by the 'scanning' property and
> the ScanDone signal, and the various BSS signals (BSSAdded, BSSRemoved,
> and the PropertiesChanged signal on existing BSSes) that are emitted
> during the scan. Scanning is also coalesced so perhaps we should just
> "flush = TRUE" for it.
>
> > 7. WPA-SCAN-FAILED
>
> This is already indicated by the 'success' parameter to the ScanDone
> signal.
>
> > 8. WPS-EVENT-OVERLAP
>
> This one needs an event still too, and we should figure out how to
> expose it.
>
> Dan
More information about the Hostap
mailing list