[BlueZ RFC 1/3] monitor: Add btsnoop data link transfer way

[BlueZ RFC 1/3] monitor: Add btsnoop data link transfer way

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR:2001.
but some of btsnoop viewer could not handle it because they just
support BTSNOOP_TYPE_HCI(1001).

So they need transform way to analyze the btsnoop file captured by
btmon.
---
 monitor/control.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor/control.h |  1 +
 monitor/main.c    | 13 +++++++++++-
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/monitor/control.c b/monitor/control.c
index e61a79d..00e4bc0 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -1131,6 +1131,67 @@ bool control_writer(const char *path)
 	return !!btsnoop_file;
 }
 
+void transfer_btsnoop_data_link(const char *path, const char *writer_path)
+{
+	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
+	uint16_t pktlen;
+	uint32_t type;
+	struct timeval tv;
+	struct btsnoop *btsnoop_write_file = NULL;
+	unsigned long num_packets = 0;
+
+	btsnoop_file = btsnoop_open(path, BTSNOOP_FLAG_PKLG_SUPPORT);
+	if (!btsnoop_file)
+		return;
+
+	btsnoop_write_file = btsnoop_create(writer_path, BTSNOOP_TYPE_HCI);
+	if (!btsnoop_write_file)
+		return;
+
+	type = btsnoop_get_type(btsnoop_file);
+
+	switch (type) {
+	case BTSNOOP_TYPE_HCI:
+	case BTSNOOP_TYPE_UART:
+	case BTSNOOP_TYPE_SIMULATOR:
+		packet_del_filter(PACKET_FILTER_SHOW_INDEX);
+		break;
+
+	case BTSNOOP_TYPE_MONITOR:
+		packet_add_filter(PACKET_FILTER_SHOW_INDEX);
+		break;
+	}
+
+	switch (type) {
+	case BTSNOOP_TYPE_HCI:
+	case BTSNOOP_TYPE_UART:
+	case BTSNOOP_TYPE_MONITOR:
+		while (1) {
+			uint16_t index, opcode;
+
+			if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
+							&opcode, buf, &pktlen))
+				break;
+
+			if (opcode == 0xffff)
+				continue;
+
+			btsnoop_write_hci(btsnoop_write_file, &tv, index,
+							opcode, buf, pktlen);
+			num_packets++;
+		}
+		break;
+	}
+
+	btsnoop_unref(btsnoop_file);
+	btsnoop_unref(btsnoop_write_file);
+
+	printf("BT Snoop data link transfer is completed for %lu packets\n",
+								num_packets);
+	printf("Output is saved in %s\n", writer_path);
+}
+
+
 void control_reader(const char *path)
 {
 	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
diff --git a/monitor/control.h b/monitor/control.h
index 28f16db..267d71b 100644
--- a/monitor/control.h
+++ b/monitor/control.h
@@ -28,5 +28,6 @@ bool control_writer(const char *path);
 void control_reader(const char *path);
 void control_server(const char *path);
 int control_tracing(void);
+void transfer_btsnoop_data_link(const char *path, const char *writer_path);
 
 void control_message(uint16_t opcode, const void *data, uint16_t size);
diff --git a/monitor/main.c b/monitor/main.c
index de48db5..6e7d4b3 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -59,6 +59,7 @@ static void usage(void)
 	printf("options:\n"
 		"\t-r, --read <file>      Read traces in btsnoop format\n"
 		"\t-w, --write <file>     Save traces in btsnoop format\n"
+		"\t-f, --transfer <file>  Transfer btsnoop data link type\n"
 		"\t-a, --analyze <file>   Analyze traces in btsnoop format\n"
 		"\t-s, --server <socket>  Start monitor server socket\n"
 		"\t-i, --index <num>      Show only specified controller\n"
@@ -72,6 +73,7 @@ static void usage(void)
 static const struct option main_options[] = {
 	{ "read",    required_argument, NULL, 'r' },
 	{ "write",   required_argument, NULL, 'w' },
+	{ "transfer", required_argument, NULL, 'f' },
 	{ "analyze", required_argument, NULL, 'a' },
 	{ "server",  required_argument, NULL, 's' },
 	{ "index",   required_argument, NULL, 'i' },
@@ -104,7 +106,7 @@ int main(int argc, char *argv[])
 	for (;;) {
 		int opt;
 
-		opt = getopt_long(argc, argv, "r:w:a:s:i:tTSE:vh",
+		opt = getopt_long(argc, argv, "r:w:f:a:s:i:tTSE:vh",
 						main_options, NULL);
 		if (opt < 0)
 			break;
@@ -116,6 +118,10 @@ int main(int argc, char *argv[])
 		case 'w':
 			writer_path = optarg;
 			break;
+		case 'f':
+			reader_path = optarg;
+			writer_path = "btsnoop_type_hci.log";
+			break;
 		case 'a':
 			analyze_path = optarg;
 			break;
@@ -191,6 +197,11 @@ int main(int argc, char *argv[])
 		return EXIT_SUCCESS;
 	}
 
+	if (reader_path && writer_path) {
+		transfer_btsnoop_data_link(reader_path, writer_path);
+		return EXIT_SUCCESS;
+	}
+
 	if (reader_path) {
 		if (ellisys_server)
 			ellisys_enable(ellisys_server, ellisys_port);
-- 
2.1.0

[BlueZ RFC 2/3] monitor: Add btsnoop data link type option

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR(2001) statically.
but some of btsnoop viewer just support BTSNOOP_TYPE_HCI(1001, so they could
not use btmon's btsnoop. If this option is set, they could parse btsnoop.
---
 monitor/control.c | 4 ++--
 monitor/control.h | 2 +-
 monitor/main.c    | 9 ++++++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/monitor/control.c b/monitor/control.c
index 00e4bc0..72204bf 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -1124,9 +1124,9 @@ void control_server(const char *path)
 	server_fd = fd;
 }
 
-bool control_writer(const char *path)
+bool control_writer(const char *path, uint32_t type)
 {
-	btsnoop_file = btsnoop_create(path, BTSNOOP_TYPE_MONITOR);
+	btsnoop_file = btsnoop_create(path, type);
 
 	return !!btsnoop_file;
 }
diff --git a/monitor/control.h b/monitor/control.h
index 267d71b..680e6b6 100644
--- a/monitor/control.h
+++ b/monitor/control.h
@@ -24,7 +24,7 @@
 
 #include <stdint.h>
 
-bool control_writer(const char *path);
+bool control_writer(const char *path, uint32_t type);
 void control_reader(const char *path);
 void control_server(const char *path);
 int control_tracing(void);
diff --git a/monitor/main.c b/monitor/main.c
index 6e7d4b3..66d7ad1 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -33,6 +33,7 @@
 #include <getopt.h>
 
 #include "src/shared/mainloop.h"
+#include "src/shared/btsnoop.h"
 
 #include "packet.h"
 #include "lmp.h"
@@ -66,6 +67,7 @@ static void usage(void)
 		"\t-t, --time             Show time instead of time offset\n"
 		"\t-T, --date             Show time and date information\n"
 		"\t-S, --sco              Dump SCO traffic\n"
+		"\t-H, --hci              Use BTSNOOP_TYPE_HCI\n"
 		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
 		"\t-h, --help             Show help options\n");
 }
@@ -80,6 +82,7 @@ static const struct option main_options[] = {
 	{ "time",    no_argument,       NULL, 't' },
 	{ "date",    no_argument,       NULL, 'T' },
 	{ "sco",     no_argument,	NULL, 'S' },
+	{ "hci",     no_argument,	NULL, 'H' },
 	{ "ellisys", required_argument, NULL, 'E' },
 	{ "todo",    no_argument,       NULL, '#' },
 	{ "version", no_argument,       NULL, 'v' },
@@ -98,6 +101,7 @@ int main(int argc, char *argv[])
 	const char *str;
 	int exit_status;
 	sigset_t mask;
+	uint32_t type = BTSNOOP_TYPE_MONITOR;
 
 	mainloop_init();
 
@@ -151,6 +155,9 @@ int main(int argc, char *argv[])
 		case 'S':
 			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
 			break;
+		case 'H':
+			type = BTSNOOP_TYPE_HCI;
+			break;
 		case 'E':
 			ellisys_server = optarg;
 			ellisys_port = 24352;
@@ -210,7 +217,7 @@ int main(int argc, char *argv[])
 		return EXIT_SUCCESS;
 	}
 
-	if (writer_path && !control_writer(writer_path)) {
+	if (writer_path && !control_writer(writer_path, type)) {
 		printf("Failed to open '%s'\n", writer_path);
 		return EXIT_FAILURE;
 	}
-- 
2.1.0

Re: [BlueZ RFC 2/3] monitor: Add btsnoop data link type option

[Szymon Janc]

Hi Chan-yeol Park,

On Thursday 23 of April 2015 03:49:24 chanyeol.park@gmail.com wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
> 
> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR(2001)
> statically. but some of btsnoop viewer just support BTSNOOP_TYPE_HCI(1001,
> so they could not use btmon's btsnoop. If this option is set, they could
> parse btsnoop. ---
>  monitor/control.c | 4 ++--
>  monitor/control.h | 2 +-
>  monitor/main.c    | 9 ++++++++-
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/monitor/control.c b/monitor/control.c
> index 00e4bc0..72204bf 100644
> --- a/monitor/control.c
> +++ b/monitor/control.c
> @@ -1124,9 +1124,9 @@ void control_server(const char *path)
>  	server_fd = fd;
>  }
> 
> -bool control_writer(const char *path)
> +bool control_writer(const char *path, uint32_t type)
>  {
> -	btsnoop_file = btsnoop_create(path, BTSNOOP_TYPE_MONITOR);
> +	btsnoop_file = btsnoop_create(path, type);
> 
>  	return !!btsnoop_file;
>  }
> diff --git a/monitor/control.h b/monitor/control.h
> index 267d71b..680e6b6 100644
> --- a/monitor/control.h
> +++ b/monitor/control.h
> @@ -24,7 +24,7 @@
> 
>  #include <stdint.h>
> 
> -bool control_writer(const char *path);
> +bool control_writer(const char *path, uint32_t type);
>  void control_reader(const char *path);
>  void control_server(const char *path);
>  int control_tracing(void);
> diff --git a/monitor/main.c b/monitor/main.c
> index 6e7d4b3..66d7ad1 100644
> --- a/monitor/main.c
> +++ b/monitor/main.c
> @@ -33,6 +33,7 @@
>  #include <getopt.h>
> 
>  #include "src/shared/mainloop.h"
> +#include "src/shared/btsnoop.h"
> 
>  #include "packet.h"
>  #include "lmp.h"
> @@ -66,6 +67,7 @@ static void usage(void)
>  		"\t-t, --time             Show time instead of time offset\n"
>  		"\t-T, --date             Show time and date information\n"
>  		"\t-S, --sco              Dump SCO traffic\n"
> +		"\t-H, --hci              Use BTSNOOP_TYPE_HCI\n"
>  		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
>  		"\t-h, --help             Show help options\n");
>  }
> @@ -80,6 +82,7 @@ static const struct option main_options[] = {
>  	{ "time",    no_argument,       NULL, 't' },
>  	{ "date",    no_argument,       NULL, 'T' },
>  	{ "sco",     no_argument,	NULL, 'S' },
> +	{ "hci",     no_argument,	NULL, 'H' },
>  	{ "ellisys", required_argument, NULL, 'E' },
>  	{ "todo",    no_argument,       NULL, '#' },
>  	{ "version", no_argument,       NULL, 'v' },
> @@ -98,6 +101,7 @@ int main(int argc, char *argv[])
>  	const char *str;
>  	int exit_status;
>  	sigset_t mask;
> +	uint32_t type = BTSNOOP_TYPE_MONITOR;
> 
>  	mainloop_init();
> 
> @@ -151,6 +155,9 @@ int main(int argc, char *argv[])
>  		case 'S':
>  			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
>  			break;
> +		case 'H':
> +			type = BTSNOOP_TYPE_HCI;
> +			break;
>  		case 'E':
>  			ellisys_server = optarg;
>  			ellisys_port = 24352;
> @@ -210,7 +217,7 @@ int main(int argc, char *argv[])
>  		return EXIT_SUCCESS;
>  	}
> 
> -	if (writer_path && !control_writer(writer_path)) {
> +	if (writer_path && !control_writer(writer_path, type)) {
>  		printf("Failed to open '%s'\n", writer_path);
>  		return EXIT_FAILURE;
>  	}

This option should be tightly coupled with --index option since legacy format 
cannot store index id.

-- 
BR
Szymon Janc

Re: [BlueZ RFC 2/3] monitor: Add btsnoop data link type option

[Marcel Holtmann]

Hi Szymon,

>> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR(2001)
>> statically. but some of btsnoop viewer just support BTSNOOP_TYPE_HCI(1001,
>> so they could not use btmon's btsnoop. If this option is set, they could
>> parse btsnoop. ---
>> monitor/control.c | 4 ++--
>> monitor/control.h | 2 +-
>> monitor/main.c    | 9 ++++++++-
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>> 
>> diff --git a/monitor/control.c b/monitor/control.c
>> index 00e4bc0..72204bf 100644
>> --- a/monitor/control.c
>> +++ b/monitor/control.c
>> @@ -1124,9 +1124,9 @@ void control_server(const char *path)
>> 	server_fd = fd;
>> }
>> 
>> -bool control_writer(const char *path)
>> +bool control_writer(const char *path, uint32_t type)
>> {
>> -	btsnoop_file = btsnoop_create(path, BTSNOOP_TYPE_MONITOR);
>> +	btsnoop_file = btsnoop_create(path, type);
>> 
>> 	return !!btsnoop_file;
>> }
>> diff --git a/monitor/control.h b/monitor/control.h
>> index 267d71b..680e6b6 100644
>> --- a/monitor/control.h
>> +++ b/monitor/control.h
>> @@ -24,7 +24,7 @@
>> 
>> #include <stdint.h>
>> 
>> -bool control_writer(const char *path);
>> +bool control_writer(const char *path, uint32_t type);
>> void control_reader(const char *path);
>> void control_server(const char *path);
>> int control_tracing(void);
>> diff --git a/monitor/main.c b/monitor/main.c
>> index 6e7d4b3..66d7ad1 100644
>> --- a/monitor/main.c
>> +++ b/monitor/main.c
>> @@ -33,6 +33,7 @@
>> #include <getopt.h>
>> 
>> #include "src/shared/mainloop.h"
>> +#include "src/shared/btsnoop.h"
>> 
>> #include "packet.h"
>> #include "lmp.h"
>> @@ -66,6 +67,7 @@ static void usage(void)
>> 		"\t-t, --time             Show time instead of time offset\n"
>> 		"\t-T, --date             Show time and date information\n"
>> 		"\t-S, --sco              Dump SCO traffic\n"
>> +		"\t-H, --hci              Use BTSNOOP_TYPE_HCI\n"
>> 		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
>> 		"\t-h, --help             Show help options\n");
>> }
>> @@ -80,6 +82,7 @@ static const struct option main_options[] = {
>> 	{ "time",    no_argument,       NULL, 't' },
>> 	{ "date",    no_argument,       NULL, 'T' },
>> 	{ "sco",     no_argument,	NULL, 'S' },
>> +	{ "hci",     no_argument,	NULL, 'H' },
>> 	{ "ellisys", required_argument, NULL, 'E' },
>> 	{ "todo",    no_argument,       NULL, '#' },
>> 	{ "version", no_argument,       NULL, 'v' },
>> @@ -98,6 +101,7 @@ int main(int argc, char *argv[])
>> 	const char *str;
>> 	int exit_status;
>> 	sigset_t mask;
>> +	uint32_t type = BTSNOOP_TYPE_MONITOR;
>> 
>> 	mainloop_init();
>> 
>> @@ -151,6 +155,9 @@ int main(int argc, char *argv[])
>> 		case 'S':
>> 			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
>> 			break;
>> +		case 'H':
>> +			type = BTSNOOP_TYPE_HCI;
>> +			break;
>> 		case 'E':
>> 			ellisys_server = optarg;
>> 			ellisys_port = 24352;
>> @@ -210,7 +217,7 @@ int main(int argc, char *argv[])
>> 		return EXIT_SUCCESS;
>> 	}
>> 
>> -	if (writer_path && !control_writer(writer_path)) {
>> +	if (writer_path && !control_writer(writer_path, type)) {
>> 		printf("Failed to open '%s'\n", writer_path);
>> 		return EXIT_FAILURE;
>> 	}
> 
> This option should be tightly coupled with --index option since legacy format 
> cannot store index id.

I do not like this. btmon should always write the new format. Otherwise you are missing the index added and removed extra meta data.

Regards

Marcel

Re: [BlueZ RFC 2/3] monitor: Add btsnoop data link type option

[Chanyeol Park]

Hi Marcel,

On Thu, 2015-04-23 at 09:53 -0700, Marcel Holtmann wrote:
> Hi Szymon,
> 
> >> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR(2001)
> >> statically. but some of btsnoop viewer just support BTSNOOP_TYPE_HCI(1001,
> >> so they could not use btmon's btsnoop. If this option is set, they could
> >> parse btsnoop. ---
> >> monitor/control.c | 4 ++--
> >> monitor/control.h | 2 +-
> >> monitor/main.c    | 9 ++++++++-
> >> 3 files changed, 11 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/monitor/control.c b/monitor/control.c
> >> index 00e4bc0..72204bf 100644
> >> --- a/monitor/control.c
> >> +++ b/monitor/control.c
> >> @@ -1124,9 +1124,9 @@ void control_server(const char *path)
> >> 	server_fd = fd;
> >> }
> >> 
> >> -bool control_writer(const char *path)
> >> +bool control_writer(const char *path, uint32_t type)
> >> {
> >> -	btsnoop_file = btsnoop_create(path, BTSNOOP_TYPE_MONITOR);
> >> +	btsnoop_file = btsnoop_create(path, type);
> >> 
> >> 	return !!btsnoop_file;
> >> }
> >> diff --git a/monitor/control.h b/monitor/control.h
> >> index 267d71b..680e6b6 100644
> >> --- a/monitor/control.h
> >> +++ b/monitor/control.h
> >> @@ -24,7 +24,7 @@
> >> 
> >> #include <stdint.h>
> >> 
> >> -bool control_writer(const char *path);
> >> +bool control_writer(const char *path, uint32_t type);
> >> void control_reader(const char *path);
> >> void control_server(const char *path);
> >> int control_tracing(void);
> >> diff --git a/monitor/main.c b/monitor/main.c
> >> index 6e7d4b3..66d7ad1 100644
> >> --- a/monitor/main.c
> >> +++ b/monitor/main.c
> >> @@ -33,6 +33,7 @@
> >> #include <getopt.h>
> >> 
> >> #include "src/shared/mainloop.h"
> >> +#include "src/shared/btsnoop.h"
> >> 
> >> #include "packet.h"
> >> #include "lmp.h"
> >> @@ -66,6 +67,7 @@ static void usage(void)
> >> 		"\t-t, --time             Show time instead of time offset\n"
> >> 		"\t-T, --date             Show time and date information\n"
> >> 		"\t-S, --sco              Dump SCO traffic\n"
> >> +		"\t-H, --hci              Use BTSNOOP_TYPE_HCI\n"
> >> 		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
> >> 		"\t-h, --help             Show help options\n");
> >> }
> >> @@ -80,6 +82,7 @@ static const struct option main_options[] = {
> >> 	{ "time",    no_argument,       NULL, 't' },
> >> 	{ "date",    no_argument,       NULL, 'T' },
> >> 	{ "sco",     no_argument,	NULL, 'S' },
> >> +	{ "hci",     no_argument,	NULL, 'H' },
> >> 	{ "ellisys", required_argument, NULL, 'E' },
> >> 	{ "todo",    no_argument,       NULL, '#' },
> >> 	{ "version", no_argument,       NULL, 'v' },
> >> @@ -98,6 +101,7 @@ int main(int argc, char *argv[])
> >> 	const char *str;
> >> 	int exit_status;
> >> 	sigset_t mask;
> >> +	uint32_t type = BTSNOOP_TYPE_MONITOR;
> >> 
> >> 	mainloop_init();
> >> 
> >> @@ -151,6 +155,9 @@ int main(int argc, char *argv[])
> >> 		case 'S':
> >> 			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
> >> 			break;
> >> +		case 'H':
> >> +			type = BTSNOOP_TYPE_HCI;
> >> +			break;
> >> 		case 'E':
> >> 			ellisys_server = optarg;
> >> 			ellisys_port = 24352;
> >> @@ -210,7 +217,7 @@ int main(int argc, char *argv[])
> >> 		return EXIT_SUCCESS;
> >> 	}
> >> 
> >> -	if (writer_path && !control_writer(writer_path)) {
> >> +	if (writer_path && !control_writer(writer_path, type)) {
> >> 		printf("Failed to open '%s'\n", writer_path);
> >> 		return EXIT_FAILURE;
> >> 	}
> > 
> > This option should be tightly coupled with --index option since legacy format 
> > cannot store index id.
> 
> I do not like this. btmon should always write the new format. Otherwise you are missing the index added and removed extra meta data.
> 
> Regards
> 
> Marcel
> 
Thanks for your opinion. I am not clear which tool(btmon, hcidump,
android hcidump in bluez) is the best option to capture Bluetooth
packets in real products.

As you know tizen uses hcidump. but it seems that development of hcidump
is stopped. So I would like to use btmon instead of hcidump if possible.

Currently our popular Bluetooth analyzer, front line viewer could not
open btmon packet. Is btmon way standard? I could not find it.

If we use your proposal well, I think the index added and removed extra
meta data could be written in hcidump file name, for example
foo_leagacy_hci0_12:34:56.log only when user wants.

Regards
Chanyeol

Re: [BlueZ RFC 2/3] monitor: Add btsnoop data link type option

[Marcel Holtmann]

Hi Chanyeol,

>>>> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR(2001)
>>>> statically. but some of btsnoop viewer just support BTSNOOP_TYPE_HCI(1001,
>>>> so they could not use btmon's btsnoop. If this option is set, they could
>>>> parse btsnoop. ---
>>>> monitor/control.c | 4 ++--
>>>> monitor/control.h | 2 +-
>>>> monitor/main.c    | 9 ++++++++-
>>>> 3 files changed, 11 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/monitor/control.c b/monitor/control.c
>>>> index 00e4bc0..72204bf 100644
>>>> --- a/monitor/control.c
>>>> +++ b/monitor/control.c
>>>> @@ -1124,9 +1124,9 @@ void control_server(const char *path)
>>>> 	server_fd = fd;
>>>> }
>>>> 
>>>> -bool control_writer(const char *path)
>>>> +bool control_writer(const char *path, uint32_t type)
>>>> {
>>>> -	btsnoop_file = btsnoop_create(path, BTSNOOP_TYPE_MONITOR);
>>>> +	btsnoop_file = btsnoop_create(path, type);
>>>> 
>>>> 	return !!btsnoop_file;
>>>> }
>>>> diff --git a/monitor/control.h b/monitor/control.h
>>>> index 267d71b..680e6b6 100644
>>>> --- a/monitor/control.h
>>>> +++ b/monitor/control.h
>>>> @@ -24,7 +24,7 @@
>>>> 
>>>> #include <stdint.h>
>>>> 
>>>> -bool control_writer(const char *path);
>>>> +bool control_writer(const char *path, uint32_t type);
>>>> void control_reader(const char *path);
>>>> void control_server(const char *path);
>>>> int control_tracing(void);
>>>> diff --git a/monitor/main.c b/monitor/main.c
>>>> index 6e7d4b3..66d7ad1 100644
>>>> --- a/monitor/main.c
>>>> +++ b/monitor/main.c
>>>> @@ -33,6 +33,7 @@
>>>> #include <getopt.h>
>>>> 
>>>> #include "src/shared/mainloop.h"
>>>> +#include "src/shared/btsnoop.h"
>>>> 
>>>> #include "packet.h"
>>>> #include "lmp.h"
>>>> @@ -66,6 +67,7 @@ static void usage(void)
>>>> 		"\t-t, --time             Show time instead of time offset\n"
>>>> 		"\t-T, --date             Show time and date information\n"
>>>> 		"\t-S, --sco              Dump SCO traffic\n"
>>>> +		"\t-H, --hci              Use BTSNOOP_TYPE_HCI\n"
>>>> 		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
>>>> 		"\t-h, --help             Show help options\n");
>>>> }
>>>> @@ -80,6 +82,7 @@ static const struct option main_options[] = {
>>>> 	{ "time",    no_argument,       NULL, 't' },
>>>> 	{ "date",    no_argument,       NULL, 'T' },
>>>> 	{ "sco",     no_argument,	NULL, 'S' },
>>>> +	{ "hci",     no_argument,	NULL, 'H' },
>>>> 	{ "ellisys", required_argument, NULL, 'E' },
>>>> 	{ "todo",    no_argument,       NULL, '#' },
>>>> 	{ "version", no_argument,       NULL, 'v' },
>>>> @@ -98,6 +101,7 @@ int main(int argc, char *argv[])
>>>> 	const char *str;
>>>> 	int exit_status;
>>>> 	sigset_t mask;
>>>> +	uint32_t type = BTSNOOP_TYPE_MONITOR;
>>>> 
>>>> 	mainloop_init();
>>>> 
>>>> @@ -151,6 +155,9 @@ int main(int argc, char *argv[])
>>>> 		case 'S':
>>>> 			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
>>>> 			break;
>>>> +		case 'H':
>>>> +			type = BTSNOOP_TYPE_HCI;
>>>> +			break;
>>>> 		case 'E':
>>>> 			ellisys_server = optarg;
>>>> 			ellisys_port = 24352;
>>>> @@ -210,7 +217,7 @@ int main(int argc, char *argv[])
>>>> 		return EXIT_SUCCESS;
>>>> 	}
>>>> 
>>>> -	if (writer_path && !control_writer(writer_path)) {
>>>> +	if (writer_path && !control_writer(writer_path, type)) {
>>>> 		printf("Failed to open '%s'\n", writer_path);
>>>> 		return EXIT_FAILURE;
>>>> 	}
>>> 
>>> This option should be tightly coupled with --index option since legacy format 
>>> cannot store index id.
>> 
>> I do not like this. btmon should always write the new format. Otherwise you are missing the index added and removed extra meta data.
>> 
>> Regards
>> 
>> Marcel
>> 
> Thanks for your opinion. I am not clear which tool(btmon, hcidump,
> android hcidump in bluez) is the best option to capture Bluetooth
> packets in real products.
> 
> As you know tizen uses hcidump. but it seems that development of hcidump
> is stopped. So I would like to use btmon instead of hcidump if possible.
> 
> Currently our popular Bluetooth analyzer, front line viewer could not
> open btmon packet. Is btmon way standard? I could not find it.

the original btsnoop format is from Frontline. I extended the format to the newer btmon format. Maybe it is time to ask Frontline to support it. Wireshark does already for example.

I expect people to use btmon and we will deprecate hcidump at some point. btmon uses the Bluetooth monitor interface which is way more efficient and clean compared to the old legacy hcidump raw interface. The old interface has problems and eventually there will be a kernel option that allows disabling it.

So use btmon and just add support to btsnoop for conversion. Seems btsnoop should get a --split option.

> If we use your proposal well, I think the index added and removed extra
> meta data could be written in hcidump file name, for example
> foo_leagacy_hci0_12:34:56.log only when user wants.

I will not add this to btmon. I am fine have a --split option for btsnoop tool.

Regards

Marcel

[BlueZ RFC 3/3] monitor: Remove unused btsnoop files

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

---
 monitor/btsnoop.c | 430 ------------------------------------------------------
 monitor/btsnoop.h |  63 --------
 2 files changed, 493 deletions(-)
 delete mode 100644 monitor/btsnoop.c
 delete mode 100644 monitor/btsnoop.h

diff --git a/monitor/btsnoop.c b/monitor/btsnoop.c
deleted file mode 100644
index a32335a..0000000
--- a/monitor/btsnoop.c
+++ /dev/null
@@ -1,430 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2011-2014  Intel Corporation
- *  Copyright (C) 2002-2010  Marcel Holtmann <marcel@holtmann.org>
- *
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdio.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <stdint.h>
-#include <string.h>
-#include <sys/stat.h>
-#include <arpa/inet.h>
-
-#include "lib/bluetooth.h"
-#include "lib/hci.h"
-
-#include "btsnoop.h"
-
-struct btsnoop_hdr {
-	uint8_t		id[8];		/* Identification Pattern */
-	uint32_t	version;	/* Version Number = 1 */
-	uint32_t	type;		/* Datalink Type */
-} __attribute__ ((packed));
-#define BTSNOOP_HDR_SIZE (sizeof(struct btsnoop_hdr))
-
-struct btsnoop_pkt {
-	uint32_t	size;		/* Original Length */
-	uint32_t	len;		/* Included Length */
-	uint32_t	flags;		/* Packet Flags */
-	uint32_t	drops;		/* Cumulative Drops */
-	uint64_t	ts;		/* Timestamp microseconds */
-	uint8_t		data[0];	/* Packet Data */
-} __attribute__ ((packed));
-#define BTSNOOP_PKT_SIZE (sizeof(struct btsnoop_pkt))
-
-static const uint8_t btsnoop_id[] = { 0x62, 0x74, 0x73, 0x6e,
-				      0x6f, 0x6f, 0x70, 0x00 };
-
-static const uint32_t btsnoop_version = 1;
-static uint32_t btsnoop_type = 0;
-
-static int btsnoop_fd = -1;
-static uint16_t btsnoop_index = 0xffff;
-
-void btsnoop_create(const char *path, uint32_t type)
-{
-	struct btsnoop_hdr hdr;
-	ssize_t written;
-
-	if (btsnoop_fd >= 0)
-		return;
-
-	btsnoop_fd = open(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC,
-				S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-	if (btsnoop_fd < 0)
-		return;
-
-	btsnoop_type = type;
-
-	memcpy(hdr.id, btsnoop_id, sizeof(btsnoop_id));
-	hdr.version = htobe32(btsnoop_version);
-	hdr.type = htobe32(btsnoop_type);
-
-	written = write(btsnoop_fd, &hdr, BTSNOOP_HDR_SIZE);
-	if (written < 0) {
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return;
-	}
-}
-
-void btsnoop_write(struct timeval *tv, uint32_t flags,
-					const void *data, uint16_t size)
-{
-	struct btsnoop_pkt pkt;
-	uint64_t ts;
-	ssize_t written;
-
-	ts = (tv->tv_sec - 946684800ll) * 1000000ll + tv->tv_usec;
-
-	pkt.size  = htobe32(size);
-	pkt.len   = htobe32(size);
-	pkt.flags = htobe32(flags);
-	pkt.drops = htobe32(0);
-	pkt.ts    = htobe64(ts + 0x00E03AB44A676000ll);
-
-	written = write(btsnoop_fd, &pkt, BTSNOOP_PKT_SIZE);
-	if (written < 0)
-		return;
-
-	if (data && size > 0) {
-		written = write(btsnoop_fd, data, size);
-		if (written < 0)
-			return;
-	}
-}
-
-static uint32_t get_flags_from_opcode(uint16_t opcode)
-{
-	switch (opcode) {
-	case BTSNOOP_OPCODE_NEW_INDEX:
-	case BTSNOOP_OPCODE_DEL_INDEX:
-		break;
-	case BTSNOOP_OPCODE_COMMAND_PKT:
-		return 0x02;
-	case BTSNOOP_OPCODE_EVENT_PKT:
-		return 0x03;
-	case BTSNOOP_OPCODE_ACL_TX_PKT:
-		return 0x00;
-	case BTSNOOP_OPCODE_ACL_RX_PKT:
-		return 0x01;
-	case BTSNOOP_OPCODE_SCO_TX_PKT:
-	case BTSNOOP_OPCODE_SCO_RX_PKT:
-		break;
-	}
-
-	return 0xff;
-}
-
-void btsnoop_write_hci(struct timeval *tv, uint16_t index, uint16_t opcode,
-					const void *data, uint16_t size)
-{
-	uint32_t flags;
-
-	if (!tv)
-		return;
-
-	if (btsnoop_fd < 0)
-		return;
-
-	switch (btsnoop_type) {
-	case BTSNOOP_TYPE_HCI:
-		if (btsnoop_index == 0xffff)
-			btsnoop_index = index;
-
-		if (index != btsnoop_index)
-			return;
-
-		flags = get_flags_from_opcode(opcode);
-		if (flags == 0xff)
-			return;
-		break;
-
-	case BTSNOOP_TYPE_MONITOR:
-		flags = (index << 16) | opcode;
-		break;
-
-	default:
-		return;
-	}
-
-	btsnoop_write(tv, flags, data, size);
-}
-
-void btsnoop_write_phy(struct timeval *tv, uint16_t frequency,
-					const void *data, uint16_t size)
-{
-	uint32_t flags;
-
-	if (!tv)
-		return;
-
-	if (btsnoop_fd < 0)
-		return;
-
-	switch (btsnoop_type) {
-	case BTSNOOP_TYPE_SIMULATOR:
-		flags = (1 << 16) | frequency;
-		break;
-
-	default:
-		return;
-	}
-
-	btsnoop_write(tv, flags, data, size);
-}
-
-int btsnoop_open(const char *path, uint32_t *type)
-{
-	struct btsnoop_hdr hdr;
-	ssize_t len;
-
-	if (btsnoop_fd >= 0) {
-		fprintf(stderr, "Too many open files\n");
-		return -1;
-	}
-
-	btsnoop_fd = open(path, O_RDONLY | O_CLOEXEC);
-	if (btsnoop_fd < 0) {
-		perror("Failed to open file");
-		return -1;
-	}
-
-	len = read(btsnoop_fd, &hdr, BTSNOOP_HDR_SIZE);
-	if (len < 0 || len != BTSNOOP_HDR_SIZE) {
-		perror("Failed to read header");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	if (memcmp(hdr.id, btsnoop_id, sizeof(btsnoop_id))) {
-		fprintf(stderr, "Invalid btsnoop header\n");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	if (be32toh(hdr.version) != btsnoop_version) {
-		fprintf(stderr, "Invalid btsnoop version\n");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	btsnoop_type = be32toh(hdr.type);
-
-	if (type)
-		*type = btsnoop_type;
-
-	return 0;
-}
-
-static uint16_t get_opcode_from_flags(uint8_t type, uint32_t flags)
-{
-	switch (type) {
-	case HCI_COMMAND_PKT:
-		return BTSNOOP_OPCODE_COMMAND_PKT;
-	case HCI_ACLDATA_PKT:
-		if (flags & 0x01)
-			return BTSNOOP_OPCODE_ACL_RX_PKT;
-		else
-			return BTSNOOP_OPCODE_ACL_TX_PKT;
-	case HCI_SCODATA_PKT:
-		if (flags & 0x01)
-			return BTSNOOP_OPCODE_SCO_RX_PKT;
-		else
-			return BTSNOOP_OPCODE_SCO_TX_PKT;
-	case HCI_EVENT_PKT:
-		return BTSNOOP_OPCODE_EVENT_PKT;
-	case 0xff:
-		if (flags & 0x02) {
-			if (flags & 0x01)
-				return BTSNOOP_OPCODE_EVENT_PKT;
-			else
-				return BTSNOOP_OPCODE_COMMAND_PKT;
-		} else {
-			if (flags & 0x01)
-				return BTSNOOP_OPCODE_ACL_RX_PKT;
-			else
-				return BTSNOOP_OPCODE_ACL_TX_PKT;
-		}
-		break;
-	}
-
-	return 0xff;
-}
-
-int btsnoop_read_hci(struct timeval *tv, uint16_t *index, uint16_t *opcode,
-						void *data, uint16_t *size)
-{
-	struct btsnoop_pkt pkt;
-	uint32_t toread, flags;
-	uint64_t ts;
-	uint8_t pkt_type;
-	ssize_t len;
-
-	if (btsnoop_fd < 0)
-		return -1;
-
-	len = read(btsnoop_fd, &pkt, BTSNOOP_PKT_SIZE);
-	if (len == 0)
-		return -1;
-
-	if (len < 0 || len != BTSNOOP_PKT_SIZE) {
-		perror("Failed to read packet");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	toread = be32toh(pkt.size);
-	if (toread > BTSNOOP_MAX_PACKET_SIZE) {
-		perror("Packet len suspicially big: %u", toread);
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	flags = be32toh(pkt.flags);
-
-	ts = be64toh(pkt.ts) - 0x00E03AB44A676000ll;
-	tv->tv_sec = (ts / 1000000ll) + 946684800ll;
-	tv->tv_usec = ts % 1000000ll;
-
-	switch (btsnoop_type) {
-	case BTSNOOP_TYPE_HCI:
-		*index = 0;
-		*opcode = get_opcode_from_flags(0xff, flags);
-		break;
-
-	case BTSNOOP_TYPE_UART:
-		len = read(btsnoop_fd, &pkt_type, 1);
-		if (len < 0) {
-			perror("Failed to read packet type");
-			close(btsnoop_fd);
-			btsnoop_fd = -1;
-			return -1;
-		}
-		toread--;
-
-		*index = 0;
-		*opcode = get_opcode_from_flags(pkt_type, flags);
-		break;
-
-	case BTSNOOP_TYPE_MONITOR:
-		*index = flags >> 16;
-		*opcode = flags & 0xffff;
-		break;
-
-	default:
-		fprintf(stderr, "Unknown packet type\n");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	len = read(btsnoop_fd, data, toread);
-	if (len < 0) {
-		perror("Failed to read data");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	*size = toread;
-
-	return 0;
-}
-
-int btsnoop_read_phy(struct timeval *tv, uint16_t *frequency,
-						void *data, uint16_t *size)
-{
-	struct btsnoop_pkt pkt;
-	uint32_t toread, flags;
-	uint64_t ts;
-	ssize_t len;
-
-	if (btsnoop_fd < 0)
-		return -1;
-
-	len = read(btsnoop_fd, &pkt, BTSNOOP_PKT_SIZE);
-	if (len == 0)
-		return -1;
-
-	if (len < 0 || len != BTSNOOP_PKT_SIZE) {
-		perror("Failed to read packet");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	toread = be32toh(pkt.size);
-	flags = be32toh(pkt.flags);
-
-	ts = be64toh(pkt.ts) - 0x00E03AB44A676000ll;
-	tv->tv_sec = (ts / 1000000ll) + 946684800ll;
-	tv->tv_usec = ts % 1000000ll;
-
-	switch (btsnoop_type) {
-	case BTSNOOP_TYPE_SIMULATOR:
-		if ((flags >> 16) != 1)
-			break;
-		*frequency = flags & 0xffff;
-		break;
-
-	default:
-		fprintf(stderr, "Unknown packet type\n");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	len = read(btsnoop_fd, data, toread);
-	if (len < 0) {
-		perror("Failed to read data");
-		close(btsnoop_fd);
-		btsnoop_fd = -1;
-		return -1;
-	}
-
-	*size = toread;
-
-	return 0;
-}
-
-void btsnoop_close(void)
-{
-	if (btsnoop_fd < 0)
-		return;
-
-	close(btsnoop_fd);
-	btsnoop_fd = -1;
-
-	btsnoop_index = 0xffff;
-}
diff --git a/monitor/btsnoop.h b/monitor/btsnoop.h
deleted file mode 100644
index be4e2ed..0000000
--- a/monitor/btsnoop.h
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2011-2014  Intel Corporation
- *  Copyright (C) 2002-2010  Marcel Holtmann <marcel@holtmann.org>
- *
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#include <stdint.h>
-#include <sys/time.h>
-
-#define BTSNOOP_TYPE_HCI		1001
-#define BTSNOOP_TYPE_UART		1002
-#define BTSNOOP_TYPE_BCSP		1003
-#define BTSNOOP_TYPE_3WIRE		1004
-#define BTSNOOP_TYPE_MONITOR		2001
-#define BTSNOOP_TYPE_SIMULATOR		2002
-
-#define BTSNOOP_OPCODE_NEW_INDEX	0
-#define BTSNOOP_OPCODE_DEL_INDEX	1
-#define BTSNOOP_OPCODE_COMMAND_PKT	2
-#define BTSNOOP_OPCODE_EVENT_PKT	3
-#define BTSNOOP_OPCODE_ACL_TX_PKT	4
-#define BTSNOOP_OPCODE_ACL_RX_PKT	5
-#define BTSNOOP_OPCODE_SCO_TX_PKT	6
-#define BTSNOOP_OPCODE_SCO_RX_PKT	7
-
-struct btsnoop_opcode_new_index {
-	uint8_t  type;
-	uint8_t  bus;
-	uint8_t  bdaddr[6];
-	char     name[8];
-} __attribute__((packed));
-
-void btsnoop_create(const char *path, uint32_t type);
-void btsnoop_write(struct timeval *tv, uint32_t flags,
-					const void *data, uint16_t size);
-void btsnoop_write_hci(struct timeval *tv, uint16_t index, uint16_t opcode,
-					const void *data, uint16_t size);
-void btsnoop_write_phy(struct timeval *tv, uint16_t frequency,
-					const void *data, uint16_t size);
-int btsnoop_open(const char *path, uint32_t *type);
-int btsnoop_read_hci(struct timeval *tv, uint16_t *index, uint16_t *opcode,
-						void *data, uint16_t *size);
-int btsnoop_read_phy(struct timeval *tv, uint16_t *frequency,
-						void *data, uint16_t *size);
-void btsnoop_close(void);
-- 
2.1.0

Re: [BlueZ RFC 1/3] monitor: Add btsnoop data link transfer way

[Szymon Janc]

Hi Chan-yeol Park,

On Thursday 23 of April 2015 03:49:23 chanyeol.park@gmail.com wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
> 
> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR:2001.
> but some of btsnoop viewer could not handle it because they just
> support BTSNOOP_TYPE_HCI(1001).
> 
> So they need transform way to analyze the btsnoop file captured by
> btmon.
> ---
>  monitor/control.c | 61
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ monitor/control.h |
>  1 +
>  monitor/main.c    | 13 +++++++++++-
>  3 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor/control.c b/monitor/control.c
> index e61a79d..00e4bc0 100644
> --- a/monitor/control.c
> +++ b/monitor/control.c
> @@ -1131,6 +1131,67 @@ bool control_writer(const char *path)
>  	return !!btsnoop_file;
>  }
> 
> +void transfer_btsnoop_data_link(const char *path, const char *writer_path)
> +{
> +	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
> +	uint16_t pktlen;
> +	uint32_t type;
> +	struct timeval tv;
> +	struct btsnoop *btsnoop_write_file = NULL;
> +	unsigned long num_packets = 0;
> +
> +	btsnoop_file = btsnoop_open(path, BTSNOOP_FLAG_PKLG_SUPPORT);
> +	if (!btsnoop_file)
> +		return;
> +
> +	btsnoop_write_file = btsnoop_create(writer_path, BTSNOOP_TYPE_HCI);
> +	if (!btsnoop_write_file)
> +		return;
> +
> +	type = btsnoop_get_type(btsnoop_file);
> +
> +	switch (type) {
> +	case BTSNOOP_TYPE_HCI:
> +	case BTSNOOP_TYPE_UART:
> +	case BTSNOOP_TYPE_SIMULATOR:
> +		packet_del_filter(PACKET_FILTER_SHOW_INDEX);
> +		break;
> +
> +	case BTSNOOP_TYPE_MONITOR:
> +		packet_add_filter(PACKET_FILTER_SHOW_INDEX);
> +		break;
> +	}
> +
> +	switch (type) {
> +	case BTSNOOP_TYPE_HCI:
> +	case BTSNOOP_TYPE_UART:
> +	case BTSNOOP_TYPE_MONITOR:
> +		while (1) {
> +			uint16_t index, opcode;
> +
> +			if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
> +							&opcode, buf, &pktlen))
> +				break;
> +
> +			if (opcode == 0xffff)
> +				continue;
> +
> +			btsnoop_write_hci(btsnoop_write_file, &tv, index,
> +							opcode, buf, pktlen);
> +			num_packets++;
> +		}
> +		break;
> +	}
> +
> +	btsnoop_unref(btsnoop_file);
> +	btsnoop_unref(btsnoop_write_file);
> +
> +	printf("BT Snoop data link transfer is completed for %lu packets\n",
> +								num_packets);
> +	printf("Output is saved in %s\n", writer_path);
> +}
> +
> +
>  void control_reader(const char *path)
>  {
>  	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
> diff --git a/monitor/control.h b/monitor/control.h
> index 28f16db..267d71b 100644
> --- a/monitor/control.h
> +++ b/monitor/control.h
> @@ -28,5 +28,6 @@ bool control_writer(const char *path);
>  void control_reader(const char *path);
>  void control_server(const char *path);
>  int control_tracing(void);
> +void transfer_btsnoop_data_link(const char *path, const char *writer_path);
> 
>  void control_message(uint16_t opcode, const void *data, uint16_t size);
> diff --git a/monitor/main.c b/monitor/main.c
> index de48db5..6e7d4b3 100644
> --- a/monitor/main.c
> +++ b/monitor/main.c
> @@ -59,6 +59,7 @@ static void usage(void)
>  	printf("options:\n"
>  		"\t-r, --read <file>      Read traces in btsnoop format\n"
>  		"\t-w, --write <file>     Save traces in btsnoop format\n"
> +		"\t-f, --transfer <file>  Transfer btsnoop data link type\n"
>  		"\t-a, --analyze <file>   Analyze traces in btsnoop format\n"
>  		"\t-s, --server <socket>  Start monitor server socket\n"
>  		"\t-i, --index <num>      Show only specified controller\n"
> @@ -72,6 +73,7 @@ static void usage(void)
>  static const struct option main_options[] = {
>  	{ "read",    required_argument, NULL, 'r' },
>  	{ "write",   required_argument, NULL, 'w' },
> +	{ "transfer", required_argument, NULL, 'f' },
>  	{ "analyze", required_argument, NULL, 'a' },
>  	{ "server",  required_argument, NULL, 's' },
>  	{ "index",   required_argument, NULL, 'i' },
> @@ -104,7 +106,7 @@ int main(int argc, char *argv[])
>  	for (;;) {
>  		int opt;
> 
> -		opt = getopt_long(argc, argv, "r:w:a:s:i:tTSE:vh",
> +		opt = getopt_long(argc, argv, "r:w:f:a:s:i:tTSE:vh",
>  						main_options, NULL);
>  		if (opt < 0)
>  			break;
> @@ -116,6 +118,10 @@ int main(int argc, char *argv[])
>  		case 'w':
>  			writer_path = optarg;
>  			break;
> +		case 'f':
> +			reader_path = optarg;
> +			writer_path = "btsnoop_type_hci.log";
> +			break;
>  		case 'a':
>  			analyze_path = optarg;
>  			break;
> @@ -191,6 +197,11 @@ int main(int argc, char *argv[])
>  		return EXIT_SUCCESS;
>  	}
> 
> +	if (reader_path && writer_path) {
> +		transfer_btsnoop_data_link(reader_path, writer_path);
> +		return EXIT_SUCCESS;
> +	}
> +
>  	if (reader_path) {
>  		if (ellisys_server)
>  			ellisys_enable(ellisys_server, ellisys_port);

Those patches will make problems with multiple adapters.
So if you really want to put this into btmon --transfer option should create 1 
file per adapter eg. btmon --trasfer foo.btsnoop would result in multiple 
foo_legacy_hciX.btsnoop files (or whatever it would be called).


Personally I wouldn't put this into monitor but maybe provide additional 
option to btsnoop tool for splitting into legacy files.

-- 
BR
Szymon Janc

Re: [BlueZ RFC 1/3] monitor: Add btsnoop data link transfer way

[Marcel Holtmann]

Hi Szymon,

>> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR:2001.
>> but some of btsnoop viewer could not handle it because they just
>> support BTSNOOP_TYPE_HCI(1001).
>> 
>> So they need transform way to analyze the btsnoop file captured by
>> btmon.
>> ---
>> monitor/control.c | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ monitor/control.h |
>> 1 +
>> monitor/main.c    | 13 +++++++++++-
>> 3 files changed, 74 insertions(+), 1 deletion(-)
>> 
>> diff --git a/monitor/control.c b/monitor/control.c
>> index e61a79d..00e4bc0 100644
>> --- a/monitor/control.c
>> +++ b/monitor/control.c
>> @@ -1131,6 +1131,67 @@ bool control_writer(const char *path)
>> 	return !!btsnoop_file;
>> }
>> 
>> +void transfer_btsnoop_data_link(const char *path, const char *writer_path)
>> +{
>> +	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
>> +	uint16_t pktlen;
>> +	uint32_t type;
>> +	struct timeval tv;
>> +	struct btsnoop *btsnoop_write_file = NULL;
>> +	unsigned long num_packets = 0;
>> +
>> +	btsnoop_file = btsnoop_open(path, BTSNOOP_FLAG_PKLG_SUPPORT);
>> +	if (!btsnoop_file)
>> +		return;
>> +
>> +	btsnoop_write_file = btsnoop_create(writer_path, BTSNOOP_TYPE_HCI);
>> +	if (!btsnoop_write_file)
>> +		return;
>> +
>> +	type = btsnoop_get_type(btsnoop_file);
>> +
>> +	switch (type) {
>> +	case BTSNOOP_TYPE_HCI:
>> +	case BTSNOOP_TYPE_UART:
>> +	case BTSNOOP_TYPE_SIMULATOR:
>> +		packet_del_filter(PACKET_FILTER_SHOW_INDEX);
>> +		break;
>> +
>> +	case BTSNOOP_TYPE_MONITOR:
>> +		packet_add_filter(PACKET_FILTER_SHOW_INDEX);
>> +		break;
>> +	}
>> +
>> +	switch (type) {
>> +	case BTSNOOP_TYPE_HCI:
>> +	case BTSNOOP_TYPE_UART:
>> +	case BTSNOOP_TYPE_MONITOR:
>> +		while (1) {
>> +			uint16_t index, opcode;
>> +
>> +			if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
>> +							&opcode, buf, &pktlen))
>> +				break;
>> +
>> +			if (opcode == 0xffff)
>> +				continue;
>> +
>> +			btsnoop_write_hci(btsnoop_write_file, &tv, index,
>> +							opcode, buf, pktlen);
>> +			num_packets++;
>> +		}
>> +		break;
>> +	}
>> +
>> +	btsnoop_unref(btsnoop_file);
>> +	btsnoop_unref(btsnoop_write_file);
>> +
>> +	printf("BT Snoop data link transfer is completed for %lu packets\n",
>> +								num_packets);
>> +	printf("Output is saved in %s\n", writer_path);
>> +}
>> +
>> +
>> void control_reader(const char *path)
>> {
>> 	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
>> diff --git a/monitor/control.h b/monitor/control.h
>> index 28f16db..267d71b 100644
>> --- a/monitor/control.h
>> +++ b/monitor/control.h
>> @@ -28,5 +28,6 @@ bool control_writer(const char *path);
>> void control_reader(const char *path);
>> void control_server(const char *path);
>> int control_tracing(void);
>> +void transfer_btsnoop_data_link(const char *path, const char *writer_path);
>> 
>> void control_message(uint16_t opcode, const void *data, uint16_t size);
>> diff --git a/monitor/main.c b/monitor/main.c
>> index de48db5..6e7d4b3 100644
>> --- a/monitor/main.c
>> +++ b/monitor/main.c
>> @@ -59,6 +59,7 @@ static void usage(void)
>> 	printf("options:\n"
>> 		"\t-r, --read <file>      Read traces in btsnoop format\n"
>> 		"\t-w, --write <file>     Save traces in btsnoop format\n"
>> +		"\t-f, --transfer <file>  Transfer btsnoop data link type\n"
>> 		"\t-a, --analyze <file>   Analyze traces in btsnoop format\n"
>> 		"\t-s, --server <socket>  Start monitor server socket\n"
>> 		"\t-i, --index <num>      Show only specified controller\n"
>> @@ -72,6 +73,7 @@ static void usage(void)
>> static const struct option main_options[] = {
>> 	{ "read",    required_argument, NULL, 'r' },
>> 	{ "write",   required_argument, NULL, 'w' },
>> +	{ "transfer", required_argument, NULL, 'f' },
>> 	{ "analyze", required_argument, NULL, 'a' },
>> 	{ "server",  required_argument, NULL, 's' },
>> 	{ "index",   required_argument, NULL, 'i' },
>> @@ -104,7 +106,7 @@ int main(int argc, char *argv[])
>> 	for (;;) {
>> 		int opt;
>> 
>> -		opt = getopt_long(argc, argv, "r:w:a:s:i:tTSE:vh",
>> +		opt = getopt_long(argc, argv, "r:w:f:a:s:i:tTSE:vh",
>> 						main_options, NULL);
>> 		if (opt < 0)
>> 			break;
>> @@ -116,6 +118,10 @@ int main(int argc, char *argv[])
>> 		case 'w':
>> 			writer_path = optarg;
>> 			break;
>> +		case 'f':
>> +			reader_path = optarg;
>> +			writer_path = "btsnoop_type_hci.log";
>> +			break;
>> 		case 'a':
>> 			analyze_path = optarg;
>> 			break;
>> @@ -191,6 +197,11 @@ int main(int argc, char *argv[])
>> 		return EXIT_SUCCESS;
>> 	}
>> 
>> +	if (reader_path && writer_path) {
>> +		transfer_btsnoop_data_link(reader_path, writer_path);
>> +		return EXIT_SUCCESS;
>> +	}
>> +
>> 	if (reader_path) {
>> 		if (ellisys_server)
>> 			ellisys_enable(ellisys_server, ellisys_port);
> 
> Those patches will make problems with multiple adapters.
> So if you really want to put this into btmon --transfer option should create 1 
> file per adapter eg. btmon --trasfer foo.btsnoop would result in multiple 
> foo_legacy_hciX.btsnoop files (or whatever it would be called).
> 
> 
> Personally I wouldn't put this into monitor but maybe provide additional 
> option to btsnoop tool for splitting into legacy files.

that is what I propose as well.

Regards

Marcel

Re: [BlueZ RFC 1/3] monitor: Add btsnoop data link transfer way

[Chanyeol Park]

Hi Marcel,

On Thu, 2015-04-23 at 09:52 -0700, Marcel Holtmann wrote:
> Hi Szymon,
> 
> >> Current btmon's btsnoop data link type is BTSNOOP_TYPE_MONITOR:2001.
> >> but some of btsnoop viewer could not handle it because they just
> >> support BTSNOOP_TYPE_HCI(1001).
> >> 
> >> So they need transform way to analyze the btsnoop file captured by
> >> btmon.
> >> ---
> >> monitor/control.c | 61
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ monitor/control.h |
> >> 1 +
> >> monitor/main.c    | 13 +++++++++++-
> >> 3 files changed, 74 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/monitor/control.c b/monitor/control.c
> >> index e61a79d..00e4bc0 100644
> >> --- a/monitor/control.c
> >> +++ b/monitor/control.c
> >> @@ -1131,6 +1131,67 @@ bool control_writer(const char *path)
> >> 	return !!btsnoop_file;
> >> }
> >> 
> >> +void transfer_btsnoop_data_link(const char *path, const char *writer_path)
> >> +{
> >> +	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
> >> +	uint16_t pktlen;
> >> +	uint32_t type;
> >> +	struct timeval tv;
> >> +	struct btsnoop *btsnoop_write_file = NULL;
> >> +	unsigned long num_packets = 0;
> >> +
> >> +	btsnoop_file = btsnoop_open(path, BTSNOOP_FLAG_PKLG_SUPPORT);
> >> +	if (!btsnoop_file)
> >> +		return;
> >> +
> >> +	btsnoop_write_file = btsnoop_create(writer_path, BTSNOOP_TYPE_HCI);
> >> +	if (!btsnoop_write_file)
> >> +		return;
> >> +
> >> +	type = btsnoop_get_type(btsnoop_file);
> >> +
> >> +	switch (type) {
> >> +	case BTSNOOP_TYPE_HCI:
> >> +	case BTSNOOP_TYPE_UART:
> >> +	case BTSNOOP_TYPE_SIMULATOR:
> >> +		packet_del_filter(PACKET_FILTER_SHOW_INDEX);
> >> +		break;
> >> +
> >> +	case BTSNOOP_TYPE_MONITOR:
> >> +		packet_add_filter(PACKET_FILTER_SHOW_INDEX);
> >> +		break;
> >> +	}
> >> +
> >> +	switch (type) {
> >> +	case BTSNOOP_TYPE_HCI:
> >> +	case BTSNOOP_TYPE_UART:
> >> +	case BTSNOOP_TYPE_MONITOR:
> >> +		while (1) {
> >> +			uint16_t index, opcode;
> >> +
> >> +			if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
> >> +							&opcode, buf, &pktlen))
> >> +				break;
> >> +
> >> +			if (opcode == 0xffff)
> >> +				continue;
> >> +
> >> +			btsnoop_write_hci(btsnoop_write_file, &tv, index,
> >> +							opcode, buf, pktlen);
> >> +			num_packets++;
> >> +		}
> >> +		break;
> >> +	}
> >> +
> >> +	btsnoop_unref(btsnoop_file);
> >> +	btsnoop_unref(btsnoop_write_file);
> >> +
> >> +	printf("BT Snoop data link transfer is completed for %lu packets\n",
> >> +								num_packets);
> >> +	printf("Output is saved in %s\n", writer_path);
> >> +}
> >> +
> >> +
> >> void control_reader(const char *path)
> >> {
> >> 	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
> >> diff --git a/monitor/control.h b/monitor/control.h
> >> index 28f16db..267d71b 100644
> >> --- a/monitor/control.h
> >> +++ b/monitor/control.h
> >> @@ -28,5 +28,6 @@ bool control_writer(const char *path);
> >> void control_reader(const char *path);
> >> void control_server(const char *path);
> >> int control_tracing(void);
> >> +void transfer_btsnoop_data_link(const char *path, const char *writer_path);
> >> 
> >> void control_message(uint16_t opcode, const void *data, uint16_t size);
> >> diff --git a/monitor/main.c b/monitor/main.c
> >> index de48db5..6e7d4b3 100644
> >> --- a/monitor/main.c
> >> +++ b/monitor/main.c
> >> @@ -59,6 +59,7 @@ static void usage(void)
> >> 	printf("options:\n"
> >> 		"\t-r, --read <file>      Read traces in btsnoop format\n"
> >> 		"\t-w, --write <file>     Save traces in btsnoop format\n"
> >> +		"\t-f, --transfer <file>  Transfer btsnoop data link type\n"
> >> 		"\t-a, --analyze <file>   Analyze traces in btsnoop format\n"
> >> 		"\t-s, --server <socket>  Start monitor server socket\n"
> >> 		"\t-i, --index <num>      Show only specified controller\n"
> >> @@ -72,6 +73,7 @@ static void usage(void)
> >> static const struct option main_options[] = {
> >> 	{ "read",    required_argument, NULL, 'r' },
> >> 	{ "write",   required_argument, NULL, 'w' },
> >> +	{ "transfer", required_argument, NULL, 'f' },
> >> 	{ "analyze", required_argument, NULL, 'a' },
> >> 	{ "server",  required_argument, NULL, 's' },
> >> 	{ "index",   required_argument, NULL, 'i' },
> >> @@ -104,7 +106,7 @@ int main(int argc, char *argv[])
> >> 	for (;;) {
> >> 		int opt;
> >> 
> >> -		opt = getopt_long(argc, argv, "r:w:a:s:i:tTSE:vh",
> >> +		opt = getopt_long(argc, argv, "r:w:f:a:s:i:tTSE:vh",
> >> 						main_options, NULL);
> >> 		if (opt < 0)
> >> 			break;
> >> @@ -116,6 +118,10 @@ int main(int argc, char *argv[])
> >> 		case 'w':
> >> 			writer_path = optarg;
> >> 			break;
> >> +		case 'f':
> >> +			reader_path = optarg;
> >> +			writer_path = "btsnoop_type_hci.log";
> >> +			break;
> >> 		case 'a':
> >> 			analyze_path = optarg;
> >> 			break;
> >> @@ -191,6 +197,11 @@ int main(int argc, char *argv[])
> >> 		return EXIT_SUCCESS;
> >> 	}
> >> 
> >> +	if (reader_path && writer_path) {
> >> +		transfer_btsnoop_data_link(reader_path, writer_path);
> >> +		return EXIT_SUCCESS;
> >> +	}
> >> +
> >> 	if (reader_path) {
> >> 		if (ellisys_server)
> >> 			ellisys_enable(ellisys_server, ellisys_port);
> > 
> > Those patches will make problems with multiple adapters.
> > So if you really want to put this into btmon --transfer option should create 1 
> > file per adapter eg. btmon --trasfer foo.btsnoop would result in multiple 
> > foo_legacy_hciX.btsnoop files (or whatever it would be called).
> > 
> > 
> > Personally I wouldn't put this into monitor but maybe provide additional 
> > option to btsnoop tool for splitting into legacy files.
> 
> that is what I propose as well.

Based on your proposal ,I would raise the patch. 

However if we have duplicated index, the previous file would be removed
automatically. For example, if we plug/unplug BT adapter repeatedly,
only last file would be saved. So I add time info in file name to keep
the previous files.

If we use this well, I think we could use old btsnoop format for special
case because user could recognize the events(adapter added, removed) by
file name.

Regards
Chanyeol