linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
@ 2012-05-28  9:47 Szymon Janc
  2012-05-28  9:47 ` [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions Szymon Janc
  2012-05-29 17:37 ` [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Gustavo Padovan
  0 siblings, 2 replies; 11+ messages in thread
From: Szymon Janc @ 2012-05-28  9:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
(2 seconds) timeout value is being used. This results in problems with
legacy pairing as remote user has only few seconds to enter PIN before
ACL is disconnected.

Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
and no linkey exists.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---
 net/bluetooth/hci_event.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ac86b65..98e8020 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		if (conn->type == ACL_LINK) {
 			conn->state = BT_CONFIG;
 			hci_conn_hold(conn);
-			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
+
+			if (!hci_conn_ssp_enabled(conn) &&
+			    !hci_find_link_key(hdev, &ev->bdaddr))
+				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
+			else
+				conn->disc_timeout = HCI_DISCONN_TIMEOUT;
 		} else
 			conn->state = BT_CONNECTED;
 
-- 
on behalf of ST-Ericsson


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

* [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions
  2012-05-28  9:47 [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Szymon Janc
@ 2012-05-28  9:47 ` Szymon Janc
  2012-05-29 17:39   ` Gustavo Padovan
  2012-05-29 17:37 ` [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Gustavo Padovan
  1 sibling, 1 reply; 11+ messages in thread
From: Szymon Janc @ 2012-05-28  9:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Those are not used anywhere in code (and never were since introduction
in 2006) so just remove them.

Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
---
 include/net/bluetooth/hci.h |    2 --
 1 file changed, 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index de09a26..acd1691 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -136,10 +136,8 @@ enum {
 #define HCIINQUIRY	_IOR('H', 240, int)
 
 /* HCI timeouts */
-#define HCI_CONNECT_TIMEOUT	(40000)	/* 40 seconds */
 #define HCI_DISCONN_TIMEOUT	(2000)	/* 2 seconds */
 #define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
-#define HCI_IDLE_TIMEOUT	(6000)	/* 6 seconds */
 #define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
 #define HCI_CMD_TIMEOUT		(1000)	/* 1 seconds */
 #define HCI_ACL_TX_TIMEOUT	(45000)	/* 45 seconds */
-- 
on behalf of ST-Ericsson


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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-05-28  9:47 [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Szymon Janc
  2012-05-28  9:47 ` [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions Szymon Janc
@ 2012-05-29 17:37 ` Gustavo Padovan
  2012-05-30  7:41   ` Szymon Janc
  1 sibling, 1 reply; 11+ messages in thread
From: Gustavo Padovan @ 2012-05-29 17:37 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2012-05-28 11:47:19 +0200]:

> Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
> before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
> (2 seconds) timeout value is being used. This results in problems with
> legacy pairing as remote user has only few seconds to enter PIN before
> ACL is disconnected.
> 
> Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
> and no linkey exists.

does this only happen with SSP disabled?

Also add the hcidump output to the commit message would be helpful.

> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
>  net/bluetooth/hci_event.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ac86b65..98e8020 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		if (conn->type == ACL_LINK) {
>  			conn->state = BT_CONFIG;
>  			hci_conn_hold(conn);
> -			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> +
> +			if (!hci_conn_ssp_enabled(conn) &&
> +			    !hci_find_link_key(hdev, &ev->bdaddr))
> +				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> +			else
> +				conn->disc_timeout = HCI_DISCONN_TIMEOUT;

You are also changing the timeout for a SDP search, for example, to
HCI_PAIRING_TIMEOUT and this is not good. I think we need to be smarter here,
we can't change the behaviour of things like SDP search here.

	Gustavo

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

* Re: [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions
  2012-05-28  9:47 ` [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions Szymon Janc
@ 2012-05-29 17:39   ` Gustavo Padovan
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Padovan @ 2012-05-29 17:39 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [2012-05-28 11:47:20 +0200]:

> Those are not used anywhere in code (and never were since introduction
> in 2006) so just remove them.
> 
> Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> ---
>  include/net/bluetooth/hci.h |    2 --
>  1 file changed, 2 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-05-29 17:37 ` [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Gustavo Padovan
@ 2012-05-30  7:41   ` Szymon Janc
  2012-06-29 10:56     ` Johan Hedberg
  0 siblings, 1 reply; 11+ messages in thread
From: Szymon Janc @ 2012-05-30  7:41 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth@vger.kernel.org

Hi Gustavo,

> > Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
> > before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
> > (2 seconds) timeout value is being used. This results in problems with
> > legacy pairing as remote user has only few seconds to enter PIN before
> > ACL is disconnected.
> > 
> > Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
> > and no linkey exists.
> 
> does this only happen with SSP disabled?

Yes, I've seen this only with SSP disabled.
After name request remote devices ask user to enter PIN and before user is
able to do so (4 sec only) link is disconnected (cause we didn't get pin
request event yet).

For SSP hci_conn_hold is called in hci_io_capa_request_evt so link is not
disconnected.

> 
> Also add the hcidump output to the commit message would be helpful.

hcidump for ssp disabled [1] and ssp enabled [2] below.

> > 
> > Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> > ---
> >  net/bluetooth/hci_event.c |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index ac86b65..98e8020 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >  		if (conn->type == ACL_LINK) {
> >  			conn->state = BT_CONFIG;
> >  			hci_conn_hold(conn);
> > -			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > +
> > +			if (!hci_conn_ssp_enabled(conn) &&
> > +			    !hci_find_link_key(hdev, &ev->bdaddr))

I could add also checking if connection is incoming to narrow case a bit further...

> > +				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> > +			else
> > +				conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> 
> You are also changing the timeout for a SDP search, for example, to
> HCI_PAIRING_TIMEOUT and this is not good. I think we need to be smarter here,
> we can't change the behaviour of things like SDP search here.

Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is connected (or disconnected)?
That should cover SDP search case..


[1]
1338360473.214785 > HCI Event: Connect Request (0x04) plen 10
1338360473.214976 < HCI Command: Accept Connection Request (0x01|0x0009) plen 7
1338360473.218782 > HCI Event: Command Status (0x0f) plen 4
1338360473.380780 > HCI Event: Role Change (0x12) plen 8
1338360473.392781 > HCI Event: Connect Complete (0x03) plen 11
1338360473.392806 < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
1338360473.393754 > HCI Event: Page Scan Repetition Mode Change (0x20) plen 7
1338360473.395752 > HCI Event: Command Status (0x0f) plen 4
1338360473.410779 > HCI Event: Max Slots Change (0x1b) plen 3
1338360473.411749 > HCI Event: Command Status (0x0f) plen 4
1338360473.426778 > HCI Event: Read Remote Supported Features (0x0b) plen 11
1338360473.426797 < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
1338360473.431755 > HCI Event: Command Status (0x0f) plen 4
1338360473.431774 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
1338360473.435780 > HCI Event: Command Status (0x0f) plen 4
1338360473.438766 > HCI Event: Read Remote Extended Features (0x23) plen 13
1338360473.438788 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
1338360473.462777 > HCI Event: Remote Name Req Complete (0x07) plen 255
1338360473.463764 > HCI Event: Command Status (0x0f) plen 4
1338360473.482777 > HCI Event: Remote Name Req Complete (0x07) plen 255
1338360477.448436 < HCI Command: Disconnect (0x01|0x0006) plen 3
1338360477.451710 > HCI Event: Command Status (0x0f) plen 4
1338360477.531708 > HCI Event: Disconn Complete (0x05) plen 4


[2]
1338360561.019888 > HCI Event: Connect Request (0x04) plen 10                                                                                                        
1338360561.020065 < HCI Command: Accept Connection Request (0x01|0x0009) plen 7                                                                                      
1338360561.023883 > HCI Event: Command Status (0x0f) plen 4
1338360561.185883 > HCI Event: Role Change (0x12) plen 8
1338360561.198881 > HCI Event: Connect Complete (0x03) plen 11
1338360561.198906 < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
1338360561.199856 > HCI Event: Page Scan Repetition Mode Change (0x20) plen 7
1338360561.201855 > HCI Event: Command Status (0x0f) plen 4
1338360561.226883 > HCI Event: Max Slots Change (0x1b) plen 3
1338360561.227852 > HCI Event: Command Status (0x0f) plen 4
1338360561.232474 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
1338360561.234855 > HCI Event: Read Remote Supported Features (0x0b) plen 11
1338360561.237852 > HCI Event: Command Status (0x0f) plen 4
1338360561.237866 < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3
1338360561.241851 > HCI Event: Command Status (0x0f) plen 4
1338360561.265852 > HCI Event: Remote Name Req Complete (0x07) plen 255
1338360561.266851 > HCI Event: Read Remote Extended Features (0x23) plen 13
1338360561.266869 < HCI Command: Remote Name Request (0x01|0x0019) plen 10
1338360561.269852 > HCI Event: Command Status (0x0f) plen 4
1338360561.296849 > HCI Event: Remote Name Req Complete (0x07) plen 255
1338360561.304851 > HCI Event: IO Capability Response (0x32) plen 9
1338360561.305850 > HCI Event: IO Capability Request (0x31) plen 6
1338360561.305972 < HCI Command: IO Capability Request Reply (0x01|0x002b) plen 9
1338360561.323850 > HCI Event: Command Complete (0x0e) plen 10
1338360562.861841 > HCI Event: User Confirmation Request (0x33) plen 10
1338360572.626696 < HCI Command: User Confirmation Request Reply (0x01|0x002c) plen 6
1338360572.662750 > HCI Event: Command Complete (0x0e) plen 10
1338360572.685766 > HCI Event: Simple Pairing Complete (0x36) plen 7
1338360572.727764 > HCI Event: Link Key Notification (0x18) plen 23
1338360572.782738 > HCI Event: Encrypt Change (0x08) plen 4
1338360572.786359 > ACL data: handle 43 flags 0x02 dlen 10


-- 
BR
Szymon

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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-05-30  7:41   ` Szymon Janc
@ 2012-06-29 10:56     ` Johan Hedberg
  2012-07-04  7:56       ` Gustavo Padovan
  0 siblings, 1 reply; 11+ messages in thread
From: Johan Hedberg @ 2012-06-29 10:56 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Gustavo Padovan, linux-bluetooth@vger.kernel.org

Hi Szymon & Gustavo,

On Wed, May 30, 2012, Szymon Janc wrote:
> > > Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
> > > before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
> > > (2 seconds) timeout value is being used. This results in problems with
> > > legacy pairing as remote user has only few seconds to enter PIN before
> > > ACL is disconnected.
> > > 
> > > Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
> > > and no linkey exists.
> > 
> > does this only happen with SSP disabled?
> 
> Yes, I've seen this only with SSP disabled.
> After name request remote devices ask user to enter PIN and before user is
> able to do so (4 sec only) link is disconnected (cause we didn't get pin
> request event yet).
> 
> For SSP hci_conn_hold is called in hci_io_capa_request_evt so link is not
> disconnected.
> 
> > 
> > Also add the hcidump output to the commit message would be helpful.
> 
> hcidump for ssp disabled [1] and ssp enabled [2] below.
> 
> > > 
> > > Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> > > ---
> > >  net/bluetooth/hci_event.c |    7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index ac86b65..98e8020 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> > >  		if (conn->type == ACL_LINK) {
> > >  			conn->state = BT_CONFIG;
> > >  			hci_conn_hold(conn);
> > > -			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > > +
> > > +			if (!hci_conn_ssp_enabled(conn) &&
> > > +			    !hci_find_link_key(hdev, &ev->bdaddr))
> 
> I could add also checking if connection is incoming to narrow case a bit further...
> 
> > > +				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> > > +			else
> > > +				conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > 
> > You are also changing the timeout for a SDP search, for example, to
> > HCI_PAIRING_TIMEOUT and this is not good. I think we need to be smarter here,
> > we can't change the behaviour of things like SDP search here.
> 
> Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> connected (or disconnected)?  That should cover SDP search case..

What happened to getting this patch upstream? To me it looks like a
definitely needed fix. After adding the fix to restore a sensible value
for disc_timeout after an L2CAP connect request either way and adding a
better explanation to the commit message (that we only get the PIN
request after user has entered one on the remote side, including a
hcidump of this) I think this should go upstream. If this had been
processed in a timely manner it could have made it to 3.5 but now it
seems too late for that (as it's not strictly speaking a regression from
3.4).

Johan

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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-06-29 10:56     ` Johan Hedberg
@ 2012-07-04  7:56       ` Gustavo Padovan
  2012-07-19  9:13         ` Szymon Janc
  0 siblings, 1 reply; 11+ messages in thread
From: Gustavo Padovan @ 2012-07-04  7:56 UTC (permalink / raw)
  To: Szymon Janc, linux-bluetooth@vger.kernel.org

Hi Szymon,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-06-29 13:56:53 +0300]:

> Hi Szymon & Gustavo,
> 
> On Wed, May 30, 2012, Szymon Janc wrote:
> > > > Some devices e.g. SonyEricsson Xperia ray and arc S don't do SDP search
> > > > before pairing. No L2CAP is connected so default HCI_DISCONN_TIMEOUT
> > > > (2 seconds) timeout value is being used. This results in problems with
> > > > legacy pairing as remote user has only few seconds to enter PIN before
> > > > ACL is disconnected.
> > > > 
> > > > Increase disconnect timeout to HCI_PAIRING_TIMEOUT if SSP is disabled
> > > > and no linkey exists.
> > > 
> > > does this only happen with SSP disabled?
> > 
> > Yes, I've seen this only with SSP disabled.
> > After name request remote devices ask user to enter PIN and before user is
> > able to do so (4 sec only) link is disconnected (cause we didn't get pin
> > request event yet).
> > 
> > For SSP hci_conn_hold is called in hci_io_capa_request_evt so link is not
> > disconnected.
> > 
> > > 
> > > Also add the hcidump output to the commit message would be helpful.
> > 
> > hcidump for ssp disabled [1] and ssp enabled [2] below.
> > 
> > > > 
> > > > Signed-off-by: Szymon Janc <szymon.janc@tieto.com>
> > > > ---
> > > >  net/bluetooth/hci_event.c |    7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > index ac86b65..98e8020 100644
> > > > --- a/net/bluetooth/hci_event.c
> > > > +++ b/net/bluetooth/hci_event.c
> > > > @@ -1762,7 +1762,12 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> > > >  		if (conn->type == ACL_LINK) {
> > > >  			conn->state = BT_CONFIG;
> > > >  			hci_conn_hold(conn);
> > > > -			conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > > > +
> > > > +			if (!hci_conn_ssp_enabled(conn) &&
> > > > +			    !hci_find_link_key(hdev, &ev->bdaddr))
> > 
> > I could add also checking if connection is incoming to narrow case a bit further...
> > 
> > > > +				conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> > > > +			else
> > > > +				conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> > > 
> > > You are also changing the timeout for a SDP search, for example, to
> > > HCI_PAIRING_TIMEOUT and this is not good. I think we need to be smarter here,
> > > we can't change the behaviour of things like SDP search here.
> > 
> > Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> > connected (or disconnected)?  That should cover SDP search case..
> 
> What happened to getting this patch upstream? To me it looks like a
> definitely needed fix. After adding the fix to restore a sensible value
> for disc_timeout after an L2CAP connect request either way and adding a
> better explanation to the commit message (that we only get the PIN
> request after user has entered one on the remote side, including a
> hcidump of this) I think this should go upstream. If this had been
> processed in a timely manner it could have made it to 3.5 but now it
> seems too late for that (as it's not strictly speaking a regression from
> 3.4).

Could you re-do this patch as Johan says so we can try push it to 3.5?

	Gustavo

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

* Re: Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-07-04  7:56       ` Gustavo Padovan
@ 2012-07-19  9:13         ` Szymon Janc
  2012-07-19  9:34           ` Johan Hedberg
  0 siblings, 1 reply; 11+ messages in thread
From: Szymon Janc @ 2012-07-19  9:13 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth@vger.kernel.org

(as plain text this time..)

Hi Gustavo & Johan,

> > > Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> > > connected (or disconnected)? That should cover SDP search case..
> >
> > What happened to getting this patch upstream? To me it looks like a
> > definitely needed fix. After adding the fix to restore a sensible value
> > for disc_timeout after an L2CAP connect request either way and adding a
> > better explanation to the commit message (that we only get the PIN
> > request after user has entered one on the remote side, including a
> > hcidump of this) I think this should go upstream. If this had been
> > processed in a timely manner it could have made it to 3.5 but now it
> > seems too late for that (as it's not strictly speaking a regression from
> > 3.4).
>
> Could you re-do this patch as Johan says so we can try push it to 3.5?

Sorry for late reply, been on rather long holiday..

I now feel that this is a bug in bluez not in kernel - bluez should not cancel
PIN request for agent if acl is disconnected but just reconnect when PIN is
provided.

So, if you still like to have this workaround in kernel for devices which are
highly unlikely to have this fixed I can send new version.

--
BR
Szymon Janc

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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-07-19  9:13         ` Szymon Janc
@ 2012-07-19  9:34           ` Johan Hedberg
  2012-07-19 10:17             ` Szymon Janc
  0 siblings, 1 reply; 11+ messages in thread
From: Johan Hedberg @ 2012-07-19  9:34 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Gustavo Padovan, linux-bluetooth@vger.kernel.org

Hi Szymon,

On Thu, Jul 19, 2012, Szymon Janc wrote:
> (as plain text this time..)
> 
> Hi Gustavo & Johan,
> 
> > > > Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> > > > connected (or disconnected)? That should cover SDP search case..
> > >
> > > What happened to getting this patch upstream? To me it looks like a
> > > definitely needed fix. After adding the fix to restore a sensible value
> > > for disc_timeout after an L2CAP connect request either way and adding a
> > > better explanation to the commit message (that we only get the PIN
> > > request after user has entered one on the remote side, including a
> > > hcidump of this) I think this should go upstream. If this had been
> > > processed in a timely manner it could have made it to 3.5 but now it
> > > seems too late for that (as it's not strictly speaking a regression from
> > > 3.4).
> >
> > Could you re-do this patch as Johan says so we can try push it to 3.5?
> 
> Sorry for late reply, been on rather long holiday..
> 
> I now feel that this is a bug in bluez not in kernel - bluez should not cancel
> PIN request for agent if acl is disconnected but just reconnect when PIN is
> provided.
> 
> So, if you still like to have this workaround in kernel for devices which are
> highly unlikely to have this fixed I can send new version.

I don't agree with your categorization of the kernel change as a
workaround. Since it was the remote side the initiated the initial ACL I
don't think it's right for us to have any kind of automation of creating
a second ACL in the opposite direction. So please do the improvements to
your patch as discussed and resend. Thanks.

Johan

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

* Re: Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-07-19  9:34           ` Johan Hedberg
@ 2012-07-19 10:17             ` Szymon Janc
  2012-07-19 10:25               ` Johan Hedberg
  0 siblings, 1 reply; 11+ messages in thread
From: Szymon Janc @ 2012-07-19 10:17 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Gustavo Padovan, linux-bluetooth@vger.kernel.org

On Thursday 19 of July 2012 12:34:52 Johan Hedberg wrote:

> > > > > Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> > > > > connected (or disconnected)? That should cover SDP search case..
> > > >
> > > > What happened to getting this patch upstream? To me it looks like a
> > > > definitely needed fix. After adding the fix to restore a sensible value
> > > > for disc_timeout after an L2CAP connect request either way and adding a
> > > > better explanation to the commit message (that we only get the PIN
> > > > request after user has entered one on the remote side, including a
> > > > hcidump of this) I think this should go upstream. If this had been
> > > > processed in a timely manner it could have made it to 3.5 but now it
> > > > seems too late for that (as it's not strictly speaking a regression from
> > > > 3.4).
> > >
> > > Could you re-do this patch as Johan says so we can try push it to 3.5?
> > 
> > Sorry for late reply, been on rather long holiday..
> > 
> > I now feel that this is a bug in bluez not in kernel - bluez should not cancel
> > PIN request for agent if acl is disconnected but just reconnect when PIN is
> > provided.
> > 
> > So, if you still like to have this workaround in kernel for devices which are
> > highly unlikely to have this fixed I can send new version.
> 
> I don't agree with your categorization of the kernel change as a
> workaround. Since it was the remote side the initiated the initial ACL I
> don't think it's right for us to have any kind of automation of creating
> a second ACL in the opposite direction. So please do the improvements to
> your patch as discussed and resend. Thanks.

I guess I wasn't clear enough.. I meant that the bug is in bluez which is also
running on remote device and is unlikely to get updated. No connection in 
opposite direction is taking place. It is bluez on initiator side that should
reconnect when it received PIN from user if acl was disconnected. Most agents
do service search in the meantime so l2cap exists and acl is not disconnected.

This can be easily reproduced with simple-agent which is not doing service
search.

-- 
BR
Szymon Janc

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

* Re: [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices
  2012-07-19 10:17             ` Szymon Janc
@ 2012-07-19 10:25               ` Johan Hedberg
  0 siblings, 0 replies; 11+ messages in thread
From: Johan Hedberg @ 2012-07-19 10:25 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Gustavo Padovan, linux-bluetooth@vger.kernel.org

Hi Szymon,

On Thu, Jul 19, 2012, Szymon Janc wrote:
> On Thursday 19 of July 2012 12:34:52 Johan Hedberg wrote:
> 
> > > > > > Maybe we could set timeout back to HCI_DICONN_TIMEOUT when l2cap is
> > > > > > connected (or disconnected)? That should cover SDP search case..
> > > > >
> > > > > What happened to getting this patch upstream? To me it looks like a
> > > > > definitely needed fix. After adding the fix to restore a sensible value
> > > > > for disc_timeout after an L2CAP connect request either way and adding a
> > > > > better explanation to the commit message (that we only get the PIN
> > > > > request after user has entered one on the remote side, including a
> > > > > hcidump of this) I think this should go upstream. If this had been
> > > > > processed in a timely manner it could have made it to 3.5 but now it
> > > > > seems too late for that (as it's not strictly speaking a regression from
> > > > > 3.4).
> > > >
> > > > Could you re-do this patch as Johan says so we can try push it to 3.5?
> > > 
> > > Sorry for late reply, been on rather long holiday..
> > > 
> > > I now feel that this is a bug in bluez not in kernel - bluez should not cancel
> > > PIN request for agent if acl is disconnected but just reconnect when PIN is
> > > provided.
> > > 
> > > So, if you still like to have this workaround in kernel for devices which are
> > > highly unlikely to have this fixed I can send new version.
> > 
> > I don't agree with your categorization of the kernel change as a
> > workaround. Since it was the remote side the initiated the initial ACL I
> > don't think it's right for us to have any kind of automation of creating
> > a second ACL in the opposite direction. So please do the improvements to
> > your patch as discussed and resend. Thanks.
> 
> I guess I wasn't clear enough.. I meant that the bug is in bluez which is also
> running on remote device and is unlikely to get updated. No connection in 
> opposite direction is taking place. It is bluez on initiator side that should
> reconnect when it received PIN from user if acl was disconnected. Most agents
> do service search in the meantime so l2cap exists and acl is not disconnected.
> 
> This can be easily reproduced with simple-agent which is not doing service
> search.

Understood. I still think that we should do the necessary changes to the
disconnect timer. I've actually seen this kind of behavior quite often
at UnplugFests and I doubt all of the other parties have been running
BlueZ.

Johan

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

end of thread, other threads:[~2012-07-19 10:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-28  9:47 [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Szymon Janc
2012-05-28  9:47 ` [PATCH 2/2] Bluetooth: Remove unused HCI timeouts definitions Szymon Janc
2012-05-29 17:39   ` Gustavo Padovan
2012-05-29 17:37 ` [PATCH 1/2] Bluetooth: Fix legacy pairing with some devices Gustavo Padovan
2012-05-30  7:41   ` Szymon Janc
2012-06-29 10:56     ` Johan Hedberg
2012-07-04  7:56       ` Gustavo Padovan
2012-07-19  9:13         ` Szymon Janc
2012-07-19  9:34           ` Johan Hedberg
2012-07-19 10:17             ` Szymon Janc
2012-07-19 10:25               ` Johan Hedberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).