linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Pull request: Fixes for the Enhanced Retransmission Mode
@ 2010-06-05  7:50 Gustavo F. Padovan
  2010-06-05  7:50 ` [PATCH 01/14] Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel

Hi Marcel,

The following changes since commit 67a3e12b05e055c0415c556a315a3d3eb637e29e:

  Linux 2.6.35-rc1 (2010-05-30 13:21:02 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-testing.git for-upstream

Gustavo F. Padovan (13):
      Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP
      Bluetooth: Remove L2CAP Extended Features from Kconfig
      Bluetooth: Fix drop of packets with invalid req_seq/tx_seq
      Bluetooth: Check skb_clone return to avoid NULL dereference
      Bluetooth: Fix ERTM vars increment
      Bluetooth: Check packet FCS earlier
      Bluetooth: Only check SAR bits if frame is I-frame
      Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT
      Bluetooth: Fix ERTM channel shutdown
      Bluetooth: Fix crash when sending frames after connection is closed
      Bluetooth: Fix handle of received P-bit
      Bluetooth: Fix l2cap_ertm_send() behavior
      Bluetooth: Fix missing retransmission action with RR(P=1)

João Paulo Rechi Vita (1):
      Bluetooth: Fix SREJ_QUEUE corruption

 net/bluetooth/Kconfig |   13 ----
 net/bluetooth/l2cap.c |  156 +++++++++++++++++++++++++++++++------------------
 2 files changed, 99 insertions(+), 70 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/14] Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP
  2010-06-05  7:50 Pull request: Fixes for the Enhanced Retransmission Mode Gustavo F. Padovan
@ 2010-06-05  7:50 ` Gustavo F. Padovan
  2010-06-05  7:50   ` [PATCH 02/14] Bluetooth: Remove L2CAP Extended Features from Kconfig Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1b682a5..5c636b3 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -60,8 +60,6 @@ static int enable_ertm = 1;
 #else
 static int enable_ertm = 0;
 #endif
-static int max_transmit = L2CAP_DEFAULT_MAX_TX;
-static int tx_window = L2CAP_DEFAULT_TX_WINDOW;
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };
@@ -808,9 +806,9 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 			pi->mode = L2CAP_MODE_ERTM;
 		else
 			pi->mode = L2CAP_MODE_BASIC;
-		pi->max_tx = max_transmit;
+		pi->max_tx = L2CAP_DEFAULT_MAX_TX;
 		pi->fcs  = L2CAP_FCS_CRC16;
-		pi->tx_win = tx_window;
+		pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
 		pi->sec_level = BT_SECURITY_LOW;
 		pi->role_switch = 0;
 		pi->force_reliable = 0;
@@ -4674,12 +4672,6 @@ module_exit(l2cap_exit);
 module_param(enable_ertm, bool, 0644);
 MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");
 
-module_param(max_transmit, uint, 0644);
-MODULE_PARM_DESC(max_transmit, "Max transmit value (default = 3)");
-
-module_param(tx_window, uint, 0644);
-MODULE_PARM_DESC(tx_window, "Transmission window size value (default = 63)");
-
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth L2CAP ver " VERSION);
 MODULE_VERSION(VERSION);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/14] Bluetooth: Remove L2CAP Extended Features from Kconfig
  2010-06-05  7:50 ` [PATCH 01/14] Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP Gustavo F. Padovan
@ 2010-06-05  7:50   ` Gustavo F. Padovan
  2010-06-05  7:50     ` [PATCH 03/14] Bluetooth: Fix drop of packets with invalid req_seq/tx_seq Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

This reverts commit 84fb0a6334af0ccad3544f6972c055d90fbb9fbe
One can use other mechanisms to enable L2CAP Extended Features.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/Kconfig |   13 -------------
 net/bluetooth/l2cap.c |    4 ----
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index ee3b304..ed37168 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -43,19 +43,6 @@ config BT_L2CAP
 	  Say Y here to compile L2CAP support into the kernel or say M to
 	  compile it as module (l2cap).
 
-config BT_L2CAP_EXT_FEATURES
-	bool "L2CAP Extended Features support (EXPERIMENTAL)"
-	depends on BT_L2CAP && EXPERIMENTAL
-	help
-	  This option enables the L2CAP Extended Features support. These
-	  new features include the Enhanced Retransmission and Streaming
-	  Modes, the Frame Check Sequence (FCS), and Segmentation and
-	  Reassembly (SAR) for L2CAP packets. They are a required for the
-	  new Alternate MAC/PHY and the Bluetooth Medical Profile.
-
-	  You should say N unless you know what you are doing. Note that
-	  this is in an experimental state yet.
-
 config BT_SCO
 	tristate "SCO links support"
 	depends on BT
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 5c636b3..e036419 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -55,11 +55,7 @@
 
 #define VERSION "2.14"
 
-#ifdef CONFIG_BT_L2CAP_EXT_FEATURES
-static int enable_ertm = 1;
-#else
 static int enable_ertm = 0;
-#endif
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/14] Bluetooth: Fix drop of packets with invalid req_seq/tx_seq
  2010-06-05  7:50   ` [PATCH 02/14] Bluetooth: Remove L2CAP Extended Features from Kconfig Gustavo F. Padovan
@ 2010-06-05  7:50     ` Gustavo F. Padovan
  2010-06-05  7:50       ` [PATCH 04/14] Bluetooth: Check skb_clone return to avoid NULL dereference Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

We can't use an unsigned var since we are expecting negatives value
there too.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e036419..b08731d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3789,7 +3789,7 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 	u8 tx_seq = __get_txseq(rx_control);
 	u8 req_seq = __get_reqseq(rx_control);
 	u8 sar = rx_control >> L2CAP_CTRL_SAR_SHIFT;
-	u8 tx_seq_offset, expected_tx_seq_offset;
+	int tx_seq_offset, expected_tx_seq_offset;
 	int num_to_ack = (pi->tx_win/6) + 1;
 	int err = 0;
 
@@ -4074,7 +4074,8 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 	struct sock *sk;
 	struct l2cap_pinfo *pi;
 	u16 control, len;
-	u8 tx_seq, req_seq, next_tx_seq_offset, req_seq_offset;
+	u8 tx_seq, req_seq;
+	int next_tx_seq_offset, req_seq_offset;
 
 	sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
 	if (!sk) {
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/14] Bluetooth: Check skb_clone return to avoid NULL dereference
  2010-06-05  7:50     ` [PATCH 03/14] Bluetooth: Fix drop of packets with invalid req_seq/tx_seq Gustavo F. Padovan
@ 2010-06-05  7:50       ` Gustavo F. Padovan
  2010-06-05  7:50         ` [PATCH 05/14] Bluetooth: Fix ERTM vars increment Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

skb can be NULL and a Ooops can happen if we do not check for NULL.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index b08731d..27e69f6 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1335,6 +1335,8 @@ static int l2cap_streaming_send(struct sock *sk)
 
 	while ((skb = sk->sk_send_head)) {
 		tx_skb = skb_clone(skb, GFP_ATOMIC);
+		if (!tx_skb)
+			break;
 
 		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
 		control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT;
@@ -1420,6 +1422,8 @@ static int l2cap_ertm_send(struct sock *sk)
 		}
 
 		tx_skb = skb_clone(skb, GFP_ATOMIC);
+		if (!tx_skb)
+			break;
 
 		bt_cb(skb)->retries++;
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/14] Bluetooth: Fix ERTM vars increment
  2010-06-05  7:50       ` [PATCH 04/14] Bluetooth: Check skb_clone return to avoid NULL dereference Gustavo F. Padovan
@ 2010-06-05  7:50         ` Gustavo F. Padovan
  2010-06-05  7:50           ` [PATCH 06/14] Bluetooth: Check packet FCS earlier Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

All ERTM operations regarding the txWindow should be modulo 64,
otherwise we screw up ERTM logic.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 27e69f6..a567614 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3745,7 +3745,7 @@ static void l2cap_check_srej_gap(struct sock *sk, u8 tx_seq)
 		l2cap_ertm_reassembly_sdu(sk, skb, control);
 		l2cap_pi(sk)->buffer_seq_srej =
 			(l2cap_pi(sk)->buffer_seq_srej + 1) % 64;
-		tx_seq++;
+		tx_seq = (tx_seq + 1) % 64;
 	}
 }
 
@@ -3781,10 +3781,11 @@ static void l2cap_send_srejframe(struct sock *sk, u8 tx_seq)
 		l2cap_send_sframe(pi, control);
 
 		new = kzalloc(sizeof(struct srej_list), GFP_ATOMIC);
-		new->tx_seq = pi->expected_tx_seq++;
+		new->tx_seq = pi->expected_tx_seq;
+		pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
 		list_add_tail(&new->list, SREJ_LIST(sk));
 	}
-	pi->expected_tx_seq++;
+	pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
 }
 
 static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, struct sk_buff *skb)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/14] Bluetooth: Check packet FCS earlier
  2010-06-05  7:50         ` [PATCH 05/14] Bluetooth: Fix ERTM vars increment Gustavo F. Padovan
@ 2010-06-05  7:50           ` Gustavo F. Padovan
  2010-06-05  7:50             ` [PATCH 07/14] Bluetooth: Only check SAR bits if frame is I-frame Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

This way, if FCS is enabled and the packet is corrupted, we just drop it
without read it len, which could be corrupted.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a567614..97584d8 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4114,25 +4114,25 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		skb_pull(skb, 2);
 		len = skb->len;
 
+		/*
+		 * We can just drop the corrupted I-frame here.
+		 * Receiver will miss it and start proper recovery
+		 * procedures and ask retransmission.
+		 */
+		if (l2cap_check_fcs(pi, skb))
+			goto drop;
+
 		if (__is_sar_start(control))
 			len -= 2;
 
 		if (pi->fcs == L2CAP_FCS_CRC16)
 			len -= 2;
 
-		/*
-		 * We can just drop the corrupted I-frame here.
-		 * Receiver will miss it and start proper recovery
-		 * procedures and ask retransmission.
-		 */
 		if (len > pi->mps) {
 			l2cap_send_disconn_req(pi->conn, sk);
 			goto drop;
 		}
 
-		if (l2cap_check_fcs(pi, skb))
-			goto drop;
-
 		req_seq = __get_reqseq(control);
 		req_seq_offset = (req_seq - pi->expected_ack_seq) % 64;
 		if (req_seq_offset < 0)
@@ -4172,6 +4172,9 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		skb_pull(skb, 2);
 		len = skb->len;
 
+		if (l2cap_check_fcs(pi, skb))
+			goto drop;
+
 		if (__is_sar_start(control))
 			len -= 2;
 
@@ -4181,9 +4184,6 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		if (len > pi->mps || len < 4 || __is_sframe(control))
 			goto drop;
 
-		if (l2cap_check_fcs(pi, skb))
-			goto drop;
-
 		tx_seq = __get_txseq(control);
 
 		if (pi->expected_tx_seq == tx_seq)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/14] Bluetooth: Only check SAR bits if frame is I-frame
  2010-06-05  7:50           ` [PATCH 06/14] Bluetooth: Check packet FCS earlier Gustavo F. Padovan
@ 2010-06-05  7:50             ` Gustavo F. Padovan
  2010-06-05  7:50               ` [PATCH 08/14] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

Check the SAR bit only make sense for an I-frame. Also check SAR for
S-frame lead to errors.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 97584d8..7a8454a 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4122,7 +4122,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		if (l2cap_check_fcs(pi, skb))
 			goto drop;
 
-		if (__is_sar_start(control))
+		if (__is_sar_start(control) && __is_iframe(control))
 			len -= 2;
 
 		if (pi->fcs == L2CAP_FCS_CRC16)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/14] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT
  2010-06-05  7:50             ` [PATCH 07/14] Bluetooth: Only check SAR bits if frame is I-frame Gustavo F. Padovan
@ 2010-06-05  7:50               ` Gustavo F. Padovan
  2010-06-05  7:50                 ` [PATCH 09/14] Bluetooth: Fix ERTM channel shutdown Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

The ack_timer is implemation specific, disabling it in such situation
avoids some potencial errors in the ERTM protocol.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 7a8454a..35b7791 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3632,6 +3632,8 @@ static int l2cap_push_rx_skb(struct sock *sk, struct sk_buff *skb, u16 control)
 
 	pi->conn_state |= L2CAP_CONN_RNR_SENT;
 
+	del_timer(&pi->ack_timer);
+
 	queue_work(_busy_wq, &pi->busy_work);
 
 	return err;
@@ -3881,6 +3883,8 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 		pi->conn_state |= L2CAP_CONN_SEND_PBIT;
 
 		l2cap_send_srejframe(sk, tx_seq);
+
+		del_timer(&pi->ack_timer);
 	}
 	return 0;
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/14] Bluetooth: Fix ERTM channel shutdown
  2010-06-05  7:50               ` [PATCH 08/14] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT Gustavo F. Padovan
@ 2010-06-05  7:50                 ` Gustavo F. Padovan
  2010-06-05  7:50                   ` [PATCH 10/14] Bluetooth: Fix crash when sending frames after connection is closed Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

After send a Disconnection Request we shall no send and receive frames
anymore. So we set it to BT_DISCONN when a Disconnection Request is
sent then L2CAP will not be able to send or receive any new packet, as
specified by L2CAP spec.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |   50 +++++++++++++++++++++++++++++++++---------------
 1 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 35b7791..3db0078 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -428,14 +428,41 @@ static void l2cap_do_start(struct sock *sk)
 	}
 }
 
+static inline void l2cap_ertm_shutdown(struct sock *sk)
+{
+	struct srej_list *l, *tmp;
+
+	del_timer(&l2cap_pi(sk)->retrans_timer);
+	del_timer(&l2cap_pi(sk)->monitor_timer);
+	del_timer(&l2cap_pi(sk)->ack_timer);
+
+	skb_queue_purge(SREJ_QUEUE(sk));
+	skb_queue_purge(BUSY_QUEUE(sk));
+
+	list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
+		list_del(&l->list);
+		kfree(l);
+	}
+}
+
 static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct sock *sk)
 {
 	struct l2cap_disconn_req req;
 
+	skb_queue_purge(TX_QUEUE(sk));
+
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+		del_timer(&l2cap_pi(sk)->retrans_timer);
+		del_timer(&l2cap_pi(sk)->monitor_timer);
+		del_timer(&l2cap_pi(sk)->ack_timer);
+	}
+
 	req.dcid = cpu_to_le16(l2cap_pi(sk)->dcid);
 	req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
 	l2cap_send_cmd(conn, l2cap_get_ident(conn),
 			L2CAP_DISCONN_REQ, sizeof(req), &req);
+
+	sk->sk_state = BT_DISCONN;
 }
 
 /* ---- L2CAP connections ---- */
@@ -726,7 +753,6 @@ static void __l2cap_sock_close(struct sock *sk, int reason)
 				sk->sk_type == SOCK_STREAM) {
 			struct l2cap_conn *conn = l2cap_pi(sk)->conn;
 
-			sk->sk_state = BT_DISCONN;
 			l2cap_sock_set_timer(sk, sk->sk_sndtimeo);
 			l2cap_send_disconn_req(conn, sk);
 		} else
@@ -1409,6 +1435,9 @@ static int l2cap_ertm_send(struct sock *sk)
 	u16 control, fcs;
 	int nsent = 0;
 
+	if (sk->sk_state != BT_CONNECTED)
+		return -ENOTCONN;
+
 	if (pi->conn_state & L2CAP_CONN_WAIT_F)
 		return 0;
 
@@ -3068,7 +3097,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 
 	default:
 		sk->sk_state = BT_DISCONN;
-		sk->sk_err = ECONNRESET;
 		l2cap_sock_set_timer(sk, HZ * 5);
 		l2cap_send_disconn_req(conn, sk);
 		goto done;
@@ -3123,13 +3151,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 
 	skb_queue_purge(TX_QUEUE(sk));
 
-	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
-		skb_queue_purge(SREJ_QUEUE(sk));
-		skb_queue_purge(BUSY_QUEUE(sk));
-		del_timer(&l2cap_pi(sk)->retrans_timer);
-		del_timer(&l2cap_pi(sk)->monitor_timer);
-		del_timer(&l2cap_pi(sk)->ack_timer);
-	}
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM)
+		l2cap_ertm_shutdown(sk);
 
 	l2cap_chan_del(sk, ECONNRESET);
 	bh_unlock_sock(sk);
@@ -3155,13 +3178,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 
 	skb_queue_purge(TX_QUEUE(sk));
 
-	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
-		skb_queue_purge(SREJ_QUEUE(sk));
-		skb_queue_purge(BUSY_QUEUE(sk));
-		del_timer(&l2cap_pi(sk)->retrans_timer);
-		del_timer(&l2cap_pi(sk)->monitor_timer);
-		del_timer(&l2cap_pi(sk)->ack_timer);
-	}
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM)
+		l2cap_ertm_shutdown(sk);
 
 	l2cap_chan_del(sk, 0);
 	bh_unlock_sock(sk);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/14] Bluetooth: Fix crash when sending frames after connection is closed
  2010-06-05  7:50                 ` [PATCH 09/14] Bluetooth: Fix ERTM channel shutdown Gustavo F. Padovan
@ 2010-06-05  7:50                   ` Gustavo F. Padovan
  2010-06-05  7:50                     ` [PATCH 11/14] Bluetooth: Fix handle of received P-bit Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

At the time the channel is closed we can't really know if the timer
was really deleted, since we used del_timer(). We can't call
del_timer_sync() in interrupt context! So sometimes the acktimeout
expires and try to send a acknowledgement, but we don't have any
connection anymore.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 3db0078..e5b766d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -345,8 +345,12 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
 	struct sk_buff *skb;
 	struct l2cap_hdr *lh;
 	struct l2cap_conn *conn = pi->conn;
+	struct sock *sk = (struct sock *)pi;
 	int count, hlen = L2CAP_HDR_SIZE + 2;
 
+	if (sk->sk_state != BT_CONNECTED)
+		return;
+
 	if (pi->fcs == L2CAP_FCS_CRC16)
 		hlen += 2;
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 11/14] Bluetooth: Fix handle of received P-bit
  2010-06-05  7:50                   ` [PATCH 10/14] Bluetooth: Fix crash when sending frames after connection is closed Gustavo F. Padovan
@ 2010-06-05  7:50                     ` Gustavo F. Padovan
  2010-06-05  7:50                       ` [PATCH 12/14] Bluetooth: Fix l2cap_ertm_send() behavior Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

This is a protocol error. We shall send a F-bit in response
to any P-bit received.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e5b766d..f116307 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1420,8 +1420,15 @@ static void l2cap_retransmit_one_frame(struct sock *sk, u8 tx_seq)
 	tx_skb = skb_clone(skb, GFP_ATOMIC);
 	bt_cb(skb)->retries++;
 	control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
+
+	if (pi->conn_state & L2CAP_CONN_SEND_FBIT) {
+		control |= L2CAP_CTRL_FINAL;
+		pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
+	}
+
 	control |= (pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 			| (tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
+
 	put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
 
 	if (pi->fcs == L2CAP_FCS_CRC16) {
@@ -3383,7 +3390,6 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 	u16 control = 0;
 
 	pi->frames_sent = 0;
-	pi->conn_state |= L2CAP_CONN_SEND_FBIT;
 
 	control |= pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;
 
@@ -3952,6 +3958,7 @@ static inline void l2cap_data_channel_rrframe(struct sock *sk, u16 rx_control)
 	l2cap_drop_acked_frames(sk);
 
 	if (rx_control & L2CAP_CTRL_POLL) {
+		pi->conn_state |= L2CAP_CONN_SEND_FBIT;
 		if (pi->conn_state & L2CAP_CONN_SREJ_SENT) {
 			if ((pi->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
 					(pi->unacked_frames > 0))
@@ -4019,6 +4026,8 @@ static inline void l2cap_data_channel_srejframe(struct sock *sk, u16 rx_control)
 	if (rx_control & L2CAP_CTRL_POLL) {
 		pi->expected_ack_seq = tx_seq;
 		l2cap_drop_acked_frames(sk);
+
+		pi->conn_state |= L2CAP_CONN_SEND_FBIT;
 		l2cap_retransmit_one_frame(sk, tx_seq);
 
 		spin_lock_bh(&pi->send_lock);
@@ -4053,6 +4062,9 @@ static inline void l2cap_data_channel_rnrframe(struct sock *sk, u16 rx_control)
 	pi->expected_ack_seq = tx_seq;
 	l2cap_drop_acked_frames(sk);
 
+	if (rx_control & L2CAP_CTRL_POLL)
+		pi->conn_state |= L2CAP_CONN_SEND_FBIT;
+
 	if (!(pi->conn_state & L2CAP_CONN_SREJ_SENT)) {
 		del_timer(&pi->retrans_timer);
 		if (rx_control & L2CAP_CTRL_POLL)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 12/14] Bluetooth: Fix l2cap_ertm_send() behavior
  2010-06-05  7:50                     ` [PATCH 11/14] Bluetooth: Fix handle of received P-bit Gustavo F. Padovan
@ 2010-06-05  7:50                       ` Gustavo F. Padovan
  2010-06-05  7:50                         ` [PATCH 13/14] Bluetooth: Fix SREJ_QUEUE corruption Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

This patch makes l2cap_ertm_send() similar to the Send-Data action of
the ERTM spec. We do not have to check for RemoteBusy or WAIT_F state
inside l2cap_ertm_send(). Such check was causing a bug in the
retransmission logic of ERTM.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index f116307..59c4690 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1449,11 +1449,7 @@ static int l2cap_ertm_send(struct sock *sk)
 	if (sk->sk_state != BT_CONNECTED)
 		return -ENOTCONN;
 
-	if (pi->conn_state & L2CAP_CONN_WAIT_F)
-		return 0;
-
-	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk)) &&
-			!(pi->conn_state & L2CAP_CONN_REMOTE_BUSY)) {
+	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk))) {
 
 		if (pi->remote_max_tx &&
 				bt_cb(skb)->retries == pi->remote_max_tx) {
@@ -1828,6 +1824,11 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 		if (pi->mode == L2CAP_MODE_STREAMING) {
 			err = l2cap_streaming_send(sk);
 		} else {
+			if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY &&
+					pi->conn_state && L2CAP_CONN_WAIT_F) {
+				err = len;
+				break;
+			}
 			spin_lock_bh(&pi->send_lock);
 			err = l2cap_ertm_send(sk);
 			spin_unlock_bh(&pi->send_lock);
@@ -3403,8 +3404,6 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 	if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
 		__mod_retrans_timer();
 
-	pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
-
 	spin_lock_bh(&pi->send_lock);
 	l2cap_ertm_send(sk);
 	spin_unlock_bh(&pi->send_lock);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 13/14] Bluetooth: Fix SREJ_QUEUE corruption
  2010-06-05  7:50                       ` [PATCH 12/14] Bluetooth: Fix l2cap_ertm_send() behavior Gustavo F. Padovan
@ 2010-06-05  7:50                         ` Gustavo F. Padovan
  2010-06-05  7:50                           ` [PATCH 14/14] Bluetooth: Fix missing retransmission action with RR(P=1) Gustavo F. Padovan
  0 siblings, 1 reply; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, João Paulo Rechi Vita

From: João Paulo Rechi Vita <jprvita@profusion.mobi>

Since all TxSeq values are modulo, we can't compare them directly. We
have to compare their offset inside the TxWindow instead.

Signed-off-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
Acked-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 59c4690..4a737b2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3418,6 +3418,8 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_seq, u8 sar)
 {
 	struct sk_buff *next_skb;
+	struct l2cap_pinfo *pi = l2cap_pi(sk);
+	int tx_seq_offset, next_tx_seq_offset;
 
 	bt_cb(skb)->tx_seq = tx_seq;
 	bt_cb(skb)->sar = sar;
@@ -3428,11 +3430,20 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
 		return 0;
 	}
 
+	tx_seq_offset = (tx_seq - pi->buffer_seq) % 64;
+	if (tx_seq_offset < 0)
+		tx_seq_offset += 64;
+
 	do {
 		if (bt_cb(next_skb)->tx_seq == tx_seq)
 			return -EINVAL;
 
-		if (bt_cb(next_skb)->tx_seq > tx_seq) {
+		next_tx_seq_offset = (bt_cb(next_skb)->tx_seq -
+						pi->buffer_seq) % 64;
+		if (next_tx_seq_offset < 0)
+			next_tx_seq_offset += 64;
+
+		if (next_tx_seq_offset > tx_seq_offset) {
 			__skb_queue_before(SREJ_QUEUE(sk), next_skb, skb);
 			return 0;
 		}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 14/14] Bluetooth: Fix missing retransmission action with RR(P=1)
  2010-06-05  7:50                         ` [PATCH 13/14] Bluetooth: Fix SREJ_QUEUE corruption Gustavo F. Padovan
@ 2010-06-05  7:50                           ` Gustavo F. Padovan
  0 siblings, 0 replies; 15+ messages in thread
From: Gustavo F. Padovan @ 2010-06-05  7:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

The Bluetooth SIG Profile Tuning Suite Software uses the CSA1 spec
to run the L2CAP tests. The new 3.0 spec has a missing
Retransmit-I-Frames action when the Remote side is Busy.
We still start the retransmission timer if Remote is Busy and unacked
frames > 0. We do everything we did before this change plus the
Retransmission of I-frames.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
 net/bluetooth/l2cap.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 4a737b2..1c35ff2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3401,8 +3401,8 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
 		pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
 	}
 
-	if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
-		__mod_retrans_timer();
+	if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY)
+		l2cap_retransmit_frames(sk);
 
 	spin_lock_bh(&pi->send_lock);
 	l2cap_ertm_send(sk);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-06-05  7:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-05  7:50 Pull request: Fixes for the Enhanced Retransmission Mode Gustavo F. Padovan
2010-06-05  7:50 ` [PATCH 01/14] Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP Gustavo F. Padovan
2010-06-05  7:50   ` [PATCH 02/14] Bluetooth: Remove L2CAP Extended Features from Kconfig Gustavo F. Padovan
2010-06-05  7:50     ` [PATCH 03/14] Bluetooth: Fix drop of packets with invalid req_seq/tx_seq Gustavo F. Padovan
2010-06-05  7:50       ` [PATCH 04/14] Bluetooth: Check skb_clone return to avoid NULL dereference Gustavo F. Padovan
2010-06-05  7:50         ` [PATCH 05/14] Bluetooth: Fix ERTM vars increment Gustavo F. Padovan
2010-06-05  7:50           ` [PATCH 06/14] Bluetooth: Check packet FCS earlier Gustavo F. Padovan
2010-06-05  7:50             ` [PATCH 07/14] Bluetooth: Only check SAR bits if frame is I-frame Gustavo F. Padovan
2010-06-05  7:50               ` [PATCH 08/14] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT Gustavo F. Padovan
2010-06-05  7:50                 ` [PATCH 09/14] Bluetooth: Fix ERTM channel shutdown Gustavo F. Padovan
2010-06-05  7:50                   ` [PATCH 10/14] Bluetooth: Fix crash when sending frames after connection is closed Gustavo F. Padovan
2010-06-05  7:50                     ` [PATCH 11/14] Bluetooth: Fix handle of received P-bit Gustavo F. Padovan
2010-06-05  7:50                       ` [PATCH 12/14] Bluetooth: Fix l2cap_ertm_send() behavior Gustavo F. Padovan
2010-06-05  7:50                         ` [PATCH 13/14] Bluetooth: Fix SREJ_QUEUE corruption Gustavo F. Padovan
2010-06-05  7:50                           ` [PATCH 14/14] Bluetooth: Fix missing retransmission action with RR(P=1) Gustavo F. Padovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).