This patch moves whole packet filering logic of service discovery
into new function is_filter_match. It's done because logic inside
mgmt_device_found is very complicated and needs some
simplification.
Also having whole logic in one place will allow to simplify it in
the future.
Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/mgmt.c | 141 +++++++++++++++++++++++++++++----------------------
1 file changed, 79 insertions(+), 62 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1e4635a..a41a5ef 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7280,32 +7280,16 @@ static void restart_le_scan(struct hci_dev *hdev)
DISCOV_LE_RESTART_DELAY);
}
-void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
- u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
- u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
+static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
+ u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
{
- char buf[512];
- struct mgmt_ev_device_found *ev = (void *) buf;
- size_t ev_size;
bool match;
- /* Don't send events for a non-kernel initiated discovery. With
- * LE one exception is if we have pend_le_reports > 0 in which
- * case we're doing passive scanning and want these events.
- */
- if (!hci_discovery_active(hdev)) {
- if (link_type == ACL_LINK)
- return;
- if (link_type == LE_LINK && list_empty(&hdev->pend_le_reports))
- return;
- }
-
- /* When using service discovery with a RSSI threshold, then check
- * if such a RSSI threshold is specified. If a RSSI threshold has
- * been specified, and HCI_QUIRK_STRICT_DUPLICATE_FILTER is not set,
- * then all results with a RSSI smaller than the RSSI threshold will be
- * dropped. If the quirk is set, let it through for further processing,
- * as we might need to restart the scan.
+ /* If a RSSI threshold has been specified, and
+ * HCI_QUIRK_STRICT_DUPLICATE_FILTER is not set, then all results with
+ * a RSSI smaller than the RSSI threshold will be dropped. If the quirk
+ * is set, let it through for further processing, as we might need to
+ * restart the scan.
*
* For BR/EDR devices (pre 1.2) providing no RSSI during inquiry,
* the results are also dropped.
@@ -7314,32 +7298,8 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
(rssi == HCI_RSSI_INVALID ||
(rssi < hdev->discovery.rssi &&
!test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks))))
- return;
-
- /* Make sure that the buffer is big enough. The 5 extra bytes
- * are for the potential CoD field.
- */
- if (sizeof(*ev) + eir_len + scan_rsp_len + 5 > sizeof(buf))
- return;
-
- memset(buf, 0, sizeof(buf));
-
- /* In case of device discovery with BR/EDR devices (pre 1.2), the
- * RSSI value was reported as 0 when not available. This behavior
- * is kept when using device discovery. This is required for full
- * backwards compatibility with the API.
- *
- * However when using service discovery, the value 127 will be
- * returned when the RSSI is not available.
- */
- if (rssi == HCI_RSSI_INVALID && !hdev->discovery.report_invalid_rssi &&
- link_type == ACL_LINK)
- rssi = 0;
+ return false;
- bacpy(&ev->addr.bdaddr, bdaddr);
- ev->addr.type = link_to_bdaddr(link_type, addr_type);
- ev->rssi = rssi;
- ev->flags = cpu_to_le32(flags);
if (eir_len > 0) {
/* When using service discovery and a list of UUID is
@@ -7364,25 +7324,18 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
}
if (!match && !scan_rsp_len)
- return;
-
- /* Copy EIR or advertising data into event */
- memcpy(ev->eir, eir, eir_len);
+ return false;
} else {
/* When using service discovery and a list of UUID is
* provided, results with empty EIR or advertising data
* should be dropped since they do not match any UUID.
*/
if (hdev->discovery.uuid_count > 0 && !scan_rsp_len)
- return;
+ return false;
match = false;
}
- if (dev_class && !eir_has_data_type(ev->eir, eir_len, EIR_CLASS_OF_DEV))
- eir_len = eir_append_data(ev->eir, eir_len, EIR_CLASS_OF_DEV,
- dev_class, 3);
-
if (scan_rsp_len > 0) {
/* When using service discovery and a list of UUID is
* provided, results with no matching UUID should be
@@ -7393,7 +7346,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
if (!match && !eir_has_uuids(scan_rsp, scan_rsp_len,
hdev->discovery.uuid_count,
hdev->discovery.uuids))
- return;
+ return false;
/* If duplicate filtering does not report RSSI changes,
* then restart scanning to ensure updated result with
@@ -7403,16 +7356,13 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
&hdev->quirks))
restart_le_scan(hdev);
}
-
- /* Append scan response data to event */
- memcpy(ev->eir + eir_len, scan_rsp, scan_rsp_len);
} else {
/* When using service discovery and a list of UUID is
* provided, results with empty scan response and no
* previous matched advertising data should be dropped.
*/
if (hdev->discovery.uuid_count > 0 && !match)
- return;
+ return false;
}
/* Validate the reported RSSI value against the RSSI threshold once more
@@ -7421,8 +7371,75 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
*/
if (hdev->discovery.rssi != HCI_RSSI_INVALID &&
rssi < hdev->discovery.rssi)
+ return false;
+
+ return true;
+}
+
+void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
+ u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
+ u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
+{
+ char buf[512];
+ struct mgmt_ev_device_found *ev = (void *)buf;
+ size_t ev_size;
+
+ /* Don't send events for a non-kernel initiated discovery. With
+ * LE one exception is if we have pend_le_reports > 0 in which
+ * case we're doing passive scanning and want these events.
+ */
+ if (!hci_discovery_active(hdev)) {
+ if (link_type == ACL_LINK)
+ return;
+ if (link_type == LE_LINK && list_empty(&hdev->pend_le_reports))
+ return;
+ }
+
+ if (hdev->discovery.rssi != HCI_RSSI_INVALID ||
+ hdev->discovery.uuid_count > 0) {
+ /* We are using service discovery */
+ if (!is_filter_match(hdev, rssi, eir, eir_len, scan_rsp,
+ scan_rsp_len))
+ return;
+ }
+
+ /* Make sure that the buffer is big enough. The 5 extra bytes
+ * are for the potential CoD field.
+ */
+ if (sizeof(*ev) + eir_len + scan_rsp_len + 5 > sizeof(buf))
return;
+ memset(buf, 0, sizeof(buf));
+
+ /* In case of device discovery with BR/EDR devices (pre 1.2), the
+ * RSSI value was reported as 0 when not available. This behavior
+ * is kept when using device discovery. This is required for full
+ * backwards compatibility with the API.
+ *
+ * However when using service discovery, the value 127 will be
+ * returned when the RSSI is not available.
+ */
+ if (rssi == HCI_RSSI_INVALID && !hdev->discovery.report_invalid_rssi &&
+ link_type == ACL_LINK)
+ rssi = 0;
+
+ bacpy(&ev->addr.bdaddr, bdaddr);
+ ev->addr.type = link_to_bdaddr(link_type, addr_type);
+ ev->rssi = rssi;
+ ev->flags = cpu_to_le32(flags);
+
+ if (eir_len > 0)
+ /* Copy EIR or advertising data into event */
+ memcpy(ev->eir, eir, eir_len);
+
+ if (dev_class && !eir_has_data_type(ev->eir, eir_len, EIR_CLASS_OF_DEV))
+ eir_len = eir_append_data(ev->eir, eir_len, EIR_CLASS_OF_DEV,
+ dev_class, 3);
+
+ if (scan_rsp_len > 0)
+ /* Append scan response data to event */
+ memcpy(ev->eir + eir_len, scan_rsp, scan_rsp_len);
+
ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
--
2.2.0.rc0.207.ga3a616c
Hi Jakub,
On Wed, Mar 04, 2015, Jakub Pawlowski wrote:
> This patch moves whole packet filering logic of service discovery
> into new function is_filter_match. It's done because logic inside
> mgmt_device_found is very complicated and needs some
> simplification.
>
> Also having whole logic in one place will allow to simplify it in
> the future.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> net/bluetooth/mgmt.c | 141 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 79 insertions(+), 62 deletions(-)
All three patches in this set have now been applied. Thanks.
Johan
This patch fixes service discovery behaviour, when provided uuid filter
is empty and HCI_QUIRK_STRICT_DUPLICATE_FILTER is set. Before this
patch, empty uuid filter was unable to trigger scan restart, and that
caused inconsistent behaviour in applications.
Example: two DBus clients call BlueZ, one to find all devices with
service abcd, second to find all devices with rssi smaller than -90.
Sum of those filters, that is passed to mgmt_service_scan is empty
filter, with no rssi or uuids set.
That caused kernel not to restart scan when quirk was set.
That was inconsistent with what happen when there's only one of those
two filters set (scan is restarted and reports devices).
To fix that, new variable hdev->discovery.result_filtering was
introduced. It can indicate that filtered scan is running, no matter
what uuid or rssi filter is set.
Signed-off-by: Jakub Pawlowski <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/mgmt.c | 7 +++----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index acec914..15c761c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -76,6 +76,7 @@ struct discovery_state {
u8 last_adv_data[HCI_MAX_AD_LENGTH];
u8 last_adv_data_len;
bool report_invalid_rssi;
+ bool result_filtering;
s8 rssi;
u16 uuid_count;
u8 (*uuids)[16];
@@ -525,6 +526,7 @@ static inline void discovery_init(struct hci_dev *hdev)
static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
{
+ hdev->discovery.result_filtering = false;
hdev->discovery.report_invalid_rssi = true;
hdev->discovery.rssi = HCI_RSSI_INVALID;
hdev->discovery.uuid_count = 0;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bc09c5a..967f07f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3933,8 +3933,7 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status,
*/
if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER,
&hdev->quirks) &&
- (hdev->discovery.uuid_count > 0 ||
- hdev->discovery.rssi != HCI_RSSI_INVALID)) {
+ hdev->discovery.result_filtering) {
hdev->discovery.scan_start = jiffies;
hdev->discovery.scan_duration = timeout;
}
@@ -4087,6 +4086,7 @@ static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
*/
hci_discovery_filter_clear(hdev);
+ hdev->discovery.result_filtering = true;
hdev->discovery.type = cp->type;
hdev->discovery.rssi = cp->rssi;
hdev->discovery.uuid_count = uuid_count;
@@ -7344,8 +7344,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
return;
}
- if (hdev->discovery.rssi != HCI_RSSI_INVALID ||
- hdev->discovery.uuid_count > 0) {
+ if (hdev->discovery.result_filtering) {
/* We are using service discovery */
if (!is_filter_match(hdev, rssi, eir, eir_len, scan_rsp,
scan_rsp_len))
--
2.2.0.rc0.207.ga3a616c
This patch refactor code responsible for filtering when service
discovery method is used. Previously this code was mixed with
mgmt_device found logic. Now when it's in one place whole logic can
be greatly simplified. That includes removing no longer necessary
length field and merging checks for eir and scan_rsp.
Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/mgmt.c | 89 +++++++++++-----------------------------------------
1 file changed, 19 insertions(+), 70 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a41a5ef..bc09c5a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7283,8 +7283,6 @@ static void restart_le_scan(struct hci_dev *hdev)
static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
{
- bool match;
-
/* If a RSSI threshold has been specified, and
* HCI_QUIRK_STRICT_DUPLICATE_FILTER is not set, then all results with
* a RSSI smaller than the RSSI threshold will be dropped. If the quirk
@@ -7300,78 +7298,29 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
!test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks))))
return false;
-
- if (eir_len > 0) {
- /* When using service discovery and a list of UUID is
- * provided, results with no matching UUID should be
- * dropped. In case there is a match the result is
- * kept and checking possible scan response data
- * will be skipped.
- */
- if (hdev->discovery.uuid_count > 0) {
- match = eir_has_uuids(eir, eir_len,
- hdev->discovery.uuid_count,
- hdev->discovery.uuids);
- /* If duplicate filtering does not report RSSI changes,
- * then restart scanning to ensure updated result with
- * updated RSSI values.
- */
- if (match && test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER,
- &hdev->quirks))
- restart_le_scan(hdev);
- } else {
- match = true;
- }
-
- if (!match && !scan_rsp_len)
- return false;
- } else {
- /* When using service discovery and a list of UUID is
- * provided, results with empty EIR or advertising data
- * should be dropped since they do not match any UUID.
- */
- if (hdev->discovery.uuid_count > 0 && !scan_rsp_len)
- return false;
-
- match = false;
- }
-
- if (scan_rsp_len > 0) {
- /* When using service discovery and a list of UUID is
- * provided, results with no matching UUID should be
- * dropped if there is no previous match from the
- * advertising data.
- */
- if (hdev->discovery.uuid_count > 0) {
- if (!match && !eir_has_uuids(scan_rsp, scan_rsp_len,
- hdev->discovery.uuid_count,
- hdev->discovery.uuids))
- return false;
-
- /* If duplicate filtering does not report RSSI changes,
- * then restart scanning to ensure updated result with
- * updated RSSI values.
- */
- if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER,
- &hdev->quirks))
- restart_le_scan(hdev);
- }
- } else {
- /* When using service discovery and a list of UUID is
- * provided, results with empty scan response and no
- * previous matched advertising data should be dropped.
+ if (hdev->discovery.uuid_count != 0) {
+ /* If a list of UUIDs is provided in filter, results with no
+ * matching UUID should be dropped.
*/
- if (hdev->discovery.uuid_count > 0 && !match)
- return false;
+ if (!eir_has_uuids(eir, eir_len, hdev->discovery.uuid_count,
+ hdev->discovery.uuids) &&
+ !eir_has_uuids(scan_rsp, scan_rsp_len,
+ hdev->discovery.uuid_count,
+ hdev->discovery.uuids))
+ return false;
}
- /* Validate the reported RSSI value against the RSSI threshold once more
- * incase HCI_QUIRK_STRICT_DUPLICATE_FILTER forced a restart of LE
- * scanning.
+ /* If duplicate filtering does not report RSSI changes, then restart
+ * scanning to ensure updated result with updated RSSI values.
*/
- if (hdev->discovery.rssi != HCI_RSSI_INVALID &&
- rssi < hdev->discovery.rssi)
- return false;
+ if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks)) {
+ restart_le_scan(hdev);
+
+ /* Validate RSSI value against the RSSI threshold once more. */
+ if (hdev->discovery.rssi != HCI_RSSI_INVALID &&
+ rssi < hdev->discovery.rssi)
+ return false;
+ }
return true;
}
--
2.2.0.rc0.207.ga3a616c