[RFC 0/8] ERTM state machine changes, part 2

[RFC 0/8] ERTM state machine changes, part 2

[Mat Martineau]

This is the second patch series reworking the ERTM state machine and
closely related streaming mode code.  These patches include a couple
of bug fixes, preparation for future patch series, and segmentation of
outgoing L2CAP data.

Mat Martineau (8):
  Bluetooth: Improve ERTM sequence number offset calculation
  Bluetooth: Initialize new l2cap_chan structure members
  Bluetooth: Remove duplicate structure members from bt_skb_cb
  Bluetooth: Move recently-added ERTM header packing functions
  Bluetooth: Move recently-added ERTM header unpacking functions
  Bluetooth: Lock the L2CAP channel when sending
  Bluetooth: Refactor L2CAP ERTM and streaming transmit segmentation
  Bluetooth: Add Code Aurora Forum copyright

 include/net/bluetooth/bluetooth.h |    3 -
 include/net/bluetooth/l2cap.h     |   12 +-
 net/bluetooth/l2cap_core.c        |  435 +++++++++++++++++++++----------------
 3 files changed, 249 insertions(+), 201 deletions(-)

-- 
1.7.10

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

[RFC 1/8] Bluetooth: Improve ERTM sequence number offset calculation

[Mat Martineau]

Instead of using modular division, the offset can be calculated using
only addition and subtraction.  The previous calculation did not work
as intended and was more difficult to understand, involving unsigned
integer underflow and a check for a negative value where one was not
possible.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 22e9ec9..92c0423 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -724,13 +724,10 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
 
 static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
 {
-	int offset;
-
-	offset = (seq1 - seq2) % (chan->tx_win_max + 1);
-	if (offset < 0)
-		offset += (chan->tx_win_max + 1);
-
-	return offset;
+	if (seq1 >= seq2)
+		return seq1 - seq2;
+	else
+		return chan->tx_win_max + 1 - seq2 + seq1;
 }
 
 static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
-- 
1.7.10

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

[RFC 2/8] Bluetooth: Initialize new l2cap_chan structure members

[Mat Martineau]

Structure members used by ERTM or streaming mode need to be
initialized when an ERTM or streaming mode link is configured.  Some
duplicate code is also eliminated by moving in to the ERTM init
function.

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index beb7194..35c0a29 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2315,17 +2315,30 @@ static inline int l2cap_ertm_init(struct l2cap_chan *chan)
 {
 	int err;
 
+	chan->next_tx_seq = 0;
+	chan->expected_tx_seq = 0;
 	chan->expected_ack_seq = 0;
 	chan->unacked_frames = 0;
 	chan->buffer_seq = 0;
 	chan->num_acked = 0;
 	chan->frames_sent = 0;
+	chan->last_acked_seq = 0;
+	chan->sdu = NULL;
+	chan->sdu_last_frag = NULL;
+	chan->sdu_len = 0;
+
+	if (chan->mode != L2CAP_MODE_ERTM)
+		return 0;
+
+	chan->rx_state = L2CAP_RX_STATE_RECV;
+	chan->tx_state = L2CAP_TX_STATE_XMIT;
 
 	INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout);
 	INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout);
 	INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout);
 
 	skb_queue_head_init(&chan->srej_q);
+	skb_queue_head_init(&chan->tx_q);
 
 	INIT_LIST_HEAD(&chan->srej_l);
 	err = l2cap_seq_list_init(&chan->srej_list, chan->tx_win);
@@ -3193,10 +3206,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 
 		l2cap_state_change(chan, BT_CONNECTED);
 
-		chan->next_tx_seq = 0;
-		chan->expected_tx_seq = 0;
-		skb_queue_head_init(&chan->tx_q);
-		if (chan->mode == L2CAP_MODE_ERTM)
+		if (chan->mode == L2CAP_MODE_ERTM ||
+					chan->mode == L2CAP_MODE_STREAMING)
 			err = l2cap_ertm_init(chan);
 
 		if (err < 0)
@@ -3328,10 +3339,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 		set_default_fcs(chan);
 
 		l2cap_state_change(chan, BT_CONNECTED);
-		chan->next_tx_seq = 0;
-		chan->expected_tx_seq = 0;
-		skb_queue_head_init(&chan->tx_q);
-		if (chan->mode ==  L2CAP_MODE_ERTM)
+		if (chan->mode ==  L2CAP_MODE_ERTM ||
+					chan->mode == L2CAP_MODE_STREAMING)
 			err = l2cap_ertm_init(chan);
 
 		if (err < 0)
-- 
1.7.10

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

[RFC 3/8] Bluetooth: Remove duplicate structure members from bt_skb_cb

[Mat Martineau]

These values are now in the nested l2cap_ctrl struct.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 include/net/bluetooth/bluetooth.h |    3 ---
 net/bluetooth/l2cap_core.c        |   34 +++++++++++++++++-----------------
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a6a93..2fb268f 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -235,9 +235,6 @@ struct bt_skb_cb {
 	__u8 pkt_type;
 	__u8 incoming;
 	__u16 expect;
-	__u16 tx_seq;
-	__u8 retries;
-	__u8 sar;
 	__u8 force_active;
 	struct l2cap_ctrl control;
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35c0a29..9a33f21 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1621,7 +1621,7 @@ static void l2cap_drop_acked_frames(struct l2cap_chan *chan)
 
 	while ((skb = skb_peek(&chan->tx_q)) &&
 			chan->unacked_frames) {
-		if (bt_cb(skb)->tx_seq == chan->expected_ack_seq)
+		if (bt_cb(skb)->control.txseq == chan->expected_ack_seq)
 			break;
 
 		skb = skb_dequeue(&chan->tx_q);
@@ -1668,7 +1668,7 @@ static void l2cap_retransmit_one_frame(struct l2cap_chan *chan, u16 tx_seq)
 	if (!skb)
 		return;
 
-	while (bt_cb(skb)->tx_seq != tx_seq) {
+	while (bt_cb(skb)->control.txseq != tx_seq) {
 		if (skb_queue_is_last(&chan->tx_q, skb))
 			return;
 
@@ -1676,13 +1676,13 @@ static void l2cap_retransmit_one_frame(struct l2cap_chan *chan, u16 tx_seq)
 	}
 
 	if (chan->remote_max_tx &&
-			bt_cb(skb)->retries == chan->remote_max_tx) {
+			bt_cb(skb)->control.retries == chan->remote_max_tx) {
 		l2cap_send_disconn_req(chan->conn, chan, ECONNABORTED);
 		return;
 	}
 
 	tx_skb = skb_clone(skb, GFP_ATOMIC);
-	bt_cb(skb)->retries++;
+	bt_cb(skb)->control.retries++;
 
 	control = __get_control(chan, tx_skb->data + L2CAP_HDR_SIZE);
 	control &= __get_sar_mask(chan);
@@ -1718,14 +1718,14 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 	while ((skb = chan->tx_send_head) && (!l2cap_tx_window_full(chan))) {
 
 		if (chan->remote_max_tx &&
-				bt_cb(skb)->retries == chan->remote_max_tx) {
+			bt_cb(skb)->control.retries == chan->remote_max_tx) {
 			l2cap_send_disconn_req(chan->conn, chan, ECONNABORTED);
 			break;
 		}
 
 		tx_skb = skb_clone(skb, GFP_ATOMIC);
 
-		bt_cb(skb)->retries++;
+		bt_cb(skb)->control.retries++;
 
 		control = __get_control(chan, tx_skb->data + L2CAP_HDR_SIZE);
 		control &= __get_sar_mask(chan);
@@ -1749,11 +1749,11 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 
 		__set_retrans_timer(chan);
 
-		bt_cb(skb)->tx_seq = chan->next_tx_seq;
+		bt_cb(skb)->control.txseq = chan->next_tx_seq;
 
 		chan->next_tx_seq = __next_seq(chan, chan->next_tx_seq);
 
-		if (bt_cb(skb)->retries == 1) {
+		if (bt_cb(skb)->control.retries == 1) {
 			chan->unacked_frames++;
 
 			if (!nsent++)
@@ -1979,7 +1979,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
 	if (chan->fcs == L2CAP_FCS_CRC16)
 		put_unaligned_le16(0, skb_put(skb, L2CAP_FCS_SIZE));
 
-	bt_cb(skb)->retries = 0;
+	bt_cb(skb)->control.retries = 0;
 	return skb;
 }
 
@@ -3960,19 +3960,19 @@ static int l2cap_add_to_srej_queue(struct l2cap_chan *chan, struct sk_buff *skb,
 	struct sk_buff *next_skb;
 	int tx_seq_offset, next_tx_seq_offset;
 
-	bt_cb(skb)->tx_seq = tx_seq;
-	bt_cb(skb)->sar = sar;
+	bt_cb(skb)->control.txseq = tx_seq;
+	bt_cb(skb)->control.sar = sar;
 
 	next_skb = skb_peek(&chan->srej_q);
 
 	tx_seq_offset = __seq_offset(chan, tx_seq, chan->buffer_seq);
 
 	while (next_skb) {
-		if (bt_cb(next_skb)->tx_seq == tx_seq)
+		if (bt_cb(next_skb)->control.txseq == tx_seq)
 			return -EINVAL;
 
 		next_tx_seq_offset = __seq_offset(chan,
-				bt_cb(next_skb)->tx_seq, chan->buffer_seq);
+			bt_cb(next_skb)->control.txseq, chan->buffer_seq);
 
 		if (next_tx_seq_offset > tx_seq_offset) {
 			__skb_queue_before(&chan->srej_q, next_skb, skb);
@@ -4144,11 +4144,11 @@ static void l2cap_check_srej_gap(struct l2cap_chan *chan, u16 tx_seq)
 			!test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
 		int err;
 
-		if (bt_cb(skb)->tx_seq != tx_seq)
+		if (bt_cb(skb)->control.txseq != tx_seq)
 			break;
 
 		skb = skb_dequeue(&chan->srej_q);
-		control = __set_ctrl_sar(chan, bt_cb(skb)->sar);
+		control = __set_ctrl_sar(chan, bt_cb(skb)->control.sar);
 		err = l2cap_reassemble_sdu(chan, skb, control);
 
 		if (err < 0) {
@@ -4319,8 +4319,8 @@ expected:
 	chan->expected_tx_seq = __next_seq(chan, chan->expected_tx_seq);
 
 	if (test_bit(CONN_SREJ_SENT, &chan->conn_state)) {
-		bt_cb(skb)->tx_seq = tx_seq;
-		bt_cb(skb)->sar = sar;
+		bt_cb(skb)->control.txseq = tx_seq;
+		bt_cb(skb)->control.sar = sar;
 		__skb_queue_tail(&chan->srej_q, skb);
 		return 0;
 	}
-- 
1.7.10

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

[RFC 4/8] Bluetooth: Move recently-added ERTM header packing functions

[Mat Martineau]

Moving these functions simplifies future patches by eliminating
forward declarations, makes future patches easier to review, and
better preserves 'git blame' information.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c |  102 ++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9a33f21..3221f17 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -725,6 +725,57 @@ static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 	hci_send_acl(chan->conn->hchan, skb, flags);
 }
 
+static u32 __pack_extended_control(struct l2cap_ctrl *control)
+{
+	u32 packed;
+
+	packed = control->reqseq << L2CAP_EXT_CTRL_REQSEQ_SHIFT;
+	packed |= control->final << L2CAP_EXT_CTRL_FINAL_SHIFT;
+
+	if (control->sframe) {
+		packed |= control->poll << L2CAP_EXT_CTRL_POLL_SHIFT;
+		packed |= control->super << L2CAP_EXT_CTRL_SUPER_SHIFT;
+		packed |= L2CAP_EXT_CTRL_FRAME_TYPE;
+	} else {
+		packed |= control->sar << L2CAP_EXT_CTRL_SAR_SHIFT;
+		packed |= control->txseq << L2CAP_EXT_CTRL_TXSEQ_SHIFT;
+	}
+
+	return packed;
+}
+
+static u16 __pack_enhanced_control(struct l2cap_ctrl *control)
+{
+	u16 packed;
+
+	packed = control->reqseq << L2CAP_CTRL_REQSEQ_SHIFT;
+	packed |= control->final << L2CAP_CTRL_FINAL_SHIFT;
+
+	if (control->sframe) {
+		packed |= control->poll << L2CAP_CTRL_POLL_SHIFT;
+		packed |= control->super << L2CAP_CTRL_SUPER_SHIFT;
+		packed |= L2CAP_CTRL_FRAME_TYPE;
+	} else {
+		packed |= control->sar << L2CAP_CTRL_SAR_SHIFT;
+		packed |= control->txseq << L2CAP_CTRL_TXSEQ_SHIFT;
+	}
+
+	return packed;
+}
+
+static inline void __pack_control(struct l2cap_chan *chan,
+				  struct l2cap_ctrl *control,
+				  struct sk_buff *skb)
+{
+	if (test_bit(FLAG_EXT_CTRL, &chan->flags)) {
+		put_unaligned_le32(__pack_extended_control(control),
+				   skb->data + L2CAP_HDR_SIZE);
+	} else {
+		put_unaligned_le16(__pack_enhanced_control(control),
+				   skb->data + L2CAP_HDR_SIZE);
+	}
+}
+
 static inline void l2cap_send_sframe(struct l2cap_chan *chan, u32 control)
 {
 	struct sk_buff *skb;
@@ -787,25 +838,6 @@ static inline void l2cap_send_rr_or_rnr(struct l2cap_chan *chan, u32 control)
 	l2cap_send_sframe(chan, control);
 }
 
-static u16 __pack_enhanced_control(struct l2cap_ctrl *control)
-{
-	u16 packed;
-
-	packed = control->reqseq << L2CAP_CTRL_REQSEQ_SHIFT;
-	packed |= control->final << L2CAP_CTRL_FINAL_SHIFT;
-
-	if (control->sframe) {
-		packed |= control->poll << L2CAP_CTRL_POLL_SHIFT;
-		packed |= control->super << L2CAP_CTRL_SUPER_SHIFT;
-		packed |= L2CAP_CTRL_FRAME_TYPE;
-	} else {
-		packed |= control->sar << L2CAP_CTRL_SAR_SHIFT;
-		packed |= control->txseq << L2CAP_CTRL_TXSEQ_SHIFT;
-	}
-
-	return packed;
-}
-
 static void __unpack_enhanced_control(u16 enh, struct l2cap_ctrl *control)
 {
 	control->reqseq = (enh & L2CAP_CTRL_REQSEQ) >> L2CAP_CTRL_REQSEQ_SHIFT;
@@ -830,25 +862,6 @@ static void __unpack_enhanced_control(u16 enh, struct l2cap_ctrl *control)
 	}
 }
 
-static u32 __pack_extended_control(struct l2cap_ctrl *control)
-{
-	u32 packed;
-
-	packed = control->reqseq << L2CAP_EXT_CTRL_REQSEQ_SHIFT;
-	packed |= control->final << L2CAP_EXT_CTRL_FINAL_SHIFT;
-
-	if (control->sframe) {
-		packed |= control->poll << L2CAP_EXT_CTRL_POLL_SHIFT;
-		packed |= control->super << L2CAP_EXT_CTRL_SUPER_SHIFT;
-		packed |= L2CAP_EXT_CTRL_FRAME_TYPE;
-	} else {
-		packed |= control->sar << L2CAP_EXT_CTRL_SAR_SHIFT;
-		packed |= control->txseq << L2CAP_EXT_CTRL_TXSEQ_SHIFT;
-	}
-
-	return packed;
-}
-
 static void __unpack_extended_control(u32 ext, struct l2cap_ctrl *control)
 {
 	control->reqseq = (ext & L2CAP_EXT_CTRL_REQSEQ) >> L2CAP_EXT_CTRL_REQSEQ_SHIFT;
@@ -885,19 +898,6 @@ static inline void __unpack_control(struct l2cap_chan *chan,
 	}
 }
 
-static inline void __pack_control(struct l2cap_chan *chan,
-				  struct l2cap_ctrl *control,
-				  struct sk_buff *skb)
-{
-	if (test_bit(FLAG_EXT_CTRL, &chan->flags)) {
-		put_unaligned_le32(__pack_extended_control(control),
-				   skb->data + L2CAP_HDR_SIZE);
-	} else {
-		put_unaligned_le16(__pack_enhanced_control(control),
-				   skb->data + L2CAP_HDR_SIZE);
-	}
-}
-
 static inline int __l2cap_no_conn_pending(struct l2cap_chan *chan)
 {
 	return !test_bit(CONF_CONNECT_PEND, &chan->conf_state);
-- 
1.7.10

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

[RFC 5/8] Bluetooth: Move recently-added ERTM header unpacking functions

[Mat Martineau]

Moving these functions simplifies future patches by eliminating
forward declarations, makes future patches easier to review, and
better preserves 'git blame' information.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c |  120 ++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3221f17..6aefeaa 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -725,6 +725,66 @@ static void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 	hci_send_acl(chan->conn->hchan, skb, flags);
 }
 
+static void __unpack_enhanced_control(u16 enh, struct l2cap_ctrl *control)
+{
+	control->reqseq = (enh & L2CAP_CTRL_REQSEQ) >> L2CAP_CTRL_REQSEQ_SHIFT;
+	control->final = (enh & L2CAP_CTRL_FINAL) >> L2CAP_CTRL_FINAL_SHIFT;
+
+	if (enh & L2CAP_CTRL_FRAME_TYPE) {
+		/* S-Frame */
+		control->sframe = 1;
+		control->poll = (enh & L2CAP_CTRL_POLL) >> L2CAP_CTRL_POLL_SHIFT;
+		control->super = (enh & L2CAP_CTRL_SUPERVISE) >> L2CAP_CTRL_SUPER_SHIFT;
+
+		control->sar = 0;
+		control->txseq = 0;
+	} else {
+		/* I-Frame */
+		control->sframe = 0;
+		control->sar = (enh & L2CAP_CTRL_SAR) >> L2CAP_CTRL_SAR_SHIFT;
+		control->txseq = (enh & L2CAP_CTRL_TXSEQ) >> L2CAP_CTRL_TXSEQ_SHIFT;
+
+		control->poll = 0;
+		control->super = 0;
+	}
+}
+
+static void __unpack_extended_control(u32 ext, struct l2cap_ctrl *control)
+{
+	control->reqseq = (ext & L2CAP_EXT_CTRL_REQSEQ) >> L2CAP_EXT_CTRL_REQSEQ_SHIFT;
+	control->final = (ext & L2CAP_EXT_CTRL_FINAL) >> L2CAP_EXT_CTRL_FINAL_SHIFT;
+
+	if (ext & L2CAP_EXT_CTRL_FRAME_TYPE) {
+		/* S-Frame */
+		control->sframe = 1;
+		control->poll = (ext & L2CAP_EXT_CTRL_POLL) >> L2CAP_EXT_CTRL_POLL_SHIFT;
+		control->super = (ext & L2CAP_EXT_CTRL_SUPERVISE) >> L2CAP_EXT_CTRL_SUPER_SHIFT;
+
+		control->sar = 0;
+		control->txseq = 0;
+	} else {
+		/* I-Frame */
+		control->sframe = 0;
+		control->sar = (ext & L2CAP_EXT_CTRL_SAR) >> L2CAP_EXT_CTRL_SAR_SHIFT;
+		control->txseq = (ext & L2CAP_EXT_CTRL_TXSEQ) >> L2CAP_EXT_CTRL_TXSEQ_SHIFT;
+
+		control->poll = 0;
+		control->super = 0;
+	}
+}
+
+static inline void __unpack_control(struct l2cap_chan *chan,
+				    struct sk_buff *skb)
+{
+	if (test_bit(FLAG_EXT_CTRL, &chan->flags)) {
+		__unpack_extended_control(get_unaligned_le32(skb->data),
+					  &bt_cb(skb)->control);
+	} else {
+		__unpack_enhanced_control(get_unaligned_le16(skb->data),
+					  &bt_cb(skb)->control);
+	}
+}
+
 static u32 __pack_extended_control(struct l2cap_ctrl *control)
 {
 	u32 packed;
@@ -838,66 +898,6 @@ static inline void l2cap_send_rr_or_rnr(struct l2cap_chan *chan, u32 control)
 	l2cap_send_sframe(chan, control);
 }
 
-static void __unpack_enhanced_control(u16 enh, struct l2cap_ctrl *control)
-{
-	control->reqseq = (enh & L2CAP_CTRL_REQSEQ) >> L2CAP_CTRL_REQSEQ_SHIFT;
-	control->final = (enh & L2CAP_CTRL_FINAL) >> L2CAP_CTRL_FINAL_SHIFT;
-
-	if (enh & L2CAP_CTRL_FRAME_TYPE) {
-		/* S-Frame */
-		control->sframe = 1;
-		control->poll = (enh & L2CAP_CTRL_POLL) >> L2CAP_CTRL_POLL_SHIFT;
-		control->super = (enh & L2CAP_CTRL_SUPERVISE) >> L2CAP_CTRL_SUPER_SHIFT;
-
-		control->sar = 0;
-		control->txseq = 0;
-	} else {
-		/* I-Frame */
-		control->sframe = 0;
-		control->sar = (enh & L2CAP_CTRL_SAR) >> L2CAP_CTRL_SAR_SHIFT;
-		control->txseq = (enh & L2CAP_CTRL_TXSEQ) >> L2CAP_CTRL_TXSEQ_SHIFT;
-
-		control->poll = 0;
-		control->super = 0;
-	}
-}
-
-static void __unpack_extended_control(u32 ext, struct l2cap_ctrl *control)
-{
-	control->reqseq = (ext & L2CAP_EXT_CTRL_REQSEQ) >> L2CAP_EXT_CTRL_REQSEQ_SHIFT;
-	control->final = (ext & L2CAP_EXT_CTRL_FINAL) >> L2CAP_EXT_CTRL_FINAL_SHIFT;
-
-	if (ext & L2CAP_EXT_CTRL_FRAME_TYPE) {
-		/* S-Frame */
-		control->sframe = 1;
-		control->poll = (ext & L2CAP_EXT_CTRL_POLL) >> L2CAP_EXT_CTRL_POLL_SHIFT;
-		control->super = (ext & L2CAP_EXT_CTRL_SUPERVISE) >> L2CAP_EXT_CTRL_SUPER_SHIFT;
-
-		control->sar = 0;
-		control->txseq = 0;
-	} else {
-		/* I-Frame */
-		control->sframe = 0;
-		control->sar = (ext & L2CAP_EXT_CTRL_SAR) >> L2CAP_EXT_CTRL_SAR_SHIFT;
-		control->txseq = (ext & L2CAP_EXT_CTRL_TXSEQ) >> L2CAP_EXT_CTRL_TXSEQ_SHIFT;
-
-		control->poll = 0;
-		control->super = 0;
-	}
-}
-
-static inline void __unpack_control(struct l2cap_chan *chan,
-				    struct sk_buff *skb)
-{
-	if (test_bit(FLAG_EXT_CTRL, &chan->flags)) {
-		__unpack_extended_control(get_unaligned_le32(skb->data),
-					  &bt_cb(skb)->control);
-	} else {
-		__unpack_enhanced_control(get_unaligned_le16(skb->data),
-					  &bt_cb(skb)->control);
-	}
-}
-
 static inline int __l2cap_no_conn_pending(struct l2cap_chan *chan)
 {
 	return !test_bit(CONF_CONNECT_PEND, &chan->conf_state);
-- 
1.7.10

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

[RFC 6/8] Bluetooth: Lock the L2CAP channel when sending

[Mat Martineau]

The ERTM and streaming mode transmit queue must only be accessed while
the L2CAP channel lock is held.  Adding this lock ensures that
multiple threads cannot simultaneously manipulate the queue when
sending and receiving concurrently.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c |   40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6aefeaa..8ed6a74 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2035,26 +2035,35 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 	u32 control;
 	int err;
 
+	l2cap_chan_hold(chan);
+
 	/* Connectionless channel */
 	if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
 		skb = l2cap_create_connless_pdu(chan, msg, len, priority);
-		if (IS_ERR(skb))
-			return PTR_ERR(skb);
+		if (IS_ERR(skb)) {
+			err = PTR_ERR(skb);
+			goto done;
+		}
 
 		l2cap_do_send(chan, skb);
-		return len;
+		err = len;
+		goto done;
 	}
 
 	switch (chan->mode) {
 	case L2CAP_MODE_BASIC:
 		/* Check outgoing MTU */
-		if (len > chan->omtu)
-			return -EMSGSIZE;
+		if (len > chan->omtu) {
+			err = -EMSGSIZE;
+			break;
+		}
 
 		/* Create a basic PDU */
 		skb = l2cap_create_basic_pdu(chan, msg, len, priority);
-		if (IS_ERR(skb))
-			return PTR_ERR(skb);
+		if (IS_ERR(skb)) {
+			err = PTR_ERR(skb);
+			break;
+		}
 
 		l2cap_do_send(chan, skb);
 		err = len;
@@ -2067,30 +2076,38 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 			control = __set_ctrl_sar(chan, L2CAP_SAR_UNSEGMENTED);
 			skb = l2cap_create_iframe_pdu(chan, msg, len, control,
 									0);
-			if (IS_ERR(skb))
-				return PTR_ERR(skb);
+			if (IS_ERR(skb)) {
+				err = PTR_ERR(skb);
+				break;
+			}
 
+			l2cap_chan_lock(chan);
 			__skb_queue_tail(&chan->tx_q, skb);
 
 			if (chan->tx_send_head == NULL)
 				chan->tx_send_head = skb;
 
+			l2cap_chan_unlock(chan);
 		} else {
 			/* Segment SDU into multiples PDUs */
 			err = l2cap_sar_segment_sdu(chan, msg, len);
 			if (err < 0)
-				return err;
+				break;
 		}
 
 		if (chan->mode == L2CAP_MODE_STREAMING) {
+			l2cap_chan_lock(chan);
 			l2cap_streaming_send(chan);
+			l2cap_chan_unlock(chan);
 			err = len;
 			break;
 		}
 
+		l2cap_chan_lock(chan);
 		if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state) &&
 				test_bit(CONN_WAIT_F, &chan->conn_state)) {
 			err = len;
+			l2cap_chan_unlock(chan);
 			break;
 		}
 
@@ -2098,6 +2115,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 		if (err >= 0)
 			err = len;
 
+		l2cap_chan_unlock(chan);
 		break;
 
 	default:
@@ -2105,6 +2123,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 		err = -EBADFD;
 	}
 
+done:
+	l2cap_chan_put(chan);
 	return err;
 }
 
-- 
1.7.10

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

[RFC 7/8] Bluetooth: Refactor L2CAP ERTM and streaming transmit segmentation

[Mat Martineau]

Use more common code for ERTM and streaming mode segmentation and
transmission, and begin using skb control block data for delaying
extended or enhanced header generation until just before the packet is
transmitted.  This code is also better suited for resegmentation,
which is needed when L2CAP links are reconfigured after an AMP channel
move.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 92c0423..88c128a 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -44,6 +44,7 @@
 #define L2CAP_DEFAULT_MAX_SDU_SIZE	0xFFFF
 #define L2CAP_DEFAULT_SDU_ITIME		0xFFFFFFFF
 #define L2CAP_DEFAULT_ACC_LAT		0xFFFFFFFF
+#define L2CAP_BREDR_MAX_PAYLOAD		1019    /* 3-DH5 packet */
 
 #define L2CAP_DISC_TIMEOUT		msecs_to_jiffies(100)
 #define L2CAP_DISC_REJ_TIMEOUT		msecs_to_jiffies(5000)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8ed6a74..da7c38a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1643,6 +1643,7 @@ static void l2cap_streaming_send(struct l2cap_chan *chan)
 	while ((skb = skb_dequeue(&chan->tx_q))) {
 		control = __get_control(chan, skb->data + L2CAP_HDR_SIZE);
 		control |= __set_txseq(chan, chan->next_tx_seq);
+		control |= __set_ctrl_sar(chan, bt_cb(skb)->control.sar);
 		__put_control(chan, control, skb->data + L2CAP_HDR_SIZE);
 
 		if (chan->fcs == L2CAP_FCS_CRC16) {
@@ -1715,6 +1716,9 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 	if (chan->state != BT_CONNECTED)
 		return -ENOTCONN;
 
+	if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state))
+		return 0;
+
 	while ((skb = chan->tx_send_head) && (!l2cap_tx_window_full(chan))) {
 
 		if (chan->remote_max_tx &&
@@ -1735,6 +1739,7 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
 
 		control |= __set_reqseq(chan, chan->buffer_seq);
 		control |= __set_txseq(chan, chan->next_tx_seq);
+		control |= __set_ctrl_sar(chan, bt_cb(skb)->control.sar);
 
 		__put_control(chan, control, tx_skb->data + L2CAP_HDR_SIZE);
 
@@ -1930,7 +1935,7 @@ static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
 
 static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
 						struct msghdr *msg, size_t len,
-						u32 control, u16 sdulen)
+						u16 sdulen)
 {
 	struct l2cap_conn *conn = chan->conn;
 	struct sk_buff *skb;
@@ -1965,7 +1970,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
 	lh->cid = cpu_to_le16(chan->dcid);
 	lh->len = cpu_to_le16(len + (hlen - L2CAP_HDR_SIZE));
 
-	__put_control(chan, control, skb_put(skb, __ctrl_size(chan)));
+	__put_control(chan, 0, skb_put(skb, __ctrl_size(chan)));
 
 	if (sdulen)
 		put_unaligned_le16(sdulen, skb_put(skb, L2CAP_SDULEN_SIZE));
@@ -1983,57 +1988,78 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
 	return skb;
 }
 
-static int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
+static int l2cap_segment_sdu(struct l2cap_chan *chan,
+			     struct sk_buff_head *seg_queue,
+			     struct msghdr *msg, size_t len)
 {
 	struct sk_buff *skb;
-	struct sk_buff_head sar_queue;
-	u32 control;
-	size_t size = 0;
+	u16 sdu_len;
+	size_t pdu_len;
+	int err = 0;
+	u8 sar;
 
-	skb_queue_head_init(&sar_queue);
-	control = __set_ctrl_sar(chan, L2CAP_SAR_START);
-	skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps, control, len);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
+	BT_DBG("chan %p, msg %p, len %d", chan, msg, (int)len);
 
-	__skb_queue_tail(&sar_queue, skb);
-	len -= chan->remote_mps;
-	size += chan->remote_mps;
+	/* It is critical that ERTM PDUs fit in a single HCI fragment,
+	 * so fragmented skbs are not used.  The HCI layer's handling
+	 * of fragmented skbs is not compatible with ERTM's queueing.
+	 */
 
-	while (len > 0) {
-		size_t buflen;
+	/* PDU size is derived from the HCI MTU */
+	pdu_len = chan->conn->mtu;
 
-		if (len > chan->remote_mps) {
-			control = __set_ctrl_sar(chan, L2CAP_SAR_CONTINUE);
-			buflen = chan->remote_mps;
-		} else {
-			control = __set_ctrl_sar(chan, L2CAP_SAR_END);
-			buflen = len;
-		}
+	pdu_len = min_t(size_t, pdu_len, L2CAP_BREDR_MAX_PAYLOAD);
+
+	/* Adjust for largest possible L2CAP overhead. */
+	pdu_len -= L2CAP_EXT_HDR_SIZE + L2CAP_FCS_SIZE;
+
+	/* Remote device may have requested smaller PDUs */
+	pdu_len = min_t(size_t, pdu_len, chan->remote_mps);
+
+	if (len <= pdu_len) {
+		sar = L2CAP_SAR_UNSEGMENTED;
+		sdu_len = 0;
+		pdu_len = len;
+	} else {
+		sar = L2CAP_SAR_START;
+		sdu_len = len;
+		pdu_len -= L2CAP_SDULEN_SIZE;
+	}
+
+	while (len > 0) {
+		skb = l2cap_create_iframe_pdu(chan, msg, pdu_len, sdu_len);
 
-		skb = l2cap_create_iframe_pdu(chan, msg, buflen, control, 0);
 		if (IS_ERR(skb)) {
-			skb_queue_purge(&sar_queue);
+			__skb_queue_purge(seg_queue);
 			return PTR_ERR(skb);
 		}
 
-		__skb_queue_tail(&sar_queue, skb);
-		len -= buflen;
-		size += buflen;
+		bt_cb(skb)->control.sar = sar;
+		__skb_queue_tail(seg_queue, skb);
+
+		len -= pdu_len;
+		if (sdu_len) {
+			sdu_len = 0;
+			pdu_len += L2CAP_SDULEN_SIZE;
+		}
+
+		if (len <= pdu_len) {
+			sar = L2CAP_SAR_END;
+			pdu_len = len;
+		} else {
+			sar = L2CAP_SAR_CONTINUE;
+		}
 	}
-	skb_queue_splice_tail(&sar_queue, &chan->tx_q);
-	if (chan->tx_send_head == NULL)
-		chan->tx_send_head = sar_queue.next;
 
-	return size;
+	return err;
 }
 
 int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 								u32 priority)
 {
 	struct sk_buff *skb;
-	u32 control;
 	int err;
+	struct sk_buff_head seg_queue;
 
 	l2cap_chan_hold(chan);
 
@@ -2071,51 +2097,48 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
 
 	case L2CAP_MODE_ERTM:
 	case L2CAP_MODE_STREAMING:
-		/* Entire SDU fits into one PDU */
-		if (len <= chan->remote_mps) {
-			control = __set_ctrl_sar(chan, L2CAP_SAR_UNSEGMENTED);
-			skb = l2cap_create_iframe_pdu(chan, msg, len, control,
-									0);
-			if (IS_ERR(skb)) {
-				err = PTR_ERR(skb);
-				break;
-			}
+		/* Check outgoing MTU */
+		if (len > chan->omtu) {
+			err = -EMSGSIZE;
+			break;
+		}
 
-			l2cap_chan_lock(chan);
-			__skb_queue_tail(&chan->tx_q, skb);
+		__skb_queue_head_init(&seg_queue);
 
-			if (chan->tx_send_head == NULL)
-				chan->tx_send_head = skb;
+		/* Do segmentation before calling in to the state machine,
+		 * since it's possible to block while waiting for memory
+		 * allocation.
+		 */
+		err = l2cap_segment_sdu(chan, &seg_queue, msg, len);
 
-			l2cap_chan_unlock(chan);
-		} else {
-			/* Segment SDU into multiples PDUs */
-			err = l2cap_sar_segment_sdu(chan, msg, len);
-			if (err < 0)
-				break;
+		/* The channel could have been closed while segmenting,
+		 * check that it is still connected.
+		 */
+		if (chan->state != BT_CONNECTED) {
+			__skb_queue_purge(&seg_queue);
+			err = -ENOTCONN;
 		}
 
-		if (chan->mode == L2CAP_MODE_STREAMING) {
-			l2cap_chan_lock(chan);
-			l2cap_streaming_send(chan);
-			l2cap_chan_unlock(chan);
-			err = len;
+		if (err)
 			break;
-		}
 
 		l2cap_chan_lock(chan);
-		if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state) &&
-				test_bit(CONN_WAIT_F, &chan->conn_state)) {
-			err = len;
-			l2cap_chan_unlock(chan);
-			break;
-		}
 
-		err = l2cap_ertm_send(chan);
-		if (err >= 0)
+		skb_queue_splice_tail_init(&seg_queue, &chan->tx_q);
+		if (chan->mode == L2CAP_MODE_ERTM)
+			err = l2cap_ertm_send(chan);
+		else
+			l2cap_streaming_send(chan);
+
+		if (!err)
 			err = len;
 
 		l2cap_chan_unlock(chan);
+
+		/* If the skbs were not queued for sending, they'll still be in
+		 * seg_queue and need to be purged.
+		 */
+		__skb_queue_purge(&seg_queue);
 		break;
 
 	default:
-- 
1.7.10

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

[RFC 8/8] Bluetooth: Add Code Aurora Forum copyright

[Mat Martineau]

Adding Code Aurora Forum copyright information due to significant
additions of code.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
 net/bluetooth/l2cap_core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index da7c38a..8d09d56 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4,6 +4,7 @@
    Copyright (C) 2009-2010 Gustavo F. Padovan <gustavo@padovan.org>
    Copyright (C) 2010 Google Inc.
    Copyright (C) 2011 ProFUSION Embedded Systems
+   Copyright (c) 2012 Code Aurora Forum.  All rights reserved.
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
-- 
1.7.10

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

Re: [RFC 2/8] Bluetooth: Initialize new l2cap_chan structure members

[Marcel Holtmann]

Hi Mat,

> Structure members used by ERTM or streaming mode need to be
> initialized when an ERTM or streaming mode link is configured.  Some
> duplicate code is also eliminated by moving in to the ERTM init
> function.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |   25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index beb7194..35c0a29 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2315,17 +2315,30 @@ static inline int l2cap_ertm_init(struct l2cap_chan *chan)
>  {
>  	int err;
>  
> +	chan->next_tx_seq = 0;
> +	chan->expected_tx_seq = 0;
>  	chan->expected_ack_seq = 0;
>  	chan->unacked_frames = 0;
>  	chan->buffer_seq = 0;
>  	chan->num_acked = 0;
>  	chan->frames_sent = 0;
> +	chan->last_acked_seq = 0;
> +	chan->sdu = NULL;
> +	chan->sdu_last_frag = NULL;
> +	chan->sdu_len = 0;
> +
> +	if (chan->mode != L2CAP_MODE_ERTM)
> +		return 0;
> +
> +	chan->rx_state = L2CAP_RX_STATE_RECV;
> +	chan->tx_state = L2CAP_TX_STATE_XMIT;
>  
>  	INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout);
>  	INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout);
>  	INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout);
>  
>  	skb_queue_head_init(&chan->srej_q);
> +	skb_queue_head_init(&chan->tx_q);
>  
>  	INIT_LIST_HEAD(&chan->srej_l);
>  	err = l2cap_seq_list_init(&chan->srej_list, chan->tx_win);
> @@ -3193,10 +3206,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>  
>  		l2cap_state_change(chan, BT_CONNECTED);
>  
> -		chan->next_tx_seq = 0;
> -		chan->expected_tx_seq = 0;
> -		skb_queue_head_init(&chan->tx_q);
> -		if (chan->mode == L2CAP_MODE_ERTM)
> +		if (chan->mode == L2CAP_MODE_ERTM ||
> +					chan->mode == L2CAP_MODE_STREAMING)
>  			err = l2cap_ertm_init(chan);

we need to make this compliant with the coding style the Dave Miller
forced onto us. So either we turn this into a switch statement or we
have to do it like this:

		if (chan->mode == ... ||
		    chan->mode == ...)
			err = l2cap_ertm_init(...);

Please keep this in mind. Otherwise we do not get the patches merged
into the networking trees. If you wanna avoid this, I am fine using
switch statements:

		switch (chan->mode) {
		case L2CAP_MODE_ERTM:
		case L2CAP_MODE_STREAMING:
			err = l2cap_ertm_init(...);
			break;
		}

Regards

Marcel

Re: [RFC 1/8] Bluetooth: Improve ERTM sequence number offset calculation

[Marcel Holtmann]

Hi Mat,

> Instead of using modular division, the offset can be calculated using
> only addition and subtraction.  The previous calculation did not work
> as intended and was more difficult to understand, involving unsigned
> integer underflow and a check for a negative value where one was not
> possible.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 3/8] Bluetooth: Remove duplicate structure members from bt_skb_cb

[Marcel Holtmann]

Hi Mat,

> These values are now in the nested l2cap_ctrl struct.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/bluetooth.h |    3 ---
>  net/bluetooth/l2cap_core.c        |   34 +++++++++++++++++-----------------
>  2 files changed, 17 insertions(+), 20 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 4/8] Bluetooth: Move recently-added ERTM header packing functions

[Marcel Holtmann]

Hi Mat,

> Moving these functions simplifies future patches by eliminating
> forward declarations, makes future patches easier to review, and
> better preserves 'git blame' information.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |  102 ++++++++++++++++++++++----------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 5/8] Bluetooth: Move recently-added ERTM header unpacking functions

[Marcel Holtmann]

Hi Mat,

> Moving these functions simplifies future patches by eliminating
> forward declarations, makes future patches easier to review, and
> better preserves 'git blame' information.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |  120 ++++++++++++++++++++++----------------------
>  1 file changed, 60 insertions(+), 60 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 6/8] Bluetooth: Lock the L2CAP channel when sending

[Marcel Holtmann]

Hi Mat,

> The ERTM and streaming mode transmit queue must only be accessed while
> the L2CAP channel lock is held.  Adding this lock ensures that
> multiple threads cannot simultaneously manipulate the queue when
> sending and receiving concurrently.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |   40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 7/8] Bluetooth: Refactor L2CAP ERTM and streaming transmit segmentation

[Marcel Holtmann]

Hi Mat,

> Use more common code for ERTM and streaming mode segmentation and
> transmission, and begin using skb control block data for delaying
> extended or enhanced header generation until just before the packet is
> transmitted.  This code is also better suited for resegmentation,
> which is needed when L2CAP links are reconfigured after an AMP channel
> move.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |    1 +
>  net/bluetooth/l2cap_core.c    |  155 +++++++++++++++++++++++------------------
>  2 files changed, 90 insertions(+), 66 deletions(-)

from what I can see, this looks good, but I like to see a second review
of this from someone else.

Regards

Marcel

Re: [RFC 8/8] Bluetooth: Add Code Aurora Forum copyright

[Marcel Holtmann]

Hi Mat,

> Adding Code Aurora Forum copyright information due to significant
> additions of code.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |    1 +
>  1 file changed, 1 insertion(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [RFC 1/8] Bluetooth: Improve ERTM sequence number offset calculation

[Andrei Emeltchenko]

Hi Mat,

On Wed, Apr 25, 2012 at 04:36:12PM -0700, Mat Martineau wrote:
> Instead of using modular division, the offset can be calculated using
> only addition and subtraction.  The previous calculation did not work
> as intended and was more difficult to understand, involving unsigned
> integer underflow and a check for a negative value where one was not
> possible.

BTW: in what cases this was not working?

> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 22e9ec9..92c0423 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -724,13 +724,10 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
>  
>  static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
>  {
> -	int offset;
> -
> -	offset = (seq1 - seq2) % (chan->tx_win_max + 1);
> -	if (offset < 0)
> -		offset += (chan->tx_win_max + 1);
> -
> -	return offset;
> +	if (seq1 >= seq2)
> +		return seq1 - seq2;
> +	else
> +		return chan->tx_win_max + 1 - seq2 + seq1;
>  }

You seems are changing logic, not improving as you patch states. Your offset might be bigger
then tx_win_max. Was this intended?

Best regards 
Andrei Emeltchenko 

Re: [RFC 6/8] Bluetooth: Lock the L2CAP channel when sending

[Andrei Emeltchenko]

Hi Mat,

On Wed, Apr 25, 2012 at 04:36:17PM -0700, Mat Martineau wrote:
> The ERTM and streaming mode transmit queue must only be accessed while
> the L2CAP channel lock is held.  Adding this lock ensures that
> multiple threads cannot simultaneously manipulate the queue when
> sending and receiving concurrently.

I think the lock shall be added in l2cap_sock_sendmsg like:

lock()
err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
unlock()

I actually use this l2cap_chan_send in A2MP code already because it gets
locked in l2cap_data_channel.

Best regards 
Andrei Emeltchenko 

> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |   40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 6aefeaa..8ed6a74 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2035,26 +2035,35 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>  	u32 control;
>  	int err;
>  
> +	l2cap_chan_hold(chan);
> +
>  	/* Connectionless channel */
>  	if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
>  		skb = l2cap_create_connless_pdu(chan, msg, len, priority);
> -		if (IS_ERR(skb))
> -			return PTR_ERR(skb);
> +		if (IS_ERR(skb)) {
> +			err = PTR_ERR(skb);
> +			goto done;
> +		}
>  
>  		l2cap_do_send(chan, skb);
> -		return len;
> +		err = len;
> +		goto done;
>  	}
>  
>  	switch (chan->mode) {
>  	case L2CAP_MODE_BASIC:
>  		/* Check outgoing MTU */
> -		if (len > chan->omtu)
> -			return -EMSGSIZE;
> +		if (len > chan->omtu) {
> +			err = -EMSGSIZE;
> +			break;
> +		}
>  
>  		/* Create a basic PDU */
>  		skb = l2cap_create_basic_pdu(chan, msg, len, priority);
> -		if (IS_ERR(skb))
> -			return PTR_ERR(skb);
> +		if (IS_ERR(skb)) {
> +			err = PTR_ERR(skb);
> +			break;
> +		}
>  
>  		l2cap_do_send(chan, skb);
>  		err = len;
> @@ -2067,30 +2076,38 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>  			control = __set_ctrl_sar(chan, L2CAP_SAR_UNSEGMENTED);
>  			skb = l2cap_create_iframe_pdu(chan, msg, len, control,
>  									0);
> -			if (IS_ERR(skb))
> -				return PTR_ERR(skb);
> +			if (IS_ERR(skb)) {
> +				err = PTR_ERR(skb);
> +				break;
> +			}
>  
> +			l2cap_chan_lock(chan);
>  			__skb_queue_tail(&chan->tx_q, skb);
>  
>  			if (chan->tx_send_head == NULL)
>  				chan->tx_send_head = skb;
>  
> +			l2cap_chan_unlock(chan);
>  		} else {
>  			/* Segment SDU into multiples PDUs */
>  			err = l2cap_sar_segment_sdu(chan, msg, len);
>  			if (err < 0)
> -				return err;
> +				break;
>  		}
>  
>  		if (chan->mode == L2CAP_MODE_STREAMING) {
> +			l2cap_chan_lock(chan);
>  			l2cap_streaming_send(chan);
> +			l2cap_chan_unlock(chan);
>  			err = len;
>  			break;
>  		}
>  
> +		l2cap_chan_lock(chan);
>  		if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state) &&
>  				test_bit(CONN_WAIT_F, &chan->conn_state)) {
>  			err = len;
> +			l2cap_chan_unlock(chan);
>  			break;
>  		}
>  
> @@ -2098,6 +2115,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>  		if (err >= 0)
>  			err = len;
>  
> +		l2cap_chan_unlock(chan);
>  		break;
>  
>  	default:
> @@ -2105,6 +2123,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>  		err = -EBADFD;
>  	}
>  
> +done:
> +	l2cap_chan_put(chan);
>  	return err;
>  }
>  
> -- 
> 1.7.10
> 
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC 2/8] Bluetooth: Initialize new l2cap_chan structure members

[Mat Martineau]

Marcel -

On Thu, 26 Apr 2012, Marcel Holtmann wrote:

> Hi Mat,
>
>> Structure members used by ERTM or streaming mode need to be
>> initialized when an ERTM or streaming mode link is configured.  Some
>> duplicate code is also eliminated by moving in to the ERTM init
>> function.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c |   25 +++++++++++++++++--------
>>  1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index beb7194..35c0a29 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2315,17 +2315,30 @@ static inline int l2cap_ertm_init(struct l2cap_chan *chan)
>>  {
>>  	int err;
>>
>> +	chan->next_tx_seq = 0;
>> +	chan->expected_tx_seq = 0;
>>  	chan->expected_ack_seq = 0;
>>  	chan->unacked_frames = 0;
>>  	chan->buffer_seq = 0;
>>  	chan->num_acked = 0;
>>  	chan->frames_sent = 0;
>> +	chan->last_acked_seq = 0;
>> +	chan->sdu = NULL;
>> +	chan->sdu_last_frag = NULL;
>> +	chan->sdu_len = 0;
>> +
>> +	if (chan->mode != L2CAP_MODE_ERTM)
>> +		return 0;
>> +
>> +	chan->rx_state = L2CAP_RX_STATE_RECV;
>> +	chan->tx_state = L2CAP_TX_STATE_XMIT;
>>
>>  	INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout);
>>  	INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout);
>>  	INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout);
>>
>>  	skb_queue_head_init(&chan->srej_q);
>> +	skb_queue_head_init(&chan->tx_q);
>>
>>  	INIT_LIST_HEAD(&chan->srej_l);
>>  	err = l2cap_seq_list_init(&chan->srej_list, chan->tx_win);
>> @@ -3193,10 +3206,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
>>
>>  		l2cap_state_change(chan, BT_CONNECTED);
>>
>> -		chan->next_tx_seq = 0;
>> -		chan->expected_tx_seq = 0;
>> -		skb_queue_head_init(&chan->tx_q);
>> -		if (chan->mode == L2CAP_MODE_ERTM)
>> +		if (chan->mode == L2CAP_MODE_ERTM ||
>> +					chan->mode == L2CAP_MODE_STREAMING)
>>  			err = l2cap_ertm_init(chan);
>
> we need to make this compliant with the coding style the Dave Miller
> forced onto us. So either we turn this into a switch statement or we
> have to do it like this:
>
> 		if (chan->mode == ... ||
> 		    chan->mode == ...)
> 			err = l2cap_ertm_init(...);

Ok, I had misunderstood what Dave wanted.  From this message:

http://article.gmane.org/gmane.linux.network/222716

I thought he was only looking for that kind of alignment in function 
calls and prototypes.  However, here:

http://article.gmane.org/gmane.linux.kernel.wireless.general/85043

he mentions 'if' statements.


I will update my changes to reflect Dave's style on all long lines 
with parens.


> Please keep this in mind. Otherwise we do not get the patches merged
> into the networking trees. If you wanna avoid this, I am fine using
> switch statements:
>
> 		switch (chan->mode) {
> 		case L2CAP_MODE_ERTM:
> 		case L2CAP_MODE_STREAMING:
> 			err = l2cap_ertm_init(...);
> 			break;
> 		}


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

Re: [RFC 1/8] Bluetooth: Improve ERTM sequence number offset calculation

[Mat Martineau]

Hi Andrei -

On Thu, 26 Apr 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Apr 25, 2012 at 04:36:12PM -0700, Mat Martineau wrote:
>> Instead of using modular division, the offset can be calculated using
>> only addition and subtraction.  The previous calculation did not work
>> as intended and was more difficult to understand, involving unsigned
>> integer underflow and a check for a negative value where one was not
>> possible.
>
> BTW: in what cases this was not working?

"offset" was always positive, because "x % y" always gives a positive 
result if y is a positive number.  The 'if' clause was dead code, 
which was obviously not intended.

If seq2 > seq1, then the value would wrap back around to a large 
positive integer because both seq1 and seq2 are unsigned, so the value 
of (seq1 - seq2) is also unsigned.  Suppose seq1 is 0 and seq2 is 1. 
In unsigned 16-bit integer math, 0 - 1 == 65535 due to underflow.

65535 % (63+1) is 63: luckily, the right answer for the most common tx 
window (63).

65535 % (163+1) is 99: but the offset should be 163

>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  include/net/bluetooth/l2cap.h |   11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 22e9ec9..92c0423 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -724,13 +724,10 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
>>
>>  static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
>>  {
>> -	int offset;
>> -
>> -	offset = (seq1 - seq2) % (chan->tx_win_max + 1);
>> -	if (offset < 0)
>> -		offset += (chan->tx_win_max + 1);
>> -
>> -	return offset;
>> +	if (seq1 >= seq2)
>> +		return seq1 - seq2;
>> +	else
>> +		return chan->tx_win_max + 1 - seq2 + seq1;
>>  }
>
> You seems are changing logic, not improving as you patch states. 
> Your offset might be bigger then tx_win_max. Was this intended?

The new code is correct.

Here's a python test program for all possible inputs with
tx_win_max == 63:

<code>
#!/usr/bin/env python
def seq_offset(tx_win_max, seq1, seq2):
     if (seq1 >= seq2):
         return seq1 - seq2
     else:
         return tx_win_max + 1 - seq2 + seq1

tx_win_max = 63
max_offset = -1
min_offset = tx_win_max + 1

for i in range(tx_win_max+1):
     for j in range(tx_win_max+1):
         offset = seq_offset(tx_win_max, i, j)
         min_offset = min(offset, min_offset)
         max_offset = max(offset, max_offset)

print "min: %d, max: %d" % (min_offset, max_offset)
</code>

It prints:
min: 0, max: 63


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

Re: [RFC 6/8] Bluetooth: Lock the L2CAP channel when sending

[Mat Martineau]

Andrei -

On Thu, 26 Apr 2012, Andrei Emeltchenko wrote:

> Hi Mat,
>
> On Wed, Apr 25, 2012 at 04:36:17PM -0700, Mat Martineau wrote:
>> The ERTM and streaming mode transmit queue must only be accessed while
>> the L2CAP channel lock is held.  Adding this lock ensures that
>> multiple threads cannot simultaneously manipulate the queue when
>> sending and receiving concurrently.
>
> I think the lock shall be added in l2cap_sock_sendmsg like:
>
> lock()
> err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
> unlock()
>
> I actually use this l2cap_chan_send in A2MP code already because it gets
> locked in l2cap_data_channel.
>

You're right.

There's still a problem where the lock needs to be released when 
bt_skb_send_alloc is called, because that function can block.  What do 
you think about releasing and reacquiring the channel lock in 
l2cap_sock_alloc_skb_cb() instead?

>
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap_core.c |   40 ++++++++++++++++++++++++++++++----------
>>  1 file changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 6aefeaa..8ed6a74 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2035,26 +2035,35 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>>  	u32 control;
>>  	int err;
>>
>> +	l2cap_chan_hold(chan);
>> +
>>  	/* Connectionless channel */
>>  	if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
>>  		skb = l2cap_create_connless_pdu(chan, msg, len, priority);
>> -		if (IS_ERR(skb))
>> -			return PTR_ERR(skb);
>> +		if (IS_ERR(skb)) {
>> +			err = PTR_ERR(skb);
>> +			goto done;
>> +		}
>>
>>  		l2cap_do_send(chan, skb);
>> -		return len;
>> +		err = len;
>> +		goto done;
>>  	}
>>
>>  	switch (chan->mode) {
>>  	case L2CAP_MODE_BASIC:
>>  		/* Check outgoing MTU */
>> -		if (len > chan->omtu)
>> -			return -EMSGSIZE;
>> +		if (len > chan->omtu) {
>> +			err = -EMSGSIZE;
>> +			break;
>> +		}
>>
>>  		/* Create a basic PDU */
>>  		skb = l2cap_create_basic_pdu(chan, msg, len, priority);
>> -		if (IS_ERR(skb))
>> -			return PTR_ERR(skb);
>> +		if (IS_ERR(skb)) {
>> +			err = PTR_ERR(skb);
>> +			break;
>> +		}
>>
>>  		l2cap_do_send(chan, skb);
>>  		err = len;
>> @@ -2067,30 +2076,38 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>>  			control = __set_ctrl_sar(chan, L2CAP_SAR_UNSEGMENTED);
>>  			skb = l2cap_create_iframe_pdu(chan, msg, len, control,
>>  									0);
>> -			if (IS_ERR(skb))
>> -				return PTR_ERR(skb);
>> +			if (IS_ERR(skb)) {
>> +				err = PTR_ERR(skb);
>> +				break;
>> +			}
>>
>> +			l2cap_chan_lock(chan);
>>  			__skb_queue_tail(&chan->tx_q, skb);
>>
>>  			if (chan->tx_send_head == NULL)
>>  				chan->tx_send_head = skb;
>>
>> +			l2cap_chan_unlock(chan);
>>  		} else {
>>  			/* Segment SDU into multiples PDUs */
>>  			err = l2cap_sar_segment_sdu(chan, msg, len);
>>  			if (err < 0)
>> -				return err;
>> +				break;
>>  		}
>>
>>  		if (chan->mode == L2CAP_MODE_STREAMING) {
>> +			l2cap_chan_lock(chan);
>>  			l2cap_streaming_send(chan);
>> +			l2cap_chan_unlock(chan);
>>  			err = len;
>>  			break;
>>  		}
>>
>> +		l2cap_chan_lock(chan);
>>  		if (test_bit(CONN_REMOTE_BUSY, &chan->conn_state) &&
>>  				test_bit(CONN_WAIT_F, &chan->conn_state)) {
>>  			err = len;
>> +			l2cap_chan_unlock(chan);
>>  			break;
>>  		}
>>
>> @@ -2098,6 +2115,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>>  		if (err >= 0)
>>  			err = len;
>>
>> +		l2cap_chan_unlock(chan);
>>  		break;
>>
>>  	default:
>> @@ -2105,6 +2123,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>>  		err = -EBADFD;
>>  	}
>>
>> +done:
>> +	l2cap_chan_put(chan);
>>  	return err;
>>  }
>>
>> --
>> 1.7.10

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

Re: [RFC 1/8] Bluetooth: Improve ERTM sequence number offset calculation

[Gustavo Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2012-04-25 16:36:12 -0700]:

> Instead of using modular division, the offset can be calculated using
> only addition and subtraction.  The previous calculation did not work
> as intended and was more difficult to understand, involving unsigned
> integer underflow and a check for a negative value where one was not
> possible.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/l2cap.h |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Patch has been applied to bluetooth-next. thanks.

	Gustavo

Re: [RFC 3/8] Bluetooth: Remove duplicate structure members from bt_skb_cb

[Gustavo Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2012-04-25 16:36:14 -0700]:

> These values are now in the nested l2cap_ctrl struct.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  include/net/bluetooth/bluetooth.h |    3 ---
>  net/bluetooth/l2cap_core.c        |   34 +++++++++++++++++-----------------
>  2 files changed, 17 insertions(+), 20 deletions(-)

Patch has been applied to bluetooth-next. thanks.

	Gustavo

Re: [RFC 4/8] Bluetooth: Move recently-added ERTM header packing functions

[Gustavo Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2012-04-25 16:36:15 -0700]:

> Moving these functions simplifies future patches by eliminating
> forward declarations, makes future patches easier to review, and
> better preserves 'git blame' information.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |  102 ++++++++++++++++++++++----------------------
>  1 file changed, 51 insertions(+), 51 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

Re: [RFC 5/8] Bluetooth: Move recently-added ERTM header unpacking functions

[Gustavo Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2012-04-25 16:36:16 -0700]:

> Moving these functions simplifies future patches by eliminating
> forward declarations, makes future patches easier to review, and
> better preserves 'git blame' information.
> 
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
>  net/bluetooth/l2cap_core.c |  120 ++++++++++++++++++++++----------------------
>  1 file changed, 60 insertions(+), 60 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

[RFCv0] Bluetooth: Change locking logic in sock send

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Use chan_lock instead of lock_sock when sending L2CAP pkts.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 include/net/bluetooth/bluetooth.h |    2 --
 net/bluetooth/l2cap_sock.c        |    8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 2fb268f..90678a9 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -256,12 +256,10 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
 {
 	struct sk_buff *skb;
 
-	release_sock(sk);
 	if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) {
 		skb_reserve(skb, BT_SKB_RESERVE);
 		bt_cb(skb)->incoming  = 0;
 	}
-	lock_sock(sk);
 
 	if (!skb && *err)
 		return NULL;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 0f30785..e622072 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -716,16 +716,16 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	if (msg->msg_flags & MSG_OOB)
 		return -EOPNOTSUPP;
 
-	lock_sock(sk);
-
 	if (sk->sk_state != BT_CONNECTED) {
-		release_sock(sk);
 		return -ENOTCONN;
 	}
 
+	l2cap_chan_lock(chan);
+
 	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
 
-	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
-- 
1.7.9.5