Message ID | 20201208172912.4352-1-hadess@hadess.net |
---|---|
State | Accepted |
Commit | 98d2c3e1731007acf03addf83c863df6694beb95 |
Headers | show |
Series | Bluetooth: L2CAP: Try harder to accept device not knowing options | expand |
On Tue, 2020-12-08 at 10:09 -0800, Luiz Augusto von Dentz wrote: > Hi Bastien, > > On Tue, Dec 8, 2020 at 9:36 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > The current implementation of L2CAP options negotiation will > > continue > > the negotiation when a device responds with L2CAP_CONF_UNACCEPT > > ("unaccepted > > options"), but not when the device replies with L2CAP_CONF_UNKNOWN > > ("unknown > > options"). > > > > Trying to continue the negotiation without ERTM support will allow > > Bluetooth-capable XBox One controllers (notably models 1708 and > > 1797) > > to connect. > > While the bellow traces looks fine we need to confirm that it doesn't > break the qualification tests e.g: > > L2CAP/COS/CFD/BV-14-C [Unknown Mandatory Options Request] > > • Test Purpose Verify that the IUT can give the appropriate error > code > when the Lower Tester proposes any number of unknown options where at > least one is mandatory. > > Afaik it should be fine to continue with another round of > configuration given that it only expects the error 0x0003, but we > better confirm PTS doesn't expect a L2CAP Disconnect after it. The tests fail for me in the same on a kernel with and without the patch: - Expected that the IUT transmits an L2CAP_ConfigRsp includes the unsupported option that Lower Tester sent. Final Verdict:FAIL L2CAP/COS/CFD/BV-14-C finished Is this expected? I was using an 5.10-rc7 kernel with and without the patch I sent. I can send you the full results off-list if you want them.
On Wed, 2020-12-16 at 16:49 +0100, Bastien Nocera wrote: > <snip> > The tests fail for me in the same on a kernel with and without > the > patch: > - Expected that the IUT transmits an L2CAP_ConfigRsp includes the > unsupported option that Lower Tester sent. > Final Verdict:FAIL > L2CAP/COS/CFD/BV-14-C finished > > Is this expected? I was using an 5.10-rc7 kernel with and without the > patch I sent. I can send you the full results off-list if you want > them. Any news on that? Is the error expected, should I test with a newer version of the kernel? I'd really like to finally land this...
Hi Bastien, For the test L2CAP/COS/CFD/BV-14-C, this patch is required to pass it. https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=5b8ec15d02f12148ef0185825217162b3bc341f4 I don't think it is merged yet into 5.10, so you might need to apply the patch yourself. Thanks, Archie On Wed, 6 Jan 2021 at 17:28, Bastien Nocera <hadess@hadess.net> wrote: > > On Wed, 2020-12-16 at 16:49 +0100, Bastien Nocera wrote: > > <snip> > > The tests fail for me in the same on a kernel with and without > > the > > patch: > > - Expected that the IUT transmits an L2CAP_ConfigRsp includes the > > unsupported option that Lower Tester sent. > > Final Verdict:FAIL > > L2CAP/COS/CFD/BV-14-C finished > > > > Is this expected? I was using an 5.10-rc7 kernel with and without the > > patch I sent. I can send you the full results off-list if you want > > them. > > Any news on that? Is the error expected, should I test with a newer > version of the kernel? I'd really like to finally land this... >
On Wed, 2021-01-06 at 17:47 +0800, Archie Pusaka wrote: > Hi Bastien, > > For the test L2CAP/COS/CFD/BV-14-C, this patch is required to pass > it. > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=5b8ec15d02f12148ef0185825217162b3bc341f4 > > I don't think it is merged yet into 5.10, so you might need to apply > the patch yourself. > > Thanks, > Archie Thank you Archie, much appreciated. I'll rebuild my test kernels and report back.
On Tue, 2020-12-08 at 10:09 -0800, Luiz Augusto von Dentz wrote: > Hi Bastien, > > On Tue, Dec 8, 2020 at 9:36 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > The current implementation of L2CAP options negotiation will > > continue > > the negotiation when a device responds with L2CAP_CONF_UNACCEPT > > ("unaccepted > > options"), but not when the device replies with L2CAP_CONF_UNKNOWN > > ("unknown > > options"). > > > > Trying to continue the negotiation without ERTM support will allow > > Bluetooth-capable XBox One controllers (notably models 1708 and > > 1797) > > to connect. > > While the bellow traces looks fine we need to confirm that it doesn't > break the qualification tests e.g: > > L2CAP/COS/CFD/BV-14-C [Unknown Mandatory Options Request] > > • Test Purpose Verify that the IUT can give the appropriate error > code > when the Lower Tester proposes any number of unknown options where at > least one is mandatory. > > Afaik it should be fine to continue with another round of > configuration given that it only expects the error 0x0003, but we > better confirm PTS doesn't expect a L2CAP Disconnect after it. I tested this today using Fedora's kernel-5.11.0-0.rc2.114.fc34: https://koji.fedoraproject.org/koji/buildinfo?buildID=1664670 And a local build using the same source kernel with this patch on top. Both managed to pass the test without any problems. I'll send the results of the test privately to you and Marcel. Cheers
Hi, On Tue, Dec 8, 2020 at 9:36 AM Bastien Nocera <hadess@hadess.net> wrote: > > The current implementation of L2CAP options negotiation will continue > the negotiation when a device responds with L2CAP_CONF_UNACCEPT ("unaccepted > options"), but not when the device replies with L2CAP_CONF_UNKNOWN ("unknown > options"). > > Trying to continue the negotiation without ERTM support will allow > Bluetooth-capable XBox One controllers (notably models 1708 and 1797) > to connect. > > btmon before patch: > > ACL Data RX: Handle 256 flags 0x02 dlen 16 #64 [hci0] 59.182702 > L2CAP: Connection Response (0x03) ident 2 len 8 > Destination CID: 64 > Source CID: 64 > Result: Connection successful (0x0000) > Status: No further information available (0x0000) > < ACL Data TX: Handle 256 flags 0x00 dlen 23 #65 [hci0] 59.182744 > L2CAP: Configure Request (0x04) ident 3 len 15 > Destination CID: 64 > Flags: 0x0000 > Option: Retransmission and Flow Control (0x04) [mandatory] > Mode: Basic (0x00) > TX window size: 0 > Max transmit: 0 > Retransmission timeout: 0 > Monitor timeout: 0 > Maximum PDU size: 0 > > ACL Data RX: Handle 256 flags 0x02 dlen 16 #66 [hci0] 59.183948 > L2CAP: Configure Request (0x04) ident 1 len 8 > Destination CID: 64 > Flags: 0x0000 > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > < ACL Data TX: Handle 256 flags 0x00 dlen 18 #67 [hci0] 59.183994 > L2CAP: Configure Response (0x05) ident 1 len 10 > Source CID: 64 > Flags: 0x0000 > Result: Success (0x0000) > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > > ACL Data RX: Handle 256 flags 0x02 dlen 15 #69 [hci0] 59.187676 > L2CAP: Configure Response (0x05) ident 3 len 7 > Source CID: 64 > Flags: 0x0000 > Result: Failure - unknown options (0x0003) > 04 . > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #70 [hci0] 59.187722 > L2CAP: Disconnection Request (0x06) ident 4 len 4 > Destination CID: 64 > Source CID: 64 > > ACL Data RX: Handle 256 flags 0x02 dlen 12 #73 [hci0] 59.192714 > L2CAP: Disconnection Response (0x07) ident 4 len 4 > Destination CID: 64 > Source CID: 64 > > btmon after patch: > > ACL Data RX: Handle 256 flags 0x02 dlen 16 #248 [hci0] 103.502970 > L2CAP: Connection Response (0x03) ident 5 len 8 > Destination CID: 65 > Source CID: 65 > Result: Connection pending (0x0001) > Status: No further information available (0x0000) > > ACL Data RX: Handle 256 flags 0x02 dlen 16 #249 [hci0] 103.504184 > L2CAP: Connection Response (0x03) ident 5 len 8 > Destination CID: 65 > Source CID: 65 > Result: Connection successful (0x0000) > Status: No further information available (0x0000) > < ACL Data TX: Handle 256 flags 0x00 dlen 23 #250 [hci0] 103.504398 > L2CAP: Configure Request (0x04) ident 6 len 15 > Destination CID: 65 > Flags: 0x0000 > Option: Retransmission and Flow Control (0x04) [mandatory] > Mode: Basic (0x00) > TX window size: 0 > Max transmit: 0 > Retransmission timeout: 0 > Monitor timeout: 0 > Maximum PDU size: 0 > > ACL Data RX: Handle 256 flags 0x02 dlen 16 #251 [hci0] 103.505472 > L2CAP: Configure Request (0x04) ident 3 len 8 > Destination CID: 65 > Flags: 0x0000 > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > < ACL Data TX: Handle 256 flags 0x00 dlen 18 #252 [hci0] 103.505689 > L2CAP: Configure Response (0x05) ident 3 len 10 > Source CID: 65 > Flags: 0x0000 > Result: Success (0x0000) > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > > ACL Data RX: Handle 256 flags 0x02 dlen 15 #254 [hci0] 103.509165 > L2CAP: Configure Response (0x05) ident 6 len 7 > Source CID: 65 > Flags: 0x0000 > Result: Failure - unknown options (0x0003) > 04 . > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #255 [hci0] 103.509426 > L2CAP: Configure Request (0x04) ident 7 len 4 > Destination CID: 65 > Flags: 0x0000 > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #257 [hci0] 103.511870 > L2CAP: Connection Request (0x02) ident 8 len 4 > PSM: 1 (0x0001) > Source CID: 66 > > ACL Data RX: Handle 256 flags 0x02 dlen 14 #259 [hci0] 103.514121 > L2CAP: Configure Response (0x05) ident 7 len 6 > Source CID: 65 > Flags: 0x0000 > Result: Success (0x0000) > > Signed-off-by: Florian Dollinger <dollinger.florian@gmx.de> > Co-developed-by: Florian Dollinger <dollinger.florian@gmx.de> Reviewed-by: Luiz Augusto Von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/l2cap_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 1ab27b90ddcb..3ab95ea2cd80 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -4513,6 +4513,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, > } > goto done; > > + case L2CAP_CONF_UNKNOWN: > case L2CAP_CONF_UNACCEPT: > if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) { > char req[64]; > -- > 2.29.2 > -- Luiz Augusto von Dentz
On Thu, 2021-01-07 at 17:29 -0800, Luiz Augusto von Dentz wrote: > Hi, > > On Tue, Dec 8, 2020 at 9:36 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > The current implementation of L2CAP options negotiation will > > continue > > the negotiation when a device responds with L2CAP_CONF_UNACCEPT > > ("unaccepted > > options"), but not when the device replies with L2CAP_CONF_UNKNOWN > > ("unknown > > options"). > > > > Trying to continue the negotiation without ERTM support will allow > > Bluetooth-capable XBox One controllers (notably models 1708 and > > 1797) > > to connect. > <snip> > Reviewed-by: Luiz Augusto Von Dentz <luiz.von.dentz@intel.com> Marcel? Anything else that would need to be done to land this? Cheers
Hi Bastien, > The current implementation of L2CAP options negotiation will continue > the negotiation when a device responds with L2CAP_CONF_UNACCEPT ("unaccepted > options"), but not when the device replies with L2CAP_CONF_UNKNOWN ("unknown > options"). > > Trying to continue the negotiation without ERTM support will allow > Bluetooth-capable XBox One controllers (notably models 1708 and 1797) > to connect. > > btmon before patch: >> ACL Data RX: Handle 256 flags 0x02 dlen 16 #64 [hci0] 59.182702 > L2CAP: Connection Response (0x03) ident 2 len 8 > Destination CID: 64 > Source CID: 64 > Result: Connection successful (0x0000) > Status: No further information available (0x0000) > < ACL Data TX: Handle 256 flags 0x00 dlen 23 #65 [hci0] 59.182744 > L2CAP: Configure Request (0x04) ident 3 len 15 > Destination CID: 64 > Flags: 0x0000 > Option: Retransmission and Flow Control (0x04) [mandatory] > Mode: Basic (0x00) > TX window size: 0 > Max transmit: 0 > Retransmission timeout: 0 > Monitor timeout: 0 > Maximum PDU size: 0 >> ACL Data RX: Handle 256 flags 0x02 dlen 16 #66 [hci0] 59.183948 > L2CAP: Configure Request (0x04) ident 1 len 8 > Destination CID: 64 > Flags: 0x0000 > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > < ACL Data TX: Handle 256 flags 0x00 dlen 18 #67 [hci0] 59.183994 > L2CAP: Configure Response (0x05) ident 1 len 10 > Source CID: 64 > Flags: 0x0000 > Result: Success (0x0000) > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 >> ACL Data RX: Handle 256 flags 0x02 dlen 15 #69 [hci0] 59.187676 > L2CAP: Configure Response (0x05) ident 3 len 7 > Source CID: 64 > Flags: 0x0000 > Result: Failure - unknown options (0x0003) > 04 . > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #70 [hci0] 59.187722 > L2CAP: Disconnection Request (0x06) ident 4 len 4 > Destination CID: 64 > Source CID: 64 >> ACL Data RX: Handle 256 flags 0x02 dlen 12 #73 [hci0] 59.192714 > L2CAP: Disconnection Response (0x07) ident 4 len 4 > Destination CID: 64 > Source CID: 64 > > btmon after patch: >> ACL Data RX: Handle 256 flags 0x02 dlen 16 #248 [hci0] 103.502970 > L2CAP: Connection Response (0x03) ident 5 len 8 > Destination CID: 65 > Source CID: 65 > Result: Connection pending (0x0001) > Status: No further information available (0x0000) >> ACL Data RX: Handle 256 flags 0x02 dlen 16 #249 [hci0] 103.504184 > L2CAP: Connection Response (0x03) ident 5 len 8 > Destination CID: 65 > Source CID: 65 > Result: Connection successful (0x0000) > Status: No further information available (0x0000) > < ACL Data TX: Handle 256 flags 0x00 dlen 23 #250 [hci0] 103.504398 > L2CAP: Configure Request (0x04) ident 6 len 15 > Destination CID: 65 > Flags: 0x0000 > Option: Retransmission and Flow Control (0x04) [mandatory] > Mode: Basic (0x00) > TX window size: 0 > Max transmit: 0 > Retransmission timeout: 0 > Monitor timeout: 0 > Maximum PDU size: 0 >> ACL Data RX: Handle 256 flags 0x02 dlen 16 #251 [hci0] 103.505472 > L2CAP: Configure Request (0x04) ident 3 len 8 > Destination CID: 65 > Flags: 0x0000 > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 > < ACL Data TX: Handle 256 flags 0x00 dlen 18 #252 [hci0] 103.505689 > L2CAP: Configure Response (0x05) ident 3 len 10 > Source CID: 65 > Flags: 0x0000 > Result: Success (0x0000) > Option: Maximum Transmission Unit (0x01) [mandatory] > MTU: 1480 >> ACL Data RX: Handle 256 flags 0x02 dlen 15 #254 [hci0] 103.509165 > L2CAP: Configure Response (0x05) ident 6 len 7 > Source CID: 65 > Flags: 0x0000 > Result: Failure - unknown options (0x0003) > 04 . > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #255 [hci0] 103.509426 > L2CAP: Configure Request (0x04) ident 7 len 4 > Destination CID: 65 > Flags: 0x0000 > < ACL Data TX: Handle 256 flags 0x00 dlen 12 #257 [hci0] 103.511870 > L2CAP: Connection Request (0x02) ident 8 len 4 > PSM: 1 (0x0001) > Source CID: 66 >> ACL Data RX: Handle 256 flags 0x02 dlen 14 #259 [hci0] 103.514121 > L2CAP: Configure Response (0x05) ident 7 len 6 > Source CID: 65 > Flags: 0x0000 > Result: Success (0x0000) > > Signed-off-by: Florian Dollinger <dollinger.florian@gmx.de> > Co-developed-by: Florian Dollinger <dollinger.florian@gmx.de> > --- > net/bluetooth/l2cap_core.c | 1 + > 1 file changed, 1 insertion(+) patch has been applied to bluetooth-next tree. Regards Marcel
On Thu, 2021-01-07 at 17:29 -0800, Luiz Augusto von Dentz wrote: > Hi, > > On Tue, Dec 8, 2020 at 9:36 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > The current implementation of L2CAP options negotiation will > > continue > > the negotiation when a device responds with L2CAP_CONF_UNACCEPT > > ("unaccepted > > options"), but not when the device replies with L2CAP_CONF_UNKNOWN > > ("unknown > > options"). > > > > Trying to continue the negotiation without ERTM support will allow > > Bluetooth-capable XBox One controllers (notably models 1708 and > > 1797) > > to connect. > > > > <snip> > > Signed-off-by: Florian Dollinger <dollinger.florian@gmx.de> > > Co-developed-by: Florian Dollinger <dollinger.florian@gmx.de> > > Reviewed-by: Luiz Augusto Von Dentz <luiz.von.dentz@intel.com> Marcel, any chance of a review on this one? I've sent you the results of the PTS tests privately, but looks like you weren't CC:ed on the earlier mail thread. Cheers
On Mon, 2021-01-25 at 19:29 +0100, Bastien Nocera wrote: > <snip> > Marcel, any chance of a review on this one? I've sent you the > results > of the PTS tests privately, but looks like you weren't CC:ed on the > earlier mail thread. Our e-mails only just crossed paths. Thanks for merging it!
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 1ab27b90ddcb..3ab95ea2cd80 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -4513,6 +4513,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, } goto done; + case L2CAP_CONF_UNKNOWN: case L2CAP_CONF_UNACCEPT: if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) { char req[64];