2010-10-22 23:56:56

by Anderson Briglia

[permalink] [raw]
Subject: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

From: Vinicius Costa Gomes <[email protected]>

As L2CAP packets coming over LE don't have any more encapsulation,
other than L2CAP, we are able to process them as soon as they arrive.

Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/l2cap.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2bf083e..1ac44f4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4768,17 +4768,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
{
struct l2cap_conn *conn = hcon->l2cap_data;
+ struct l2cap_hdr *hdr;
+ int len;

if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
goto drop;

BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);

+ if (hcon->type == LE_LINK) {
+ hdr = (struct l2cap_hdr *) skb->data;
+ len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
+
+ if (len == skb->len) {
+ /* Complete frame received */
+ l2cap_recv_frame(conn, skb);
+ return 0;
+ }
+
+ goto drop;
+ }
+
if (flags & ACL_START) {
- struct l2cap_hdr *hdr;
struct sock *sk;
u16 cid;
- int len;

if (conn->rx_len) {
BT_ERR("Unexpected start frame (len %d)", skb->len);
--
1.7.0.4



2010-10-30 03:31:37

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

Hi Gustavo,

On Fri, Oct 29, 2010 at 5:50 PM, Gustavo F. Padovan
<[email protected]> wrote:
> Hi,
>
> * Vinicius Gomes <[email protected]> [2010-10-29 09:41:55 -0400]:
>
>> Hi Ville,
>>
>> On Fri, Oct 29, 2010 at 6:44 AM, Ville Tervo <[email protected]> wrote:
>> > Hi Anderson,
>> >
>> > On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
>> >> From: Vinicius Costa Gomes <[email protected]>
>> >>
>> >> As L2CAP packets coming over LE don't have any more encapsulation,
>> >> other than L2CAP, we are able to process them as soon as they arrive.
>> >
>> > Why is this change needed? Was something broken without this patch?
>> >
>>
>> This change is needed because without it the receiving side would
>> always think that it was receiving continuation frames.
>>
>> As the flags parameter is zero, it would fall into the "} else {"
>> condition, and because no frame was received before, conn->rx_len
>> would be zero and the frame would be discarded. Without this patch I
>> was seeing those "Unexpected continuation frame ..." messages on the
>> receiving side.
>
> From what I understood the flags are only for ACL connections and LE
> links doesn't have fragmentation, right? So we don't need to check flags
> here, actually it seems we don't have flags for LE like ACL.
>

Yeah, that is what the patch tries to solve. I was just trying to make
clear why the old code doesn't work (seems that I didn't do a good job
;-) .

> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
>


Cheers,
--
Vinicius

2010-10-29 20:50:33

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

Hi,

* Vinicius Gomes <[email protected]> [2010-10-29 09:41:55 -0400]:

> Hi Ville,
>
> On Fri, Oct 29, 2010 at 6:44 AM, Ville Tervo <[email protected]> wrote:
> > Hi Anderson,
> >
> > On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
> >> From: Vinicius Costa Gomes <[email protected]>
> >>
> >> As L2CAP packets coming over LE don't have any more encapsulation,
> >> other than L2CAP, we are able to process them as soon as they arrive.
> >
> > Why is this change needed? Was something broken without this patch?
> >
>
> This change is needed because without it the receiving side would
> always think that it was receiving continuation frames.
>
> As the flags parameter is zero, it would fall into the "} else {"
> condition, and because no frame was received before, conn->rx_len
> would be zero and the frame would be discarded. Without this patch I
> was seeing those "Unexpected continuation frame ..." messages on the
> receiving side.

>From what I understood the flags are only for ACL connections and LE
links doesn't have fragmentation, right? So we don't need to check flags
here, actually it seems we don't have flags for LE like ACL.

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-10-29 13:41:55

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

Hi Ville,

On Fri, Oct 29, 2010 at 6:44 AM, Ville Tervo <[email protected]> wrote:
> Hi Anderson,
>
> On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
>> From: Vinicius Costa Gomes <[email protected]>
>>
>> As L2CAP packets coming over LE don't have any more encapsulation,
>> other than L2CAP, we are able to process them as soon as they arrive.
>
> Why is this change needed? Was something broken without this patch?
>

This change is needed because without it the receiving side would
always think that it was receiving continuation frames.

As the flags parameter is zero, it would fall into the "} else {"
condition, and because no frame was received before, conn->rx_len
would be zero and the frame would be discarded. Without this patch I
was seeing those "Unexpected continuation frame ..." messages on the
receiving side.

>
>
>>
>> Signed-off-by: Vinicius Costa Gomes <[email protected]>
>> ---
>>  net/bluetooth/l2cap.c |   17 +++++++++++++++--
>>  1 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 2bf083e..1ac44f4 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -4768,17 +4768,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>>  static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>>  {
>>       struct l2cap_conn *conn = hcon->l2cap_data;
>> +     struct l2cap_hdr *hdr;
>> +     int len;
>>
>>       if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
>>               goto drop;
>>
>>       BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
>>
>> +     if (hcon->type == LE_LINK) {
>> +             hdr = (struct l2cap_hdr *) skb->data;
>> +             len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
>> +
>> +             if (len == skb->len) {
>> +                     /* Complete frame received */
>> +                     l2cap_recv_frame(conn, skb);
>> +                     return 0;
>> +             }
>> +
>> +             goto drop;
>> +     }
>> +
>>       if (flags & ACL_START) {
>> -             struct l2cap_hdr *hdr;
>>               struct sock *sk;
>>               u16 cid;
>> -             int len;
>>
>>               if (conn->rx_len) {
>>                       BT_ERR("Unexpected start frame (len %d)", skb->len);
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Ville
>


Cheers,
--
Vinicius

2010-10-29 10:44:35

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

Hi Anderson,

On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
> From: Vinicius Costa Gomes <[email protected]>
>
> As L2CAP packets coming over LE don't have any more encapsulation,
> other than L2CAP, we are able to process them as soon as they arrive.

Why is this change needed? Was something broken without this patch?



>
> Signed-off-by: Vinicius Costa Gomes <[email protected]>
> ---
> net/bluetooth/l2cap.c | 17 +++++++++++++++--
> 1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 2bf083e..1ac44f4 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -4768,17 +4768,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> {
> struct l2cap_conn *conn = hcon->l2cap_data;
> + struct l2cap_hdr *hdr;
> + int len;
>
> if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
> goto drop;
>
> BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
>
> + if (hcon->type == LE_LINK) {
> + hdr = (struct l2cap_hdr *) skb->data;
> + len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
> +
> + if (len == skb->len) {
> + /* Complete frame received */
> + l2cap_recv_frame(conn, skb);
> + return 0;
> + }
> +
> + goto drop;
> + }
> +
> if (flags & ACL_START) {
> - struct l2cap_hdr *hdr;
> struct sock *sk;
> u16 cid;
> - int len;
>
> if (conn->rx_len) {
> BT_ERR("Unexpected start frame (len %d)", skb->len);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Ville

2010-11-01 08:51:49

by Ville Tervo

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE

Hi,

On Fri, Oct 29, 2010 at 10:50:33PM +0200, ext Gustavo F. Padovan wrote:
> Hi,
>
> * Vinicius Gomes <[email protected]> [2010-10-29 09:41:55 -0400]:
>
> > Hi Ville,
> >
> > On Fri, Oct 29, 2010 at 6:44 AM, Ville Tervo <[email protected]> wrote:
> > > Hi Anderson,
> > >
> > > On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
> > >> From: Vinicius Costa Gomes <[email protected]>
> > >>
> > >> As L2CAP packets coming over LE don't have any more encapsulation,
> > >> other than L2CAP, we are able to process them as soon as they arrive.
> > >
> > > Why is this change needed? Was something broken without this patch?
> > >
> >
> > This change is needed because without it the receiving side would
> > always think that it was receiving continuation frames.
> >
> > As the flags parameter is zero, it would fall into the "} else {"
> > condition, and because no frame was received before, conn->rx_len
> > would be zero and the frame would be discarded. Without this patch I
> > was seeing those "Unexpected continuation frame ..." messages on the
> > receiving side.
>
> From what I understood the flags are only for ACL connections and LE
> links doesn't have fragmentation, right? So we don't need to check flags
> here, actually it seems we don't have flags for LE like ACL.

Yes it has. See spec. Vol 2 Part E 5.4.2.

It seems that controller should not be sending 0 PB flag in any situation
(except loopback). Vinicius that controller are you using?

I haven't seen this ever in my testing.

--
Ville