[PATCH] monitor: Add option to disable SCO packets

[PATCH] monitor: Add option to disable SCO packets

[João Paulo Rechi Vita]

It is difficult to follow anything else happening when a SCO connection
is active, as the log gets floded with SCO packets. This adds an option
to disable dumping SCO packets.
---
 monitor/main.c   | 9 +++++++--
 monitor/packet.c | 6 ++++--
 monitor/packet.h | 1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/monitor/main.c b/monitor/main.c
index 5fa87ea3f..6a8618c41 100644
--- a/monitor/main.c
+++ b/monitor/main.c
@@ -70,7 +70,8 @@ static void usage(void)
 		"\t-V, --vendor <compid>  Set default company identifier\n"
 		"\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-N, --no-sco-packets   Do not dump SCO packets\n"
+		"\t-S, --sco              Dump SCO traffic (without -N)\n"
 		"\t-A, --a2dp             Dump A2DP stream traffic\n"
 		"\t-E, --ellisys [ip]     Send Ellisys HCI Injection\n"
 		"\t-P, --no-pager         Disable pager usage\n"
@@ -89,6 +90,7 @@ static const struct option main_options[] = {
 	{ "vendor",    required_argument, NULL, 'V' },
 	{ "time",      no_argument,       NULL, 't' },
 	{ "date",      no_argument,       NULL, 'T' },
+	{ "no-sco",    no_argument,       NULL, 'N' },
 	{ "sco",       no_argument,       NULL, 'S' },
 	{ "a2dp",      no_argument,       NULL, 'A' },
 	{ "ellisys",   required_argument, NULL, 'E' },
@@ -122,7 +124,7 @@ int main(int argc, char *argv[])
 		int opt;
 		struct sockaddr_un addr;
 
-		opt = getopt_long(argc, argv, "r:w:a:s:p:i:d:B:V:tTSAEPvh",
+		opt = getopt_long(argc, argv, "r:w:a:s:p:i:d:B:V:tTNSAEPvh",
 							main_options, NULL);
 		if (opt < 0)
 			break;
@@ -181,6 +183,9 @@ int main(int argc, char *argv[])
 			filter_mask |= PACKET_FILTER_SHOW_TIME;
 			filter_mask |= PACKET_FILTER_SHOW_DATE;
 			break;
+		case 'N':
+			filter_mask |= PACKET_FILTER_NO_SCO_PACKET;
+			break;
 		case 'S':
 			filter_mask |= PACKET_FILTER_SHOW_SCO_DATA;
 			break;
diff --git a/monitor/packet.c b/monitor/packet.c
index 7705d2ed5..9972b3044 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -4047,10 +4047,12 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
 		packet_hci_acldata(tv, cred, index, true, data, size);
 		break;
 	case BTSNOOP_OPCODE_SCO_TX_PKT:
-		packet_hci_scodata(tv, cred, index, false, data, size);
+		if (!(filter_mask & PACKET_FILTER_NO_SCO_PACKET))
+			packet_hci_scodata(tv, cred, index, false, data, size);
 		break;
 	case BTSNOOP_OPCODE_SCO_RX_PKT:
-		packet_hci_scodata(tv, cred, index, true, data, size);
+		if (!(filter_mask & PACKET_FILTER_NO_SCO_PACKET))
+			packet_hci_scodata(tv, cred, index, true, data, size);
 		break;
 	case BTSNOOP_OPCODE_OPEN_INDEX:
 		if (index < MAX_INDEX)
diff --git a/monitor/packet.h b/monitor/packet.h
index 03279e114..b35708e83 100644
--- a/monitor/packet.h
+++ b/monitor/packet.h
@@ -34,6 +34,7 @@
 #define PACKET_FILTER_SHOW_ACL_DATA	(1 << 4)
 #define PACKET_FILTER_SHOW_SCO_DATA	(1 << 5)
 #define PACKET_FILTER_SHOW_A2DP_STREAM	(1 << 6)
+#define PACKET_FILTER_NO_SCO_PACKET	(1 << 7)
 
 bool packet_has_filter(unsigned long filter);
 void packet_set_filter(unsigned long filter);
-- 
2.17.1

Re: [PATCH] monitor: Add option to disable SCO packets

[Marcel Holtmann]

Hi Joao Paulo,

> It is difficult to follow anything else happening when a SCO connection
> is active, as the log gets floded with SCO packets. This adds an option
> to disable dumping SCO packets.
> ---
> monitor/main.c   | 9 +++++++--
> monitor/packet.c | 6 ++++--
> monitor/packet.h | 1 +
> 3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/monitor/main.c b/monitor/main.c
> index 5fa87ea3f..6a8618c41 100644
> --- a/monitor/main.c
> +++ b/monitor/main.c
> @@ -70,7 +70,8 @@ static void usage(void)
> 		"\t-V, --vendor <compid>  Set default company identifier\n"
> 		"\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-N, --no-sco-packets   Do not dump SCO packets\n"
> +		"\t-S, --sco              Dump SCO traffic (without -N)\n”

I am confused. I assumed I made SCO off by default.

Anyhow, I don’t want to see tons of independent options. Since next thing is you tell me that AVDTP media channel is also something you want to filter out. So then we better find a generic way to apply filters and also notifications about “dropped x packets due to filter” kinda hints.

Regards

Marcel

Re: [PATCH] monitor: Add option to disable SCO packets

[João Paulo Rechi Vita]

Hello Marcel, thanks for the quick reply.

On Mon, Jun 18, 2018 at 10:50 AM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>
> I am confused. I assumed I made SCO off by default.
>

Dumping the contents for SCO packets is OFF by default (enabled by
-S), that is, the hex dump part on the output bellow:

  SCO Data RX: Handle 257 flags 0x00 dlen 48

                                                               #3537
[hci0] 16.106123
        23 00 2b 00 20 00 06 00 0f 00 21 00 15 00 05 00  #.+. .....!.....
        1d 00 13 00 ff ff 15 00 1b 00 f8 ff f1 ff 0f 00  ................
        0f 00 fc ff f3 ff f8 ff f8 ff fa ff 02 00 00 00  ................

But without -S we still log the header / flags part for each packet:

  SCO Data RX: Handle 258 flags 0x00 dlen 48

                                                               #3541
[hci0] 14.128754

> Anyhow, I don=E2=80=99t want to see tons of independent options. Since ne=
xt thing is you tell me that AVDTP media channel is also something you want=
 to filter out. So then we better find a generic way to apply filters and a=
lso notifications about =E2=80=9Cdropped x packets due to filter=E2=80=9D k=
inda hints.
>

My argument behind filtering out SCO is because AFAIU it is only data,
not signaling. On a quick look I had assumed there was no easy way to
separate AVDTP signaling from data, but on a second look is actually
quite obvious, so you are right, that would be an obvious next step
:-)

Maybe we could hide the SCO and AVDTP media channels by default? In
this case we could print a notification when the program starts that
these channels are being suppressed, and / or every time the channel
is established. That would be reflected on the package count printed
with the timestamp, and I'm not sure if that would be a problem: is
that number reflecting how many packets have been exchanged or
printed?

As much as I like the idea of a generic configurable filter, I can't
look into that atm.

Regards,

--
Jo=C3=A3o Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH] monitor: Add option to disable SCO packets

[Luiz Augusto von Dentz]

Hi Joao,

On Mon, Jun 18, 2018 at 9:57 PM, Jo=C3=A3o Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> Hello Marcel, thanks for the quick reply.
>
> On Mon, Jun 18, 2018 at 10:50 AM, Marcel Holtmann <marcel@holtmann.org> w=
rote:
>>
>> I am confused. I assumed I made SCO off by default.
>>
>
> Dumping the contents for SCO packets is OFF by default (enabled by
> -S), that is, the hex dump part on the output bellow:
>
>   SCO Data RX: Handle 257 flags 0x00 dlen 48
>
>                                                                #3537
> [hci0] 16.106123
>         23 00 2b 00 20 00 06 00 0f 00 21 00 15 00 05 00  #.+. .....!.....
>         1d 00 13 00 ff ff 15 00 1b 00 f8 ff f1 ff 0f 00  ................
>         0f 00 fc ff f3 ff f8 ff f8 ff fa ff 02 00 00 00  ................
>
> But without -S we still log the header / flags part for each packet:
>
>   SCO Data RX: Handle 258 flags 0x00 dlen 48
>
>                                                                #3541
> [hci0] 14.128754
>
>> Anyhow, I don=E2=80=99t want to see tons of independent options. Since n=
ext thing is you tell me that AVDTP media channel is also something you wan=
t to filter out. So then we better find a generic way to apply filters and =
also notifications about =E2=80=9Cdropped x packets due to filter=E2=80=9D =
kinda hints.
>>

We might look at doing this kind of filtering using BPF instead which
perhaps could be loaded from a file.

> My argument behind filtering out SCO is because AFAIU it is only data,
> not signaling. On a quick look I had assumed there was no easy way to
> separate AVDTP signaling from data, but on a second look is actually
> quite obvious, so you are right, that would be an obvious next step
> :-)
>
> Maybe we could hide the SCO and AVDTP media channels by default? In
> this case we could print a notification when the program starts that
> these channels are being suppressed, and / or every time the channel
> is established. That would be reflected on the package count printed
> with the timestamp, and I'm not sure if that would be a problem: is
> that number reflecting how many packets have been exchanged or
> printed?
>
> As much as I like the idea of a generic configurable filter, I can't
> look into that atm.

BPF already works with Bluetooth socket so perhaps we just need a way
to load the filter from file.

@Marcel: We never applied the patch below, are you okay with it:

https://www.spinics.net/lists/linux-bluetooth/msg71245.html

> Regards,
>
> --
> Jo=C3=A3o Paulo Rechi Vita
> http://about.me/jprvita
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--=20
Luiz Augusto von Dentz

Re: [PATCH] monitor: Add option to disable SCO packets

[Marcel Holtmann]

Hi Luiz,

>>> I am confused. I assumed I made SCO off by default.
>>> 
>> 
>> Dumping the contents for SCO packets is OFF by default (enabled by
>> -S), that is, the hex dump part on the output bellow:
>> 
>>  SCO Data RX: Handle 257 flags 0x00 dlen 48
>> 
>>                                                               #3537
>> [hci0] 16.106123
>>        23 00 2b 00 20 00 06 00 0f 00 21 00 15 00 05 00  #.+. .....!.....
>>        1d 00 13 00 ff ff 15 00 1b 00 f8 ff f1 ff 0f 00  ................
>>        0f 00 fc ff f3 ff f8 ff f8 ff fa ff 02 00 00 00  ................
>> 
>> But without -S we still log the header / flags part for each packet:
>> 
>>  SCO Data RX: Handle 258 flags 0x00 dlen 48
>> 
>>                                                               #3541
>> [hci0] 14.128754
>> 
>>> Anyhow, I don’t want to see tons of independent options. Since next thing is you tell me that AVDTP media channel is also something you want to filter out. So then we better find a generic way to apply filters and also notifications about “dropped x packets due to filter” kinda hints.
>>> 
> 
> We might look at doing this kind of filtering using BPF instead which
> perhaps could be loaded from a file.

doing this BPF would be best.

>> My argument behind filtering out SCO is because AFAIU it is only data,
>> not signaling. On a quick look I had assumed there was no easy way to
>> separate AVDTP signaling from data, but on a second look is actually
>> quite obvious, so you are right, that would be an obvious next step
>> :-)
>> 
>> Maybe we could hide the SCO and AVDTP media channels by default? In
>> this case we could print a notification when the program starts that
>> these channels are being suppressed, and / or every time the channel
>> is established. That would be reflected on the package count printed
>> with the timestamp, and I'm not sure if that would be a problem: is
>> that number reflecting how many packets have been exchanged or
>> printed?
>> 
>> As much as I like the idea of a generic configurable filter, I can't
>> look into that atm.
> 
> BPF already works with Bluetooth socket so perhaps we just need a way
> to load the filter from file.
> 
> @Marcel: We never applied the patch below, are you okay with it:
> 
> https://www.spinics.net/lists/linux-bluetooth/msg71245.html

I somehow never saw that patch. Can you re-send it to the mailing list.

Regards

Marcel