Patchwork P2P: Make GO negotiation peer and group information available over D-Bus

login
register
mail settings
Submitter Johannes Berg
Date Dec. 13, 2011, 9:27 a.m.
Message ID <1323768463.3355.4.camel@jlt3.sipsolutions.net>
Download mbox | patch
Permalink /patch/131036/
State Accepted
Commit e5a359cf7e2c65d5527c6d2c704183c773fb0ab9
Headers show

Comments

Johannes Berg - Dec. 13, 2011, 9:27 a.m.
From: Reinette Chatre <reinette.chatre@intel.com>

The GO negotiation response is very cryptic at the moment. For a success
message we only know on which interface the negotiation succeeded, not
which peer. For a failure we know the interface also and a status code
(number).

It will be very useful for clients to know upon receipt of such a message
which peer the negotiation occurred with.

Now that the peer information is available and the API is changed
already, the function composing the D-Bus message might as well include
all GO negotiation information. This is done with a dict to make things
easier on clients if this result information changes down the line.

Signed-hostap: Reinette Chatre <reinette.chatre@intel.com>
Signed-hostap: Johannes Berg <johannes.berg@intel.com>
---
 src/p2p/p2p.c                   |    2 +-
 src/p2p/p2p.h                   |    2 +
 wpa_supplicant/dbus/dbus_new.c  |   86 ++++++++++++++++++++++++++++++++++-----
 wpa_supplicant/dbus/dbus_new.h  |    4 +-
 wpa_supplicant/notify.c         |    4 +-
 wpa_supplicant/notify.h         |    2 +-
 wpa_supplicant/p2p_supplicant.c |    4 +-
 7 files changed, 86 insertions(+), 18 deletions(-)
Jouni Malinen - Dec. 18, 2011, 2:56 p.m.
On Tue, Dec 13, 2011 at 10:27:43AM +0100, Johannes Berg wrote:
> The GO negotiation response is very cryptic at the moment. For a success
> message we only know on which interface the negotiation succeeded, not
> which peer. For a failure we know the interface also and a status code
> (number).

Well.. There can only be a single GO negotiation in progress, so it
should be clear which peer was used here at the completion of the
negotiation.

> It will be very useful for clients to know upon receipt of such a message
> which peer the negotiation occurred with.
> 
> Now that the peer information is available and the API is changed
> already, the function composing the D-Bus message might as well include
> all GO negotiation information. This is done with a dict to make things
> easier on clients if this result information changes down the line.

I agree that this can make some operations cleaner on the client and I
did apply the patch. However, we do need to become a bit more careful on
changes to the D-Bus interface to avoid problems for programs that
expect to get a more stable interface for interacting with
wpa_supplicant.
Johannes Berg - Dec. 19, 2011, 4 p.m.
On Sun, 2011-12-18 at 16:56 +0200, Jouni Malinen wrote:
> On Tue, Dec 13, 2011 at 10:27:43AM +0100, Johannes Berg wrote:
> > The GO negotiation response is very cryptic at the moment. For a success
> > message we only know on which interface the negotiation succeeded, not
> > which peer. For a failure we know the interface also and a status code
> > (number).
> 
> Well.. There can only be a single GO negotiation in progress, so it
> should be clear which peer was used here at the completion of the
> negotiation.

True.

> > It will be very useful for clients to know upon receipt of such a message
> > which peer the negotiation occurred with.
> > 
> > Now that the peer information is available and the API is changed
> > already, the function composing the D-Bus message might as well include
> > all GO negotiation information. This is done with a dict to make things
> > easier on clients if this result information changes down the line.
> 
> I agree that this can make some operations cleaner on the client and I
> did apply the patch. However, we do need to become a bit more careful on
> changes to the D-Bus interface to avoid problems for programs that
> expect to get a more stable interface for interacting with
> wpa_supplicant.

I agree, but part of the problem is that so far we had very little users
of the APIs, so a lot of these cases haven't really been discovered yet.
I'll look more at compatibility though in future patches.

johannes

Patch

diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c
index cce3b79..c3c1bac 100644
--- a/src/p2p/p2p.c
+++ b/src/p2p/p2p.c
@@ -3134,7 +3134,7 @@  int p2p_reject(struct p2p_data *p2p, const u8 *peer_addr)
 }
 
 
-static const char * p2p_wps_method_text(enum p2p_wps_method method)
+const char * p2p_wps_method_text(enum p2p_wps_method method)
 {
 	switch (method) {
 	case WPS_NOT_READY:
diff --git a/src/p2p/p2p.h b/src/p2p/p2p.h
index 48caaee..a3e079c 100644
--- a/src/p2p/p2p.h
+++ b/src/p2p/p2p.h
@@ -766,6 +766,8 @@  struct p2p_config {
 	void (*group_custom_ies_update)(void *ctx, int beacon);
 };
 
+/* Util */
+const char * p2p_wps_method_text(enum p2p_wps_method method);
 
 /* P2P module initialization/deinitialization */
 
diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
index 1c58553..b750af4 100644
--- a/wpa_supplicant/dbus/dbus_new.c
+++ b/wpa_supplicant/dbus/dbus_new.c
@@ -1044,35 +1044,101 @@  nomem:
  * on status.
  * @status: Status of the GO neg request. 0 for success, other for errors.
  */
-void wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, int status)
+void wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, struct p2p_go_neg_results *res)
 {
 	DBusMessage *msg;
-	DBusMessageIter iter;
+	DBusMessageIter iter, dict_iter;
+	DBusMessageIter iter_dict_entry, iter_dict_val, iter_dict_array;
 	struct wpas_dbus_priv *iface;
+	char peer_obj_path[WPAS_DBUS_OBJECT_PATH_MAX], *path;
+	dbus_int32_t freqs[P2P_MAX_CHANNELS];
+	dbus_int32_t *f_array = freqs;
+
 
 	iface = wpa_s->global->dbus;
 
+	memset(freqs, 0, sizeof(freqs));
 	/* Do nothing if the control interface is not turned on */
 	if (iface == NULL)
 		return;
 
+	os_snprintf(peer_obj_path, WPAS_DBUS_OBJECT_PATH_MAX,
+		    "%s/" WPAS_DBUS_NEW_P2P_PEERS_PART "/" COMPACT_MACSTR,
+		    wpa_s->dbus_new_path, MAC2STR(res->peer_device_addr));
+	path = peer_obj_path;
+
 	msg = dbus_message_new_signal(wpa_s->dbus_new_path,
 				      WPAS_DBUS_NEW_IFACE_P2PDEVICE,
-				      status ? "GONegotiationFailure" :
-					       "GONegotiationSuccess");
+				      res->status ? "GONegotiationFailure" :
+						    "GONegotiationSuccess");
 	if (msg == NULL)
 		return;
 
-	if (status) {
-		dbus_message_iter_init_append(msg, &iter);
-		if (!dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT32,
-						    &status)) {
-			wpa_printf(MSG_ERROR,
-				   "dbus: Failed to construct signal");
+	dbus_message_iter_init_append(msg, &iter);
+	if (!wpa_dbus_dict_open_write(&iter, &dict_iter))
+		goto err;
+	if (!wpa_dbus_dict_append_object_path(&dict_iter, "peer_object",
+					      path) ||
+	    !wpa_dbus_dict_append_int32(&dict_iter, "status", res->status))
+		goto err;
+
+	if (!res->status) {
+		int i = 0;
+		int freq_list_num = 0;
+		if (res->role_go)
+			if (!wpa_dbus_dict_append_byte_array(&dict_iter, "passphrase",
+						     (const char *) res->passphrase,
+						     sizeof(res->passphrase)))
+				goto err;
+		if (!wpa_dbus_dict_append_string(&dict_iter, "role_go", res->role_go ? "GO" : "client") ||
+		    !wpa_dbus_dict_append_int32(&dict_iter, "frequency", res->freq) ||
+		    !wpa_dbus_dict_append_byte_array(&dict_iter, "ssid",
+						     (const char *) res->ssid,
+						     res->ssid_len) ||
+		    !wpa_dbus_dict_append_byte_array(&dict_iter, "peer_device_addr",
+						     (const char *) res->peer_device_addr,
+						     ETH_ALEN) ||
+		    !wpa_dbus_dict_append_byte_array(&dict_iter, "peer_interface_addr",
+						     (const char *) res->peer_interface_addr,
+						     ETH_ALEN) ||
+		    !wpa_dbus_dict_append_string(&dict_iter, "wps_method", p2p_wps_method_text(res->wps_method)))
 			goto err;
+
+		for (i = 0; i < P2P_MAX_CHANNELS; i++) {
+			if (res->freq_list[i]) {
+				freqs[i] = res->freq_list[i];
+				freq_list_num++;
+			}
 		}
+
+		if (!wpa_dbus_dict_begin_array(&dict_iter,
+					       "frequency_list",
+					       DBUS_TYPE_INT32_AS_STRING,
+					       &iter_dict_entry,
+					       &iter_dict_val,
+					       &iter_dict_array))
+			goto err;
+
+		if (!dbus_message_iter_append_fixed_array(&iter_dict_array,
+							   DBUS_TYPE_INT32, &f_array, freq_list_num))
+			goto err;
+
+		if (!wpa_dbus_dict_end_array(&dict_iter,
+					     &iter_dict_entry,
+					     &iter_dict_val,
+					     &iter_dict_array))
+			goto err;
+
+		if (!wpa_dbus_dict_append_int32(&dict_iter, "persistent_group",
+						res->persistent_group) ||
+		    !wpa_dbus_dict_append_uint32(&dict_iter, "peer_config_timeout",
+						res->peer_config_timeout))
+			goto err;
 	}
 
+	if (!wpa_dbus_dict_close_write(&iter, &dict_iter))
+		goto err;
+
 	dbus_connection_send(iface->con, msg, NULL);
 err:
 	dbus_message_unref(msg);
diff --git a/wpa_supplicant/dbus/dbus_new.h b/wpa_supplicant/dbus/dbus_new.h
index 3e12b63..ba8f26c 100644
--- a/wpa_supplicant/dbus/dbus_new.h
+++ b/wpa_supplicant/dbus/dbus_new.h
@@ -188,7 +188,7 @@  void wpas_dbus_signal_p2p_group_started(struct wpa_supplicant *wpa_s,
 					int client, int network_id);
 void wpas_dbus_register_p2p_group(struct wpa_supplicant *wpa_s,
 				  struct wpa_ssid *ssid);
-void wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, int status);
+void wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, struct p2p_go_neg_results *res);
 void wpas_dbus_unregister_p2p_group(struct wpa_supplicant *wpa_s,
 				    const struct wpa_ssid *ssid);
 int wpas_dbus_register_persistent_group(struct wpa_supplicant *wpa_s,
@@ -401,7 +401,7 @@  static inline int wpas_dbus_unregister_persistent_group(
 }
 
 static inline void
-wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, int status)
+wpas_dbus_signal_p2p_go_neg_resp(struct wpa_supplicant *wpa_s, struct p2p_go_neg_results *res)
 {
 }
 
diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
index c1a29c0..d5717d8 100644
--- a/wpa_supplicant/notify.c
+++ b/wpa_supplicant/notify.c
@@ -443,9 +443,9 @@  void wpas_notify_p2p_go_neg_req(struct wpa_supplicant *wpa_s,
 }
 
 
-void wpas_notify_p2p_go_neg_completed(struct wpa_supplicant *wpa_s, int status)
+void wpas_notify_p2p_go_neg_completed(struct wpa_supplicant *wpa_s, struct p2p_go_neg_results *res)
 {
-	wpas_dbus_signal_p2p_go_neg_resp(wpa_s, status);
+	wpas_dbus_signal_p2p_go_neg_resp(wpa_s, res);
 }
 
 
diff --git a/wpa_supplicant/notify.h b/wpa_supplicant/notify.h
index e354d41..84616c2 100644
--- a/wpa_supplicant/notify.h
+++ b/wpa_supplicant/notify.h
@@ -97,7 +97,7 @@  void wpas_notify_p2p_group_removed(struct wpa_supplicant *wpa_s,
 void wpas_notify_p2p_go_neg_req(struct wpa_supplicant *wpa_s,
 				const u8 *src, u16 dev_passwd_id);
 void wpas_notify_p2p_go_neg_completed(struct wpa_supplicant *wpa_s,
-				      int status);
+				      struct p2p_go_neg_results *res);
 void wpas_notify_p2p_invitation_result(struct wpa_supplicant *wpa_s,
 				       int status, const u8 *bssid);
 void wpas_notify_p2p_sd_request(struct wpa_supplicant *wpa_s,
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index 745c97b..dc20617 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -920,13 +920,13 @@  void wpas_go_neg_completed(void *ctx, struct p2p_go_neg_results *res)
 	if (res->status) {
 		wpa_msg(wpa_s, MSG_INFO, P2P_EVENT_GO_NEG_FAILURE "status=%d",
 			res->status);
-		wpas_notify_p2p_go_neg_completed(wpa_s, res->status);
+		wpas_notify_p2p_go_neg_completed(wpa_s, res);
 		wpas_p2p_remove_pending_group_interface(wpa_s);
 		return;
 	}
 
 	wpa_msg(wpa_s, MSG_INFO, P2P_EVENT_GO_NEG_SUCCESS);
-	wpas_notify_p2p_go_neg_completed(wpa_s, P2P_SC_SUCCESS);
+	wpas_notify_p2p_go_neg_completed(wpa_s, res);
 
 	if (wpa_s->create_p2p_iface) {
 		struct wpa_supplicant *group_wpa_s =