* Re: [RFC 4/7] Bluetooth: Linearize received L2CAP skbuffs.
From: Marcel Holtmann @ 2010-08-10 21:38 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-5-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Fragmented skbs are only encountered by in-kernel protocols and
> profiles when receiving ERTM or streaming mode L2CAP data. BNEP,
> CMTP, HIDP, and RFCOMM generally use basic mode, so will not
> generally need the extra memcpy's necessary to put the L2CAP
> payload in to a linear buffer. However, it is possible to
> use enhanced L2CAP in these modes so the possibility needs
> to be handled.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/bnep/core.c | 5 ++++-
> net/bluetooth/cmtp/core.c | 5 ++++-
> net/bluetooth/hidp/core.c | 10 ++++++++--
> net/bluetooth/rfcomm/core.c | 5 ++++-
> 4 files changed, 20 insertions(+), 5 deletions(-)
in general I am fine with this.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Marcel Holtmann @ 2010-08-10 21:29 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-4-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> In order to not limit ERTM and streaming mode PDUs to the HCI MTU
> size, L2CAP must be able to split PDUs in to multple HCI fragments.
> This is done by allocating space for the FCS in the last fragment.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 39 ++++++++++++++++++++++++++++++++++-----
> 1 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index aa69c84..b485c4a 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
> {
> struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> struct sk_buff **frag;
> + struct sk_buff *final;
> int err, sent = 0;
>
> + BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
> + msg, (int)len, (int)count, skb);
> +
> if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
> return -EFAULT;
>
> sent += count;
> len -= count;
> + final = skb;
>
> /* Continuation fragments (no L2CAP header) */
> frag = &skb_shinfo(skb)->frag_list;
> while (len) {
> + int skblen;
> count = min_t(unsigned int, conn->mtu, len);
I prefer that you either make the int skblen global or add an empty
line. There should be always an empty line between variable declaration
and code.
>
> - *frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
> + /* Add room for the FCS if it fits */
> + if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
> + len + L2CAP_FCS_SIZE <= conn->mtu)
> + skblen = count + L2CAP_FCS_SIZE;
So in general we do double indent for the second line of the if:
if (l2cap_pi ...
len + ...
skblen = count ...
This is not strictly enforced and every kernel code does it a little bit
different.
The rest looks just fine.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 1/7] Bluetooth: Calculate L2CAP FCS on fragmented skbuffs.
From: Marcel Holtmann @ 2010-08-10 21:22 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281467704-5378-2-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> When L2CAP PDUs are longer than the HCI MTU, they are stored in
> fragmented skbuffs. This adds a function to calculate the FCS
> on any skbuff (fragmented or not), and replaces direct calls
> to crc16 with calls to the new apply_fcs() function.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 38 insertions(+), 19 deletions(-)
looks fine to me. Just lets get the other ones in first and then we can
apply this one.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/5] Bluetooth: Validate PSM values in calls to connect() and bind().
From: Marcel Holtmann @ 2010-08-10 21:21 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281466850-5297-3-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Valid L2CAP PSMs are odd numbers, and the least significant bit of the
> most significant byte must be 0.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 23 +++++++++++++++++++----
> 1 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index c784703..61aa961 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1008,10 +1008,19 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> goto done;
> }
>
> - if (la.l2_psm && __le16_to_cpu(la.l2_psm) < 0x1001 &&
> - !capable(CAP_NET_BIND_SERVICE)) {
> - err = -EACCES;
> - goto done;
> + /* If specified, PSM must be odd and lsb of upper byte must be 0 */
> + if (la.l2_psm) {
> + __u16 psm = __le16_to_cpu(la.l2_psm);
> +
I would just put the comment here:
/* PSM must be odd and lsb of upper byte must be 0 */
> + if ((psm & 0x0101) != 0x0001) {
> + err = -EINVAL;
> + goto done;
> + }
And then while you at it:
/* Restrict usage of well known PSMs */
> + if (psm < 0x1001 && !capable(CAP_NET_BIND_SERVICE)) {
> + err = -EACCES;
> + goto done;
> + }
> }
Maybe there is actually a second place for adding that comment.
Otherwise I am fine with this, but since it is not a regression, this
will be only taking in the next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/5] Bluetooth: Only enable L2CAP FCS for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-10 21:17 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1281466850-5297-2-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
> with L2CAP modes other than ERTM or streaming. At present, this only
> affects the FCS value shown with getsockopt() for basic mode.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 19 +++++++++++++------
> 1 files changed, 13 insertions(+), 6 deletions(-)
patch has been applied, but please stop adding a . at the end of your
subject line ;)
Regards
Marcel
^ permalink raw reply
* Re: patch for firmware download to Qualcomm Bluetooth chip
From: Marcel Holtmann @ 2010-08-10 21:14 UTC (permalink / raw)
To: 1267142545.30787.6.camel; +Cc: linux-bluetooth, johan.hedberg
In-Reply-To: <4C619FA6.8030606@codeaurora.org>
Hi Ron,
> Here's the a ping regarding the patch submitted by Matt back in March? I
> believe. For you convenience I've included the patch again. Please take
> a look, and provide your general comments. I'm sure there are changes
> that need to be made.
>
> Fyi. sorry about the column wrap in the patch. Don't feel like fixing it
> for just a quick review.
>
> From 76d0bdd82a0a4e5b3b9544bb864c31888f20cea1 Mon Sep 17 00:00:00 2001
> From: Wilson, Matt <mtwilson@codeaurora.org>
> Date: Thu, 11 Feb 2010 11:53:29 -0600
> Subject: [PATCH] Firmware download for Qualcomm Bluetooth devices
>
> ---
> Makefile.tools | 3 +-
> tools/hciattach.c | 9 ++
> tools/hciattach.h | 4 +
> tools/hciattach_qualcomm.c | 279
> ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+), 1 deletions(-)
> create mode 100644 tools/hciattach_qualcomm.c
>
> diff --git a/Makefile.tools b/Makefile.tools
> index 2735d68..7b92c8f 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -23,7 +23,8 @@ tools_l2ping_LDADD = lib/libbluetooth.la
> tools_hciattach_SOURCES = tools/hciattach.c tools/hciattach.h \
> tools/hciattach_st.c \
> tools/hciattach_ti.c \
> - tools/hciattach_tialt.c
> + tools/hciattach_tialt.c \
> + tools/hciattach_qualcomm.c
> tools_hciattach_LDADD = lib/libbluetooth.la
>
> tools_hciconfig_SOURCES = tools/hciconfig.c tools/csr.h tools/csr.c \
> diff --git a/tools/hciattach.c b/tools/hciattach.c
> index 364c5ff..d6aafbe 100644
> --- a/tools/hciattach.c
> +++ b/tools/hciattach.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2000-2001 Qualcomm Incorporated
> * Copyright (C) 2002-2003 Maxim Krasnyansky <maxk@qualcomm.com>
> * Copyright (C) 2002-2010 Marcel Holtmann <marcel@holtmann.org>
> + * Copyright (C) 2010, Code Aurora Forum. All rights reserved.
> *
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -299,6 +300,11 @@ static int texasalt(int fd, struct uart_t *u, struct
> termios *ti)
> return texasalt_init(fd, u->speed, ti);
> }
>
> +static int qualcomm(int fd, struct uart_t *u, struct termios *ti)
> +{
> + return qualcomm_init(fd, u->speed, ti, u->bdaddr);
> +}
> +
> static int read_check(int fd, void *buf, int count)
> {
> int res;
> @@ -1071,6 +1077,9 @@ struct uart_t uart[] = {
> /* Broadcom BCM2035 */
> { "bcm2035", 0x0A5C, 0x2035, HCI_UART_H4, 115200, 460800, FLOW_CTL,
> NULL, bcm2035 },
>
> + /* QUALCOMM BTS */
> + { "qualcomm", 0x0000, 0x0000, HCI_UART_H4, 115200, 115200, FLOW_CTL,
> NULL, qualcomm },
> +
> { NULL, 0 }
> };
>
> diff --git a/tools/hciattach.h b/tools/hciattach.h
> index 867563b..5c89013 100644
> --- a/tools/hciattach.h
> +++ b/tools/hciattach.h
> @@ -3,6 +3,7 @@
> * BlueZ - Bluetooth protocol stack for Linux
> *
> * Copyright (C) 2003-2010 Marcel Holtmann <marcel@holtmann.org>
> + * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
> *
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -45,3 +46,6 @@ int texas_post(int fd, struct termios *ti);
> int texasalt_init(int fd, int speed, struct termios *ti);
> int stlc2500_init(int fd, bdaddr_t *bdaddr);
> int bgb2xx_init(int dd, bdaddr_t *bdaddr);
> +int qualcomm_init(int fd, int speed, struct termios *ti, const char
> *bdaddr);
> +
> +
I am actually fine with this. Except strip it from claiming copyright on
hciattach.c and hciattach.h since these changes are not really anything
new. They are just wrapping code.
So fix this and submit a new clean patch.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] Use HCI device type to gate BR/EDR-specific HCI commands
From: Johan Hedberg @ 2010-08-10 20:15 UTC (permalink / raw)
To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <1281469904-30408-1-git-send-email-dscherba@codeaurora.org>
Hi David,
On Tue, Aug 10, 2010, David Scherba wrote:
> Use the hci_dev_info structure member 'type' to classify whether a HCI device
> is BR/EDR, or not. If not, gate BR/EDR-specific HCI commands.
> ---
> plugins/hciops.c | 20 +++++++++++++-------
> src/adapter.c | 2 +-
> src/hcid.h | 8 ++++++++
> src/security.c | 4 ++--
> 4 files changed, 24 insertions(+), 10 deletions(-)
Thanks. This one is now also pushed upstream.
Johan
^ permalink raw reply
* [PATCH 1/1] Use HCI device type to gate BR/EDR-specific HCI commands
From: David Scherba @ 2010-08-10 19:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, marcel, rshaffer, David Scherba
In-Reply-To: <20100810191616.GA26091@jh-x301>
Use the hci_dev_info structure member 'type' to classify whether a HCI device
is BR/EDR, or not. If not, gate BR/EDR-specific HCI commands.
---
plugins/hciops.c | 20 +++++++++++++-------
src/adapter.c | 2 +-
src/hcid.h | 8 ++++++++
src/security.c | 4 ++--
4 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 3e3e172..9c97c5a 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -85,7 +85,7 @@ static void device_devup_setup(int index)
if (hci_devinfo(index, &di) < 0)
return;
- if (hci_test_bit(HCI_RAW, &di.flags))
+ if (ignore_device(&di))
return;
dd = hci_open_dev(index);
@@ -122,6 +122,7 @@ static void device_devup_setup(int index)
static void init_device(int index)
{
struct hci_dev_req dr;
+ struct hci_dev_info di;
pid_t pid;
int dd, err;
@@ -159,12 +160,17 @@ static void init_device(int index)
index, strerror(err), err);
}
- /* Set link policy */
- dr.dev_opt = main_opts.link_policy;
- if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
+ /* Set link policy for BR/EDR HCI devices */
+ if (hci_devinfo(index, &di) < 0)
+ goto fail;
+
+ if (!ignore_device(&di)) {
+ dr.dev_opt = main_opts.link_policy;
+ if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
errno != ENETDOWN) {
- error("Can't set link policy on hci%d: %s (%d)",
- index, strerror(errno), errno);
+ error("Can't set link policy on hci%d: %s (%d)",
+ index, strerror(errno), errno);
+ }
}
/* Start HCI device */
@@ -196,7 +202,7 @@ static void device_devreg_setup(int index)
devup = hci_test_bit(HCI_UP, &di.flags);
- if (!hci_test_bit(HCI_RAW, &di.flags))
+ if (!ignore_device(&di))
manager_register_adapter(index, devup);
}
diff --git a/src/adapter.c b/src/adapter.c
index af44a59..fc1e123 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2300,7 +2300,7 @@ int adapter_start(struct btd_adapter *adapter)
if (hci_devinfo(adapter->dev_id, &di) < 0)
return -errno;
- if (hci_test_bit(HCI_RAW, &di.flags)) {
+ if (ignore_device(&di)) {
dev->ignore = 1;
return -1;
}
diff --git a/src/hcid.h b/src/hcid.h
index 9c5f1d6..50bdb65 100644
--- a/src/hcid.h
+++ b/src/hcid.h
@@ -23,6 +23,9 @@
*
*/
+#include <bluetooth/hci.h>
+#include <bluetooth/hci_lib.h>
+
/* When all services should trust a remote device */
#define GLOBAL_TRUST "[all]"
@@ -99,3 +102,8 @@ void rfkill_exit(void);
void __probe_servers(const char *adapter);
void __remove_servers(const char *adapter);
+
+static inline int ignore_device(struct hci_dev_info *di)
+{
+ return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
+}
diff --git a/src/security.c b/src/security.c
index ca394e1..667f1f1 100644
--- a/src/security.c
+++ b/src/security.c
@@ -999,7 +999,7 @@ static gboolean io_security_event(GIOChannel *chan, GIOCondition cond,
ioctl(dev, HCIGETDEVINFO, (void *) di);
- if (hci_test_bit(HCI_RAW, &di->flags))
+ if (ignore_device(di))
return TRUE;
switch (eh->evt) {
@@ -1185,7 +1185,7 @@ void start_security_manager(int hdev)
io_data[hdev].channel = chan;
io_data[hdev].pin_length = -1;
- if (hci_test_bit(HCI_RAW, &di->flags))
+ if (ignore_device(di))
return;
bacpy(&cp.bdaddr, BDADDR_ANY);
--
1.7.0.2
--
David Scherba
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
From: David Scherba @ 2010-08-10 19:49 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <20100810191616.GA26091@jh-x301>
On 8/10/2010 2:16 PM, Johan Hedberg wrote:
>> +static inline int ignore_device(struct hci_dev_info *di)
>> +{
>> + return hci_test_bit(HCI_RAW,&di->flags) || di->type>> 4 != HCI_BREDR;
>> +}
>
> Since this is hci_lib.h (something I didn't realize in the previous
> review) the function should follow the same namespace as all other
> functions in the header file, i.e. hci_*. However, does the function
> really need to be exported in this public library header file, i.e.
> isn't there some private header file that could be used instead?
It looks like src/hcid.h fits the bill, although the modified hcid.h
will need to include the hci headers to have the appropriate defines...
Modified patch coming up.
Thanks,
David
--
David Scherba
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH 1/2] Remove non-functional hci_devinfo calls in init_device()
From: Johan Hedberg @ 2010-08-10 19:20 UTC (permalink / raw)
To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <1281453304-30408-2-git-send-email-dscherba@codeaurora.org>
Hi David,
On Tue, Aug 10, 2010, David Scherba wrote:
> ---
> plugins/hciops.c | 8 --------
> 1 files changed, 0 insertions(+), 8 deletions(-)
This patch is now upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 2/2] Use HCI device type to gate BR/EDR-specific HCI commands
From: Johan Hedberg @ 2010-08-10 19:16 UTC (permalink / raw)
To: David Scherba; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <1281455145-1777-1-git-send-email-dscherba@codeaurora.org>
Hi David,
On Tue, Aug 10, 2010, David Scherba wrote:
> --- a/lib/hci_lib.h
> +++ b/lib/hci_lib.h
> @@ -169,6 +169,11 @@ static inline int hci_test_bit(int nr, void *addr)
> return *((uint32_t *) addr + (nr >> 5)) & (1 << (nr & 31));
> }
>
> +static inline int ignore_device(struct hci_dev_info *di)
> +{
> + return hci_test_bit(HCI_RAW, &di->flags) || di->type >> 4 != HCI_BREDR;
> +}
Since this is hci_lib.h (something I didn't realize in the previous
review) the function should follow the same namespace as all other
functions in the header file, i.e. hci_*. However, does the function
really need to be exported in this public library header file, i.e.
isn't there some private header file that could be used instead?
Johan
^ permalink raw reply
* [RFC 7/7] Bluetooth: Do not limit enhanced L2CAP max PDU size to HCI MTU.
From: Mat Martineau @ 2010-08-10 19:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
HCI MTU and PDU size have no relationship, since ERTM and streaming PDUs
can be fragmented over HCI just like other L2CAP frames. The only
applicable limit for PDU size is the L2CAP MTU, which is now checked
during L2CAP configuration.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0212035..15fbb1d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2621,8 +2621,8 @@ done:
rfc.retrans_timeout = 0;
rfc.monitor_timeout = 0;
rfc.max_pdu_size = cpu_to_le16(L2CAP_DEFAULT_MAX_PDU_SIZE);
- if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
- rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->imtu)
+ rfc.max_pdu_size = cpu_to_le16(pi->imtu);
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
(unsigned long) &rfc);
@@ -2644,8 +2644,8 @@ done:
rfc.retrans_timeout = 0;
rfc.monitor_timeout = 0;
rfc.max_pdu_size = cpu_to_le16(L2CAP_DEFAULT_MAX_PDU_SIZE);
- if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->conn->mtu - 10)
- rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ if (L2CAP_DEFAULT_MAX_PDU_SIZE > pi->imtu)
+ rfc.max_pdu_size = cpu_to_le16(pi->imtu);
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
(unsigned long) &rfc);
@@ -2778,8 +2778,8 @@ done:
pi->remote_tx_win = rfc.txwin_size;
pi->remote_max_tx = rfc.max_transmit;
- if (le16_to_cpu(rfc.max_pdu_size) > pi->conn->mtu - 10)
- rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ if (le16_to_cpu(rfc.max_pdu_size) > pi->omtu)
+ rfc.max_pdu_size = cpu_to_le16(pi->omtu);
pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
@@ -2796,8 +2796,8 @@ done:
break;
case L2CAP_MODE_STREAMING:
- if (le16_to_cpu(rfc.max_pdu_size) > pi->conn->mtu - 10)
- rfc.max_pdu_size = cpu_to_le16(pi->conn->mtu - 10);
+ if (le16_to_cpu(rfc.max_pdu_size) > pi->omtu)
+ rfc.max_pdu_size = cpu_to_le16(pi->omtu);
pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 6/7] Bluetooth: Reassemble enhanced L2CAP PDUs using skb fragments.
From: Mat Martineau @ 2010-08-10 19:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
As enhanced L2CAP PDUs arrive, it is not necessary to copy them
in to a separate skbuff. Instead, the skbuffs can be linked
together as fragments, only being copied in to a linear buffer
when the data is copied to userspace. This avoids the need
to allocate additional buffers for incoming data, and
eliminates copying of data payloads during SDU reassembly.
This is of greater concern with high-speed AMP links than
with BR/EDR.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/l2cap.c | 66 +++++++++++++++++++++-------------------
2 files changed, 36 insertions(+), 31 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 2f3222f..9384e87 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -357,6 +357,7 @@ struct l2cap_pinfo {
__u16 sdu_len;
__u16 partial_sdu_len;
struct sk_buff *sdu;
+ struct sk_buff *sdu_last_frag;
__u8 ident;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index b485c4a..0212035 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -290,6 +290,9 @@ static void l2cap_chan_del(struct sock *sk, int err)
skb_queue_purge(SREJ_QUEUE(sk));
skb_queue_purge(BUSY_QUEUE(sk));
+ if (l2cap_pi(sk)->sdu)
+ kfree_skb(l2cap_pi(sk)->sdu);
+
list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
list_del(&l->list);
kfree(l);
@@ -3635,6 +3638,27 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
return 0;
}
+static inline void append_skb_frag(struct sk_buff *skb,
+ struct sk_buff *new_frag, struct sk_buff **last_frag)
+{
+ /* skb->len reflects data in skb as well as all fragments
+ skb->data_len reflects only data in fragments
+ */
+ BT_DBG("skb %p, new_frag %p, *last_frag %p", skb, new_frag, *last_frag);
+
+ if (!skb_has_frags(skb))
+ skb_shinfo(skb)->frag_list = new_frag;
+
+ new_frag->next = NULL;
+
+ (*last_frag)->next = new_frag;
+ *last_frag = new_frag;
+
+ skb->len += new_frag->len;
+ skb->data_len += new_frag->len;
+ skb->truesize += new_frag->truesize;
+}
+
static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 control)
{
struct l2cap_pinfo *pi = l2cap_pi(sk);
@@ -3643,7 +3667,7 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
switch (control & L2CAP_CTRL_SAR) {
case L2CAP_SDU_UNSEGMENTED:
- if (pi->conn_state & L2CAP_CONN_SAR_SDU)
+ if (pi->sdu)
goto drop;
err = sock_queue_rcv_skb(sk, skb);
@@ -3653,61 +3677,42 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
break;
case L2CAP_SDU_START:
- if (pi->conn_state & L2CAP_CONN_SAR_SDU)
+ if (pi->sdu)
goto drop;
pi->sdu_len = get_unaligned_le16(skb->data);
+ skb_pull(skb, 2);
if (pi->sdu_len > pi->imtu)
goto disconnect;
- pi->sdu = bt_skb_alloc(pi->sdu_len, GFP_ATOMIC);
- if (!pi->sdu)
- return -ENOMEM;
-
- /* pull sdu_len bytes only after alloc, because of Local Busy
- * condition we have to be sure that this will be executed
- * only once, i.e., when alloc does not fail */
- skb_pull(skb, 2);
-
- memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
-
- pi->conn_state |= L2CAP_CONN_SAR_SDU;
- pi->partial_sdu_len = skb->len;
+ pi->sdu = skb;
+ pi->sdu_last_frag = skb;
break;
case L2CAP_SDU_CONTINUE:
- if (!(pi->conn_state & L2CAP_CONN_SAR_SDU))
- goto disconnect;
-
if (!pi->sdu)
goto disconnect;
- pi->partial_sdu_len += skb->len;
- if (pi->partial_sdu_len > pi->sdu_len)
- goto drop;
+ append_skb_frag(pi->sdu, skb, &pi->sdu_last_frag);
- memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
+ if (pi->sdu->len > pi->sdu_len)
+ goto drop;
break;
case L2CAP_SDU_END:
- if (!(pi->conn_state & L2CAP_CONN_SAR_SDU))
- goto disconnect;
-
if (!pi->sdu)
goto disconnect;
if (!(pi->conn_state & L2CAP_CONN_SAR_RETRY)) {
- pi->partial_sdu_len += skb->len;
+ append_skb_frag(pi->sdu, skb, &pi->sdu_last_frag);
- if (pi->partial_sdu_len > pi->imtu)
+ if (pi->sdu->len > pi->sdu_len)
goto drop;
- if (pi->partial_sdu_len != pi->sdu_len)
+ if (pi->sdu->len != pi->sdu_len)
goto drop;
-
- memcpy(skb_put(pi->sdu, skb->len), skb->data, skb->len);
}
_skb = skb_clone(pi->sdu, GFP_ATOMIC);
@@ -3724,7 +3729,6 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
}
pi->conn_state &= ~L2CAP_CONN_SAR_RETRY;
- pi->conn_state &= ~L2CAP_CONN_SAR_SDU;
kfree_skb(pi->sdu);
break;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 5/7] Bluetooth: Handle fragmented skbs in bt_sock_stream_recvmsg()
From: Mat Martineau @ 2010-08-10 19:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
When reading L2CAP skbuffs, add the ability to copy from
fragmented skbuffs generated during ERTM or streaming mode
reassembly. This defers extra copying of L2CAP payloads
until the final, unavoidable copy to userspace.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/af_bluetooth.c | 30 ++++++++++++++++++++++++++++--
1 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 77a26fe..5e0375b 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -342,7 +342,7 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
}
chunk = min_t(unsigned int, skb->len, size);
- if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
+ if (skb_copy_datagram_iovec(skb, 0, msg->msg_iov, chunk)) {
skb_queue_head(&sk->sk_receive_queue, skb);
if (!copied)
copied = -EFAULT;
@@ -354,7 +354,33 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
sock_recv_ts_and_drops(msg, sk, skb);
if (!(flags & MSG_PEEK)) {
- skb_pull(skb, chunk);
+ int skb_len = skb_headlen(skb);
+
+ if (chunk <= skb_len) {
+ __skb_pull(skb, chunk);
+ } else {
+ struct sk_buff *frag;
+
+ __skb_pull(skb, skb_len);
+ chunk -= skb_len;
+
+ skb_walk_frags(skb, frag) {
+ if (chunk <= frag->len) {
+ /* Pulling partial data */
+ skb->len -= chunk;
+ skb->data_len -= chunk;
+ __skb_pull(frag, chunk);
+ break;
+ } else if (frag->len) {
+ /* Pulling all frag data */
+ chunk -= frag->len;
+ skb->len -= frag->len;
+ skb->data_len -= frag->len;
+ __skb_pull(frag, frag->len);
+ }
+ }
+ }
+
if (skb->len) {
skb_queue_head(&sk->sk_receive_queue, skb);
break;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 4/7] Bluetooth: Linearize received L2CAP skbuffs.
From: Mat Martineau @ 2010-08-10 19:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
Fragmented skbs are only encountered by in-kernel protocols and
profiles when receiving ERTM or streaming mode L2CAP data. BNEP,
CMTP, HIDP, and RFCOMM generally use basic mode, so will not
generally need the extra memcpy's necessary to put the L2CAP
payload in to a linear buffer. However, it is possible to
use enhanced L2CAP in these modes so the possibility needs
to be handled.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/bnep/core.c | 5 ++++-
net/bluetooth/cmtp/core.c | 5 ++++-
net/bluetooth/hidp/core.c | 10 ++++++++--
net/bluetooth/rfcomm/core.c | 5 ++++-
4 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index f10b41f..9f60443 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -481,7 +481,10 @@ static int bnep_session(void *arg)
// RX
while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
skb_orphan(skb);
- bnep_rx_frame(s, skb);
+ if (!skb_linearize(skb))
+ bnep_rx_frame(s, skb);
+ else
+ kfree_skb(skb);
}
if (sk->sk_state != BT_CONNECTED)
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index d4c6af0..48ac812 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -293,7 +293,10 @@ static int cmtp_session(void *arg)
while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
skb_orphan(skb);
- cmtp_recv_frame(session, skb);
+ if (!skb_linearize(skb))
+ cmtp_recv_frame(session, skb);
+ else
+ kfree_skb(skb);
}
cmtp_process_transmit(session);
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index bfe641b..097ff25 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -571,12 +571,18 @@ static int hidp_session(void *arg)
while ((skb = skb_dequeue(&ctrl_sk->sk_receive_queue))) {
skb_orphan(skb);
- hidp_recv_ctrl_frame(session, skb);
+ if (!skb_linearize(skb))
+ hidp_recv_ctrl_frame(session, skb);
+ else
+ kfree_skb(skb);
}
while ((skb = skb_dequeue(&intr_sk->sk_receive_queue))) {
skb_orphan(skb);
- hidp_recv_intr_frame(session, skb);
+ if (!skb_linearize(skb))
+ hidp_recv_intr_frame(session, skb);
+ else
+ kfree_skb(skb);
}
hidp_process_transmit(session);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..ee27f59 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1845,7 +1845,10 @@ static inline void rfcomm_process_rx(struct rfcomm_session *s)
/* Get data directly from socket receive queue without copying it. */
while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
skb_orphan(skb);
- rfcomm_recv_frame(s, skb);
+ if (!skb_linearize(skb))
+ rfcomm_recv_frame(s, skb);
+ else
+ kfree_skb(skb);
}
if (sk->sk_state == BT_CLOSED) {
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Mat Martineau @ 2010-08-10 19:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
In order to not limit ERTM and streaming mode PDUs to the HCI MTU
size, L2CAP must be able to split PDUs in to multple HCI fragments.
This is done by allocating space for the FCS in the last fragment.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 39 ++++++++++++++++++++++++++++++++++-----
1 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index aa69c84..b485c4a 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
{
struct l2cap_conn *conn = l2cap_pi(sk)->conn;
struct sk_buff **frag;
+ struct sk_buff *final;
int err, sent = 0;
+ BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
+ msg, (int)len, (int)count, skb);
+
if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
return -EFAULT;
sent += count;
len -= count;
+ final = skb;
/* Continuation fragments (no L2CAP header) */
frag = &skb_shinfo(skb)->frag_list;
while (len) {
+ int skblen;
count = min_t(unsigned int, conn->mtu, len);
- *frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
+ /* Add room for the FCS if it fits */
+ if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
+ len + L2CAP_FCS_SIZE <= conn->mtu)
+ skblen = count + L2CAP_FCS_SIZE;
+ else
+ skblen = count;
+
+ *frag = bt_skb_send_alloc(sk, skblen,
+ msg->msg_flags & MSG_DONTWAIT, &err);
if (!*frag)
return -EFAULT;
- if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
+
+ if (memcpy_fromiovec(skb_put(*frag, count),
+ msg->msg_iov, count))
return -EFAULT;
sent += count;
len -= count;
+ final = *frag;
+
frag = &(*frag)->next;
}
+ if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16) {
+ if (skb_tailroom(final) < L2CAP_FCS_SIZE) {
+ *frag = bt_skb_send_alloc(sk, L2CAP_FCS_SIZE,
+ msg->msg_flags & MSG_DONTWAIT,
+ &err);
+ if (!*frag)
+ return -EFAULT;
+
+ final = *frag;
+ }
+
+ skb_put(final, L2CAP_FCS_SIZE);
+ }
+
return sent;
}
@@ -1790,9 +1822,6 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
return ERR_PTR(err);
}
- if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
- put_unaligned_le16(0, skb_put(skb, 2));
-
bt_cb(skb)->retries = 0;
return skb;
}
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 2/7] Bluetooth: Use enhanced L2CAP header structure and symbolic values.
From: Mat Martineau @ 2010-08-10 19:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 23 +++++++++++++----------
1 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 5e78c18..aa69c84 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1753,33 +1753,36 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
{
struct l2cap_conn *conn = l2cap_pi(sk)->conn;
struct sk_buff *skb;
- int err, count, hlen = L2CAP_HDR_SIZE + 2;
- struct l2cap_hdr *lh;
+ int err, count, hlen = L2CAP_ENHANCED_HDR_SIZE;
+ struct l2cap_enhanced_hdr *lh;
- BT_DBG("sk %p len %d", sk, (int)len);
+ BT_DBG("sk %p, msg %p, len %d, control %x, sdulen %d",
+ sk, msg, (int)len, control, (int)sdulen);
if (!conn)
return ERR_PTR(-ENOTCONN);
if (sdulen)
- hlen += 2;
+ hlen += L2CAP_SDULEN_SIZE;
if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
- hlen += 2;
+ hlen += L2CAP_FCS_SIZE;
count = min_t(unsigned int, (conn->mtu - hlen), len);
+
skb = bt_skb_send_alloc(sk, count + hlen,
msg->msg_flags & MSG_DONTWAIT, &err);
if (!skb)
- return ERR_PTR(-ENOMEM);
+ return ERR_PTR(err);
/* Create L2CAP header */
- lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
+ lh = (struct l2cap_enhanced_hdr *) skb_put(skb,
+ L2CAP_ENHANCED_HDR_SIZE);
lh->cid = cpu_to_le16(l2cap_pi(sk)->dcid);
- lh->len = cpu_to_le16(len + (hlen - L2CAP_HDR_SIZE));
- put_unaligned_le16(control, skb_put(skb, 2));
+ lh->len = cpu_to_le16(len + hlen - L2CAP_HDR_SIZE);
+ lh->control = cpu_to_le16(control);
if (sdulen)
- put_unaligned_le16(sdulen, skb_put(skb, 2));
+ put_unaligned_le16(sdulen, skb_put(skb, L2CAP_SDULEN_SIZE));
err = l2cap_skbuff_fromiovec(sk, msg, len, count, skb);
if (unlikely(err < 0)) {
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 1/7] Bluetooth: Calculate L2CAP FCS on fragmented skbuffs.
From: Mat Martineau @ 2010-08-10 19:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281467704-5378-1-git-send-email-mathewm@codeaurora.org>
When L2CAP PDUs are longer than the HCI MTU, they are stored in
fragmented skbuffs. This adds a function to calculate the FCS
on any skbuff (fragmented or not), and replaces direct calls
to crc16 with calls to the new apply_fcs() function.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++----------------
1 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 8d362d7..5e78c18 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -351,6 +351,33 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
return id;
}
+static void apply_fcs(struct sk_buff *skb)
+{
+ size_t len;
+ u16 partial_crc;
+ struct sk_buff *iter;
+ struct sk_buff *final_frag = skb;
+
+ if (skb_has_frags(skb))
+ len = skb_headlen(skb);
+ else
+ len = skb->len - L2CAP_FCS_SIZE;
+
+ partial_crc = crc16(0, (u8 *) skb->data, len);
+
+ skb_walk_frags(skb, iter) {
+ len = iter->len;
+ if (!iter->next)
+ len -= L2CAP_FCS_SIZE;
+
+ partial_crc = crc16(partial_crc, iter->data, len);
+ final_frag = iter;
+ }
+
+ put_unaligned_le16(partial_crc,
+ final_frag->data + final_frag->len - L2CAP_FCS_SIZE);
+}
+
static inline void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
{
struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
@@ -401,10 +428,8 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
lh->cid = cpu_to_le16(pi->dcid);
put_unaligned_le16(control, skb_put(skb, 2));
- if (pi->fcs == L2CAP_FCS_CRC16) {
- u16 fcs = crc16(0, (u8 *)lh, count - 2);
- put_unaligned_le16(fcs, skb_put(skb, 2));
- }
+ if (pi->fcs == L2CAP_FCS_CRC16)
+ apply_fcs(skb);
hci_send_acl(pi->conn->hcon, skb, 0);
}
@@ -1458,7 +1483,7 @@ static void l2cap_streaming_send(struct sock *sk)
{
struct sk_buff *skb, *tx_skb;
struct l2cap_pinfo *pi = l2cap_pi(sk);
- u16 control, fcs;
+ u16 control;
while ((skb = sk->sk_send_head)) {
tx_skb = skb_clone(skb, GFP_ATOMIC);
@@ -1467,10 +1492,8 @@ static void l2cap_streaming_send(struct sock *sk)
control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT;
put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
- if (pi->fcs == L2CAP_FCS_CRC16) {
- fcs = crc16(0, (u8 *)tx_skb->data, tx_skb->len - 2);
- put_unaligned_le16(fcs, tx_skb->data + tx_skb->len - 2);
- }
+ if (pi->fcs == L2CAP_FCS_CRC16)
+ apply_fcs(tx_skb);
l2cap_do_send(sk, tx_skb);
@@ -1490,7 +1513,7 @@ static void l2cap_retransmit_one_frame(struct sock *sk, u8 tx_seq)
{
struct l2cap_pinfo *pi = l2cap_pi(sk);
struct sk_buff *skb, *tx_skb;
- u16 control, fcs;
+ u16 control;
skb = skb_peek(TX_QUEUE(sk));
if (!skb)
@@ -1525,10 +1548,8 @@ static void l2cap_retransmit_one_frame(struct sock *sk, u8 tx_seq)
put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
- if (pi->fcs == L2CAP_FCS_CRC16) {
- fcs = crc16(0, (u8 *)tx_skb->data, tx_skb->len - 2);
- put_unaligned_le16(fcs, tx_skb->data + tx_skb->len - 2);
- }
+ if (pi->fcs == L2CAP_FCS_CRC16)
+ apply_fcs(tx_skb);
l2cap_do_send(sk, tx_skb);
}
@@ -1537,7 +1558,7 @@ static int l2cap_ertm_send(struct sock *sk)
{
struct sk_buff *skb, *tx_skb;
struct l2cap_pinfo *pi = l2cap_pi(sk);
- u16 control, fcs;
+ u16 control;
int nsent = 0;
if (sk->sk_state != BT_CONNECTED)
@@ -1567,10 +1588,8 @@ static int l2cap_ertm_send(struct sock *sk)
put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
- if (pi->fcs == L2CAP_FCS_CRC16) {
- fcs = crc16(0, (u8 *)skb->data, tx_skb->len - 2);
- put_unaligned_le16(fcs, skb->data + tx_skb->len - 2);
- }
+ if (pi->fcs == L2CAP_FCS_CRC16)
+ apply_fcs(tx_skb);
l2cap_do_send(sk, tx_skb);
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [RFC 0/7] L2CAP fragmentation changes
From: Mat Martineau @ 2010-08-10 19:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm
Since the previous L2CAP patch set has not been fully merged, I'm only
posting these patches for review at this time.
ERTM and streaming mode currently have a limitation where the
trasmitted PDU size cannot exceed the HCI MTU. This is a problem with
some basebands with small HCI MTU values (some are around 300 bytes),
since it is not possible to use larger BR/EDR packets over the air
when the PDUs are too small. Bandwidth is also wasted with extra
L2CAP header overhead. Patches 1-3 add the capability to calculate
checksums on PDUs that have HCI continuation fragments.
Patches 4-6 change the way ERTM PDUs are reassembled, to avoid
extra data copying at receive time. At higher AMP data rates,
this efficiency improvement becomes more important. Each PDU
is linked together using skbuff fragments, similar to the way
outgoing data uses skbuff frag_lists for HCI continuations. This
way, L2CAP must only copy the data to a linear buffer when it
is copied out to userspace.
Patch 7 changes the max PDU size configuration to allow larger PDU
sizes to be set up, enabling ERTM and streaming mode to send larger
PDUs.
^ permalink raw reply
* [PATCH 5/5] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281466850-5297-1-git-send-email-mathewm@codeaurora.org>
L2CAP ERTM sockets can be opened with the SOCK_STREAM socket type,
which is a mandatory request for ERTM mode.
However, these sockets still have SOCK_SEQPACKET read semantics when
bt_sock_recvmsg() is used to pull data from the receive queue. If the
application is only reading part of a frame, then the unread portion
of the frame is discarded. If the application requests more bytes
than are in the current frame, only the current frame's data is
returned.
This patch utilizes common code derived from RFCOMM's recvmsg()
function to make L2CAP SOCK_STREAM reads behave like RFCOMM reads (and
other SOCK_STREAM sockets in general). The application may read one
byte at a time from the input stream and not lose any data, and may
also read across L2CAP frame boundaries.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 61aa961..8d362d7 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1959,6 +1959,9 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
release_sock(sk);
+ if (sock->type == SOCK_STREAM)
+ return bt_sock_stream_recvmsg(iocb, sock, msg, len, flags);
+
return bt_sock_recvmsg(iocb, sock, msg, len, flags);
}
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 4/5] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM.
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281466850-5297-1-git-send-email-mathewm@codeaurora.org>
To reduce code duplication, have rfcomm_sock_recvmsg() call
bt_sock_stream_recvmsg(). The common bt_sock_stream_recvmsg()
code is nearly identical, with the RFCOMM-specific functionality
for deferred setup and connection unthrottling left in
rfcomm_sock_recvmsg().
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/rfcomm/sock.c | 104 +++----------------------------------------
1 files changed, 6 insertions(+), 98 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..4396f47 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -617,121 +617,29 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
return sent;
}
-static long rfcomm_sock_data_wait(struct sock *sk, long timeo)
-{
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue(sk_sleep(sk), &wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
-
- if (!skb_queue_empty(&sk->sk_receive_queue) ||
- sk->sk_err ||
- (sk->sk_shutdown & RCV_SHUTDOWN) ||
- signal_pending(current) ||
- !timeo)
- break;
-
- set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- release_sock(sk);
- timeo = schedule_timeout(timeo);
- lock_sock(sk);
- clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- }
-
- __set_current_state(TASK_RUNNING);
- remove_wait_queue(sk_sleep(sk), &wait);
- return timeo;
-}
-
static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t size, int flags)
{
struct sock *sk = sock->sk;
struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
- int err = 0;
- size_t target, copied = 0;
- long timeo;
+ int len;
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
rfcomm_dlc_accept(d);
return 0;
}
- if (flags & MSG_OOB)
- return -EOPNOTSUPP;
-
- msg->msg_namelen = 0;
-
- BT_DBG("sk %p size %zu", sk, size);
+ len = bt_sock_stream_recvmsg(iocb, sock, msg, size, flags);
lock_sock(sk);
+ if (!(flags & MSG_PEEK) && len > 0)
+ atomic_sub(len, &sk->sk_rmem_alloc);
- target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
- timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
-
- do {
- struct sk_buff *skb;
- int chunk;
-
- skb = skb_dequeue(&sk->sk_receive_queue);
- if (!skb) {
- if (copied >= target)
- break;
-
- if ((err = sock_error(sk)) != 0)
- break;
- if (sk->sk_shutdown & RCV_SHUTDOWN)
- break;
-
- err = -EAGAIN;
- if (!timeo)
- break;
-
- timeo = rfcomm_sock_data_wait(sk, timeo);
-
- if (signal_pending(current)) {
- err = sock_intr_errno(timeo);
- goto out;
- }
- continue;
- }
-
- chunk = min_t(unsigned int, skb->len, size);
- if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- if (!copied)
- copied = -EFAULT;
- break;
- }
- copied += chunk;
- size -= chunk;
-
- sock_recv_ts_and_drops(msg, sk, skb);
-
- if (!(flags & MSG_PEEK)) {
- atomic_sub(chunk, &sk->sk_rmem_alloc);
-
- skb_pull(skb, chunk);
- if (skb->len) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- kfree_skb(skb);
-
- } else {
- /* put message back and return */
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- } while (size);
-
-out:
if (atomic_read(&sk->sk_rmem_alloc) <= (sk->sk_rcvbuf >> 2))
rfcomm_dlc_unthrottle(rfcomm_pi(sk)->dlc);
-
release_sock(sk);
- return copied ? : err;
+
+ return len;
}
static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname, char __user *optval, unsigned int optlen)
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 3/5] Bluetooth: Add common code for stream-oriented recvmsg()
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281466850-5297-1-git-send-email-mathewm@codeaurora.org>
This commit adds a bt_sock_stream_recvmsg() function for use by any
Bluetooth code that uses SOCK_STREAM sockets. This code is copied
from rfcomm_sock_recvmsg() with minimal modifications to remove
RFCOMM-specific functionality and improve readability.
L2CAP (with the SOCK_STREAM socket type) and RFCOMM have common needs
when it comes to reading data. Proper stream read semantics require
that applications can read from a stream one byte at a time and not
lose any data. The RFCOMM code already operated on and pulled data
from the underlying L2CAP socket, so very few changes were required to
make the code more generic for use with non-RFCOMM data over L2CAP.
Applications that need more awareness of L2CAP frame boundaries are
still free to use SOCK_SEQPACKET sockets, and may verify that they
connection did not fall back to basic mode by calling getsockopt().
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/bluetooth.h | 2 +
net/bluetooth/af_bluetooth.c | 109 +++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..08b6c2a 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -126,6 +126,8 @@ int bt_sock_unregister(int proto);
void bt_sock_link(struct bt_sock_list *l, struct sock *s);
void bt_sock_unlink(struct bt_sock_list *l, struct sock *s);
int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags);
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t len, int flags);
uint bt_sock_poll(struct file * file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 421c45b..77a26fe 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -265,6 +265,115 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
}
EXPORT_SYMBOL(bt_sock_recvmsg);
+static long bt_sock_data_wait(struct sock *sk, long timeo)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (!skb_queue_empty(&sk->sk_receive_queue))
+ break;
+
+ if (sk->sk_err || (sk->sk_shutdown & RCV_SHUTDOWN))
+ break;
+
+ if (signal_pending(current) || !timeo)
+ break;
+
+ set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ release_sock(sk);
+ timeo = schedule_timeout(timeo);
+ lock_sock(sk);
+ clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ }
+
+ __set_current_state(TASK_RUNNING);
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return timeo;
+}
+
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ size_t target, copied = 0;
+ long timeo;
+
+ if (flags & MSG_OOB)
+ return -EOPNOTSUPP;
+
+ msg->msg_namelen = 0;
+
+ BT_DBG("sk %p size %zu", sk, size);
+
+ lock_sock(sk);
+
+ target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
+ timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
+ do {
+ struct sk_buff *skb;
+ int chunk;
+
+ skb = skb_dequeue(&sk->sk_receive_queue);
+ if (!skb) {
+ if (copied >= target)
+ break;
+
+ if ((err = sock_error(sk)) != 0)
+ break;
+ if (sk->sk_shutdown & RCV_SHUTDOWN)
+ break;
+
+ err = -EAGAIN;
+ if (!timeo)
+ break;
+
+ timeo = bt_sock_data_wait(sk, timeo);
+
+ if (signal_pending(current)) {
+ err = sock_intr_errno(timeo);
+ goto out;
+ }
+ continue;
+ }
+
+ chunk = min_t(unsigned int, skb->len, size);
+ if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ if (!copied)
+ copied = -EFAULT;
+ break;
+ }
+ copied += chunk;
+ size -= chunk;
+
+ sock_recv_ts_and_drops(msg, sk, skb);
+
+ if (!(flags & MSG_PEEK)) {
+ skb_pull(skb, chunk);
+ if (skb->len) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ kfree_skb(skb);
+
+ } else {
+ /* put message back and return */
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ } while (size);
+
+out:
+ release_sock(sk);
+ return copied ? : err;
+}
+EXPORT_SYMBOL(bt_sock_stream_recvmsg);
+
static inline unsigned int bt_accept_poll(struct sock *parent)
{
struct list_head *p, *n;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 2/5] Bluetooth: Validate PSM values in calls to connect() and bind().
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281466850-5297-1-git-send-email-mathewm@codeaurora.org>
Valid L2CAP PSMs are odd numbers, and the least significant bit of the
most significant byte must be 0.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c784703..61aa961 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1008,10 +1008,19 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
goto done;
}
- if (la.l2_psm && __le16_to_cpu(la.l2_psm) < 0x1001 &&
- !capable(CAP_NET_BIND_SERVICE)) {
- err = -EACCES;
- goto done;
+ /* If specified, PSM must be odd and lsb of upper byte must be 0 */
+ if (la.l2_psm) {
+ __u16 psm = __le16_to_cpu(la.l2_psm);
+
+ if ((psm & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
+ if (psm < 0x1001 && !capable(CAP_NET_BIND_SERVICE)) {
+ err = -EACCES;
+ goto done;
+ }
}
write_lock_bh(&l2cap_sk_list.lock);
@@ -1190,6 +1199,12 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
goto done;
}
+ /* PSM must be odd and lsb of upper byte must be 0 */
+ if ((__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
/* Set destination address and psm */
bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 1/5] Bluetooth: Only enable L2CAP FCS for ERTM or streaming.
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1281466850-5297-1-git-send-email-mathewm@codeaurora.org>
This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
with L2CAP modes other than ERTM or streaming. At present, this only
affects the FCS value shown with getsockopt() for basic mode.
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index fadf26b..c784703 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3071,6 +3071,17 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
return 0;
}
+static inline void set_default_fcs(struct l2cap_pinfo *pi)
+{
+ /* FCS is enabled only in ERTM or streaming mode, if one or both
+ * sides request it.
+ */
+ if (pi->mode != L2CAP_MODE_ERTM && pi->mode != L2CAP_MODE_STREAMING)
+ pi->fcs = L2CAP_FCS_NONE;
+ else if (!(pi->conf_state & L2CAP_CONF_NO_FCS_RECV))
+ pi->fcs = L2CAP_FCS_CRC16;
+}
+
static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data)
{
struct l2cap_conf_req *req = (struct l2cap_conf_req *) data;
@@ -3135,9 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
goto unlock;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
- l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+ set_default_fcs(l2cap_pi(sk));
sk->sk_state = BT_CONNECTED;
@@ -3225,9 +3234,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_OUTPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
- l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+ set_default_fcs(l2cap_pi(sk));
sk->sk_state = BT_CONNECTED;
l2cap_pi(sk)->next_tx_seq = 0;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH v4 0/5] L2CAP updates for FCS, valid PSMs, and stream recv
From: Mat Martineau @ 2010-08-10 19:00 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm
The previous version of this patch set was "[PATCH v3 0/9] Bluetooth:
L2CAP updates for PSM validation and ERTM" - four patches were already
merged, and this group of patches incorporates additional feedback
from the v3 iteration.
^ permalink raw reply
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