[PATCH -v2 00/12] Another step in l2cap_core/sock separation

[PATCH -v2 00/12] Another step in l2cap_core/sock separation

[Gustavo Padovan]

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

This is just a rebase on top of lastest bluetooth-next tree, as the
__constant_cpu_to_le patch from Andrei broke this patch series.

Gustavo Padovan (12):
  Bluetooth: Remove extra l2cap_state_change(BT_CONNECTED)
  Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c
  Bluetooth: Add l2cap_chan->ops->ready()
  Bluetooth: Use l2cap_chan_ready() in LE path
  Bluetooth: Use chan->state instead of sk->sk_state
  Bluetooth: Move check for backlog size to l2cap_sock.c
  Bluetooth: Create DEFER_SETUP flag in conf_state
  Bluetooth: Add chan->ops->defer()
  Bluetooth: check for already existent channel before create new one
  Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c
  Bluetooth: Remove parent socket usage from l2cap_core.c
  Bluetooth: Used void * as parameter in alloc_skb()

 include/net/bluetooth/l2cap.h |    8 ++-
 net/bluetooth/l2cap_core.c    |  152 ++++++++++-------------------------------
 net/bluetooth/l2cap_sock.c    |  129 ++++++++++++++++++++++++++++++++--
 3 files changed, 164 insertions(+), 125 deletions(-)

-- 
1.7.10.1

[PATCH -v2 01/12] Bluetooth: Remove extra l2cap_state_change(BT_CONNECTED)

[Gustavo Padovan]

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

This is already performed inside l2cap_chan_ready(), so we don't need it
here again.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/l2cap_core.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6eb2ef2..339b388 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3645,8 +3645,6 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 	if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
 		set_default_fcs(chan);
 
-		l2cap_state_change(chan, BT_CONNECTED);
-
 		if (chan->mode == L2CAP_MODE_ERTM ||
 		    chan->mode == L2CAP_MODE_STREAMING)
 			err = l2cap_ertm_init(chan);
@@ -3777,7 +3775,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 	if (test_bit(CONF_OUTPUT_DONE, &chan->conf_state)) {
 		set_default_fcs(chan);
 
-		l2cap_state_change(chan, BT_CONNECTED);
 		if (chan->mode == L2CAP_MODE_ERTM ||
 		    chan->mode == L2CAP_MODE_STREAMING)
 			err = l2cap_ertm_init(chan);
-- 
1.7.10.1

Re: [PATCH -v2 01/12] Bluetooth: Remove extra l2cap_state_change(BT_CONNECTED)

[Marcel Holtmann]

Hi Gustavo,

> This is already performed inside l2cap_chan_ready(), so we don't need it
> here again.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/l2cap_core.c |    3 ---
>  1 file changed, 3 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH -v2 02/12] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c

[Gustavo Padovan]

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

This remove a bit more of socket code from l2cap core, this calls set the
SOCK_ZAPPED and do some clean up depending on the socket state.

Reported-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   55 ++++++-----------------------------
 net/bluetooth/l2cap_sock.c    |   63 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f44344b..184e542 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -530,6 +530,7 @@ struct l2cap_ops {
 	struct l2cap_chan	*(*new_connection) (void *data);
 	int			(*recv) (void *data, struct sk_buff *skb);
 	void			(*close) (void *data);
+	void			(*teardown) (void *data, int err);
 	void			(*state_change) (void *data, int state);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 339b388..21cb761 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -493,9 +493,7 @@ static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 
 static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 {
-	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
-	struct sock *parent = bt_sk(sk)->parent;
 
 	__clear_chan_timer(chan);
 
@@ -511,21 +509,8 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 		hci_conn_put(conn->hcon);
 	}
 
-	lock_sock(sk);
-
-	__l2cap_state_change(chan, BT_CLOSED);
-	sock_set_flag(sk, SOCK_ZAPPED);
-
-	if (err)
-		__l2cap_chan_set_err(chan, err);
-
-	if (parent) {
-		bt_accept_unlink(sk);
-		parent->sk_data_ready(parent, 0);
-	} else
-		sk->sk_state_change(sk);
-
-	release_sock(sk);
+	if (chan->ops->teardown)
+		chan->ops->teardown(chan->data, err);
 
 	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
 		return;
@@ -554,25 +539,6 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	return;
 }
 
-static void l2cap_chan_cleanup_listen(struct sock *parent)
-{
-	struct sock *sk;
-
-	BT_DBG("parent %p", parent);
-
-	/* Close not yet accepted channels */
-	while ((sk = bt_accept_dequeue(parent, NULL))) {
-		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
-
-		l2cap_chan_lock(chan);
-		__clear_chan_timer(chan);
-		l2cap_chan_close(chan, ECONNRESET);
-		l2cap_chan_unlock(chan);
-
-		chan->ops->close(chan->data);
-	}
-}
-
 void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 {
 	struct l2cap_conn *conn = chan->conn;
@@ -583,12 +549,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 
 	switch (chan->state) {
 	case BT_LISTEN:
-		lock_sock(sk);
-		l2cap_chan_cleanup_listen(sk);
-
-		__l2cap_state_change(chan, BT_CLOSED);
-		sock_set_flag(sk, SOCK_ZAPPED);
-		release_sock(sk);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan->data, 0);
 		break;
 
 	case BT_CONNECTED:
@@ -630,9 +592,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 		break;
 
 	default:
-		lock_sock(sk);
-		sock_set_flag(sk, SOCK_ZAPPED);
-		release_sock(sk);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan->data, 0);
 		break;
 	}
 }
@@ -3416,7 +3377,9 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	/* Check if we already have channel with that dcid */
 	if (__l2cap_get_chan_by_dcid(conn, scid)) {
-		sock_set_flag(sk, SOCK_ZAPPED);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan->data, 0);
+
 		chan->ops->close(chan->data);
 		goto response;
 	}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index d244361..b4de4cb 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -872,6 +872,25 @@ static int l2cap_sock_release(struct socket *sock)
 	return err;
 }
 
+static void l2cap_sock_cleanup_listen(struct sock *parent)
+{
+	struct sock *sk;
+
+	BT_DBG("parent %p", parent);
+
+	/* Close not yet accepted channels */
+	while ((sk = bt_accept_dequeue(parent, NULL))) {
+		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+
+		l2cap_chan_lock(chan);
+		__clear_chan_timer(chan);
+		l2cap_chan_close(chan, ECONNRESET);
+		l2cap_chan_unlock(chan);
+
+		l2cap_sock_kill(sk);
+	}
+}
+
 static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 {
 	struct sock *sk, *parent = data;
@@ -931,6 +950,49 @@ static void l2cap_sock_close_cb(void *data)
 	l2cap_sock_kill(sk);
 }
 
+static void l2cap_sock_teardown_cb(void *data, int err)
+{
+	struct sock *sk = data;
+	struct sock *parent;
+	struct l2cap_chan *chan;
+
+	lock_sock(sk);
+
+	parent = bt_sk(sk)->parent;
+	chan = l2cap_pi(sk)->chan;
+
+	sock_set_flag(sk, SOCK_ZAPPED);
+
+	switch (chan->state) {
+	case BT_OPEN:
+	case BT_BOUND:
+	case BT_CLOSED:
+		break;
+	case BT_LISTEN:
+		l2cap_sock_cleanup_listen(sk);
+		sk->sk_state = BT_CLOSED;
+		chan->state = BT_CLOSED;
+
+		break;
+	default:
+		sk->sk_state = BT_CLOSED;
+		chan->state = BT_CLOSED;
+
+		sk->sk_err = err;
+
+		if (parent) {
+			bt_accept_unlink(sk);
+			parent->sk_data_ready(parent, 0);
+		} else {
+			sk->sk_state_change(sk);
+		}
+
+		break;
+	}
+
+	release_sock(sk);
+}
+
 static void l2cap_sock_state_change_cb(void *data, int state)
 {
 	struct sock *sk = data;
@@ -959,6 +1021,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.new_connection	= l2cap_sock_new_connection_cb,
 	.recv		= l2cap_sock_recv_cb,
 	.close		= l2cap_sock_close_cb,
+	.teardown	= l2cap_sock_teardown_cb,
 	.state_change	= l2cap_sock_state_change_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 };
-- 
1.7.10.1

Re: [PATCH -v2 02/12] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c

[Marcel Holtmann]

Hi Gustavo,

> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> This remove a bit more of socket code from l2cap core, this calls set the
> SOCK_ZAPPED and do some clean up depending on the socket state.
> 
> Reported-by: Mat Martineau <mathewm@codeaurora.org>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

the signed-off-by line indicates that this patch originally was written
by Andrei. So I prefer he gets the authorship then. The From: line at
the top should say his name.

Regards

Marcel

Re: [PATCH -v2 02/12] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c

[Andrei Emeltchenko]

Hi Marcel and Gustavo,

On Sun, May 27, 2012 at 07:00:28AM +0200, Marcel Holtmann wrote:
> Hi Gustavo,
> 
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > This remove a bit more of socket code from l2cap core, this calls set the
> > SOCK_ZAPPED and do some clean up depending on the socket state.
> > 
> > Reported-by: Mat Martineau <mathewm@codeaurora.org>
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> the signed-off-by line indicates that this patch originally was written
> by Andrei. So I prefer he gets the authorship then. The From: line at
> the top should say his name.

This patch was so heavily reworked by Gustavo that he could remove my
Signed-off-by line.

Best regards 
Andrei Emeltchenko 

[PATCH -v2 03/12] Bluetooth: Add l2cap_chan->ops->ready()

[Gustavo Padovan]

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

This move socket specific code to l2cap_sock.c.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   18 +++---------------
 net/bluetooth/l2cap_sock.c    |   21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 184e542..3b05f3e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -534,6 +534,7 @@ struct l2cap_ops {
 	void			(*state_change) (void *data, int state);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
+	void			(*ready) (void *data);
 };
 
 struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 21cb761..ef84aac 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -931,26 +931,14 @@ static void l2cap_send_conn_req(struct l2cap_chan *chan)
 
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
-	struct sock *sk = chan->sk;
-	struct sock *parent;
-
-	lock_sock(sk);
-
-	parent = bt_sk(sk)->parent;
-
-	BT_DBG("sk %p, parent %p", sk, parent);
-
 	/* This clears all conf flags, including CONF_NOT_COMPLETE */
 	chan->conf_state = 0;
 	__clear_chan_timer(chan);
 
-	__l2cap_state_change(chan, BT_CONNECTED);
-	sk->sk_state_change(sk);
-
-	if (parent)
-		parent->sk_data_ready(parent, 0);
+	chan->state = BT_CONNECTED;
 
-	release_sock(sk);
+	if (chan->ops->ready)
+		chan->ops->ready(chan->data);
 }
 
 static void l2cap_do_start(struct l2cap_chan *chan)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index b4de4cb..1019c2e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1016,6 +1016,26 @@ static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
 	return skb;
 }
 
+static void l2cap_sock_ready_cb(void *data)
+{
+	struct sock *sk = data;
+	struct sock *parent;
+
+	lock_sock(sk);
+
+	parent = bt_sk(sk)->parent;
+
+	BT_DBG("sk %p, parent %p", sk, parent);
+
+	sk->sk_state = BT_CONNECTED;
+	sk->sk_state_change(sk);
+
+	if (parent)
+		parent->sk_data_ready(parent, 0);
+
+	release_sock(sk);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1024,6 +1044,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.teardown	= l2cap_sock_teardown_cb,
 	.state_change	= l2cap_sock_state_change_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
+	.ready		= l2cap_sock_ready_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.7.10.1

Re: [PATCH -v2 03/12] Bluetooth: Add l2cap_chan->ops->ready()

[Marcel Holtmann]

Hi Gustavo,

> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> This move socket specific code to l2cap_sock.c.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

same here. If this is Andrei's patch, then he deserves the author line.

Regards

Marcel

[PATCH -v2 04/12] Bluetooth: Use l2cap_chan_ready() in LE path

[Gustavo Padovan]

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

This replace code in l2cap_le_conn_ready() by a similar code in
l2cap_chan_ready().

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ef84aac..3d35210 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1177,10 +1177,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 	l2cap_chan_add(conn, chan);
 
-	__set_chan_timer(chan, sk->sk_sndtimeo);
-
-	__l2cap_state_change(chan, BT_CONNECTED);
-	parent->sk_data_ready(parent, 0);
+	l2cap_chan_ready(chan);
 
 clean:
 	release_sock(parent);
-- 
1.7.10.1

Re: [PATCH -v2 04/12] Bluetooth: Use l2cap_chan_ready() in LE path

[Marcel Holtmann]

Hi Gustavo,

> This replace code in l2cap_le_conn_ready() by a similar code in
> l2cap_chan_ready().
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  net/bluetooth/l2cap_core.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH -v2 05/12] Bluetooth: Use chan->state instead of sk->sk_state

[Gustavo Padovan]

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

These vars are kept in sync so we can use chan->state here.

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3d35210..53e21ca 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1444,7 +1444,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 
 	lock_sock(sk);
 
-	switch (sk->sk_state) {
+	switch (chan->state) {
 	case BT_CONNECT:
 	case BT_CONNECT2:
 	case BT_CONFIG:
-- 
1.7.10.1

Re: [PATCH -v2 05/12] Bluetooth: Use chan->state instead of sk->sk_state

[Marcel Holtmann]

Hi Gustavo,

> These vars are kept in sync so we can use chan->state here.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  net/bluetooth/l2cap_core.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 3d35210..53e21ca 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1444,7 +1444,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>  
>  	lock_sock(sk);
>  
> -	switch (sk->sk_state) {
> +	switch (chan->state) {
>  	case BT_CONNECT:
>  	case BT_CONNECT2:
>  	case BT_CONFIG:

if this is so, then why are we bothering with the socket lock here?

Regards

Marcel

[PATCH -v2 06/12] Bluetooth: Move check for backlog size to l2cap_sock.c

[Gustavo Padovan]

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

Remove socket specific code from l2cap_core.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/l2cap_core.c |   12 ------------
 net/bluetooth/l2cap_sock.c |    6 ++++++
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 53e21ca..01171b1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1156,12 +1156,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 	lock_sock(parent);
 
-	/* Check for backlog size */
-	if (sk_acceptq_is_full(parent)) {
-		BT_DBG("backlog full %d", parent->sk_ack_backlog);
-		goto clean;
-	}
-
 	chan = pchan->ops->new_connection(pchan->data);
 	if (!chan)
 		goto clean;
@@ -3348,12 +3342,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	result = L2CAP_CR_NO_MEM;
 
-	/* Check for backlog size */
-	if (sk_acceptq_is_full(parent)) {
-		BT_DBG("backlog full %d", parent->sk_ack_backlog);
-		goto response;
-	}
-
 	chan = pchan->ops->new_connection(pchan->data);
 	if (!chan)
 		goto response;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1019c2e..d2d91b3 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -895,6 +895,12 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 {
 	struct sock *sk, *parent = data;
 
+	/* Check for backlog size */
+	if (sk_acceptq_is_full(parent)) {
+		BT_DBG("backlog full %d", parent->sk_ack_backlog);
+		return NULL;
+	}
+
 	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
 								GFP_ATOMIC);
 	if (!sk)
-- 
1.7.10.1

[PATCH -v2 07/12] Bluetooth: Create DEFER_SETUP flag in conf_state

[Gustavo Padovan]

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

Remove another socket usage from l2cap_core.c

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 3b05f3e..c55b28f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -599,6 +599,7 @@ enum {
 	CONF_LOC_CONF_PEND,
 	CONF_REM_CONF_PEND,
 	CONF_NOT_COMPLETE,
+	CONF_DEFER_SETUP,
 };
 
 #define L2CAP_CONF_MAX_CONF_REQ 2
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 01171b1..388bc70 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -569,7 +569,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(CONF_DEFER_SETUP, &chan->conf_state))
 				result = L2CAP_CR_SEC_BLOCK;
 			else
 				result = L2CAP_CR_BAD_PSM;
@@ -1056,8 +1056,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(CONF_DEFER_SETUP,
+					     &chan->conf_state)) {
 					struct sock *parent = bt_sk(sk)->parent;
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
@@ -3376,7 +3376,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	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(CONF_DEFER_SETUP, &chan->conf_state)) {
 				__l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
@@ -5406,8 +5406,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(CONF_DEFER_SETUP,
+					     &chan->conf_state)) {
 					struct sock *parent = bt_sk(sk)->parent;
 					res = L2CAP_CR_PEND;
 					stat = L2CAP_CS_AUTHOR_PEND;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index d2d91b3..51d05a7 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 			break;
 		}
 
-		if (opt)
+		if (opt) {
 			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
-		else
+			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
+		} else {
 			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
+		}
+
 		break;
 
 	case BT_FLUSHABLE:
-- 
1.7.10.1

Re: [PATCH -v2 07/12] Bluetooth: Create DEFER_SETUP flag in conf_state

[Marcel Holtmann]

Hi Gustavo,

> Remove another socket usage from l2cap_core.c
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |    1 +
>  net/bluetooth/l2cap_core.c    |   12 ++++++------
>  net/bluetooth/l2cap_sock.c    |    8 ++++++--
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 3b05f3e..c55b28f 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -599,6 +599,7 @@ enum {
>  	CONF_LOC_CONF_PEND,
>  	CONF_REM_CONF_PEND,
>  	CONF_NOT_COMPLETE,
> +	CONF_DEFER_SETUP,
>  };
>  
>  #define L2CAP_CONF_MAX_CONF_REQ 2
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 01171b1..388bc70 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -569,7 +569,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(CONF_DEFER_SETUP, &chan->conf_state))
>  				result = L2CAP_CR_SEC_BLOCK;
>  			else
>  				result = L2CAP_CR_BAD_PSM;
> @@ -1056,8 +1056,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(CONF_DEFER_SETUP,
> +					     &chan->conf_state)) {
>  					struct sock *parent = bt_sk(sk)->parent;
>  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
>  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> @@ -3376,7 +3376,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
>  
>  	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(CONF_DEFER_SETUP, &chan->conf_state)) {
>  				__l2cap_state_change(chan, BT_CONNECT2);
>  				result = L2CAP_CR_PEND;
>  				status = L2CAP_CS_AUTHOR_PEND;
> @@ -5406,8 +5406,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(CONF_DEFER_SETUP,
> +					     &chan->conf_state)) {
>  					struct sock *parent = bt_sk(sk)->parent;
>  					res = L2CAP_CR_PEND;
>  					stat = L2CAP_CS_AUTHOR_PEND;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index d2d91b3..51d05a7 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
>  			break;
>  		}
>  
> -		if (opt)
> +		if (opt) {
>  			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> -		else
> +			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
> +		} else {
>  			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> +			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
> +		}
> +
>  		break;

and what about the lines above testing this together with BT_SECURITY?

Regards

Marcel

Re: [PATCH -v2 07/12] Bluetooth: Create DEFER_SETUP flag in conf_state

[Gustavo Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2012-05-27 07:07:15 +0200]:

> Hi Gustavo,
> 
> > Remove another socket usage from l2cap_core.c
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  include/net/bluetooth/l2cap.h |    1 +
> >  net/bluetooth/l2cap_core.c    |   12 ++++++------
> >  net/bluetooth/l2cap_sock.c    |    8 ++++++--
> >  3 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index 3b05f3e..c55b28f 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -599,6 +599,7 @@ enum {
> >  	CONF_LOC_CONF_PEND,
> >  	CONF_REM_CONF_PEND,
> >  	CONF_NOT_COMPLETE,
> > +	CONF_DEFER_SETUP,
> >  };
> >  
> >  #define L2CAP_CONF_MAX_CONF_REQ 2
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 01171b1..388bc70 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -569,7 +569,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(CONF_DEFER_SETUP, &chan->conf_state))
> >  				result = L2CAP_CR_SEC_BLOCK;
> >  			else
> >  				result = L2CAP_CR_BAD_PSM;
> > @@ -1056,8 +1056,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(CONF_DEFER_SETUP,
> > +					     &chan->conf_state)) {
> >  					struct sock *parent = bt_sk(sk)->parent;
> >  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
> >  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> > @@ -3376,7 +3376,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> >  
> >  	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(CONF_DEFER_SETUP, &chan->conf_state)) {
> >  				__l2cap_state_change(chan, BT_CONNECT2);
> >  				result = L2CAP_CR_PEND;
> >  				status = L2CAP_CS_AUTHOR_PEND;
> > @@ -5406,8 +5406,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(CONF_DEFER_SETUP,
> > +					     &chan->conf_state)) {
> >  					struct sock *parent = bt_sk(sk)->parent;
> >  					res = L2CAP_CR_PEND;
> >  					stat = L2CAP_CS_AUTHOR_PEND;
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index d2d91b3..51d05a7 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> >  			break;
> >  		}
> >  
> > -		if (opt)
> > +		if (opt) {
> >  			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > -		else
> > +			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > +		} else {
> >  			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > +			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > +		}
> > +
> >  		break;
> 
> and what about the lines above testing this together with BT_SECURITY?

I'm ok with those lines using BT_SK_DEFER_SETUP. CONF_DEFER_SETUP is meant for
l2cap_core.c use where we should not have access to sk.

	Gustavo

Re: [PATCH -v2 07/12] Bluetooth: Create DEFER_SETUP flag in conf_state

[Marcel Holtmann]

Hi Gustavo,

> > > Remove another socket usage from l2cap_core.c
> > > 
> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > ---
> > >  include/net/bluetooth/l2cap.h |    1 +
> > >  net/bluetooth/l2cap_core.c    |   12 ++++++------
> > >  net/bluetooth/l2cap_sock.c    |    8 ++++++--
> > >  3 files changed, 13 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > > index 3b05f3e..c55b28f 100644
> > > --- a/include/net/bluetooth/l2cap.h
> > > +++ b/include/net/bluetooth/l2cap.h
> > > @@ -599,6 +599,7 @@ enum {
> > >  	CONF_LOC_CONF_PEND,
> > >  	CONF_REM_CONF_PEND,
> > >  	CONF_NOT_COMPLETE,
> > > +	CONF_DEFER_SETUP,
> > >  };
> > >  
> > >  #define L2CAP_CONF_MAX_CONF_REQ 2
> > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > index 01171b1..388bc70 100644
> > > --- a/net/bluetooth/l2cap_core.c
> > > +++ b/net/bluetooth/l2cap_core.c
> > > @@ -569,7 +569,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(CONF_DEFER_SETUP, &chan->conf_state))
> > >  				result = L2CAP_CR_SEC_BLOCK;
> > >  			else
> > >  				result = L2CAP_CR_BAD_PSM;
> > > @@ -1056,8 +1056,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(CONF_DEFER_SETUP,
> > > +					     &chan->conf_state)) {
> > >  					struct sock *parent = bt_sk(sk)->parent;
> > >  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
> > >  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> > > @@ -3376,7 +3376,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> > >  
> > >  	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(CONF_DEFER_SETUP, &chan->conf_state)) {
> > >  				__l2cap_state_change(chan, BT_CONNECT2);
> > >  				result = L2CAP_CR_PEND;
> > >  				status = L2CAP_CS_AUTHOR_PEND;
> > > @@ -5406,8 +5406,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(CONF_DEFER_SETUP,
> > > +					     &chan->conf_state)) {
> > >  					struct sock *parent = bt_sk(sk)->parent;
> > >  					res = L2CAP_CR_PEND;
> > >  					stat = L2CAP_CS_AUTHOR_PEND;
> > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > > index d2d91b3..51d05a7 100644
> > > --- a/net/bluetooth/l2cap_sock.c
> > > +++ b/net/bluetooth/l2cap_sock.c
> > > @@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> > >  			break;
> > >  		}
> > >  
> > > -		if (opt)
> > > +		if (opt) {
> > >  			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > > -		else
> > > +			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > > +		} else {
> > >  			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > > +			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > > +		}
> > > +
> > >  		break;
> > 
> > and what about the lines above testing this together with BT_SECURITY?
> 
> I'm ok with those lines using BT_SK_DEFER_SETUP. CONF_DEFER_SETUP is meant for
> l2cap_core.c use where we should not have access to sk.

I am not since it is more states we have to keep in sync. That is a bad
idea. What is the real plan here?

Regards

Marcel

Re: [PATCH -v2 07/12] Bluetooth: Create DEFER_SETUP flag in conf_state

[Gustavo Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2012-05-28 05:48:48 +0200]:

> Hi Gustavo,
> 
> > > > Remove another socket usage from l2cap_core.c
> > > > 
> > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > ---
> > > >  include/net/bluetooth/l2cap.h |    1 +
> > > >  net/bluetooth/l2cap_core.c    |   12 ++++++------
> > > >  net/bluetooth/l2cap_sock.c    |    8 ++++++--
> > > >  3 files changed, 13 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > > > index 3b05f3e..c55b28f 100644
> > > > --- a/include/net/bluetooth/l2cap.h
> > > > +++ b/include/net/bluetooth/l2cap.h
> > > > @@ -599,6 +599,7 @@ enum {
> > > >  	CONF_LOC_CONF_PEND,
> > > >  	CONF_REM_CONF_PEND,
> > > >  	CONF_NOT_COMPLETE,
> > > > +	CONF_DEFER_SETUP,
> > > >  };
> > > >  
> > > >  #define L2CAP_CONF_MAX_CONF_REQ 2
> > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > > index 01171b1..388bc70 100644
> > > > --- a/net/bluetooth/l2cap_core.c
> > > > +++ b/net/bluetooth/l2cap_core.c
> > > > @@ -569,7 +569,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(CONF_DEFER_SETUP, &chan->conf_state))
> > > >  				result = L2CAP_CR_SEC_BLOCK;
> > > >  			else
> > > >  				result = L2CAP_CR_BAD_PSM;
> > > > @@ -1056,8 +1056,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(CONF_DEFER_SETUP,
> > > > +					     &chan->conf_state)) {
> > > >  					struct sock *parent = bt_sk(sk)->parent;
> > > >  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
> > > >  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> > > > @@ -3376,7 +3376,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> > > >  
> > > >  	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(CONF_DEFER_SETUP, &chan->conf_state)) {
> > > >  				__l2cap_state_change(chan, BT_CONNECT2);
> > > >  				result = L2CAP_CR_PEND;
> > > >  				status = L2CAP_CS_AUTHOR_PEND;
> > > > @@ -5406,8 +5406,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(CONF_DEFER_SETUP,
> > > > +					     &chan->conf_state)) {
> > > >  					struct sock *parent = bt_sk(sk)->parent;
> > > >  					res = L2CAP_CR_PEND;
> > > >  					stat = L2CAP_CS_AUTHOR_PEND;
> > > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > > > index d2d91b3..51d05a7 100644
> > > > --- a/net/bluetooth/l2cap_sock.c
> > > > +++ b/net/bluetooth/l2cap_sock.c
> > > > @@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> > > >  			break;
> > > >  		}
> > > >  
> > > > -		if (opt)
> > > > +		if (opt) {
> > > >  			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > > > -		else
> > > > +			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > > > +		} else {
> > > >  			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
> > > > +			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
> > > > +		}
> > > > +
> > > >  		break;
> > > 
> > > and what about the lines above testing this together with BT_SECURITY?
> > 
> > I'm ok with those lines using BT_SK_DEFER_SETUP. CONF_DEFER_SETUP is meant for
> > l2cap_core.c use where we should not have access to sk.
> 
> I am not since it is more states we have to keep in sync. That is a bad
> idea. What is the real plan here?

The plan is to remove the use of bt_sk(sk)->flags from l2cap_core.c and the
only idea I had until now was coping the DEFER_SETUP bit to chan->conf_state
flags, however after the copy we can't remove this flags from bt_sk(sk) since
it is used is socket specific doe too.

	Gustavo

[PATCH -v2 08/12] Bluetooth: Add chan->ops->defer()

[Gustavo Padovan]

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

When DEFER_SETUP is set defer() will trigger an authorization request
to the userspace.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c55b28f..25a85ab 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -535,6 +535,7 @@ struct l2cap_ops {
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
 	void			(*ready) (void *data);
+	void			(*defer) (void *data);
 };
 
 struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 388bc70..9bd16a9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1058,12 +1058,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 				lock_sock(sk);
 				if (test_bit(CONF_DEFER_SETUP,
 					     &chan->conf_state)) {
-					struct sock *parent = bt_sk(sk)->parent;
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
-					if (parent)
-						parent->sk_data_ready(parent, 0);
-
+					if(chan->ops->defer)
+						chan->ops->defer(chan->data);
 				} else {
 					__l2cap_state_change(chan, BT_CONFIG);
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
@@ -3380,7 +3378,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 				__l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
-				parent->sk_data_ready(parent, 0);
+				if(chan->ops->defer)
+					chan->ops->defer(chan->data);
 			} else {
 				__l2cap_state_change(chan, BT_CONFIG);
 				result = L2CAP_CR_SUCCESS;
@@ -5408,11 +5407,10 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 			if (!status) {
 				if (test_bit(CONF_DEFER_SETUP,
 					     &chan->conf_state)) {
-					struct sock *parent = bt_sk(sk)->parent;
 					res = L2CAP_CR_PEND;
 					stat = L2CAP_CS_AUTHOR_PEND;
-					if (parent)
-						parent->sk_data_ready(parent, 0);
+					if(chan->ops->defer)
+						chan->ops->defer(chan->data);
 				} else {
 					__l2cap_state_change(chan, BT_CONFIG);
 					res = L2CAP_CR_SUCCESS;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 51d05a7..b19366c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1046,6 +1046,17 @@ static void l2cap_sock_ready_cb(void *data)
 	release_sock(sk);
 }
 
+static void l2cap_sock_defer_cb(void *data)
+{
+	struct sock *sk = data;
+	struct sock *parent;
+
+	parent = bt_sk(sk)->parent;
+
+	if (parent)
+		parent->sk_data_ready(parent, 0);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1055,6 +1066,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.state_change	= l2cap_sock_state_change_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 	.ready		= l2cap_sock_ready_cb,
+	.defer		= l2cap_sock_defer_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.7.10.1

Re: [PATCH -v2 08/12] Bluetooth: Add chan->ops->defer()

[Marcel Holtmann]

Hi Gustavo,

> When DEFER_SETUP is set defer() will trigger an authorization request
> to the userspace.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |    1 +
>  net/bluetooth/l2cap_core.c    |   14 ++++++--------
>  net/bluetooth/l2cap_sock.c    |   12 ++++++++++++
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c55b28f..25a85ab 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -535,6 +535,7 @@ struct l2cap_ops {
>  	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
>  					       unsigned long len, int nb);
>  	void			(*ready) (void *data);
> +	void			(*defer) (void *data);

I think this has been asked before, why not give l2cap_chan here. And
when called they can do chan->data for the specific data.

Regards

Marcel

[PATCH -v2 09/12] Bluetooth: check for already existent channel before create new one

[Gustavo Padovan]

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

Move this check to before the channel time creation simplifies the code
and avoid memory allocation if the channel already exist.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/l2cap_core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9bd16a9..e53a8f9 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3340,21 +3340,16 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	result = L2CAP_CR_NO_MEM;
 
+	/* Check if we already have channel with that dcid */
+	if (__l2cap_get_chan_by_dcid(conn, scid))
+		goto response;
+
 	chan = pchan->ops->new_connection(pchan->data);
 	if (!chan)
 		goto response;
 
 	sk = chan->sk;
 
-	/* Check if we already have channel with that dcid */
-	if (__l2cap_get_chan_by_dcid(conn, scid)) {
-		if (chan->ops->teardown)
-			chan->ops->teardown(chan->data, 0);
-
-		chan->ops->close(chan->data);
-		goto response;
-	}
-
 	hci_conn_hold(conn->hcon);
 
 	bacpy(&bt_sk(sk)->src, conn->src);
-- 
1.7.10.1

Re: [PATCH -v2 09/12] Bluetooth: check for already existent channel before create new one

[Marcel Holtmann]

Hi Gustavo,

> Move this check to before the channel time creation simplifies the code
> and avoid memory allocation if the channel already exist.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/l2cap_core.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)

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

Regards

Marcel

[PATCH -v2 10/12] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c

[Gustavo Padovan]

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

bt_accept_enqueue() can be easily placed at the end of
l2cap_sock_new_connection_cb().
There is no problem in moving to bt_accept_enqueue() to an earlier point
in the code, bt_accept_enqueue() only the sk to the it parents queue
basically.

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e53a8f9..d34d662 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1165,8 +1165,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	bacpy(&bt_sk(sk)->src, conn->src);
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 
-	bt_accept_enqueue(parent, sk);
-
 	l2cap_chan_add(conn, chan);
 
 	l2cap_chan_ready(chan);
@@ -3357,8 +3355,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	chan->psm  = psm;
 	chan->dcid = scid;
 
-	bt_accept_enqueue(parent, sk);
-
 	__l2cap_chan_add(conn, chan);
 
 	dcid = chan->scid;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index b19366c..27158f2 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -914,6 +914,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 
 	l2cap_sock_init(sk, parent);
 
+	bt_accept_enqueue(parent, sk);
+
 	return l2cap_pi(sk)->chan;
 }
 
-- 
1.7.10.1

Re: [PATCH -v2 10/12] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c

[Mat Martineau]

On Fri, 25 May 2012, Gustavo Padovan wrote:

> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> bt_accept_enqueue() can be easily placed at the end of
> l2cap_sock_new_connection_cb().
> There is no problem in moving to bt_accept_enqueue() to an earlier point
> in the code, bt_accept_enqueue() only the sk to the it parents queue
> basically.

"only adds the sk to its parent's queue"?

> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/l2cap_core.c |    4 ----
> net/bluetooth/l2cap_sock.c |    2 ++
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index e53a8f9..d34d662 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1165,8 +1165,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
> 	bacpy(&bt_sk(sk)->src, conn->src);
> 	bacpy(&bt_sk(sk)->dst, conn->dst);
>
> -	bt_accept_enqueue(parent, sk);
> -
> 	l2cap_chan_add(conn, chan);
>
> 	l2cap_chan_ready(chan);
> @@ -3357,8 +3355,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> 	chan->psm  = psm;
> 	chan->dcid = scid;
>
> -	bt_accept_enqueue(parent, sk);
> -
> 	__l2cap_chan_add(conn, chan);
>
> 	dcid = chan->scid;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index b19366c..27158f2 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -914,6 +914,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
>
> 	l2cap_sock_init(sk, parent);
>
> +	bt_accept_enqueue(parent, sk);
> +
> 	return l2cap_pi(sk)->chan;
> }
>
> -- 
> 1.7.10.1

Other than that, looks good!


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

[PATCH -v2 11/12] Bluetooth: Remove parent socket usage from l2cap_core.c

[Gustavo Padovan]

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

We can lock the parent lock only inside the new_connection() call, then we
just use the l2cap_chan_lock() in core code.

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

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d34d662..0071025 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1139,7 +1139,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 *sk;
 	struct l2cap_chan *chan, *pchan;
 
 	BT_DBG("");
@@ -1150,9 +1150,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->data);
 	if (!chan)
@@ -1170,7 +1168,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)
@@ -3308,7 +3306,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	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;
+	struct sock *sk = NULL;
 	int result, status = L2CAP_CS_NO_INFO;
 
 	u16 dcid = 0, scid = __le16_to_cpu(req->scid);
@@ -3323,10 +3321,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		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) &&
@@ -3388,7 +3384,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	}
 
 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 27158f2..bd2fc78 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -899,16 +899,21 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 {
 	struct sock *sk, *parent = data;
 
+	lock_sock(parent);
+
 	/* Check for backlog size */
 	if (sk_acceptq_is_full(parent)) {
 		BT_DBG("backlog full %d", parent->sk_ack_backlog);
+		release_sock(parent);
 		return NULL;
 	}
 
 	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
 								GFP_ATOMIC);
-	if (!sk)
+	if (!sk) {
+		release_sock(parent);
 		return NULL;
+	}
 
 	bt_sock_reclassify_lock(sk, BTPROTO_L2CAP);
 
@@ -916,6 +921,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 
 	bt_accept_enqueue(parent, sk);
 
+	release_sock(parent);
+
 	return l2cap_pi(sk)->chan;
 }
 
-- 
1.7.10.1

[PATCH -v2 12/12] Bluetooth: Used void * as parameter in alloc_skb()

[Gustavo Padovan]

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

This keep l2cap chan ops functions parameters in sync.

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

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 25a85ab..0f8886b 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -532,8 +532,8 @@ struct l2cap_ops {
 	void			(*close) (void *data);
 	void			(*teardown) (void *data, int err);
 	void			(*state_change) (void *data, int state);
-	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
-					       unsigned long len, int nb);
+	struct sk_buff		*(*alloc_skb) (void *data, unsigned long len,
+					       int nb);
 	void			(*ready) (void *data);
 	void			(*defer) (void *data);
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0071025..bd80b03 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1893,7 +1893,7 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
 
 		count = min_t(unsigned int, conn->mtu, len);
 
-		tmp = chan->ops->alloc_skb(chan, count,
+		tmp = chan->ops->alloc_skb(chan->data, count,
 					   msg->msg_flags & MSG_DONTWAIT);
 		if (IS_ERR(tmp))
 			return PTR_ERR(tmp);
@@ -1930,7 +1930,7 @@ static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
 
 	count = min_t(unsigned int, (conn->mtu - hlen), len);
 
-	skb = chan->ops->alloc_skb(chan, count + hlen,
+	skb = chan->ops->alloc_skb(chan->data, count + hlen,
 				   msg->msg_flags & MSG_DONTWAIT);
 	if (IS_ERR(skb))
 		return skb;
@@ -1964,7 +1964,7 @@ static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
 
 	count = min_t(unsigned int, (conn->mtu - L2CAP_HDR_SIZE), len);
 
-	skb = chan->ops->alloc_skb(chan, count + L2CAP_HDR_SIZE,
+	skb = chan->ops->alloc_skb(chan->data, count + L2CAP_HDR_SIZE,
 				   msg->msg_flags & MSG_DONTWAIT);
 	if (IS_ERR(skb))
 		return skb;
@@ -2011,7 +2011,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
 
 	count = min_t(unsigned int, (conn->mtu - hlen), len);
 
-	skb = chan->ops->alloc_skb(chan, count + hlen,
+	skb = chan->ops->alloc_skb(chan->data, count + hlen,
 				   msg->msg_flags & MSG_DONTWAIT);
 	if (IS_ERR(skb))
 		return skb;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index bd2fc78..f559e64 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1019,14 +1019,16 @@ static void l2cap_sock_state_change_cb(void *data, int state)
 	sk->sk_state = state;
 }
 
-static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
-					       unsigned long len, int nb)
+static struct sk_buff *l2cap_sock_alloc_skb_cb(void *data, unsigned long len,
+					       int nb)
 {
+	struct sock *sk = data;
+	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
 	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)
-- 
1.7.10.1

Re: [PATCH -v2 12/12] Bluetooth: Used void * as parameter in alloc_skb()

[Andrei Emeltchenko]

Hi Gustavo,

On Fri, May 25, 2012 at 09:31:04AM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> This keep l2cap chan ops functions parameters in sync.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

...

> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 25a85ab..0f8886b 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -532,8 +532,8 @@ struct l2cap_ops {
>  	void			(*close) (void *data);
>  	void			(*teardown) (void *data, int err);
>  	void			(*state_change) (void *data, int state);
> -	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
> -					       unsigned long len, int nb);
> +	struct sk_buff		*(*alloc_skb) (void *data, unsigned long len,
> +					       int nb);

...

> @@ -1893,7 +1893,7 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
>  
>  		count = min_t(unsigned int, conn->mtu, len);
>  
> -		tmp = chan->ops->alloc_skb(chan, count,
> +		tmp = chan->ops->alloc_skb(chan->data, count,

Why do we need to pass chan->data instead of chan?

Best regards 
Andrei Emeltchenko 

Re: [PATCH -v2 12/12] Bluetooth: Used void * as parameter in alloc_skb()

[Gustavo Padovan]

Hi Andrei,

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2012-05-25 15:39:34 +0300]:

> Hi Gustavo,
> 
> On Fri, May 25, 2012 at 09:31:04AM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > This keep l2cap chan ops functions parameters in sync.
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> ...
> 
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index 25a85ab..0f8886b 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -532,8 +532,8 @@ struct l2cap_ops {
> >  	void			(*close) (void *data);
> >  	void			(*teardown) (void *data, int err);
> >  	void			(*state_change) (void *data, int state);
> > -	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
> > -					       unsigned long len, int nb);
> > +	struct sk_buff		*(*alloc_skb) (void *data, unsigned long len,
> > +					       int nb);
> 
> ...
> 
> > @@ -1893,7 +1893,7 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
> >  
> >  		count = min_t(unsigned int, conn->mtu, len);
> >  
> > -		tmp = chan->ops->alloc_skb(chan, count,
> > +		tmp = chan->ops->alloc_skb(chan->data, count,
> 
> Why do we need to pass chan->data instead of chan?

We are changing this to make this call have the same parameter as the other
ops calls, i.e., a void *, so we just pass chan->data here which is the user
data (struct sock *sk in the l2cap_sock.c case).

	Gustavo

Re: [PATCH -v2 12/12] Bluetooth: Used void * as parameter in alloc_skb()

[Marcel Holtmann]

Hi Gustavo,

> > On Fri, May 25, 2012 at 09:31:04AM -0300, Gustavo Padovan wrote:
> > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > 
> > > This keep l2cap chan ops functions parameters in sync.
> > > 
> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > ...
> > 
> > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > > index 25a85ab..0f8886b 100644
> > > --- a/include/net/bluetooth/l2cap.h
> > > +++ b/include/net/bluetooth/l2cap.h
> > > @@ -532,8 +532,8 @@ struct l2cap_ops {
> > >  	void			(*close) (void *data);
> > >  	void			(*teardown) (void *data, int err);
> > >  	void			(*state_change) (void *data, int state);
> > > -	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
> > > -					       unsigned long len, int nb);
> > > +	struct sk_buff		*(*alloc_skb) (void *data, unsigned long len,
> > > +					       int nb);
> > 
> > ...
> > 
> > > @@ -1893,7 +1893,7 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
> > >  
> > >  		count = min_t(unsigned int, conn->mtu, len);
> > >  
> > > -		tmp = chan->ops->alloc_skb(chan, count,
> > > +		tmp = chan->ops->alloc_skb(chan->data, count,
> > 
> > Why do we need to pass chan->data instead of chan?
> 
> We are changing this to make this call have the same parameter as the other
> ops calls, i.e., a void *, so we just pass chan->data here which is the user
> data (struct sock *sk in the l2cap_sock.c case).

so what is the problem to just pass l2cap_chan in here for all calls and
then have the it do chan->data inside the callback. This void *data
thing is dead ugly.

Regards

Marcel