[PATCH 00/16] Completely remove socket dependency from l2cap_core.c

[PATCH 00/16] Completely remove socket dependency from l2cap_core.c

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This is the last series of patches to completely remove the l2cap socket usage
in the l2cap_core.c file. We now have a C API to access L2CAP and as a next
step we should convert RFCOMM to use this API.

Gustavo Padovan (16):
  Bluetooth: Add src and dst info to struct l2cap_chan
  Bluetooth: Remove sk_sndtimeo from l2cap_core.c
  Bluetooth: extend state_change() call to report errors too
  Bluetooth: add l2cap_state_change_and_error()
  Bluetooth: Add missing braces to an "else if"
  Bluetooth: use l2cap_chan_ready() instead of duplicate code
  Bluetooth: duplicate DEFER_SETUP flag on l2cap_chan
  Bluetooth: Improving locking in l2cap_conn_start()
  Bluetooth: lock socket in defer_cb call
  Bluetooth: Remove socket lock from state_change() in l2cap_core
  Bluetooth: remove parent socket usage from l2cap_core.c
  Bluetooth: Use abstract chan->data in comparison
  Bluetooth: Move l2cap_wait_ack() to l2cap_sock.c
  Bluetooth: Create l2cap->ops->resume()
  Bluetooth: Create l2cap->ops->set_shutdown()
  Bluetooth: Remove sk member from struct l2cap_chan

 include/net/bluetooth/l2cap.h |  22 ++++-
 net/bluetooth/a2mp.c          |   4 +-
 net/bluetooth/l2cap_core.c    | 200 ++++++++++++------------------------------
 net/bluetooth/l2cap_sock.c    |  96 +++++++++++++++++---
 net/bluetooth/rfcomm/core.c   |  15 ++--
 5 files changed, 169 insertions(+), 168 deletions(-)

-- 
1.8.1.2

[PATCH 01/16] Bluetooth: Add src and dst info to struct l2cap_chan

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Adding these two info to l2cap_chan makes the l2cap_core.c a little more
independent of the struct sock.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |  3 +++
 net/bluetooth/l2cap_core.c    | 46 ++++++++++++++++++-------------------------
 net/bluetooth/l2cap_sock.c    |  6 +++---
 net/bluetooth/rfcomm/core.c   | 15 +++++++-------
 4 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index cdd3302..6040743 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -435,6 +435,9 @@ struct l2cap_seq_list {
 struct l2cap_chan {
 	struct sock *sk;
 
+	bdaddr_t	src;
+	bdaddr_t	dst;
+
 	struct l2cap_conn	*conn;
 	struct hci_conn		*hs_hcon;
 	struct hci_chan		*hs_hchan;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7c7e932..4a88fa1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -148,7 +148,7 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
 	struct l2cap_chan *c;
 
 	list_for_each_entry(c, &chan_list, global_l) {
-		if (c->sport == psm && !bacmp(&bt_sk(c->sk)->src, src))
+		if (c->sport == psm && !bacmp(&c->src, src))
 			return c;
 	}
 	return NULL;
@@ -1305,7 +1305,6 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 	read_lock(&chan_list_lock);
 
 	list_for_each_entry(c, &chan_list, global_l) {
-		struct sock *sk = c->sk;
 
 		if (state && c->state != state)
 			continue;
@@ -1315,16 +1314,16 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 			int src_any, dst_any;
 
 			/* Exact match. */
-			src_match = !bacmp(&bt_sk(sk)->src, src);
-			dst_match = !bacmp(&bt_sk(sk)->dst, dst);
+			src_match = !bacmp(&c->src, src);
+			dst_match = !bacmp(&c->dst, dst);
 			if (src_match && dst_match) {
 				read_unlock(&chan_list_lock);
 				return c;
 			}
 
 			/* Closest match */
-			src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY);
-			dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY);
+			src_any = !bacmp(&c->src, BDADDR_ANY);
+			dst_any = !bacmp(&c->dst, BDADDR_ANY);
 			if ((src_match && dst_any) || (src_any && dst_match) ||
 			    (src_any && dst_any))
 				c1 = c;
@@ -1338,7 +1337,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 
 static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
-	struct sock *parent, *sk;
+	struct sock *parent;
 	struct l2cap_chan *chan, *pchan;
 
 	BT_DBG("");
@@ -1357,13 +1356,11 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	if (!chan)
 		goto clean;
 
-	sk = chan->sk;
-
 	hci_conn_hold(conn->hcon);
 	conn->hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
 
-	bacpy(&bt_sk(sk)->src, conn->src);
-	bacpy(&bt_sk(sk)->dst, conn->dst);
+	bacpy(&chan->src, conn->src);
+	bacpy(&chan->dst, conn->dst);
 
 	l2cap_chan_add(conn, chan);
 
@@ -1572,7 +1569,6 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
 	read_lock(&chan_list_lock);
 
 	list_for_each_entry(c, &chan_list, global_l) {
-		struct sock *sk = c->sk;
 
 		if (state && c->state != state)
 			continue;
@@ -1582,16 +1578,16 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
 			int src_any, dst_any;
 
 			/* Exact match. */
-			src_match = !bacmp(&bt_sk(sk)->src, src);
-			dst_match = !bacmp(&bt_sk(sk)->dst, dst);
+			src_match = !bacmp(&c->src, src);
+			dst_match = !bacmp(&c->dst, dst);
 			if (src_match && dst_match) {
 				read_unlock(&chan_list_lock);
 				return c;
 			}
 
 			/* Closest match */
-			src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY);
-			dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY);
+			src_any = !bacmp(&c->src, BDADDR_ANY);
+			dst_any = !bacmp(&c->dst, BDADDR_ANY);
 			if ((src_match && dst_any) || (src_any && dst_match) ||
 			    (src_any && dst_any))
 				c1 = c;
@@ -1607,7 +1603,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		       bdaddr_t *dst, u8 dst_type)
 {
 	struct sock *sk = chan->sk;
-	bdaddr_t *src = &bt_sk(sk)->src;
+	bdaddr_t *src = &chan->src;
 	struct l2cap_conn *conn;
 	struct hci_conn *hcon;
 	struct hci_dev *hdev;
@@ -1674,9 +1670,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 	}
 
 	/* Set destination address and psm */
-	lock_sock(sk);
-	bacpy(&bt_sk(sk)->dst, dst);
-	release_sock(sk);
+	bacpy(&chan->dst, dst);
 
 	chan->psm = psm;
 	chan->dcid = cid;
@@ -3637,8 +3631,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 
 	hci_conn_hold(conn->hcon);
 
-	bacpy(&bt_sk(sk)->src, conn->src);
-	bacpy(&bt_sk(sk)->dst, conn->dst);
+	bacpy(&chan->src, conn->src);
+	bacpy(&chan->dst, conn->dst);
 	chan->psm  = psm;
 	chan->dcid = scid;
 	chan->local_amp_id = amp_id;
@@ -6285,17 +6279,16 @@ int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
 	/* Find listening sockets and check their link_mode */
 	read_lock(&chan_list_lock);
 	list_for_each_entry(c, &chan_list, global_l) {
-		struct sock *sk = c->sk;
 
 		if (c->state != BT_LISTEN)
 			continue;
 
-		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr)) {
+		if (!bacmp(&c->src, &hdev->bdaddr)) {
 			lm1 |= HCI_LM_ACCEPT;
 			if (test_bit(FLAG_ROLE_SWITCH, &c->flags))
 				lm1 |= HCI_LM_MASTER;
 			exact++;
-		} else if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
+		} else if (!bacmp(&c->src, BDADDR_ANY)) {
 			lm2 |= HCI_LM_ACCEPT;
 			if (test_bit(FLAG_ROLE_SWITCH, &c->flags))
 				lm2 |= HCI_LM_MASTER;
@@ -6579,10 +6572,9 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
 	read_lock(&chan_list_lock);
 
 	list_for_each_entry(c, &chan_list, global_l) {
-		struct sock *sk = c->sk;
 
 		seq_printf(f, "%pMR %pMR %d %d 0x%4.4x 0x%4.4x %d %d %d %d\n",
-			   &bt_sk(sk)->src, &bt_sk(sk)->dst,
+			   &c->src, &c->dst,
 			   c->state, __le16_to_cpu(c->psm),
 			   c->scid, c->dcid, c->imtu, c->omtu,
 			   c->sec_level, c->mode);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1bcfb84..0977966 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -97,7 +97,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 	    __le16_to_cpu(la.l2_psm) == L2CAP_PSM_RFCOMM)
 		chan->sec_level = BT_SECURITY_SDP;
 
-	bacpy(&bt_sk(sk)->src, &la.l2_bdaddr);
+	bacpy(&chan->src, &la.l2_bdaddr);
 
 	chan->state = BT_BOUND;
 	sk->sk_state = BT_BOUND;
@@ -259,11 +259,11 @@ static int l2cap_sock_getname(struct socket *sock, struct sockaddr *addr,
 
 	if (peer) {
 		la->l2_psm = chan->psm;
-		bacpy(&la->l2_bdaddr, &bt_sk(sk)->dst);
+		bacpy(&la->l2_bdaddr, &chan->dst);
 		la->l2_cid = cpu_to_le16(chan->dcid);
 	} else {
 		la->l2_psm = chan->sport;
-		bacpy(&la->l2_bdaddr, &bt_sk(sk)->src);
+		bacpy(&la->l2_bdaddr, &chan->src);
 		la->l2_cid = cpu_to_le16(chan->scid);
 	}
 
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 201fdf7..15dc078 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -632,13 +632,13 @@ static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst)
 {
 	struct rfcomm_session *s;
 	struct list_head *p, *n;
-	struct bt_sock *sk;
+	struct l2cap_chan *chan;
 	list_for_each_safe(p, n, &session_list) {
 		s = list_entry(p, struct rfcomm_session, list);
-		sk = bt_sk(s->sock->sk);
+		chan = l2cap_pi(s->sock->sk)->chan;
 
-		if ((!bacmp(src, BDADDR_ANY) || !bacmp(&sk->src, src)) &&
-				!bacmp(&sk->dst, dst))
+		if ((!bacmp(src, BDADDR_ANY) || !bacmp(&chan->src, src)) &&
+				!bacmp(&chan->dst, dst))
 			return s;
 	}
 	return NULL;
@@ -727,9 +727,9 @@ void rfcomm_session_getaddr(struct rfcomm_session *s, bdaddr_t *src, bdaddr_t *d
 {
 	struct sock *sk = s->sock->sk;
 	if (src)
-		bacpy(src, &bt_sk(sk)->src);
+		bacpy(src, &l2cap_pi(sk)->chan->src);
 	if (dst)
-		bacpy(dst, &bt_sk(sk)->dst);
+		bacpy(dst, &l2cap_pi(sk)->chan->dst);
 }
 
 /* ---- RFCOMM frame sending ---- */
@@ -2126,7 +2126,8 @@ static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
 			struct sock *sk = s->sock->sk;
 
 			seq_printf(f, "%pMR %pMR %ld %d %d %d %d\n",
-				   &bt_sk(sk)->src, &bt_sk(sk)->dst,
+				   &l2cap_pi(sk)->chan->src,
+				   &l2cap_pi(sk)->chan->dst,
 				   d->state, d->dlci, d->mtu,
 				   d->rx_credits, d->tx_credits);
 		}
-- 
1.8.1.2

Re: [PATCH 01/16] Bluetooth: Add src and dst info to struct l2cap_chan

[Marcel Holtmann]

Hi Gustavo,

> Adding these two info to l2cap_chan makes the l2cap_core.c a little more
> independent of the struct sock.

why are we doing this? What is the goal here? Explain it in the commit
message.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |  3 +++
>  net/bluetooth/l2cap_core.c    | 46 ++++++++++++++++++-------------------------
>  net/bluetooth/l2cap_sock.c    |  6 +++---
>  net/bluetooth/rfcomm/core.c   | 15 +++++++-------
>  4 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index cdd3302..6040743 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -435,6 +435,9 @@ struct l2cap_seq_list {
>  struct l2cap_chan {
>  	struct sock *sk;
>  
> +	bdaddr_t	src;
> +	bdaddr_t	dst;
> +
>  	struct l2cap_conn	*conn;
>  	struct hci_conn		*hs_hcon;
>  	struct hci_chan		*hs_hchan;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7c7e932..4a88fa1 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -148,7 +148,7 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
>  	struct l2cap_chan *c;
>  
>  	list_for_each_entry(c, &chan_list, global_l) {
> -		if (c->sport == psm && !bacmp(&bt_sk(c->sk)->src, src))
> +		if (c->sport == psm && !bacmp(&c->src, src))
>  			return c;
>  	}
>  	return NULL;
> @@ -1305,7 +1305,6 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
>  	read_lock(&chan_list_lock);
>  
>  	list_for_each_entry(c, &chan_list, global_l) {
> -		struct sock *sk = c->sk;
>  
>  		if (state && c->state != state)
>  			continue;
> @@ -1315,16 +1314,16 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
>  			int src_any, dst_any;
>  
>  			/* Exact match. */
> -			src_match = !bacmp(&bt_sk(sk)->src, src);
> -			dst_match = !bacmp(&bt_sk(sk)->dst, dst);
> +			src_match = !bacmp(&c->src, src);
> +			dst_match = !bacmp(&c->dst, dst);
>  			if (src_match && dst_match) {
>  				read_unlock(&chan_list_lock);
>  				return c;
>  			}
>  
>  			/* Closest match */
> -			src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY);
> -			dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY);
> +			src_any = !bacmp(&c->src, BDADDR_ANY);
> +			dst_any = !bacmp(&c->dst, BDADDR_ANY);
>  			if ((src_match && dst_any) || (src_any && dst_match) ||
>  			    (src_any && dst_any))
>  				c1 = c;
> @@ -1338,7 +1337,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
>  
>  static void l2cap_le_conn_ready(struct l2cap_conn *conn)
>  {
> -	struct sock *parent, *sk;
> +	struct sock *parent;
>  	struct l2cap_chan *chan, *pchan;
>  
>  	BT_DBG("");
> @@ -1357,13 +1356,11 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
>  	if (!chan)
>  		goto clean;
>  
> -	sk = chan->sk;
> -
>  	hci_conn_hold(conn->hcon);
>  	conn->hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
>  
> -	bacpy(&bt_sk(sk)->src, conn->src);
> -	bacpy(&bt_sk(sk)->dst, conn->dst);
> +	bacpy(&chan->src, conn->src);
> +	bacpy(&chan->dst, conn->dst);
>  
>  	l2cap_chan_add(conn, chan);
>  
> @@ -1572,7 +1569,6 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
>  	read_lock(&chan_list_lock);
>  
>  	list_for_each_entry(c, &chan_list, global_l) {
> -		struct sock *sk = c->sk;
>  

You leave an empty line here?

>  		if (state && c->state != state)
>  			continue;
> @@ -1582,16 +1578,16 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
>  			int src_any, dst_any;
>  
>  			/* Exact match. */
> -			src_match = !bacmp(&bt_sk(sk)->src, src);
> -			dst_match = !bacmp(&bt_sk(sk)->dst, dst);
> +			src_match = !bacmp(&c->src, src);
> +			dst_match = !bacmp(&c->dst, dst);
>  			if (src_match && dst_match) {
>  				read_unlock(&chan_list_lock);
>  				return c;
>  			}
>  
>  			/* Closest match */
> -			src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY);
> -			dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY);
> +			src_any = !bacmp(&c->src, BDADDR_ANY);
> +			dst_any = !bacmp(&c->dst, BDADDR_ANY);
>  			if ((src_match && dst_any) || (src_any && dst_match) ||
>  			    (src_any && dst_any))
>  				c1 = c;
> @@ -1607,7 +1603,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>  		       bdaddr_t *dst, u8 dst_type)
>  {
>  	struct sock *sk = chan->sk;
> -	bdaddr_t *src = &bt_sk(sk)->src;
> +	bdaddr_t *src = &chan->src;
>  	struct l2cap_conn *conn;
>  	struct hci_conn *hcon;
>  	struct hci_dev *hdev;
> @@ -1674,9 +1670,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>  	}
>  
>  	/* Set destination address and psm */
> -	lock_sock(sk);
> -	bacpy(&bt_sk(sk)->dst, dst);
> -	release_sock(sk);
> +	bacpy(&chan->dst, dst);
>  
>  	chan->psm = psm;
>  	chan->dcid = cid;
> @@ -3637,8 +3631,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  
>  	hci_conn_hold(conn->hcon);
>  
> -	bacpy(&bt_sk(sk)->src, conn->src);
> -	bacpy(&bt_sk(sk)->dst, conn->dst);
> +	bacpy(&chan->src, conn->src);
> +	bacpy(&chan->dst, conn->dst);
>  	chan->psm  = psm;
>  	chan->dcid = scid;
>  	chan->local_amp_id = amp_id;
> @@ -6285,17 +6279,16 @@ int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr)
>  	/* Find listening sockets and check their link_mode */
>  	read_lock(&chan_list_lock);
>  	list_for_each_entry(c, &chan_list, global_l) {
> -		struct sock *sk = c->sk;
>  

Same here.

>  		if (c->state != BT_LISTEN)
>  			continue;
>  
> -		if (!bacmp(&bt_sk(sk)->src, &hdev->bdaddr)) {
> +		if (!bacmp(&c->src, &hdev->bdaddr)) {
>  			lm1 |= HCI_LM_ACCEPT;
>  			if (test_bit(FLAG_ROLE_SWITCH, &c->flags))
>  				lm1 |= HCI_LM_MASTER;
>  			exact++;
> -		} else if (!bacmp(&bt_sk(sk)->src, BDADDR_ANY)) {
> +		} else if (!bacmp(&c->src, BDADDR_ANY)) {
>  			lm2 |= HCI_LM_ACCEPT;
>  			if (test_bit(FLAG_ROLE_SWITCH, &c->flags))
>  				lm2 |= HCI_LM_MASTER;
> @@ -6579,10 +6572,9 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
>  	read_lock(&chan_list_lock);
>  
>  	list_for_each_entry(c, &chan_list, global_l) {
> -		struct sock *sk = c->sk;
>  

And here.

>  		seq_printf(f, "%pMR %pMR %d %d 0x%4.4x 0x%4.4x %d %d %d %d\n",
> -			   &bt_sk(sk)->src, &bt_sk(sk)->dst,
> +			   &c->src, &c->dst,
>  			   c->state, __le16_to_cpu(c->psm),
>  			   c->scid, c->dcid, c->imtu, c->omtu,
>  			   c->sec_level, c->mode);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 1bcfb84..0977966 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -97,7 +97,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
>  	    __le16_to_cpu(la.l2_psm) == L2CAP_PSM_RFCOMM)
>  		chan->sec_level = BT_SECURITY_SDP;
>  
> -	bacpy(&bt_sk(sk)->src, &la.l2_bdaddr);
> +	bacpy(&chan->src, &la.l2_bdaddr);
>  
>  	chan->state = BT_BOUND;
>  	sk->sk_state = BT_BOUND;
> @@ -259,11 +259,11 @@ static int l2cap_sock_getname(struct socket *sock, struct sockaddr *addr,
>  
>  	if (peer) {
>  		la->l2_psm = chan->psm;
> -		bacpy(&la->l2_bdaddr, &bt_sk(sk)->dst);
> +		bacpy(&la->l2_bdaddr, &chan->dst);
>  		la->l2_cid = cpu_to_le16(chan->dcid);
>  	} else {
>  		la->l2_psm = chan->sport;
> -		bacpy(&la->l2_bdaddr, &bt_sk(sk)->src);
> +		bacpy(&la->l2_bdaddr, &chan->src);
>  		la->l2_cid = cpu_to_le16(chan->scid);
>  	}
>  
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 201fdf7..15dc078 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -632,13 +632,13 @@ static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst)
>  {
>  	struct rfcomm_session *s;
>  	struct list_head *p, *n;
> -	struct bt_sock *sk;
> +	struct l2cap_chan *chan;
>  	list_for_each_safe(p, n, &session_list) {
>  		s = list_entry(p, struct rfcomm_session, list);
> -		sk = bt_sk(s->sock->sk);
> +		chan = l2cap_pi(s->sock->sk)->chan;
>  
> -		if ((!bacmp(src, BDADDR_ANY) || !bacmp(&sk->src, src)) &&
> -				!bacmp(&sk->dst, dst))
> +		if ((!bacmp(src, BDADDR_ANY) || !bacmp(&chan->src, src)) &&
> +				!bacmp(&chan->dst, dst))
>  			return s;
>  	}
>  	return NULL;
> @@ -727,9 +727,9 @@ void rfcomm_session_getaddr(struct rfcomm_session *s, bdaddr_t *src, bdaddr_t *d
>  {
>  	struct sock *sk = s->sock->sk;
>  	if (src)
> -		bacpy(src, &bt_sk(sk)->src);
> +		bacpy(src, &l2cap_pi(sk)->chan->src);
>  	if (dst)
> -		bacpy(dst, &bt_sk(sk)->dst);
> +		bacpy(dst, &l2cap_pi(sk)->chan->dst);
>  }
>  
>  /* ---- RFCOMM frame sending ---- */
> @@ -2126,7 +2126,8 @@ static int rfcomm_dlc_debugfs_show(struct seq_file *f, void *x)
>  			struct sock *sk = s->sock->sk;
>  
>  			seq_printf(f, "%pMR %pMR %ld %d %d %d %d\n",
> -				   &bt_sk(sk)->src, &bt_sk(sk)->dst,
> +				   &l2cap_pi(sk)->chan->src,
> +				   &l2cap_pi(sk)->chan->dst,
>  				   d->state, d->dlci, d->mtu,
>  				   d->rx_credits, d->tx_credits);
>  		}

Regards

Marcel

[PATCH 02/16] Bluetooth: Remove sk_sndtimeo from l2cap_core.c

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

We now have a new struct member in l2cap_chan to store this value for
access inside l2cap_core.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h | 1 +
 net/bluetooth/l2cap_core.c    | 7 +++----
 net/bluetooth/l2cap_sock.c    | 1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 6040743..cf94e68 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -480,6 +480,7 @@ struct l2cap_chan {
 	__u8		tx_state;
 	__u8		rx_state;
 
+	unsigned long	sndtimeo;
 	unsigned long	conf_state;
 	unsigned long	conn_state;
 	unsigned long	flags;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4a88fa1..a306cc8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -630,7 +630,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 	case BT_CONFIG:
 		if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED &&
 		    conn->hcon->type == ACL_LINK) {
-			__set_chan_timer(chan, sk->sk_sndtimeo);
+			__set_chan_timer(chan, chan->sndtimeo);
 			l2cap_send_disconn_req(chan, reason);
 		} else
 			l2cap_chan_del(chan, reason);
@@ -1602,7 +1602,6 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
 int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		       bdaddr_t *dst, u8 dst_type)
 {
-	struct sock *sk = chan->sk;
 	bdaddr_t *src = &chan->src;
 	struct l2cap_conn *conn;
 	struct hci_conn *hcon;
@@ -1716,7 +1715,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 	l2cap_chan_lock(chan);
 
 	l2cap_state_change(chan, BT_CONNECT);
-	__set_chan_timer(chan, sk->sk_sndtimeo);
+	__set_chan_timer(chan, chan->sndtimeo);
 
 	if (hcon->state == BT_CONNECTED) {
 		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
@@ -3641,7 +3640,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 
 	dcid = chan->scid;
 
-	__set_chan_timer(chan, sk->sk_sndtimeo);
+	__set_chan_timer(chan, chan->sndtimeo);
 
 	chan->ident = cmd->ident;
 
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 0977966..221aef9 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1178,6 +1178,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 
 	/* Default config options */
 	chan->flush_to = L2CAP_DEFAULT_FLUSH_TO;
+	chan->sndtimeo = sk->sk_sndtimeo;
 
 	chan->data = sk;
 	chan->ops = &l2cap_chan_ops;
-- 
1.8.1.2

Re: [PATCH 02/16] Bluetooth: Remove sk_sndtimeo from l2cap_core.c

[Marcel Holtmann]

Hi Gustavo,

> We now have a new struct member in l2cap_chan to store this value for
> access inside l2cap_core.c

more commit message explaining what is happening in this patch and why.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h | 1 +
>  net/bluetooth/l2cap_core.c    | 7 +++----
>  net/bluetooth/l2cap_sock.c    | 1 +
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 6040743..cf94e68 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -480,6 +480,7 @@ struct l2cap_chan {
>  	__u8		tx_state;
>  	__u8		rx_state;
>  
> +	unsigned long	sndtimeo;

The socket one is long, why is this one unsigned long?

>  	unsigned long	conf_state;
>  	unsigned long	conn_state;
>  	unsigned long	flags;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 4a88fa1..a306cc8 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -630,7 +630,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
>  	case BT_CONFIG:
>  		if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED &&
>  		    conn->hcon->type == ACL_LINK) {
> -			__set_chan_timer(chan, sk->sk_sndtimeo);
> +			__set_chan_timer(chan, chan->sndtimeo);
>  			l2cap_send_disconn_req(chan, reason);
>  		} else
>  			l2cap_chan_del(chan, reason);
> @@ -1602,7 +1602,6 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm,
>  int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>  		       bdaddr_t *dst, u8 dst_type)
>  {
> -	struct sock *sk = chan->sk;
>  	bdaddr_t *src = &chan->src;
>  	struct l2cap_conn *conn;
>  	struct hci_conn *hcon;
> @@ -1716,7 +1715,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>  	l2cap_chan_lock(chan);
>  
>  	l2cap_state_change(chan, BT_CONNECT);
> -	__set_chan_timer(chan, sk->sk_sndtimeo);
> +	__set_chan_timer(chan, chan->sndtimeo);
>  
>  	if (hcon->state == BT_CONNECTED) {
>  		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
> @@ -3641,7 +3640,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  
>  	dcid = chan->scid;
>  
> -	__set_chan_timer(chan, sk->sk_sndtimeo);
> +	__set_chan_timer(chan, chan->sndtimeo);
>  
>  	chan->ident = cmd->ident;
>  
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 0977966..221aef9 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1178,6 +1178,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
>  
>  	/* Default config options */
>  	chan->flush_to = L2CAP_DEFAULT_FLUSH_TO;
> +	chan->sndtimeo = sk->sk_sndtimeo;

Explain to me how this actually works. Have you thought about
SO_SNDTIMEO socket option?

Regards

Marcel

[PATCH 03/16] Bluetooth: extend state_change() call to report errors too

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Now l2cap_core doesn't need to touch sk_err element anymore, it just tell
l2cap_sock via this call which error it wants to set.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h | 2 +-
 net/bluetooth/a2mp.c          | 2 +-
 net/bluetooth/l2cap_core.c    | 6 ++----
 net/bluetooth/l2cap_sock.c    | 6 +++++-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index cf94e68..4f28a8c 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -550,7 +550,7 @@ struct l2cap_ops {
 	void			(*teardown) (struct l2cap_chan *chan, int err);
 	void			(*close) (struct l2cap_chan *chan);
 	void			(*state_change) (struct l2cap_chan *chan,
-						 int state);
+						 int state, int err);
 	void			(*ready) (struct l2cap_chan *chan);
 	void			(*defer) (struct l2cap_chan *chan);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index eb0f4b1..ad6e42f 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -671,7 +671,7 @@ static void a2mp_chan_close_cb(struct l2cap_chan *chan)
 	l2cap_chan_put(chan);
 }
 
-static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state)
+static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state, int err)
 {
 	struct amp_mgr *mgr = chan->data;
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a306cc8..775c83f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -216,7 +216,7 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
 	       state_to_string(state));
 
 	chan->state = state;
-	chan->ops->state_change(chan, state);
+	chan->ops->state_change(chan, state, 0);
 }
 
 static void l2cap_state_change(struct l2cap_chan *chan, int state)
@@ -230,9 +230,7 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
 
 static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
 {
-	struct sock *sk = chan->sk;
-
-	sk->sk_err = err;
+	chan->ops->state_change(chan, chan->state, err);
 }
 
 static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 221aef9..8adea0f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1044,11 +1044,15 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
 	release_sock(sk);
 }
 
-static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
+static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
+				       int err)
 {
 	struct sock *sk = chan->data;
 
 	sk->sk_state = state;
+
+	if (err)
+		sk->sk_err = err;
 }
 
 static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
-- 
1.8.1.2

Re: [PATCH 03/16] Bluetooth: extend state_change() call to report errors too

[Marcel Holtmann]

Hi Gustavo,

> Now l2cap_core doesn't need to touch sk_err element anymore, it just tell
> l2cap_sock via this call which error it wants to set.

I need an explanation here since the code and the commit message are
unclear to me.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h | 2 +-
>  net/bluetooth/a2mp.c          | 2 +-
>  net/bluetooth/l2cap_core.c    | 6 ++----
>  net/bluetooth/l2cap_sock.c    | 6 +++++-
>  4 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index cf94e68..4f28a8c 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -550,7 +550,7 @@ struct l2cap_ops {
>  	void			(*teardown) (struct l2cap_chan *chan, int err);
>  	void			(*close) (struct l2cap_chan *chan);
>  	void			(*state_change) (struct l2cap_chan *chan,
> -						 int state);
> +						 int state, int err);

Why is this a good idea? Why not a separate error callback. You need to
explain this.

>  	void			(*ready) (struct l2cap_chan *chan);
>  	void			(*defer) (struct l2cap_chan *chan);
>  	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index eb0f4b1..ad6e42f 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -671,7 +671,7 @@ static void a2mp_chan_close_cb(struct l2cap_chan *chan)
>  	l2cap_chan_put(chan);
>  }
>  
> -static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state)
> +static void a2mp_chan_state_change_cb(struct l2cap_chan *chan, int state, int err)
>  {
>  	struct amp_mgr *mgr = chan->data;
>  
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index a306cc8..775c83f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -216,7 +216,7 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
>  	       state_to_string(state));
>  
>  	chan->state = state;
> -	chan->ops->state_change(chan, state);
> +	chan->ops->state_change(chan, state, 0);
>  }
>  
>  static void l2cap_state_change(struct l2cap_chan *chan, int state)
> @@ -230,9 +230,7 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
>  
>  static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
>  {
> -	struct sock *sk = chan->sk;
> -
> -	sk->sk_err = err;
> +	chan->ops->state_change(chan, chan->state, err);
>  }
>  
>  static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 221aef9..8adea0f 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1044,11 +1044,15 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
>  	release_sock(sk);
>  }
>  
> -static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
> +static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
> +				       int err)
>  {
>  	struct sock *sk = chan->data;
>  
>  	sk->sk_state = state;
> +
> +	if (err)
> +		sk->sk_err = err;
>  }
>  
>  static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,

Regards

Marcel

[PATCH 04/16] Bluetooth: add l2cap_state_change_and_error()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Add helper to set both state and error at the same time.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 775c83f..adf7fbb 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -228,9 +228,15 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
 	release_sock(sk);
 }
 
-static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
+static void l2cap_state_change_and_error(struct l2cap_chan *chan, int state,
+					 int err)
 {
-	chan->ops->state_change(chan, chan->state, err);
+	struct sock *sk = chan->sk;
+
+	lock_sock(sk);
+	chan->state = state;
+	chan->ops->state_change(chan, state, err);
+	release_sock(sk);
 }
 
 static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
@@ -238,7 +244,7 @@ static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
 	struct sock *sk = chan->sk;
 
 	lock_sock(sk);
-	__l2cap_chan_set_err(chan, err);
+	chan->ops->state_change(chan, chan->state, err);
 	release_sock(sk);
 }
 
@@ -1180,7 +1186,6 @@ static inline int l2cap_mode_supported(__u8 mode, __u32 feat_mask)
 
 static void l2cap_send_disconn_req(struct l2cap_chan *chan, int err)
 {
-	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
 	struct l2cap_disconn_req req;
 
@@ -1203,10 +1208,7 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan, int err)
 	l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_DISCONN_REQ,
 		       sizeof(req), &req);
 
-	lock_sock(sk);
-	__l2cap_state_change(chan, BT_DISCONN);
-	__l2cap_chan_set_err(chan, err);
-	release_sock(sk);
+	l2cap_state_change_and_error(chan, BT_DISCONN, err);
 }
 
 /* ---- L2CAP connections ---- */
-- 
1.8.1.2

Re: [PATCH 04/16] Bluetooth: add l2cap_state_change_and_error()

[Marcel Holtmann]

Hi Gustavo,

> Add helper to set both state and error at the same time.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  net/bluetooth/l2cap_core.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 775c83f..adf7fbb 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -228,9 +228,15 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
>  	release_sock(sk);
>  }
>  
> -static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
> +static void l2cap_state_change_and_error(struct l2cap_chan *chan, int state,
> +					 int err)
>  {
> -	chan->ops->state_change(chan, chan->state, err);
> +	struct sock *sk = chan->sk;
> +
> +	lock_sock(sk);
> +	chan->state = state;
> +	chan->ops->state_change(chan, state, err);
> +	release_sock(sk);
>  }

I really do not think this is a good idea. Why are we combining this.
You need to explain why this makes sense. Since I clearly do not see it
and would have kept it separate.

Regards

Marcel

[PATCH 05/16] Bluetooth: Add missing braces to an "else if"

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Trivial change in the coding style.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adf7fbb..ac3205e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1406,8 +1406,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 			sk->sk_state_change(sk);
 			release_sock(sk);
 
-		} else if (chan->state == BT_CONNECT)
+		} else if (chan->state == BT_CONNECT) {
 			l2cap_do_start(chan);
+		}
 
 		l2cap_chan_unlock(chan);
 	}
-- 
1.8.1.2

Re: [PATCH 05/16] Bluetooth: Add missing braces to an "else if"

[Marcel Holtmann]

Hi Gustavo,

> Trivial change in the coding style.

I would really start with such a fix and not in the middle of a large
patch series. Get these out of the way right away and not some point
later.

Regards

Marcel

[PATCH 06/16] Bluetooth: use l2cap_chan_ready() instead of duplicate code

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

l2cap_chan_ready() does exactly what we want here avoiding duplicate code.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ac3205e..fd7fe5e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1399,12 +1399,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 				l2cap_chan_ready(chan);
 
 		} else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
-			struct sock *sk = chan->sk;
-			__clear_chan_timer(chan);
-			lock_sock(sk);
-			__l2cap_state_change(chan, BT_CONNECTED);
-			sk->sk_state_change(sk);
-			release_sock(sk);
+			l2cap_chan_ready(chan);
 
 		} else if (chan->state == BT_CONNECT) {
 			l2cap_do_start(chan);
-- 
1.8.1.2

Re: [PATCH 06/16] Bluetooth: use l2cap_chan_ready() instead of duplicate code

[Marcel Holtmann]

Hi Gustavo,

> l2cap_chan_ready() does exactly what we want here avoiding duplicate code.

what do we want here exactly. Why is l2cap_chan_ready() correct and has
no side effects. Why was the previous code wrong? There must have been a
reason.

Regards

Marcel

[PATCH 07/16] Bluetooth: duplicate DEFER_SETUP flag on l2cap_chan

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Now l2cap_chan also has the DEFER_SETUP flag, so we don't need to access
the bt_sk(sk) to read this flag inside l2cap_core.c. We need to kep it
duplicate since it is used in Bluetooth socket code as well.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |  1 +
 net/bluetooth/l2cap_core.c    | 16 +++++++---------
 net/bluetooth/l2cap_sock.c    |  7 +++++--
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 4f28a8c..0c76c55 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -635,6 +635,7 @@ enum {
 	CONN_REJ_ACT,
 	CONN_SEND_FBIT,
 	CONN_RNR_SENT,
+	CONN_DEFER_SETUP,
 };
 
 /* Definitions for flags in l2cap_chan */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fd7fe5e..2c7ec1b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -620,10 +620,8 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 {
 	struct l2cap_conn *conn = chan->conn;
-	struct sock *sk = chan->sk;
 
-	BT_DBG("chan %p state %s sk %p", chan, state_to_string(chan->state),
-	       sk);
+	BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
 
 	switch (chan->state) {
 	case BT_LISTEN:
@@ -646,7 +644,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 			struct l2cap_conn_rsp rsp;
 			__u16 result;
 
-			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
+			if (test_bit(CONN_DEFER_SETUP, &chan->conn_state))
 				result = L2CAP_CR_SEC_BLOCK;
 			else
 				result = L2CAP_CR_BAD_PSM;
@@ -1255,8 +1253,8 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 
 			if (l2cap_chan_check_security(chan)) {
 				lock_sock(sk);
-				if (test_bit(BT_SK_DEFER_SETUP,
-					     &bt_sk(sk)->flags)) {
+				if (test_bit(CONN_DEFER_SETUP,
+					     &chan->conn_state)) {
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
 					chan->ops->defer(chan);
@@ -3642,7 +3640,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 
 	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
 		if (l2cap_chan_check_security(chan)) {
-			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
+			if (test_bit(CONN_DEFER_SETUP, &chan->conn_state)) {
 				__l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
@@ -6413,8 +6411,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 			lock_sock(sk);
 
 			if (!status) {
-				if (test_bit(BT_SK_DEFER_SETUP,
-					     &bt_sk(sk)->flags)) {
+				if (test_bit(CONN_DEFER_SETUP,
+					     &chan->conn_state)) {
 					res = L2CAP_CR_PEND;
 					stat = L2CAP_CS_AUTHOR_PEND;
 					chan->ops->defer(chan);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 8adea0f..9cd6fba 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -659,10 +659,13 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 			break;
 		}
 
-		if (opt)
+		if (opt) {
 			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
-		else
+			set_bit(CONN_DEFER_SETUP, &chan->conn_state);
+		} else {
 			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+			clear_bit(CONN_DEFER_SETUP, &chan->conn_state);
+		}
 		break;
 
 	case BT_FLUSHABLE:
-- 
1.8.1.2

Re: [PATCH 07/16] Bluetooth: duplicate DEFER_SETUP flag on l2cap_chan

[Marcel Holtmann]

Hi Gustavo,

> Now l2cap_chan also has the DEFER_SETUP flag, so we don't need to access
> the bt_sk(sk) to read this flag inside l2cap_core.c. We need to kep it
> duplicate since it is used in Bluetooth socket code as well.

can we phrase this in a way that everybody understands what it means.
Since I do not actually.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |  1 +
>  net/bluetooth/l2cap_core.c    | 16 +++++++---------
>  net/bluetooth/l2cap_sock.c    |  7 +++++--
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 4f28a8c..0c76c55 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -635,6 +635,7 @@ enum {
>  	CONN_REJ_ACT,
>  	CONN_SEND_FBIT,
>  	CONN_RNR_SENT,
> +	CONN_DEFER_SETUP,
>  };
>  
>  /* Definitions for flags in l2cap_chan */
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index fd7fe5e..2c7ec1b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -620,10 +620,8 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
>  void l2cap_chan_close(struct l2cap_chan *chan, int reason)
>  {
>  	struct l2cap_conn *conn = chan->conn;
> -	struct sock *sk = chan->sk;
>  
> -	BT_DBG("chan %p state %s sk %p", chan, state_to_string(chan->state),
> -	       sk);
> +	BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
>  
>  	switch (chan->state) {
>  	case BT_LISTEN:
> @@ -646,7 +644,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
>  			struct l2cap_conn_rsp rsp;
>  			__u16 result;
>  
> -			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
> +			if (test_bit(CONN_DEFER_SETUP, &chan->conn_state))
>  				result = L2CAP_CR_SEC_BLOCK;
>  			else
>  				result = L2CAP_CR_BAD_PSM;
> @@ -1255,8 +1253,8 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>  
>  			if (l2cap_chan_check_security(chan)) {
>  				lock_sock(sk);
> -				if (test_bit(BT_SK_DEFER_SETUP,
> -					     &bt_sk(sk)->flags)) {
> +				if (test_bit(CONN_DEFER_SETUP,
> +					     &chan->conn_state)) {
>  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
>  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
>  					chan->ops->defer(chan);
> @@ -3642,7 +3640,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>  
>  	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
>  		if (l2cap_chan_check_security(chan)) {
> -			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> +			if (test_bit(CONN_DEFER_SETUP, &chan->conn_state)) {
>  				__l2cap_state_change(chan, BT_CONNECT2);
>  				result = L2CAP_CR_PEND;
>  				status = L2CAP_CS_AUTHOR_PEND;
> @@ -6413,8 +6411,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>  			lock_sock(sk);
>  
>  			if (!status) {
> -				if (test_bit(BT_SK_DEFER_SETUP,
> -					     &bt_sk(sk)->flags)) {
> +				if (test_bit(CONN_DEFER_SETUP,
> +					     &chan->conn_state)) {
>  					res = L2CAP_CR_PEND;
>  					stat = L2CAP_CS_AUTHOR_PEND;
>  					chan->ops->defer(chan);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 8adea0f..9cd6fba 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -659,10 +659,13 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  			break;
>  		}
>  
> -		if (opt)
> +		if (opt) {
>  			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> -		else
> +			set_bit(CONN_DEFER_SETUP, &chan->conn_state);
> +		} else {
>  			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +			clear_bit(CONN_DEFER_SETUP, &chan->conn_state);
> +		}
>  		break;
>  
>  	case BT_FLUSHABLE:

Regards

Marcel

[PATCH 08/16] Bluetooth: Improving locking in l2cap_conn_start()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Since we don't read bt_sk(sk) in here anymore we can remove the lock from
the block and only use it when calling l2cap_state_change().

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2c7ec1b..979a195 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1252,7 +1252,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 			rsp.dcid = cpu_to_le16(chan->scid);
 
 			if (l2cap_chan_check_security(chan)) {
-				lock_sock(sk);
 				if (test_bit(CONN_DEFER_SETUP,
 					     &chan->conn_state)) {
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
@@ -1260,11 +1259,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 					chan->ops->defer(chan);
 
 				} else {
-					__l2cap_state_change(chan, BT_CONFIG);
+					l2cap_state_change(chan, BT_CONFIG);
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
 				}
-				release_sock(sk);
 			} else {
 				rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 				rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
@@ -6404,12 +6402,9 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 				__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
 			}
 		} else if (chan->state == BT_CONNECT2) {
-			struct sock *sk = chan->sk;
 			struct l2cap_conn_rsp rsp;
 			__u16 res, stat;
 
-			lock_sock(sk);
-
 			if (!status) {
 				if (test_bit(CONN_DEFER_SETUP,
 					     &chan->conn_state)) {
@@ -6417,19 +6412,17 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 					stat = L2CAP_CS_AUTHOR_PEND;
 					chan->ops->defer(chan);
 				} else {
-					__l2cap_state_change(chan, BT_CONFIG);
+					l2cap_state_change(chan, BT_CONFIG);
 					res = L2CAP_CR_SUCCESS;
 					stat = L2CAP_CS_NO_INFO;
 				}
 			} else {
-				__l2cap_state_change(chan, BT_DISCONN);
+				l2cap_state_change(chan, BT_DISCONN);
 				__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
 				res = L2CAP_CR_SEC_BLOCK;
 				stat = L2CAP_CS_NO_INFO;
 			}
 
-			release_sock(sk);
-
 			rsp.scid   = cpu_to_le16(chan->dcid);
 			rsp.dcid   = cpu_to_le16(chan->scid);
 			rsp.result = cpu_to_le16(res);
-- 
1.8.1.2

Re: [PATCH 08/16] Bluetooth: Improving locking in l2cap_conn_start()

[Marcel Holtmann]

Hi Gustavo,

> Since we don't read bt_sk(sk) in here anymore we can remove the lock from
> the block and only use it when calling l2cap_state_change().

you switch from __l2cap_ to l2cap_ function. You need to mention that as
well and how that all makes a difference and why it is safe.

Regards

Marcel

[PATCH 09/16] Bluetooth: lock socket in defer_cb call

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Since we removed the lock around the ops->defer() call inside
l2cap_core.c we need now to protect the defer_cb call.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_sock.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 9cd6fba..6a95d37 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1096,11 +1096,15 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
 
 static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
 {
-	struct sock *sk = chan->data;
-	struct sock *parent = bt_sk(sk)->parent;
+	struct sock *parent, *sk = chan->data;
+
+	lock_sock(sk);
 
+	parent = bt_sk(sk)->parent;
 	if (parent)
 		parent->sk_data_ready(parent, 0);
+
+	release_sock(sk);
 }
 
 static struct l2cap_ops l2cap_chan_ops = {
-- 
1.8.1.2

[PATCH 10/16] Bluetooth: Remove socket lock from state_change() in l2cap_core

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This simplifies a lot the state change handling inside l2cap_core.c,
we got rid of __l2cap_state_change() and l2cap_state_change() doesn't lock
the socket anymore, instead the socket is locked inside the ops user code
in l2cap_sock.c.

In some places we were not using the locked version, and now we are using
it. There is no side effect in locking the socket in these places.

Handle the operation of lock the socket to ops user benefit A2MP, since
there is no socket lock there it doesn't need any special function in
l2cap work without touching socket locks.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 32 +++++++-------------------------
 net/bluetooth/l2cap_sock.c |  4 ++++
 2 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 979a195..6bd02ba 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -210,7 +210,7 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn)
 	return 0;
 }
 
-static void __l2cap_state_change(struct l2cap_chan *chan, int state)
+static void l2cap_state_change(struct l2cap_chan *chan, int state)
 {
 	BT_DBG("chan %p %s -> %s", chan, state_to_string(chan->state),
 	       state_to_string(state));
@@ -219,33 +219,16 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
 	chan->ops->state_change(chan, state, 0);
 }
 
-static void l2cap_state_change(struct l2cap_chan *chan, int state)
-{
-	struct sock *sk = chan->sk;
-
-	lock_sock(sk);
-	__l2cap_state_change(chan, state);
-	release_sock(sk);
-}
-
 static void l2cap_state_change_and_error(struct l2cap_chan *chan, int state,
 					 int err)
 {
-	struct sock *sk = chan->sk;
-
-	lock_sock(sk);
 	chan->state = state;
 	chan->ops->state_change(chan, state, err);
-	release_sock(sk);
 }
 
 static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
 {
-	struct sock *sk = chan->sk;
-
-	lock_sock(sk);
 	chan->ops->state_change(chan, chan->state, err);
-	release_sock(sk);
 }
 
 static void __set_retrans_timer(struct l2cap_chan *chan)
@@ -1219,7 +1202,6 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 	mutex_lock(&conn->chan_lock);
 
 	list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
-		struct sock *sk = chan->sk;
 
 		l2cap_chan_lock(chan);
 
@@ -3639,7 +3621,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
 		if (l2cap_chan_check_security(chan)) {
 			if (test_bit(CONN_DEFER_SETUP, &chan->conn_state)) {
-				__l2cap_state_change(chan, BT_CONNECT2);
+				l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
 				chan->ops->defer(chan);
@@ -3649,21 +3631,21 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 				 * physical link is up.
 				 */
 				if (amp_id) {
-					__l2cap_state_change(chan, BT_CONNECT2);
+					l2cap_state_change(chan, BT_CONNECT2);
 					result = L2CAP_CR_PEND;
 				} else {
-					__l2cap_state_change(chan, BT_CONFIG);
+					l2cap_state_change(chan, BT_CONFIG);
 					result = L2CAP_CR_SUCCESS;
 				}
 				status = L2CAP_CS_NO_INFO;
 			}
 		} else {
-			__l2cap_state_change(chan, BT_CONNECT2);
+			l2cap_state_change(chan, BT_CONNECT2);
 			result = L2CAP_CR_PEND;
 			status = L2CAP_CS_AUTHEN_PEND;
 		}
 	} else {
-		__l2cap_state_change(chan, BT_CONNECT2);
+		l2cap_state_change(chan, BT_CONNECT2);
 		result = L2CAP_CR_PEND;
 		status = L2CAP_CS_NO_INFO;
 	}
@@ -4553,7 +4535,7 @@ static void l2cap_do_create(struct l2cap_chan *chan, int result,
 			       sizeof(rsp), &rsp);
 
 		if (result == L2CAP_CR_SUCCESS) {
-			__l2cap_state_change(chan, BT_CONFIG);
+			l2cap_state_change(chan, BT_CONFIG);
 			set_bit(CONF_REQ_SENT, &chan->conf_state);
 			l2cap_send_cmd(chan->conn, l2cap_get_ident(chan->conn),
 				       L2CAP_CONF_REQ,
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 6a95d37..cf8f187 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1052,10 +1052,14 @@ static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
 {
 	struct sock *sk = chan->data;
 
+	lock_sock(sk);
+
 	sk->sk_state = state;
 
 	if (err)
 		sk->sk_err = err;
+
+	release_sock(sk);
 }
 
 static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
-- 
1.8.1.2

Re: [PATCH 10/16] Bluetooth: Remove socket lock from state_change() in l2cap_core

[Marcel Holtmann]

Hi Gustavo,

> This simplifies a lot the state change handling inside l2cap_core.c,
> we got rid of __l2cap_state_change() and l2cap_state_change() doesn't lock
> the socket anymore, instead the socket is locked inside the ops user code
> in l2cap_sock.c.
> 
> In some places we were not using the locked version, and now we are using
> it. There is no side effect in locking the socket in these places.
> 
> Handle the operation of lock the socket to ops user benefit A2MP, since
> there is no socket lock there it doesn't need any special function in
> l2cap work without touching socket locks.

I am lost now. You are changing locking around from a to b to c. A
function previously using locking is now no longer using it. This is
confusing. Why are we doing this again.

Regards

Marcel

[PATCH 11/16] Bluetooth: remove parent socket usage from l2cap_core.c

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Since we do not touch the parent sock in l2cap_core.c anymore we don't
need to lock it there anymore. That lock was replaced by the
l2cap_chan_lock and inside the new_connection() call for l2cap_sock.c the
parent lock is locked, so the operations that uses it can be performed
safely.

The l2cap_chan_lock give us the needed protection to handle the incoming
connections.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 16 ++++------------
 net/bluetooth/l2cap_sock.c |  4 ++++
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6bd02ba..8bc1708 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1315,7 +1315,6 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 
 static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
-	struct sock *parent;
 	struct l2cap_chan *chan, *pchan;
 
 	BT_DBG("");
@@ -1326,9 +1325,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	if (!pchan)
 		return;
 
-	parent = pchan->sk;
-
-	lock_sock(parent);
+	l2cap_chan_lock(pchan);
 
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
@@ -1345,7 +1342,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	l2cap_chan_ready(chan);
 
 clean:
-	release_sock(parent);
+	l2cap_chan_unlock(pchan);
 }
 
 static void l2cap_conn_ready(struct l2cap_conn *conn)
@@ -3562,7 +3559,6 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
 	struct l2cap_conn_rsp rsp;
 	struct l2cap_chan *chan = NULL, *pchan;
-	struct sock *parent, *sk = NULL;
 	int result, status = L2CAP_CS_NO_INFO;
 
 	u16 dcid = 0, scid = __le16_to_cpu(req->scid);
@@ -3577,10 +3573,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 		goto sendresp;
 	}
 
-	parent = pchan->sk;
-
 	mutex_lock(&conn->chan_lock);
-	lock_sock(parent);
+	l2cap_chan_lock(pchan);
 
 	/* Check if the ACL is secure enough (if not SDP) */
 	if (psm != __constant_cpu_to_le16(L2CAP_PSM_SDP) &&
@@ -3600,8 +3594,6 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	if (!chan)
 		goto response;
 
-	sk = chan->sk;
-
 	hci_conn_hold(conn->hcon);
 
 	bacpy(&chan->src, conn->src);
@@ -3651,7 +3643,7 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	}
 
 response:
-	release_sock(parent);
+	l2cap_chan_unlock(pchan);
 	mutex_unlock(&conn->chan_lock);
 
 sendresp:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index cf8f187..3d76de8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -943,6 +943,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk, *parent = chan->data;
 
+	lock_sock(parent);
+
 	/* Check for backlog size */
 	if (sk_acceptq_is_full(parent)) {
 		BT_DBG("backlog full %d", parent->sk_ack_backlog);
@@ -960,6 +962,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 
 	bt_accept_enqueue(parent, sk);
 
+	release_sock(parent);
+
 	return l2cap_pi(sk)->chan;
 }
 
-- 
1.8.1.2

Re: [PATCH 11/16] Bluetooth: remove parent socket usage from l2cap_core.c

[Marcel Holtmann]

Hi Gustavo,

> Since we do not touch the parent sock in l2cap_core.c anymore we don't
> need to lock it there anymore. That lock was replaced by the
> l2cap_chan_lock and inside the new_connection() call for l2cap_sock.c the
> parent lock is locked, so the operations that uses it can be performed
> safely.
> 
> The l2cap_chan_lock give us the needed protection to handle the incoming
> connections.

and what about the newly added lock_sock(parent).

Regards

Marcel

[PATCH 12/16] Bluetooth: Use abstract chan->data in comparison

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

If the L2CAP user is l2cap_sock.c chan->data is a pointer to the l2cap
socket so chan->sk and chan->data are the same thing. Then we can just
compare with chan->data instead.

Non-socket users will have skb->sk = NULL, thus this change does not
interfere in other users.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h | 2 +-
 net/bluetooth/l2cap_core.c    | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0c76c55..6986140 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -536,7 +536,7 @@ struct l2cap_chan {
 	struct list_head	list;
 	struct list_head	global_l;
 
-	void			*data;
+	void			*data; /* l2cap user data. eg: sk for sockets */
 	struct l2cap_ops	*ops;
 	struct mutex		lock;
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8bc1708..98e4cda 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2677,12 +2677,11 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
 	mutex_lock(&conn->chan_lock);
 
 	list_for_each_entry(chan, &conn->chan_l, list) {
-		struct sock *sk = chan->sk;
 		if (chan->chan_type != L2CAP_CHAN_RAW)
 			continue;
 
 		/* Don't send frame to the socket it came from */
-		if (skb->sk == sk)
+		if (skb->sk && skb->sk == chan->data)
 			continue;
 		nskb = skb_clone(skb, GFP_KERNEL);
 		if (!nskb)
-- 
1.8.1.2

[PATCH 13/16] Bluetooth: Move l2cap_wait_ack() to l2cap_sock.c

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The wait_ack code has a heavy dependency on the socket data structures
and, as of now, it won't be worthless change it to use non-socket
structures as the only user of such feature is a socket.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |  3 ++-
 net/bluetooth/l2cap_core.c    | 32 --------------------------------
 net/bluetooth/l2cap_sock.c    | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 6986140..574b3ae 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -757,6 +757,8 @@ static inline bool l2cap_clear_timer(struct l2cap_chan *chan,
 		msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
 #define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)
 
+#define __missing_ack(c) (c->unacked_frames > 0 && c->conn)
+
 static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
 {
 	if (seq1 >= seq2)
@@ -793,7 +795,6 @@ int l2cap_init_sockets(void);
 void l2cap_cleanup_sockets(void);
 
 void __l2cap_connect_rsp_defer(struct l2cap_chan *chan);
-int __l2cap_wait_ack(struct sock *sk);
 
 int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm);
 int l2cap_add_scid(struct l2cap_chan *chan,  __u16 scid);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 98e4cda..2a099b7 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1706,38 +1706,6 @@ done:
 	return err;
 }
 
-int __l2cap_wait_ack(struct sock *sk)
-{
-	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
-	DECLARE_WAITQUEUE(wait, current);
-	int err = 0;
-	int timeo = HZ/5;
-
-	add_wait_queue(sk_sleep(sk), &wait);
-	set_current_state(TASK_INTERRUPTIBLE);
-	while (chan->unacked_frames > 0 && chan->conn) {
-		if (!timeo)
-			timeo = HZ/5;
-
-		if (signal_pending(current)) {
-			err = sock_intr_errno(timeo);
-			break;
-		}
-
-		release_sock(sk);
-		timeo = schedule_timeout(timeo);
-		lock_sock(sk);
-		set_current_state(TASK_INTERRUPTIBLE);
-
-		err = sock_error(sk);
-		if (err)
-			break;
-	}
-	set_current_state(TASK_RUNNING);
-	remove_wait_queue(sk_sleep(sk), &wait);
-	return err;
-}
-
 static void l2cap_monitor_timeout(struct work_struct *work)
 {
 	struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 3d76de8..58b4a5d 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -853,6 +853,38 @@ static void l2cap_sock_kill(struct sock *sk)
 	sock_put(sk);
 }
 
+static int __l2cap_wait_ack(struct sock *sk)
+{
+	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+	DECLARE_WAITQUEUE(wait, current);
+	int err = 0;
+	int timeo = HZ/5;
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	set_current_state(TASK_INTERRUPTIBLE);
+	while (__missing_ack(chan)) {
+		if (!timeo)
+			timeo = HZ/5;
+
+		if (signal_pending(current)) {
+			err = sock_intr_errno(timeo);
+			break;
+		}
+
+		release_sock(sk);
+		timeo = schedule_timeout(timeo);
+		lock_sock(sk);
+		set_current_state(TASK_INTERRUPTIBLE);
+
+		err = sock_error(sk);
+		if (err)
+			break;
+	}
+	set_current_state(TASK_RUNNING);
+	remove_wait_queue(sk_sleep(sk), &wait);
+	return err;
+}
+
 static int l2cap_sock_shutdown(struct socket *sock, int how)
 {
 	struct sock *sk = sock->sk;
-- 
1.8.1.2

[PATCH 14/16] Bluetooth: Create l2cap->ops->resume()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

resume() will isolate the code the code to get a socket back from the
suspended state when a security elevation happens.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |  5 +++++
 net/bluetooth/a2mp.c          |  1 +
 net/bluetooth/l2cap_core.c    |  6 +-----
 net/bluetooth/l2cap_sock.c    | 13 +++++++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 574b3ae..cab7773 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -555,6 +555,7 @@ struct l2cap_ops {
 	void			(*defer) (struct l2cap_chan *chan);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
+	void			(*resume) (struct l2cap_chan *chan);
 };
 
 struct l2cap_conn {
@@ -789,6 +790,10 @@ static inline void l2cap_chan_no_defer(struct l2cap_chan *chan)
 {
 }
 
+static inline void l2cap_chan_no_resume(struct l2cap_chan *chan)
+{
+}
+
 extern bool disable_ertm;
 
 int l2cap_init_sockets(void);
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index ad6e42f..4a542b3 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -708,6 +708,7 @@ static struct l2cap_ops a2mp_chan_ops = {
 	.teardown = l2cap_chan_no_teardown,
 	.ready = l2cap_chan_no_ready,
 	.defer = l2cap_chan_no_defer,
+	.resume = l2cap_chan_no_resume,
 };
 
 static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2a099b7..201041e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6326,11 +6326,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 
 		if (!status && (chan->state == BT_CONNECTED ||
 				chan->state == BT_CONFIG)) {
-			struct sock *sk = chan->sk;
-
-			clear_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags);
-			sk->sk_state_change(sk);
-
+			chan->ops->resume(chan);
 			l2cap_check_encryption(chan, encrypt);
 			l2cap_chan_unlock(chan);
 			continue;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 58b4a5d..e7b3291 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1147,6 +1147,18 @@ static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
 	release_sock(sk);
 }
 
+static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
+{
+	struct sock *sk = chan->data;
+
+	lock_sock(sk);
+
+	clear_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags);
+	sk->sk_state_change(sk);
+
+	release_sock(sk);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1157,6 +1169,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.ready		= l2cap_sock_ready_cb,
 	.defer		= l2cap_sock_defer_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
+	.resume		= l2cap_sock_resume_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.8.1.2

[PATCH 15/16] Bluetooth: Create l2cap->ops->set_shutdown()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Isolates the code that sets the socket shutdown mask. This is the last
commit to remove the socket usage from l2cap_core.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |  5 +++++
 net/bluetooth/a2mp.c          |  1 +
 net/bluetooth/l2cap_core.c    |  7 +------
 net/bluetooth/l2cap_sock.c    | 10 ++++++++++
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index cab7773..d3c8f57 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -556,6 +556,7 @@ struct l2cap_ops {
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
 	void			(*resume) (struct l2cap_chan *chan);
+	void			(*set_shutdown) (struct l2cap_chan *chan);
 };
 
 struct l2cap_conn {
@@ -794,6 +795,10 @@ static inline void l2cap_chan_no_resume(struct l2cap_chan *chan)
 {
 }
 
+static inline void l2cap_chan_no_set_shutdown(struct l2cap_chan *chan)
+{
+}
+
 extern bool disable_ertm;
 
 int l2cap_init_sockets(void);
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 4a542b3..0a2ec84 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -709,6 +709,7 @@ static struct l2cap_ops a2mp_chan_ops = {
 	.ready = l2cap_chan_no_ready,
 	.defer = l2cap_chan_no_defer,
 	.resume = l2cap_chan_no_resume,
+	.set_shutdown = l2cap_chan_no_set_shutdown,
 };
 
 static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 201041e..d71ff07 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3984,7 +3984,6 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
 	struct l2cap_disconn_rsp rsp;
 	u16 dcid, scid;
 	struct l2cap_chan *chan;
-	struct sock *sk;
 
 	scid = __le16_to_cpu(req->scid);
 	dcid = __le16_to_cpu(req->dcid);
@@ -4001,15 +4000,11 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
 
 	l2cap_chan_lock(chan);
 
-	sk = chan->sk;
-
 	rsp.dcid = cpu_to_le16(chan->scid);
 	rsp.scid = cpu_to_le16(chan->dcid);
 	l2cap_send_cmd(conn, cmd->ident, L2CAP_DISCONN_RSP, sizeof(rsp), &rsp);
 
-	lock_sock(sk);
-	sk->sk_shutdown = SHUTDOWN_MASK;
-	release_sock(sk);
+	chan->ops->set_shutdown(chan);
 
 	l2cap_chan_hold(chan);
 	l2cap_chan_del(chan, ECONNRESET);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index e7b3291..07ef90c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1159,6 +1159,15 @@ static void l2cap_sock_resume_cb(struct l2cap_chan *chan)
 	release_sock(sk);
 }
 
+static void l2cap_sock_set_shutdown_cb(struct l2cap_chan *chan)
+{
+	struct sock *sk = chan->data;
+
+	lock_sock(sk);
+	sk->sk_shutdown = SHUTDOWN_MASK;
+	release_sock(sk);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1170,6 +1179,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.defer		= l2cap_sock_defer_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 	.resume		= l2cap_sock_resume_cb,
+	.set_shutdown	= l2cap_sock_set_shutdown_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.8.1.2

[PATCH 16/16] Bluetooth: Remove sk member from struct l2cap_chan

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Now that the removal of socket usage from l2cap_core.c is done we can
remove sk from struct l2cap_chan since we do not use it anywhere anymore.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h | 2 --
 net/bluetooth/l2cap_sock.c    | 5 ++---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d3c8f57..e444d55 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -433,8 +433,6 @@ struct l2cap_seq_list {
 #define L2CAP_SEQ_LIST_TAIL	0x8000
 
 struct l2cap_chan {
-	struct sock *sk;
-
 	bdaddr_t	src;
 	bdaddr_t	dst;
 
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 07ef90c..910c9d7 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1101,11 +1101,12 @@ static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
 static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
 					       unsigned long len, int nb)
 {
+	struct sock *sk = chan->data;
 	struct sk_buff *skb;
 	int err;
 
 	l2cap_chan_unlock(chan);
-	skb = bt_skb_send_alloc(chan->sk, len, nb, &err);
+	skb = bt_skb_send_alloc(sk, len, nb, &err);
 	l2cap_chan_lock(chan);
 
 	if (!skb)
@@ -1293,8 +1294,6 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
 
 	l2cap_chan_hold(chan);
 
-	chan->sk = sk;
-
 	l2cap_pi(sk)->chan = chan;
 
 	return sk;
-- 
1.8.1.2

[PATCH 00/16] Completely remove socket dependency from l2cap_core.c

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This is the last series of patches to completely remove the l2cap socket usage
in the l2cap_core.c file. We now have a C API to access L2CAP and as a next
step we should convert RFCOMM to use this API.

Gustavo Padovan (16):
  Bluetooth: Add src and dst info to struct l2cap_chan
  Bluetooth: Remove sk_sndtimeo from l2cap_core.c
  Bluetooth: extend state_change() call to report errors too
  Bluetooth: add l2cap_state_change_and_error()
  Bluetooth: Add missing braces to an "else if"
  Bluetooth: use l2cap_chan_ready() instead of duplicate code
  Bluetooth: duplicate DEFER_SETUP flag on l2cap_chan
  Bluetooth: Improving locking in l2cap_conn_start()
  Bluetooth: lock socket in defer_cb call
  Bluetooth: Remove socket lock from state_change() in l2cap_core
  Bluetooth: remove parent socket usage from l2cap_core.c
  Bluetooth: Use abstract chan->data in comparison
  Bluetooth: Move l2cap_wait_ack() to l2cap_sock.c
  Bluetooth: Create l2cap->ops->resume()
  Bluetooth: Create l2cap->ops->set_shutdown()
  Bluetooth: Remove sk member from struct l2cap_chan

 include/net/bluetooth/l2cap.h |  22 ++++-
 net/bluetooth/a2mp.c          |   4 +-
 net/bluetooth/l2cap_core.c    | 200 ++++++++++++------------------------------
 net/bluetooth/l2cap_sock.c    |  96 +++++++++++++++++---
 net/bluetooth/rfcomm/core.c   |  15 ++--
 5 files changed, 169 insertions(+), 168 deletions(-)

-- 
1.8.0.2

[PATCH 05/16] Bluetooth: Add missing braces to an "else if"

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Trivial change in the coding style.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index cdd7f44..2ad952b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1406,8 +1406,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 			sk->sk_state_change(sk);
 			release_sock(sk);
 
-		} else if (chan->state == BT_CONNECT)
+		} else if (chan->state == BT_CONNECT) {
 			l2cap_do_start(chan);
+		}
 
 		l2cap_chan_unlock(chan);
 	}
-- 
1.8.0.2