[PATCHv3 0/4] Support bonding callsbacks and retrying bondings

[PATCHv3 0/4] Support bonding callsbacks and retrying bondings

[Scott James Remnant]

Rationale: in many pairing scenarios, it's desirable to be automatically
provide a PIN (either generated or fixed) for a device and only involve
the user if that PIN is rejected by the host.

Both GNOME Bluetooth and Android handle this inside their Agent which
means each deployment has to implement its own code to do the same
thing. These patches allow this behavior to be common by moving it into
BlueZ.

This patch set adds plugin support for a callback called when bonding
of a device successfully completes, fails, or is cancelled. In the
success and failure cases, the callback may return TRUE to cause the
bonding to be retried after a short backoff period.

Backoff drops the ACL connection and reopens again later, this is not
only cleaner to implement in the bluetoothd code itself, but matches
the behavior of both Android (which implements this in its Agent) and
OS X, so is more likely to be compatible with devices out there even
though we could theoretically hold the ACL link open.

btsnoop of OS X performing PIN generation and falling back to PIN can
be seen here: 

http://chromium-os.googlecode.com/issues/attachment?aid=280220006001&
 name=BT+Motorola+Wireless+Keyboard.cfa&
 token=SA_vDEsF87_CU-BcqJ1C8bWd6Zk%3A1333665473760

Scott James Remnant (4):
  Add support for retrying a bonding
  plugin: Add bonding callback support for plugins
  bonding: retry if callback returns TRUE
  bonding: call plugin callback on cancellation

 src/adapter.c |    2 +-
 src/device.c  |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/device.h  |    9 ++++++
 3 files changed, 99 insertions(+), 1 deletions(-)

-- 
1.7.7.3

[PATCHv3 1/4] Add support for retrying a bonding

[Scott James Remnant]

In order to retry a bonding we need a timer that will perform the
retry, we need to stash the status and capability of the bonding
request so it can use them again, and in the case of a retrying
bonding attempt we need to not tear down the temporary D-Bus device
object on the adapter.
---
 src/adapter.c |    2 +-
 src/device.c  |   14 ++++++++++++++
 src/device.h  |    1 +
 3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index d5c1a04..4ae926f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3004,7 +3004,7 @@ void adapter_remove_connection(struct btd_adapter *adapter,
 	if (device_is_authenticating(device))
 		device_cancel_authentication(device, TRUE);
 
-	if (device_is_temporary(device)) {
+	if (device_is_temporary(device) && !device_is_retrying(device)) {
 		const char *path = device_get_path(device);
 
 		DBG("Removing temporary device %s", path);
diff --git a/src/device.c b/src/device.c
index df3fbac..feb330c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -88,6 +88,9 @@ struct bonding_req {
 	DBusMessage *msg;
 	guint listener_id;
 	struct btd_device *device;
+	uint8_t capability;
+	uint8_t status;
+	guint retry_timer;
 };
 
 struct authentication_req {
@@ -2364,6 +2367,9 @@ static void bonding_request_free(struct bonding_req *bonding)
 	if (bonding->conn)
 		dbus_connection_unref(bonding->conn);
 
+	if (bonding->retry_timer)
+		g_source_remove(bonding->retry_timer);
+
 	device = bonding->device;
 	g_free(bonding);
 
@@ -2436,6 +2442,7 @@ proceed:
 
 	bonding->conn = dbus_connection_ref(conn);
 	bonding->msg = dbus_message_ref(msg);
+	bonding->capability = capability;
 
 	return bonding;
 }
@@ -2535,6 +2542,13 @@ static void device_auth_req_free(struct btd_device *device)
 	device->authr = NULL;
 }
 
+gboolean device_is_retrying(struct btd_device *device)
+{
+	struct bonding_req *bonding = device->bonding;
+
+	return bonding && bonding->retry_timer != 0;
+}
+
 void device_bonding_complete(struct btd_device *device, uint8_t status)
 {
 	struct bonding_req *bonding = device->bonding;
diff --git a/src/device.h b/src/device.h
index f3d6c70..f06042c 100644
--- a/src/device.h
+++ b/src/device.h
@@ -75,6 +75,7 @@ void device_set_temporary(struct btd_device *device, gboolean temporary);
 void device_set_bonded(struct btd_device *device, gboolean bonded);
 void device_set_auto_connect(struct btd_device *device, gboolean enable);
 gboolean device_is_connected(struct btd_device *device);
+gboolean device_is_retrying(struct btd_device *device);
 DBusMessage *device_create_bonding(struct btd_device *device,
 				DBusConnection *conn, DBusMessage *msg,
 				const char *agent_path, uint8_t capability);
-- 
1.7.7.3

[PATCHv3 2/4] plugin: Add bonding callback support for plugins

[Scott James Remnant]

Allow plugins to register a bonding callback on a device, this will be
called on completion or cancellation of a bonding attempt on that
device and allow retrying of the bonding attempt.

These callbacks will only be called once, in the case of retrying the
callback must be registered again separately from another callback
(e.g. the pincode callback).
---
 src/device.c |   19 +++++++++++++++++++
 src/device.h |    8 ++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index feb330c..53ca977 100644
--- a/src/device.c
+++ b/src/device.c
@@ -147,6 +147,7 @@ struct btd_device {
 	GSList		*primaries;		/* List of primary services */
 	GSList		*drivers;		/* List of device drivers */
 	GSList		*watches;		/* List of disconnect_data */
+	GSList          *bonding_callbacks;	/* List of bonding callbacks */
 	gboolean	temporary;
 	struct agent	*agent;
 	guint		disconn_timer;
@@ -267,8 +268,12 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->attios, g_free);
 	g_slist_free_full(device->attios_offline, g_free);
 
+	g_slist_free(device->bonding_callbacks);
+
 	att_cleanup(device);
 
+	g_attrib_unref(device->attrib);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -2542,6 +2547,20 @@ static void device_auth_req_free(struct btd_device *device)
 	device->authr = NULL;
 }
 
+void btd_device_register_bonding_cb(struct btd_device *device,
+						btd_device_bonding_cb_t cb)
+{
+	device->bonding_callbacks = g_slist_prepend(
+						device->bonding_callbacks, cb);
+}
+
+void btd_device_unregister_bonding_cb(struct btd_device *device,
+						btd_device_bonding_cb_t cb)
+{
+	device->bonding_callbacks = g_slist_remove(
+						device->bonding_callbacks, cb);
+}
+
 gboolean device_is_retrying(struct btd_device *device)
 {
 	struct bonding_req *bonding = device->bonding;
diff --git a/src/device.h b/src/device.h
index f06042c..2a7409a 100644
--- a/src/device.h
+++ b/src/device.h
@@ -103,6 +103,14 @@ guint device_add_disconnect_watch(struct btd_device *device,
 void device_remove_disconnect_watch(struct btd_device *device, guint id);
 void device_set_class(struct btd_device *device, uint32_t value);
 
+typedef gboolean (*btd_device_bonding_cb_t) (struct btd_device *device,
+					gboolean complete, uint8_t status);
+
+void btd_device_register_bonding_cb(struct btd_device *dev,
+						btd_device_bonding_cb_t cb);
+void btd_device_unregister_bonding_cb(struct btd_device *dev,
+						btd_device_bonding_cb_t cb);
+
 #define BTD_UUIDS(args...) ((const char *[]) { args, NULL } )
 
 struct btd_device_driver {
-- 
1.7.7.3

Re: [PATCHv3 2/4] plugin: Add bonding callback support for plugins

[Vinicius Costa Gomes]

Hi Scott,

On 15:40 Thu 05 Apr, Scott James Remnant wrote:
> Allow plugins to register a bonding callback on a device, this will be
> called on completion or cancellation of a bonding attempt on that
> device and allow retrying of the bonding attempt.
> 
> These callbacks will only be called once, in the case of retrying the
> callback must be registered again separately from another callback
> (e.g. the pincode callback).
> ---
>  src/device.c |   19 +++++++++++++++++++
>  src/device.h |    8 ++++++++
>  2 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/src/device.c b/src/device.c
> index feb330c..53ca977 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -147,6 +147,7 @@ struct btd_device {
>  	GSList		*primaries;		/* List of primary services */
>  	GSList		*drivers;		/* List of device drivers */
>  	GSList		*watches;		/* List of disconnect_data */
> +	GSList          *bonding_callbacks;	/* List of bonding callbacks */
>  	gboolean	temporary;
>  	struct agent	*agent;
>  	guint		disconn_timer;
> @@ -267,8 +268,12 @@ static void device_free(gpointer user_data)
>  	g_slist_free_full(device->attios, g_free);
>  	g_slist_free_full(device->attios_offline, g_free);
>  
> +	g_slist_free(device->bonding_callbacks);
> +
>  	att_cleanup(device);
>  
> +	g_attrib_unref(device->attrib);
> +

This seems unrelated to your series.

>  	if (device->tmp_records)
>  		sdp_list_free(device->tmp_records,
>  					(sdp_free_func_t) sdp_record_free);
> @@ -2542,6 +2547,20 @@ static void device_auth_req_free(struct btd_device *device)
>  	device->authr = NULL;
>  }
>  
> +void btd_device_register_bonding_cb(struct btd_device *device,
> +						btd_device_bonding_cb_t cb)
> +{
> +	device->bonding_callbacks = g_slist_prepend(
> +						device->bonding_callbacks, cb);
> +}
> +
> +void btd_device_unregister_bonding_cb(struct btd_device *device,
> +						btd_device_bonding_cb_t cb)
> +{
> +	device->bonding_callbacks = g_slist_remove(
> +						device->bonding_callbacks, cb);
> +}
> +
>  gboolean device_is_retrying(struct btd_device *device)
>  {
>  	struct bonding_req *bonding = device->bonding;
> diff --git a/src/device.h b/src/device.h
> index f06042c..2a7409a 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -103,6 +103,14 @@ guint device_add_disconnect_watch(struct btd_device *device,
>  void device_remove_disconnect_watch(struct btd_device *device, guint id);
>  void device_set_class(struct btd_device *device, uint32_t value);
>  
> +typedef gboolean (*btd_device_bonding_cb_t) (struct btd_device *device,
> +					gboolean complete, uint8_t status);
> +
> +void btd_device_register_bonding_cb(struct btd_device *dev,
> +						btd_device_bonding_cb_t cb);
> +void btd_device_unregister_bonding_cb(struct btd_device *dev,
> +						btd_device_bonding_cb_t cb);
> +
>  #define BTD_UUIDS(args...) ((const char *[]) { args, NULL } )
>  
>  struct btd_device_driver {
> -- 
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Cheers,
-- 
Vinicius

Re: [PATCHv3 2/4] plugin: Add bonding callback support for plugins

[Scott James Remnant]

On Thu, Apr 5, 2012 at 3:54 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:

> Hi Scott,
>
>> =A0 =A0 =A0 att_cleanup(device);
>>
>> + =A0 =A0 g_attrib_unref(device->attrib);
>> +
>
> This seems unrelated to your series.
>

Ah, good catch, looks like a rebase error; corrected patch to follow this m=
ail.

Scott
--=20
Have you ever, ever felt like this?
Had strange things happen? =A0Are you going round the twist?

[PATCHv3 2/4v2] bonding: retry if callback returns TRUE

[Scott James Remnant]

When a bonding completes, pass the status to any plugin bonding
callbacks; if any return TRUE than set a timer to retry the bonding
after an appropriate backoff period.
---
 src/device.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 5ecf546..cd659d3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2565,6 +2565,44 @@ void btd_device_unregister_bonding_cb(struct btd_device *device,
 						device->bonding_callbacks, cb);
 }
 
+static gboolean device_bonding_retry(gpointer data)
+{
+	struct btd_device *device = data;
+	struct btd_adapter *adapter = device_get_adapter(device);
+	struct bonding_req *bonding = device->bonding;
+	int err;
+
+	if (!bonding)
+		return FALSE;
+
+	DBG("retrying bonding");
+	err = adapter_create_bonding(adapter, &device->bdaddr,
+					device->type, bonding->capability);
+	if (err < 0)
+		device_bonding_complete(device, bonding->status);
+
+	bonding->retry_timer = 0;
+	return FALSE;
+}
+
+static gboolean device_bonding_get_retry(struct btd_device *device,
+								uint8_t status)
+{
+	GSList *l;
+	btd_device_bonding_cb_t cb;
+	gboolean retry = FALSE;
+
+	for (l = device->bonding_callbacks; l != NULL; l = g_slist_next(l)) {
+		cb = l->data;
+		retry |= cb(device, TRUE, status);
+	}
+
+	g_slist_free(device->bonding_callbacks);
+	device->bonding_callbacks = NULL;
+
+	return retry;
+}
+
 gboolean device_is_retrying(struct btd_device *device)
 {
 	struct bonding_req *bonding = device->bonding;
@@ -2579,6 +2617,14 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
 
 	DBG("bonding %p status 0x%02x", bonding, status);
 
+	if (device_bonding_get_retry(device, status) && status) {
+		DBG("backing off and retrying");
+		bonding->status = status;
+		bonding->retry_timer = g_timeout_add(3000,
+						device_bonding_retry, device);
+		return;
+	}
+
 	if (auth && (auth->type == AUTH_TYPE_NOTIFY_PASSKEY
 		     || auth->type == AUTH_TYPE_NOTIFY_PINCODE) && auth->agent)
 		agent_cancel(auth->agent);
-- 
1.7.7.3

Re: [PATCHv3 2/4v2] bonding: retry if callback returns TRUE

[Johan Hedberg]

Hi Scott,

On Fri, Apr 06, 2012, Scott James Remnant wrote:
> When a bonding completes, pass the status to any plugin bonding
> callbacks; if any return TRUE than set a timer to retry the bonding
> after an appropriate backoff period.
> ---
>  src/device.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 46 insertions(+), 0 deletions(-)

I think you messed up something here. This is not v2 of patch 2/4 but
seems to be a resend of patch 3/4.

Johan

[PATCHv3 3/4] bonding: retry if callback returns TRUE

[Scott James Remnant]

When a bonding completes, pass the status to any plugin bonding
callbacks; if any return TRUE than set a timer to retry the bonding
after an appropriate backoff period.
---
 src/device.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 53ca977..343e2bc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2561,6 +2561,44 @@ void btd_device_unregister_bonding_cb(struct btd_device *device,
 						device->bonding_callbacks, cb);
 }
 
+static gboolean device_bonding_retry(gpointer data)
+{
+	struct btd_device *device = data;
+	struct btd_adapter *adapter = device_get_adapter(device);
+	struct bonding_req *bonding = device->bonding;
+	int err;
+
+	if (!bonding)
+		return FALSE;
+
+	DBG("retrying bonding");
+	err = adapter_create_bonding(adapter, &device->bdaddr,
+					device->type, bonding->capability);
+	if (err < 0)
+		device_bonding_complete(device, bonding->status);
+
+	bonding->retry_timer = 0;
+	return FALSE;
+}
+
+static gboolean device_bonding_get_retry(struct btd_device *device,
+								uint8_t status)
+{
+	GSList *l;
+	btd_device_bonding_cb_t cb;
+	gboolean retry = FALSE;
+
+	for (l = device->bonding_callbacks; l != NULL; l = g_slist_next(l)) {
+		cb = l->data;
+		retry |= cb(device, TRUE, status);
+	}
+
+	g_slist_free(device->bonding_callbacks);
+	device->bonding_callbacks = NULL;
+
+	return retry;
+}
+
 gboolean device_is_retrying(struct btd_device *device)
 {
 	struct bonding_req *bonding = device->bonding;
@@ -2575,6 +2613,14 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
 
 	DBG("bonding %p status 0x%02x", bonding, status);
 
+	if (device_bonding_get_retry(device, status) && status) {
+		DBG("backing off and retrying");
+		bonding->status = status;
+		bonding->retry_timer = g_timeout_add(3000,
+						device_bonding_retry, device);
+		return;
+	}
+
 	if (auth && auth->type == AUTH_TYPE_NOTIFY && auth->agent)
 		agent_cancel(auth->agent);
 
-- 
1.7.7.3

[PATCHv3 4/4] bonding: call plugin callback on cancellation

[Scott James Remnant]

Call the plugin callbacks when a bonding request is cancelled.
---
 src/device.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 343e2bc..448c8fd 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2715,6 +2715,8 @@ void device_cancel_bonding(struct btd_device *device, uint8_t status)
 	struct bonding_req *bonding = device->bonding;
 	DBusMessage *reply;
 	char addr[18];
+	GSList *l;
+	btd_device_bonding_cb_t cb;
 
 	if (!bonding)
 		return;
@@ -2722,6 +2724,14 @@ void device_cancel_bonding(struct btd_device *device, uint8_t status)
 	ba2str(&device->bdaddr, addr);
 	DBG("Canceling bonding request for %s", addr);
 
+	for (l = device->bonding_callbacks; l != NULL; l = g_slist_next(l)) {
+		cb = l->data;
+		cb(device, FALSE, 0);
+	}
+
+	g_slist_free(device->bonding_callbacks);
+	device->bonding_callbacks = NULL;
+
 	if (device->authr)
 		device_cancel_authentication(device, FALSE);
 
-- 
1.7.7.3