Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] core: Add btd_adapter_recreate_bonding()
From: Alfonso Acosta @ 2014-10-10 23:56 UTC (permalink / raw)
  To: BlueZ development
In-Reply-To: <1412985213-5447-1-git-send-email-fons@spotify.com>

Please note that this patch depends on a previously submitted kernel
patch [1] to ensure that connection parameters are not lost when
rebonding.


[1] [PATCH] Bluetooth: Defer connection-parameter removal when
unpairing without disconnecting

On Sat, Oct 11, 2014 at 1:53 AM, Alfonso Acosta <fons@spotify.com> wrote:
> This patch adds btd_adapter_recreate_bonding() which rebonds a device,
> i.e. it performs an unbonding operation inmediately followed by a
> bonding operation.
>
> Although there is no internal use for btd_adapter_recreate_bonding()
> yet, it is useful for plugins dealing with devices which require
> renegotiaing their keys. For instance, when dealing with devices
> without persistent storage which lose their keys on reset.
>
> Certain caveats have been taken into account when rebonding with a LE
> device:
>
>  * If the device to rebond is already connected, the rebonding is done
>    without disconnecting to avoid the extra latency of reestablishing
>    the connection.
>
>  * If no connection exists, we connect before unbonding anyways to
>    avoid losing the device's autoconnection and connection parameters,
>    which are removed by the kernel when unpairing if no connection
>    exists.
>
>  * Not closing the connection requires defering attribute discovery
>    until the rebonding is done. Otherwise, the security level could be
>    elavated with the old LTK, which may be invalid since we are
>    rebonding. When rebonding, attribute discovery is suspended and the
>    ATT socket is saved to allow resuming it once bonding is finished.
> ---
>  src/adapter.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/adapter.h |  7 ++++++-
>  src/device.c  | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/device.h  |  4 ++++
>  4 files changed, 119 insertions(+), 7 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 92ee1a0..81e8f22 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -5195,14 +5195,15 @@ int btd_adapter_read_clock(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
>  }
>
>  int btd_adapter_remove_bonding(struct btd_adapter *adapter,
> -                               const bdaddr_t *bdaddr, uint8_t bdaddr_type)
> +                               const bdaddr_t *bdaddr, uint8_t bdaddr_type,
> +                               uint8_t disconnect)
>  {
>         struct mgmt_cp_unpair_device cp;
>
>         memset(&cp, 0, sizeof(cp));
>         bacpy(&cp.addr.bdaddr, bdaddr);
>         cp.addr.type = bdaddr_type;
> -       cp.disconnect = 1;
> +       cp.disconnect = disconnect;
>
>         if (mgmt_send(adapter->mgmt, MGMT_OP_UNPAIR_DEVICE,
>                                 adapter->dev_id, sizeof(cp), &cp,
> @@ -5212,6 +5213,53 @@ int btd_adapter_remove_bonding(struct btd_adapter *adapter,
>         return -EIO;
>  }
>
> +int btd_adapter_recreate_bonding(struct btd_adapter *adapter,
> +                                       const bdaddr_t *bdaddr,
> +                                       uint8_t bdaddr_type,
> +                                       uint8_t io_cap)
> +{
> +       struct btd_device *device;
> +       int err;
> +
> +       device = btd_adapter_get_device(adapter, bdaddr, bdaddr_type);
> +
> +       if (!device || !device_is_bonded(device, bdaddr_type))
> +               return -EINVAL;
> +
> +       device_set_rebonding(device, bdaddr_type, true);
> +
> +       /* Make sure the device is connected before unbonding to avoid
> +        * losing the device's autoconnection and connection
> +        * parameters, which are removed by the kernel when unpairing
> +        * if no connection exists. We would had anyways implicitly
> +        * connected when bonding later on, so we might as well just
> +        * do it explicitly now.
> +        */
> +       if (bdaddr_type != BDADDR_BREDR && !btd_device_is_connected(device)) {
> +               err = device_connect_le(device);
> +               if (err < 0)
> +                       goto failed;
> +       }
> +
> +       /* Unbond without disconnecting to avoid the connection
> +        * re-establishment latency
> +        */
> +       err = btd_adapter_remove_bonding(adapter, bdaddr, bdaddr_type, 0);
> +       if (err < 0)
> +               goto failed;
> +
> +       err = adapter_create_bonding(adapter, bdaddr, bdaddr_type, io_cap);
> +       if (err < 0)
> +               goto failed;
> +
> +       return 0;
> +
> +failed:
> +       error("failed: %s", strerror(-err));
> +       device_set_rebonding(device, bdaddr_type, false);
> +       return err;
> +}
> +
>  static void pincode_reply_complete(uint8_t status, uint16_t length,
>                                         const void *param, void *user_data)
>  {
> @@ -5594,8 +5642,11 @@ static void bonding_complete(struct btd_adapter *adapter,
>         else
>                 device = btd_adapter_find_device(adapter, bdaddr, addr_type);
>
> -       if (device != NULL)
> +       if (device != NULL) {
>                 device_bonding_complete(device, addr_type, status);
> +               if (device_is_rebonding(device, addr_type))
> +                       device_rebonding_complete(device, addr_type);
> +       }
>
>         resume_discovery(adapter);
>
> diff --git a/src/adapter.h b/src/adapter.h
> index 6801fee..bd00d3e 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -158,7 +158,12 @@ int btd_adapter_disconnect_device(struct btd_adapter *adapter,
>                                                         uint8_t bdaddr_type);
>
>  int btd_adapter_remove_bonding(struct btd_adapter *adapter,
> -                               const bdaddr_t *bdaddr, uint8_t bdaddr_type);
> +                               const bdaddr_t *bdaddr, uint8_t bdaddr_type,
> +                               uint8_t disconnect);
> +int btd_adapter_recreate_bonding(struct btd_adapter *adapter,
> +                                       const bdaddr_t *bdaddr,
> +                                       uint8_t bdaddr_type,
> +                                       uint8_t io_cap);
>
>  int btd_adapter_pincode_reply(struct btd_adapter *adapter,
>                                         const  bdaddr_t *bdaddr,
> diff --git a/src/device.c b/src/device.c
> index 875a5c5..4aab349 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -158,6 +158,7 @@ struct att_callbacks {
>  struct bearer_state {
>         bool paired;
>         bool bonded;
> +       bool rebonding;
>         bool connected;
>         bool svc_resolved;
>  };
> @@ -221,6 +222,8 @@ struct btd_device {
>         int8_t          rssi;
>
>         GIOChannel      *att_io;
> +       GIOChannel      *att_rebond_io;
> +
>         guint           cleanup_id;
>         guint           store_id;
>  };
> @@ -522,6 +525,9 @@ static void device_free(gpointer user_data)
>
>         attio_cleanup(device);
>
> +       if (device->att_rebond_io)
> +               g_io_channel_unref(device->att_rebond_io);
> +
>         if (device->tmp_records)
>                 sdp_list_free(device->tmp_records,
>                                         (sdp_free_func_t) sdp_record_free);
> @@ -1739,6 +1745,23 @@ long device_bonding_last_duration(struct btd_device *device)
>         return bonding->last_attempt_duration_ms;
>  }
>
> +bool device_is_rebonding(struct btd_device *device, uint8_t bdaddr_type)
> +{
> +       struct bearer_state *state = get_state(device, bdaddr_type);
> +
> +       return state->rebonding;
> +}
> +
> +void device_set_rebonding(struct btd_device *device, uint8_t bdaddr_type,
> +                               bool rebonding)
> +{
> +       struct bearer_state *state = get_state(device, bdaddr_type);
> +
> +       state->rebonding = rebonding;
> +
> +       DBG("rebonding = %d", rebonding);
> +}
> +
>  static void create_bond_req_exit(DBusConnection *conn, void *user_data)
>  {
>         struct btd_device *device = user_data;
> @@ -2029,7 +2052,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
>
>         if (state->paired && !state->bonded)
>                 btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
> -                                                               bdaddr_type);
> +                                                               bdaddr_type, 1);
>
>         if (device->bredr_state.connected || device->le_state.connected)
>                 return;
> @@ -2639,13 +2662,13 @@ static void device_remove_stored(struct btd_device *device)
>         if (device->bredr_state.bonded) {
>                 device->bredr_state.bonded = false;
>                 btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
> -                                                               BDADDR_BREDR);
> +                                                       BDADDR_BREDR, 1);
>         }
>
>         if (device->le_state.bonded) {
>                 device->le_state.bonded = false;
>                 btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
> -                                                       device->bdaddr_type);
> +                                                       device->bdaddr_type, 1);
>         }
>
>         device->bredr_state.paired = false;
> @@ -3628,6 +3651,18 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>         GAttrib *attrib;
>         BtIOSecLevel sec_level;
>
> +       DBG("");
> +
> +       if (dev->le_state.rebonding) {
> +               DBG("postponing due to rebonding");
> +               /* Keep the att socket, and defer attaching the attributes
> +                * until rebonding is done.
> +                */
> +               if (!dev->att_rebond_io)
> +                       dev->att_rebond_io = g_io_channel_ref(io);
> +               return false;
> +       }
> +
>         bt_io_get(io, &gerr, BT_IO_OPT_SEC_LEVEL, &sec_level,
>                                                 BT_IO_OPT_INVALID);
>         if (gerr) {
> @@ -4269,6 +4304,23 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>         }
>  }
>
> +bool device_rebonding_complete(struct btd_device *device, uint8_t bdaddr_type)
> +{
> +       bool ret = true;
> +
> +       DBG("");
> +
> +       device_set_rebonding(device, bdaddr_type, false);
> +
> +       if (bdaddr_type != BDADDR_BREDR && device->att_rebond_io) {
> +               ret = device_attach_attrib(device, device->att_rebond_io);
> +               g_io_channel_unref(device->att_rebond_io);
> +               device->att_rebond_io = NULL;
> +       }
> +
> +       return ret;
> +}
> +
>  static gboolean svc_idle_cb(gpointer user_data)
>  {
>         struct svc_callback *cb = user_data;
> diff --git a/src/device.h b/src/device.h
> index 2e0473e..65b1018 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -94,6 +94,10 @@ bool device_is_retrying(struct btd_device *device);
>  void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>                                                         uint8_t status);
>  gboolean device_is_bonding(struct btd_device *device, const char *sender);
> +bool device_is_rebonding(struct btd_device *device, uint8_t bdaddr_type);
> +void device_set_rebonding(struct btd_device *device, uint8_t bdaddr_type,
> +                               bool rebonding);
> +bool device_complete_rebonding(struct btd_device *device, uint8_t bdaddr_type);
>  void device_bonding_attempt_failed(struct btd_device *device, uint8_t status);
>  void device_bonding_failed(struct btd_device *device, uint8_t status);
>  struct btd_adapter_pin_cb_iter *device_bonding_iter(struct btd_device *device);
> --
> 1.9.1
>



-- 
Alfonso Acosta

Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com

^ permalink raw reply

* [PATCH] core: Add btd_adapter_recreate_bonding()
From: Alfonso Acosta @ 2014-10-10 23:53 UTC (permalink / raw)
  To: linux-bluetooth

This patch adds btd_adapter_recreate_bonding() which rebonds a device,
i.e. it performs an unbonding operation inmediately followed by a
bonding operation.

Although there is no internal use for btd_adapter_recreate_bonding()
yet, it is useful for plugins dealing with devices which require
renegotiaing their keys. For instance, when dealing with devices
without persistent storage which lose their keys on reset.

Certain caveats have been taken into account when rebonding with a LE
device:

 * If the device to rebond is already connected, the rebonding is done
   without disconnecting to avoid the extra latency of reestablishing
   the connection.

 * If no connection exists, we connect before unbonding anyways to
   avoid losing the device's autoconnection and connection parameters,
   which are removed by the kernel when unpairing if no connection
   exists.

 * Not closing the connection requires defering attribute discovery
   until the rebonding is done. Otherwise, the security level could be
   elavated with the old LTK, which may be invalid since we are
   rebonding. When rebonding, attribute discovery is suspended and the
   ATT socket is saved to allow resuming it once bonding is finished.
---
 src/adapter.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/adapter.h |  7 ++++++-
 src/device.c  | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/device.h  |  4 ++++
 4 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 92ee1a0..81e8f22 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5195,14 +5195,15 @@ int btd_adapter_read_clock(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 }
 
 int btd_adapter_remove_bonding(struct btd_adapter *adapter,
-				const bdaddr_t *bdaddr, uint8_t bdaddr_type)
+				const bdaddr_t *bdaddr, uint8_t bdaddr_type,
+				uint8_t disconnect)
 {
 	struct mgmt_cp_unpair_device cp;
 
 	memset(&cp, 0, sizeof(cp));
 	bacpy(&cp.addr.bdaddr, bdaddr);
 	cp.addr.type = bdaddr_type;
-	cp.disconnect = 1;
+	cp.disconnect = disconnect;
 
 	if (mgmt_send(adapter->mgmt, MGMT_OP_UNPAIR_DEVICE,
 				adapter->dev_id, sizeof(cp), &cp,
@@ -5212,6 +5213,53 @@ int btd_adapter_remove_bonding(struct btd_adapter *adapter,
 	return -EIO;
 }
 
+int btd_adapter_recreate_bonding(struct btd_adapter *adapter,
+					const bdaddr_t *bdaddr,
+					uint8_t bdaddr_type,
+					uint8_t io_cap)
+{
+	struct btd_device *device;
+	int err;
+
+	device = btd_adapter_get_device(adapter, bdaddr, bdaddr_type);
+
+	if (!device || !device_is_bonded(device, bdaddr_type))
+		return -EINVAL;
+
+	device_set_rebonding(device, bdaddr_type, true);
+
+	/* Make sure the device is connected before unbonding to avoid
+	 * losing the device's autoconnection and connection
+	 * parameters, which are removed by the kernel when unpairing
+	 * if no connection exists. We would had anyways implicitly
+	 * connected when bonding later on, so we might as well just
+	 * do it explicitly now.
+	 */
+	if (bdaddr_type != BDADDR_BREDR && !btd_device_is_connected(device)) {
+		err = device_connect_le(device);
+		if (err < 0)
+			goto failed;
+	}
+
+	/* Unbond without disconnecting to avoid the connection
+	 * re-establishment latency
+	 */
+	err = btd_adapter_remove_bonding(adapter, bdaddr, bdaddr_type, 0);
+	if (err < 0)
+		goto failed;
+
+	err = adapter_create_bonding(adapter, bdaddr, bdaddr_type, io_cap);
+	if (err < 0)
+		goto failed;
+
+	return 0;
+
+failed:
+	error("failed: %s", strerror(-err));
+	device_set_rebonding(device, bdaddr_type, false);
+	return err;
+}
+
 static void pincode_reply_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -5594,8 +5642,11 @@ static void bonding_complete(struct btd_adapter *adapter,
 	else
 		device = btd_adapter_find_device(adapter, bdaddr, addr_type);
 
-	if (device != NULL)
+	if (device != NULL) {
 		device_bonding_complete(device, addr_type, status);
+		if (device_is_rebonding(device, addr_type))
+			device_rebonding_complete(device, addr_type);
+	}
 
 	resume_discovery(adapter);
 
diff --git a/src/adapter.h b/src/adapter.h
index 6801fee..bd00d3e 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -158,7 +158,12 @@ int btd_adapter_disconnect_device(struct btd_adapter *adapter,
 							uint8_t bdaddr_type);
 
 int btd_adapter_remove_bonding(struct btd_adapter *adapter,
-				const bdaddr_t *bdaddr, uint8_t bdaddr_type);
+				const bdaddr_t *bdaddr, uint8_t bdaddr_type,
+				uint8_t disconnect);
+int btd_adapter_recreate_bonding(struct btd_adapter *adapter,
+					const bdaddr_t *bdaddr,
+					uint8_t bdaddr_type,
+					uint8_t io_cap);
 
 int btd_adapter_pincode_reply(struct btd_adapter *adapter,
 					const  bdaddr_t *bdaddr,
diff --git a/src/device.c b/src/device.c
index 875a5c5..4aab349 100644
--- a/src/device.c
+++ b/src/device.c
@@ -158,6 +158,7 @@ struct att_callbacks {
 struct bearer_state {
 	bool paired;
 	bool bonded;
+	bool rebonding;
 	bool connected;
 	bool svc_resolved;
 };
@@ -221,6 +222,8 @@ struct btd_device {
 	int8_t		rssi;
 
 	GIOChannel	*att_io;
+	GIOChannel	*att_rebond_io;
+
 	guint		cleanup_id;
 	guint		store_id;
 };
@@ -522,6 +525,9 @@ static void device_free(gpointer user_data)
 
 	attio_cleanup(device);
 
+	if (device->att_rebond_io)
+		g_io_channel_unref(device->att_rebond_io);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -1739,6 +1745,23 @@ long device_bonding_last_duration(struct btd_device *device)
 	return bonding->last_attempt_duration_ms;
 }
 
+bool device_is_rebonding(struct btd_device *device, uint8_t bdaddr_type)
+{
+	struct bearer_state *state = get_state(device, bdaddr_type);
+
+	return state->rebonding;
+}
+
+void device_set_rebonding(struct btd_device *device, uint8_t bdaddr_type,
+				bool rebonding)
+{
+	struct bearer_state *state = get_state(device, bdaddr_type);
+
+	state->rebonding = rebonding;
+
+	DBG("rebonding = %d", rebonding);
+}
+
 static void create_bond_req_exit(DBusConnection *conn, void *user_data)
 {
 	struct btd_device *device = user_data;
@@ -2029,7 +2052,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
 
 	if (state->paired && !state->bonded)
 		btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
-								bdaddr_type);
+								bdaddr_type, 1);
 
 	if (device->bredr_state.connected || device->le_state.connected)
 		return;
@@ -2639,13 +2662,13 @@ static void device_remove_stored(struct btd_device *device)
 	if (device->bredr_state.bonded) {
 		device->bredr_state.bonded = false;
 		btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
-								BDADDR_BREDR);
+							BDADDR_BREDR, 1);
 	}
 
 	if (device->le_state.bonded) {
 		device->le_state.bonded = false;
 		btd_adapter_remove_bonding(device->adapter, &device->bdaddr,
-							device->bdaddr_type);
+							device->bdaddr_type, 1);
 	}
 
 	device->bredr_state.paired = false;
@@ -3628,6 +3651,18 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 	GAttrib *attrib;
 	BtIOSecLevel sec_level;
 
+	DBG("");
+
+	if (dev->le_state.rebonding) {
+		DBG("postponing due to rebonding");
+		/* Keep the att socket, and defer attaching the attributes
+		 * until rebonding is done.
+		 */
+		if (!dev->att_rebond_io)
+			dev->att_rebond_io = g_io_channel_ref(io);
+		return false;
+	}
+
 	bt_io_get(io, &gerr, BT_IO_OPT_SEC_LEVEL, &sec_level,
 						BT_IO_OPT_INVALID);
 	if (gerr) {
@@ -4269,6 +4304,23 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
 	}
 }
 
+bool device_rebonding_complete(struct btd_device *device, uint8_t bdaddr_type)
+{
+	bool ret = true;
+
+	DBG("");
+
+	device_set_rebonding(device, bdaddr_type, false);
+
+	if (bdaddr_type != BDADDR_BREDR && device->att_rebond_io) {
+		ret = device_attach_attrib(device, device->att_rebond_io);
+		g_io_channel_unref(device->att_rebond_io);
+		device->att_rebond_io = NULL;
+	}
+
+	return ret;
+}
+
 static gboolean svc_idle_cb(gpointer user_data)
 {
 	struct svc_callback *cb = user_data;
diff --git a/src/device.h b/src/device.h
index 2e0473e..65b1018 100644
--- a/src/device.h
+++ b/src/device.h
@@ -94,6 +94,10 @@ bool device_is_retrying(struct btd_device *device);
 void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
 							uint8_t status);
 gboolean device_is_bonding(struct btd_device *device, const char *sender);
+bool device_is_rebonding(struct btd_device *device, uint8_t bdaddr_type);
+void device_set_rebonding(struct btd_device *device, uint8_t bdaddr_type,
+				bool rebonding);
+bool device_complete_rebonding(struct btd_device *device, uint8_t bdaddr_type);
 void device_bonding_attempt_failed(struct btd_device *device, uint8_t status);
 void device_bonding_failed(struct btd_device *device, uint8_t status);
 struct btd_adapter_pin_cb_iter *device_bonding_iter(struct btd_device *device);
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH v5 bluetooth-next] 6lowpan: Use pskb_expand_head in IPHC decompression.
From: Alexander Aring @ 2014-10-10 19:41 UTC (permalink / raw)
  To: Martin Townsend
  Cc: linux-bluetooth, linux-wpan, marcel, jukka.rissanen,
	Martin Townsend, werner
In-Reply-To: <1412840794-17283-2-git-send-email-martin.townsend@xsilon.com>

Hi Martin,

I reconsider these steps which we do here now and saw some new
improvements/issues.

On Thu, Oct 09, 2014 at 08:46:34AM +0100, Martin Townsend wrote:
> Currently there are potentially 2 skb_copy_expand calls in IPHC
> decompression.  This patch replaces this with one call to
> pskb_expand_head.  It also checks to see if there is enough headroom
> first to ensure it's only done if necessary.
> As pskb_expand_head must only have one reference the calling code
> now ensures this.
> 
> Signed-off-by: Martin Townsend <martin.townsend@xsilon.com>
> ---
>  net/6lowpan/iphc.c      | 51 ++++++++++++++++++++++++-------------------------
>  net/bluetooth/6lowpan.c |  7 +++++++
>  2 files changed, 32 insertions(+), 26 deletions(-)
> 
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 142eef5..853b4b8 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -174,30 +174,22 @@ static int uncompress_context_based_src_addr(struct sk_buff *skb,
>  static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
>  		       struct net_device *dev, skb_delivery_cb deliver_skb)
>  {
> -	struct sk_buff *new;
>  	int stat;
>  
> -	new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb),
> -			      GFP_ATOMIC);
> -	kfree_skb(skb);
> -
> -	if (!new)
> -		return -ENOMEM;
> -
> -	skb_push(new, sizeof(struct ipv6hdr));
> -	skb_reset_network_header(new);
> -	skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr));
> +	skb_push(skb, sizeof(struct ipv6hdr));
> +	skb_reset_network_header(skb);
> +	skb_copy_to_linear_data(skb, hdr, sizeof(struct ipv6hdr));
>  
> -	new->protocol = htons(ETH_P_IPV6);
> -	new->pkt_type = PACKET_HOST;
> -	new->dev = dev;
> +	skb->protocol = htons(ETH_P_IPV6);
> +	skb->pkt_type = PACKET_HOST;
> +	skb->dev = dev;
>  
>  	raw_dump_table(__func__, "raw skb data dump before receiving",
> -		       new->data, new->len);
> +		       skb->data, skb->len);
>  
> -	stat = deliver_skb(new, dev);
> +	stat = deliver_skb(skb, dev);
>  
> -	kfree_skb(new);
> +	consume_skb(skb);
>  
>  	return stat;
>  }
> @@ -460,7 +452,7 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>  	/* UDP data uncompression */
>  	if (iphc0 & LOWPAN_IPHC_NH_C) {
>  		struct udphdr uh;
> -		struct sk_buff *new;
> +		const int needed = sizeof(struct udphdr) + sizeof(hdr);
>  
>  		if (uncompress_udp_header(skb, &uh))
>  			goto drop;
> @@ -468,14 +460,13 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>  		/* replace the compressed UDP head by the uncompressed UDP
>  		 * header
>  		 */
> -		new = skb_copy_expand(skb, sizeof(struct udphdr),
> -				      skb_tailroom(skb), GFP_ATOMIC);
> -		kfree_skb(skb);
> -
> -		if (!new)
> -			return -ENOMEM;
> -
> -		skb = new;
> +		if (skb_headroom(skb) < needed) {
> +			err = pskb_expand_head(skb, needed, 0, GFP_ATOMIC);
> +			if (unlikely(err)) {
> +				kfree_skb(skb);
> +				return err;
> +			}
> +		}
>  
>  		skb_push(skb, sizeof(struct udphdr));
>  		skb_reset_transport_header(skb);
> @@ -485,6 +476,14 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
>  			       (u8 *)&uh, sizeof(uh));
>  
>  		hdr.nexthdr = UIP_PROTO_UDP;
> +	} else {
> +		if (skb_headroom(skb) < sizeof(hdr)) {
> +			err = pskb_expand_head(skb, sizeof(hdr), 0, GFP_ATOMIC);
> +			if (unlikely(err)) {
> +				kfree_skb(skb);
> +				return err;
> +			}
> +		}

What we here try to do is a usual sk_buff principle.

There exist a function skb_cow [0]:

<qoute>
static inline int __skb_cow(struct sk_buff *skb, unsigned int headroom,
                            int cloned)
{
        int delta = 0;

        if (headroom > skb_headroom(skb))
                delta = headroom - skb_headroom(skb);

        if (delta || cloned)
                return pskb_expand_head(skb, ALIGN(delta, NET_SKB_PAD), 0,
                                        GFP_ATOMIC);
        return 0;
}
</qoute>

This is the wrapper call which is called by skb_cow. I see here much
similarity and also more performance. It calculates a delta size and do
also a NET_SKB_PAD which is a align to the cache_lines and I think they
try do some "false sharing" here. [1]

We should use this function, if you agree here.



There exist also some skb_cow_head function [2].

code commentar:
<qoute>
It should be used when you only need to push on some header and do not
need to modify the data.
</qoute>

I am not sure about if we can use skb_cow_head here, because we modify
the data. I mean we run skb_pull and then skb_cow_head and run skb_push
with different data buffer. (IPv6 header).

Currently I am not sure if this works.

>  	}
>  
>  	hdr.payload_len = htons(skb->len);
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index c2e0d14..6643a7c 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -343,6 +343,13 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>  		kfree_skb(local_skb);
>  		kfree_skb(skb);
>  	} else {
> +		/* Decompression may use pskb_expand_head so no shared skb's */
> +		skb = skb_share_check(skb, GFP_ATOMIC);
> +		if (!skb) {
> +			dev->stats.rx_dropped++;
> +			return NET_RX_DROP;
> +		}
> +
>  		switch (skb->data[0] & 0xe0) {
>  		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
>  			local_skb = skb_clone(skb, GFP_ATOMIC);


Now we come to this function.

There exist two global rules here:

1.

If we change only the skb pointers head/tail with e.g. skb_push then we
need a cloned skb only. We don't change the skb data buffer here.

We need this in LOWPAN_DISPATCH_IPV6, here we run only one skb_pull for
one byte, and the FRAGN and FRAG1 dispatches in 802.15.4 6LoWPAN.

skb_share_check do this, we have surely a clone afterwards and the
sk_buff attributes are not changed elsewhere, which means we are
allowed to manipulate the skb attributes.

2.

If we touch the data buffer, then we need a skb_copy. We need this at
LOWPAN_DISPATCH_IPHC because here we replacing the data buffer.

skb_unshare do this for us.

If we can use skb_cow_head like above, I think then we don't need a
skb_unshare here. But I am not 100% sure if we really can use
skb_cow_head here. We don't need skb_unshare then because, skb_cow_head
makes the head writeable only, this should something similar like a
private copy of headroom then.


Summary:

We need always a skb_share_check at begin of evaluate the dispatch, for
the LOWPAN_DISPATCH_IPHC we need a skb_unshare before. That's fine for
now, maybe later we can do some tricks with skb_cow_head.

skb_share_check:

We always have a cloned skb and we can manipulate sk_buff attributes.

skb_unshare:

We have a private copy of data buffer and can replace 6LoWPAN header
with IPv6 header.

Do you agree here?

- Alex

[0] http://lxr.free-electrons.com/source/include/linux/skbuff.h#L2316
[1] http://en.wikipedia.org/wiki/False_sharing
[2] http://lxr.free-electrons.com/source/include/linux/skbuff.h#L2331 

^ permalink raw reply

* Re: [PATCH] Bluetooth: Defer connection-parameter removal when unpairing without disconnecting
From: Marcel Holtmann @ 2014-10-10 15:57 UTC (permalink / raw)
  To: Alfonso Acosta; +Cc: BlueZ development
In-Reply-To: <CAHF=Y4oTZjk6k0YeBA6xEpksZRWU1TTtkZfB+YESWmmRxNs0GA@mail.gmail.com>

Hi Alfonso,

> As an alternative to this patch, I am thinking that it may be worth
> considering two other options:
> 
> 1. Adding an additional repairing operation: MGMT_OP_REPAIR_DEVICE.
> Making the repairing semantics explicit  would allow us to keep the connection
> parameters even if we choose to disconnect when unpairing.  On the
> other hand, one could argue that it clutters the API with an extra
> operation which is the composite of two already existing operations.
> 
> 2. Make MGMT_OP_PAIR_DEVICE behave like the repairing operation in (1)
> if the device was already paired. If I recall properly, Johan suggested
> this on IRC.

I am not in favor of making Pair Device just do re-pairing. I think that is a dangerous behavior since want bondings in the kernel only in two ways. Either they are loaded via Load Long Term Keys or via Pair Device. Doing something magic is something that I consider dangerous and can lead into some flaws. If we come with a clear error than we at least know that something went wrong.

Repairing command is possible, but I am not 100% convinced that it is a good idea. The normal workflow is that you pair and unpair a device. If you get stuck in a weird state, you unpair first which ensures that everything is cleaned out and then pair again. For the connection parameters we can just be smarter.

I bet there is still a lot of improvement for our connection parameter handling. For example we could just keep all connection parameters around in cache and just expire them after 1 day or so. That would improve general handling with devices with do not pair in the first. So I would focus on being smart when dropping connection parameters and not trying to bind it too much to mgmt API visible states.

Regards

Marcel


^ permalink raw reply

* Re: [RFC] need for new scan api for bluetooth low energy.
From: Jakub Pawlowski @ 2014-10-10 15:49 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Arman Uguray, BlueZ development, Scott James Remnant
In-Reply-To: <8A0217F1-C062-466F-8C04-38E324BDD55D@holtmann.org>

Thanks for info!

I'll look into that.

On Fri, Oct 10, 2014 at 3:52 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Jakub,
>
>>>>> Ok, new updated version - we do only passive scan, and we send Device
>>>>> found event:
>>>>>
>>>>>
>>>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>>>> index 11e2490..3d03617 100644
>>>>> --- a/doc/mgmt-api.txt
>>>>> +++ b/doc/mgmt-api.txt
>>>>> @@ -2135,6 +2135,29 @@ Set Public Address Command
>>>>>                              Invalid Index
>>>>>
>>>>>
>>>>> +Start Passive LE Filtered Device Scan
>>>>> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
>>>>> +
>>>>> +       Command Code:           0x003A
>>>>> +       Controller Index:       <controller id>
>>>>> +       Command Parameters:     UUID (16 Octets)
>>>>> +                    max_pathloss (1 octet)
>>>>> +
>>>>> +       Return Parameters:
>>>>> +
>>>>> +       This command starts passive LE scan, and filter received
>>>>> advertisements: if AD contains both TX power level and Service
>>>>> Solicitation, and UUID is contained in Service Solicitation and
>>>>> computed pathloss is smaller than max_pathloss parameter, then a
>>>>> Device Found event will be sent.
>>>>
>>>> generally we tried to avoid to specific LE only commands. Our attempts=
 with the kernel APIs where to make them as generic as possible. So I think=
 something along the lines of adding and removing UUID filters should be so=
mething we should target at.
>>>>
>>>> Add UUID Notification Filter
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
>>>>
>>>>       Command Parameters:     Address_Type (1 Octet)
>>>>                               UUID (16 Octets)
>>>>                               Max_Pathloss (1 Octet)
>>>>       Return Parameters:      Address_Type (1 Octet)
>>>>
>>>>
>>>> Remove UUID Notification Filter
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
>>>>
>>>>       Command Parameters:     Address_Type (1 Octet)
>>>>                               UUID (16 Octets)
>>>>       Return Parameters:      Address_Type (1 Octet)
>>>>
>>>> The Address_Type would be defined the same way as for Start Discovery =
command.
>>>>
>>>> Now we can discuss if on the mgmt level we want to use pathloss or rat=
her a range of RSSI. I have seen that RSSI is preferred and that the transl=
ation from RSSI and advertising data with TX power level is done in the dae=
mon. So for kernel API it might be better to expose RSSI range.
>>>>
>>>> Besides a RSSI range, we might also want to allow giving a RSSI thresh=
old that defines how much the RSSI to change between events to be reported =
again.
>>>
>>> Some mobile devices that I'm working can change power level for
>>> advertising, that makes raw RSSI filter useless. It must be pathloss,
>>> that's why I require TX power in advertisement.
>>
>> you will still get it, but just have to do the math in the daemon. The o=
nly difference is that the daemon gets woken up for RSSI ranges that in the=
 end you might filter out.
>>
>>>>
>>>> That said, the danger with a generic notification filter like this is =
that we can not implement it efficiently with current hardware without vend=
or commands. If you use duplicate filtering, then Broadcom and CSR chips be=
have fully different. Broadcom only filters on BD_ADDR and report devices o=
nce no matter what the RSSI, while CSR will report the same device again if=
 the RSSI changes.
>>>>
>>>> With that in mind, it might be safer to introduce a one-shot mgmt API =
that needs to be repeatedly called to keep scanning for new devices.
>>>>
>>>> Start Service Discovery
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>>
>>>>       Command Parameters:     Address_Type (1 Octet)
>>>>                               Min_RSSI (1 Octet)
>>>>                               Max_RSSI (1 Octet)
>>>>                               Num_UUID (1 Octet)
>>>>                               UUID[i] (16 Octets)
>>>>       Return Parameters:      Address_Type (1 Octet)
>>>>
>>>> Stop Service Discovery
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>>
>>>>       Command Parameters:     Address_Type (1 Octet)
>>>>       Return Parameters:      Address_Type (1 Octet)
>>>>
>>>> Without having dedicated controller support for filtering, having such=
 a dedicated discovery for services with a specific UUID seems to be more a=
ppropriate and more safe. Since we could always add controller based filter=
ing for background scanning once there is controller support.
>>>
>>> I preffer the "Add UUID Notification Filter" and "Remove UUID
>>> Notification Filter". For controllers like Broadcom that reports only
>>> one advertisement can we restart scan internally in kernel ? Why
>>> propagate that event higher ?
>>
>> Personally I would prefer just adding another filter as well. However it=
 might be better to start with something like a one-shot handling and see w=
here that is taking us. It is a little bit less intrusive and you could als=
o use active scanning. At the same time you could also easily make it work =
for BR/EDR. I am thinking here along the lines of something wanting to pair=
 a mouse. You just give the HID UUID and start interleaved discovery for th=
at service on both transports.
>
> I send a proposal for doing an one-shot service discovery. So you can pro=
pose a RSSI Threshold and a list of possible UUIDs to look for.
>
> If RSSI Threshold is 127 or the Num_UUID is 0, then it will behave the sa=
me way as Start Discovery. I think this is how we should get started to tac=
kle this problem.
>
> Regards
>
> Marcel
>

^ permalink raw reply

* Re: Bluez dual-mode support
From: Marcel Holtmann @ 2014-10-10 15:40 UTC (permalink / raw)
  To: Boulais, Marc-Andre; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <29A2697B0516A946B1023D5E798DFCF65B5C257D@mail-yang>

Hi Marc,

> Does BlueZ support a "dual mode" where both Bluetooth "classic" and "low-energy" are enabled? I need to support classic and LE Bluetooth devices in my network.
> 
> Furthermore, is there an recent list of Bluetooth chipsets supported by BlueZ for Low Energy? any chipset recommendations would be greatly appreciated. 

yes, BlueZ 5.x fully supports dual-mode operation. Almost all recent chipsets should be supported. So any USB based Bluetooth controller should actually work out of the box.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH] Bluetooth: Defer connection-parameter removal when unpairing without disconnecting
From: Alfonso Acosta @ 2014-10-10 15:40 UTC (permalink / raw)
  To: Marcel Holtmann, johan.hedberg; +Cc: BlueZ development
In-Reply-To: <CAHF=Y4p3Rn=Q3pdW3ML7=H00pBcGUSjr3SfAsm6v+Da8_LPcCQ@mail.gmail.com>

Hi Jhoan and Marcel,

I sent v2 which I hope addresses all your remarks.

Thanks,

Fons

On Fri, Oct 10, 2014 at 10:14 AM, Alfonso Acosta <fons@spotify.com> wrote:
> Hi Marcel,
>
>
>> don't we also need to clear the flag when the new pairing succeed?
>
> Yep, in fact that's the whole point of the flag .... which I missed.
> Thanks for your patience :)
>
>> If we destroy hci_conn anyway, there is pretty much no point in test_and_clear_bit and we could just use test_bit here.
>
> Fair point, I will change them to test_bit.
>
>
>
>
> --
> Alfonso Acosta
>
> Embedded Systems Engineer at Spotify
> Birger Jarlsgatan 61, Stockholm, Sweden
> http://www.spotify.com



-- 
Alfonso Acosta

Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com

^ permalink raw reply

* Re: [PATCH] core: Add Manufacturer Specific Data EIR field
From: Alfonso Acosta @ 2014-10-10 15:27 UTC (permalink / raw)
  To: BlueZ development
In-Reply-To: <1412941053-11835-1-git-send-email-fons@spotify.com>

Please ignore this first patch. I sent a new bundle "[PATCH v2 0/2] core:
Add plugin-support for Manufacturer Specific Data EIR" with more
context and an extra patch for subscribing to MSD from plugins.

On Fri, Oct 10, 2014 at 1:37 PM, Alfonso Acosta <fons@spotify.com> wrote:
> Athough the Manufacturer Specific Data field is not used internally,
> it's useful for external plugins, e.g. iBeacon.
> ---
>  src/eir.c | 8 ++++++++
>  src/eir.h | 4 ++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/src/eir.c b/src/eir.c
> index d22ad91..f130855 100644
> --- a/src/eir.c
> +++ b/src/eir.c
> @@ -240,6 +240,14 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>                         eir->did_product = data[4] | (data[5] << 8);
>                         eir->did_version = data[6] | (data[7] << 8);
>                         break;
> +
> +               case EIR_MANUFACTURER_DATA:
> +                       if (data_len < 2)
> +                               break;
> +                       eir->msd_company = get_le16(data);
> +                       eir->msd_data_len = data_len - 2;
> +                       memcpy(&eir->msd_data, data+2, eir->msd_data_len);
> +                       break;
>                 }
>
>                 eir_data += field_len + 1;
> diff --git a/src/eir.h b/src/eir.h
> index e486fa2..88a5bf0 100644
> --- a/src/eir.h
> +++ b/src/eir.h
> @@ -37,6 +37,7 @@
>  #define EIR_SSP_RANDOMIZER          0x0F  /* SSP Randomizer */
>  #define EIR_DEVICE_ID               0x10  /* device ID */
>  #define EIR_GAP_APPEARANCE          0x19  /* GAP appearance */
> +#define EIR_MANUFACTURER_DATA       0xFF  /* Manufacturer Specific Data */
>
>  /* Flags Descriptions */
>  #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
> @@ -62,6 +63,9 @@ struct eir_data {
>         uint16_t did_product;
>         uint16_t did_version;
>         uint16_t did_source;
> +       uint16_t msd_company;
> +       uint8_t msd_data[HCI_MAX_EIR_LENGTH];
> +       uint8_t msd_data_len;
>  };
>
>  void eir_data_free(struct eir_data *eir);
> --
> 1.9.1
>



-- 
Alfonso Acosta

Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com

^ permalink raw reply

* [PATCH v2 2/2] core: Add subscription API for Manufacturer Specific Data
From: Alfonso Acosta @ 2014-10-10 15:23 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1412954626-30226-1-git-send-email-fons@spotify.com>

This patch allows plugins to be notified whenever an adapter receives
Manufacturer Specific Data inside from a device.

This can happen when new device is discovered or when we autoconnect
to it.
---
 plugins/neard.c |  1 -
 src/adapter.c   | 38 +++++++++++++++++++++++++++++++++++++-
 src/adapter.h   | 10 ++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 137d601..a975417 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -38,7 +38,6 @@
 #include "src/dbus-common.h"
 #include "src/adapter.h"
 #include "src/device.h"
-#include "src/eir.h"
 #include "src/agent.h"
 #include "src/hcid.h"
 
diff --git a/src/adapter.c b/src/adapter.c
index 92ee1a0..8302cb4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -68,7 +68,6 @@
 #include "attrib/att.h"
 #include "attrib/gatt.h"
 #include "attrib-server.h"
-#include "eir.h"
 
 #define ADAPTER_INTERFACE	"org.bluez.Adapter1"
 
@@ -206,6 +205,7 @@ struct btd_adapter {
 	gboolean initialized;
 
 	GSList *pin_callbacks;
+	GSList *msd_callbacks;
 
 	GSList *drivers;
 	GSList *profiles;
@@ -4549,6 +4549,10 @@ static void adapter_remove(struct btd_adapter *adapter)
 
 	g_slist_free(adapter->pin_callbacks);
 	adapter->pin_callbacks = NULL;
+
+	g_slist_free(adapter->msd_callbacks);
+	adapter->msd_callbacks = NULL;
+
 }
 
 const char *adapter_get_path(struct btd_adapter *adapter)
@@ -4647,6 +4651,20 @@ static void confirm_name(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 						confirm_name_timeout, adapter);
 }
 
+void adapter_msd_notify(struct btd_adapter *adapter, struct btd_device *dev,
+				const struct eir_msd *msd)
+{
+	GSList *l, *next;
+
+	for (l = adapter->msd_callbacks; l != NULL; l = next) {
+		btd_msd_cb_t cb = l->data;
+
+		next = g_slist_next(l);
+
+		cb(adapter, dev, msd);
+	}
+}
+
 static void update_found_devices(struct btd_adapter *adapter,
 					const bdaddr_t *bdaddr,
 					uint8_t bdaddr_type, int8_t rssi,
@@ -4738,6 +4756,9 @@ static void update_found_devices(struct btd_adapter *adapter,
 
 	device_add_eir_uuids(dev, eir_data.services);
 
+	if (eir_data.msd)
+		adapter_msd_notify(adapter, dev, eir_data.msd);
+
 	eir_data_free(&eir_data);
 
 	/*
@@ -5173,6 +5194,18 @@ void btd_adapter_unregister_pin_cb(struct btd_adapter *adapter,
 	adapter->pin_callbacks = g_slist_remove(adapter->pin_callbacks, cb);
 }
 
+void btd_adapter_unregister_msd_cb(struct btd_adapter *adapter,
+							btd_msd_cb_t cb)
+{
+	adapter->msd_callbacks = g_slist_remove(adapter->msd_callbacks, cb);
+}
+
+void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
+							btd_msd_cb_t cb)
+{
+	adapter->msd_callbacks = g_slist_prepend(adapter->msd_callbacks, cb);
+}
+
 int btd_adapter_set_fast_connectable(struct btd_adapter *adapter,
 							gboolean enable)
 {
@@ -6663,6 +6696,9 @@ static void connected_callback(uint16_t index, uint16_t length,
 		btd_device_device_set_name(device, eir_data.name);
 	}
 
+	if (eir_data.msd)
+		adapter_msd_notify(adapter, device, eir_data.msd);
+
 	eir_data_free(&eir_data);
 }
 
diff --git a/src/adapter.h b/src/adapter.h
index 6801fee..e72af89 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -30,6 +30,8 @@
 #include <glib.h>
 #include <stdbool.h>
 
+#include "eir.h"
+
 #define MAX_NAME_LENGTH		248
 
 /* Invalid SSP passkey value used to indicate negative replies */
@@ -138,6 +140,14 @@ struct btd_adapter_pin_cb_iter *btd_adapter_pin_cb_iter_new(
 void btd_adapter_pin_cb_iter_free(struct btd_adapter_pin_cb_iter *iter);
 bool btd_adapter_pin_cb_iter_end(struct btd_adapter_pin_cb_iter *iter);
 
+typedef void (*btd_msd_cb_t) (struct btd_adapter *adapter,
+				struct btd_device *dev,
+				const struct eir_msd *msd);
+void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
+					btd_msd_cb_t cb);
+void btd_adapter_unregister_msd_cb(struct btd_adapter *adapter,
+						btd_msd_cb_t cb);
+
 /* If TRUE, enables fast connectabe, i.e. reduces page scan interval and changes
  * type. If FALSE, disables fast connectable, i.e. sets page scan interval and
  * type to default values. Valid for both connectable and discoverable modes. */
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 1/2] core: Add Manufacturer Specific Data EIR field
From: Alfonso Acosta @ 2014-10-10 15:23 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1412954626-30226-1-git-send-email-fons@spotify.com>

Add data structure and parsing support.
---
 src/eir.c | 11 +++++++++++
 src/eir.h |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/src/eir.c b/src/eir.c
index d22ad91..4a92efd 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -53,6 +53,8 @@ void eir_data_free(struct eir_data *eir)
 	eir->hash = NULL;
 	g_free(eir->randomizer);
 	eir->randomizer = NULL;
+	g_free(eir->msd);
+	eir->msd = NULL;
 }
 
 static void eir_parse_uuid16(struct eir_data *eir, const void *data,
@@ -240,6 +242,15 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
 			eir->did_product = data[4] | (data[5] << 8);
 			eir->did_version = data[6] | (data[7] << 8);
 			break;
+
+		case EIR_MANUFACTURER_DATA:
+			if (data_len < 2)
+				break;
+			eir->msd = g_malloc(sizeof(*eir->msd));
+			eir->msd->company = get_le16(data);
+			eir->msd->data_len = data_len - 2;
+			memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
+			break;
 		}
 
 		eir_data += field_len + 1;
diff --git a/src/eir.h b/src/eir.h
index e486fa2..4cc9dbf 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -37,6 +37,7 @@
 #define EIR_SSP_RANDOMIZER          0x0F  /* SSP Randomizer */
 #define EIR_DEVICE_ID               0x10  /* device ID */
 #define EIR_GAP_APPEARANCE          0x19  /* GAP appearance */
+#define EIR_MANUFACTURER_DATA       0xFF  /* Manufacturer Specific Data */
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -47,6 +48,12 @@
 #define EIR_SIM_HOST                0x10 /* Simultaneous LE and BR/EDR to Same
 					    Device Capable (Host) */
 
+struct eir_msd {
+	uint16_t company;
+	uint8_t data[HCI_MAX_EIR_LENGTH];
+	uint8_t data_len;
+};
+
 struct eir_data {
 	GSList *services;
 	unsigned int flags;
@@ -62,6 +69,7 @@ struct eir_data {
 	uint16_t did_product;
 	uint16_t did_version;
 	uint16_t did_source;
+	struct eir_msd *msd;
 };
 
 void eir_data_free(struct eir_data *eir);
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 0/2] core: Add plugin-support for Manufacturer Specific Data EIR
From: Alfonso Acosta @ 2014-10-10 15:23 UTC (permalink / raw)
  To: linux-bluetooth

Athough the Manufacturer Specific Data (MSD) field is not used
internally, it's useful for external plugins, e.g. iBeacon. 

For instance we are dealing with a device which, when losing its LTK
on a reset, notifies the master about the need of rebonding by broadcasting
specific content in the MSD field of ADV_IND reports.

The first patch simply adds the field and parsing support.

The second one adds a subscription mechanism for plugins so that they can
be notified about new Manufacturer Specific Data being received.

Alfonso Acosta (2):
  core: Add Manufacturer Specific Data EIR field
  core: Add subscription API for Manufacturer Specific Data

 plugins/neard.c |  1 -
 src/adapter.c   | 38 +++++++++++++++++++++++++++++++++++++-
 src/adapter.h   | 10 ++++++++++
 src/eir.c       | 11 +++++++++++
 src/eir.h       |  8 ++++++++
 5 files changed, 66 insertions(+), 2 deletions(-)

-- 
1.9.1


^ permalink raw reply

* Bluez dual-mode support
From: Boulais, Marc-Andre @ 2014-10-10 15:15 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org

Hi,
Does BlueZ support a "dual mode" where both Bluetooth "classic" and "low-energy" are enabled? I need to support classic and LE Bluetooth devices in my network.

Furthermore, is there an recent list of Bluetooth chipsets supported by BlueZ for Low Energy? any chipset recommendations would be greatly appreciated. 

Thanks,
Marc

^ permalink raw reply

* Re: [PATCH BlueZ 1/8] android/avrcp-lib: Add support for SetBrowsedPlayer PDU
From: Luiz Augusto von Dentz @ 2014-10-10 14:15 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org
In-Reply-To: <1412771509-7996-1-git-send-email-luiz.dentz@gmail.com>

Hi,

On Wed, Oct 8, 2014 at 3:31 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> ---
>  android/avrcp-lib.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  android/avrcp-lib.h |  6 +++++
>  2 files changed, 83 insertions(+)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index 9a074ae..8350497 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -1243,6 +1243,30 @@ static void avrcp_set_control_handlers(struct avrcp *session,
>         session->control_data = user_data;
>  }
>
> +static ssize_t set_browsed(struct avrcp *session, uint8_t transaction,
> +                                       uint16_t params_len, uint8_t *params,
> +                                       void *user_data)
> +{
> +       struct avrcp_player *player = user_data;
> +       struct set_browsed_req *req;
> +       uint16_t id;
> +
> +       DBG("");
> +
> +       if (!player->ind || !player->ind->set_browsed)
> +               return -ENOSYS;
> +
> +       if (!params || params_len != sizeof(*req))
> +               return -EINVAL;
> +
> +       req = (void *) params;
> +
> +       id = get_be16(&req->id);
> +
> +       return player->ind->set_browsed(session, transaction, id,
> +                                                       player->user_data);
> +}
> +
>  static ssize_t get_folder_items(struct avrcp *session, uint8_t transaction,
>                                         uint16_t params_len, uint8_t *params,
>                                         void *user_data)
> @@ -1449,6 +1473,7 @@ static ssize_t add_to_now_playing(struct avrcp *session, uint8_t transaction,
>  }
>
>  static const struct avrcp_browsing_handler browsing_handlers[] = {
> +               { AVRCP_SET_BROWSED_PLAYER, set_browsed },
>                 { AVRCP_GET_FOLDER_ITEMS, get_folder_items },
>                 { AVRCP_CHANGE_PATH, change_path },
>                 { AVRCP_GET_ITEM_ATTRIBUTES, get_item_attributes },
> @@ -3295,6 +3320,58 @@ int avrcp_set_addressed_player_rsp(struct avrcp *session, uint8_t transaction,
>                                 &iov, 1);
>  }
>
> +int avrcp_set_browsed_player_rsp(struct avrcp *session, uint8_t transaction,
> +                                       uint8_t status, uint16_t counter,
> +                                       uint32_t items, uint8_t depth,
> +                                       const char **folders)
> +{
> +       struct iovec iov[UINT8_MAX * 2 + 1];
> +       struct set_browsed_rsp rsp;
> +       uint16_t len[UINT8_MAX];
> +       int i;
> +
> +       if (status != AVRCP_STATUS_SUCCESS) {
> +               iov[0].iov_base = &status;
> +               iov[0].iov_len = sizeof(status);
> +               return avrcp_send_browsing(session, transaction,
> +                                               AVRCP_SET_BROWSED_PLAYER,
> +                                               iov, 1);
> +       }
> +
> +       rsp.status = status;
> +       put_be16(counter, &rsp.counter);
> +       put_be32(items, &rsp.items);
> +       put_be16(AVRCP_CHARSET_UTF8, &rsp.charset);
> +       rsp.depth = depth;
> +
> +       iov[0].iov_base = &rsp;
> +       iov[0].iov_len = sizeof(rsp);
> +
> +       if (!depth)
> +               return avrcp_send_browsing(session, transaction,
> +                                               AVRCP_SET_BROWSED_PLAYER,
> +                                               iov, 1);
> +
> +       for (i = 0; i < depth; i++) {
> +               if (!folders[i])
> +                       return -EINVAL;
> +
> +               len[i] = strlen(folders[i]);
> +
> +               iov[i * 2 + 2].iov_base = (void *) folders[i];
> +               iov[i * 2 + 2].iov_len = len[i];
> +
> +               put_be16(len[i], &len[i]);
> +
> +               iov[i * 2 + 1].iov_base = &len[i];
> +               iov[i * 2 + 1].iov_len = sizeof(len[i]);
> +       }
> +
> +       return avrcp_send_browsing(session, transaction,
> +                                       AVRCP_SET_BROWSED_PLAYER, iov,
> +                                       depth * 2 + 1);
> +}
> +
>  int avrcp_get_folder_items_rsp(struct avrcp *session, uint8_t transaction,
>                                         uint16_t counter, uint8_t number,
>                                         uint8_t *type, uint16_t *len,
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index 2b6eccb..dd82deb 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -161,6 +161,8 @@ struct avrcp_control_ind {
>                                         uint8_t volume, void *user_data);
>         int (*set_addressed) (struct avrcp *session, uint8_t transaction,
>                                         uint16_t id, void *user_data);
> +       int (*set_browsed) (struct avrcp *session, uint8_t transaction,
> +                                       uint16_t id, void *user_data);
>         int (*get_folder_items) (struct avrcp *session, uint8_t transaction,
>                                         uint8_t scope, uint32_t start,
>                                         uint32_t end, uint16_t number,
> @@ -319,6 +321,10 @@ int avrcp_set_volume_rsp(struct avrcp *session, uint8_t transaction,
>                                                         uint8_t volume);
>  int avrcp_set_addressed_player_rsp(struct avrcp *session, uint8_t transaction,
>                                                         uint8_t status);
> +int avrcp_set_browsed_player_rsp(struct avrcp *session, uint8_t transaction,
> +                                       uint8_t status, uint16_t counter,
> +                                       uint32_t items, uint8_t depth,
> +                                       const char **folders);
>  int avrcp_get_folder_items_rsp(struct avrcp *session, uint8_t transaction,
>                                         uint16_t counter, uint8_t number,
>                                         uint8_t *type, uint16_t *len,
> --
> 1.9.3

Applied.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH v2 2/2] Bluetooth: Improve RFCOMM __test_pf macro robustness
From: Szymon Janc @ 2014-10-10 13:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1412948593-32106-1-git-send-email-szymon.janc@tieto.com>

Value returned by this macro might be used as bit value so it should
return either 0 or 1 to avoid possible bugs (similar to NSC bug)
when shifting it.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---
 net/bluetooth/rfcomm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index aa38fa8..7e9ed17 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,7 @@ static struct rfcomm_session *rfcomm_session_del(struct rfcomm_session *s);
 
 #define __test_ea(b)      ((b & 0x01))
 #define __test_cr(b)      (!!(b & 0x02))
-#define __test_pf(b)      ((b & 0x10))
+#define __test_pf(b)      (!!(b & 0x10))
 
 #define __addr(cr, dlci)       (((dlci & 0x3f) << 2) | (cr << 1) | 0x01)
 #define __ctrl(type, pf)       (((type & 0xef) | (pf << 4)))
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 1/2] Bluetooth: Fix RFCOMM NSC response
From: Szymon Janc @ 2014-10-10 13:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

rfcomm_send_nsc expects cr to be either 0 or 1 since it is later
passed to __mcc_type macro and shitfed. Unfortunatelly cr extracted
from received frame type was not sanitized and shifted value was passed
resulting in bogus response.

Note: shifted value was also passed to other functions but was used
only in if satements so this bug appears only for NSC case.

This was affecting TC_RFC_BV_25_C PTS qualification test.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---

V2: moved sanitization to macro ifself
    added second patch that also fix __test_pf 

 net/bluetooth/rfcomm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index ca957d3..aa38fa8 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -78,7 +78,7 @@ static struct rfcomm_session *rfcomm_session_del(struct rfcomm_session *s);
 #define __get_type(b)     ((b & 0xef))
 
 #define __test_ea(b)      ((b & 0x01))
-#define __test_cr(b)      ((b & 0x02))
+#define __test_cr(b)      (!!(b & 0x02))
 #define __test_pf(b)      ((b & 0x10))
 
 #define __addr(cr, dlci)       (((dlci & 0x3f) << 2) | (cr << 1) | 0x01)
-- 
1.9.1


^ permalink raw reply related

* [PATCH] Bluetooth: Fix RFCOMM NSC response
From: Szymon Janc @ 2014-10-10 13:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

rfcomm_send_nsc expects cr to be either 0 or 1 since it is later
passed to __mcc_type macro and shitfed. Unfortunatelly cr extracted
from received frame type was not sanitized and shifted value was passed
resulting in bogus response.

Note: shifted value was also passed to other functions but was used
only in if satements so this bug appears only for NSC case.

This was affecting TC_RFC_BV_25_C PTS qualification test.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---
 net/bluetooth/rfcomm/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index ca957d3..d340577 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1558,7 +1558,7 @@ static int rfcomm_recv_mcc(struct rfcomm_session *s, struct sk_buff *skb)
 	struct rfcomm_mcc *mcc = (void *) skb->data;
 	u8 type, cr, len;
 
-	cr   = __test_cr(mcc->type);
+	cr   = !!__test_cr(mcc->type);
 	type = __get_mcc_type(mcc->type);
 	len  = __get_mcc_len(mcc->len);
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH 2/2] android/pts: Update PTS files for SPP
From: Sebastian Chlad @ 2014-10-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad
In-Reply-To: <1412943017-17823-1-git-send-email-sebastian.chlad@tieto.com>

---
 android/pics-spp.txt  | 2 +-
 android/pixit-spp.txt | 2 +-
 android/pts-spp.txt   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/pics-spp.txt b/android/pics-spp.txt
index 3296738..f015236 100644
--- a/android/pics-spp.txt
+++ b/android/pics-spp.txt
@@ -1,6 +1,6 @@
 SPP PICS for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 # - not yet implemented/supported
diff --git a/android/pixit-spp.txt b/android/pixit-spp.txt
index bb9605e..a8a8202 100644
--- a/android/pixit-spp.txt
+++ b/android/pixit-spp.txt
@@ -1,6 +1,6 @@
 SPP PIXIT for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 & - should be set to IUT Bluetooth address
diff --git a/android/pts-spp.txt b/android/pts-spp.txt
index ba511eb..328c949 100644
--- a/android/pts-spp.txt
+++ b/android/pts-spp.txt
@@ -1,7 +1,7 @@
 PTS test results for SPP
 
-PTS version: 5.2
-Tested: 21-July-2014
+PTS version: 5.3
+Tested: 10-October-2014
 Android version: 4.4.4
 
 Results:
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 1/2] android/pts: Update HSP files for PTS 5.3
From: Sebastian Chlad @ 2014-10-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

---
 android/pics-hsp.txt  | 2 +-
 android/pixit-hsp.txt | 2 +-
 android/pts-hsp.txt   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/pics-hsp.txt b/android/pics-hsp.txt
index 330a197..7e71b14 100644
--- a/android/pics-hsp.txt
+++ b/android/pics-hsp.txt
@@ -1,6 +1,6 @@
 HSP PICS for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 # - not yet implemented/supported
diff --git a/android/pixit-hsp.txt b/android/pixit-hsp.txt
index a097277..6be4731 100644
--- a/android/pixit-hsp.txt
+++ b/android/pixit-hsp.txt
@@ -1,6 +1,6 @@
 HSP PIXIT for the PTS tool.
 
-PTS version: 5.2
+PTS version: 5.3
 
 * - different than PTS defaults
 & - should be set to IUT Bluetooth address
diff --git a/android/pts-hsp.txt b/android/pts-hsp.txt
index 0df9806..9031b82 100644
--- a/android/pts-hsp.txt
+++ b/android/pts-hsp.txt
@@ -1,7 +1,7 @@
 PTS test results for HSP
 
-PTS version: 5.2
-Tested: 15-July-2014
+PTS version: 5.3
+Tested: 10-October-2014
 Android version: 4.4.4
 
 Results:
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH] core: Add Manufacturer Specific Data EIR field
From: Alfonso Acosta @ 2014-10-10 11:37 UTC (permalink / raw)
  To: linux-bluetooth

Athough the Manufacturer Specific Data field is not used internally,
it's useful for external plugins, e.g. iBeacon.
---
 src/eir.c | 8 ++++++++
 src/eir.h | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/src/eir.c b/src/eir.c
index d22ad91..f130855 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -240,6 +240,14 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
 			eir->did_product = data[4] | (data[5] << 8);
 			eir->did_version = data[6] | (data[7] << 8);
 			break;
+
+		case EIR_MANUFACTURER_DATA:
+			if (data_len < 2)
+				break;
+			eir->msd_company = get_le16(data);
+			eir->msd_data_len = data_len - 2;
+			memcpy(&eir->msd_data, data+2, eir->msd_data_len);
+			break;
 		}
 
 		eir_data += field_len + 1;
diff --git a/src/eir.h b/src/eir.h
index e486fa2..88a5bf0 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -37,6 +37,7 @@
 #define EIR_SSP_RANDOMIZER          0x0F  /* SSP Randomizer */
 #define EIR_DEVICE_ID               0x10  /* device ID */
 #define EIR_GAP_APPEARANCE          0x19  /* GAP appearance */
+#define EIR_MANUFACTURER_DATA       0xFF  /* Manufacturer Specific Data */
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -62,6 +63,9 @@ struct eir_data {
 	uint16_t did_product;
 	uint16_t did_version;
 	uint16_t did_source;
+	uint16_t msd_company;
+	uint8_t msd_data[HCI_MAX_EIR_LENGTH];
+	uint8_t msd_data_len;
 };
 
 void eir_data_free(struct eir_data *eir);
-- 
1.9.1


^ permalink raw reply related

* Re: [RFC] need for new scan api for bluetooth low energy.
From: Marcel Holtmann @ 2014-10-10 10:52 UTC (permalink / raw)
  To: Jakub Pawlowski; +Cc: Arman Uguray, BlueZ development, Scott James Remnant
In-Reply-To: <93AC6768-49F3-43CC-8014-83DD3C0EA3D3@holtmann.org>

Hi Jakub,

>>>> Ok, new updated version - we do only passive scan, and we send Device
>>>> found event:
>>>> 
>>>> 
>>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>>> index 11e2490..3d03617 100644
>>>> --- a/doc/mgmt-api.txt
>>>> +++ b/doc/mgmt-api.txt
>>>> @@ -2135,6 +2135,29 @@ Set Public Address Command
>>>>                              Invalid Index
>>>> 
>>>> 
>>>> +Start Passive LE Filtered Device Scan
>>>> +=======================
>>>> +
>>>> +       Command Code:           0x003A
>>>> +       Controller Index:       <controller id>
>>>> +       Command Parameters:     UUID (16 Octets)
>>>> +                    max_pathloss (1 octet)
>>>> +
>>>> +       Return Parameters:
>>>> +
>>>> +       This command starts passive LE scan, and filter received
>>>> advertisements: if AD contains both TX power level and Service
>>>> Solicitation, and UUID is contained in Service Solicitation and
>>>> computed pathloss is smaller than max_pathloss parameter, then a
>>>> Device Found event will be sent.
>>> 
>>> generally we tried to avoid to specific LE only commands. Our attempts with the kernel APIs where to make them as generic as possible. So I think something along the lines of adding and removing UUID filters should be something we should target at.
>>> 
>>> Add UUID Notification Filter
>>> ============================
>>> 
>>>       Command Parameters:     Address_Type (1 Octet)
>>>                               UUID (16 Octets)
>>>                               Max_Pathloss (1 Octet)
>>>       Return Parameters:      Address_Type (1 Octet)
>>> 
>>> 
>>> Remove UUID Notification Filter
>>> ===============================
>>> 
>>>       Command Parameters:     Address_Type (1 Octet)
>>>                               UUID (16 Octets)
>>>       Return Parameters:      Address_Type (1 Octet)
>>> 
>>> The Address_Type would be defined the same way as for Start Discovery command.
>>> 
>>> Now we can discuss if on the mgmt level we want to use pathloss or rather a range of RSSI. I have seen that RSSI is preferred and that the translation from RSSI and advertising data with TX power level is done in the daemon. So for kernel API it might be better to expose RSSI range.
>>> 
>>> Besides a RSSI range, we might also want to allow giving a RSSI threshold that defines how much the RSSI to change between events to be reported again.
>> 
>> Some mobile devices that I'm working can change power level for
>> advertising, that makes raw RSSI filter useless. It must be pathloss,
>> that's why I require TX power in advertisement.
> 
> you will still get it, but just have to do the math in the daemon. The only difference is that the daemon gets woken up for RSSI ranges that in the end you might filter out.
> 
>>> 
>>> That said, the danger with a generic notification filter like this is that we can not implement it efficiently with current hardware without vendor commands. If you use duplicate filtering, then Broadcom and CSR chips behave fully different. Broadcom only filters on BD_ADDR and report devices once no matter what the RSSI, while CSR will report the same device again if the RSSI changes.
>>> 
>>> With that in mind, it might be safer to introduce a one-shot mgmt API that needs to be repeatedly called to keep scanning for new devices.
>>> 
>>> Start Service Discovery
>>> =======================
>>> 
>>>       Command Parameters:     Address_Type (1 Octet)
>>>                               Min_RSSI (1 Octet)
>>>                               Max_RSSI (1 Octet)
>>>                               Num_UUID (1 Octet)
>>>                               UUID[i] (16 Octets)
>>>       Return Parameters:      Address_Type (1 Octet)
>>> 
>>> Stop Service Discovery
>>> ======================
>>> 
>>>       Command Parameters:     Address_Type (1 Octet)
>>>       Return Parameters:      Address_Type (1 Octet)
>>> 
>>> Without having dedicated controller support for filtering, having such a dedicated discovery for services with a specific UUID seems to be more appropriate and more safe. Since we could always add controller based filtering for background scanning once there is controller support.
>> 
>> I preffer the "Add UUID Notification Filter" and "Remove UUID
>> Notification Filter". For controllers like Broadcom that reports only
>> one advertisement can we restart scan internally in kernel ? Why
>> propagate that event higher ?
> 
> Personally I would prefer just adding another filter as well. However it might be better to start with something like a one-shot handling and see where that is taking us. It is a little bit less intrusive and you could also use active scanning. At the same time you could also easily make it work for BR/EDR. I am thinking here along the lines of something wanting to pair a mouse. You just give the HID UUID and start interleaved discovery for that service on both transports.

I send a proposal for doing an one-shot service discovery. So you can propose a RSSI Threshold and a list of possible UUIDs to look for.

If RSSI Threshold is 127 or the Num_UUID is 0, then it will behave the same way as Start Discovery. I think this is how we should get started to tackle this problem.

Regards

Marcel


^ permalink raw reply

* [PATCH] doc: Add commands for Start Service Discovery and Stop Service Discovery
From: Marcel Holtmann @ 2014-10-10 10:49 UTC (permalink / raw)
  To: linux-bluetooth

---
 doc/mgmt-api.txt | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index daf036b4c1ec..2038f883cc95 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -2135,6 +2135,89 @@ Set Public Address Command
 				Invalid Index
 
 
+Start Service Discovery Command
+===============================
+
+	Command Code:		0x003a
+	Controller Index:	<controller id>
+	Command Parameters:	Address_Type (1 Octet)
+				RSSI_Threshold (1 Octet)
+				Num_UUID (1 Octet)
+				UUID[i] (16 Octets)
+	Return Parameters:	Address_Type (1 Octet)
+
+	This command is used to start the process of discovering remote
+	devices with a specific UUID. A Device Found event will be sent
+	for each discovered device.
+
+	Possible values for the Address_Type parameter are a bit-wise or
+	of the following bits:
+
+		0	BR/EDR
+		1	LE Public
+		2	LE Random
+
+	By combining these e.g. the following values are possible:
+
+		1	BR/EDR
+		6	LE (public & random)
+		7	BR/EDR/LE (interleaved discovery)
+
+	The service discovery uses active scanning for Low Energy scanning
+	and will search for UUID in both advertising data and scan response
+	data.
+
+	Found devices that have a RSSI value smaller than RSSI_Threshold
+	are not reported via DeviceFound event. Setting a value of 127
+	will cause all devices to be reported.
+
+	The list of UUIDs identifies a logical OR. Only one of the UUIDs
+	have to match to cause a DeviceFound event. Providing an empty
+	list of UUIDs with Num_UUID set to 0 which means that DeviceFound
+	events are send out for all devices above the RSSI_Threshold.
+
+	In case the RSSI_Threshold is set to 127 and the Num_UUID is 0,
+	then this command behaves exactly the same as Start Discovery.
+
+	When the discovery procedure starts the Discovery event will
+	notify this similar to Start Discovery.
+
+	This command can only be used when the controller is powered.
+
+	This command generates a Command Complete event on success
+	or failure.
+
+	Possible errors:	Busy
+				Not Supported
+				Invalid Parameters
+				Not Powered
+				Invalid Index
+
+
+Stop Service Discovery Command
+==============================
+
+	Command Code:		0x003b
+	Controller Index:	<controller id>
+	Command Parameters:	Address_Type (1 Octet)
+	Return Parameters:	Address_Type (1 Octet)
+
+	This command is used to stop the service discovery process started
+	using the Start Service Discovery command.
+
+	When the discovery procedure stops the Discovery event will
+	notify this similar to Stop Discovery.
+
+	This command can only be used when the controller is powered.
+
+	This command generates a Command Complete event on success
+	or failure.
+
+	Possible errors:	Rejected
+				Invalid Parameters
+				Invalid Index
+
+
 Command Complete Event
 ======================
 
@@ -2555,7 +2638,10 @@ Discovering Event
 
 	This event indicates that the controller has started discovering
 	devices. This discovering state can come and go multiple times
-	between a StartDiscover and a StopDiscovery command.
+	between a Start Discover and a Stop Discovery commands.
+
+	The Start Service Discovery and Stop Service Discovery commands
+	will also trigger this event.
 
 	The valid values for the Discovering parameter are 0x01
 	(enabled) and 0x00 (disabled).
-- 
1.9.3


^ permalink raw reply related

* Re: [PATCH] Adds Sony Move Navigation controller.
From: Szymon Janc @ 2014-10-10 10:07 UTC (permalink / raw)
  To: Alex Gal; +Cc: linux-bluetooth
In-Reply-To: <1412697877-7508-1-git-send-email-a.gal@miip.ca>

Hi Alex,

Thanks for patch, some small comments from my side.

On Tuesday 07 of October 2014 12:04:37 Alex Gal wrote:
> Signed-off-by: Alex <a.gal@miip.ca>

We don't use SOB in userspace so this is not needed.

Also I'd prefer if you could put plugin change into separate patch.
And prefix them appropriately ie. "sixaxis: " for plugin and "input: "
for input change.

> ---
>  plugins/sixaxis.c       | 7 +++++++
>  profiles/input/server.c | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index b09404a..6f7893e 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -60,6 +60,13 @@ static const struct {
>  		.pid = 0x0268,
>  		.version = 0x0000,
>  	},
> +	{
> +		.name = "PLAYSTATION(R)3 Controller", /* Move Navigation */

Does Move Navigation use same name as DS3?

> +		.source = 0x0002,
> +		.vid = 0x054c,
> +		.pid = 0x042f,
> +		.version = 0x0000,
> +	},
>  };
>  
>  struct leds_data {
> diff --git a/profiles/input/server.c b/profiles/input/server.c
> index 307db96..5463311 100644
> --- a/profiles/input/server.c
> +++ b/profiles/input/server.c
> @@ -140,6 +140,10 @@ static bool dev_is_sixaxis(const bdaddr_t *src, const bdaddr_t *dst)
>  	if (vid == 0x054c && pid == 0x05c4)
>  		return true;
>  
> +    /* Move Navigation */
> +	if (vid == 0x054c && pid == 0x042f)
> +		return true;
> +
>  	return false;
>  }
>  
> 

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* [PATCH v2] Bluetooth: Defer connection-parameter removal when unpairing without disconnecting
From: Alfonso Acosta @ 2014-10-10  9:16 UTC (permalink / raw)
  To: linux-bluetooth

Systematically removing the LE connection parameters and autoconnect
action is inconvenient for rebonding without disconnecting from
userland (i.e. unpairing followed by repairing without
disconnecting). The parameters will be lost after unparing and
userland needs to take care of book-keeping them and re-adding them.

This patch allows userland to forget about parameter management when
rebonding without disconnecting. It defers clearing the connection
parameters when unparing without disconnecting, giving a chance of
keeping the parameters if a repairing happens before the connection is
closed.

Signed-off-by: Alfonso Acosta <fons@spotify.com>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_conn.c         |  6 ++++++
 net/bluetooth/mgmt.c             | 27 +++++++++++++++++++++------
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 07ddeed62..b8685a7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -555,6 +555,7 @@ enum {
 	HCI_CONN_STK_ENCRYPT,
 	HCI_CONN_AUTH_INITIATOR,
 	HCI_CONN_DROP,
+	HCI_CONN_PARAM_REMOVAL_PEND,
 };
 
 static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b9517bd..eb9988f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -356,6 +356,9 @@ static void hci_conn_timeout(struct work_struct *work)
 		conn->state = BT_CLOSED;
 		break;
 	}
+
+	if (test_and_clear_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags))
+		hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
 }
 
 /* Enter sniff mode */
@@ -544,6 +547,9 @@ int hci_conn_del(struct hci_conn *conn)
 
 	hci_conn_del_sysfs(conn);
 
+	if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags))
+		hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
+
 	hci_dev_put(hdev);
 
 	hci_conn_put(conn);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3fd88b0..943df45 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2699,7 +2699,7 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct mgmt_rp_unpair_device rp;
 	struct hci_cp_disconnect dc;
 	struct pending_cmd *cmd;
-	struct hci_conn *conn;
+	struct hci_conn *uninitialized_var(conn);
 	int err;
 
 	memset(&rp, 0, sizeof(rp));
@@ -2736,7 +2736,17 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
 
-		hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
+		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
+					       &cp->addr.bdaddr);
+
+		/* If the BLE connection is being used, defer clearing up
+		 * the connection parameters until closing to give a
+		 * chance of keeping them if a repairing happens.
+		 */
+		if (conn && !cp->disconnect)
+			set_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags);
+		else
+			hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
 
 		err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
 	}
@@ -2748,12 +2758,12 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	}
 
 	if (cp->disconnect) {
+		/* Only lookup the connection in the BR/EDR since the
+		 * LE connection was already looked up earlier.
+		 */
 		if (cp->addr.type == BDADDR_BREDR)
 			conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
 						       &cp->addr.bdaddr);
-		else
-			conn = hci_conn_hash_lookup_ba(hdev, LE_LINK,
-						       &cp->addr.bdaddr);
 	} else {
 		conn = NULL;
 	}
@@ -3212,8 +3222,13 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	cmd->user_data = hci_conn_get(conn);
 
 	if ((conn->state == BT_CONNECTED || conn->state == BT_CONFIG) &&
-	    hci_conn_security(conn, sec_level, auth_type, true))
+	    hci_conn_security(conn, sec_level, auth_type, true)) {
+		/* The device is paired so there is no need to remove
+		 * its connection parameters anymore.
+		 */
+		clear_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags);
 		pairing_complete(cmd, 0);
+	}
 
 	err = 0;
 
-- 
1.9.1


^ permalink raw reply related

* [PATCHv4 14/14] shared/gatt: Fix searching descriptors
From: Marcin Kraglak @ 2014-10-10  9:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1412932548-28363-1-git-send-email-marcin.kraglak@tieto.com>

Descriptor discovery range started from characteristic value handle + 1
and end with characteristic end handle. If characteristic value handle
is 0xffff, then discovery range was set to 0x0000-0xffff.
Found during PTS test case TC_GAD_CL_BV_03_C.
---
 src/shared/gatt-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 679242c..045f9f9 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -714,8 +714,8 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 	for (i = 0; i < chrc_count; i++) {
 		op->cur_chrc_index = i;
 		op->cur_chrc = chrcs + i;
-		desc_start = chrcs[i].chrc_external.value_handle + 1;
-		if (desc_start > chrcs[i].chrc_external.end_handle)
+		desc_start = chrcs[i].chrc_external.value_handle;
+		if (desc_start++ == chrcs[i].chrc_external.end_handle)
 			continue;
 
 		if (bt_gatt_discover_descriptors(client->att, desc_start,
-- 
1.9.3


^ permalink raw reply related

* [PATCHv4 13/14] tools/btgatt-client: Print found include services
From: Marcin Kraglak @ 2014-10-10  9:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1412932548-28363-1-git-send-email-marcin.kraglak@tieto.com>

---
 tools/btgatt-client.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 5c692ff..5465d53 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -174,7 +174,9 @@ static void print_uuid(const uint8_t uuid[16])
 static void print_service(const bt_gatt_service_t *service)
 {
 	struct bt_gatt_characteristic_iter iter;
+	struct bt_gatt_incl_service_iter include_iter;
 	const bt_gatt_characteristic_t *chrc;
+	const bt_gatt_included_service_t *incl;
 	size_t i;
 
 	if (!bt_gatt_characteristic_iter_init(&iter, service)) {
@@ -182,12 +184,24 @@ static void print_service(const bt_gatt_service_t *service)
 		return;
 	}
 
+	if (!bt_gatt_include_service_iter_init(&include_iter, service)) {
+		PRLOG("Failed to initialize include service iterator\n");
+		return;
+	}
 	printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
 				"end: 0x%04x, uuid: ",
 				service->primary ? "primary" : "second.",
 				service->start_handle, service->end_handle);
 	print_uuid(service->uuid);
 
+	while (bt_gatt_include_service_iter_next(&include_iter, &incl)) {
+		printf("\t  " COLOR_GREEN "include" COLOR_OFF " - handle: "
+					"0x%04x, - start: 0x%04x, end: 0x%04x,"
+					"uuid: ", incl->handle,
+					incl->start_handle, incl->end_handle);
+			print_uuid(incl->uuid);
+	}
+
 	while (bt_gatt_characteristic_iter_next(&iter, &chrc)) {
 		printf("\t  " COLOR_YELLOW "charac" COLOR_OFF
 				" - start: 0x%04x, end: 0x%04x, "
-- 
1.9.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox