Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 1/8] Bluetooth:  Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-02 19:38 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-2-git-send-email-mathewm@codeaurora.org>

Hi Mat,

> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 9ba1e8e..aed72f2 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -3127,8 +3127,10 @@ 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)
> +		if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> +			l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> +			(!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> +			l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
>  			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;

this becomes unreadable and my brain starts to throw a core dump. So it
clearly needs to be put into a helper inline function.

Regards

Marcel



^ permalink raw reply

* [PATCH 8/8] Bluetooth:  Use 3-DH5 payload size for default ERTM max PDU size.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 16e412f..93aba17 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -35,7 +35,7 @@
 #define L2CAP_DEFAULT_MAX_TX		3
 #define L2CAP_DEFAULT_RETRANS_TO	2000    /* 2 seconds */
 #define L2CAP_DEFAULT_MONITOR_TO	12000   /* 12 seconds */
-#define L2CAP_DEFAULT_MAX_PDU_SIZE	672
+#define L2CAP_DEFAULT_MAX_PDU_SIZE	1011    /* Sized for 3-DH5 packet */
 #define L2CAP_DEFAULT_ACK_TO		200
 #define L2CAP_LOCAL_BUSY_TRIES		12
 
-- 
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 7/8] Bluetooth:  Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 582975b..8e9fa51 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1923,6 +1923,7 @@ done:
 static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags)
 {
 	struct sock *sk = sock->sk;
+	int len_or_err;
 
 	lock_sock(sk);
 
@@ -1956,7 +1957,13 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
 
 	release_sock(sk);
 
-	return bt_sock_recvmsg(iocb, sock, msg, len, flags);
+	if (sock->type == SOCK_STREAM)
+		len_or_err = bt_sock_stream_recvmsg(iocb, sock, msg, len,
+						flags);
+	else
+		len_or_err = bt_sock_recvmsg(iocb, sock, msg, len, flags);
+
+	return len_or_err;
 }
 
 static int l2cap_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 6/8] Bluetooth:  Move stream-oriented recvmsg code so it can be used by L2CAP.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/bluetooth.h |    2 +
 net/bluetooth/af_bluetooth.c      |  107 +++++++++++++++++++++++++++++++++++++
 net/bluetooth/rfcomm/sock.c       |  104 ++---------------------------------
 3 files changed, 115 insertions(+), 98 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..73047f5 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -265,6 +265,113 @@ 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) ||
+		    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;
+}
+
+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;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..5c92929 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_or_err;
 
 	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_or_err = bt_sock_stream_recvmsg(iocb, sock, msg, size, flags);
 
 	lock_sock(sk);
+	if (!(flags & MSG_PEEK) && len_or_err > 0)
+		atomic_sub(len_or_err, &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_or_err;
 }
 
 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 5/8] Bluetooth:  Don't modify remote_tx_win when receiving a config response. Only config requests should set remote_tx_win.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9780ab0..582975b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2808,7 +2808,6 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
 	if (*result == L2CAP_CONF_SUCCESS) {
 		switch (rfc.mode) {
 		case L2CAP_MODE_ERTM:
-			pi->remote_tx_win   = rfc.txwin_size;
 			pi->retrans_timeout = le16_to_cpu(rfc.retrans_timeout);
 			pi->monitor_timeout = le16_to_cpu(rfc.monitor_timeout);
 			pi->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

* [PATCH 4/8] Bluetooth:  Do endianness conversion on MPS configuration value before doing comparisons.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 44bc6ee..9780ab0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2708,10 +2708,10 @@ done:
 		case L2CAP_MODE_ERTM:
 			pi->remote_tx_win = rfc.txwin_size;
 			pi->remote_max_tx = rfc.max_transmit;
-			if (rfc.max_pdu_size > pi->conn->mtu - 10)
-				rfc.max_pdu_size = le16_to_cpu(pi->conn->mtu - 10);
 
 			pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
+			if (pi->remote_mps > pi->conn->mtu - 10)
+				pi->remote_mps = pi->conn->mtu - 10;
 
 			rfc.retrans_timeout =
 				le16_to_cpu(L2CAP_DEFAULT_RETRANS_TO);
@@ -2726,10 +2726,9 @@ done:
 			break;
 
 		case L2CAP_MODE_STREAMING:
-			if (rfc.max_pdu_size > pi->conn->mtu - 10)
-				rfc.max_pdu_size = le16_to_cpu(pi->conn->mtu - 10);
-
 			pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
+			if (pi->remote_mps > pi->conn->mtu - 10)
+				pi->remote_mps = pi->conn->mtu - 10;
 
 			pi->conf_state |= L2CAP_CONF_MODE_DONE;
 
-- 
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/8] Bluetooth:  Validate PSM values in calls to connect() and bind().
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index aed72f2..44bc6ee 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1008,6 +1008,12 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 		goto done;
 	}
 
+	/* If specified, PSM must be odd and lsb of upper byte must be 0 */
+	if (la.l2_psm && (__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
+		err = -EINVAL;
+		goto done;
+	}
+
 	if (la.l2_psm && __le16_to_cpu(la.l2_psm) < 0x1001 &&
 				!capable(CAP_NET_BIND_SERVICE)) {
 		err = -EACCES;
@@ -1190,6 +1196,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 2/8] Bluetooth:  Change default ERTM retransmit timeout to 2 seconds, as the spec requires.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/l2cap.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 636724b..16e412f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -33,7 +33,7 @@
 #define L2CAP_DEFAULT_FLUSH_TO		0xffff
 #define L2CAP_DEFAULT_TX_WINDOW		63
 #define L2CAP_DEFAULT_MAX_TX		3
-#define L2CAP_DEFAULT_RETRANS_TO	1000    /* 1 second */
+#define L2CAP_DEFAULT_RETRANS_TO	2000    /* 2 seconds */
 #define L2CAP_DEFAULT_MONITOR_TO	12000   /* 12 seconds */
 #define L2CAP_DEFAULT_MAX_PDU_SIZE	672
 #define L2CAP_DEFAULT_ACK_TO		200
-- 
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/8] Bluetooth:  Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>


Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..aed72f2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3127,8 +3127,10 @@ 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)
+		if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
+			l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
+			(!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
+			l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
 			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
 
 		sk->sk_state = BT_CONNECTED;
@@ -3217,8 +3219,10 @@ 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)
+		if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
+			l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
+			(!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
+			l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
 			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
 
 		sk->sk_state = BT_CONNECTED;
-- 
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 0/8] Bluetooth:  L2CAP updates for PSM validation and ERTM.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm

This is a collection of ERTM-related patches, including some L2CAP 
configuration fixes and allowing partial-frame reads from L2CAP
SOCK_STREAM sockets.  PSM validation is also included, although that
is not restricted to ERTM.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: Getting link quality or RSSI
From: Marcel Holtmann @ 2010-08-02 18:58 UTC (permalink / raw)
  To: Davide Pesavento; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQeQ-XoM5yuWQYx5_1-xWf1H0ax_J=BbtDfG2T@mail.gmail.com>

Hi Davide,

> >> I'm writing an application to retrieve the current link quality (or
> >> RSSI) between my laptop's Bluetooth adapter and a specific remote
> >> device (my mobile phone). In order to do that I'm of course using the
> >> latest version of BlueZ, but I've encountered several issues so far.
> >>
> >> The dbus API exposed by bluetoothd doesn't have any GetRSSI or
> >> GetLinkQuality methods. Would it be possible to export these methods
> >> in the public API?
> >
> > the link quality is vendor specific according to the specification and
> > the RSSI of an existing connection is rather useless. So we don't bother
> > to export those.
> >
> 
> Does this mean you're going to reject a patch which adds those methods
> to the dbus API?

constantly polling them via D-Bus, yes I would reject such a patch. To
make this proper you would need a kernel patch first that polls the RSSI
and/or link quality when a ACL is established and not in power saving
mode. Then you need to use this data to send async signals via D-Bus.

I have done both, let me assure you that some chips don't provide proper
RSSI values. Then link quality is vendor specific and we can't do
anything real useful with it (except it is a CSR chip). So I have don't
the whole exercise and figured out that it is rather useless feature of
Bluetooth.

> >> Then I looked at the bluetooth HCI library that comes with bluez.
> >> First of all, is there any reasons why it's totally undocumented?
> >> Anyway, I noticed hci_read_link_quality() and hci_read_rssi() in
> >> hci_lib.h and tried using them. Since they seem to require an
> >> established connection, I also used hci_create_connection(). However I
> >> soon discovered that creating a connection requires root privileges,
> >> is that right or did I do something wrong?
> >>
> >> Furthermore, even when running the program as root, the connection
> >> gets established only for a few seconds and then it disconnects from
> >> the remote device. Is this behaviour intended? How can I specify to
> >> keep the connection alive indefinitely?
> >
> > If you don't have an active connection that is used, the kernel will
> > terminate any idle ones. So using hcitool for this is rather pointless
> > unless you have a profile already using that connection.
> >
> 
> "using a connection" means pushing some traffic over it?
> Is there no other way to prevent the kernel from terminating idle connections?

You don't need to transfer data, but you need a reference count on the
ACL link. And that can only happen by opening a L2CAP socket.

> By the way, I've read in the Bluetooth specs that there exists an
> extended inquiry mode which allows the host to gather the RSSI of
> available devices too. How can I perform such kind of inquiry from my
> application using bluez?

Nice idea, but RSSI from inquiry result and RSSI from an ACL are not the
same. You can't compare them properly to make sense out of them. I tried
that as well. Please read the specification again to see their
difference when it comes to power control on the low level baseband.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH v4 0/5] Enhanced support for extended inquiry response
From: Johan Hedberg @ 2010-08-02 18:55 UTC (permalink / raw)
  To: ingas; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <feb3c412e1be4c1b7b42efdbc581aabb.squirrel@www.codeaurora.org>

Hi Inga,

First of all, sorry for the slight delay in my reply. I've been on
holiday last week with quite limted email processing capability.

On Wed, Jul 28, 2010, ingas@codeaurora.org wrote:
> In the current implementation,  EIR write is keyed off HCI Command
> Complete events for a set of commands (in security.c).
>  - When first registering an OPP service with sdptool, the
> update_svcclass_list()  (sdpd-service.c) calls manager_update_svc()
> (manager.c) which in turn calls adapter_update()  (adapter.c).  BTW, why
> this level of indirection, since manager_update_svc() is called only from
> one place in the code and calling adapter_update()  is all it does? 
> Probably some earlier architectural decision I am not aware of…

That looks pretty stupid to me too. Might be some historical artifact.
Anyway, feel free to send a patch to remove this indirection (i.e. call
adapter_update directly).

> The current implementation may be extended as following:
> Since update_svcclass_list() is being called throughout sdpd-service.c in
> all the places where EIR needs to be updated as well, I propose to modify
> the corresponding function  adapter_set_service_classes() (adapter.c) :
> 
> **********************
> Before:
> /* If we already have the CoD we want or the cache is enabled or an
> * existing CoD write is in progress just bail out */
> if (adapter->current_cod == adapter->wanted_cod ||
> 	adapter->cache_enable || adapter->pending_cod)
> return 0;
> 
> 
> ***********************
> 
> After:
> /* If we already have the CoD we want or the cache is enabled or an
> * existing CoD write is in progress just bail out */
> if (adapter->cache_enable || adapter->pending_cod)
> return 0;
> 
> /* If we already have the CoD we want, update EIR and return */
> if (adapter->current_cod == adapter->wanted_cod ) {
> 	update_ext_inquiry_response(adapter);
> 	return 0;
> }
> 
> **********************
> Will that be acceptable? I tested this modification and it’s working with
> both d-bus methods and sdptool.

Seems good to me.

> As a side note, in future it might be a good idea to expose EIR writes to
> an application layer providing external API. The intent of EIR is to allow
> quick scan of the surroundings to find a particular service that a device
> is interested at the moment (and in turn, exposing the services that the
> device choses to expose at the moment) without actually going into full
> blown connection mode.   EIR space is at a premium for sophisticated
> devices with multiple services. Not every single service needs to be
> exposed in EIR. It would be nice to be able to pick which one to add. For
> example, if a device supports both headset and handsfree profiles (as all
> of the phones do), it might not be necessary to advertise headset uuid in
> EIR since the headsets  out there are mostly legacy devices that cannot
> perform EIR anyway… Also, as more new services that use uuid128 are
> introduced, the EIR buffer pretty quickly if every single one of them is
> added without discretion. Just something to throw out there :)

Good point (not that any concrete examples of such a device/situation
would have come accross yet). One possibility to solve the issue without
directly exposing this to applications would be to have a (rather
static) setting in main.conf of UUIDs to be prioritized over others in
the EIR data.

Johan

^ permalink raw reply

* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Marcel Holtmann @ 2010-08-02 18:53 UTC (permalink / raw)
  To: tmonahan
  Cc: David Vrabel, Inga Stotland, linux-bluetooth, rshaffer,
	johan.hedberg
In-Reply-To: <8bfb018fa669b6ace6840881379a1a01.squirrel@www.codeaurora.org>

Hi Tim,

please fix your mail client to NOT break threading.

> >> AMP vs BR/EDR preference for L2CAP channel can be configured as command
> line argument using new option "-J". Possible values:
> >>                  "require_br_edr",
> >>                  "prefer_amp",
> >>                  "prefer_br_edr"
> >> If no preference indicated, the default is set to require BR/EDR.
> >
> > I think an explicit channel move command and a channel move complete
> event are what many applications/profiles will need to be able to use an
> AMP effectively.
> 
> A future AMP policy setting of "manual" could certainly give each
> application the freedom to move an l2cap channel at will, after applying
> its own decision making process. However, with that freedom comes the
> responsibility of processing the AMP related events and initiating the AMP
> commands to make the decision. For example, the application needs to know
> if AMP controllers are available both locally and remotely, which in turn
> requires initiating the AMP discovery process and looking over the
> available local HCI devices; a loss of the physical link event means the
> application must move the link back to BR/EDR, etc. All of the additional
> machinery to accomplish that could indeed be exposed for use by
> applications. I think such an approach, if really needed, would not
> conflict with these proposed policies that hide all of that within the
> kernel.
> 
> To illustrate how these policies simplify applications, consider OBEX. An
> OBEX server application would choose "prefer_br_edr", thus leaving the
> remote OBEX client to decide and initiate the channel move to an AMP, and
> thus granting advance permission to the kernel for the move to take place.
> The OBEX client could examine the size of an upcoming transfer and choose
> "prefer_amp", thereby instructing the kernel to initiate the move to an
> AMP if all local and remote conditions allow it. Thus, the OBEX client and
> server applications make simplistic decisions to set up AMP, and can focus
> on moving data over the l2cap channel, leaving the management of what
> medium the data flows over up to the kernel, who keeps it transparent. In
> this way, work to make an existing profile "AMP aware" is also minimal.
> 
> In the discussion with Marcel regarding our AMP proposal in March ("RFC:
> QuIC's AMP + eL2CAP Technical Plans"), where we had listed these AMP
> control policies, he seemed to indicate that these policies would
> automatically trigger AMP discovery within the kernel, which it will. As
> he said, "Less options are less confusing for users" (apologies in advance
> if I misinterpreted his answer).

my current approach here is still that we wanna do this automatically in
the background for the profiles. I think at most the profiles should
just provide a threshold for either a) staying on BR/EDR or b) moving
over to AMP.

I know that the Bluetooth SIG tries to define such policies as part of
the profiles (like FTP for example), but I think this makes sense. I
doubt the real usage of making this manual. A profile/application should
just give its preference or intent and then we try to accommodate them
as best as possible.

The other problem here is also that besides whatever the profiles wants,
it might happen that the WiFi policy is stronger and only allows using
the WiFi device as WiFi or as AMP. And if WiFi wins, then we have to rip
the AMP out of the system. At that point whatever the Bluetooth profiles
wanted is pointless since the AMP controller is gone anyway.

That said, nothing stops us from allowing to make our AMP polices
dynamic and allow the application to change it threshold during
lifetime. For example it starts out as BR/EDR only and then during the
usage of the link it decides that now AMP preferred would make sense. In
that sense you would have manual switching to some level.

Regards

Marcel



^ permalink raw reply

* Re: Question about a patch set that is comprised of patches for separate git repositories
From: Marcel Holtmann @ 2010-08-02 18:45 UTC (permalink / raw)
  To: ingas; +Cc: linux-bluetooth
In-Reply-To: <1c6058affcf5b834ec33d0cd2bf43c61.squirrel@www.codeaurora.org>

Hi Inga,

> I am planning on submitting a patch set for hcidump that allows parsing of
> AMP related HCI commands and events. While the modification of parsing
> code is constrained to bluez-hcidump git repository, the code relies on
> changes to hci.h file that resides in bluez git repository. Basically, I
> had to create new structures and defines for OCFs and attributes and hci.h
> seemed like a logical place to do that. However, nothing else in bluez
> subdirectory is referring those new structs and constants. So the question
> is: do I have 2 separate submissions: one for bluez and the other for
> hcidump noting that hcidump depends on the patch for hci.h in bluez, or do
> I somehow generate a combined patch set?

send the patch for hci.h first. Then we see. The LE additions took like
a few rounds of review. So we do these first.

Regards

Marcel



^ permalink raw reply

* Question about a patch set that is comprised of patches for separate git repositories
From: ingas @ 2010-08-02 18:19 UTC (permalink / raw)
  To: marcel; +Cc: linux-bluetooth

Hi Marcel,

I am planning on submitting a patch set for hcidump that allows parsing of
AMP related HCI commands and events. While the modification of parsing
code is constrained to bluez-hcidump git repository, the code relies on
changes to hci.h file that resides in bluez git repository. Basically, I
had to create new structures and defines for OCFs and attributes and hci.h
seemed like a logical place to do that. However, nothing else in bluez
subdirectory is referring those new structs and constants. So the question
is: do I have 2 separate submissions: one for bluez and the other for
hcidump noting that hcidump depends on the patch for hci.h in bluez, or do
I somehow generate a combined patch set?

Thank you,

Inga

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

* Re: Getting link quality or RSSI
From: Luiz Augusto von Dentz @ 2010-08-02 18:12 UTC (permalink / raw)
  To: Davide Pesavento; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTimQeQ-XoM5yuWQYx5_1-xWf1H0ax_J=BbtDfG2T@mail.gmail.com>

Hi,

On Mon, Aug 2, 2010 at 7:34 PM, Davide Pesavento <davidepesa@gmail.com> wrote:
> "using a connection" means pushing some traffic over it?
> Is there no other way to prevent the kernel from terminating idle connections?

You should really think twice if you are creating a connection just to
see the link quality, it really doesn't worth.

> By the way, I've read in the Bluetooth specs that there exists an
> extended inquiry mode which allows the host to gather the RSSI of
> available devices too. How can I perform such kind of inquiry from my
> application using bluez?

We do expose the RSSI when doing the device discovery, take a look at
DeviceFound documentation under doc/adapter-api.txt, as for link
quality maybe it can be made as a Property of device object, but as
Marcel said this is vendor specific so the use cases are very limited.
In the other hand it would be very nice if ui could show the link
quality and we could possible use it to route the audio back to the
speaker if the connection quality is too low.


-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: tmonahan @ 2010-08-02 16:55 UTC (permalink / raw)
  To: David Vrabel
  Cc: Inga Stotland, linux-bluetooth, rshaffer, johan.hedberg, marcel

Hi, David,

>> AMP vs BR/EDR preference for L2CAP channel can be configured as command
line argument using new option "-J". Possible values:
>>                  "require_br_edr",
>>                  "prefer_amp",
>>                  "prefer_br_edr"
>> If no preference indicated, the default is set to require BR/EDR.
>
> I think an explicit channel move command and a channel move complete
event are what many applications/profiles will need to be able to use an
AMP effectively.

A future AMP policy setting of "manual" could certainly give each
application the freedom to move an l2cap channel at will, after applying
its own decision making process. However, with that freedom comes the
responsibility of processing the AMP related events and initiating the AMP
commands to make the decision. For example, the application needs to know
if AMP controllers are available both locally and remotely, which in turn
requires initiating the AMP discovery process and looking over the
available local HCI devices; a loss of the physical link event means the
application must move the link back to BR/EDR, etc. All of the additional
machinery to accomplish that could indeed be exposed for use by
applications. I think such an approach, if really needed, would not
conflict with these proposed policies that hide all of that within the
kernel.

To illustrate how these policies simplify applications, consider OBEX. An
OBEX server application would choose "prefer_br_edr", thus leaving the
remote OBEX client to decide and initiate the channel move to an AMP, and
thus granting advance permission to the kernel for the move to take place.
The OBEX client could examine the size of an upcoming transfer and choose
"prefer_amp", thereby instructing the kernel to initiate the move to an
AMP if all local and remote conditions allow it. Thus, the OBEX client and
server applications make simplistic decisions to set up AMP, and can focus
on moving data over the l2cap channel, leaving the management of what
medium the data flows over up to the kernel, who keeps it transparent. In
this way, work to make an existing profile "AMP aware" is also minimal.

In the discussion with Marcel regarding our AMP proposal in March ("RFC:
QuIC's AMP + eL2CAP Technical Plans"), where we had listed these AMP
control policies, he seemed to indicate that these policies would
automatically trigger AMP discovery within the kernel, which it will. As
he said, "Less options are less confusing for users" (apologies in advance
if I misinterpreted his answer).

Best regards,
Tim Monahan-Mitchell

-- 
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

* Re: Getting link quality or RSSI
From: Davide Pesavento @ 2010-08-02 16:34 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1280764595.12579.19.camel@localhost.localdomain>

On Mon, Aug 2, 2010 at 17:56, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Davide,
>
>> I'm writing an application to retrieve the current link quality (or
>> RSSI) between my laptop's Bluetooth adapter and a specific remote
>> device (my mobile phone). In order to do that I'm of course using the
>> latest version of BlueZ, but I've encountered several issues so far.
>>
>> The dbus API exposed by bluetoothd doesn't have any GetRSSI or
>> GetLinkQuality methods. Would it be possible to export these methods
>> in the public API?
>
> the link quality is vendor specific according to the specification and
> the RSSI of an existing connection is rather useless. So we don't bother
> to export those.
>

Does this mean you're going to reject a patch which adds those methods
to the dbus API?

>> Then I looked at the bluetooth HCI library that comes with bluez.
>> First of all, is there any reasons why it's totally undocumented?
>> Anyway, I noticed hci_read_link_quality() and hci_read_rssi() in
>> hci_lib.h and tried using them. Since they seem to require an
>> established connection, I also used hci_create_connection(). However I
>> soon discovered that creating a connection requires root privileges,
>> is that right or did I do something wrong?
>>
>> Furthermore, even when running the program as root, the connection
>> gets established only for a few seconds and then it disconnects from
>> the remote device. Is this behaviour intended? How can I specify to
>> keep the connection alive indefinitely?
>
> If you don't have an active connection that is used, the kernel will
> terminate any idle ones. So using hcitool for this is rather pointless
> unless you have a profile already using that connection.
>

"using a connection" means pushing some traffic over it?
Is there no other way to prevent the kernel from terminating idle connections?

By the way, I've read in the Bluetooth specs that there exists an
extended inquiry mode which allows the host to gather the RSSI of
available devices too. How can I perform such kind of inquiry from my
application using bluez?

Thanks,
Davide

^ permalink raw reply

* Re: Getting link quality or RSSI
From: Marcel Holtmann @ 2010-08-02 15:56 UTC (permalink / raw)
  To: Davide Pesavento; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimA4dnDeoZVX1N4c=2sR1JkgAFWb4d0-0K+8S1C@mail.gmail.com>

Hi Davide,

> I'm writing an application to retrieve the current link quality (or
> RSSI) between my laptop's Bluetooth adapter and a specific remote
> device (my mobile phone). In order to do that I'm of course using the
> latest version of BlueZ, but I've encountered several issues so far.
> 
> The dbus API exposed by bluetoothd doesn't have any GetRSSI or
> GetLinkQuality methods. Would it be possible to export these methods
> in the public API?

the link quality is vendor specific according to the specification and
the RSSI of an existing connection is rather useless. So we don't bother
to export those.

> Then I looked at the bluetooth HCI library that comes with bluez.
> First of all, is there any reasons why it's totally undocumented?
> Anyway, I noticed hci_read_link_quality() and hci_read_rssi() in
> hci_lib.h and tried using them. Since they seem to require an
> established connection, I also used hci_create_connection(). However I
> soon discovered that creating a connection requires root privileges,
> is that right or did I do something wrong?
> 
> Furthermore, even when running the program as root, the connection
> gets established only for a few seconds and then it disconnects from
> the remote device. Is this behaviour intended? How can I specify to
> keep the connection alive indefinitely?

If you don't have an active connection that is used, the kernel will
terminate any idle ones. So using hcitool for this is rather pointless
unless you have a profile already using that connection.

Regards

Marcel



^ permalink raw reply

* Getting link quality or RSSI
From: Davide Pesavento @ 2010-08-02 14:51 UTC (permalink / raw)
  To: linux-bluetooth

Hello,

I'm writing an application to retrieve the current link quality (or
RSSI) between my laptop's Bluetooth adapter and a specific remote
device (my mobile phone). In order to do that I'm of course using the
latest version of BlueZ, but I've encountered several issues so far.

The dbus API exposed by bluetoothd doesn't have any GetRSSI or
GetLinkQuality methods. Would it be possible to export these methods
in the public API?

Then I looked at the bluetooth HCI library that comes with bluez.
First of all, is there any reasons why it's totally undocumented?
Anyway, I noticed hci_read_link_quality() and hci_read_rssi() in
hci_lib.h and tried using them. Since they seem to require an
established connection, I also used hci_create_connection(). However I
soon discovered that creating a connection requires root privileges,
is that right or did I do something wrong?

Furthermore, even when running the program as root, the connection
gets established only for a few seconds and then it disconnects from
the remote device. Is this behaviour intended? How can I specify to
keep the connection alive indefinitely?

Thanks a lot in advance for any help!

Regards,
Davide

^ permalink raw reply

* Re: [PATCH 1/3] Add BT3 AMP device support, by Atheros Linux BT3 team.
From: David Vrabel @ 2010-08-02 14:39 UTC (permalink / raw)
  To: Dan Tian; +Cc: linux-bluetooth@vger.kernel.org, Haijun Liu, Luis Rodriguez
In-Reply-To: <CDAFEDABF718A54BABD0DA5476695307CE881EC1@SHEXMB-01.global.atheros.com>

I've now given these patches a brief try and have successfully
established a physical and logical link and used these to transfer data.

Some general comments:

1. There is almost zero documentation.  This is particularly a problem
for the userspace ABI (e.g., the additional fields in struct
l2cap_options).  It would also be useful to add comments for code that
handles the more quirky parts of the spec (e.g., the use of the physical
link handle in Rx'd AMP ACL packets).

2. Lockdep identifies a locking bug (see below for complete trace) when
creating a physical link on the responder.

3. The decision on whether to use the AMP logical link or the BR radio
is taken too late.  The L2CAP packet are fragmented using the MTU of the
BR radio and this MTU is far too small to get good performance out of
AMP radios.

4. There is no API to signal to the user space application/profile that
the channel has moved to/from the AMP.

[  470.022933] =======================================================
[  470.022936] [ INFO: possible circular locking dependency detected ]
[  470.022939] 2.6.35-rc4 #1
[  470.022941] -------------------------------------------------------
[  470.022944] ksoftirqd/0/3 is trying to acquire lock:
[  470.022946]  (slock-AF_BLUETOOTH){+.-...}, at: [<f829bfdf>]
l2cap_phylink_update+0x2ef/0x11b0 [l2cap]
[  470.022961]
[  470.022962] but task is already holding lock:
[  470.022964]  (&amp_contr->context_q.lock){+.-...}, at: [<f82af8f7>]
amp_create_phy_link_cfm+0x57/0x5c0 [l2cap]
[  470.022977]
[  470.022977] which lock already depends on the new lock.
[  470.022978]
[  470.022981]
[  470.022981] the existing dependency chain (in reverse order) is:
[  470.022984]
[  470.022985] -> #1 (&amp_contr->context_q.lock){+.-...}:
[  470.022990]        [<c01691b8>] __lock_acquire+0x2f8/0xac0
[  470.022998]        [<c0169a06>] lock_acquire+0x86/0x100
[  470.023002]        [<c03f37e0>] _raw_spin_lock_irqsave+0x50/0x70
[  470.023008]        [<c0135030>] __wake_up+0x20/0x60
[  470.023013]        [<f829a3c4>] l2cap_drop_acked_frames+0x84/0xb0 [l2cap]
[  470.023023]        [<f82a7343>] l2cap_recv_frame+0x3d83/0x7cf0 [l2cap]
[  470.023032]        [<f82abdbd>] l2cap_recv_acldata+0x43d/0x450 [l2cap]
[  470.023041]        [<f9a23186>] hci_rx_task+0x376/0x620 [bluetooth]
[  470.023053]        [<c013f26b>] tasklet_action+0x5b/0xd0
[  470.023059]        [<c013e4d7>] __do_softirq+0xa7/0x1e0
[  470.023063]        [<c013e677>] do_softirq+0x67/0x70
[  470.023067]        [<c013eae6>] irq_exit+0x76/0x80
[  470.023071]        [<c03fa664>] do_IRQ+0x54/0xc0
[  470.023075]        [<c01031ae>] common_interrupt+0x2e/0x40
[  470.023080]        [<c03438f6>] cpuidle_idle_call+0x76/0x100
[  470.023085]        [<c0101c61>] cpu_idle+0x51/0x90
[  470.023089]        [<c03df6aa>] rest_init+0xba/0xd0
[  470.023094]        [<c05a1aea>] start_kernel+0x2fa/0x390
[  470.023099]        [<c05a109c>] i386_start_kernel+0x9c/0xe0
[  470.023103]
[  470.023104] -> #0 (slock-AF_BLUETOOTH){+.-...}:
[  470.023109]        [<c0168eb6>] validate_chain+0x1076/0x1080
[  470.023114]        [<c01691b8>] __lock_acquire+0x2f8/0xac0
[  470.023118]        [<c0169a06>] lock_acquire+0x86/0x100
[  470.023122]        [<c03f36b8>] _raw_spin_lock+0x38/0x50
[  470.023126]        [<f829bfdf>] l2cap_phylink_update+0x2ef/0x11b0 [l2cap]
[  470.023136]        [<f82af9e1>] amp_create_phy_link_cfm+0x141/0x5c0
[l2cap]
[  470.023145]        [<f9a289b1>] hci_phylink_complete_evt+0xc1/0x130
[bluetooth]
[  470.023157]        [<f9a2b536>] hci_event_packet+0x236/0x3cb0 [bluetooth]
[  470.023167]        [<f9a230f9>] hci_rx_task+0x2e9/0x620 [bluetooth]
[  470.023178]        [<c013f26b>] tasklet_action+0x5b/0xd0
[  470.023182]        [<c013e4d7>] __do_softirq+0xa7/0x1e0
[  470.023186]        [<c013e677>] do_softirq+0x67/0x70
[  470.023190]        [<c013e89e>] run_ksoftirqd+0x6e/0x110
[  470.023193]        [<c0153324>] kthread+0x74/0x80
[  470.023198]        [<c01031c6>] kernel_thread_helper+0x6/0x10
[  470.023202]
[  470.023203] other info that might help us debug this:
[  470.023204]
[  470.023207] 5 locks held by ksoftirqd/0/3:
[  470.023209]  #0:  (hci_task_lock){++.-..}, at: [<f9a22e52>]
hci_rx_task+0x42/0x620 [bluetooth]
[  470.023223]  #1:  (&(&hdev->lock)->rlock){+.-...}, at: [<f9a2893d>]
hci_phylink_complete_evt+0x4d/0x130 [bluetooth]
[  470.023236]  #2:  (hci_cb_list_lock){++.-..}, at: [<f9a28986>]
hci_phylink_complete_evt+0x96/0x130 [bluetooth]
[  470.023250]  #3:  (&amp_contr->context_q.lock){+.-...}, at:
[<f82af8f7>] amp_create_phy_link_cfm+0x57/0x5c0 [l2cap]
[  470.023263]  #4:  (&conn->chan_list.lock){++--..}, at: [<f829bfa4>]
l2cap_phylink_update+0x2b4/0x11b0 [l2cap]
[  470.023274]
[  470.023275] stack backtrace:
[  470.023279] Pid: 3, comm: ksoftirqd/0 Not tainted 2.6.35-rc4 #1
[  470.023281] Call Trace:
[  470.023286]  [<c016774b>] print_circular_bug+0xbb/0xc0
[  470.023290]  [<c0168eb6>] validate_chain+0x1076/0x1080
[  470.023294]  [<c01655ba>] ? save_trace+0x3a/0xc0
[  470.023299]  [<c01691b8>] __lock_acquire+0x2f8/0xac0
[  470.023303]  [<c0169a06>] lock_acquire+0x86/0x100
[  470.023312]  [<f829bfdf>] ? l2cap_phylink_update+0x2ef/0x11b0 [l2cap]
[  470.023317]  [<c03f36b8>] _raw_spin_lock+0x38/0x50
[  470.023325]  [<f829bfdf>] ? l2cap_phylink_update+0x2ef/0x11b0 [l2cap]
[  470.023334]  [<f829bfdf>] l2cap_phylink_update+0x2ef/0x11b0 [l2cap]
[  470.023338]  [<c0166615>] ? mark_held_locks+0x55/0x70
[  470.023342]  [<c03f3e85>] ? _raw_spin_unlock_irqrestore+0x55/0x70
[  470.023346]  [<c0166686>] ? trace_hardirqs_on_caller+0x56/0x170
[  470.023350]  [<c01667ab>] ? trace_hardirqs_on+0xb/0x10
[  470.023358]  [<f8295cca>] ? amp_clear_phylink_timer+0x3a/0x70 [l2cap]
[  470.023368]  [<f82af9e1>] amp_create_phy_link_cfm+0x141/0x5c0 [l2cap]
[  470.023372]  [<c0167f29>] ? validate_chain+0xe9/0x1080
[  470.023384]  [<f82af8a0>] ? amp_create_phy_link_cfm+0x0/0x5c0 [l2cap]
[  470.023394]  [<f9a289b1>] hci_phylink_complete_evt+0xc1/0x130 [bluetooth]
[  470.023405]  [<f9a2b536>] hci_event_packet+0x236/0x3cb0 [bluetooth]
[  470.023410]  [<c0127a1c>] ? scale_rt_power+0x2c/0xb0
[  470.023414]  [<c01691b8>] ? __lock_acquire+0x2f8/0xac0
[  470.023427]  [<f9a2f705>] ? hci_send_to_sock+0x45/0x1c0 [bluetooth]
[  470.023438]  [<f9a2f7ab>] ? hci_send_to_sock+0xeb/0x1c0 [bluetooth]
[  470.023444]  [<c03f3c02>] ? _raw_read_unlock+0x22/0x30
[  470.023453]  [<f9a2f7ab>] ? hci_send_to_sock+0xeb/0x1c0 [bluetooth]
[  470.023458]  [<c01667ab>] ? trace_hardirqs_on+0xb/0x10
[  470.023468]  [<f9a230f9>] hci_rx_task+0x2e9/0x620 [bluetooth]
[  470.023472]  [<c03f3e27>] ? _raw_spin_unlock_irq+0x27/0x30
[  470.023476]  [<c013f24b>] ? tasklet_action+0x3b/0xd0
[  470.023480]  [<c0166686>] ? trace_hardirqs_on_caller+0x56/0x170
[  470.023484]  [<c013f26b>] tasklet_action+0x5b/0xd0
[  470.023488]  [<c013e4d7>] __do_softirq+0xa7/0x1e0
[  470.023492]  [<c03f1080>] ? schedule+0x220/0x600
[  470.023496]  [<c013e677>] do_softirq+0x67/0x70
[  470.023500]  [<c013e89e>] run_ksoftirqd+0x6e/0x110
[  470.023504]  [<c013e830>] ? run_ksoftirqd+0x0/0x110
[  470.023508]  [<c0153324>] kthread+0x74/0x80
[  470.023512]  [<c01532b0>] ? kthread+0x0/0x80
[  470.023516]  [<c01031c6>] kernel_thread_helper+0x6/0x10

-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* Re: [PATCH] Notify to device drivers when the SDP records change
From: Luiz Augusto von Dentz @ 2010-08-02 14:39 UTC (permalink / raw)
  To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1280130927-4137-2-git-send-email-santoscadenas@gmail.com>

Hi,

On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> When the remote SDP records that make a driver to be loaded change
> (added, remove, or updated) And the driver is not going to be
> removed, a notification is sent to the device driver. This
> notification is optional annnd does not fail  if the driver does
> not implemment it.

How this is supposed to work? If does seems to me that you guys are
planning to keep polling the services records to update the drivers
which IMO is not a good idea, DiscoverServices is not meant to update
the drivers either it is basically a replacement to sdptool to so
application can do it over dbus. Also if you take a closer look in the
plugins that use rfcomm channel, which can be changed at any point,
they basically fetch the record when attempting to connect, avdtp also
work in a similar way, we have to do the discover + get capabilities
every time we want to connect to a sink/source since there is no way
to keep this information updated locally.

In the other hand, this can be useful for LE which we can subscribe to
changes, but for sdp I don't see the point.

-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* [PATCH] Add NULL checking for name param
From: Radoslaw Jablonski @ 2010-08-02 14:24 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Radoslaw Jablonski

Added NULL checking for "name" param in pbap_open_* functions. If on
this level this variable will be NULL, then setting -EBADR error code
and ending function. This fixes many obexd crashes.
---
 plugins/pbap.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index af4b452..8742a87 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -555,7 +555,7 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
 
 	DBG("name %s type %s pbap %p", name, type, pbap);
 
-	if (type == NULL)
+	if (type == NULL || name == NULL)
 		return -EBADR;
 
 	rsize = obex_aparam_read(os, obj, &buffer);
@@ -689,6 +689,11 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 	}
 
+	if (name == NULL) {
+		ret = -EBADR;
+		goto fail;
+	}
+
 	if (pbap->params->maxlistcount == 0)
 		cb = phonebook_size_result;
 	else
@@ -720,6 +725,10 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 	}
 
+	if (name == NULL) {
+		ret = -EBADR;
+		goto fail;
+	}
 	/* PullvCardListing always get the contacts from the cache */
 
 	if (pbap->cache.valid) {
@@ -758,7 +767,7 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 	}
 
-	if (sscanf(name, "%u.vcf", &handle) != 1) {
+	if (name == NULL || sscanf(name, "%u.vcf", &handle) != 1) {
 		ret = -EBADR;
 		goto fail;
 	}
-- 
1.7.0.4


^ permalink raw reply related

* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: David Vrabel @ 2010-08-02 12:04 UTC (permalink / raw)
  To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, johan.hedberg, marcel
In-Reply-To: <1280514626-22559-1-git-send-email-ingas@codeaurora.org>

Inga Stotland wrote:
> AMP vs BR/EDR preference for L2CAP channel can be configured as 
> command line argument using new option "-J". Possible values: 
>                  "require_br_edr", 
>                  "prefer_amp", 
>                  "prefer_br_edr"
> If no preference indicated, the default is set to require BR/EDR. 

I think an explicit channel move command and a channel move complete
event are what many applications/profiles will need to be able to use an
AMP effectively.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* Re: [PATCH 3/4] Added command argument "-J" to indicate preference of using AMP vs BR/EDR.
From: ingas @ 2010-08-02  6:18 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, rshaffer
In-Reply-To: <20100731231454.GB29039@vigoh>

Hi Gustavo,

> Hi Inga,
>
> * Inga Stotland <ingas@codeaurora.org> [2010-07-30 11:30:25 -0700]:
>
>> Possible values: "require_br_edr", "prefer_amp", "prefer_br_edr"
>> ---
>>  test/l2test.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 files changed, 33 insertions(+), 1 deletions(-)
>
> I would prefer to review and merge the kernel bits first and then go to
> the userspace stuff. Doing that we'll have more sure that the socket
> options won't change.
>
> --
> Gustavo F. Padovan
> http://padovan.org
> --

I don't disagree with you. The kernel bits are coming this week. we will
retest l2test again when kernel stuff is upstreamed. This is just a sample
of how we envision the socket options to control the use of AMP...

And anyway, it takes me up to five iterations to get the style right :)

Regards,

Inga


--
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox