[PATCH 1/2] doc/btsnoop: Add nop opcode

[PATCH 1/2] doc/btsnoop: Add nop opcode

[Andrzej Kaczmarek]

This adds 'no operation' opcode for the cases where we do not want to
include any particular payload, just the header is valid.

For example, when some packets are dropped over TTY implementation can
pass this information only in some other packet and this won't happen
until there's actually something to send. With this addition it can
just send nop after some time to indicate there were packets dropped.
---
 doc/btsnoop.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/btsnoop.txt b/doc/btsnoop.txt
index 975a53f6d..865b81554 100644
--- a/doc/btsnoop.txt
+++ b/doc/btsnoop.txt
@@ -115,6 +115,13 @@ User Logging
 
 	User logging information.
 
+NOP
+-----------
+
+	Code:        0xffff
+
+	No operation.
+
 
 TTY-based protocol
 ==================
-- 
2.14.1

[PATCH 2/2] monitor: Add handling of nop opcode

[Andrzej Kaczmarek]

---
 monitor/packet.c     | 2 ++
 src/shared/btsnoop.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index f214790ac..3145b075b 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -4110,6 +4110,8 @@ void packet_monitor(struct timeval *tv, struct ucred *cred,
 	case BTSNOOP_OPCODE_CTRL_EVENT:
 		packet_ctrl_event(tv, cred, index, data, size);
 		break;
+	case BTSNOOP_OPCODE_NOP:
+		break;
 	default:
 		sprintf(extra_str, "(code %d len %d)", opcode, size);
 		print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
diff --git a/src/shared/btsnoop.h b/src/shared/btsnoop.h
index 3df8998a3..0038117ad 100644
--- a/src/shared/btsnoop.h
+++ b/src/shared/btsnoop.h
@@ -53,6 +53,7 @@
 #define BTSNOOP_OPCODE_CTRL_CLOSE	15
 #define BTSNOOP_OPCODE_CTRL_COMMAND	16
 #define BTSNOOP_OPCODE_CTRL_EVENT	17
+#define BTSNOOP_OPCODE_NOP		65535
 
 #define BTSNOOP_MAX_PACKET_SIZE		(1486 + 4)
 
-- 
2.14.1

Re: [PATCH 1/2] doc/btsnoop: Add nop opcode

[Marcel Holtmann]

Hi Andrzej,

> This adds 'no operation' opcode for the cases where we do not want to
> include any particular payload, just the header is valid.
> 
> For example, when some packets are dropped over TTY implementation can
> pass this information only in some other packet and this won't happen
> until there's actually something to send. With this addition it can
> just send nop after some time to indicate there were packets dropped.
> ---
> doc/btsnoop.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/doc/btsnoop.txt b/doc/btsnoop.txt
> index 975a53f6d..865b81554 100644
> --- a/doc/btsnoop.txt
> +++ b/doc/btsnoop.txt
> @@ -115,6 +115,13 @@ User Logging
> 
> 	User logging information.
> 
> +NOP
> +-----------
> +
> +	Code:        0xffff
> +
> +	No operation.
> +

Hmmm. I am not a huge fan of this. Then I see that we defined the ext_hdr that allows to deal with the dropped packets.

I need to think about this a bit since there are some other opcode extensions we have to do for certain cases where we have unexpected packets. Right now I would propose to use vendor diagnostic or system note with empty payload.

Regards

Marcel

Re: [PATCH 1/2] doc/btsnoop: Add nop opcode

[Andrzej Kaczmarek]

[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]

Hi Marcel,

On Fri, Sep 1, 2017 at 2:29 PM, Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Andrzej,
>
> > This adds 'no operation' opcode for the cases where we do not want to
> > include any particular payload, just the header is valid.
> >
> > For example, when some packets are dropped over TTY implementation can
> > pass this information only in some other packet and this won't happen
> > until there's actually something to send. With this addition it can
> > just send nop after some time to indicate there were packets dropped.
> > ---
> > doc/btsnoop.txt | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/doc/btsnoop.txt b/doc/btsnoop.txt
> > index 975a53f6d..865b81554 100644
> > --- a/doc/btsnoop.txt
> > +++ b/doc/btsnoop.txt
> > @@ -115,6 +115,13 @@ User Logging
> >
> >       User logging information.
> >
> > +NOP
> > +-----------
> > +
> > +     Code:        0xffff
> > +
> > +     No operation.
> > +
>
> Hmmm. I am not a huge fan of this. Then I see that we defined the ext_hdr
> that allows to deal with the dropped packets.
>
> I need to think about this a bit since there are some other opcode
> extensions we have to do for certain cases where we have unexpected
> packets. Right now I would propose to use vendor diagnostic or system note
> with empty payload.
>

Vendor diagnostic would produce a message in btmon which may be misleading
and system note with empty payload will print some garbage (i.e. previous
contents​ of buffer) since message is not null-terminated.
But in this case I can just fix btmon to skip system notes with no payload
and it will be basically the same as nop - does it sound good enough for
now?


>
> Regards
>
> Marce
> ​l
>

Best regards,

Andrzej​

>

[-- Attachment #2: Type: text/html, Size: 3455 bytes --]

Re: [PATCH 1/2] doc/btsnoop: Add nop opcode

[Andrzej Kaczmarek]

Hi Marcel,

On Fri, Sep 1, 2017 at 2:29 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrzej,
>
>> This adds 'no operation' opcode for the cases where we do not want to
>> include any particular payload, just the header is valid.
>>
>> For example, when some packets are dropped over TTY implementation can
>> pass this information only in some other packet and this won't happen
>> until there's actually something to send. With this addition it can
>> just send nop after some time to indicate there were packets dropped.
>> ---
>> doc/btsnoop.txt | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/doc/btsnoop.txt b/doc/btsnoop.txt
>> index 975a53f6d..865b81554 100644
>> --- a/doc/btsnoop.txt
>> +++ b/doc/btsnoop.txt
>> @@ -115,6 +115,13 @@ User Logging
>>
>>       User logging information.
>>
>> +NOP
>> +-----------
>> +
>> +     Code:        0xffff
>> +
>> +     No operation.
>> +
>
> Hmmm. I am not a huge fan of this. Then I see that we defined the ext_hdr that allows to deal with the dropped packets.
>
> I need to think about this a bit since there are some other opcode extensions we have to do for certain cases where we have unexpected packets. Right now I would propose to use vendor diagnostic or system note with empty payload.

Vendor diagnostic would produce a message in btmon which may be
misleading and system note with empty payload will print some garbage
(i.e. previous contents of buffer) since message is not
null-terminated.
But in this case I can just fix btmon to skip system notes with no
payload and it will be basically the same as nop - does it sound good
enough for now?

> Regards
>
> Marcel

Best regards,
Andrzej