[PATCH v6 1/4] add one more buffer for stream reassembly

[PATCH v6 1/4] add one more buffer for stream reassembly

[Suraj Sumangala]

Additional reassembly buffer to keep track of stream reasembly

Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
 include/net/bluetooth/hci_core.h |    4 ++--
 net/bluetooth/hci_core.c         |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 600372d..10ad3ed 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -66,7 +66,7 @@ struct bdaddr_list {
 	struct list_head list;
 	bdaddr_t bdaddr;
 };
-
+#define NUM_REASSEMBLY_BUFFER 4
 struct hci_dev {
 	struct list_head list;
 	spinlock_t	lock;
@@ -123,7 +123,7 @@ struct hci_dev {
 	struct sk_buff_head	cmd_q;
 
 	struct sk_buff		*sent_cmd;
-	struct sk_buff		*reassembly[3];
+	struct sk_buff		*reassembly[NUM_REASSEMBLY_BUFFER];
 
 	struct mutex		req_lock;
 	wait_queue_head_t	req_wait_q;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index aeb2982..5df09fd 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -914,7 +914,7 @@ int hci_register_dev(struct hci_dev *hdev)
 	skb_queue_head_init(&hdev->cmd_q);
 	skb_queue_head_init(&hdev->raw_q);
 
-	for (i = 0; i < 3; i++)
+	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
 		hdev->reassembly[i] = NULL;
 
 	init_waitqueue_head(&hdev->req_wait_q);
@@ -973,7 +973,7 @@ int hci_unregister_dev(struct hci_dev *hdev)
 
 	hci_dev_do_close(hdev);
 
-	for (i = 0; i < 3; i++)
+	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
 		kfree_skb(hdev->reassembly[i]);
 
 	hci_notify(hdev, HCI_DEV_UNREG);
-- 
1.7.0.4

[PATCH v6 2/4] Implements hci_reassembly to reassemble Rx packets

[Suraj Sumangala]

Implements feature to reassemble received HCI frames from any input stream

Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/hci_core.c          |  109 +++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index ff77e8f..d6b150c 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -138,6 +138,7 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 struct bt_skb_cb {
 	__u8 pkt_type;
 	__u8 incoming;
+	__u16 expect;
 	__u8 tx_seq;
 	__u8 retries;
 	__u8 sar;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5df09fd..55d4f6d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1033,6 +1033,115 @@ int hci_recv_frame(struct sk_buff *skb)
 }
 EXPORT_SYMBOL(hci_recv_frame);
 
+static int hci_reassembly(struct hci_dev *hdev, int type, void *data,
+			  int count, __u8 index, gfp_t gfp_mask)
+{
+	int len = 0;
+	int hlen = 0;
+	int remain = count;
+	struct sk_buff *skb;
+	struct bt_skb_cb *scb;
+
+	if ((type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT) ||
+				index >= NUM_REASSEMBLY_BUFFER)
+		return -EILSEQ;
+
+	skb = hdev->reassembly[index];
+
+	if (!skb) {
+		switch (type) {
+		case HCI_ACLDATA_PKT:
+			len = HCI_MAX_FRAME_SIZE;
+			hlen = HCI_ACL_HDR_SIZE;
+			break;
+		case HCI_EVENT_PKT:
+			len = HCI_MAX_EVENT_SIZE;
+			hlen = HCI_EVENT_HDR_SIZE;
+			break;
+		case HCI_SCODATA_PKT:
+			len = HCI_MAX_SCO_SIZE;
+			hlen = HCI_SCO_HDR_SIZE;
+			break;
+		}
+
+		skb = bt_skb_alloc(len, gfp_mask);
+		if (!skb)
+			return -ENOMEM;
+
+		scb = (void *) skb->cb;
+		scb->expect = hlen;
+		scb->pkt_type = type;
+
+		skb->dev = (void *) hdev;
+		hdev->reassembly[index] = skb;
+	}
+
+	while (count) {
+		scb = (void *) skb->cb;
+		len = min(scb->expect, (__u16)count);
+
+		memcpy(skb_put(skb, len), data, len);
+
+		count -= len;
+		data += len;
+		scb->expect -= len;
+		remain = count;
+
+		switch (type) {
+		case HCI_EVENT_PKT:
+			if (skb->len == HCI_EVENT_HDR_SIZE) {
+				struct hci_event_hdr *h = hci_event_hdr(skb);
+				scb->expect = h->plen;
+
+				if (skb_tailroom(skb) < scb->expect) {
+					kfree_skb(skb);
+					hdev->reassembly[index] = NULL;
+					return -ENOMEM;
+				}
+			}
+			break;
+
+		case HCI_ACLDATA_PKT:
+			if (skb->len  == HCI_ACL_HDR_SIZE) {
+				struct hci_acl_hdr *h = hci_acl_hdr(skb);
+				scb->expect = __le16_to_cpu(h->dlen);
+
+				if (skb_tailroom(skb) < scb->expect) {
+					kfree_skb(skb);
+					hdev->reassembly[index] = NULL;
+					return -ENOMEM;
+				}
+			}
+			break;
+
+		case HCI_SCODATA_PKT:
+			if (skb->len == HCI_SCO_HDR_SIZE) {
+				struct hci_sco_hdr *h = hci_sco_hdr(skb);
+				scb->expect = h->dlen;
+
+				if (skb_tailroom(skb) < scb->expect) {
+					kfree_skb(skb);
+					hdev->reassembly[index] = NULL;
+					return -ENOMEM;
+				}
+			}
+			break;
+		}
+
+		if (scb->expect == 0) {
+			/* Complete frame */
+
+			bt_cb(skb)->pkt_type = type;
+			hci_recv_frame(skb);
+
+			hdev->reassembly[index] = NULL;
+			return remain;
+		}
+	}
+
+	return remain;
+}
+
 /* Receive packet type fragment */
 #define __reassembly(hdev, type)  ((hdev)->reassembly[(type) - 2])
 
-- 
1.7.0.4

[PATCH v6 3/4] Modified hci_recv_fragment() to use hci_reassembly

[Suraj Sumangala]

modified packet based reassembly function hci_recv_fragment() to use hci_reassembly()

Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
 net/bluetooth/hci_core.c |   85 ++++++----------------------------------------
 1 files changed, 11 insertions(+), 74 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 55d4f6d..eb13815 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1142,87 +1142,24 @@ static int hci_reassembly(struct hci_dev *hdev, int type, void *data,
 	return remain;
 }
 
-/* Receive packet type fragment */
-#define __reassembly(hdev, type)  ((hdev)->reassembly[(type) - 2])
-
 int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count)
 {
+	int rem = 0;
+
 	if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
 		return -EILSEQ;
 
-	while (count) {
-		struct sk_buff *skb = __reassembly(hdev, type);
-		struct { int expect; } *scb;
-		int len = 0;
-
-		if (!skb) {
-			/* Start of the frame */
-
-			switch (type) {
-			case HCI_EVENT_PKT:
-				if (count >= HCI_EVENT_HDR_SIZE) {
-					struct hci_event_hdr *h = data;
-					len = HCI_EVENT_HDR_SIZE + h->plen;
-				} else
-					return -EILSEQ;
-				break;
-
-			case HCI_ACLDATA_PKT:
-				if (count >= HCI_ACL_HDR_SIZE) {
-					struct hci_acl_hdr *h = data;
-					len = HCI_ACL_HDR_SIZE + __le16_to_cpu(h->dlen);
-				} else
-					return -EILSEQ;
-				break;
-
-			case HCI_SCODATA_PKT:
-				if (count >= HCI_SCO_HDR_SIZE) {
-					struct hci_sco_hdr *h = data;
-					len = HCI_SCO_HDR_SIZE + h->dlen;
-				} else
-					return -EILSEQ;
-				break;
-			}
-
-			skb = bt_skb_alloc(len, GFP_ATOMIC);
-			if (!skb) {
-				BT_ERR("%s no memory for packet", hdev->name);
-				return -ENOMEM;
-			}
-
-			skb->dev = (void *) hdev;
-			bt_cb(skb)->pkt_type = type;
-
-			__reassembly(hdev, type) = skb;
-
-			scb = (void *) skb->cb;
-			scb->expect = len;
-		} else {
-			/* Continuation */
-
-			scb = (void *) skb->cb;
-			len = scb->expect;
-		}
-
-		len = min(len, count);
-
-		memcpy(skb_put(skb, len), data, len);
-
-		scb->expect -= len;
-
-		if (scb->expect == 0) {
-			/* Complete frame */
+	do {
+		rem = hci_reassembly(hdev, type, data, count,
+						type - 1, GFP_ATOMIC);
+		if (rem < 0)
+			return rem;
 
-			__reassembly(hdev, type) = NULL;
+		data += (count - rem);
+		count = rem;
+	} while (count);
 
-			bt_cb(skb)->pkt_type = type;
-			hci_recv_frame(skb);
-		}
-
-		count -= len; data += len;
-	}
-
-	return 0;
+	return rem;
 }
 EXPORT_SYMBOL(hci_recv_fragment);
 
-- 
1.7.0.4

[PATCH v6 4/4] Implemented HCI frame reassembly for Rx from stream

[Suraj Sumangala]

Implemented frame reassembly implementation for reassembling fragments
received from stream.

Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/hci_core.c         |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 10ad3ed..a6453a8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -437,6 +437,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
 
 int hci_recv_frame(struct sk_buff *skb);
 int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
+int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
 
 int hci_register_sysfs(struct hci_dev *hdev);
 void hci_unregister_sysfs(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index eb13815..d8366f1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1163,6 +1163,41 @@ int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count)
 }
 EXPORT_SYMBOL(hci_recv_fragment);
 
+#define STREAM_BUF_INDEX 0
+
+int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
+{
+	int type;
+	int rem = 0;
+
+	do {
+		struct sk_buff *skb = hdev->reassembly[STREAM_BUF_INDEX];
+
+		if (!skb) {
+			struct { char type; } *pkt;
+
+			/* Start of the frame */
+			pkt = data;
+			type = pkt->type;
+
+			data++;
+			count--;
+		} else
+			type = bt_cb(skb)->pkt_type;
+
+		rem = hci_reassembly(hdev, type, data,
+					count, STREAM_BUF_INDEX, GFP_ATOMIC);
+		if (rem < 0)
+			return rem;
+
+		data += (count - rem);
+		count = rem;
+	} while (count);
+
+	return rem;
+}
+EXPORT_SYMBOL(hci_recv_stream_fragment);
+
 /* ---- Interface to upper protocols ---- */
 
 /* Register/Unregister protocols.
-- 
1.7.0.4

Re: [PATCH v6 1/4] add one more buffer for stream reassembly

[Marcel Holtmann]

Hi Suraj,

> Additional reassembly buffer to keep track of stream reasembly
> 
> Signed-off-by: Suraj Sumangala <suraj@atheros.com>
> ---
>  include/net/bluetooth/hci_core.h |    4 ++--
>  net/bluetooth/hci_core.c         |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 600372d..10ad3ed 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -66,7 +66,7 @@ struct bdaddr_list {
>  	struct list_head list;
>  	bdaddr_t bdaddr;
>  };
> -
> +#define NUM_REASSEMBLY_BUFFER 4

Just call it NUM_REASSEMBLY.

>  struct hci_dev {
>  	struct list_head list;
>  	spinlock_t	lock;
> @@ -123,7 +123,7 @@ struct hci_dev {
>  	struct sk_buff_head	cmd_q;
>  
>  	struct sk_buff		*sent_cmd;
> -	struct sk_buff		*reassembly[3];
> +	struct sk_buff		*reassembly[NUM_REASSEMBLY_BUFFER];
>  
>  	struct mutex		req_lock;
>  	wait_queue_head_t	req_wait_q;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index aeb2982..5df09fd 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -914,7 +914,7 @@ int hci_register_dev(struct hci_dev *hdev)
>  	skb_queue_head_init(&hdev->cmd_q);
>  	skb_queue_head_init(&hdev->raw_q);
>  
> -	for (i = 0; i < 3; i++)
> +	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
>  		hdev->reassembly[i] = NULL;
>  
>  	init_waitqueue_head(&hdev->req_wait_q);
> @@ -973,7 +973,7 @@ int hci_unregister_dev(struct hci_dev *hdev)
>  
>  	hci_dev_do_close(hdev);
>  
> -	for (i = 0; i < 3; i++)
> +	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
>  		kfree_skb(hdev->reassembly[i]);
>  
>  	hci_notify(hdev, HCI_DEV_UNREG);

And don't forget to modify the __reassembly macro to do -1 instead of -2
now. Since we said we keep the [0] for the stream reassembly.

Regards

Marcel

Re: [PATCH v6 1/4] add one more buffer for stream reassembly

[Gustavo F. Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2010-07-13 17:49:11 -0300]:

> Hi Suraj,
> 
> > Additional reassembly buffer to keep track of stream reasembly
> > 
> > Signed-off-by: Suraj Sumangala <suraj@atheros.com>
> > ---
> >  include/net/bluetooth/hci_core.h |    4 ++--
> >  net/bluetooth/hci_core.c         |    4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 600372d..10ad3ed 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -66,7 +66,7 @@ struct bdaddr_list {
> >  	struct list_head list;
> >  	bdaddr_t bdaddr;
> >  };
> > -
> > +#define NUM_REASSEMBLY_BUFFER 4
> 
> Just call it NUM_REASSEMBLY.
> 
> >  struct hci_dev {
> >  	struct list_head list;
> >  	spinlock_t	lock;
> > @@ -123,7 +123,7 @@ struct hci_dev {
> >  	struct sk_buff_head	cmd_q;
> >  
> >  	struct sk_buff		*sent_cmd;
> > -	struct sk_buff		*reassembly[3];
> > +	struct sk_buff		*reassembly[NUM_REASSEMBLY_BUFFER];
> >  
> >  	struct mutex		req_lock;
> >  	wait_queue_head_t	req_wait_q;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index aeb2982..5df09fd 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -914,7 +914,7 @@ int hci_register_dev(struct hci_dev *hdev)
> >  	skb_queue_head_init(&hdev->cmd_q);
> >  	skb_queue_head_init(&hdev->raw_q);
> >  
> > -	for (i = 0; i < 3; i++)
> > +	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
> >  		hdev->reassembly[i] = NULL;
> >  
> >  	init_waitqueue_head(&hdev->req_wait_q);
> > @@ -973,7 +973,7 @@ int hci_unregister_dev(struct hci_dev *hdev)
> >  
> >  	hci_dev_do_close(hdev);
> >  
> > -	for (i = 0; i < 3; i++)
> > +	for (i = 0; i < NUM_REASSEMBLY_BUFFER; i++)
> >  		kfree_skb(hdev->reassembly[i]);
> >  
> >  	hci_notify(hdev, HCI_DEV_UNREG);
> 
> And don't forget to modify the __reassembly macro to do -1 instead of -2
> now. Since we said we keep the [0] for the stream reassembly.

The __reassembly macro is removed in the following patches.

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

Re: [PATCH v6 1/4] add one more buffer for stream reassembly

[Marcel Holtmann]

Hi Gustavo,

> > And don't forget to modify the __reassembly macro to do -1 instead of -2
> > now. Since we said we keep the [0] for the stream reassembly.
> 
> The __reassembly macro is removed in the following patches.

I know, but I prefer to keep these patches in a bisectable fashion to
have them make sense by itself.

Regards

Marcel