All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
@ 2020-12-08 17:29 Bastien Nocera
  2020-12-08 18:05 ` bluez.test.bot
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Bastien Nocera @ 2020-12-08 17:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Florian Dollinger

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(+)

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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* RE: Bluetooth: L2CAP: Try harder to accept device not knowing options
  2020-12-08 17:29 [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options Bastien Nocera
@ 2020-12-08 18:05 ` bluez.test.bot
       [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: bluez.test.bot @ 2020-12-08 18:05 UTC (permalink / raw)
  To: linux-bluetooth, hadess

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=398401

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
Bluetooth: L2CAP: Try harder to accept device not knowing options
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8: 
the negotiation when a device responds with L2CAP_CONF_UNACCEPT ("unaccepted

WARNING: Co-developed-by: must be immediately followed by Signed-off-by:
#120: 
Co-developed-by: Florian Dollinger <dollinger.florian@gmx.de>

ERROR: Missing Signed-off-by: line by nominal patch author 'Bastien Nocera <hadess@hadess.net>'

total: 1 errors, 2 warnings, 0 checks, 7 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing" has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - FAIL
Output:
Bluetooth: L2CAP: Try harder to accept device not knowing options
13: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 16                            #64 [hci0] 59.182702"
19: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 23                            #65 [hci0] 59.182744"
30: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 16                            #66 [hci0] 59.183948"
36: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 18                            #67 [hci0] 59.183994"
43: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 15                            #69 [hci0] 59.187676"
49: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 12                            #70 [hci0] 59.187722"
53: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 12                            #73 [hci0] 59.192714"
59: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 16                          #248 [hci0] 103.502970"
65: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 16                          #249 [hci0] 103.504184"
71: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 23                          #250 [hci0] 103.504398"
82: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 16                          #251 [hci0] 103.505472"
88: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 18                          #252 [hci0] 103.505689"
95: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 15                          #254 [hci0] 103.509165"
101: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 12                          #255 [hci0] 103.509426"
105: B1 Line exceeds max length (92>80): "< ACL Data TX: Handle 256 flags 0x00 dlen 12                          #257 [hci0] 103.511870"
109: B1 Line exceeds max length (92>80): "> ACL Data RX: Handle 256 flags 0x02 dlen 14                          #259 [hci0] 103.514121"


##############################
Test: CheckBuildK - PASS



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
       [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
@ 2020-12-08 18:27   ` Bastien Nocera
  2020-12-08 18:48     ` Luiz Augusto von Dentz
  2020-12-16 15:49   ` Bastien Nocera
  2021-01-06 16:42   ` Bastien Nocera
  2 siblings, 1 reply; 14+ messages in thread
From: Bastien Nocera @ 2020-12-08 18:27 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Florian Dollinger

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 have a Windows machine, and the PTS dongle. How do I set up the
qualification test and run it against the Linux machine before and
after the patch?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2020-12-08 18:27   ` [PATCH] " Bastien Nocera
@ 2020-12-08 18:48     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2020-12-08 18:48 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-bluetooth, Florian Dollinger, An, Tedd

Hi Tedd,

On Tue, Dec 8, 2020 at 10:27 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> 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 have a Windows machine, and the PTS dongle. How do I set up the
> qualification test and run it against the Linux machine before and
> after the patch?

@Tedd: Do we happen to autopts working with BlueZ for L2CAP tests? Or
perhaps anything that Bastien can use as a reference to test his
changes.
@Batien: We would like to have autopts
(https://github.com/intel/auto-pts) to handle the qualification but
there is still some work pending in order to enable it to work with
L2CAP. Ultimately we could even attempt to integrate autopts on CI to
run it automatically, but in order to do that we need:

1. We need to emulate the PTS dongle (USB emulator?)
2. Run a Windows VM on CI, attach the emulated dongle to it and run
PTS on top (not sure if github actions do allow this)
3. Identify what tests need to run e.g. L2CAP changes should trigger
only L2CAP tests

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
       [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
  2020-12-08 18:27   ` [PATCH] " Bastien Nocera
@ 2020-12-16 15:49   ` Bastien Nocera
  2021-01-06  9:25     ` Bastien Nocera
  2021-01-06 16:42   ` Bastien Nocera
  2 siblings, 1 reply; 14+ messages in thread
From: Bastien Nocera @ 2020-12-16 15:49 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Florian Dollinger

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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2020-12-16 15:49   ` Bastien Nocera
@ 2021-01-06  9:25     ` Bastien Nocera
  2021-01-06  9:47       ` Archie Pusaka
  0 siblings, 1 reply; 14+ messages in thread
From: Bastien Nocera @ 2021-01-06  9:25 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Florian Dollinger

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...


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2021-01-06  9:25     ` Bastien Nocera
@ 2021-01-06  9:47       ` Archie Pusaka
  2021-01-06 10:36         ` Bastien Nocera
  0 siblings, 1 reply; 14+ messages in thread
From: Archie Pusaka @ 2021-01-06  9:47 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Florian Dollinger

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...
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2021-01-06  9:47       ` Archie Pusaka
@ 2021-01-06 10:36         ` Bastien Nocera
  0 siblings, 0 replies; 14+ messages in thread
From: Bastien Nocera @ 2021-01-06 10:36 UTC (permalink / raw)
  To: Archie Pusaka; +Cc: Luiz Augusto von Dentz, linux-bluetooth, Florian Dollinger

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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
       [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
  2020-12-08 18:27   ` [PATCH] " Bastien Nocera
  2020-12-16 15:49   ` Bastien Nocera
@ 2021-01-06 16:42   ` Bastien Nocera
  2 siblings, 0 replies; 14+ messages in thread
From: Bastien Nocera @ 2021-01-06 16:42 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Florian Dollinger

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2020-12-08 17:29 [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options Bastien Nocera
  2020-12-08 18:05 ` bluez.test.bot
       [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
@ 2021-01-08  1:29 ` Luiz Augusto von Dentz
  2021-01-13 12:32   ` Bastien Nocera
  2021-01-25 18:29   ` Bastien Nocera
  2021-01-25 18:29 ` Marcel Holtmann
  3 siblings, 2 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-08  1:29 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-bluetooth, Florian Dollinger

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2021-01-08  1:29 ` Luiz Augusto von Dentz
@ 2021-01-13 12:32   ` Bastien Nocera
  2021-01-25 18:29   ` Bastien Nocera
  1 sibling, 0 replies; 14+ messages in thread
From: Bastien Nocera @ 2021-01-13 12:32 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Florian Dollinger

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2020-12-08 17:29 [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options Bastien Nocera
                   ` (2 preceding siblings ...)
  2021-01-08  1:29 ` Luiz Augusto von Dentz
@ 2021-01-25 18:29 ` Marcel Holtmann
  3 siblings, 0 replies; 14+ messages in thread
From: Marcel Holtmann @ 2021-01-25 18:29 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: Bluetooth Kernel Mailing List, Florian Dollinger

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2021-01-08  1:29 ` Luiz Augusto von Dentz
  2021-01-13 12:32   ` Bastien Nocera
@ 2021-01-25 18:29   ` Bastien Nocera
  2021-01-25 18:31     ` Bastien Nocera
  1 sibling, 1 reply; 14+ messages in thread
From: Bastien Nocera @ 2021-01-25 18:29 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Marcel Holtmann
  Cc: linux-bluetooth, Florian Dollinger

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options
  2021-01-25 18:29   ` Bastien Nocera
@ 2021-01-25 18:31     ` Bastien Nocera
  0 siblings, 0 replies; 14+ messages in thread
From: Bastien Nocera @ 2021-01-25 18:31 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Marcel Holtmann
  Cc: linux-bluetooth, Florian Dollinger

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!


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-01-26  5:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08 17:29 [PATCH] Bluetooth: L2CAP: Try harder to accept device not knowing options Bastien Nocera
2020-12-08 18:05 ` bluez.test.bot
     [not found] ` <CABBYNZJNTDek+kKS5wtrr67Xx8DmFGvcV13cLSxULgJRa5N+3g@mail.gmail.com>
2020-12-08 18:27   ` [PATCH] " Bastien Nocera
2020-12-08 18:48     ` Luiz Augusto von Dentz
2020-12-16 15:49   ` Bastien Nocera
2021-01-06  9:25     ` Bastien Nocera
2021-01-06  9:47       ` Archie Pusaka
2021-01-06 10:36         ` Bastien Nocera
2021-01-06 16:42   ` Bastien Nocera
2021-01-08  1:29 ` Luiz Augusto von Dentz
2021-01-13 12:32   ` Bastien Nocera
2021-01-25 18:29   ` Bastien Nocera
2021-01-25 18:31     ` Bastien Nocera
2021-01-25 18:29 ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.