[PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Gustavo F. Padovan]

From: Gustavo F. Padovan <padovan@profusion.mobi>

If the remote side doesn't support Enhanced Retransmission Mode neither
Streaming Mode, we shall not send the RFC option.
Some devices that only supports Basic Mode do not understanding the RFC
option. This patch fix the regression found with that devices.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..0f34e12 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2527,6 +2527,10 @@ done:
 		if (pi->imtu != L2CAP_DEFAULT_MTU)
 			l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
 
+		if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
+				!(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
+			break;
+
 		rfc.mode            = L2CAP_MODE_BASIC;
 		rfc.txwin_size      = 0;
 		rfc.max_transmit    = 0;
@@ -2534,6 +2538,8 @@ done:
 		rfc.monitor_timeout = 0;
 		rfc.max_pdu_size    = 0;
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
 		break;
 
 	case L2CAP_MODE_ERTM:
@@ -2546,6 +2552,9 @@ done:
 		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
 			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
+
 		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
 			break;
 
@@ -2566,6 +2575,9 @@ done:
 		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
 			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
+
 		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
 			break;
 
@@ -2577,9 +2589,6 @@ done:
 		break;
 	}
 
-	l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
-						(unsigned long) &rfc);
-
 	/* FIXME: Need actual value of the flush timeout */
 	//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
 	//   l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
-- 
1.7.1.1

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Marcel Holtmann]

Hi Gustavo,

> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
>  net/bluetooth/l2cap.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)

patch has been applied. Thanks.

Regards

Marcel

Re: [PATCH] Bluetooth: Resend ConfigReq on unknown options failure

[Ville Tervo]

On 07/28/2010 10:16 AM, ext Gustavo F. Padovan wrote:
> Hi Ville,
>
> * Gustavo F. Padovan<gustavo@padovan.org>  [2010-07-28 04:11:20 -0300]:
>
>> From: Gustavo F. Padovan<padovan@profusion.mobi>
>>
>> If the remote device send a ConfigRsp with the unknown options failure we
>> should remove that option from the ConfigReq and send it again.
>> This patch only remove the RFC option in the case it is a unknown option.
>>
>> Signed-off-by: Gustavo F. Padovan<padovan@profusion.mobi>
>> ---
>>   include/net/bluetooth/l2cap.h |   17 +++++++++--------
>>   net/bluetooth/l2cap.c         |   28 ++++++++++++++++++++++++----
>>   2 files changed, 33 insertions(+), 12 deletions(-)
>
> Can you please test thi patch? I think it fix the unknown option
> problem.

No it didn't help. I'll try to find out why it still fails.

-- 
Ville

[PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Gustavo F. Padovan]

From: Gustavo F. Padovan <padovan@profusion.mobi>

If the remote side doesn't support Enhanced Retransmission Mode neither
Streaming Mode, we shall not send the RFC option.
Some devices that only supports Basic Mode do not understanding the RFC
option. This patch fix the regression found with that devices.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..0f34e12 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2527,6 +2527,10 @@ done:
 		if (pi->imtu != L2CAP_DEFAULT_MTU)
 			l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
 
+		if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
+				!(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
+			break;
+
 		rfc.mode            = L2CAP_MODE_BASIC;
 		rfc.txwin_size      = 0;
 		rfc.max_transmit    = 0;
@@ -2534,6 +2538,8 @@ done:
 		rfc.monitor_timeout = 0;
 		rfc.max_pdu_size    = 0;
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
 		break;
 
 	case L2CAP_MODE_ERTM:
@@ -2546,6 +2552,9 @@ done:
 		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
 			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
+
 		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
 			break;
 
@@ -2566,6 +2575,9 @@ done:
 		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
 			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
 
+		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+							(unsigned long) &rfc);
+
 		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
 			break;
 
@@ -2577,9 +2589,6 @@ done:
 		break;
 	}
 
-	l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
-						(unsigned long) &rfc);
-
 	/* FIXME: Need actual value of the flush timeout */
 	//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
 	//   l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
-- 
1.7.1.1

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Gustavo F. Padovan]

Hi Ville,

* Gustavo F. Padovan <gustavo@padovan.org> [2010-07-29 15:00:44 -0300]:

> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
>  net/bluetooth/l2cap.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)


Could you please test this patch, I'm pretty sure that it fixes the
issue with Nokia 9300.

> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 9ba1e8e..0f34e12 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2527,6 +2527,10 @@ done:
>  		if (pi->imtu != L2CAP_DEFAULT_MTU)
>  			l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
>  
> +		if (!(pi->conn->feat_mask & L2CAP_FEAT_ERTM) &&
> +				!(pi->conn->feat_mask & L2CAP_FEAT_STREAMING))
> +			break;
> +
>  		rfc.mode            = L2CAP_MODE_BASIC;
>  		rfc.txwin_size      = 0;
>  		rfc.max_transmit    = 0;
> @@ -2534,6 +2538,8 @@ done:
>  		rfc.monitor_timeout = 0;
>  		rfc.max_pdu_size    = 0;
>  
> +		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> +							(unsigned long) &rfc);
>  		break;
>  
>  	case L2CAP_MODE_ERTM:
> @@ -2546,6 +2552,9 @@ done:
>  		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
>  			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
>  
> +		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> +							(unsigned long) &rfc);
> +
>  		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
>  			break;
>  
> @@ -2566,6 +2575,9 @@ done:
>  		if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
>  			rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
>  
> +		l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> +							(unsigned long) &rfc);
> +
>  		if (!(pi->conn->feat_mask & L2CAP_FEAT_FCS))
>  			break;
>  
> @@ -2577,9 +2589,6 @@ done:
>  		break;
>  	}
>  
> -	l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
> -						(unsigned long) &rfc);
> -
>  	/* FIXME: Need actual value of the flush timeout */
>  	//if (flush_to != L2CAP_DEFAULT_FLUSH_TO)
>  	//   l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, pi->flush_to);
> -- 
> 1.7.1.1
> 

-- 
Gustavo F. Padovan
http://padovan.org

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Ville Tervo]

Hi,

On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> From: Gustavo F. Padovan<padovan@profusion.mobi>
>
> If the remote side doesn't support Enhanced Retransmission Mode neither
> Streaming Mode, we shall not send the RFC option.
> Some devices that only supports Basic Mode do not understanding the RFC
> option. This patch fix the regression found with that devices.


Yes this is better. After some research i found out that quite many old 
devices are not handling properly unknown options.

However I found another regression. And this kind of patch is needed 
also. Otherwise the info rsp code is reading feat_mask from failed 
response (and does invalid memory access).


diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0f34e12..1e174a4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3348,6 +3348,13 @@ static inline int l2cap_information_rsp(struct 
l2cap_conn *conn, struct l2cap_cm

         del_timer(&conn->info_timer);

+       if (result != L2CAP_IR_SUCCESS) {
+               conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
+               conn->info_ident = 0;
+               l2cap_conn_start(conn);
+               return 0;
+       }
+
         if (type == L2CAP_IT_FEAT_MASK) {
                 conn->feat_mask = get_unaligned_le32(rsp->data);



-- 
Ville

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Gustavo F. Padovan]

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [2010-07-30 16:13:00 +0300]:

> Hi,
> 
> On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> >From: Gustavo F. Padovan<padovan@profusion.mobi>
> >
> >If the remote side doesn't support Enhanced Retransmission Mode neither
> >Streaming Mode, we shall not send the RFC option.
> >Some devices that only supports Basic Mode do not understanding the RFC
> >option. This patch fix the regression found with that devices.
> 
> 
> Yes this is better. After some research i found out that quite many
> old devices are not handling properly unknown options.

Nice, we can put this upstream, do you agree?

> 
> However I found another regression. And this kind of patch is needed
> also. Otherwise the info rsp code is reading feat_mask from failed
> response (and does invalid memory access).
> 
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 0f34e12..1e174a4 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -3348,6 +3348,13 @@ static inline int
> l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> 
>         del_timer(&conn->info_timer);
> 
> +       if (result != L2CAP_IR_SUCCESS) {
> +               conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> +               conn->info_ident = 0;
> +               l2cap_conn_start(conn);
> +               return 0;
> +       }
> +
>         if (type == L2CAP_IT_FEAT_MASK) {
>                 conn->feat_mask = get_unaligned_le32(rsp->data);
> 

Ack. Send a proper GIT patch then we can push this fix. 


-- 
Gustavo F. Padovan
http://padovan.org

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Marcel Holtmann]

Hi Ville,

> > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > >
> > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > >Streaming Mode, we shall not send the RFC option.
> > >Some devices that only supports Basic Mode do not understanding the RFC
> > >option. This patch fix the regression found with that devices.
> > 
> > 
> > Yes this is better. After some research i found out that quite many
> > old devices are not handling properly unknown options.
> 
> Nice, we can put this upstream, do you agree?
> 
> > 
> > However I found another regression. And this kind of patch is needed
> > also. Otherwise the info rsp code is reading feat_mask from failed
> > response (and does invalid memory access).
> > 
> > 
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 0f34e12..1e174a4 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -3348,6 +3348,13 @@ static inline int
> > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> > 
> >         del_timer(&conn->info_timer);
> > 
> > +       if (result != L2CAP_IR_SUCCESS) {
> > +               conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > +               conn->info_ident = 0;
> > +               l2cap_conn_start(conn);
> > +               return 0;
> > +       }
> > +
> >         if (type == L2CAP_IT_FEAT_MASK) {
> >                 conn->feat_mask = get_unaligned_le32(rsp->data);
> > 
> 
> Ack. Send a proper GIT patch then we can push this fix. 

am I getting this proper patch or not?

Regards

Marcel

Re: [PATCH] Bluetooth: Don't send RFC for Basic Mode if only it is supported

[Ville Tervo]

On Wed, Aug 04, 2010 at 03:56:31AM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
> 
> > > On 07/29/2010 09:00 PM, ext Gustavo F. Padovan wrote:
> > > >From: Gustavo F. Padovan<padovan@profusion.mobi>
> > > >
> > > >If the remote side doesn't support Enhanced Retransmission Mode neither
> > > >Streaming Mode, we shall not send the RFC option.
> > > >Some devices that only supports Basic Mode do not understanding the RFC
> > > >option. This patch fix the regression found with that devices.
> > > 
> > > 
> > > Yes this is better. After some research i found out that quite many
> > > old devices are not handling properly unknown options.
> > 
> > Nice, we can put this upstream, do you agree?
> > 
> > > 
> > > However I found another regression. And this kind of patch is needed
> > > also. Otherwise the info rsp code is reading feat_mask from failed
> > > response (and does invalid memory access).
> > > 
> > > 
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 0f34e12..1e174a4 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -3348,6 +3348,13 @@ static inline int
> > > l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
> > > 
> > >         del_timer(&conn->info_timer);
> > > 
> > > +       if (result != L2CAP_IR_SUCCESS) {
> > > +               conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
> > > +               conn->info_ident = 0;
> > > +               l2cap_conn_start(conn);
> > > +               return 0;
> > > +       }
> > > +
> > >         if (type == L2CAP_IT_FEAT_MASK) {
> > >                 conn->feat_mask = get_unaligned_le32(rsp->data);
> > > 
> > 
> > Ack. Send a proper GIT patch then we can push this fix. 
> 
> am I getting this proper patch or not?

Yes.

-- 
Ville