* Re: Proposed MAP client D-Bus API (was: Proposed MAP API)
From: Johan Hedberg @ 2010-06-14 14:16 UTC (permalink / raw)
To: Counihan, Tom; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <B09AD526409DF14F8DA79CC5429052C28F7AAD48@irsmsx505.ger.corp.intel.com>
Hi Tom,
On Mon, Jun 14, 2010, Counihan, Tom wrote:
> Many thanks for the prompt response Johan,
No problem :)
> Not sure at all really. I had my suspicion, but wanted to start somewhere.
> What you suggest makes perfect sense really - back to the drawing
> board for me :$ - just need to figure out a logical home for this.
Note that I'm not strictly saying that it can't be D-Bus based. Only
that there's a considerable risk with that and that you should
understand the pros and cons of the different alternatives. E.g.
compared to D-Bus a library would restrict you more in terms of what
programing languages can be used for the application whereas there's
quite a plethora of D-Bus bindings out there. You'd also need to
consider potential multithreading issues and pay close attention in
designing the API so that it can be conveniently hooked up to whatever
mainloop the application has, or then simply decide that only one
mainloop, e.g. the glib one, is supported.
> On an aside, could the same comment be made regarding PBAP re contact
> pictures?
Yes, I think so.
Johan
^ permalink raw reply
* RE: Proposed MAP client D-Bus API (was: Proposed MAP API)
From: Counihan, Tom @ 2010-06-14 13:21 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20100614091849.GA20885@jh-x301>
Many thanks for the prompt response Johan,
>It'd have been good to clearly state that you're only talking about the
>client side of MAP (it became clear to me only after noticing that
>you've got org.openobex.client in the D-Bus API). The server side would
>e.g. be implemented through an obexd plugin.
Duly noted, apologies for any confusion. Note, I'm interested in bringing both sides of this interface up, the original was a modest attempt at kick starting myself on this profile.
>Are you sure you want to use D-Bus for this? I suspect the required
>bandwidth will be too high to be efficiently handled using D-Bus.
>Wouldn't a MAP specific library on top of OpenOBEX make more sense?
Not sure at all really. I had my suspicion, but wanted to start somewhere.
What you suggest makes perfect sense really - back to the drawing board for me :$ - just need to figure out a logical home for this.
On an aside, could the same comment be made regarding PBAP re contact pictures?
Warm Regards
Tom.
--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH 0/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-06-14 11:45 UTC (permalink / raw)
To: Marcel Holtmann, David S. Miller, Jiri Kosina, Michael Poole,
Bastien Nocera, Eric Dumazet, linux-bluetooth, linux-kernel,
netdev
In-Reply-To: <4C155945.1030500@signal11.us>
On 06/13/2010 06:18 PM, Alan Ott wrote:
> 3. A blocking, synchronous GET_REPORT transfer was easy when I
> implemented this for USB because data is both sent and received as
> part of a single control transfer. Because of the nature of Bluetooth
> however, where it is viewed more as an asynchronous network device,
> and with hidraw allowing multiple handles to a single device to exist,
> there could be a race when two handles call the hidp_get_raw_report()
> function concurrently, requesting the same report. I've convinced
> myself that this is not a problem, because since both callers
> requested the same report, the worst that could happen is that one
> could get a report which is slightly out of date.
>
> Consider the following case:
> 1. Client 1 requests report (Userspace call to HIDIOCGFEATURE)
> 2. Client 2 requests report (Userspace call to HIDIOCGFEATURE)
> 3. Client 1's report is returned, and delivered to BOTH clients
> 4. Client 2's report is returned (and discarded)
>
> Note here that Client 1's report and Client 2's report are the same
> report, ie: they reflect the state of the same data on the device,
> just at different times. In this case, they are indeed exactly the
> same data, but consider this case:
> 1. Client 1 requests report (Userspace call to HIDIOCGFEATURE)
> 2. Client 2 SETS report (Userspace call to HIDIOCSFEATURE)
> 2. Client 2 requests report (Userspace call to HIDIOCGFEATURE)
> 3. Client 1's report is returned, and delivered to Clients 1 and 2
> 4. Client 2's report is returned
>
> In this case, client 2 receives OLD data (since it set new data, and
> the call to write the reports is currently not synchronous). To make
> writes synchronous, we'd run into the same problem, of two writes
> happening concurrently, and the 2nd one receiving the ACK from the
> first one.
>
> Alan.
>
I just remembered to look at the hidraw.c source, to see that the call
to the hid_get_raw_report() function pointer (which points to
hidp_get_raw_report()) is called with a global mutex held. I believe
this will prevent the race mentioned in #3 above in the case that all
clients are communicating with the device using hidraw. Of course, the
situation above could still occur if one of the clients represents an
actual driver (which isn't subject to the hidraw mutex).
^ permalink raw reply
* Proposed MAP client D-Bus API (was: Proposed MAP API)
From: Johan Hedberg @ 2010-06-14 9:18 UTC (permalink / raw)
To: Counihan, Tom; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <B09AD526409DF14F8DA79CC5429052C28F7AAA6C@irsmsx505.ger.corp.intel.com>
Hi Tom,
On Mon, Jun 14, 2010, Counihan, Tom wrote:
> Please find below proposed Bluetooth MAP API, based upon Message
> Access Profile, ver 10r00.
It'd have been good to clearly state that you're only talking about the
client side of MAP (it became clear to me only after noticing that
you've got org.openobex.client in the D-Bus API). The server side would
e.g. be implemented through an obexd plugin.
> Comments most welcome.
> Of note below is missing the PushMessage function - Section 5.8 of
> mentioned spec.
> Reason being, I'm trying to get my head around what should be returned
> from a GetMessage call below, and conversely what should be passed to
> a potential PushMessage - bMessage can take on a number of different
> guises.
> Any guidance/opinion would be greatly appreciated in this area, or any
> other suggested improvements.
Are you sure you want to use D-Bus for this? I suspect the required
bandwidth will be too high to be efficiently handled using D-Bus.
Wouldn't a MAP specific library on top of OpenOBEX make more sense?
Johan
^ permalink raw reply
* Proposed MAP API
From: Counihan, Tom @ 2010-06-14 8:59 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
Hi Folks,
Please find below proposed Bluetooth MAP API, based upon Message Access Profile, ver 10r00.
Comments most welcome.
Of note below is missing the PushMessage function - Section 5.8 of mentioned spec.
Reason being, I'm trying to get my head around what should be returned from a GetMessage call below, and conversely what should be passed to a potential PushMessage - bMessage can take on a number of different guises.
Any guidance/opinion would be greatly appreciated in this area, or any other suggested improvements.
Warm Regards
Tom.
------------------------------------------
Message Access hierarchy
=======================
Service org.openobex.client
Interface org.openobex.MessageAccess
Object path [variable prefix]/{session0,session1,...}
Methods void SetFolder(string folder)
Change the current folder of the remote MSE device
dict GetProperties()
Returns all properties for the interface. See the
properties section for available properties.
array{string} GetFolderListing()
Returns a Folder-Listing containing information about
the current folder content. The return shall not contain
message entries.
array{dict} GetMessageListing()
Returns a dictionary containing information about
the current folder's Message content.
The following keys are defined:
uint64 Handle: Unique handle for message
string subject: summary of message
string datetime: timestamp in format "YYYMMDDTHHMMSS"
string senderName: name of sender
string senderAddressing: address info of sender
string replytoAddressing: Reply-To info of sender
string recipientName: name of recipient
string recipientAddressing: address info of recipient
string type: ("EMAIL" | "SMS_GSM" | "SMS_CDMA" | "MMS")
string size: Size in bytes of original message
string text: ("yes"|"no") - text vs binary content
string receptionStatus: reception status on MSE
string attachmentSize: size of attachment in bytes
string priority: ("yes"|"no") - high or no priority
string read: ("yes"|"no") has message been read on MSE
string sent: ("yes"|"no") has message been sent
string protected: ("yes"|"no") is all/part DRM protected
string GetMessage(uint64 handle)
Retrieves a messages, identified by unique handle from MSE.
void UpdateInbox()
Trigger the MSE to update its inbox.
void SetReadStatus(uint64 handle, string status)
Sets the messges's read status on the MSE
handle: unique handle for the message
status: "yes" sets message status to read
"no" sets message status to unread
void SetDeletStatus(uint64 handle, string status)
Sets the messges's delete status on the MSE
handle: unique handle for the message
status: "yes" sets message status to delete
"no" sets message status to non-delete
void EnableNotification()
Request the MSE to send message notification events
void DisableNotification()
Request the MSE to disable message notifications.
Signals MessageEventReport(dict)
Sent when an event on the MSE affects is message listing.
The following keys are defined:
string type: ("NewMessage" | " DeliverySuccess" |
"SendingSuccess" | "DeliveryFailure" |
"SendingFailure" | "MemoryFull" |
"MemoryAvailable" | "MessageDeleted" | "MessageShifted")
uint64 handle: Unique handle for message
string folder: name of folder, including path which message has been populated on MSE
string oldFolder: Facilitates moving msgs between folders
string msgType: ("EMAIL" | "SMS_GSM" | "SMS_CDMA" | "MMS")
Properties boolean NotificationEnabled [readonly]
Reflects if MSE has been directed to send message Notification
--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH v3 1/3] Implements hci_reassembly to reassemble Rx packets
From: suraj @ 2010-06-14 7:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1276088748.31226.6.camel@atheros013-desktop>
Hi Marcel,
On Wed, 2010-06-09 at 18:35 +0530, suraj wrote:
> Implements feature to reassemble received HCI frames from any input stream.
>
> Signed-off-by: Suraj Sumangala <suraj@atheros.com>
> ---
> net/bluetooth/hci_core.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 115 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5e83f8e..631a185 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1030,6 +1030,121 @@ 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, struct sk_buff **skb_ptr, int *remain)
> +{
> + int len = 0;
> + int header_len = 0;
> + struct sk_buff *skb = *skb_ptr;
> + struct { __u8 pkt_type; int expect; } *scb;
> +
> + *remain = count;
> +
> + if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
> + return -EILSEQ;
> +
> + if (!skb) {
> +
> + switch (type) {
> + case HCI_ACLDATA_PKT:
> + len = HCI_MAX_FRAME_SIZE;
> + header_len = HCI_ACL_HDR_SIZE;
> + break;
> + case HCI_EVENT_PKT:
> + len = HCI_MAX_EVENT_SIZE;
> + header_len = HCI_EVENT_HDR_SIZE;
> + break;
> + case HCI_SCODATA_PKT:
> + len = HCI_MAX_SCO_SIZE;
> + header_len = HCI_SCO_HDR_SIZE;
> + break;
> + }
> +
> + skb = bt_skb_alloc(len, GFP_ATOMIC);
> +
> + if (!skb)
> + return -ENOMEM;
> +
> + scb = (void *) skb->cb;
> + scb->expect = header_len;
> + scb->pkt_type = (__u8)type;
> +
> + skb->dev = (void *) hdev;
> + *skb_ptr = skb;
> +
> + }
> +
> + while (count) {
> +
> + scb = (void *) skb->cb;
> + len = min(scb->expect, 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);
> + *skb_ptr = 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);
> + *skb_ptr = 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);
> + *skb_ptr = NULL;
> +
> + return -ENOMEM;
> + }
> + }
> + break;
> + }
> +
> + if (scb->expect == 0) {
> +
> + /* Complete frame */
> + bt_cb(skb)->pkt_type = type;
> + hci_recv_frame(skb);
> +
> + *skb_ptr = NULL;
> +
> + return 0;
> + }
> +
> + }
> +
> + return 0;
> +}
> /* Receive packet type fragment */
> #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
>
Do let me know your comments on the patch.
Regards
Suraj
^ permalink raw reply
* [PATCH] Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state
From: Gustavo F. Padovan @ 2010-06-14 5:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo F. Padovan
In-Reply-To: <20100614005041.GB6390@vigoh>
From: Gustavo F. Padovan <padovan@profusion.mobi>
If such event happens we shall reply with a Command Reject, because we are
not expecting any configure request.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9753b69..f725fcc 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2741,8 +2741,14 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
if (!sk)
return -ENOENT;
- if (sk->sk_state == BT_DISCONN)
+ if (sk->sk_state != BT_CONFIG) {
+ struct l2cap_cmd_rej rej;
+
+ rej.reason = cpu_to_le16(0x0002);
+ l2cap_send_cmd(conn, cmd.ident, L2CAP_COMMAND_REJ,
+ sizeof(rej), &rej);
goto unlock;
+ }
/* Reject if config buffer is too small. */
len = cmd_len - sizeof(*req);
--
1.7.1
^ permalink raw reply related
* Re: odd cid change in l2cap negotiation
From: Gustavo F. Padovan @ 2010-06-14 0:50 UTC (permalink / raw)
To: Andrew Kohlsmith (mailing lists account); +Cc: linux-bluetooth
In-Reply-To: <201006131728.36687.aklists@mixdown.ca>
Hi Andrew,
* Andrew Kohlsmith (mailing lists account) <aklists@mixdown.ca> [2010-06-13 17:28:35 -0400]:
> Good afternoon,
>
> I'm working on a small embedded system with its own stack and using BlueZ on
> Ubuntu 9.04 (4.32 according to apt-cache).
>
> I am using a very stupid l2cap test program (listed below) to just open a
> connection to psm 0x1001 and send "Hello, World!".
>
> The embedded system receives the L2CAP connection request from BlueZ (scid of
> 0x0040) and responds by sending a configuration request. It uses dcid 0x0040.
> BlueZ responds to the configuration request with a successful configuration
> response, but it sets the scid to 0x0.000 and I can't for the life of me figure
> out why.
>
> The raw HCI packets (obtained from hcidump) are as follows:
>
> BlueZ connection request:
> 02 2a 20 0c 00 08 00 01 00 02 02 04 00 01 10 40 00
There is a missing connection response from the Embedded here. It sends
the configure response directly.
Bluez shouldn't even send a config request in this case, this is another
bug btw. Not related to yours.
>
> Embedded configure request:
> 02 2a 20 10 00 0c 00 01 00 04 02 08 00 40 00 00 00 01 02 96 00
>
> BlueZ configure response:
> 02 2a 20 12 00 0e 00 01 00 05 02 0a 00 00 00 00 00 00 00 01 02 96 00
>
> BlueZ configure request:
> 02 2a 20 0c 00 08 00 01 00 04 03 04 00 00 00 00 00
>
> As you can see, the connection request is using scid 0x0040 to psm 0x1001, and
> the configure request coming back to BlueZ is using dcid 0x0040. The BlueZ
> configure response (and subsequent request) are using channel id 0x0000. Why is
> this?
>
> If I patch out the embedded system's L2CAP handler code to accept the
> configuration response coming from the wrong cid BlueZ continues by sending the
> data packet ("Hello, World!" but again, to dcid 0x0000...
>
> Why is BlueZ flipping the channel ID like this? I am assuming that I am making
> a mistake somewhere as BlueZ gets a LOT more use than this embedded stack, but
> I can't figure out what this is happening for nor how to correctly work with
> it.
>
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: odd cid change in l2cap negotiation
From: Andrew Kohlsmith (mailing lists account) @ 2010-06-13 23:12 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <201006131728.36687.aklists@mixdown.ca>
On Sunday, June 13, 2010 05:28:35 pm Andrew Kohlsmith (mailing lists account)
wrote:
> I'm working on a small embedded system with its own stack and using BlueZ
> on Ubuntu 9.04 (4.32 according to apt-cache).
I should also note the kernel I'm running, as some discussion on the IRC
channel indicated that this stuff is actually in the kernel and not bluetoothd.
$ uname -a
Linux office.mixdown.ca 2.6.28-19-generic #61-Ubuntu SMP Wed May 26 23:32:46 UTC
2010 x86_64 GNU/Linux
-A.
^ permalink raw reply
* [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-06-13 22:20 UTC (permalink / raw)
To: Marcel Holtmann, David S Miller, Jiri Kosina, Michael Poole,
Bastien Nocera, Eric Dumazet, linux-bluetooth, linux-kernel,
netdev
Cc: Alan Ott
This patch adds support or getting and setting feature reports for bluetooth
HID devices from HIDRAW.
Signed-off-by: Alan Ott <alan@signal11.us>
---
net/bluetooth/hidp/core.c | 121 +++++++++++++++++++++++++++++++++++++++++++--
net/bluetooth/hidp/hidp.h | 8 +++
2 files changed, 125 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index bfe641b..0f068a0 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -36,6 +36,7 @@
#include <linux/file.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/mutex.h>
#include <net/sock.h>
#include <linux/input.h>
@@ -313,6 +314,93 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
return hidp_queue_report(session, buf, rsize);
}
+static int hidp_get_raw_report(struct hid_device *hid,
+ unsigned char report_number,
+ unsigned char *data, size_t count,
+ unsigned char report_type)
+{
+ struct hidp_session *session = hid->driver_data;
+ struct sk_buff *skb;
+ size_t len;
+ int numbered_reports = hid->report_enum[report_type].numbered;
+
+ switch (report_type) {
+ case HID_FEATURE_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_FEATURE;
+ break;
+ case HID_INPUT_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_INPUT;
+ break;
+ case HID_OUTPUT_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_OUPUT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (mutex_lock_interruptible(&session->report_mutex))
+ return -ERESTARTSYS;
+
+ /* Set up our wait, and send the report request to the device. */
+ session->waiting_report_type = report_type & HIDP_DATA_RTYPE_MASK;
+ session->waiting_report_number = numbered_reports ? report_number : -1;
+ set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ data[0] = report_number;
+ if (hidp_send_ctrl_message(hid->driver_data, report_type, data, 1))
+ goto err_eio;
+
+ /* Wait for the return of the report. The returned report
+ gets put in session->report_return. */
+ while (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
+ int res;
+
+ res = wait_event_interruptible_timeout(session->report_queue,
+ !test_bit(HIDP_WAITING_FOR_RETURN, &session->flags),
+ 5*HZ);
+ if (res == 0) {
+ /* timeout */
+ goto err_eio;
+ }
+ if (res < 0) {
+ /* signal */
+ goto err_restartsys;
+ }
+ }
+
+ skb = session->report_return;
+ if (skb) {
+ if (numbered_reports) {
+ /* Strip off the report number. */
+ size_t rpt_len = skb->len-1;
+ len = rpt_len < count ? rpt_len : count;
+ memcpy(data, skb->data+1, len);
+ } else {
+ len = skb->len < count ? skb->len : count;
+ memcpy(data, skb->data, len);
+ }
+
+ kfree_skb(skb);
+ session->report_return = NULL;
+ } else {
+ /* Device returned a HANDSHAKE, indicating protocol error. */
+ len = -EIO;
+ }
+
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+
+ return len;
+
+err_restartsys:
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+ return -ERESTARTSYS;
+err_eio:
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+ return -EIO;
+}
+
static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
unsigned char report_type)
{
@@ -367,6 +455,10 @@ static void hidp_process_handshake(struct hidp_session *session,
case HIDP_HSHK_ERR_INVALID_REPORT_ID:
case HIDP_HSHK_ERR_UNSUPPORTED_REQUEST:
case HIDP_HSHK_ERR_INVALID_PARAMETER:
+ if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ wake_up_interruptible(&session->report_queue);
+ }
/* FIXME: Call into SET_ GET_ handlers here */
break;
@@ -403,9 +495,11 @@ static void hidp_process_hid_control(struct hidp_session *session,
}
}
-static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
+/* Returns true if the passed-in skb should be freed by the caller. */
+static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
unsigned char param)
{
+ int done_with_skb = 1;
BT_DBG("session %p skb %p len %d param 0x%02x", session, skb, skb->len, param);
switch (param) {
@@ -417,7 +511,6 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
if (session->hid)
hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0);
-
break;
case HIDP_DATA_RTYPE_OTHER:
@@ -429,12 +522,27 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
__hidp_send_ctrl_message(session,
HIDP_TRANS_HANDSHAKE | HIDP_HSHK_ERR_INVALID_PARAMETER, NULL, 0);
}
+
+ if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags) &&
+ param == session->waiting_report_type) {
+ if (session->waiting_report_number < 0 ||
+ session->waiting_report_number == skb->data[0]) {
+ /* hidp_get_raw_report() is waiting on this report. */
+ session->report_return = skb;
+ done_with_skb = 0;
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ wake_up_interruptible(&session->report_queue);
+ }
+ }
+
+ return done_with_skb;
}
static void hidp_recv_ctrl_frame(struct hidp_session *session,
struct sk_buff *skb)
{
unsigned char hdr, type, param;
+ int free_skb = 1;
BT_DBG("session %p skb %p len %d", session, skb, skb->len);
@@ -454,7 +562,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
break;
case HIDP_TRANS_DATA:
- hidp_process_data(session, skb, param);
+ free_skb = hidp_process_data(session, skb, param);
break;
default:
@@ -463,7 +571,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
break;
}
- kfree_skb(skb);
+ if (free_skb)
+ kfree_skb(skb);
}
static void hidp_recv_intr_frame(struct hidp_session *session,
@@ -797,6 +906,7 @@ static int hidp_setup_hid(struct hidp_session *session,
hid->dev.parent = hidp_get_device(session);
hid->ll_driver = &hidp_hid_driver;
+ hid->hid_get_raw_report = hidp_get_raw_report;
hid->hid_output_raw_report = hidp_output_raw_report;
err = hid_add_device(hid);
@@ -857,6 +967,9 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
skb_queue_head_init(&session->ctrl_transmit);
skb_queue_head_init(&session->intr_transmit);
+ mutex_init(&session->report_mutex);
+ init_waitqueue_head(&session->report_queue);
+
session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID);
session->idle_to = req->idle_to;
diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
index 8d934a1..00e71dd 100644
--- a/net/bluetooth/hidp/hidp.h
+++ b/net/bluetooth/hidp/hidp.h
@@ -80,6 +80,7 @@
#define HIDP_VIRTUAL_CABLE_UNPLUG 0
#define HIDP_BOOT_PROTOCOL_MODE 1
#define HIDP_BLUETOOTH_VENDOR_ID 9
+#define HIDP_WAITING_FOR_RETURN 10
struct hidp_connadd_req {
int ctrl_sock; // Connected control socket
@@ -154,6 +155,13 @@ struct hidp_session {
struct sk_buff_head ctrl_transmit;
struct sk_buff_head intr_transmit;
+ /* Used in hidp_get_raw_report() */
+ int waiting_report_type; /* HIDP_DATA_RTYPE_* */
+ int waiting_report_number; /* -1 for not numbered */
+ struct mutex report_mutex;
+ struct sk_buff *report_return;
+ wait_queue_head_t report_queue;
+
/* Report descriptor */
__u8 *rd_data;
uint rd_size;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 0/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-06-13 22:18 UTC (permalink / raw)
To: Marcel Holtmann, David S. Miller, Jiri Kosina, Michael Poole,
Bastien Nocera, Eric Dumazet, linux-bluetooth, linux-kernel,
netdev
This patch adds support to the bluetooth hidp module for getting and
setting FEATURE reports from hidraw, as requested by Jiri Kosina. This
patch depends on the patch named:
[PATCH v2] HID: Add Support for Setting and Getting Feature Reports
from hidraw
I have a couple of concerns with this patch, which I hope someone here
can clarify and/or help me with.
1. Is it ok to use test_bit()/set_bit()/clear_bit() on session->flags,
when other parts in the code may not be using these functions to access
it? This currently isn't a problem because the other code which uses
flags only sets bits at initialization time (and deletion time). best I
can tell, flags is never actually used or read other than by my new code
(using the *_bit() functions). The solution here may be to change the
other code to use the *_bit() functions to access flags.
2. Is the loop in hidp_get_raw_report() sufficient without a mutex,
since I'm synchronizing with the atomic call to test_bit() (and
clear_bit())? I have convinced myself that in this case, with one
reader, and one writer, to one pointer, synchronized with
wait_event_interruptible_timeout() and atomic access through test_bit(),
that a mutex is not needed.
3. A blocking, synchronous GET_REPORT transfer was easy when I
implemented this for USB because data is both sent and received as part
of a single control transfer. Because of the nature of Bluetooth
however, where it is viewed more as an asynchronous network device, and
with hidraw allowing multiple handles to a single device to exist, there
could be a race when two handles call the hidp_get_raw_report() function
concurrently, requesting the same report. I've convinced myself that
this is not a problem, because since both callers requested the same
report, the worst that could happen is that one could get a report which
is slightly out of date.
Consider the following case:
1. Client 1 requests report (Userspace call to HIDIOCGFEATURE)
2. Client 2 requests report (Userspace call to HIDIOCGFEATURE)
3. Client 1's report is returned, and delivered to BOTH clients
4. Client 2's report is returned (and discarded)
Note here that Client 1's report and Client 2's report are the same
report, ie: they reflect the state of the same data on the device, just
at different times. In this case, they are indeed exactly the same data,
but consider this case:
1. Client 1 requests report (Userspace call to HIDIOCGFEATURE)
2. Client 2 SETS report (Userspace call to HIDIOCSFEATURE)
2. Client 2 requests report (Userspace call to HIDIOCGFEATURE)
3. Client 1's report is returned, and delivered to Clients 1 and 2
4. Client 2's report is returned
In this case, client 2 receives OLD data (since it set new data, and the
call to write the reports is currently not synchronous). To make writes
synchronous, we'd run into the same problem, of two writes happening
concurrently, and the 2nd one receiving the ACK from the first one.
The questions here are:
1. Is this a problem? It's only an issues if two handles (in two
separate threads) are reading and writing the device concurrently. I'd
expect that there would be bigger problems in this case than receiving
an old report.
2. If this is a problem, is there a way to synchronize on the control
socket for the device (as opposed to just this session)? In this case
GET_REPORT and SET_REPORT would lock access to the control socket (for
all clients accessing the device) while they are active.
Your feedback is most appreciated,
Alan.
^ permalink raw reply
* odd cid change in l2cap negotiation
From: Andrew Kohlsmith (mailing lists account) @ 2010-06-13 21:28 UTC (permalink / raw)
To: linux-bluetooth
Good afternoon,
I'm working on a small embedded system with its own stack and using BlueZ on
Ubuntu 9.04 (4.32 according to apt-cache).
I am using a very stupid l2cap test program (listed below) to just open a
connection to psm 0x1001 and send "Hello, World!".
The embedded system receives the L2CAP connection request from BlueZ (scid of
0x0040) and responds by sending a configuration request. It uses dcid 0x0040.
BlueZ responds to the configuration request with a successful configuration
response, but it sets the scid to 0x0000 and I can't for the life of me figure
out why.
The raw HCI packets (obtained from hcidump) are as follows:
BlueZ connection request:
02 2a 20 0c 00 08 00 01 00 02 02 04 00 01 10 40 00
Embedded configure request:
02 2a 20 10 00 0c 00 01 00 04 02 08 00 40 00 00 00 01 02 96 00
BlueZ configure response:
02 2a 20 12 00 0e 00 01 00 05 02 0a 00 00 00 00 00 00 00 01 02 96 00
BlueZ configure request:
02 2a 20 0c 00 08 00 01 00 04 03 04 00 00 00 00 00
As you can see, the connection request is using scid 0x0040 to psm 0x1001, and
the configure request coming back to BlueZ is using dcid 0x0040. The BlueZ
configure response (and subsequent request) are using channel id 0x0000. Why is
this?
If I patch out the embedded system's L2CAP handler code to accept the
configuration response coming from the wrong cid BlueZ continues by sending the
data packet ("Hello, World!" but again, to dcid 0x0000...
Why is BlueZ flipping the channel ID like this? I am assuming that I am making
a mistake somewhere as BlueZ gets a LOT more use than this embedded stack, but
I can't figure out what this is happening for nor how to correctly work with
it.
Regards,
Andrew
l2captest program:
$ cat tools/l2captest/l2capclient.c
#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <bluetooth/bluetooth.h>
#include <bluetooth/l2cap.h>
int main(int argc, char **argv)
{
struct sockaddr_l2 addr = { 0 };
int s, status;
char *message = "Hello, world!\n";
char dest[18] = "11:22:33:44:55:66";
if(argc < 2) {
fprintf(stderr, "usage: %s <bt_addr>\n", argv[0]);
return 1;
}
strncpy(dest, argv[1], 18);
// allocate a socket
s = socket(AF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
// set the connection parameters (who to connect to)
addr.l2_family = AF_BLUETOOTH;
addr.l2_psm = htobs(0x1001);
str2ba(dest, &addr.l2_bdaddr);
// connect to server
status = connect(s, (struct sockaddr *)&addr, sizeof(addr));
// send a message
if(status == 0) {
status = write(s, message, 15);
}
if( status < 0 ) perror("uh oh");
close(s);
return 0;
}
hcidump -XV -t output:
HCI sniffer - Bluetooth packet analyzer ver 1.42
device: hci0 snap_len: 1028 filter: 0xffffffffffffffff
2010-06-13 17:27:08.423268 < HCI Command: Create Connection (0x01|0x0005) plen
13
bdaddr 11:22:33:44:55:66 ptype 0xcc18 rswitch 0x01 clkoffset 0x0000
Packet type: DM1 DM3 DM5 DH1 DH3 DH5
2010-06-13 17:27:08.495324 > HCI Event: Command Status (0x0f) plen 4
Create Connection (0x01|0x0005) status 0x00 ncmd 1
2010-06-13 17:27:09.703325 > HCI Event: Connect Complete (0x03) plen 11
status 0x00 handle 42 bdaddr 11:22:33:44:55:66 type ACL encrypt 0x00
2010-06-13 17:27:09.703363 < HCI Command: Read Remote Supported Features
(0x01|0x001b) plen 2
handle 42
2010-06-13 17:27:09.740326 > HCI Event: Command Status (0x0f) plen 4
Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 0
2010-06-13 17:27:09.741324 > HCI Event: Page Scan Repetition Mode Change
(0x20) plen 7
bdaddr 11:22:33:44:55:66 mode 1
2010-06-13 17:27:09.742327 > HCI Event: Max Slots Change (0x1b) plen 3
handle 42 slots 5
2010-06-13 17:27:09.745327 > HCI Event: Command Status (0x0f) plen 4
Unknown (0x00|0x0000) status 0x00 ncmd 1
2010-06-13 17:27:09.745343 < HCI Command: Remote Name Request (0x01|0x0019)
plen 10
bdaddr 11:22:33:44:55:66 mode 2 clkoffset 0x0000
2010-06-13 17:27:09.749324 > HCI Event: Command Status (0x0f) plen 4
Remote Name Request (0x01|0x0019) status 0x00 ncmd 1
2010-06-13 17:27:09.754206 > HCI Event: Read Remote Supported Features (0x0b)
plen 11
status 0x00 handle 42
Features: 0xff 0xff 0x8f 0xfe 0x9b 0xf9 0x00 0x80
2010-06-13 17:27:09.754227 < ACL data: handle 42 flags 0x02 dlen 10
L2CAP(s): Info req: type 2
2010-06-13 17:27:09.759326 > HCI Event: Number of Completed Packets (0x13)
plen 5
handle 42 packets 1
2010-06-13 17:27:09.777323 > HCI Event: Remote Name Req Complete (0x07) plen
255
status 0x00 bdaddr 11:22:33:44:55:66 name 'inPulse'
2010-06-13 17:27:09.824448 > ACL data: handle 42 flags 0x02 dlen 12
L2CAP(s): Info rsp: type 2 result 1
Not supported
2010-06-13 17:27:09.824469 < ACL data: handle 42 flags 0x02 dlen 12
L2CAP(s): Connect req: psm 4097 scid 0x0040
2010-06-13 17:27:09.830326 > HCI Event: Number of Completed Packets (0x13)
plen 5
handle 42 packets 1
2010-06-13 17:27:10.195699 > ACL data: handle 42 flags 0x02 dlen 16
L2CAP(s): Config req: dcid 0x0040 flags 0x00 clen 4
MTU 150
2010-06-13 17:27:10.195724 < ACL data: handle 42 flags 0x02 dlen 18
L2CAP(s): Config rsp: scid 0x0000 flags 0x00 result 0 clen 4
MTU 150
2010-06-13 17:27:10.195728 < ACL data: handle 42 flags 0x02 dlen 12
L2CAP(s): Config req: dcid 0x0000 flags 0x00 clen 0
2010-06-13 17:27:10.201327 > HCI Event: Number of Completed Packets (0x13)
plen 5
handle 42 packets 1
2010-06-13 17:27:10.203324 > HCI Event: Number of Completed Packets (0x13)
plen 5
handle 42 packets 1
(after a long pause, it times out)
2010-06-13 17:27:50.422951 < HCI Command: Disconnect (0x01|0x0006) plen 3
handle 42 reason 0x13
Reason: Remote User Terminated Connection
2010-06-13 17:27:50.426338 > HCI Event: Command Status (0x0f) plen 4
Disconnect (0x01|0x0006) status 0x00 ncmd 1
2010-06-13 17:27:50.430344 > HCI Event: Disconn Complete (0x05) plen 4
status 0x00 handle 42 reason 0x16
Reason: Connection Terminated by Local Host
^ permalink raw reply
* Re: 2.6.35-rc3: Reported regressions 2.6.33 -> 2.6.34
From: Luis R. Rodriguez @ 2010-06-13 21:02 UTC (permalink / raw)
To: linux-wireless, linux-bluetooth
Cc: Rafael J. Wysocki, Aeolus Yang, David Quan
In-Reply-To: <g77CuMUl7QI.A.5wF.V5OFMB@chimera>
For your convenience in reading the regression report here's the
802.11 and Bluetooth ones. There's one PCMCIA one which I left in as I
think these patches now go through John (?) The other ones are 1
Bluetooth regression, 1 hostap_pci, 1 iwl3945 and 1 iwlagn regression.
Luis
On Sun, Jun 13, 2010 at 7:45 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Unresolved regressions
> ----------------------
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16111
> Subject : hostap_pci: infinite registered netdevice wifi0
> Submitter : Petr Pisar <petr.pisar@atlas.cz>
> Date : 2010-06-02 20:55 (12 days old)
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16084
> Subject : iwl3945 bug in 2.6.34
> Submitter : Satish Eerpini <eerpini@gmail.com>
> Date : 2010-05-23 6:37 (22 days old)
> Message-ID : <AANLkTik_7rxDBc0TKlAfoYyM5S6Cf_Hyxbr4W5ORnTsq@mail.gmail.com>
> References : http://marc.info/?l=linux-kernel&m=127459596015626&w=2
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15862
> Subject : 2.6.34-rc4/5: iwlagn unusable until reload
> Submitter : Nico Schottelius <nico-linux-20100427@schottelius.org>
> Date : 2010-04-27 7:49 (48 days old)
> Message-ID : <20100427074934.GB3261@ikn.schottelius.org>
> References : http://marc.info/?l=linux-kernel&m=127235784004839&w=2
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15717
> Subject : bluetooth oops
> Submitter : Pavel Machek <pavel@ucw.cz>
> Date : 2010-03-14 20:14 (92 days old)
> Message-ID : <20100314201434.GE22059@elf.ucw.cz>
> References : http://marc.info/?l=linux-kernel&m=126859771528426&w=4
> Handled-By : Marcel Holtmann <marcel@holtmann.org>
> Regressions with patches
> ------------------------
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16138
> Subject : PCMCIA regression
> Submitter : Mikulas Patocka <mpatocka@redhat.com>
> Date : 2010-05-25 20:25 (20 days old)
> Message-ID : <Pine.LNX.4.64.1005251619020.12278@hs20-bc2-1.build.redhat.com>
> References : http://marc.info/?l=linux-kernel&m=127481913909672&w=2
> Handled-By : Dominik Brodowski <linux@brodo.de>
> Patch : https://bugzilla.kernel.org/attachment.cgi?id=26685
^ permalink raw reply
* Re: Software caused connection abort (103 )
From: john michelle @ 2010-06-13 8:53 UTC (permalink / raw)
To: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <AANLkTin0B-f2i5e2TvDVDLuRZ7eRND-u4lVor1GSIhGT@mail.gmail.com>
Any updates on the software caused connection abort issue.
john
On Sun, May 30, 2010 at 1:12 PM, john michelle <jhnmichelle@gmail.com> wrote:
> Hi Gustavo,
>
>>> Please post the output of hcidump when the connection abort happens.
>>>
>
> Below is the output of hcidump the moments the software caused connection abort
>
> Dongle 1:
>> HCI Event: Number of Completed Packets (0x13) plen 5
> . * . . .
> < HCI Command: Reset (0x03|0x0003) plen 0
> device: disconnected
>
> Dongle 2:
>> HCI Event: Number of Completed Packets (0x13) plen 5
> . * . . .
> < HCI Command: Reset (0x03|0x0003) plen 0
> device: disconnected
>
>
> I think something triggers hci reset which causes this problem, but what exactly
> Triggers it i don't know.
>
>
> John
>
>
> On Mon, May 17, 2010 at 5:21 PM, john michelle <jhnmichelle@gmail.com> wrote:
>>> Hi Jonh,
>>>
>>> First of all, don't do top posting in this mailing list. ;)
>>>
>>
>> Thanks for the advise , will keep that in mind for future posts.
>>
>>> In which bluez/kernel version the problem started? Any hardware update
>>> during this time? The problem happens when you are already tranfering
>>> SCO data?
>>>
>>
>> I tried bluez versions 3.XX and kernel 2.6.26(I think) the problem
>> occurs and the kernel panics with no core dumps.then i tried with
>> bluez 4.53-4.62 with
>> Kernel 2.6.31 and 2.6.32. the connection abort problem occurs but the
>> kernel doesn't
>> Panic. regarding the hardware update you mean the dongle or the box.Anyway
>> i changed both and tried different box's, this even happens on a box
>> with 2 giga ram
>> And core2 processor . as for the dongles i am using trust
>>
>> http://www.twenga.co.uk/prices-Bluetooth-2-USB-Adapter-10m-BT-2250p-TRUST-Wireless-network-card-adapter-176178-0
>>
>> and also using no name dongles all the same problem.
>>
>>> Please post the output of hcidump when the connection abort happens.
>>>
>>
>> well that is going to be a hard one since i have more than one dongle in place
>> And very hard to predict which one will crash.i will work on this and update
>> You as soon as i have the hcidump log.
>>
>>
>>
>>> I tried reproduce this issue in L2CAP but it is a bit hard, lets
>>> say it happen once in a thousand, so it's not easy to track it. I have
>>> to try reproduce that using the SCO, but I'm not used to that layer yet.
>>> ;)
>>
>> I hoped that it happens 1 in 1000 in SCO , but it actually happens 1 in 15
>>
>>
>> John
>>
>>
>>
>>>
>>> Regards,
>>>
>>>>
>>>> John
>>>>
>>>> On Fri, May 7, 2010 at 3:53 PM, Gustavo F. Padovan <gustavo@padovan.org> wrote:
>>>> > * john michelle <jhnmichelle@gmail.com> [2010-05-07 15:49:05 -0400]:
>>>> >
>>>> >> Hi Bluetooth hackers,
>>>> >>
>>>> >> i am having this consistent problem with bluez , from time to time i
>>>> >> get the error Software caused connection abort (103 )
>>>> >> And the bluetooth stick seems to disconnect and reconnects. this
>>>> >> happens during an sco connection and it occurs even
>>>> >> More when i am having the voice stream comming through the internet
>>>> >> rather than the lan.i don't know what exact
>>>> >> Details you need to solve this problem please tell me and i will give
>>>> >> you an immediate reply . i have linux kernel 2.6.33 and bluez 4.62
>>>> >
>>>> > I get the same problem sometimes when testing ERTM with l2test. So the
>>>> > problem is on l2cap too. I didn't have time to debug this yet. Now that
>>>> > someone else have confirmed it too, I'll to take a look at the problem.
>>>> >
>>>> >>
>>>
>>> --
>>> Gustavo F. Padovan
>>> http://padovan.org
>>>
>>
>
^ permalink raw reply
* Re: bug and possible patch for dbus DiscoverServices
From: Johan Hedberg @ 2010-06-11 15:46 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Tim Renouf, linux-bluetooth
In-Reply-To: <AANLkTilSgydF3eH61rsSZlHILBeW7Yoo-VtpcLzgQUzt@mail.gmail.com>
Hi Luiz,
On Fri, Jun 11, 2010, Luiz Augusto von Dentz wrote:
> There were some problems with the changes I suggested previously,
> basically device_probe_drivers was freeing there records before we
> return. The following patches should fix this issue and another one
> with btd_device_get_record not looking into records storage when it is
> missing on temporary cache.
Both patches seem fine me (I hope you tested them too) and have now been
pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: bug and possible patch for dbus DiscoverServices
From: Luiz Augusto von Dentz @ 2010-06-11 9:41 UTC (permalink / raw)
To: Tim Renouf; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinQ_Tr5fwNquGwW9o5LIuqpAGCqHveGuAA1Av3O@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]
Hi,
On Fri, Jun 11, 2010 at 1:19 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Thu, Jun 10, 2010 at 11:20 PM, Tim Renouf <tpr.vger@botech.co.uk> wrote:
>> On Thu, Jun 10, 2010 at 04:48:58PM +0300, Luiz Augusto von Dentz wrote:
>>> I would just rework it a bit to make the tmp_records to always store
>>> the last search, something like the following:
>>>
>>> - if (device->tmp_records && req->records) {
>>> + if (device->tmp_records)
>>> sdp_list_free(device->tmp_records,
>>> (sdp_free_func_t) sdp_record_free);
>>> - device->tmp_records = req->records;
>>> - req->records = NULL;
>>> - }
>>> +
>>> + device->tmp_records = req->records;
>>> + req->records = NULL;
>>>
>>> if (!req->profiles_added && !req->profiles_removed) {
>>> DBG("%s: No service update", device->path);
>>> @@ -1522,7 +1522,7 @@ send_reply:
>>>
>>> if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
>>> "DiscoverServices"))
>>> - discover_services_reply(req, err, req->records);
>>> + discover_services_reply(req, err, device->tmp_records);
>>> else if (dbus_message_is_method_call(req->msg, ADAPTER_INTERFACE,
>>> "CreatePairedDevice"))
>>> create_device_reply(device, req);
>>>
>>> What do you think?
>>
>> Thanks, that looks good, I'll use that.
>>
>> What is the procedure to get it included in future versions of BlueZ ?
>> Does someone sweep this list for patches to review and possibly
>> include ?
>
> Similar to kernel. git format-patch or git pull request, be sure that
> it applies and compiles cleanly against git master.
>
> --
> Luiz Augusto von Dentz
> Computer Engineer
>
There were some problems with the changes I suggested previously,
basically device_probe_drivers was freeing there records before we
return. The following patches should fix this issue and another one
with btd_device_get_record not looking into records storage when it is
missing on temporary cache.
--
Luiz Augusto von Dentz
Computer Engineer
[-- Attachment #2: 0001-Fix-DiscoverServices-not-retrieving-any-records.patch --]
[-- Type: text/x-patch, Size: 2007 bytes --]
From 25982ed3099f68de120ddf8b23086e3ac289ebd6 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Date: Fri, 11 Jun 2010 12:24:06 +0300
Subject: [PATCH 01/10] Fix DiscoverServices not retrieving any records
In the case that device->tmp_records was already set, req->records was
being set to NULL and so causes the empty array to be returned.
To fix that tmp_records now always has the latest discovered records and
it is used when repling to DiscoverServices.
Thanks to Tim Renouf <tpr.vger@botech.co.uk> for figuring out where was
the problem.
---
| 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
--git a/src/device.c b/src/device.c
index 5584de9..0863a79 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1267,12 +1267,6 @@ add_uuids:
g_strdup(list->data),
(GCompareFunc) strcasecmp);
}
-
- if (device->tmp_records) {
- sdp_list_free(device->tmp_records,
- (sdp_free_func_t) sdp_record_free);
- device->tmp_records = NULL;
- }
}
static void device_remove_drivers(struct btd_device *device, GSList *uuids)
@@ -1493,12 +1487,12 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
update_services(req, recs);
- if (device->tmp_records && req->records) {
+ if (device->tmp_records)
sdp_list_free(device->tmp_records,
(sdp_free_func_t) sdp_record_free);
- device->tmp_records = req->records;
- req->records = NULL;
- }
+
+ device->tmp_records = req->records;
+ req->records = NULL;
if (!req->profiles_added && !req->profiles_removed) {
DBG("%s: No service update", device->path);
@@ -1522,7 +1516,7 @@ send_reply:
if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
"DiscoverServices"))
- discover_services_reply(req, err, req->records);
+ discover_services_reply(req, err, device->tmp_records);
else if (dbus_message_is_method_call(req->msg, ADAPTER_INTERFACE,
"CreatePairedDevice"))
create_device_reply(device, req);
--
1.7.0.4
[-- Attachment #3: 0002-Fix-not-looking-into-storage-when-a-record-is-not-fo.patch --]
[-- Type: text/x-patch, Size: 879 bytes --]
From 40afc7d29c1a56be67e7a117c22d6defb83f9e5e Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Date: Fri, 11 Jun 2010 12:12:12 +0300
Subject: [PATCH 02/10] Fix not looking into storage when a record is not found in memory
---
| 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
--git a/src/device.c b/src/device.c
index 0863a79..6c085a3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2415,8 +2415,13 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
{
bdaddr_t src;
- if (device->tmp_records)
- return find_record_in_list(device->tmp_records, uuid);
+ if (device->tmp_records) {
+ const sdp_record_t *record;
+
+ record = find_record_in_list(device->tmp_records, uuid);
+ if (record != NULL)
+ return record;
+ }
adapter_get_address(device->adapter, &src);
--
1.7.0.4
^ permalink raw reply related
* Re: bug and possible patch for dbus DiscoverServices
From: Luiz Augusto von Dentz @ 2010-06-10 22:19 UTC (permalink / raw)
To: Tim Renouf; +Cc: linux-bluetooth
In-Reply-To: <20100610202023.GC16611@botech.co.uk>
Hi,
On Thu, Jun 10, 2010 at 11:20 PM, Tim Renouf <tpr.vger@botech.co.uk> wrote:
> On Thu, Jun 10, 2010 at 04:48:58PM +0300, Luiz Augusto von Dentz wrote:
>> I would just rework it a bit to make the tmp_records to always store
>> the last search, something like the following:
>>
>> - if (device->tmp_records && req->records) {
>> + if (device->tmp_records)
>> sdp_list_free(device->tmp_records,
>> (sdp_free_func_t) sdp_record_free);
>> - device->tmp_records = req->records;
>> - req->records = NULL;
>> - }
>> +
>> + device->tmp_records = req->records;
>> + req->records = NULL;
>>
>> if (!req->profiles_added && !req->profiles_removed) {
>> DBG("%s: No service update", device->path);
>> @@ -1522,7 +1522,7 @@ send_reply:
>>
>> if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
>> "DiscoverServices"))
>> - discover_services_reply(req, err, req->records);
>> + discover_services_reply(req, err, device->tmp_records);
>> else if (dbus_message_is_method_call(req->msg, ADAPTER_INTERFACE,
>> "CreatePairedDevice"))
>> create_device_reply(device, req);
>>
>> What do you think?
>
> Thanks, that looks good, I'll use that.
>
> What is the procedure to get it included in future versions of BlueZ ?
> Does someone sweep this list for patches to review and possibly
> include ?
Similar to kernel. git format-patch or git pull request, be sure that
it applies and compiles cleanly against git master.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: bug and possible patch for dbus DiscoverServices
From: Tim Renouf @ 2010-06-10 20:20 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <AANLkTinlsskks35CHsSlDueAGCi4HeicWjHLUTQcfzYC@mail.gmail.com>
On Thu, Jun 10, 2010 at 04:48:58PM +0300, Luiz Augusto von Dentz wrote:
> I would just rework it a bit to make the tmp_records to always store
> the last search, something like the following:
>
> - if (device->tmp_records && req->records) {
> + if (device->tmp_records)
> sdp_list_free(device->tmp_records,
> (sdp_free_func_t) sdp_record_free);
> - device->tmp_records = req->records;
> - req->records = NULL;
> - }
> +
> + device->tmp_records = req->records;
> + req->records = NULL;
>
> if (!req->profiles_added && !req->profiles_removed) {
> DBG("%s: No service update", device->path);
> @@ -1522,7 +1522,7 @@ send_reply:
>
> if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
> "DiscoverServices"))
> - discover_services_reply(req, err, req->records);
> + discover_services_reply(req, err, device->tmp_records);
> else if (dbus_message_is_method_call(req->msg, ADAPTER_INTERFACE,
> "CreatePairedDevice"))
> create_device_reply(device, req);
>
> What do you think?
Thanks, that looks good, I'll use that.
What is the procedure to get it included in future versions of BlueZ ?
Does someone sweep this list for patches to review and possibly
include ?
Thanks.
-tpr
^ permalink raw reply
* Re: bug and possible patch for dbus DiscoverServices
From: Luiz Augusto von Dentz @ 2010-06-10 13:48 UTC (permalink / raw)
To: Tim Renouf; +Cc: linux-bluetooth
In-Reply-To: <20100610125848.GA28631@botech.co.uk>
Hi,
On Thu, Jun 10, 2010 at 3:58 PM, Tim Renouf <tpr.vger@botech.co.uk> wrote:
> Hi Bluetooth people
>
> I am investigating a bug where dbus DiscoverServices incorrectly returns
> an empty array in some circumstances (see below).
>
> Here is my suggested patch, against BlueZ 4.47 (I don't think there are
> any more recent changes in this area):
>
> ====
> diff -ur ../eclair.old/external/bluetooth/bluez/src/device.c external/bluetooth/bluez/src/device.c
> --- ../eclair.old/external/bluetooth/bluez/src/device.c 2009-12-15 13:15:44.000000000 +0000
> +++ external/bluetooth/bluez/src/device.c 2010-06-10 11:31:04.000000000 +0100
> @@ -1380,6 +1380,7 @@
> {
> struct browse_req *req = user_data;
> struct btd_device *device = req->device;
> + sdp_list_t *records = 0;
> DBusMessage *reply;
>
> if (err < 0) {
> @@ -1389,6 +1390,7 @@
> }
>
> update_services(req, recs);
> + records = req->records;
>
> if (device->tmp_records && req->records) {
> sdp_list_free(device->tmp_records,
> @@ -1422,7 +1424,7 @@
>
> if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
> "DiscoverServices")) {
> - discover_services_reply(req, err, req->records);
> + discover_services_reply(req, err, records);
> goto cleanup;
> }
>
> ====
>
> The problem seems to arise in this bit of search_cb() at device.c:1395:
>
> if (device->tmp_records && req->records) {
> sdp_list_free(device->tmp_records,
> (sdp_free_func_t) sdp_record_free);
> device->tmp_records = req->records;
> req->records = NULL;
> }
>
> In the case that device->tmp_records was already set, this zeroes
> req->records and so causes the empty array to be returned via dbus a bit
> lower down in the function.
Good catch, I've seems this sometimes but I was never really able to
reproduce it consistently but this seems definably the source of the
problem.
> I'm not really sure what tmp_records does, and whether it should be
> non-zero at this point. So it could be either
>
> 1. tmp_records could be non-zero, so search_cb should return the results
> even if the above code kicks in and zeroes req->records (that's what my
> patch assumes); or
>
> 2. tmp_records is supposed to be zero there, so there is a bug
> elsewhere. In particular, I have only seen the bug on Android phones,
> not on my PC, and a bit of investigation showed that it is some Android
> specific code (the new dbus method GetServiceAttributeValue; see the
> device.c here:
> http://android.git.kernel.org/?p=platform/external/bluez.git;a=blob;f=src/device.c;h=67a77318ad136c12cad96f0e537f8ceac8ae62f0;hb=refs/heads/eclair-bluez
> line 789) that sets tmp_records and does not clear it again.
>
> Also, the bug does not appear when the device I am trying to discover
> services on is my PC rather than a phone. I suspect the Android
> Bluetooth startup sequence only uses this GetServiceAttributeValue on a
> remote device with some phone-specific service, so the
> device->tmp_records for my PC remains at 0.
>
> I would be grateful for any hints on what tmp_records does, and whether
> my patch is correct or whether I should be looking elsewhere.
I would just rework it a bit to make the tmp_records to always store
the last search, something like the following:
- if (device->tmp_records && req->records) {
+ if (device->tmp_records)
sdp_list_free(device->tmp_records,
(sdp_free_func_t) sdp_record_free);
- device->tmp_records = req->records;
- req->records = NULL;
- }
+
+ device->tmp_records = req->records;
+ req->records = NULL;
if (!req->profiles_added && !req->profiles_removed) {
DBG("%s: No service update", device->path);
@@ -1522,7 +1522,7 @@ send_reply:
if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
"DiscoverServices"))
- discover_services_reply(req, err, req->records);
+ discover_services_reply(req, err, device->tmp_records);
else if (dbus_message_is_method_call(req->msg, ADAPTER_INTERFACE,
"CreatePairedDevice"))
create_device_reply(device, req);
What do you think?
The point of tmp_records is to avoid having to access the storage when
the records are still in memory (we just discovered them).
Regards,
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* bug and possible patch for dbus DiscoverServices
From: Tim Renouf @ 2010-06-10 12:58 UTC (permalink / raw)
To: linux-bluetooth
Hi Bluetooth people
I am investigating a bug where dbus DiscoverServices incorrectly returns
an empty array in some circumstances (see below).
Here is my suggested patch, against BlueZ 4.47 (I don't think there are
any more recent changes in this area):
====
diff -ur ../eclair.old/external/bluetooth/bluez/src/device.c external/bluetooth/bluez/src/device.c
--- ../eclair.old/external/bluetooth/bluez/src/device.c 2009-12-15 13:15:44.000000000 +0000
+++ external/bluetooth/bluez/src/device.c 2010-06-10 11:31:04.000000000 +0100
@@ -1380,6 +1380,7 @@
{
struct browse_req *req = user_data;
struct btd_device *device = req->device;
+ sdp_list_t *records = 0;
DBusMessage *reply;
if (err < 0) {
@@ -1389,6 +1390,7 @@
}
update_services(req, recs);
+ records = req->records;
if (device->tmp_records && req->records) {
sdp_list_free(device->tmp_records,
@@ -1422,7 +1424,7 @@
if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
"DiscoverServices")) {
- discover_services_reply(req, err, req->records);
+ discover_services_reply(req, err, records);
goto cleanup;
}
====
The problem seems to arise in this bit of search_cb() at device.c:1395:
if (device->tmp_records && req->records) {
sdp_list_free(device->tmp_records,
(sdp_free_func_t) sdp_record_free);
device->tmp_records = req->records;
req->records = NULL;
}
In the case that device->tmp_records was already set, this zeroes
req->records and so causes the empty array to be returned via dbus a bit
lower down in the function.
I'm not really sure what tmp_records does, and whether it should be
non-zero at this point. So it could be either
1. tmp_records could be non-zero, so search_cb should return the results
even if the above code kicks in and zeroes req->records (that's what my
patch assumes); or
2. tmp_records is supposed to be zero there, so there is a bug
elsewhere. In particular, I have only seen the bug on Android phones,
not on my PC, and a bit of investigation showed that it is some Android
specific code (the new dbus method GetServiceAttributeValue; see the
device.c here:
http://android.git.kernel.org/?p=platform/external/bluez.git;a=blob;f=src/device.c;h=67a77318ad136c12cad96f0e537f8ceac8ae62f0;hb=refs/heads/eclair-bluez
line 789) that sets tmp_records and does not clear it again.
Also, the bug does not appear when the device I am trying to discover
services on is my PC rather than a phone. I suspect the Android
Bluetooth startup sequence only uses this GetServiceAttributeValue on a
remote device with some phone-specific service, so the
device->tmp_records for my PC remains at 0.
I would be grateful for any hints on what tmp_records does, and whether
my patch is correct or whether I should be looking elsewhere.
Thanks.
-tpr
^ permalink raw reply
* Re: [PATCH] Bluetooth: remove ERTM minimum packet length
From: Gustavo F. Padovan @ 2010-06-09 22:24 UTC (permalink / raw)
To: Nathan Holstein; +Cc: linux-bluetooth
In-Reply-To: <1276112785.3546.560.camel@strawberry>
Hi Nathan,
* Nathan Holstein <nathan@lampreynetworks.com> [2010-06-09 15:46:25 -0400]:
> ERTM and streaming mode L2CAP sockets have no minimum packet length.
> Instead, validate the the packet contained all necessary control, FCS,
> and SAR fields.
>
> Signed-off-by: Nathan Holstein <ngh@isomerica.net>
> ---
> net/bluetooth/l2cap.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
Patch has been applied to the master branch of my tree. Thanks.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* [PATCH] Bluetooth: remove ERTM minimum packet length
From: Nathan Holstein @ 2010-06-09 19:46 UTC (permalink / raw)
To: linux-bluetooth
ERTM and streaming mode L2CAP sockets have no minimum packet length.
Instead, validate the the packet contained all necessary control, FCS,
and SAR fields.
Signed-off-by: Nathan Holstein <ngh@isomerica.net>
---
net/bluetooth/l2cap.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 411b1ec..9099e6d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4131,9 +4131,9 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
{
struct sock *sk;
struct l2cap_pinfo *pi;
- u16 control, len;
+ u16 control;
u8 tx_seq, req_seq;
- int next_tx_seq_offset, req_seq_offset;
+ int len, next_tx_seq_offset, req_seq_offset;
sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
if (!sk) {
@@ -4203,7 +4203,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
}
if (__is_iframe(control)) {
- if (len < 4) {
+ if (len < 0) {
l2cap_send_disconn_req(pi->conn, sk);
goto drop;
}
@@ -4234,7 +4234,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (pi->fcs == L2CAP_FCS_CRC16)
len -= 2;
- if (len > pi->mps || len < 4 || __is_sframe(control))
+ if (len > pi->mps || len < 0 || __is_sframe(control))
goto drop;
tx_seq = __get_txseq(control);
--
1.6.0.4
^ permalink raw reply related
* [PATCH v3 3/3] Implemented HCI frame reassembly for Rx from stream
From: suraj @ 2010-06-09 13:13 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1276088968.31226.10.camel@atheros013-desktop>
Implemented frame reassembly implementation for reassembling fragments received from stream.
Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_core.c | 43 ++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e42f6ed..cd89d66 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -119,6 +119,7 @@ struct hci_dev {
struct sk_buff *sent_cmd;
struct sk_buff *reassembly[3];
+ struct sk_buff *stream_reassembly;
struct mutex req_lock;
wait_queue_head_t req_wait_q;
@@ -428,6 +429,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 48ef56e..13bb2b9 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -916,6 +916,8 @@ int hci_register_dev(struct hci_dev *hdev)
for (i = 0; i < 3; i++)
hdev->reassembly[i] = NULL;
+ hdev->stream_reassembly = NULL;
+
init_waitqueue_head(&hdev->req_wait_q);
mutex_init(&hdev->req_lock);
@@ -973,6 +975,8 @@ int hci_unregister_dev(struct hci_dev *hdev)
for (i = 0; i < 3; i++)
kfree_skb(hdev->reassembly[i]);
+ kfree_skb(hdev->stream_reassembly);
+
hci_notify(hdev, HCI_DEV_UNREG);
if (hdev->rfkill) {
@@ -1145,6 +1149,45 @@ static int hci_reassembly(struct hci_dev *hdev, int type, void *data,
return 0;
}
+
+#define __stream_reassembly(hdev) ((hdev)->stream_reassembly)
+
+int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
+{
+ int type;
+ int remaining = 0;
+ int err = 0;
+
+ do {
+ struct sk_buff *skb = __stream_reassembly(hdev);
+ if (!skb) {
+ struct { char type; } *pkt;
+
+ /* Start of the frame */
+ pkt = data;
+ type = pkt->type;
+
+ data++;
+ count--;
+ } else
+ type = bt_cb(skb)->pkt_type;
+
+ err = hci_reassembly(hdev, type, data, count, &skb, &remaining);
+
+ if (err < 0)
+ return err;
+
+ __stream_reassembly(hdev) = skb;
+
+ data += (count - remaining);
+ count = remaining;
+
+ } while (count);
+
+ return err;
+}
+EXPORT_SYMBOL(hci_recv_stream_fragment);
+
/* Receive packet type fragment */
#define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
--
1.7.0
^ permalink raw reply related
* [PATCH v3 2/3] Modified hci_recv_fragment() to use hci_reassembly
From: suraj @ 2010-06-09 13:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1276088748.31226.6.camel@atheros013-desktop>
modified packet based reassembly code to use hci_reassembly()
Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
net/bluetooth/hci_core.c | 78 +++++++--------------------------------------
1 files changed, 12 insertions(+), 66 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 631a185..48ef56e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1150,82 +1150,28 @@ static int hci_reassembly(struct hci_dev *hdev, int type, void *data,
int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count)
{
+ int remaining = 0;
+ int err = 0;
+
if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
return -EILSEQ;
- while (count) {
+ do {
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;
+ err = hci_reassembly(hdev, type, data, count, &skb, &remaining);
- 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;
- }
+ if (err < 0)
+ return err;
- 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;
- __reassembly(hdev, type) = skb;
-
- scb = (void *) skb->cb;
- scb->expect = len;
- } else {
- /* Continuation */
-
- scb = (void *) skb->cb;
- len = scb->expect;
- }
+ data += (count - remaining);
+ count = remaining;
- len = min(len, count);
+ } while (count);
- memcpy(skb_put(skb, len), data, len);
-
- scb->expect -= len;
-
- if (scb->expect == 0) {
- /* Complete frame */
-
- __reassembly(hdev, type) = NULL;
-
- bt_cb(skb)->pkt_type = type;
- hci_recv_frame(skb);
- }
-
- count -= len; data += len;
- }
-
- return 0;
+ return err;
}
EXPORT_SYMBOL(hci_recv_fragment);
--
1.7.0
^ permalink raw reply related
* [PATCH v3 1/3] Implements hci_reassembly to reassemble Rx packets
From: suraj @ 2010-06-09 13:05 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
Implements feature to reassemble received HCI frames from any input stream.
Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
net/bluetooth/hci_core.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 115 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5e83f8e..631a185 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1030,6 +1030,121 @@ 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, struct sk_buff **skb_ptr, int *remain)
+{
+ int len = 0;
+ int header_len = 0;
+ struct sk_buff *skb = *skb_ptr;
+ struct { __u8 pkt_type; int expect; } *scb;
+
+ *remain = count;
+
+ if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
+ return -EILSEQ;
+
+ if (!skb) {
+
+ switch (type) {
+ case HCI_ACLDATA_PKT:
+ len = HCI_MAX_FRAME_SIZE;
+ header_len = HCI_ACL_HDR_SIZE;
+ break;
+ case HCI_EVENT_PKT:
+ len = HCI_MAX_EVENT_SIZE;
+ header_len = HCI_EVENT_HDR_SIZE;
+ break;
+ case HCI_SCODATA_PKT:
+ len = HCI_MAX_SCO_SIZE;
+ header_len = HCI_SCO_HDR_SIZE;
+ break;
+ }
+
+ skb = bt_skb_alloc(len, GFP_ATOMIC);
+
+ if (!skb)
+ return -ENOMEM;
+
+ scb = (void *) skb->cb;
+ scb->expect = header_len;
+ scb->pkt_type = (__u8)type;
+
+ skb->dev = (void *) hdev;
+ *skb_ptr = skb;
+
+ }
+
+ while (count) {
+
+ scb = (void *) skb->cb;
+ len = min(scb->expect, 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);
+ *skb_ptr = 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);
+ *skb_ptr = 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);
+ *skb_ptr = NULL;
+
+ return -ENOMEM;
+ }
+ }
+ break;
+ }
+
+ if (scb->expect == 0) {
+
+ /* Complete frame */
+ bt_cb(skb)->pkt_type = type;
+ hci_recv_frame(skb);
+
+ *skb_ptr = NULL;
+
+ return 0;
+ }
+
+ }
+
+ return 0;
+}
/* Receive packet type fragment */
#define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
--
1.7.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox