[PATCH 1/7] Bluetooth: Put Basic Mode to be the default when using SOCK_SEQPACKET

[PATCH 1/7] Bluetooth: Put Basic Mode to be the default when using SOCK_SEQPACKET

[Gustavo F. Padovan]

We can't break userspace app that will continue using SOCK_SEQPACKET.
The default mode for SOCK_SEQPACKET is Basic Mode, so when no one
specifies the mode and sk_type is SOCK_SEQPACKET Basic Mode shall be used.

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 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 b030125..9d586fb 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2202,7 +2202,7 @@ static int l2cap_build_conf_req(struct sock *sk, void *data)
 {
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 	struct l2cap_conf_req *req = data;
-	struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_ERTM };
+	struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_BASIC };
 	void *ptr = req->data;
 
 	BT_DBG("sk %p", sk);
-- 
1.6.3.3

[PATCH 2/7] Bluetooth: Initialize variables and timers for both channel's sides

[Gustavo F. Padovan]

ERTM entity need to handle state vars, timers and counters to send and
receive I-frames. We initialize all of them to the default values here.

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 net/bluetooth/l2cap.c |   53 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9d586fb..3f9d74b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2169,6 +2169,20 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val)
 	*ptr += L2CAP_CONF_OPT_SIZE + len;
 }
 
+static inline void l2cap_ertm_init(struct sock *sk) {
+	l2cap_pi(sk)->expected_ack_seq = 0;
+	l2cap_pi(sk)->unacked_frames = 0;
+	l2cap_pi(sk)->buffer_seq = 0;
+	l2cap_pi(sk)->num_to_ack = 0;
+
+	setup_timer(&l2cap_pi(sk)->retrans_timer,
+			l2cap_retrans_timeout, (unsigned long) sk);
+	setup_timer(&l2cap_pi(sk)->monitor_timer,
+			l2cap_monitor_timeout, (unsigned long) sk);
+
+	__skb_queue_head_init(SREJ_QUEUE(sk));
+}
+
 static int l2cap_mode_supported(__u8 mode, __u32 feat_mask)
 {
 	u32 local_feat_mask = l2cap_feat_mask;
@@ -2752,17 +2766,13 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
 
 		sk->sk_state = BT_CONNECTED;
-		l2cap_pi(sk)->next_tx_seq = 0;
-		l2cap_pi(sk)->expected_ack_seq = 0;
-		l2cap_pi(sk)->unacked_frames = 0;
-
-		setup_timer(&l2cap_pi(sk)->retrans_timer,
-				l2cap_retrans_timeout, (unsigned long) sk);
-		setup_timer(&l2cap_pi(sk)->monitor_timer,
-				l2cap_monitor_timeout, (unsigned long) sk);
 
+		l2cap_pi(sk)->next_tx_seq = 0;
+		l2cap_pi(sk)->expected_tx_seq = 0;
 		__skb_queue_head_init(TX_QUEUE(sk));
-		__skb_queue_head_init(SREJ_QUEUE(sk));
+		if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM)
+			l2cap_ertm_init(sk);
+
 		l2cap_chan_ready(sk);
 		goto unlock;
 	}
@@ -2841,11 +2851,12 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
 
 		sk->sk_state = BT_CONNECTED;
+		l2cap_pi(sk)->next_tx_seq = 0;
 		l2cap_pi(sk)->expected_tx_seq = 0;
-		l2cap_pi(sk)->buffer_seq = 0;
-		l2cap_pi(sk)->num_to_ack = 0;
 		__skb_queue_head_init(TX_QUEUE(sk));
-		__skb_queue_head_init(SREJ_QUEUE(sk));
+		if (l2cap_pi(sk)->mode ==  L2CAP_MODE_ERTM)
+			l2cap_ertm_init(sk);
+
 		l2cap_chan_ready(sk);
 	}
 
@@ -2877,9 +2888,12 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 	sk->sk_shutdown = SHUTDOWN_MASK;
 
 	skb_queue_purge(TX_QUEUE(sk));
-	skb_queue_purge(SREJ_QUEUE(sk));
-	del_timer(&l2cap_pi(sk)->retrans_timer);
-	del_timer(&l2cap_pi(sk)->monitor_timer);
+
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+		skb_queue_purge(SREJ_QUEUE(sk));
+		del_timer(&l2cap_pi(sk)->retrans_timer);
+		del_timer(&l2cap_pi(sk)->monitor_timer);
+	}
 
 	l2cap_chan_del(sk, ECONNRESET);
 	bh_unlock_sock(sk);
@@ -2904,9 +2918,12 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 		return 0;
 
 	skb_queue_purge(TX_QUEUE(sk));
-	skb_queue_purge(SREJ_QUEUE(sk));
-	del_timer(&l2cap_pi(sk)->retrans_timer);
-	del_timer(&l2cap_pi(sk)->monitor_timer);
+
+	if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+		skb_queue_purge(SREJ_QUEUE(sk));
+		del_timer(&l2cap_pi(sk)->retrans_timer);
+		del_timer(&l2cap_pi(sk)->monitor_timer);
+	}
 
 	l2cap_chan_del(sk, 0);
 	bh_unlock_sock(sk);
-- 
1.6.3.3

[PATCH 3/7] Bluetooth: Fix unset of SrejActioned flag

[Gustavo F. Padovan]

pi->conn_state is the right place to set states

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 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 3f9d74b..658e27c 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3433,7 +3433,7 @@ static inline int l2cap_data_channel_sframe(struct sock *sk, u16 rx_control, str
 		} else if (rx_control & L2CAP_CTRL_FINAL) {
 			if ((pi->conn_state & L2CAP_CONN_SREJ_ACT) &&
 					pi->srej_save_reqseq == tx_seq)
-				pi->srej_save_reqseq &= ~L2CAP_CONN_SREJ_ACT;
+				pi->conn_state &= ~L2CAP_CONN_SREJ_ACT;
 			else
 				l2cap_retransmit_frame(sk, tx_seq);
 		}
-- 
1.6.3.3

[PATCH 4/7] Bluetooth: send ReqSeq on I-frames

[Gustavo F. Padovan]

A ERTM channel can acknowledge received I-frames by sending a I-frame with
the proper ReqSeq value (i.e. ReqSeq is set to ExpectedTxSeq).

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 include/net/bluetooth/l2cap.h |    1 -
 net/bluetooth/l2cap.c         |    8 ++++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 9516f4b..327eb57 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -324,7 +324,6 @@ struct l2cap_pinfo {
 
 	__u8		next_tx_seq;
 	__u8		expected_ack_seq;
-	__u8		req_seq;
 	__u8		expected_tx_seq;
 	__u8		buffer_seq;
 	__u8		buffer_seq_srej;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 658e27c..ed245c4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1329,7 +1329,7 @@ static int l2cap_retransmit_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);
-		control |= (pi->req_seq << L2CAP_CTRL_REQSEQ_SHIFT)
+		control |= (pi->expected_tx_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 				| (tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
 		put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
 
@@ -1371,7 +1371,7 @@ static int l2cap_ertm_send(struct sock *sk)
 		bt_cb(skb)->retries++;
 
 		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
-		control |= (pi->req_seq << L2CAP_CTRL_REQSEQ_SHIFT)
+		control |= (pi->expected_tx_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 				| (pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
 		put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
 
@@ -3288,12 +3288,16 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 {
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 	u8 tx_seq = __get_txseq(rx_control);
+	u8 req_seq = __get_reqseq(rx_control);
 	u16 tx_control = 0;
 	u8 sar = rx_control >> L2CAP_CTRL_SAR_SHIFT;
 	int err = 0;
 
 	BT_DBG("sk %p rx_control 0x%4.4x len %d", sk, rx_control, skb->len);
 
+	pi->expected_ack_seq = req_seq;
+	l2cap_drop_acked_frames(sk);
+
 	if (tx_seq == pi->expected_tx_seq)
 		goto expected;
 
-- 
1.6.3.3

[PATCH 5/7] Bluetooth: Implement RejActioned flag

[Gustavo F. Padovan]

RejActioned is used to prevent retransmission when a entity is on the
WAIT_F state. After send a frame with F-bit set the remote entity can
receive I-frames again. Local L2CAP receives the F-bit and start sending
I-frames.

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap.c         |   38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 327eb57..17a689f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -374,6 +374,7 @@ struct l2cap_pinfo {
 #define L2CAP_CONN_SEND_PBIT       0x10
 #define L2CAP_CONN_REMOTE_BUSY     0x20
 #define L2CAP_CONN_LOCAL_BUSY      0x40
+#define L2CAP_CONN_REJ_ACT         0x80
 
 #define __mod_retrans_timer() mod_timer(&l2cap_pi(sk)->retrans_timer, \
 		jiffies +  msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ed245c4..10bf0c4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3352,6 +3352,16 @@ expected:
 		return 0;
 	}
 
+	if (rx_control & L2CAP_CTRL_FINAL) {
+		if (pi->conn_state & L2CAP_CONN_REJ_ACT)
+			pi->conn_state &= ~L2CAP_CONN_REJ_ACT;
+		else {
+			sk->sk_send_head = TX_QUEUE(sk)->next;
+			pi->next_tx_seq = pi->expected_ack_seq;
+			l2cap_ertm_send(sk);
+		}
+	}
+
 	pi->buffer_seq = (pi->buffer_seq + 1) % 64;
 
 	err = l2cap_sar_reassembly_sdu(sk, skb, rx_control);
@@ -3388,6 +3398,14 @@ static inline int l2cap_data_channel_sframe(struct sock *sk, u16 rx_control, str
 			pi->expected_ack_seq = tx_seq;
 			l2cap_drop_acked_frames(sk);
 
+			if (pi->conn_state & L2CAP_CONN_REJ_ACT)
+				pi->conn_state &= ~L2CAP_CONN_REJ_ACT;
+			else {
+				sk->sk_send_head = TX_QUEUE(sk)->next;
+				pi->next_tx_seq = pi->expected_ack_seq;
+				l2cap_ertm_send(sk);
+			}
+
 			if (!(pi->conn_state & L2CAP_CONN_WAIT_F))
 				break;
 
@@ -3415,10 +3433,24 @@ static inline int l2cap_data_channel_sframe(struct sock *sk, u16 rx_control, str
 		pi->expected_ack_seq = __get_reqseq(rx_control);
 		l2cap_drop_acked_frames(sk);
 
-		sk->sk_send_head = TX_QUEUE(sk)->next;
-		pi->next_tx_seq = pi->expected_ack_seq;
+		if (rx_control & L2CAP_CTRL_FINAL) {
+			if (pi->conn_state & L2CAP_CONN_REJ_ACT)
+				pi->conn_state &= ~L2CAP_CONN_REJ_ACT;
+			else {
+				sk->sk_send_head = TX_QUEUE(sk)->next;
+				pi->next_tx_seq = pi->expected_ack_seq;
+				l2cap_ertm_send(sk);
+			}
+		} else {
+			sk->sk_send_head = TX_QUEUE(sk)->next;
+			pi->next_tx_seq = pi->expected_ack_seq;
+			l2cap_ertm_send(sk);
 
-		l2cap_ertm_send(sk);
+			if (pi->conn_state & L2CAP_CONN_WAIT_F) {
+				pi->srej_save_reqseq = tx_seq;
+				pi->conn_state |= L2CAP_CONN_REJ_ACT;
+			}
+		}
 
 		break;
 
-- 
1.6.3.3

[PATCH 6/7] Bluetooth: Fix ReqSeq value on sending I-frames

[Gustavo F. Padovan]

ReqSeq shall be set to BufferSeq instead of ExpectedTxSeq. BufferSeq gives
real notion of txWindow at sending side (the side sending that I-frame).
It is incremented only when I-frames are pulled by reassembly function.

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 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 10bf0c4..ddf70d6 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1329,7 +1329,7 @@ static int l2cap_retransmit_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);
-		control |= (pi->expected_tx_seq << L2CAP_CTRL_REQSEQ_SHIFT)
+		control |= (pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 				| (tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
 		put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
 
@@ -1371,7 +1371,7 @@ static int l2cap_ertm_send(struct sock *sk)
 		bt_cb(skb)->retries++;
 
 		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
-		control |= (pi->expected_tx_seq << L2CAP_CTRL_REQSEQ_SHIFT)
+		control |= (pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT)
 				| (pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
 		put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
 
-- 
1.6.3.3

[PATCH 7/7] Bluetooth: Send ReqSeq on 'SendRRorRNR' function

[Gustavo F. Padovan]

SendRRorRNR need to acknowledge received I-frames so ReqSeq is set
to BufferSeq on the the outgoing S-frame.

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 net/bluetooth/l2cap.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ddf70d6..76a7a19 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -373,6 +373,8 @@ static inline int l2cap_send_rr_or_rnr(struct l2cap_pinfo *pi, u16 control)
 	else
 		control |= L2CAP_SUPER_RCV_READY;
 
+	control |= pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;
+
 	return l2cap_send_sframe(pi, control);
 }
 
-- 
1.6.3.3

Re: [PATCH 7/7] Bluetooth: Send ReqSeq on 'SendRRorRNR' function

[Marcel Holtmann]

Hi Gustavo,

> SendRRorRNR need to acknowledge received I-frames so ReqSeq is set
> to BufferSeq on the the outgoing S-frame.

same here :(

Regards

Marcel

Re: [PATCH 6/7] Bluetooth: Fix ReqSeq value on sending I-frames

[Marcel Holtmann]

Hi Gustavo,

> ReqSeq shall be set to BufferSeq instead of ExpectedTxSeq. BufferSeq gives
> real notion of txWindow at sending side (the side sending that I-frame).
> It is incremented only when I-frames are pulled by reassembly function.

more explanation in plain English.

Regards

Marcel

Re: [PATCH 5/7] Bluetooth: Implement RejActioned flag

[Marcel Holtmann]

Hi Gustavo,

> RejActioned is used to prevent retransmission when a entity is on the
> WAIT_F state. After send a frame with F-bit set the remote entity can
> receive I-frames again. Local L2CAP receives the F-bit and start sending
> I-frames.

same as the other one. Explain why this should be included after the
merge windows has closed.

Regards

Marcel

Re: [PATCH 4/7] Bluetooth: send ReqSeq on I-frames

[Marcel Holtmann]

Hi Gustavo,

> A ERTM channel can acknowledge received I-frames by sending a I-frame with
> the proper ReqSeq value (i.e. ReqSeq is set to ExpectedTxSeq).

how does this justify for after the merge window? Explain it like I have
no idea about L2CAP ERTM.

Regards

Marcel

Re: [PATCH 3/7] Bluetooth: Fix unset of SrejActioned flag

[Marcel Holtmann]

Hi Gustavo,

> pi->conn_state is the right place to set states
> 
> Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
> ---
>  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 3f9d74b..658e27c 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -3433,7 +3433,7 @@ static inline int l2cap_data_channel_sframe(struct sock *sk, u16 rx_control, str
>  		} else if (rx_control & L2CAP_CTRL_FINAL) {
>  			if ((pi->conn_state & L2CAP_CONN_SREJ_ACT) &&
>  					pi->srej_save_reqseq == tx_seq)
> -				pi->srej_save_reqseq &= ~L2CAP_CONN_SREJ_ACT;
> +				pi->conn_state &= ~L2CAP_CONN_SREJ_ACT;
>  			else
>  				l2cap_retransmit_frame(sk, tx_seq);
>  		}

clearly a bug, but the commit message is not enough. Describe it in
plain English and not some cryptic way. The patch is easier to
understand the commit message. It should be the other way around.

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: Initialize variables and timers for both channel's sides

[Marcel Holtmann]

Hi Gustavo,

> ERTM entity need to handle state vars, timers and counters to send and
> receive I-frames. We initialize all of them to the default values here.

while this is a good idea. Where is the justification for pushing this
after the merge window?

> Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
> ---
>  net/bluetooth/l2cap.c |   53 ++++++++++++++++++++++++++++++++----------------
>  1 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 9d586fb..3f9d74b 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2169,6 +2169,20 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val)
>  	*ptr += L2CAP_CONF_OPT_SIZE + len;
>  }
>  
> +static inline void l2cap_ertm_init(struct sock *sk) {
> +	l2cap_pi(sk)->expected_ack_seq = 0;

Coding style! The {  belong on the next line.

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: Initialize variables and timers for both channel's sides

[Gustavo F. Padovan]

Hi Marcel,

On Tue, Sep 29, 2009 at 1:55 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Gustavo,
>
>> ERTM entity need to handle state vars, timers and counters to send and
>> receive I-frames. We initialize all of them to the default values here.
>
> while this is a good idea. Where is the justification for pushing this
> after the merge window?

These patches are bug fixes and implementations of missing parts of ERTM sp=
ec.
For instance, they enable ERTM to transmit in a full duplex among other thi=
ngs.
Also, ERTM isn't enabled on L2CAP, so we aren't introducing any regressions=
 and
ASAP we put this code on mainline more feedback we can get from
possible testers.

Should I put this on the commit message?


>
>> Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
>> ---
>> =A0net/bluetooth/l2cap.c | =A0 53 ++++++++++++++++++++++++++++++++------=
----------
>> =A01 files changed, 35 insertions(+), 18 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 9d586fb..3f9d74b 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -2169,6 +2169,20 @@ static void l2cap_add_conf_opt(void **ptr, u8 typ=
e, u8 len, unsigned long val)
>> =A0 =A0 =A0 *ptr +=3D L2CAP_CONF_OPT_SIZE + len;
>> =A0}
>>
>> +static inline void l2cap_ertm_init(struct sock *sk) {
>> + =A0 =A0 l2cap_pi(sk)->expected_ack_seq =3D 0;
>
> Coding style! The { =A0belong on the next line.

Sorry, :(

>
> Regards
>
> Marcel
>
>
>



--=20
Gustavo F. Padovan
http://padovan.org

Re: [PATCH 2/7] Bluetooth: Initialize variables and timers for both channel's sides

[Marcel Holtmann]

Hi Gustavo,

> >> ERTM entity need to handle state vars, timers and counters to send and
> >> receive I-frames. We initialize all of them to the default values here.
> >
> > while this is a good idea. Where is the justification for pushing this
> > after the merge window?
> 
> These patches are bug fixes and implementations of missing parts of ERTM spec.
> For instance, they enable ERTM to transmit in a full duplex among other things.
> Also, ERTM isn't enabled on L2CAP, so we aren't introducing any regressions and
> ASAP we put this code on mainline more feedback we can get from
> possible testers.
> 
> Should I put this on the commit message?

if it fixes a bug in the current implementation, then that is fine. If
it adds another feature, then it is not acceptable. However if that
feature is mandatory by the specification, then that is a different
story. Make it perfectly clear what you are fixing or adding.

L2CAP has been in the kernel before and so the not introducing a
regression is not a real valid argument. You normally only get that for
new drivers or subsystems.

And forget about the idea with the more testers. After the merge window
that comment is void.

Regards

Marcel

Re: [PATCH 1/7] Bluetooth: Put Basic Mode to be the default when using SOCK_SEQPACKET

[Marcel Holtmann]

Hi Gustavo,

> We can't break userspace app that will continue using SOCK_SEQPACKET.
> The default mode for SOCK_SEQPACKET is Basic Mode, so when no one
> specifies the mode and sk_type is SOCK_SEQPACKET Basic Mode shall be used.

this one is fine. Applied to my tree.

Regards

Marcel