[PATCH 1/3] Bluetooth: Refactor advertising report processing into its own function

[PATCH 1/3] Bluetooth: Refactor advertising report processing into its own function

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

As preparation for merging adv_ind/adv_direct_ind and scan_rsp together
into a single mgmt Device Found event refactor individual advertising
report handling into a separate function. This will help keep the code
more readable as more logic gets added.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9ee081b9c064..43872af20aa4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3941,25 +3941,30 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 	}
 }
 
+static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
+			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
+{
+	if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
+		check_pending_le_conn(hdev, bdaddr, bdaddr_type);
+
+	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
+			  data, len);
+}
+
 static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	u8 num_reports = skb->data[0];
 	void *ptr = &skb->data[1];
-	s8 rssi;
 
 	hci_dev_lock(hdev);
 
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
-
-		if (ev->evt_type == LE_ADV_IND ||
-		    ev->evt_type == LE_ADV_DIRECT_IND)
-			check_pending_le_conn(hdev, &ev->bdaddr,
-					      ev->bdaddr_type);
+		s8 rssi;
 
 		rssi = ev->data[ev->length];
-		mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
-				  NULL, rssi, 0, 1, ev->data, ev->length);
+		process_adv_report(hdev, ev->evt_type, &ev->bdaddr,
+				   ev->bdaddr_type, rssi, ev->data, ev->length);
 
 		ptr += sizeof(*ev) + ev->length + 1;
 	}
-- 
1.8.5.3

[PATCH 2/3] Bluetooth: Don't send device found events during passive scanning

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Passive LE scanning is only used by the kernel-internal connection
establishment procedure. It makes therefore little sense to send device
found events to user space.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 43872af20aa4..403c1d96331a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3944,8 +3944,12 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
 			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
 {
-	if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
-		check_pending_le_conn(hdev, bdaddr, bdaddr_type);
+	/* Passive scanning shouldn't trigger any device found events */
+	if (hdev->le_scan_type == LE_SCAN_PASSIVE) {
+		if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
+			check_pending_le_conn(hdev, bdaddr, bdaddr_type);
+	    return;
+	}
 
 	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
 			  data, len);
-- 
1.8.5.3

Re: [PATCH 2/3] Bluetooth: Don't send device found events during passive scanning

[Marcel Holtmann]

Hi Johan,

> Passive LE scanning is only used by the kernel-internal connection
> establishment procedure. It makes therefore little sense to send device
> found events to user space.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_event.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 43872af20aa4..403c1d96331a 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3944,8 +3944,12 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
> static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
> 			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
> {
> -	if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
> -		check_pending_le_conn(hdev, bdaddr, bdaddr_type);
> +	/* Passive scanning shouldn't trigger any device found events */
> +	if (hdev->le_scan_type == LE_SCAN_PASSIVE) {
> +		if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
> +			check_pending_le_conn(hdev, bdaddr, bdaddr_type);
> +	    return;

the indentation is sort of screwed up here.

> +	}
> 
> 	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
> 			  data, len);

And strictly speaking the mgmt_device_found checks by itself if it is discovering mode or not. So events will only be send out when discovering. However I think it is a good idea to shortcut this function when passive scanning.

One minor details is now that devices will no longer be connected when doing active discovery. So when you discover a device during discovery, it will not be connected, it has to wait for the next passive scan. Not sure if we really want that. Especially with direct advertising, we really want to connect right away.

Regards

Marcel

[PATCH 3/3] Bluetooth: Merge adv_ind/adv_direct_ind ans scan_rsp together

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

To avoid too many events being sent to user space and to help parsing of
all available remote device data it makes sense for us to wait for the
scan response and send a single merged Device Found event to user space.

This patch adds a new hdev->pending_adv cache which gets allocated
whenever active scanning gets started and freed when scanning stops. The
cache is able to hold a single advertising report and is operated upon
each time we get an advertising report event.

Normally we simply merge scan responses into data in the cache that was
gotten from a previous adv_ind or adv_direct_ind report. Exceptions are
if we don't get a matching scan response (or no scan response at all) or
if there is not enough space in the cache, in which case we force
sending of any data that there is before continuing further.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 10 ++++++
 net/bluetooth/hci_core.c         |  3 ++
 net/bluetooth/hci_event.c        | 74 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5f8bc05694ac..323f1e64e299 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -148,9 +148,18 @@ struct amp_assoc {
 	__u8	data[HCI_MAX_AMP_ASSOC_SIZE];
 };
 
+struct hci_adv_ind {
+	bdaddr_t	bdaddr;
+	u8		bdaddr_type;
+	s8		rssi;
+	u8		len;
+	u8		data[256];
+};
+
 #define HCI_MAX_PAGES	3
 
 #define NUM_REASSEMBLY 4
+
 struct hci_dev {
 	struct list_head list;
 	struct mutex	lock;
@@ -282,6 +291,7 @@ struct hci_dev {
 
 	struct crypto_blkcipher	*tfm_aes;
 
+	struct hci_adv_ind	*pending_adv;
 	struct discovery_state	discovery;
 	struct hci_conn_hash	conn_hash;
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1c6ffaa8902f..a8e225899e6a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2437,6 +2437,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	kfree_skb(hdev->recv_evt);
 	hdev->recv_evt = NULL;
 
+	kfree(hdev->pending_adv);
+	hdev->pending_adv = NULL;
+
 	/* After this point our queues are empty
 	 * and no tasks are scheduled. */
 	hdev->close(hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 403c1d96331a..f35a398e80a9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1018,6 +1018,12 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_dev_unlock(hdev);
 }
 
+#define ADV_CACHE_EMPTY(p)	(!bacmp(&(p)->bdaddr, BDADDR_ANY))
+#define ADV_CACHE_CLEAR(p)	do { \
+					bacpy(&(p)->bdaddr, BDADDR_ANY); \
+					(p)->len = 0; \
+				} while (0)
+
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 				      struct sk_buff *skb)
 {
@@ -1036,9 +1042,31 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 	switch (cp->enable) {
 	case LE_SCAN_ENABLE:
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
+
+		if (hdev->le_scan_type != LE_SCAN_ACTIVE)
+			break;
+
+		/* Allocate buffer for merging ADV_IND and SCAN_RSP */
+		if (!hdev->pending_adv)
+			hdev->pending_adv = kzalloc(sizeof(*hdev->pending_adv),
+						    GFP_KERNEL);
+
 		break;
 
 	case LE_SCAN_DISABLE:
+		if (hdev->pending_adv) {
+			struct hci_adv_ind *p = hdev->pending_adv;
+
+			if (!ADV_CACHE_EMPTY(p))
+				mgmt_device_found(hdev, &p->bdaddr, LE_LINK,
+						  p->bdaddr_type, NULL,
+						  p->rssi, 0, 1, p->data,
+						  p->len);
+
+			hdev->pending_adv = NULL;
+			kfree(p);
+		}
+
 		/* Cancel this timer so that we don't try to disable scanning
 		 * when it's already disabled.
 		 */
@@ -3944,6 +3972,8 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
 			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
 {
+	struct hci_adv_ind *p = hdev->pending_adv;
+
 	/* Passive scanning shouldn't trigger any device found events */
 	if (hdev->le_scan_type == LE_SCAN_PASSIVE) {
 		if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
@@ -3951,8 +3981,48 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
 	    return;
 	}
 
-	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
-			  data, len);
+	/* If we don't have a cache, or we don't have data in the cache
+	 * and we got a scan response just send the mgmt event without
+	 * doing anything else.
+	 */
+	if (!p || (ADV_CACHE_EMPTY(p) && type == LE_ADV_SCAN_RSP)) {
+		mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL,
+				  rssi, 0, 1, data, len);
+		return;
+	}
+
+	/* If there's nothing cached proceed to updating the cache */
+	if (ADV_CACHE_EMPTY(p))
+		goto update_cache;
+
+	/* If the cached data doesn't match this report or there is not
+	 * enough space in the cache (unlikely but we have to check for
+	 * it to avoid buffer overflow) then force sending of the cached
+	 * data.
+	 */
+	if (bacmp(bdaddr, &p->bdaddr) || bdaddr_type != p->bdaddr_type ||
+	    len > (sizeof(p->data) - p->len)) {
+		mgmt_device_found(hdev, &p->bdaddr, LE_LINK, p->bdaddr_type,
+				  NULL, p->rssi, 0, 1, p->data, p->len);
+		ADV_CACHE_CLEAR(p);
+	}
+
+update_cache:
+	/* Update the cache with data from this report */
+	bacpy(&p->bdaddr, bdaddr);
+	p->bdaddr_type = bdaddr_type;
+	p->rssi = rssi;
+	memcpy(p->data + p->len, data, len);
+	p->len += len;
+
+	/* If this report will not trigger a scan request just send the
+	 * device found event without doing anything else.
+	 */
+	if (type != LE_ADV_IND && type != LE_ADV_DIRECT_IND) {
+		mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL,
+				  rssi, 0, 1, p->data, p->len);
+		ADV_CACHE_CLEAR(p);
+	}
 }
 
 static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
-- 
1.8.5.3

Re: [PATCH 3/3] Bluetooth: Merge adv_ind/adv_direct_ind ans scan_rsp together

[Marcel Holtmann]

Hi Johan,

> To avoid too many events being sent to user space and to help parsing of
> all available remote device data it makes sense for us to wait for the
> scan response and send a single merged Device Found event to user space.
> 
> This patch adds a new hdev->pending_adv cache which gets allocated
> whenever active scanning gets started and freed when scanning stops. The
> cache is able to hold a single advertising report and is operated upon
> each time we get an advertising report event.
> 
> Normally we simply merge scan responses into data in the cache that was
> gotten from a previous adv_ind or adv_direct_ind report. Exceptions are
> if we don't get a matching scan response (or no scan response at all) or
> if there is not enough space in the cache, in which case we force
> sending of any data that there is before continuing further.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 10 ++++++
> net/bluetooth/hci_core.c         |  3 ++
> net/bluetooth/hci_event.c        | 74 ++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 5f8bc05694ac..323f1e64e299 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -148,9 +148,18 @@ struct amp_assoc {
> 	__u8	data[HCI_MAX_AMP_ASSOC_SIZE];
> };
> 
> +struct hci_adv_ind {
> +	bdaddr_t	bdaddr;
> +	u8		bdaddr_type;
> +	s8		rssi;
> +	u8		len;
> +	u8		data[256];

I think this should be HCI_MAX_AD_LENGTH instead of 256.

> +};
> +
> #define HCI_MAX_PAGES	3
> 
> #define NUM_REASSEMBLY 4
> +
> struct hci_dev {
> 	struct list_head list;
> 	struct mutex	lock;
> @@ -282,6 +291,7 @@ struct hci_dev {
> 
> 	struct crypto_blkcipher	*tfm_aes;
> 
> +	struct hci_adv_ind	*pending_adv;
> 	struct discovery_state	discovery;

so my idea would have been to include this into the discovery_state structure.

@@ -68,6 +68,10 @@ struct discovery_state {
        struct list_head        unknown;        /* Name state not known */
        struct list_head        resolve;        /* Name needs to be resolved */
        __u32                   timestamp;
+       bdaddr_t                last_adv_addr;
+       u8                      last_adv_addr_type;
+       u8                      last_adv_data[HCI_MAX_AD_LENGTH];
+       u8                      last_adv_data_len;
 };

That is what I was thinking about. I left the RSSI value out since in case there is no scan response we might better send it without RSSI value since the report would be delayed. However we can also store it and use it.

> 	struct hci_conn_hash	conn_hash;
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 1c6ffaa8902f..a8e225899e6a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2437,6 +2437,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> 	kfree_skb(hdev->recv_evt);
> 	hdev->recv_evt = NULL;
> 
> +	kfree(hdev->pending_adv);
> +	hdev->pending_adv = NULL;
> +
> 	/* After this point our queues are empty
> 	 * and no tasks are scheduled. */
> 	hdev->close(hdev);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 403c1d96331a..f35a398e80a9 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1018,6 +1018,12 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
> 	hci_dev_unlock(hdev);
> }
> 
> +#define ADV_CACHE_EMPTY(p)	(!bacmp(&(p)->bdaddr, BDADDR_ANY))
> +#define ADV_CACHE_CLEAR(p)	do { \
> +					bacpy(&(p)->bdaddr, BDADDR_ANY); \
> +					(p)->len = 0; \
> +				} while (0)
> +
> static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
> 				      struct sk_buff *skb)
> {
> @@ -1036,9 +1042,31 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
> 	switch (cp->enable) {
> 	case LE_SCAN_ENABLE:
> 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
> +
> +		if (hdev->le_scan_type != LE_SCAN_ACTIVE)
> +			break;
> +
> +		/* Allocate buffer for merging ADV_IND and SCAN_RSP */
> +		if (!hdev->pending_adv)
> +			hdev->pending_adv = kzalloc(sizeof(*hdev->pending_adv),
> +						    GFP_KERNEL);
> +

So instead of allocating it, sticking it into discovery_state seems better and in the end a lot simpler.

> 		break;
> 
> 	case LE_SCAN_DISABLE:
> +		if (hdev->pending_adv) {
> +			struct hci_adv_ind *p = hdev->pending_adv;
> +
> +			if (!ADV_CACHE_EMPTY(p))
> +				mgmt_device_found(hdev, &p->bdaddr, LE_LINK,
> +						  p->bdaddr_type, NULL,
> +						  p->rssi, 0, 1, p->data,
> +						  p->len);
> +
> +			hdev->pending_adv = NULL;
> +			kfree(p);
> +		}
> +

I wonder if this should not all be centralized in the discovery set_state function.

> 		/* Cancel this timer so that we don't try to disable scanning
> 		 * when it's already disabled.
> 		 */
> @@ -3944,6 +3972,8 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
> static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
> 			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
> {
> +	struct hci_adv_ind *p = hdev->pending_adv;
> +
> 	/* Passive scanning shouldn't trigger any device found events */
> 	if (hdev->le_scan_type == LE_SCAN_PASSIVE) {
> 		if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
> @@ -3951,8 +3981,48 @@ static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
> 	    return;
> 	}
> 
> -	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
> -			  data, len);
> +	/* If we don't have a cache, or we don't have data in the cache
> +	 * and we got a scan response just send the mgmt event without
> +	 * doing anything else.
> +	 */
> +	if (!p || (ADV_CACHE_EMPTY(p) && type == LE_ADV_SCAN_RSP)) {
> +		mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL,
> +				  rssi, 0, 1, data, len);
> +		return;
> +	}
> +
> +	/* If there's nothing cached proceed to updating the cache */
> +	if (ADV_CACHE_EMPTY(p))
> +		goto update_cache;
> +
> +	/* If the cached data doesn't match this report or there is not
> +	 * enough space in the cache (unlikely but we have to check for
> +	 * it to avoid buffer overflow) then force sending of the cached
> +	 * data.
> +	 */
> +	if (bacmp(bdaddr, &p->bdaddr) || bdaddr_type != p->bdaddr_type ||
> +	    len > (sizeof(p->data) - p->len)) {
> +		mgmt_device_found(hdev, &p->bdaddr, LE_LINK, p->bdaddr_type,
> +				  NULL, p->rssi, 0, 1, p->data, p->len);
> +		ADV_CACHE_CLEAR(p);
> +	}
> +
> +update_cache:
> +	/* Update the cache with data from this report */
> +	bacpy(&p->bdaddr, bdaddr);
> +	p->bdaddr_type = bdaddr_type;
> +	p->rssi = rssi;
> +	memcpy(p->data + p->len, data, len);
> +	p->len += len;
> +
> +	/* If this report will not trigger a scan request just send the
> +	 * device found event without doing anything else.
> +	 */
> +	if (type != LE_ADV_IND && type != LE_ADV_DIRECT_IND) {

		s/LE_ADV_DIRECT_IND/LE_ADV_SCAN_IND/

> +		mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL,
> +				  rssi, 0, 1, p->data, p->len);
> +		ADV_CACHE_CLEAR(p);
> +	}
> }

Regards

Marcel

Re: [PATCH 3/3] Bluetooth: Merge adv_ind/adv_direct_ind ans scan_rsp together

[Johan Hedberg]

Hi Marcel,

On Sat, Mar 22, 2014, Marcel Holtmann wrote:
> > 	case LE_SCAN_DISABLE:
> > +		if (hdev->pending_adv) {
> > +			struct hci_adv_ind *p = hdev->pending_adv;
> > +
> > +			if (!ADV_CACHE_EMPTY(p))
> > +				mgmt_device_found(hdev, &p->bdaddr, LE_LINK,
> > +						  p->bdaddr_type, NULL,
> > +						  p->rssi, 0, 1, p->data,
> > +						  p->len);
> > +
> > +			hdev->pending_adv = NULL;
> > +			kfree(p);
> > +		}
> > +
> 
> I wonder if this should not all be centralized in the discovery set_state function.

I'm fine with all your other suggestions (and have mostly completed
implementing them), but since DISCOVERY_STOPPED is set only at the end
of inquiry in the case of dual-mode discovery I think it's better to do
the sending of the pending device found event here when LE scanning stops.


Johan

Re: [PATCH 1/3] Bluetooth: Refactor advertising report processing into its own function

[Marcel Holtmann]

Hi Johan,

> As preparation for merging adv_ind/adv_direct_ind and scan_rsp together
> into a single mgmt Device Found event refactor individual advertising
> report handling into a separate function. This will help keep the code
> more readable as more logic gets added.

actually only ADV_IND and ADV_SCAN_IND can be followed by a SCAN_REQ/SCAN_RSP. The ADV_DIRECT_IND and ADV_NONCONN_IND will never see a scan response advertising report.

> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_event.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 9ee081b9c064..43872af20aa4 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3941,25 +3941,30 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
> 	}
> }
> 
> +static void process_adv_report(struct hci_dev *hdev, u8 type, bdaddr_t *bdaddr,
> +			       u8 bdaddr_type, s8 rssi, u8 *data, u8 len)
> +{
> +	if (type == LE_ADV_IND || type == LE_ADV_DIRECT_IND)
> +		check_pending_le_conn(hdev, bdaddr, bdaddr_type);
> +
> +	mgmt_device_found(hdev, bdaddr, LE_LINK, bdaddr_type, NULL, rssi, 0, 1,
> +			  data, len);
> +}
> +
> static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> 	u8 num_reports = skb->data[0];
> 	void *ptr = &skb->data[1];
> -	s8 rssi;
> 
> 	hci_dev_lock(hdev);
> 
> 	while (num_reports--) {
> 		struct hci_ev_le_advertising_info *ev = ptr;
> -
> -		if (ev->evt_type == LE_ADV_IND ||
> -		    ev->evt_type == LE_ADV_DIRECT_IND)
> -			check_pending_le_conn(hdev, &ev->bdaddr,
> -					      ev->bdaddr_type);
> +		s8 rssi;
> 
> 		rssi = ev->data[ev->length];
> -		mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
> -				  NULL, rssi, 0, 1, ev->data, ev->length);
> +		process_adv_report(hdev, ev->evt_type, &ev->bdaddr,
> +				   ev->bdaddr_type, rssi, ev->data, ev->length);
> 
> 		ptr += sizeof(*ev) + ev->length + 1;

So while the patch itself is fine to me, I think we need to update the commit message to make clear what the intention is.

Regards

Marcel