* 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).