[PATCH v2 1/6] monitor: Add AD/EIR types for mesh

[PATCH v2 1/6] monitor: Add AD/EIR types for mesh

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Print out mesh type and payload:

> HCI Event: LE Meta Event (0x3e) plen 33
      LE Advertising Report (0x02)
        Num reports: 1
        Event type: Non connectable undirected - ADV_NONCONN_IND (0x03)
        Address type: Random (0x01)
        Address: 29:D0:F2:61:17:B0 (Non-Resolvable)
        Data length: 21
        Mesh Data: 59a7ddee2e1f580463521b98c40c2a3b5086c6
        RSSI: -72 dBm (0xb8)
---
 monitor/packet.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index a3d188a..cc24165 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -3259,6 +3259,9 @@ static void print_fec(uint8_t fec)
 #define BT_EIR_TRANSPORT_DISCOVERY	0x26
 #define BT_EIR_LE_SUPPORTED_FEATURES	0x27
 #define BT_EIR_CHANNEL_MAP_UPDATE_IND	0x28
+#define BT_EIR_MESH_DATA		0x29
+#define BT_EIR_MESH_PROV		0x2a
+#define BT_EIR_MESH_BEACON		0x2b
 #define BT_EIR_3D_INFO_DATA		0x3d
 #define BT_EIR_MANUFACTURER_DATA	0xff
 
@@ -3720,6 +3723,18 @@ static void print_eir(const uint8_t *eir, uint8_t eir_len, bool le)
 			print_field("  Path Loss Threshold: %d", data[1]);
 			break;
 
+		case BT_EIR_MESH_DATA:
+			print_hex_field("Mesh Data", data, data_len);
+			break;
+
+		case BT_EIR_MESH_PROV:
+			print_hex_field("Mesh Provisioning", data, data_len);
+			break;
+
+		case BT_EIR_MESH_BEACON:
+			print_hex_field("Mesh Beacon", data, data_len);
+			break;
+
 		case BT_EIR_MANUFACTURER_DATA:
 			if (data_len < 2)
 				break;
-- 
2.9.4

[PATCH v2 2/6] monitor: Add decoding of GATT Mesh Services

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds UUIDs of GATT Mesh Provisioning Service and Mesh Proxy
Service.
---
 monitor/uuid.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/monitor/uuid.c b/monitor/uuid.c
index fb0b210..9839d03 100644
--- a/monitor/uuid.c
+++ b/monitor/uuid.c
@@ -170,7 +170,9 @@ static const struct {
 	{ 0x1824, "Transport Discovery"				},
 	{ 0x1825, "Object Transfer"				},
 	{ 0x1826, "Fitness Machine"				},
-	/* 0x1827 to 0x27ff undefined */
+	{ 0x1827, "Mesh Provisioning"				},
+	{ 0x1828, "Mesh Proxy"					},
+	/* 0x1829 to 0x27ff undefined */
 	{ 0x2800, "Primary Service"				},
 	{ 0x2801, "Secondary Service"				},
 	{ 0x2802, "Include"					},
@@ -397,6 +399,10 @@ static const struct {
 	{ 0x2ad8, "Supported Power Range"			},
 	{ 0x2ad9, "Fitness Machine Control Point"		},
 	{ 0x2ada, "Fitness Machine Status"			},
+	{ 0x2adb, "Mesh Provisioning Data In"			},
+	{ 0x2adc, "Mesh Provisioning Data Out"			},
+	{ 0x2add, "Mesh Proxy Data In"				},
+	{ 0x2ade, "Mesh Proxy Data Out"				},
 	/* vendor defined */
 	{ 0xfeff, "GN Netcom"					},
 	{ 0xfefe, "GN ReSound A/S"				},
-- 
2.9.4

[PATCH v2 3/6] monitor: Filter duplicated advertisements

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This reduces the verbosity and give a precise picture of how unique
each report is.
---
 monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index cc24165..4ed0faa 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -42,7 +42,9 @@
 #include "lib/hci.h"
 #include "lib/hci_lib.h"
 
+#include "src/shared/mainloop.h"
 #include "src/shared/util.h"
+#include "src/shared/queue.h"
 #include "src/shared/btsnoop.h"
 #include "display.h"
 #include "bt.h"
@@ -100,6 +102,9 @@
 
 #define COLOR_PHY_PACKET		COLOR_BLUE
 
+#define ADV_TIMEOUT			3000
+#define ADV_MAX_CACHE			100
+
 static time_t time_offset = ((time_t) -1);
 static int priority_level = BTSNOOP_PRIORITY_INFO;
 static unsigned long filter_mask = 0;
@@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
 
 #define MAX_INDEX 16
 
+struct adv_data {
+	struct bt_hci_evt_le_adv_report evt;
+	void *data;
+	size_t frame;
+	int timeout;
+	struct queue *cache;
+};
+
 struct index_data {
 	uint8_t  type;
 	uint8_t  bdaddr[6];
 	uint16_t manufacturer;
 	size_t	frame;
+	struct queue *adv_cache;
 };
 
 static struct index_data index_list[MAX_INDEX];
@@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
 		assign_handle(le16_to_cpu(evt->handle), 0x01);
 }
 
+static void adv_free(void *data)
+{
+	struct adv_data *adv = data;
+
+	free(adv->data);
+	free(adv);
+}
+
+static bool match_adv(const void *data, const void *user_data)
+{
+	const struct adv_data *adv = data;
+	const struct bt_hci_evt_le_adv_report *evt = user_data;
+
+	if (memcmp(&adv->evt, evt, sizeof(*evt)))
+		return false;
+
+	return memcmp(adv->data, evt->data, evt->data_len) == 0;
+}
+
+static void adv_timeout(int id, void *data)
+{
+	struct adv_data *adv = data;
+
+	/* Remove from check (adv is freed after returning) */
+	queue_remove(adv->cache, adv);
+
+	mainloop_remove_timeout(adv->timeout);
+}
+
 static void le_adv_report_evt(const void *data, uint8_t size)
 {
 	const struct bt_hci_evt_le_adv_report *evt = data;
+	struct index_data *idata = &index_list[index_current];
+	struct adv_data *match;
+	struct adv_data *adv;
 	uint8_t evt_len;
 	int8_t *rssi;
 
+	if (!idata->adv_cache)
+		idata->adv_cache = queue_new();
+
+	adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
+	if (adv) {
+		/* Update cache and modify expire timeout */
+		if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
+			mainloop_remove_timeout(adv->timeout);
+			adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
+							adv_timeout, adv,
+							adv_free);
+		}
+		queue_push_head(idata->adv_cache, adv);
+		print_field("Duplicate of #%zu", adv->frame);
+		size = 0;
+		goto rssi;
+	}
+
+	match = new0(struct adv_data, 1);
+	match->evt = *evt;
+
+	match->data = malloc(evt->data_len);
+	memcpy(match->data, evt->data, evt->data_len);
+
+	match->cache = idata->adv_cache;
+	match->frame = idata->frame;
+	match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
+						adv_free);
+	if (match->timeout <= 0) {
+		print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
+							match->timeout);
+		adv_free(match);
+		goto print;
+	}
+
+	queue_push_head(idata->adv_cache, match);
+
+	if (queue_length(idata->adv_cache) > ADV_MAX_CACHE)
+		queue_remove(idata->adv_cache,
+					queue_peek_tail(idata->adv_cache));
+
+print:
 	print_num_reports(evt->num_reports);
 
 report:
@@ -8602,6 +8690,7 @@ report:
 	print_field("Data length: %d", evt->data_len);
 	print_eir(evt->data, evt->data_len, true);
 
+rssi:
 	rssi = (int8_t *) (evt->data + evt->data_len);
 	print_rssi(*rssi);
 
-- 
2.9.4

Re: [PATCH v2 3/6] monitor: Filter duplicated advertisements

[Marcel Holtmann]

Hi Luiz,

> This reduces the verbosity and give a precise picture of how unique
> each report is.
> ---
> monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 89 insertions(+)
> 
> diff --git a/monitor/packet.c b/monitor/packet.c
> index cc24165..4ed0faa 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -42,7 +42,9 @@
> #include "lib/hci.h"
> #include "lib/hci_lib.h"
> 
> +#include "src/shared/mainloop.h"
> #include "src/shared/util.h"
> +#include "src/shared/queue.h"
> #include "src/shared/btsnoop.h"
> #include "display.h"
> #include "bt.h"
> @@ -100,6 +102,9 @@
> 
> #define COLOR_PHY_PACKET		COLOR_BLUE
> 
> +#define ADV_TIMEOUT			3000
> +#define ADV_MAX_CACHE			100
> +
> static time_t time_offset = ((time_t) -1);
> static int priority_level = BTSNOOP_PRIORITY_INFO;
> static unsigned long filter_mask = 0;
> @@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
> 
> #define MAX_INDEX 16
> 
> +struct adv_data {
> +	struct bt_hci_evt_le_adv_report evt;
> +	void *data;
> +	size_t frame;
> +	int timeout;
> +	struct queue *cache;
> +};
> +
> struct index_data {
> 	uint8_t  type;
> 	uint8_t  bdaddr[6];
> 	uint16_t manufacturer;
> 	size_t	frame;
> +	struct queue *adv_cache;
> };
> 
> static struct index_data index_list[MAX_INDEX];
> @@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
> 		assign_handle(le16_to_cpu(evt->handle), 0x01);
> }
> 
> +static void adv_free(void *data)
> +{
> +	struct adv_data *adv = data;
> +
> +	free(adv->data);
> +	free(adv);
> +}
> +
> +static bool match_adv(const void *data, const void *user_data)
> +{
> +	const struct adv_data *adv = data;
> +	const struct bt_hci_evt_le_adv_report *evt = user_data;
> +
> +	if (memcmp(&adv->evt, evt, sizeof(*evt)))
> +		return false;
> +
> +	return memcmp(adv->data, evt->data, evt->data_len) == 0;
> +}
> +
> +static void adv_timeout(int id, void *data)
> +{
> +	struct adv_data *adv = data;
> +
> +	/* Remove from check (adv is freed after returning) */
> +	queue_remove(adv->cache, adv);
> +
> +	mainloop_remove_timeout(adv->timeout);
> +}
> +
> static void le_adv_report_evt(const void *data, uint8_t size)
> {
> 	const struct bt_hci_evt_le_adv_report *evt = data;
> +	struct index_data *idata = &index_list[index_current];
> +	struct adv_data *match;
> +	struct adv_data *adv;
> 	uint8_t evt_len;
> 	int8_t *rssi;
> 
> +	if (!idata->adv_cache)
> +		idata->adv_cache = queue_new();
> +
> +	adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
> +	if (adv) {
> +		/* Update cache and modify expire timeout */
> +		if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
> +			mainloop_remove_timeout(adv->timeout);
> +			adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
> +							adv_timeout, adv,
> +							adv_free);
> +		}
> +		queue_push_head(idata->adv_cache, adv);
> +		print_field("Duplicate of #%zu", adv->frame);
> +		size = 0;
> +		goto rssi;
> +	}
> +
> +	match = new0(struct adv_data, 1);
> +	match->evt = *evt;
> +
> +	match->data = malloc(evt->data_len);
> +	memcpy(match->data, evt->data, evt->data_len);
> +
> +	match->cache = idata->adv_cache;
> +	match->frame = idata->frame;
> +	match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
> +						adv_free);
> +	if (match->timeout <= 0) {
> +		print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
> +							match->timeout);
> +		adv_free(match);
> +		goto print;
> +	}

for a tool like btmon since is clearly the wrong approach. If the wire is verbose, then so be it. If someone wants to filter via BPF, that is fine, but this magic is not scalable.

Regards

Marcel

Re: [PATCH v2 3/6] monitor: Filter duplicated advertisements

[Luiz Augusto von Dentz]

Hi Marcel,

On Wed, Jul 19, 2017 at 8:43 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This reduces the verbosity and give a precise picture of how unique
>> each report is.
>> ---
>> monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>>
>> diff --git a/monitor/packet.c b/monitor/packet.c
>> index cc24165..4ed0faa 100644
>> --- a/monitor/packet.c
>> +++ b/monitor/packet.c
>> @@ -42,7 +42,9 @@
>> #include "lib/hci.h"
>> #include "lib/hci_lib.h"
>>
>> +#include "src/shared/mainloop.h"
>> #include "src/shared/util.h"
>> +#include "src/shared/queue.h"
>> #include "src/shared/btsnoop.h"
>> #include "display.h"
>> #include "bt.h"
>> @@ -100,6 +102,9 @@
>>
>> #define COLOR_PHY_PACKET              COLOR_BLUE
>>
>> +#define ADV_TIMEOUT                  3000
>> +#define ADV_MAX_CACHE                        100
>> +
>> static time_t time_offset = ((time_t) -1);
>> static int priority_level = BTSNOOP_PRIORITY_INFO;
>> static unsigned long filter_mask = 0;
>> @@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
>>
>> #define MAX_INDEX 16
>>
>> +struct adv_data {
>> +     struct bt_hci_evt_le_adv_report evt;
>> +     void *data;
>> +     size_t frame;
>> +     int timeout;
>> +     struct queue *cache;
>> +};
>> +
>> struct index_data {
>>       uint8_t  type;
>>       uint8_t  bdaddr[6];
>>       uint16_t manufacturer;
>>       size_t  frame;
>> +     struct queue *adv_cache;
>> };
>>
>> static struct index_data index_list[MAX_INDEX];
>> @@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
>>               assign_handle(le16_to_cpu(evt->handle), 0x01);
>> }
>>
>> +static void adv_free(void *data)
>> +{
>> +     struct adv_data *adv = data;
>> +
>> +     free(adv->data);
>> +     free(adv);
>> +}
>> +
>> +static bool match_adv(const void *data, const void *user_data)
>> +{
>> +     const struct adv_data *adv = data;
>> +     const struct bt_hci_evt_le_adv_report *evt = user_data;
>> +
>> +     if (memcmp(&adv->evt, evt, sizeof(*evt)))
>> +             return false;
>> +
>> +     return memcmp(adv->data, evt->data, evt->data_len) == 0;
>> +}
>> +
>> +static void adv_timeout(int id, void *data)
>> +{
>> +     struct adv_data *adv = data;
>> +
>> +     /* Remove from check (adv is freed after returning) */
>> +     queue_remove(adv->cache, adv);
>> +
>> +     mainloop_remove_timeout(adv->timeout);
>> +}
>> +
>> static void le_adv_report_evt(const void *data, uint8_t size)
>> {
>>       const struct bt_hci_evt_le_adv_report *evt = data;
>> +     struct index_data *idata = &index_list[index_current];
>> +     struct adv_data *match;
>> +     struct adv_data *adv;
>>       uint8_t evt_len;
>>       int8_t *rssi;
>>
>> +     if (!idata->adv_cache)
>> +             idata->adv_cache = queue_new();
>> +
>> +     adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
>> +     if (adv) {
>> +             /* Update cache and modify expire timeout */
>> +             if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
>> +                     mainloop_remove_timeout(adv->timeout);
>> +                     adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
>> +                                                     adv_timeout, adv,
>> +                                                     adv_free);
>> +             }
>> +             queue_push_head(idata->adv_cache, adv);
>> +             print_field("Duplicate of #%zu", adv->frame);
>> +             size = 0;
>> +             goto rssi;
>> +     }
>> +
>> +     match = new0(struct adv_data, 1);
>> +     match->evt = *evt;
>> +
>> +     match->data = malloc(evt->data_len);
>> +     memcpy(match->data, evt->data, evt->data_len);
>> +
>> +     match->cache = idata->adv_cache;
>> +     match->frame = idata->frame;
>> +     match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
>> +                                             adv_free);
>> +     if (match->timeout <= 0) {
>> +             print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
>> +                                                     match->timeout);
>> +             adv_free(match);
>> +             goto print;
>> +     }
>
> for a tool like btmon since is clearly the wrong approach. If the wire is verbose, then so be it. If someone wants to filter via BPF, that is fine, but this magic is not scalable.

Wireshark and the like seems to be able to detect duplicated
referencing the frame it has occurred which is what Im attempting to
do here.

There doesn't seems to be a way to save context in BPF, the filter
runs on each packet, and even if we could build things like a packet
cache in BPF asm might not be the most trivial thing to maintain.

The scalable part I don't really get it, there are limits for both
number of entries and time they stay in cache so the lookups are quite
cheap actually and we can actually reduce the number even further if
we just maintain the last adv per address.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/6] monitor: Filter duplicated advertisements

[Marcel Holtmann]

Hi Luiz,

>>> This reduces the verbosity and give a precise picture of how unique
>>> each report is.
>>> ---
>>> monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 89 insertions(+)
>>> 
>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>> index cc24165..4ed0faa 100644
>>> --- a/monitor/packet.c
>>> +++ b/monitor/packet.c
>>> @@ -42,7 +42,9 @@
>>> #include "lib/hci.h"
>>> #include "lib/hci_lib.h"
>>> 
>>> +#include "src/shared/mainloop.h"
>>> #include "src/shared/util.h"
>>> +#include "src/shared/queue.h"
>>> #include "src/shared/btsnoop.h"
>>> #include "display.h"
>>> #include "bt.h"
>>> @@ -100,6 +102,9 @@
>>> 
>>> #define COLOR_PHY_PACKET              COLOR_BLUE
>>> 
>>> +#define ADV_TIMEOUT                  3000
>>> +#define ADV_MAX_CACHE                        100
>>> +
>>> static time_t time_offset = ((time_t) -1);
>>> static int priority_level = BTSNOOP_PRIORITY_INFO;
>>> static unsigned long filter_mask = 0;
>>> @@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
>>> 
>>> #define MAX_INDEX 16
>>> 
>>> +struct adv_data {
>>> +     struct bt_hci_evt_le_adv_report evt;
>>> +     void *data;
>>> +     size_t frame;
>>> +     int timeout;
>>> +     struct queue *cache;
>>> +};
>>> +
>>> struct index_data {
>>>      uint8_t  type;
>>>      uint8_t  bdaddr[6];
>>>      uint16_t manufacturer;
>>>      size_t  frame;
>>> +     struct queue *adv_cache;
>>> };
>>> 
>>> static struct index_data index_list[MAX_INDEX];
>>> @@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
>>>              assign_handle(le16_to_cpu(evt->handle), 0x01);
>>> }
>>> 
>>> +static void adv_free(void *data)
>>> +{
>>> +     struct adv_data *adv = data;
>>> +
>>> +     free(adv->data);
>>> +     free(adv);
>>> +}
>>> +
>>> +static bool match_adv(const void *data, const void *user_data)
>>> +{
>>> +     const struct adv_data *adv = data;
>>> +     const struct bt_hci_evt_le_adv_report *evt = user_data;
>>> +
>>> +     if (memcmp(&adv->evt, evt, sizeof(*evt)))
>>> +             return false;
>>> +
>>> +     return memcmp(adv->data, evt->data, evt->data_len) == 0;
>>> +}
>>> +
>>> +static void adv_timeout(int id, void *data)
>>> +{
>>> +     struct adv_data *adv = data;
>>> +
>>> +     /* Remove from check (adv is freed after returning) */
>>> +     queue_remove(adv->cache, adv);
>>> +
>>> +     mainloop_remove_timeout(adv->timeout);
>>> +}
>>> +
>>> static void le_adv_report_evt(const void *data, uint8_t size)
>>> {
>>>      const struct bt_hci_evt_le_adv_report *evt = data;
>>> +     struct index_data *idata = &index_list[index_current];
>>> +     struct adv_data *match;
>>> +     struct adv_data *adv;
>>>      uint8_t evt_len;
>>>      int8_t *rssi;
>>> 
>>> +     if (!idata->adv_cache)
>>> +             idata->adv_cache = queue_new();
>>> +
>>> +     adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
>>> +     if (adv) {
>>> +             /* Update cache and modify expire timeout */
>>> +             if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
>>> +                     mainloop_remove_timeout(adv->timeout);
>>> +                     adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
>>> +                                                     adv_timeout, adv,
>>> +                                                     adv_free);
>>> +             }
>>> +             queue_push_head(idata->adv_cache, adv);
>>> +             print_field("Duplicate of #%zu", adv->frame);
>>> +             size = 0;
>>> +             goto rssi;
>>> +     }
>>> +
>>> +     match = new0(struct adv_data, 1);
>>> +     match->evt = *evt;
>>> +
>>> +     match->data = malloc(evt->data_len);
>>> +     memcpy(match->data, evt->data, evt->data_len);
>>> +
>>> +     match->cache = idata->adv_cache;
>>> +     match->frame = idata->frame;
>>> +     match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
>>> +                                             adv_free);
>>> +     if (match->timeout <= 0) {
>>> +             print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
>>> +                                                     match->timeout);
>>> +             adv_free(match);
>>> +             goto print;
>>> +     }
>> 
>> for a tool like btmon since is clearly the wrong approach. If the wire is verbose, then so be it. If someone wants to filter via BPF, that is fine, but this magic is not scalable.
> 
> Wireshark and the like seems to be able to detect duplicated
> referencing the frame it has occurred which is what Im attempting to
> do here.

but these are not actually duplicates. These are frames from the controller that it received, because you told it to. I am really disliking this approach a lot in btmon. If you want to build a dedicated meshmon tool that enables full duty cycle passive scanning and detects duplicates, then fine, but not in btmon.

> There doesn't seems to be a way to save context in BPF, the filter
> runs on each packet, and even if we could build things like a packet
> cache in BPF asm might not be the most trivial thing to maintain.
> 
> The scalable part I don't really get it, there are limits for both
> number of entries and time they stay in cache so the lookups are quite
> cheap actually and we can actually reduce the number even further if
> we just maintain the last adv per address.

Today it is this filter, in a month it is another and by the end of the year we have 50 custom filter hacks included. I really do not want that. If BPF is limited right now, then lets try to extend it.

Regards

Marcel

Re: [PATCH v2 3/6] monitor: Filter duplicated advertisements

[Luiz Augusto von Dentz]

Hi Marcel,

On Thu, Jul 20, 2017 at 9:17 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>>>> This reduces the verbosity and give a precise picture of how unique
>>>> each report is.
>>>> ---
>>>> monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++=
++++++++
>>>> 1 file changed, 89 insertions(+)
>>>>
>>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>>> index cc24165..4ed0faa 100644
>>>> --- a/monitor/packet.c
>>>> +++ b/monitor/packet.c
>>>> @@ -42,7 +42,9 @@
>>>> #include "lib/hci.h"
>>>> #include "lib/hci_lib.h"
>>>>
>>>> +#include "src/shared/mainloop.h"
>>>> #include "src/shared/util.h"
>>>> +#include "src/shared/queue.h"
>>>> #include "src/shared/btsnoop.h"
>>>> #include "display.h"
>>>> #include "bt.h"
>>>> @@ -100,6 +102,9 @@
>>>>
>>>> #define COLOR_PHY_PACKET              COLOR_BLUE
>>>>
>>>> +#define ADV_TIMEOUT                  3000
>>>> +#define ADV_MAX_CACHE                        100
>>>> +
>>>> static time_t time_offset =3D ((time_t) -1);
>>>> static int priority_level =3D BTSNOOP_PRIORITY_INFO;
>>>> static unsigned long filter_mask =3D 0;
>>>> @@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
>>>>
>>>> #define MAX_INDEX 16
>>>>
>>>> +struct adv_data {
>>>> +     struct bt_hci_evt_le_adv_report evt;
>>>> +     void *data;
>>>> +     size_t frame;
>>>> +     int timeout;
>>>> +     struct queue *cache;
>>>> +};
>>>> +
>>>> struct index_data {
>>>>      uint8_t  type;
>>>>      uint8_t  bdaddr[6];
>>>>      uint16_t manufacturer;
>>>>      size_t  frame;
>>>> +     struct queue *adv_cache;
>>>> };
>>>>
>>>> static struct index_data index_list[MAX_INDEX];
>>>> @@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *d=
ata, uint8_t size)
>>>>              assign_handle(le16_to_cpu(evt->handle), 0x01);
>>>> }
>>>>
>>>> +static void adv_free(void *data)
>>>> +{
>>>> +     struct adv_data *adv =3D data;
>>>> +
>>>> +     free(adv->data);
>>>> +     free(adv);
>>>> +}
>>>> +
>>>> +static bool match_adv(const void *data, const void *user_data)
>>>> +{
>>>> +     const struct adv_data *adv =3D data;
>>>> +     const struct bt_hci_evt_le_adv_report *evt =3D user_data;
>>>> +
>>>> +     if (memcmp(&adv->evt, evt, sizeof(*evt)))
>>>> +             return false;
>>>> +
>>>> +     return memcmp(adv->data, evt->data, evt->data_len) =3D=3D 0;
>>>> +}
>>>> +
>>>> +static void adv_timeout(int id, void *data)
>>>> +{
>>>> +     struct adv_data *adv =3D data;
>>>> +
>>>> +     /* Remove from check (adv is freed after returning) */
>>>> +     queue_remove(adv->cache, adv);
>>>> +
>>>> +     mainloop_remove_timeout(adv->timeout);
>>>> +}
>>>> +
>>>> static void le_adv_report_evt(const void *data, uint8_t size)
>>>> {
>>>>      const struct bt_hci_evt_le_adv_report *evt =3D data;
>>>> +     struct index_data *idata =3D &index_list[index_current];
>>>> +     struct adv_data *match;
>>>> +     struct adv_data *adv;
>>>>      uint8_t evt_len;
>>>>      int8_t *rssi;
>>>>
>>>> +     if (!idata->adv_cache)
>>>> +             idata->adv_cache =3D queue_new();
>>>> +
>>>> +     adv =3D queue_remove_if(idata->adv_cache, match_adv, (void *) ev=
t);
>>>> +     if (adv) {
>>>> +             /* Update cache and modify expire timeout */
>>>> +             if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) <=
 0) {
>>>> +                     mainloop_remove_timeout(adv->timeout);
>>>> +                     adv->timeout =3D mainloop_add_timeout(ADV_TIMEOU=
T,
>>>> +                                                     adv_timeout, adv=
,
>>>> +                                                     adv_free);
>>>> +             }
>>>> +             queue_push_head(idata->adv_cache, adv);
>>>> +             print_field("Duplicate of #%zu", adv->frame);
>>>> +             size =3D 0;
>>>> +             goto rssi;
>>>> +     }
>>>> +
>>>> +     match =3D new0(struct adv_data, 1);
>>>> +     match->evt =3D *evt;
>>>> +
>>>> +     match->data =3D malloc(evt->data_len);
>>>> +     memcpy(match->data, evt->data, evt->data_len);
>>>> +
>>>> +     match->cache =3D idata->adv_cache;
>>>> +     match->frame =3D idata->frame;
>>>> +     match->timeout =3D mainloop_add_timeout(ADV_TIMEOUT, adv_timeout=
, match,
>>>> +                                             adv_free);
>>>> +     if (match->timeout <=3D 0) {
>>>> +             print_field("Cannot cache: %s (%d)", strerror(-match->ti=
meout),
>>>> +                                                     match->timeout);
>>>> +             adv_free(match);
>>>> +             goto print;
>>>> +     }
>>>
>>> for a tool like btmon since is clearly the wrong approach. If the wire =
is verbose, then so be it. If someone wants to filter via BPF, that is fine=
, but this magic is not scalable.
>>
>> Wireshark and the like seems to be able to detect duplicated
>> referencing the frame it has occurred which is what Im attempting to
>> do here.
>
> but these are not actually duplicates. These are frames from the controll=
er that it received, because you told it to. I am really disliking this app=
roach a lot in btmon. If you want to build a dedicated meshmon tool that en=
ables full duty cycle passive scanning and detects duplicates, then fine, b=
ut not in btmon.

Im not so fond of forking tools like this, most of the MGMT commands
and HCI traffic is pretty low profile including outgoing adv traffic
etc, but perhaps you want to combine outputs of meshmon with btmon.

>> There doesn't seems to be a way to save context in BPF, the filter
>> runs on each packet, and even if we could build things like a packet
>> cache in BPF asm might not be the most trivial thing to maintain.
>>
>> The scalable part I don't really get it, there are limits for both
>> number of entries and time they stay in cache so the lookups are quite
>> cheap actually and we can actually reduce the number even further if
>> we just maintain the last adv per address.
>
> Today it is this filter, in a month it is another and by the end of the y=
ear we have 50 custom filter hacks included. I really do not want that. If =
BPF is limited right now, then lets try to extend it.

I don't think BPF can do duplicate filtering or rate limit alone, it
is stateless, but anyway if you want to sit and hope someone
transforms BPF into stateful programs rather than simple filters I
guess we will be waiting for a long while.

> Regards
>
> Marcel
>



--=20
Luiz Augusto von Dentz

Re: [PATCH v2 3/6] monitor: Filter duplicated advertisements

[Marcel Holtmann]

Hi Luiz,

>>>>> This reduces the verbosity and give a precise picture of how unique
>>>>> each report is.
>>>>> ---
>>>>> monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 89 insertions(+)
>>>>> 
>>>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>>>> index cc24165..4ed0faa 100644
>>>>> --- a/monitor/packet.c
>>>>> +++ b/monitor/packet.c
>>>>> @@ -42,7 +42,9 @@
>>>>> #include "lib/hci.h"
>>>>> #include "lib/hci_lib.h"
>>>>> 
>>>>> +#include "src/shared/mainloop.h"
>>>>> #include "src/shared/util.h"
>>>>> +#include "src/shared/queue.h"
>>>>> #include "src/shared/btsnoop.h"
>>>>> #include "display.h"
>>>>> #include "bt.h"
>>>>> @@ -100,6 +102,9 @@
>>>>> 
>>>>> #define COLOR_PHY_PACKET              COLOR_BLUE
>>>>> 
>>>>> +#define ADV_TIMEOUT                  3000
>>>>> +#define ADV_MAX_CACHE                        100
>>>>> +
>>>>> static time_t time_offset = ((time_t) -1);
>>>>> static int priority_level = BTSNOOP_PRIORITY_INFO;
>>>>> static unsigned long filter_mask = 0;
>>>>> @@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
>>>>> 
>>>>> #define MAX_INDEX 16
>>>>> 
>>>>> +struct adv_data {
>>>>> +     struct bt_hci_evt_le_adv_report evt;
>>>>> +     void *data;
>>>>> +     size_t frame;
>>>>> +     int timeout;
>>>>> +     struct queue *cache;
>>>>> +};
>>>>> +
>>>>> struct index_data {
>>>>>     uint8_t  type;
>>>>>     uint8_t  bdaddr[6];
>>>>>     uint16_t manufacturer;
>>>>>     size_t  frame;
>>>>> +     struct queue *adv_cache;
>>>>> };
>>>>> 
>>>>> static struct index_data index_list[MAX_INDEX];
>>>>> @@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
>>>>>             assign_handle(le16_to_cpu(evt->handle), 0x01);
>>>>> }
>>>>> 
>>>>> +static void adv_free(void *data)
>>>>> +{
>>>>> +     struct adv_data *adv = data;
>>>>> +
>>>>> +     free(adv->data);
>>>>> +     free(adv);
>>>>> +}
>>>>> +
>>>>> +static bool match_adv(const void *data, const void *user_data)
>>>>> +{
>>>>> +     const struct adv_data *adv = data;
>>>>> +     const struct bt_hci_evt_le_adv_report *evt = user_data;
>>>>> +
>>>>> +     if (memcmp(&adv->evt, evt, sizeof(*evt)))
>>>>> +             return false;
>>>>> +
>>>>> +     return memcmp(adv->data, evt->data, evt->data_len) == 0;
>>>>> +}
>>>>> +
>>>>> +static void adv_timeout(int id, void *data)
>>>>> +{
>>>>> +     struct adv_data *adv = data;
>>>>> +
>>>>> +     /* Remove from check (adv is freed after returning) */
>>>>> +     queue_remove(adv->cache, adv);
>>>>> +
>>>>> +     mainloop_remove_timeout(adv->timeout);
>>>>> +}
>>>>> +
>>>>> static void le_adv_report_evt(const void *data, uint8_t size)
>>>>> {
>>>>>     const struct bt_hci_evt_le_adv_report *evt = data;
>>>>> +     struct index_data *idata = &index_list[index_current];
>>>>> +     struct adv_data *match;
>>>>> +     struct adv_data *adv;
>>>>>     uint8_t evt_len;
>>>>>     int8_t *rssi;
>>>>> 
>>>>> +     if (!idata->adv_cache)
>>>>> +             idata->adv_cache = queue_new();
>>>>> +
>>>>> +     adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
>>>>> +     if (adv) {
>>>>> +             /* Update cache and modify expire timeout */
>>>>> +             if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
>>>>> +                     mainloop_remove_timeout(adv->timeout);
>>>>> +                     adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
>>>>> +                                                     adv_timeout, adv,
>>>>> +                                                     adv_free);
>>>>> +             }
>>>>> +             queue_push_head(idata->adv_cache, adv);
>>>>> +             print_field("Duplicate of #%zu", adv->frame);
>>>>> +             size = 0;
>>>>> +             goto rssi;
>>>>> +     }
>>>>> +
>>>>> +     match = new0(struct adv_data, 1);
>>>>> +     match->evt = *evt;
>>>>> +
>>>>> +     match->data = malloc(evt->data_len);
>>>>> +     memcpy(match->data, evt->data, evt->data_len);
>>>>> +
>>>>> +     match->cache = idata->adv_cache;
>>>>> +     match->frame = idata->frame;
>>>>> +     match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
>>>>> +                                             adv_free);
>>>>> +     if (match->timeout <= 0) {
>>>>> +             print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
>>>>> +                                                     match->timeout);
>>>>> +             adv_free(match);
>>>>> +             goto print;
>>>>> +     }
>>>> 
>>>> for a tool like btmon since is clearly the wrong approach. If the wire is verbose, then so be it. If someone wants to filter via BPF, that is fine, but this magic is not scalable.
>>> 
>>> Wireshark and the like seems to be able to detect duplicated
>>> referencing the frame it has occurred which is what Im attempting to
>>> do here.
>> 
>> but these are not actually duplicates. These are frames from the controller that it received, because you told it to. I am really disliking this approach a lot in btmon. If you want to build a dedicated meshmon tool that enables full duty cycle passive scanning and detects duplicates, then fine, but not in btmon.
> 
> Im not so fond of forking tools like this, most of the MGMT commands
> and HCI traffic is pretty low profile including outgoing adv traffic
> etc, but perhaps you want to combine outputs of meshmon with btmon.

I think that a meshmon is needed. It also needs to be able to drive special hardware to switch into full duty cycle scanning on all 3 channels. So fundamentally 3 Zephyr controller with our mesh specific extensions is what you want meshmon to drive. Since that is the only way to actually do a proper analysis of the full network. So whatever we hack into btmon right now is just a bandaid.

And I am still holding my position, if you do full duty cycle scanning without duplicate filtering, then that is what you get. Tons of advertising reports from all around you. Frankly it would make more sense to have some filter options on ADV_NONCONN_IND and AD type than trying to come with a magic hammer of reducing all duplicates. Reality is that mesh packets will only repeated for really short period of time.

>>> There doesn't seems to be a way to save context in BPF, the filter
>>> runs on each packet, and even if we could build things like a packet
>>> cache in BPF asm might not be the most trivial thing to maintain.
>>> 
>>> The scalable part I don't really get it, there are limits for both
>>> number of entries and time they stay in cache so the lookups are quite
>>> cheap actually and we can actually reduce the number even further if
>>> we just maintain the last adv per address.
>> 
>> Today it is this filter, in a month it is another and by the end of the year we have 50 custom filter hacks included. I really do not want that. If BPF is limited right now, then lets try to extend it.
> 
> I don't think BPF can do duplicate filtering or rate limit alone, it
> is stateless, but anyway if you want to sit and hope someone
> transforms BPF into stateful programs rather than simple filters I
> guess we will be waiting for a long while.

I am pretty sure it can store states or hashes somehow. And if not it can be extended.

Regards

Marcel

[PATCH v2 3/6] monitor: Filter duplicated advertisements

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This reduces the verbosity and give a precise picture of how unique
each report is.
---
 monitor/packet.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index cc24165..4ed0faa 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -42,7 +42,9 @@
 #include "lib/hci.h"
 #include "lib/hci_lib.h"
 
+#include "src/shared/mainloop.h"
 #include "src/shared/util.h"
+#include "src/shared/queue.h"
 #include "src/shared/btsnoop.h"
 #include "display.h"
 #include "bt.h"
@@ -100,6 +102,9 @@
 
 #define COLOR_PHY_PACKET		COLOR_BLUE
 
+#define ADV_TIMEOUT			3000
+#define ADV_MAX_CACHE			100
+
 static time_t time_offset = ((time_t) -1);
 static int priority_level = BTSNOOP_PRIORITY_INFO;
 static unsigned long filter_mask = 0;
@@ -267,11 +272,20 @@ void packet_select_index(uint16_t index)
 
 #define MAX_INDEX 16
 
+struct adv_data {
+	struct bt_hci_evt_le_adv_report evt;
+	void *data;
+	size_t frame;
+	int timeout;
+	struct queue *cache;
+};
+
 struct index_data {
 	uint8_t  type;
 	uint8_t  bdaddr[6];
 	uint16_t manufacturer;
 	size_t	frame;
+	struct queue *adv_cache;
 };
 
 static struct index_data index_list[MAX_INDEX];
@@ -8587,12 +8601,86 @@ static void le_conn_complete_evt(const void *data, uint8_t size)
 		assign_handle(le16_to_cpu(evt->handle), 0x01);
 }
 
+static void adv_free(void *data)
+{
+	struct adv_data *adv = data;
+
+	free(adv->data);
+	free(adv);
+}
+
+static bool match_adv(const void *data, const void *user_data)
+{
+	const struct adv_data *adv = data;
+	const struct bt_hci_evt_le_adv_report *evt = user_data;
+
+	if (memcmp(&adv->evt, evt, sizeof(*evt)))
+		return false;
+
+	return memcmp(adv->data, evt->data, evt->data_len) == 0;
+}
+
+static void adv_timeout(int id, void *data)
+{
+	struct adv_data *adv = data;
+
+	/* Remove from check (adv is freed after returning) */
+	queue_remove(adv->cache, adv);
+
+	mainloop_remove_timeout(adv->timeout);
+}
+
 static void le_adv_report_evt(const void *data, uint8_t size)
 {
 	const struct bt_hci_evt_le_adv_report *evt = data;
+	struct index_data *idata = &index_list[index_current];
+	struct adv_data *match;
+	struct adv_data *adv;
 	uint8_t evt_len;
 	int8_t *rssi;
 
+	if (!idata->adv_cache)
+		idata->adv_cache = queue_new();
+
+	adv = queue_remove_if(idata->adv_cache, match_adv, (void *) evt);
+	if (adv) {
+		/* Update cache and modify expire timeout */
+		if (mainloop_modify_timeout(adv->timeout, ADV_TIMEOUT) < 0) {
+			mainloop_remove_timeout(adv->timeout);
+			adv->timeout = mainloop_add_timeout(ADV_TIMEOUT,
+							adv_timeout, adv,
+							adv_free);
+		}
+		queue_push_head(idata->adv_cache, adv);
+		print_field("Duplicate of #%zu", adv->frame);
+		size = 0;
+		goto rssi;
+	}
+
+	match = new0(struct adv_data, 1);
+	match->evt = *evt;
+
+	match->data = malloc(evt->data_len);
+	memcpy(match->data, evt->data, evt->data_len);
+
+	match->cache = idata->adv_cache;
+	match->frame = idata->frame;
+	match->timeout = mainloop_add_timeout(ADV_TIMEOUT, adv_timeout, match,
+						adv_free);
+	if (match->timeout <= 0) {
+		print_field("Cannot cache: %s (%d)", strerror(-match->timeout),
+							match->timeout);
+		adv_free(match);
+		goto print;
+	}
+
+	queue_push_head(idata->adv_cache, match);
+
+	if (queue_length(idata->adv_cache) > ADV_MAX_CACHE)
+		queue_remove(idata->adv_cache,
+					queue_peek_tail(idata->adv_cache));
+
+print:
 	print_num_reports(evt->num_reports);
 
 report:
@@ -8602,6 +8690,7 @@ report:
 	print_field("Data length: %d", evt->data_len);
 	print_eir(evt->data, evt->data_len, true);
 
+rssi:
 	rssi = (int8_t *) (evt->data + evt->data_len);
 	print_rssi(*rssi);
 
-- 
2.9.4