| Message ID | 20201102132733.GA77385@jimmy-ryzen-home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix for Bluetooth SIG test L2CAP/COS/CFD/BV-14-C. | expand |
Hi Jimmy, On Mon, Nov 2, 2020 at 5:33 AM Jimmy Wahlberg <jimmywa@spotify.com> wrote: > > This test case is meant to verify that multiple > unknown options is included in the response. > > Unknown options shall be in the response if > they are not hints according to Bluetooth Core > Spec v5.2. See chapter 4.5 L2CAP_CONFIGURATION_RSP Can you add the HCI trace (btmon) with and without the patch. Also perhaps we should have a comment why this is needed on the code: 'On an unknown option failure (Result=0x0003), the option(s) that contain an option type field that is not understood by the recipient of the L2CAP_CONFIGURATION_REQ packet shall be included in the L2CAP_CONFIGURATION_RSP packet unless they are hints.' > Signed-off-by: Jimmy Wahlberg <jimmywa@spotify.com> > --- > net/bluetooth/l2cap_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 1ab27b90ddcb..16956f323688 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3627,7 +3627,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > if (hint) > break; > result = L2CAP_CONF_UNKNOWN; > - *((u8 *) ptr++) = type; > + l2cap_add_conf_opt(&ptr, (u8)type, sizeof(u8), type, endptr - ptr); > break; > } > } > -- > 2.25.1 > > Hi, > > While running tests I encountered this one test that I could not pass. After some troubleshooting I landed in this patch. Please let me know what you think. > > Best regards > Jimmy
On Tue, Nov 03, 2020 at 01:43:01PM -0800, Luiz Augusto von Dentz wrote: > Hi Jimmy, > > On Mon, Nov 2, 2020 at 5:33 AM Jimmy Wahlberg <jimmywa@spotify.com> wrote: > > > > This test case is meant to verify that multiple > > unknown options is included in the response. > > > > Unknown options shall be in the response if > > they are not hints according to Bluetooth Core > > Spec v5.2. See chapter 4.5 L2CAP_CONFIGURATION_RSP > > Can you add the HCI trace (btmon) with and without the patch. Also > perhaps we should have a comment why this is needed on the code: Hi, thank you for the feedback. Here is some btmon output without the patch. > ACL Data RX: Handle 11 flags 0x02 dlen 16 #50 [hci0] 72.519062 L2CAP: Configure Request (0x04) ident 16 len 8 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [mandatory] 10 00 .. < ACL Data TX: Handle 11 flags 0x00 dlen 15 #51 [hci0] 72.519162 L2CAP: Configure Response (0x05) ident 16 len 7 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) 10 . > ACL Data RX: Handle 11 flags 0x02 dlen 20 #52 [hci0] 72.634066 L2CAP: Configure Request (0x04) ident 17 len 12 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [hint] 10 00 11 02 11 00 ...... < ACL Data TX: Handle 11 flags 0x00 dlen 15 #53 [hci0] 72.634178 L2CAP: Configure Response (0x05) ident 17 len 7 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) 11 . > HCI Event: Number of Completed Packets (0x13) plen 5 #54 [hci0] 72.656717 Num handles: 1 Handle: 11 Count: 2 > ACL Data RX: Handle 11 flags 0x02 dlen 24 #55 [hci0] 72.764088 L2CAP: Configure Request (0x04) ident 18 len 16 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [mandatory] 10 00 11 02 11 00 12 02 12 00 .......... < ACL Data TX: Handle 11 flags 0x00 dlen 17 #56 [hci0] 72.764235 L2CAP: Configure Response (0x05) ident 18 len 9 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) Option: Unknown (0x10) [mandatory] 12 Here is btmon output with the patch. Both outputs are from running the test case in subject. > ACL Data RX: Handle 11 flags 0x02 dlen 16 #89 [hci0] 388.666111 L2CAP: Configure Request (0x04) ident 3 len 8 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [mandatory] 10 00 .. < ACL Data TX: Handle 11 flags 0x00 dlen 17 #90 [hci0] 388.666216 L2CAP: Configure Response (0x05) ident 3 len 9 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) Option: Unknown (0x10) [mandatory] 10 . > HCI Event: Number of Completed Packets (0x13) plen 5 #91 [hci0] 388.804992 Num handles: 1 Handle: 11 Count: 1 > ACL Data RX: Handle 11 flags 0x02 dlen 20 #92 [hci0] 390.763564 L2CAP: Configure Request (0x04) ident 4 len 12 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [hint] 10 00 11 02 11 00 ...... < ACL Data TX: Handle 11 flags 0x00 dlen 17 #93 [hci0] 390.763727 L2CAP: Configure Response (0x05) ident 4 len 9 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) Option: Unknown (0x11) [mandatory] 11 . > HCI Event: Number of Completed Packets (0x13) plen 5 #94 [hci0] 390.930118 Num handles: 1 Handle: 11 Count: 1 > ACL Data RX: Handle 11 flags 0x02 dlen 24 #95 [hci0] 392.863591 L2CAP: Configure Request (0x04) ident 5 len 16 Destination CID: 64 Flags: 0x0000 Option: Unknown (0x10) [mandatory] 10 00 11 02 11 00 12 02 12 00 .......... < ACL Data TX: Handle 11 flags 0x00 dlen 23 #96 [hci0] 392.863742 L2CAP: Configure Response (0x05) ident 5 len 15 Source CID: 64 Flags: 0x0000 Result: Failure - unknown options (0x0003) Option: Unknown (0x10) [mandatory] 10 11 01 11 12 01 12 Regarding adding a comment. I will gladly add this comment if you think it's important. > > 'On an unknown option failure (Result=0x0003), the option(s) that contain an > option type field that is not understood by the recipient of the > L2CAP_CONFIGURATION_REQ packet shall be included in the > L2CAP_CONFIGURATION_RSP packet unless they are hints.' > > > Signed-off-by: Jimmy Wahlberg <jimmywa@spotify.com> > > --- > > net/bluetooth/l2cap_core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index 1ab27b90ddcb..16956f323688 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -3627,7 +3627,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data > > if (hint) > > break; > > result = L2CAP_CONF_UNKNOWN; > > - *((u8 *) ptr++) = type; > > + l2cap_add_conf_opt(&ptr, (u8)type, sizeof(u8), type, endptr - ptr); > > break; > > } > > } > > -- > > 2.25.1 > > > > Hi, > > > > While running tests I encountered this one test that I could not pass. After some troubleshooting I landed in this patch. Please let me know what you think. > > > > Best regards > > Jimmy > > > > -- > Luiz Augusto von Dentz
Hi Jimmy, > This test case is meant to verify that multiple > unknown options is included in the response. > > Unknown options shall be in the response if > they are not hints according to Bluetooth Core > Spec v5.2. See chapter 4.5 L2CAP_CONFIGURATION_RSP please create a commit message that has extracts from the before and after of btmon trace. Regards Marcel
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 1ab27b90ddcb..16956f323688 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -3627,7 +3627,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data if (hint) break; result = L2CAP_CONF_UNKNOWN; - *((u8 *) ptr++) = type; + l2cap_add_conf_opt(&ptr, (u8)type, sizeof(u8), type, endptr - ptr); break; } }
This test case is meant to verify that multiple unknown options is included in the response. Unknown options shall be in the response if they are not hints according to Bluetooth Core Spec v5.2. See chapter 4.5 L2CAP_CONFIGURATION_RSP Signed-off-by: Jimmy Wahlberg <jimmywa@spotify.com> --- net/bluetooth/l2cap_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)