[PATCH 1/4] monitor: remove extra black line

[PATCH 1/4] monitor: remove extra black line

[Gustavo Padovan]

---
 monitor/main.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/monitor/main.c b/monitor/main.c
index 4e8b433..90e32c5 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -92,7 +92,6 @@ int main(int argc, char *argv[])
 		}
 	}
 
-
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGINT);
 	sigaddset(&mask, SIGTERM);
-- 
1.7.10.1

[PATCH 2/4] monitor: add device filter support

[Gustavo Padovan]

-i hciX option will show only data from the specified device
---
 monitor/main.c   |   12 ++++++++++--
 monitor/packet.c |   10 +++++++++-
 monitor/packet.h |    2 +-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/monitor/main.c b/monitor/main.c
index 90e32c5..99b53e9 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -28,6 +28,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <getopt.h>
 
 #include "mainloop.h"
@@ -52,12 +53,14 @@ static void usage(void)
 		"Usage:\n");
 	printf("\tbtmon [options]\n");
 	printf("options:\n"
+		"\t-i dev                HCI device\n"
 		"\t-b, --btsnoop <file>  Save dump in btsnoop format\n"
 		"\t-h, --help            Show help options\n");
 }
 
 static const struct option main_options[] = {
 	{ "btsnoop",	required_argument, NULL, 'b'	},
+	{ "",		required_argument, NULL, 'i'	},
 	{ "version",	no_argument,	   NULL, 'v'	},
 	{ "help",	no_argument,	   NULL, 'h'	},
 	{ }
@@ -67,13 +70,14 @@ int main(int argc, char *argv[])
 {
 	unsigned long filter_mask = 0;
 	sigset_t mask;
+	int index = -1;
 
 	mainloop_init();
 
 	for (;;) {
 		int opt;
 
-		opt = getopt_long(argc, argv, "bvh", main_options, NULL);
+		opt = getopt_long(argc, argv, "bi:vh", main_options, NULL);
 		if (opt < 0)
 			break;
 
@@ -81,6 +85,10 @@ int main(int argc, char *argv[])
 		case 'b':
 			btsnoop_open(optarg);
 			break;
+		case 'i':
+			if (!strncasecmp(optarg, "hci", 3))
+				index = atoi(optarg + 3);
+			break;
 		case 'v':
 			printf("%s\n", VERSION);
 			return EXIT_SUCCESS;
@@ -102,7 +110,7 @@ int main(int argc, char *argv[])
 	filter_mask |= PACKET_FILTER_SHOW_TIME;
 	filter_mask |= PACKET_FILTER_SHOW_ACL_DATA;
 
-	packet_set_filter(filter_mask);
+	packet_set_filter(filter_mask, index);
 
 	printf("Bluetooth monitor ver %s\n", VERSION);
 
diff --git a/monitor/packet.c b/monitor/packet.c
index 356e0a3..8379074 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -45,10 +45,12 @@
 #include "packet.h"
 
 static unsigned long filter_mask = 0;
+static int filter_index = -1;
 
-void packet_set_filter(unsigned long filter)
+void packet_set_filter(unsigned long filter, int index)
 {
 	filter_mask = filter;
+	filter_index = index;
 }
 
 static void print_channel_header(struct timeval *tv, uint16_t index,
@@ -128,6 +130,9 @@ void packet_hexdump(const unsigned char *buf, uint16_t len)
 void packet_control(struct timeval *tv, uint16_t index, uint16_t opcode,
 					const void *data, uint16_t size)
 {
+	if (filter_index >= 0 && filter_index != index)
+		return;
+
 	print_channel_header(tv, index, HCI_CHANNEL_CONTROL);
 
 	control_message(opcode, data, size);
@@ -163,6 +168,9 @@ void packet_monitor(struct timeval *tv, uint16_t index, uint16_t opcode,
 	const struct monitor_new_index *ni;
 	char str[18];
 
+	if (filter_index >= 0 && filter_index != index)
+		return;
+
 	switch (opcode) {
 	case MONITOR_NEW_INDEX:
 		ni = data;
diff --git a/monitor/packet.h b/monitor/packet.h
index 90fc7ec..525ce83 100644
--- a/monitor/packet.h
+++ b/monitor/packet.h
@@ -31,7 +31,7 @@
 #define PACKET_FILTER_SHOW_ACL_DATA	(1 << 3)
 #define PACKET_FILTER_SHOW_SCO_DATA	(1 << 4)
 
-void packet_set_filter(unsigned long filter);
+void packet_set_filter(unsigned long filter, int index);
 
 void packet_hexdump(const unsigned char *buf, uint16_t len);
 
-- 
1.7.10.1

[PATCH 3/4] monitor: add command line filter options

[Gustavo Padovan]

right now "mgmt", "hci", "l2cap" and "sco" is supported.
---
 monitor/main.c   |   45 ++++++++++++++++++++++++++++++++++++++++-----
 monitor/packet.c |   15 +++++++++++++++
 monitor/packet.h |    4 ++++
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/monitor/main.c b/monitor/main.c
index 99b53e9..6096441 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -47,6 +47,34 @@ static void signal_callback(int signum, void *user_data)
 	}
 }
 
+static struct {
+	char *name;
+	int  flag;
+} filters[] = {
+	{ "mgmt",	PACKET_FILTER_SHOW_MGMT	},
+	{ "hci",	PACKET_FILTER_SHOW_HCI	},
+	{ "l2cap",	PACKET_FILTER_SHOW_ACL	},
+	{ "sco",	PACKET_FILTER_SHOW_SCO	},
+	{ 0 }
+};
+
+static unsigned long parse_filter(int argc, char **argv)
+{
+	unsigned long filter = 0;
+	int i,n;
+
+	for (i = 0; i < argc; i++) {
+		for (n = 0; filters[n].name; n++) {
+			if (!strcasecmp(filters[n].name, argv[i])) {
+				filter |= filters[n].flag;
+				break;
+			}
+		}
+	}
+
+	return filter;
+}
+
 static void usage(void)
 {
 	printf("btmon - Bluetooth monitor\n"
@@ -100,18 +128,25 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	sigemptyset(&mask);
-	sigaddset(&mask, SIGINT);
-	sigaddset(&mask, SIGTERM);
+	filter_mask = parse_filter(argc, argv);
 
-	mainloop_set_signal(&mask, signal_callback, NULL, NULL);
+	if(!filter_mask)
+		filter_mask = ~0L;
 
 	filter_mask |= PACKET_FILTER_SHOW_INDEX;
-	filter_mask |= PACKET_FILTER_SHOW_TIME;
+	filter_mask &= ~PACKET_FILTER_SHOW_DATE;
 	filter_mask |= PACKET_FILTER_SHOW_ACL_DATA;
+	filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
+	filter_mask |= PACKET_FILTER_SHOW_TIME;
 
 	packet_set_filter(filter_mask, index);
 
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+
+	mainloop_set_signal(&mask, signal_callback, NULL, NULL);
+
 	printf("Bluetooth monitor ver %s\n", VERSION);
 
 	if (control_tracing() < 0) {
diff --git a/monitor/packet.c b/monitor/packet.c
index 8379074..6ab7ee7 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -133,6 +133,9 @@ void packet_control(struct timeval *tv, uint16_t index, uint16_t opcode,
 	if (filter_index >= 0 && filter_index != index)
 		return;
 
+	if (!(filter_mask & PACKET_FILTER_SHOW_MGMT))
+		return;
+
 	print_channel_header(tv, index, HCI_CHANNEL_CONTROL);
 
 	control_message(opcode, data, size);
@@ -581,6 +584,9 @@ void packet_hci_command(struct timeval *tv, uint16_t index,
 
 	btsnoop_write(tv, index, 0x02, data, size);
 
+	if (!(filter_mask & PACKET_FILTER_SHOW_HCI))
+		return;
+
 	print_header(tv, index);
 
 	if (size < HCI_COMMAND_HDR_SIZE) {
@@ -602,6 +608,9 @@ void packet_hci_event(struct timeval *tv, uint16_t index,
 {
 	const hci_event_hdr *hdr = data;
 
+	if (!(filter_mask & PACKET_FILTER_SHOW_HCI))
+		return;
+
 	btsnoop_write(tv, index, 0x03, data, size);
 
 	print_header(tv, index);
@@ -628,6 +637,9 @@ void packet_hci_acldata(struct timeval *tv, uint16_t index, bool in,
 	uint16_t dlen = btohs(hdr->dlen);
 	uint8_t flags = acl_flags(handle);
 
+	if (!(filter_mask & PACKET_FILTER_SHOW_ACL))
+		return;
+
 	btsnoop_write(tv, index, in ? 0x01 : 0x00, data, size);
 
 	print_header(tv, index);
@@ -654,6 +666,9 @@ void packet_hci_scodata(struct timeval *tv, uint16_t index, bool in,
 	uint16_t handle = btohs(hdr->handle);
 	uint8_t flags = acl_flags(handle);
 
+	if (!(filter_mask & PACKET_FILTER_SHOW_SCO))
+		return;
+
 	print_header(tv, index);
 
 	if (size < HCI_SCO_HDR_SIZE) {
diff --git a/monitor/packet.h b/monitor/packet.h
index 525ce83..65f90bb 100644
--- a/monitor/packet.h
+++ b/monitor/packet.h
@@ -30,6 +30,10 @@
 #define PACKET_FILTER_SHOW_TIME		(1 << 2)
 #define PACKET_FILTER_SHOW_ACL_DATA	(1 << 3)
 #define PACKET_FILTER_SHOW_SCO_DATA	(1 << 4)
+#define PACKET_FILTER_SHOW_HCI		(1 << 5)
+#define PACKET_FILTER_SHOW_ACL		(1 << 6)
+#define PACKET_FILTER_SHOW_SCO		(1 << 7)
+#define PACKET_FILTER_SHOW_MGMT		(1 << 8)
 
 void packet_set_filter(unsigned long filter, int index);
 
-- 
1.7.10.1

[PATCH 4/4] monitor: show index only when -i is not specified

[Gustavo Padovan]

---
 monitor/main.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/monitor/main.c b/monitor/main.c
index 6096441..f44cdf1 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -133,7 +133,11 @@ int main(int argc, char *argv[])
 	if(!filter_mask)
 		filter_mask = ~0L;
 
-	filter_mask |= PACKET_FILTER_SHOW_INDEX;
+	if (index >= 0)
+		filter_mask &= ~PACKET_FILTER_SHOW_INDEX;
+	else
+		filter_mask |= PACKET_FILTER_SHOW_INDEX;
+
 	filter_mask &= ~PACKET_FILTER_SHOW_DATE;
 	filter_mask |= PACKET_FILTER_SHOW_ACL_DATA;
 	filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
-- 
1.7.10.1

[PATCH -v2 1/4] monitor: remove extra blank line

[Gustavo Padovan]

---
 monitor/main.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/monitor/main.c b/monitor/main.c
index 4e8b433..90e32c5 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -92,7 +92,6 @@ int main(int argc, char *argv[])
 		}
 	}
 
-
 	sigemptyset(&mask);
 	sigaddset(&mask, SIGINT);
 	sigaddset(&mask, SIGTERM);
-- 
1.7.10.1

Re: [PATCH 1/4] monitor: remove extra black line

[Vinicius Costa Gomes]

Hi Gustavo,

On 13:18 Fri 11 May, Gustavo Padovan wrote:
> ---
>  monitor/main.c |    1 -
>  1 file changed, 1 deletion(-)
> 

Minor nitpick: in the subject: black -> blank


Cheers,
-- 
Vinicius

Re: [PATCH 2/4] monitor: add device filter support

[Vinicius Costa Gomes]

Hi Gustavo,

On 13:18 Fri 11 May, Gustavo Padovan wrote:
> -i hciX option will show only data from the specified device
> ---
>  monitor/main.c   |   12 ++++++++++--
>  monitor/packet.c |   10 +++++++++-
>  monitor/packet.h |    2 +-
>  3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/monitor/main.c b/monitor/main.c
> index 90e32c5..99b53e9 100644
> --- a/monitor/main.c
> +++ b/monitor/main.c
> @@ -28,6 +28,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <getopt.h>
>  
>  #include "mainloop.h"
> @@ -52,12 +53,14 @@ static void usage(void)
>  		"Usage:\n");
>  	printf("\tbtmon [options]\n");
>  	printf("options:\n"
> +		"\t-i dev                HCI device\n"
>  		"\t-b, --btsnoop <file>  Save dump in btsnoop format\n"
>  		"\t-h, --help            Show help options\n");
>  }
>  
>  static const struct option main_options[] = {
>  	{ "btsnoop",	required_argument, NULL, 'b'	},
> +	{ "",		required_argument, NULL, 'i'	},

btmgmt is calling this "index", how about using the same long name for
consistency?

[snip]


Cheers,
-- 
Vinicius

Re: [PATCH 1/4] monitor: remove extra black line

[Marcel Holtmann]

Hi Gustavo,

> ---
>  monitor/main.c |    1 -
>  1 file changed, 1 deletion(-)

patch has been applied.

Regards

Marcel

Re: [PATCH 2/4] monitor: add device filter support

[Marcel Holtmann]

Hi Gustavo,

> -i hciX option will show only data from the specified device
> ---
>  monitor/main.c   |   12 ++++++++++--
>  monitor/packet.c |   10 +++++++++-
>  monitor/packet.h |    2 +-
>  3 files changed, 20 insertions(+), 4 deletions(-)

so what is this option actually good for? I think it is the wrong
approach for dealing with this.

We better have some generic filtering mechanism, then trying to just
duplicate options from hcidump into btmon.

btmon is different in the fact that it is actually able to record all
packets from all devices. You also do not wanna filter based on index
number. You wanna filter based on BD_ADDR and BR/EDR vs AMP.

Regards

Marcel

Re: [PATCH 2/4] monitor: add device filter support

[Gustavo Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2012-05-13 19:58:46 -0700]:

> Hi Gustavo,
> 
> > -i hciX option will show only data from the specified device
> > ---
> >  monitor/main.c   |   12 ++++++++++--
> >  monitor/packet.c |   10 +++++++++-
> >  monitor/packet.h |    2 +-
> >  3 files changed, 20 insertions(+), 4 deletions(-)
> 
> so what is this option actually good for? I think it is the wrong
> approach for dealing with this.

I only want to see the output for one device. We one my test use case is two
devices in the machine communicating with each other.

> 
> We better have some generic filtering mechanism, then trying to just
> duplicate options from hcidump into btmon.
> 
> btmon is different in the fact that it is actually able to record all
> packets from all devices. You also do not wanna filter based on index
> number. You wanna filter based on BD_ADDR and BR/EDR vs AMP.

Filter based on BD_ADDR would work too, but is more painful to the user
however.

	Gustavo

Re: [PATCH 2/4] monitor: add device filter support

[Marcel Holtmann]

Hi Gustavo,

> > > -i hciX option will show only data from the specified device
> > > ---
> > >  monitor/main.c   |   12 ++++++++++--
> > >  monitor/packet.c |   10 +++++++++-
> > >  monitor/packet.h |    2 +-
> > >  3 files changed, 20 insertions(+), 4 deletions(-)
> > 
> > so what is this option actually good for? I think it is the wrong
> > approach for dealing with this.
> 
> I only want to see the output for one device. We one my test use case is two
> devices in the machine communicating with each other.

so btmon is designed to really get everything at once. Including
adapters coming and going. More important it can tell if at one point an
adapter was hci0 and next time it becomes hci1.

Keep in mind that btmon is suppose to be run even with no adapters
attached. So you can start it before doing anything. Filtering on index
numbers becomes kinda funky then since you do not yet know the index of
an adapter. You are just wildly guessing.

> > We better have some generic filtering mechanism, then trying to just
> > duplicate options from hcidump into btmon.
> > 
> > btmon is different in the fact that it is actually able to record all
> > packets from all devices. You also do not wanna filter based on index
> > number. You wanna filter based on BD_ADDR and BR/EDR vs AMP.
> 
> Filter based on BD_ADDR would work too, but is more painful to the user
> however.

I actually think having some sort of -f <filter> file where you can
store BD_ADDR and potentially other filter options makes way more sense.

Keep in mind that even for BD_ADDR filtering, you need to first inspect
a few HCI packets on a newly added controller. Since a new controller
has no BD_ADDR until Read_BD_Addr command has successfully. Only already
added controllers have the BD_ADDR read.

Regards

Marcel

Re: [PATCH 2/4] monitor: add device filter support

[Marcel Holtmann]

Hi Gustavo,

> > > We better have some generic filtering mechanism, then trying to just
> > > duplicate options from hcidump into btmon.
> > > 
> > > btmon is different in the fact that it is actually able to record all
> > > packets from all devices. You also do not wanna filter based on index
> > > number. You wanna filter based on BD_ADDR and BR/EDR vs AMP.
> > 
> > Filter based on BD_ADDR would work too, but is more painful to the user
> > however.
> 
> I actually think having some sort of -f <filter> file where you can
> store BD_ADDR and potentially other filter options makes way more sense.

coming to think about this, the -f <filter> is the only proper way to
deal with this anyway. Everything else is just a hack. So I am currently
thinking in just doing a simple SSH config style filter file here.

	Controller aa:bb:cc:*
		Ignore

	Controller amp
		Ignore

	Controller bb:cc:*
		Hide sco,l2cap
		Show rfcomm
		Timestamps on

	Controller 11:22:33:44:55:66
		File x.btsnoop

Or something really similar to this. That way you can just have multiple
of these and only need to specify the correct file for what you are
trying to test. Including the capabilities to store BTSnoop files with
the same one command.

Regards

Marcel