dbus: Expose connected stations on D-Bus

From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>

Make it possible to list connected stations in AP mode over D-Bus, along
with some of their properties: rx/tx packets, bytes, capabilities, etc.

Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>

Rebased by Julian Andres Klode <juliank@ubuntu.com> and updated to use
the new getter API.

Further modified by Andrej Shadura to not error out when not in AP mode.

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
 wpa_supplicant/dbus/dbus_new.c          | 245 +++++++++++++++++--
 wpa_supplicant/dbus/dbus_new.h          |  25 ++
 wpa_supplicant/dbus/dbus_new_handlers.c | 313 ++++++++++++++++++++++++
 wpa_supplicant/dbus/dbus_new_handlers.h |  14 ++
 wpa_supplicant/notify.c                 |   6 +
 5 files changed, 589 insertions(+), 14 deletions(-)

On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
> 
> Make it possible to list connected stations in AP mode over D-Bus,
> along
> with some of their properties: rx/tx packets, bytes, capabilities,
> etc.

It looks like this patch changes the signal signature of
StaAuthorized/StaDeauthorized, is that correct?  That won't be
backwards compatible with existing D-Bus clients and we should probably
choose new signal names to allow old clients to continue working.

Also, you'll want to update doc/dbus.doxygen with any changes you make
to the D-Bus API, including additions.

Dan

> Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-
> lapierre@canonical.com>
> 
> Rebased by Julian Andres Klode <juliank@ubuntu.com> and updated to
> use
> the new getter API.
> 
> Further modified by Andrej Shadura to not error out when not in AP
> mode.
> 
> Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
> ---
>  wpa_supplicant/dbus/dbus_new.c          | 245 +++++++++++++++++--
>  wpa_supplicant/dbus/dbus_new.h          |  25 ++
>  wpa_supplicant/dbus/dbus_new_handlers.c | 313
> ++++++++++++++++++++++++
>  wpa_supplicant/dbus/dbus_new_handlers.h |  14 ++
>  wpa_supplicant/notify.c                 |   6 +
>  5 files changed, 589 insertions(+), 14 deletions(-)
> 
> diff --git a/wpa_supplicant/dbus/dbus_new.c
> b/wpa_supplicant/dbus/dbus_new.c
> index e0f16bbda..4139131e9 100644
> --- a/wpa_supplicant/dbus/dbus_new.c
> +++ b/wpa_supplicant/dbus/dbus_new.c
> @@ -25,6 +25,7 @@
>  #include "dbus_new_handlers_p2p.h"
>  #include "p2p/p2p.h"
>  #include "../p2p_supplicant.h"
> +#include "ap/sta_info.h"
>  
>  #ifdef CONFIG_AP /* until needed by something else */
>  
> @@ -1016,15 +1017,19 @@ void wpas_dbus_signal_eap_status(struct
> wpa_supplicant *wpa_s,
>   * Notify listeners about event related with station
>   */
>  static void wpas_dbus_signal_sta(struct wpa_supplicant *wpa_s,
> -				 const u8 *sta, const char
> *sig_name)
> +				 const u8 *sta, const char
> *sig_name,
> +                                 int properties)

This is just a bool, so maybe dbus_bool_t would be more appropriate.

>  {
>  	struct wpas_dbus_priv *iface;
>  	DBusMessage *msg;
> -	char sta_mac[WPAS_DBUS_OBJECT_PATH_MAX];
> -	char *dev_mac;
> +	DBusMessageIter iter;
> +	char sta_obj_path[WPAS_DBUS_OBJECT_PATH_MAX];
> +	char *path;
>  
> -	os_snprintf(sta_mac, WPAS_DBUS_OBJECT_PATH_MAX, MACSTR,
> MAC2STR(sta));
> -	dev_mac = sta_mac;
> +	os_snprintf(sta_obj_path, WPAS_DBUS_OBJECT_PATH_MAX,
> +		    "%s/" WPAS_DBUS_NEW_STAS_PART "/"
> COMPACT_MACSTR,
> +		    wpa_s->dbus_new_path, MAC2STR(sta));
> +	path = sta_obj_path;
>  
>  	iface = wpa_s->global->dbus;
>  
> @@ -1037,15 +1042,28 @@ static void wpas_dbus_signal_sta(struct
> wpa_supplicant *wpa_s,
>  	if (msg == NULL)
>  		return;
>  
> -	if (dbus_message_append_args(msg, DBUS_TYPE_STRING,
> &dev_mac,
> -				     DBUS_TYPE_INVALID))
> -		dbus_connection_send(iface->con, msg, NULL);
> -	else
> -		wpa_printf(MSG_ERROR, "dbus: Failed to construct
> signal");
> +	dbus_message_iter_init_append(msg, &iter);
> +	if (!dbus_message_iter_append_basic(&iter,
> DBUS_TYPE_OBJECT_PATH,
> +					    &path))
> +		goto err;
> +
> +	if (properties) {
> +		if (!wpa_dbus_get_object_properties(iface, path,
> +						    WPAS_DBUS_NEW_IF
> ACE_STA,
> +						    &iter))
> +			goto err;
> +	}
> +
> +	wpa_printf(MSG_DEBUG, "dbus: Station MAC address '" MACSTR
> "' '%s'",
> +		   MAC2STR(sta), sig_name);
> +
> +	dbus_connection_send(iface->con, msg, NULL);
>  	dbus_message_unref(msg);
> +	return;
>  
> -	wpa_printf(MSG_DEBUG, "dbus: Station MAC address '%s' '%s'",
> -		   sta_mac, sig_name);
> +err:
> +	wpa_printf(MSG_ERROR, "dbus: Failed to construct signal");
> +	dbus_message_unref(msg);
>  }
>  
>  
> @@ -1059,7 +1077,7 @@ static void wpas_dbus_signal_sta(struct
> wpa_supplicant *wpa_s,
>  void wpas_dbus_signal_sta_authorized(struct wpa_supplicant *wpa_s,
>  				     const u8 *sta)
>  {
> -	wpas_dbus_signal_sta(wpa_s, sta, "StaAuthorized");
> +	wpas_dbus_signal_sta(wpa_s, sta, "StaAuthorized", TRUE);
>  }
>  
>  
> @@ -1073,7 +1091,7 @@ void wpas_dbus_signal_sta_authorized(struct
> wpa_supplicant *wpa_s,
>  void wpas_dbus_signal_sta_deauthorized(struct wpa_supplicant *wpa_s,
>  				       const u8 *sta)
>  {
> -	wpas_dbus_signal_sta(wpa_s, sta, "StaDeauthorized");
> +	wpas_dbus_signal_sta(wpa_s, sta, "StaDeauthorized", FALSE);
>  }
>  
>  
> @@ -2151,6 +2169,9 @@ void wpas_dbus_signal_prop_changed(struct
> wpa_supplicant *wpa_s,
>  	case WPAS_DBUS_PROP_BSSS:
>  		prop = "BSSs";
>  		break;
> +	case WPAS_DBUS_PROP_STAS:
> +		prop = "Stations";
> +		break;
>  	case WPAS_DBUS_PROP_CURRENT_AUTH_MODE:
>  		prop = "CurrentAuthMode";
>  		break;
> @@ -2243,6 +2264,39 @@ void wpas_dbus_bss_signal_prop_changed(struct
> wpa_supplicant *wpa_s,
>  }
>  
>  
> +/**
> + * wpas_dbus_sta_signal_prop_changed - Signals change of STA
> property
> + * @wpa_s: %wpa_supplicant network interface data
> + * @property: indicates which property has changed
> + * @address: unique BSS identifier
> + *
> + * Sends PropertyChanged signals with path, interface, and arguments
> depending
> + * on which property has changed.
> + */
> +void wpas_dbus_sta_signal_prop_changed(struct wpa_supplicant *wpa_s,
> +				       enum wpas_dbus_bss_prop
> property,
> +				       u8 address[ETH_ALEN])
> +{
> +	char path[WPAS_DBUS_OBJECT_PATH_MAX];
> +	char *prop;
> +
> +	switch (property) {
> +	case WPAS_DBUS_STA_PROP_ADDRESS:
> +		prop = "Address";
> +		break;
> +	default:
> +		wpa_printf(MSG_ERROR, "dbus: %s: Unknown Property
> value %d",
> +			   __func__, property);
> +		return;
> +	}
> +
> +	os_snprintf(path, WPAS_DBUS_OBJECT_PATH_MAX,
> +		    "%s/" WPAS_DBUS_NEW_STAS_PART "/"
> COMPACT_MACSTR,
> +		    wpa_s->dbus_new_path, MAC2STR(address));
> +
> +	wpa_dbus_mark_property_changed(wpa_s->global->dbus, path,
> +				       WPAS_DBUS_NEW_IFACE_STA,
> prop);
> +}
>  /**
>   * wpas_dbus_signal_debug_level_changed - Signals change of debug
> param
>   * @global: wpa_global structure
> @@ -2857,6 +2911,164 @@ err:
>  }
>  
>  
> +static const struct wpa_dbus_property_desc
> wpas_dbus_sta_properties[] = {
> +	{ "Address", WPAS_DBUS_NEW_IFACE_STA, "ay",
> +	  wpas_dbus_getter_sta_address,
> +	  NULL
> +	},
> +	{ "AID", WPAS_DBUS_NEW_IFACE_STA, "q",
> +	  wpas_dbus_getter_sta_aid,
> +	  NULL
> +	},
> +	{ "Flags", WPAS_DBUS_NEW_IFACE_STA, "u",
> +	  wpas_dbus_getter_sta_flags,
> +	  NULL
> +	},
> +	{ "Capabilities", WPAS_DBUS_NEW_IFACE_STA, "q",
> +	  wpas_dbus_getter_sta_caps,
> +	  NULL
> +	},
> +	{ "RxPackets", WPAS_DBUS_NEW_IFACE_STA, "t",
> +	  wpas_dbus_getter_sta_rx_packets,
> +	  NULL
> +	},
> +	{ "TxPackets", WPAS_DBUS_NEW_IFACE_STA, "t",
> +	  wpas_dbus_getter_sta_tx_packets,
> +	  NULL
> +	},
> +	{ "RxBytes", WPAS_DBUS_NEW_IFACE_STA, "t",
> +	  wpas_dbus_getter_sta_rx_bytes,
> +	  NULL
> +	},
> +	{ "TxBytes", WPAS_DBUS_NEW_IFACE_STA, "t",
> +	  wpas_dbus_getter_sta_tx_bytes,
> +	  NULL
> +	},
> +	{ NULL, NULL, NULL, NULL, NULL }
> +};
> +
> +
> +static const struct wpa_dbus_signal_desc wpas_dbus_sta_signals[] = {
> +	/* Deprecated: use
> org.freedesktop.DBus.Properties.PropertiesChanged */
> +	{ "PropertiesChanged", WPAS_DBUS_NEW_IFACE_STA,
> +	  {
> +		  { "properties", "a{sv}", ARG_OUT },
> +		  END_ARGS
> +	  }
> +	},
> +	{ NULL, NULL, { END_ARGS } }
> +};
> +
> +
> +/**
> + * wpas_dbus_unregister_sta - Unregister a connected station from
> dbus
> + * @wpa_s: wpa_supplicant interface structure
> + * @bssid: connected station bssid
> + * @id: unique station identifier
> + * Returns: 0 on success, -1 on failure
> + *
> + * Unregisters STA representing object from dbus
> + */
> +int wpas_dbus_unregister_sta(struct wpa_supplicant *wpa_s,
> +			     const u8 *sta)
> +{
> +	struct wpas_dbus_priv *ctrl_iface;
> +	char sta_obj_path[WPAS_DBUS_OBJECT_PATH_MAX];
> +
> +	/* Do nothing if the control interface is not turned on */
> +	if (wpa_s == NULL || wpa_s->global == NULL)
> +		return 0;
> +	ctrl_iface = wpa_s->global->dbus;
> +	if (ctrl_iface == NULL)
> +		return 0;
> +
> +	os_snprintf(sta_obj_path, WPAS_DBUS_OBJECT_PATH_MAX,
> +		    "%s/" WPAS_DBUS_NEW_STAS_PART "/"
> COMPACT_MACSTR,
> +		    wpa_s->dbus_new_path, MAC2STR(sta));
> +
> +	wpa_printf(MSG_DEBUG, "dbus: Unregister STA object '%s'",
> +		   sta_obj_path);
> +	if (wpa_dbus_unregister_object_per_iface(ctrl_iface,
> sta_obj_path)) {
> +		wpa_printf(MSG_ERROR, "dbus: Cannot unregister STA
> object %s",
> +			   sta_obj_path);
> +		return -1;
> +	}
> +
> +	wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_STAS);
> +
> +	return 0;
> +}
> +
> +
> +/**
> + * wpas_dbus_register_sta - Register a scanned station with dbus
> + * @wpa_s: wpa_supplicant interface structure
> + * @bssid: connection network station
> + * @id: unique STA identifier
> + * Returns: 0 on success, -1 on failure
> + *
> + * Registers STA representing object with dbus
> + */
> +int wpas_dbus_register_sta(struct wpa_supplicant *wpa_s,
> +			   const u8 *sta)
> +{
> +	struct wpas_dbus_priv *ctrl_iface;
> +	struct wpa_dbus_object_desc *obj_desc;
> +	char sta_obj_path[WPAS_DBUS_OBJECT_PATH_MAX];
> +	struct sta_handler_args *arg;
> +
> +	/* Do nothing if the control interface is not turned on */
> +	if (wpa_s == NULL || wpa_s->global == NULL)
> +		return 0;
> +	ctrl_iface = wpa_s->global->dbus;
> +	if (ctrl_iface == NULL)
> +		return 0;
> +
> +	os_snprintf(sta_obj_path, WPAS_DBUS_OBJECT_PATH_MAX,
> +		    "%s/" WPAS_DBUS_NEW_STAS_PART "/"
> COMPACT_MACSTR,
> +		    wpa_s->dbus_new_path, MAC2STR(sta));
> +
> +	obj_desc = os_zalloc(sizeof(struct wpa_dbus_object_desc));
> +	if (!obj_desc) {
> +		wpa_printf(MSG_ERROR, "Not enough memory "
> +			   "to create object description");
> +		goto err;
> +	}
> +
> +	arg = os_zalloc(sizeof(struct sta_handler_args));
> +	if (!arg) {
> +		wpa_printf(MSG_ERROR, "Not enough memory "
> +			   "to create arguments for handler");
> +		goto err;
> +	}
> +	arg->wpa_s = wpa_s;
> +	arg->sta = sta;
> +
> +	wpas_dbus_register(obj_desc, arg, wpa_dbus_free,
> +			   NULL,
> +			   wpas_dbus_sta_properties,
> +			   wpas_dbus_sta_signals);
> +
> +	wpa_printf(MSG_DEBUG, "dbus: Register STA object '%s'",
> +		   sta_obj_path);
> +	if (wpa_dbus_register_object_per_iface(ctrl_iface,
> sta_obj_path,
> +					       wpa_s->ifname,
> obj_desc)) {
> +		wpa_printf(MSG_ERROR,
> +			   "Cannot register STA dbus object %s.",
> +			   sta_obj_path);
> +		goto err;
> +	}
> +
> +	wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_STAS);
> +
> +	return 0;
> +
> +err:
> +	free_dbus_object_desc(obj_desc);
> +	return -1;
> +}
> +
> +
>  static const struct wpa_dbus_method_desc
> wpas_dbus_interface_methods[] = {
>  	{ "Scan", WPAS_DBUS_NEW_IFACE_INTERFACE,
>  	  (WPADBusMethodHandler) wpas_dbus_handler_scan,
> @@ -3501,6 +3713,11 @@ static const struct wpa_dbus_property_desc
> wpas_dbus_interface_properties[] = {
>  	  NULL
>  	},
>  #endif /* CONFIG_MESH */
> +	{ "Stations", WPAS_DBUS_NEW_IFACE_INTERFACE, "ao",
> +	  wpas_dbus_getter_stas,
> +	  NULL,
> +	  NULL
> +	},
>  	{ NULL, NULL, NULL, NULL, NULL, NULL }
>  };
>  
> diff --git a/wpa_supplicant/dbus/dbus_new.h
> b/wpa_supplicant/dbus/dbus_new.h
> index e68acb7a1..f434cc147 100644
> --- a/wpa_supplicant/dbus/dbus_new.h
> +++ b/wpa_supplicant/dbus/dbus_new.h
> @@ -12,6 +12,7 @@
>  
>  #include "common/defs.h"
>  #include "p2p/p2p.h"
> +#include "ap/sta_info.h"
>  
>  struct wpa_global;
>  struct wpa_supplicant;
> @@ -29,6 +30,7 @@ enum wpas_dbus_prop {
>  	WPAS_DBUS_PROP_CURRENT_NETWORK,
>  	WPAS_DBUS_PROP_CURRENT_AUTH_MODE,
>  	WPAS_DBUS_PROP_BSSS,
> +	WPAS_DBUS_PROP_STAS,
>  	WPAS_DBUS_PROP_DISCONNECT_REASON,
>  	WPAS_DBUS_PROP_ASSOC_STATUS_CODE,
>  };
> @@ -46,6 +48,10 @@ enum wpas_dbus_bss_prop {
>  	WPAS_DBUS_BSS_PROP_AGE,
>  };
>  
> +enum wpas_dbus_sta_prop {
> +	WPAS_DBUS_STA_PROP_ADDRESS,
> +};
> +
>  #define WPAS_DBUS_OBJECT_PATH_MAX 150
>  
>  #define WPAS_DBUS_NEW_SERVICE		"fi.w1.wpa_supplicant1"
> @@ -62,6 +68,9 @@ enum wpas_dbus_bss_prop {
>  #define WPAS_DBUS_NEW_BSSIDS_PART "BSSs"
>  #define WPAS_DBUS_NEW_IFACE_BSS	WPAS_DBUS_NEW_INTERFACE
> ".BSS"
>  
> +#define WPAS_DBUS_NEW_STAS_PART "Stations"
> +#define WPAS_DBUS_NEW_IFACE_STA	WPAS_DBUS_NEW_INTERFACE
> ".Station"
> +
>  #define WPAS_DBUS_NEW_IFACE_P2PDEVICE	\
>  		WPAS_DBUS_NEW_IFACE_INTERFACE ".P2PDevice"
>  
> @@ -164,6 +173,10 @@ int wpas_dbus_unregister_bss(struct
> wpa_supplicant *wpa_s,
>  			     u8 bssid[ETH_ALEN], unsigned int id);
>  int wpas_dbus_register_bss(struct wpa_supplicant *wpa_s,
>  			   u8 bssid[ETH_ALEN], unsigned int id);
> +int wpas_dbus_unregister_sta(struct wpa_supplicant *wpa_s,
> +			     const u8 *sta);
> +int wpas_dbus_register_sta(struct wpa_supplicant *wpa_s,
> +			   const u8 *sta);
>  void wpas_dbus_signal_blob_added(struct wpa_supplicant *wpa_s,
>  				 const char *name);
>  void wpas_dbus_signal_blob_removed(struct wpa_supplicant *wpa_s,
> @@ -346,6 +359,18 @@ static inline int wpas_dbus_register_bss(struct
> wpa_supplicant *wpa_s,
>  	return 0;
>  }
>  
> +static inline int wpas_dbus_unregister_sta(struct wpa_supplicant
> *wpa_s,
> +					   const u8 *sta)
> +{
> +	return 0;
> +}
> +
> +static inline int wpas_dbus_register_sta(struct wpa_supplicant
> *wpa_s,
> +					 const u8 *sta)
> +{
> +	return 0;
> +}
> +
>  static inline void wpas_dbus_signal_blob_added(struct wpa_supplicant
> *wpa_s,
>  					       const char *name)
>  {
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c
> b/wpa_supplicant/dbus/dbus_new_handlers.c
> index a3c98fadd..01a5bdb86 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> @@ -22,6 +22,10 @@
>  #include "../bss.h"
>  #include "../scan.h"
>  #include "../autoscan.h"
> +#include "../ap.h"
> +#include "ap/hostapd.h"
> +#include "ap/sta_info.h"
> +#include "ap/ap_drv_ops.h"
>  #include "dbus_new_helpers.h"
>  #include "dbus_new.h"
>  #include "dbus_new_handlers.h"
> @@ -3854,6 +3858,315 @@ dbus_bool_t wpas_dbus_setter_iface_global(
>  }
>  
>  
> +/**
> + * wpas_dbus_getter_stas - Get connected stations for an interface
> + * @iter: Pointer to incoming dbus message iter
> + * @error: Location to store error on failure
> + * @user_data: Function specific data
> + * Returns: a list of stations
> + *
> + * Getter for "Stations" property.
> + */
> +dbus_bool_t wpas_dbus_getter_stas(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct wpa_supplicant *wpa_s = user_data;
> +	struct hostapd_data *hapd;
> +	struct sta_info *sta = NULL;
> +	char **paths = NULL;
> +	unsigned int i = 0, num = 0;
> +	dbus_bool_t success = FALSE;
> +
> +	if (!wpa_s->dbus_new_path) {
> +		dbus_set_error(error, DBUS_ERROR_FAILED,
> +			       "%s: no D-Bus interface", __func__);
> +		return FALSE;
> +	}
> +
> +	if (wpa_s->ap_iface) {
> +		hapd = wpa_s->ap_iface->bss[0];
> +		sta = hapd->sta_list;
> +		num = hapd->num_sta;
> +	}
> +
> +	paths = os_calloc(num, sizeof(char *));
> +	if (!paths) {
> +		dbus_set_error_const(error, DBUS_ERROR_NO_MEMORY,
> "no memory");
> +		return FALSE;
> +	}
> +
> +	/* Loop through scan results and append each result's object
> path */
> +	for (; sta; sta = sta->next) {
> +		paths[i] = os_zalloc(WPAS_DBUS_OBJECT_PATH_MAX);
> +		if (paths[i] == NULL) {
> +			dbus_set_error_const(error,
> DBUS_ERROR_NO_MEMORY,
> +					     "no memory");
> +			goto out;
> +		}
> +		/* Construct the object path for this BSS. */
> +		os_snprintf(paths[i++], WPAS_DBUS_OBJECT_PATH_MAX,
> +			    "%s/" WPAS_DBUS_NEW_STAS_PART "/"
> COMPACT_MACSTR,
> +			    wpa_s->dbus_new_path, MAC2STR(sta-
> >addr));
> +	}
> +
> +	success = wpas_dbus_simple_array_property_getter(iter,
> +							 DBUS_TYPE_O
> BJECT_PATH,
> +							 paths, num,
> +							 error);
> +
> +out:
> +	while (i)
> +		os_free(paths[--i]);
> +	os_free(paths);
> +	return success;
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_sta_address - Return the BSSID of a connected
> station
> + * @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 "Address" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_address(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +
> +	sta = ap_get_sta(args->wpa_s->ap_iface->bss[0], args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_array_property_getter(iter,
> DBUS_TYPE_BYTE,
> +						      sta->addr,
> ETH_ALEN,
> +						      error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_sta_aid - Return the AID of a connected station
> + * @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 "AID" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_aid(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +
> +	sta = ap_get_sta(args->wpa_s->ap_iface->bss[0], args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT16,
> +						&sta->aid,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_sta_flags - Return the flags of a connected
> station
> + * @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 "Flags" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_flags(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +
> +	sta = ap_get_sta(args->wpa_s->ap_iface->bss[0], args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT32,
> +						&sta->flags,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_sta_caps - Return the capabilities of a station
> + * @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 "Capabilities" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_caps(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +
> +	sta = ap_get_sta(args->wpa_s->ap_iface->bss[0], args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT16,
> +						&sta->capability,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_rx_packets - Return the received packets for a
> station
> + * @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 "RxPackets" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_rx_packets(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +	struct hostap_sta_driver_data data;
> +	struct hostapd_data *hapd;
> +
> +	if (!args->wpa_s->ap_iface)
> +		return FALSE;
> +
> +	hapd = args->wpa_s->ap_iface->bss[0];
> +	sta = ap_get_sta(hapd, args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +        if (hostapd_drv_read_sta_data(hapd, &data, sta->addr) < 0)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT64,
> +						&data.rx_packets,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_tx_packets - Return the transmitted packets for
> a station
> + * @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 "TxPackets" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_tx_packets(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +	struct hostap_sta_driver_data data;
> +	struct hostapd_data *hapd;
> +
> +	if (!args->wpa_s->ap_iface)
> +		return FALSE;
> +
> +	hapd = args->wpa_s->ap_iface->bss[0];
> +	sta = ap_get_sta(hapd, args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +        if (hostapd_drv_read_sta_data(hapd, &data, sta->addr) < 0)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT64,
> +						&data.tx_packets,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_tx_bytes - Return the transmitted bytes for a
> station
> + * @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 "TxBytes" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_tx_bytes(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +	struct hostap_sta_driver_data data;
> +	struct hostapd_data *hapd;
> +
> +	if (!args->wpa_s->ap_iface)
> +		return FALSE;
> +
> +	hapd = args->wpa_s->ap_iface->bss[0];
> +	sta = ap_get_sta(hapd, args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +        if (hostapd_drv_read_sta_data(hapd, &data, sta->addr) < 0)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT64,
> +						&data.tx_bytes,
> +						error);
> +}
> +
> +
> +/**
> + * wpas_dbus_getter_rx_bytes - Return the received bytes for a
> station
> + * @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 "RxBytes" property.
> + */
> +dbus_bool_t wpas_dbus_getter_sta_rx_bytes(
> +	const struct wpa_dbus_property_desc *property_desc,
> +	DBusMessageIter *iter, DBusError *error, void *user_data)
> +{
> +	struct sta_handler_args *args = user_data;
> +	struct sta_info *sta;
> +	struct hostap_sta_driver_data data;
> +	struct hostapd_data *hapd;
> +
> +	if (!args->wpa_s->ap_iface)
> +		return FALSE;
> +
> +	hapd = args->wpa_s->ap_iface->bss[0];
> +	sta = ap_get_sta(hapd, args->sta);
> +	if (!sta)
> +		return FALSE;
> +
> +        if (hostapd_drv_read_sta_data(hapd, &data, sta->addr) < 0)
> +		return FALSE;
> +
> +	return wpas_dbus_simple_property_getter(iter,
> DBUS_TYPE_UINT64,
> +						&data.rx_bytes,
> +						error);
> +}
> +
> +
>  static struct wpa_bss * get_bss_helper(struct bss_handler_args
> *args,
>  				       DBusError *error, const char
> *func_name)
>  {
> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.h
> b/wpa_supplicant/dbus/dbus_new_handlers.h
> index 26652ad3d..9a4f66171 100644
> --- a/wpa_supplicant/dbus/dbus_new_handlers.h
> +++ b/wpa_supplicant/dbus/dbus_new_handlers.h
> @@ -22,6 +22,11 @@ struct bss_handler_args {
>  	unsigned int id;
>  };
>  
> +struct sta_handler_args {
> +	struct wpa_supplicant *wpa_s;
> +	const u8 *sta;
> +};
> +
>  dbus_bool_t wpas_dbus_simple_property_getter(DBusMessageIter *iter,
>  					     const int type,
>  					     const void *val,
> @@ -168,6 +173,15 @@ DECLARE_ACCESSOR(wpas_dbus_getter_networks);
>  DECLARE_ACCESSOR(wpas_dbus_getter_pkcs11_engine_path);
>  DECLARE_ACCESSOR(wpas_dbus_getter_pkcs11_module_path);
>  DECLARE_ACCESSOR(wpas_dbus_getter_blobs);
> +DECLARE_ACCESSOR(wpas_dbus_getter_stas);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_address);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_aid);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_flags);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_caps);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_rx_packets);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_tx_packets);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_tx_bytes);
> +DECLARE_ACCESSOR(wpas_dbus_getter_sta_rx_bytes);
>  DECLARE_ACCESSOR(wpas_dbus_getter_bss_bssid);
>  DECLARE_ACCESSOR(wpas_dbus_getter_bss_ssid);
>  DECLARE_ACCESSOR(wpas_dbus_getter_bss_privacy);
> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
> index 83df04f39..b9bbe260a 100644
> --- a/wpa_supplicant/notify.c
> +++ b/wpa_supplicant/notify.c
> @@ -720,6 +720,9 @@ static void wpas_notify_ap_sta_authorized(struct
> wpa_supplicant *wpa_s,
>  		wpas_dbus_signal_p2p_peer_joined(wpa_s,
> p2p_dev_addr);
>  #endif /* CONFIG_P2P */
>  
> +	/* Unregister the station */
> +	wpas_dbus_register_sta(wpa_s, sta);
> +
>  	/* Notify listeners a new station has been authorized */
>  	wpas_dbus_signal_sta_authorized(wpa_s, sta);
>  }
> @@ -740,6 +743,9 @@ static void
> wpas_notify_ap_sta_deauthorized(struct wpa_supplicant *wpa_s,
>  
>  	/* Notify listeners a station has been deauthorized */
>  	wpas_dbus_signal_sta_deauthorized(wpa_s, sta);
> +
> +	/* Unregister the station */
> +	wpas_dbus_unregister_sta(wpa_s, sta);
>  }
>  
>

On 08/05/18 18:53, Dan Williams wrote:
> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
>>
>> Make it possible to list connected stations in AP mode over D-Bus,
>> along
>> with some of their properties: rx/tx packets, bytes, capabilities,
>> etc.
> 
> It looks like this patch changes the signal signature of
> StaAuthorized/StaDeauthorized, is that correct?  That won't be
> backwards compatible with existing D-Bus clients and we should probably
> choose new signal names to allow old clients to continue working.

Hmm, indeed it does, and it probably shouldn’t. What would you call a
new signal with the new signature?

Mathieu, what applications rely on the proposed new signature of this
signal?

> Also, you'll want to update doc/dbus.doxygen with any changes you make
> to the D-Bus API, including additions.

Will do in the next iteration.

On Thu, May 10, 2018 at 1:07 PM, Andrej Shadura
<andrew.shadura@collabora.co.uk> wrote:
> On 08/05/18 18:53, Dan Williams wrote:
>> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
>>>
>>> Make it possible to list connected stations in AP mode over D-Bus,
>>> along
>>> with some of their properties: rx/tx packets, bytes, capabilities,
>>> etc.
>>
>> It looks like this patch changes the signal signature of
>> StaAuthorized/StaDeauthorized, is that correct?  That won't be
>> backwards compatible with existing D-Bus clients and we should probably
>> choose new signal names to allow old clients to continue working.
>
> Hmm, indeed it does, and it probably shouldn’t. What would you call a
> new signal with the new signature?
>
> Mathieu, what applications rely on the proposed new signature of this
> signal?
>

I had written this a long while ago to use to inform GPS software,
that would help triagulate using APs in range.

FWIW, I won't need it at this point; feel free to rework the code to
your heart's content if you can make use of it. :)

On 10/05/18 20:07, Andrej Shadura wrote:
> On 08/05/18 18:53, Dan Williams wrote:
>> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
>>>
>>> Make it possible to list connected stations in AP mode over D-Bus,
>>> along
>>> with some of their properties: rx/tx packets, bytes, capabilities,
>>> etc.
>> It looks like this patch changes the signal signature of
>> StaAuthorized/StaDeauthorized, is that correct?  That won't be
>> backwards compatible with existing D-Bus clients and we should probably
>> choose new signal names to allow old clients to continue working.
> Hmm, indeed it does, and it probably shouldn’t. What would you call a
> new signal with the new signature?
>
> Mathieu, what applications rely on the proposed new signature of this
> signal?

I don't think


https://git.merproject.org/mer-core/libgsupplicant/blob/master/src/gsupplicant_interface.c#L1987

On 10/05/18 20:07, Andrej Shadura wrote:
> On 08/05/18 18:53, Dan Williams wrote:
>> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
>>>
>>> Make it possible to list connected stations in AP mode over D-Bus,
>>> along
>>> with some of their properties: rx/tx packets, bytes, capabilities,
>>> etc.
>> It looks like this patch changes the signal signature of
>> StaAuthorized/StaDeauthorized, is that correct?  That won't be
>> backwards compatible with existing D-Bus clients and we should probably
>> choose new signal names to allow old clients to continue working.
> Hmm, indeed it does, and it probably shouldn’t. What would you call a
> new signal with the new signature?
>
> Mathieu, what applications rely on the proposed new signature of this
> signal?

Sorry for the spam, I accidentally hit the send button.

I don't think that Mathieu (or anyone else) knows the full list of 
applications relying on the current signature of this particular D-Bus 
signal. Here is an example of code which I'm sure Mathieu wasn't aware of:

https://git.merproject.org/mer-core/libgsupplicant/blob/master/src/gsupplicant_interface.c#L1987

Please don't break backward compatibility. It's not cool.

Cheers,
-Slava

On 12/05/18 15:57, Slava Monich wrote:
> On 10/05/18 20:07, Andrej Shadura wrote:
>> On 08/05/18 18:53, Dan Williams wrote:
>>> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>>>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
>>>>
>>>> Make it possible to list connected stations in AP mode over D-Bus,
>>>> along
>>>> with some of their properties: rx/tx packets, bytes, capabilities,
>>>> etc.
>>> It looks like this patch changes the signal signature of
>>> StaAuthorized/StaDeauthorized, is that correct?  That won't be
>>> backwards compatible with existing D-Bus clients and we should probably
>>> choose new signal names to allow old clients to continue working.
>> Hmm, indeed it does, and it probably shouldn’t. What would you call a
>> new signal with the new signature?
>>
>> Mathieu, what applications rely on the proposed new signature of this
>> signal?
> 
> Sorry for the spam, I accidentally hit the send button.
> 
> I don't think that Mathieu (or anyone else) knows the full list of
> applications relying on the current signature of this particular D-Bus
> signal. Here is an example of code which I'm sure Mathieu wasn't aware of:
> 
> https://git.merproject.org/mer-core/libgsupplicant/blob/master/src/gsupplicant_interface.c#L1987
> 
> Please don't break backward compatibility. It's not cool.

Sure. My question was rather what relies on the new signature (Ubuntu
was shipping that patch for about two years I think).

I think Mathieu’s message didn’t reach the mailing list, so let me copy
his response here:

On 10/05/18 23:07, Mathieu Trudel-Lapierre wrote:
> I had written this a long while ago to use to inform GPS software,
> that would help triagulate using APs in range.
>
> FWIW, I won't need it at this point; feel free to rework the code to
> your heart's content if you can make use of it. :)

On Thu, 2018-05-10 at 17:07 -0400, Mathieu Trudel-Lapierre wrote:
> On Thu, May 10, 2018 at 1:07 PM, Andrej Shadura
> <andrew.shadura@collabora.co.uk> wrote:
> > On 08/05/18 18:53, Dan Williams wrote:
> > > On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
> > > > From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonica
> > > > l.com>
> > > > 
> > > > Make it possible to list connected stations in AP mode over D-
> > > > Bus,
> > > > along
> > > > with some of their properties: rx/tx packets, bytes,
> > > > capabilities,
> > > > etc.
> > > 
> > > It looks like this patch changes the signal signature of
> > > StaAuthorized/StaDeauthorized, is that correct?  That won't be
> > > backwards compatible with existing D-Bus clients and we should
> > > probably
> > > choose new signal names to allow old clients to continue working.
> > 
> > Hmm, indeed it does, and it probably shouldn’t. What would you call
> > a
> > new signal with the new signature?
> > 
> > Mathieu, what applications rely on the proposed new signature of
> > this
> > signal?
> > 
> 
> I had written this a long while ago to use to inform GPS software,
> that would help triagulate using APs in range.
> 
> FWIW, I won't need it at this point; feel free to rework the code to
> your heart's content if you can make use of it. :)

Andrej/Mathieu,

I think the patch is useful and I think there are fairly simple ways to
make it not break D-Bus API. Would either of you be able to make some
of those changes?

Perhaps instead of changing wpas_dbus_signal_sta() and the
corresponding signature, an additional signal (emitted at the same
time) could be "StationAdded"/"StationRemoved" with properties since we
now have a "Stations" property and real station objects.

Dan

On 18/05/18 16:11, Dan Williams wrote:
> On Thu, 2018-05-10 at 17:07 -0400, Mathieu Trudel-Lapierre wrote:
>> On Thu, May 10, 2018 at 1:07 PM, Andrej Shadura
>> <andrew.shadura@collabora.co.uk> wrote:
>>> On 08/05/18 18:53, Dan Williams wrote:
>>>> On Mon, 2018-05-07 at 20:19 +0200, Andrej Shadura wrote:
>>>>> From: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonica
>>>>> l.com>
>>>>>
>>>>> Make it possible to list connected stations in AP mode over D-
>>>>> Bus,
>>>>> along
>>>>> with some of their properties: rx/tx packets, bytes,
>>>>> capabilities,
>>>>> etc.
>>>>
>>>> It looks like this patch changes the signal signature of
>>>> StaAuthorized/StaDeauthorized, is that correct?  That won't be
>>>> backwards compatible with existing D-Bus clients and we should
>>>> probably
>>>> choose new signal names to allow old clients to continue working.
>>>
>>> Hmm, indeed it does, and it probably shouldn’t. What would you call
>>> a
>>> new signal with the new signature?
>>>
>>> Mathieu, what applications rely on the proposed new signature of
>>> this
>>> signal?
>>>
>>
>> I had written this a long while ago to use to inform GPS software,
>> that would help triagulate using APs in range.
>>
>> FWIW, I won't need it at this point; feel free to rework the code to
>> your heart's content if you can make use of it. :)
> 
> Andrej/Mathieu,
> 
> I think the patch is useful and I think there are fairly simple ways to
> make it not break D-Bus API. Would either of you be able to make some
> of those changes?
> 
> Perhaps instead of changing wpas_dbus_signal_sta() and the
> corresponding signature, an additional signal (emitted at the same
> time) could be "StationAdded"/"StationRemoved" with properties since we
> now have a "Stations" property and real station objects.

Yes, I will try to do the changes.