* [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops
@ 2012-05-28 1:27 Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
` (8 more replies)
0 siblings, 9 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo Padovan
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Use chan instead of void * makes more sense here.
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
include/net/bluetooth/l2cap.h | 10 ++++++----
net/bluetooth/l2cap_core.c | 30 +++++++++++++++---------------
net/bluetooth/l2cap_sock.c | 16 ++++++++--------
3 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f44344b..aa2dbc6 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -527,10 +527,12 @@ struct l2cap_chan {
struct l2cap_ops {
char *name;
- struct l2cap_chan *(*new_connection) (void *data);
- int (*recv) (void *data, struct sk_buff *skb);
- void (*close) (void *data);
- void (*state_change) (void *data, int state);
+ struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan);
+ int (*recv) (struct l2cap_chan * chan,
+ struct sk_buff *skb);
+ void (*close) (struct l2cap_chan *chan);
+ void (*state_change) (struct l2cap_chan *chan,
+ 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 de0dc9e..7edc814 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -180,7 +180,7 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
state_to_string(state));
chan->state = state;
- chan->ops->state_change(chan->data, state);
+ chan->ops->state_change(chan, state);
}
static void l2cap_state_change(struct l2cap_chan *chan, int state)
@@ -381,7 +381,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
l2cap_chan_unlock(chan);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
mutex_unlock(&conn->chan_lock);
l2cap_chan_put(chan);
@@ -569,7 +569,7 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
l2cap_chan_close(chan, ECONNRESET);
l2cap_chan_unlock(chan);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
}
}
@@ -1213,7 +1213,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
goto clean;
}
- chan = pchan->ops->new_connection(pchan->data);
+ chan = pchan->ops->new_connection(pchan);
if (!chan)
goto clean;
@@ -1324,7 +1324,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
l2cap_chan_unlock(chan);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
l2cap_chan_put(chan);
}
@@ -2568,7 +2568,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
if (!nskb)
continue;
- if (chan->ops->recv(chan->data, nskb))
+ if (chan->ops->recv(chan, nskb))
kfree_skb(nskb);
}
@@ -3411,7 +3411,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
goto response;
}
- chan = pchan->ops->new_connection(pchan->data);
+ chan = pchan->ops->new_connection(pchan);
if (!chan)
goto response;
@@ -3420,7 +3420,7 @@ 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);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
goto response;
}
@@ -3831,7 +3831,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
l2cap_chan_unlock(chan);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
l2cap_chan_put(chan);
mutex_unlock(&conn->chan_lock);
@@ -3865,7 +3865,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
l2cap_chan_unlock(chan);
- chan->ops->close(chan->data);
+ chan->ops->close(chan);
l2cap_chan_put(chan);
mutex_unlock(&conn->chan_lock);
@@ -4435,7 +4435,7 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
if (chan->sdu)
break;
- err = chan->ops->recv(chan->data, skb);
+ err = chan->ops->recv(chan, skb);
break;
case L2CAP_SAR_START:
@@ -4485,7 +4485,7 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
if (chan->sdu->len != chan->sdu_len)
break;
- err = chan->ops->recv(chan->data, chan->sdu);
+ err = chan->ops->recv(chan, chan->sdu);
if (!err) {
/* Reassembly complete */
@@ -5207,7 +5207,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (chan->imtu < skb->len)
goto drop;
- if (!chan->ops->recv(chan->data, skb))
+ if (!chan->ops->recv(chan, skb))
goto done;
break;
@@ -5246,7 +5246,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
if (chan->imtu < skb->len)
goto drop;
- if (!chan->ops->recv(chan->data, skb))
+ if (!chan->ops->recv(chan, skb))
return 0;
drop:
@@ -5272,7 +5272,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, u16 cid,
if (chan->imtu < skb->len)
goto drop;
- if (!chan->ops->recv(chan->data, skb))
+ if (!chan->ops->recv(chan, skb))
return 0;
drop:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index d244361..db787f6 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -872,9 +872,9 @@ static int l2cap_sock_release(struct socket *sock)
return err;
}
-static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
+static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
{
- struct sock *sk, *parent = data;
+ struct sock *sk, *parent = chan->data;
sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
GFP_ATOMIC);
@@ -888,10 +888,10 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
return l2cap_pi(sk)->chan;
}
-static int l2cap_sock_recv_cb(void *data, struct sk_buff *skb)
+static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
{
int err;
- struct sock *sk = data;
+ struct sock *sk = chan->data;
struct l2cap_pinfo *pi = l2cap_pi(sk);
lock_sock(sk);
@@ -924,16 +924,16 @@ done:
return err;
}
-static void l2cap_sock_close_cb(void *data)
+static void l2cap_sock_close_cb(struct l2cap_chan *chan)
{
- struct sock *sk = data;
+ struct sock *sk = chan->data;
l2cap_sock_kill(sk);
}
-static void l2cap_sock_state_change_cb(void *data, int state)
+static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
{
- struct sock *sk = data;
+ struct sock *sk = chan->data;
sk->sk_state = state;
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrei Emeltchenko
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
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 | 61 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 46 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index aa2dbc6..76b0e7e 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) (struct l2cap_chan *chan);
int (*recv) (struct l2cap_chan * chan,
struct sk_buff *skb);
+ void (*teardown) (struct l2cap_chan *chan, int err);
void (*close) (struct l2cap_chan *chan);
void (*state_change) (struct l2cap_chan *chan,
int state);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7edc814..1f4c720 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, 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);
- }
-}
-
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, 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, 0);
break;
}
}
@@ -3419,7 +3380,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, 0);
+
chan->ops->close(chan);
goto response;
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index db787f6..3f59463 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(struct l2cap_chan *chan)
{
struct sock *sk, *parent = chan->data;
@@ -931,6 +950,47 @@ static void l2cap_sock_close_cb(struct l2cap_chan *chan)
l2cap_sock_kill(sk);
}
+static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
+{
+ struct sock *sk = chan->data;
+ struct sock *parent;
+
+ lock_sock(sk);
+
+ parent = bt_sk(sk)->parent;
+
+ 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(struct l2cap_chan *chan, int state)
{
struct sock *sk = chan->data;
@@ -959,6 +1019,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.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready()
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrei Emeltchenko
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
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 76b0e7e..c5726c2 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -534,6 +534,7 @@ struct l2cap_ops {
void (*close) (struct l2cap_chan *chan);
void (*state_change) (struct l2cap_chan *chan,
int state);
+ void (*ready) (struct l2cap_chan *chan);
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 1f4c720..5947eb1 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);
+ chan->state = BT_CONNECTED;
- if (parent)
- parent->sk_data_ready(parent, 0);
-
- release_sock(sk);
+ if (chan->ops->ready)
+ chan->ops->ready(chan);
}
static void l2cap_do_start(struct l2cap_chan *chan)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 3f59463..5563023 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1014,6 +1014,26 @@ static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
return skb;
}
+static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
+{
+ struct sock *sk = chan->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,
@@ -1021,6 +1041,7 @@ static struct l2cap_ops l2cap_chan_ops = {
.close = l2cap_sock_close_cb,
.teardown = l2cap_sock_teardown_cb,
.state_change = l2cap_sock_state_change_cb,
+ .ready = l2cap_sock_ready_cb,
.alloc_skb = l2cap_sock_alloc_skb_cb,
};
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5947eb1..35e6d7d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1442,21 +1442,17 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
goto done;
}
- lock_sock(sk);
-
- switch (sk->sk_state) {
+ switch (chan->state) {
case BT_CONNECT:
case BT_CONNECT2:
case BT_CONFIG:
/* Already connecting */
err = 0;
- release_sock(sk);
goto done;
case BT_CONNECTED:
/* Already connected */
err = -EISCONN;
- release_sock(sk);
goto done;
case BT_OPEN:
@@ -1466,13 +1462,12 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
default:
err = -EBADFD;
- release_sock(sk);
goto done;
}
/* Set destination address and psm */
+ lock_sock(sk);
bacpy(&bt_sk(sk)->dst, dst);
-
release_sock(sk);
chan->psm = psm;
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (2 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 3:59 ` Marcel Holtmann
2012-05-28 1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
` (4 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 35e6d7d..d64c836 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);
if (!chan)
goto clean;
@@ -3349,12 +3343,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);
if (!chan)
goto response;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 5563023..d856cc8 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(struct l2cap_chan *chan)
{
struct sock *sk, *parent = chan->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.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (3 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 c5726c2..0ea4cf1 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -601,6 +601,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 d64c836..d0c1ebe 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);
@@ -3377,7 +3377,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;
@@ -5407,8 +5407,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 d856cc8..a421a4e 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.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (4 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 3:54 ` Marcel Holtmann
2012-05-28 1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 0ea4cf1..6d0864e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -535,6 +535,7 @@ struct l2cap_ops {
void (*state_change) (struct l2cap_chan *chan,
int state);
void (*ready) (struct l2cap_chan *chan);
+ void (*defer) (struct l2cap_chan *chan);
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 d0c1ebe..35c2c52 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);
@@ -3381,7 +3379,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;
@@ -5409,11 +5408,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 a421a4e..e81e910 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
release_sock(sk);
}
+static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
+{
+ struct sock *sk = chan->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,
@@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
.teardown = l2cap_sock_teardown_cb,
.state_change = l2cap_sock_state_change_cb,
.ready = l2cap_sock_ready_cb,
+ .defer = l2cap_sock_defer_cb,
.alloc_skb = l2cap_sock_alloc_skb_cb,
};
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (5 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 4:01 ` Marcel Holtmann
2012-05-28 1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
2012-05-28 1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan
8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
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 35c2c52..584c66f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3341,21 +3341,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);
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, 0);
-
- chan->ops->close(chan);
- goto response;
- }
-
hci_conn_hold(conn->hcon);
bacpy(&bt_sk(sk)->src, conn->src);
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (6 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
@ 2012-05-28 1:27 ` Gustavo Padovan
2012-05-28 1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:27 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 adds 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 584c66f..74843d2 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);
@@ -3358,8 +3356,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 e81e910..700eef8 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(struct l2cap_chan *chan)
l2cap_sock_init(sk, parent);
+ bt_accept_enqueue(parent, sk);
+
return l2cap_pi(sk)->chan;
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
` (7 preceding siblings ...)
2012-05-28 1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
@ 2012-05-28 1:28 ` Gustavo Padovan
8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 1:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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 74843d2..5920a7c 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);
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)
@@ -3309,7 +3307,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);
@@ -3324,10 +3322,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) &&
@@ -3389,7 +3385,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 700eef8..cfb97ca 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(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);
+ 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(struct l2cap_chan *chan)
bt_accept_enqueue(parent, sk);
+ release_sock(parent);
+
return l2cap_pi(sk)->chan;
}
--
1.7.10.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
2012-05-28 1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
@ 2012-05-28 3:54 ` Marcel Holtmann
2012-05-28 14:12 ` Andrei Emeltchenko
2012-05-28 16:23 ` Gustavo Padovan
0 siblings, 2 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28 3:54 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
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 0ea4cf1..6d0864e 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -535,6 +535,7 @@ struct l2cap_ops {
> void (*state_change) (struct l2cap_chan *chan,
> int state);
> void (*ready) (struct l2cap_chan *chan);
> + void (*defer) (struct l2cap_chan *chan);
> 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 d0c1ebe..35c2c52 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)
Coding style issue here.
> + chan->ops->defer(chan->data);
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> @@ -3381,7 +3379,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)
And here.
> + chan->ops->defer(chan->data);
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> result = L2CAP_CR_SUCCESS;
> @@ -5409,11 +5408,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)
And here.
> + 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 a421a4e..e81e910 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
> release_sock(sk);
> }
>
> +static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
> +{
> + struct sock *sk = chan->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,
> @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> .teardown = l2cap_sock_teardown_cb,
> .state_change = l2cap_sock_state_change_cb,
> .ready = l2cap_sock_ready_cb,
> + .defer = l2cap_sock_defer_cb,
> .alloc_skb = l2cap_sock_alloc_skb_cb,
> };
>
In addition can you explain to me how this is suppose to work if defer
is not set. Then we never call ready.
Also this is a pretty bad idea inside the code. We better do something
like l2cap_sock_no_defer like the network subsystem does for general
socket. Since otherwise you keep spreading if (defer) all over the
places.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c
2012-05-28 1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
@ 2012-05-28 3:59 ` Marcel Holtmann
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28 3:59 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
Hi Gustavo,
> 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(-)
I applied this the first 5 patches to bluetooth-next so far.
<snip>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 5563023..d856cc8 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(struct l2cap_chan *chan)
> {
> struct sock *sk, *parent = chan->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)
And on a side note, does this still have to be GFP_ATOMIC?
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one
2012-05-28 1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
@ 2012-05-28 4:01 ` Marcel Holtmann
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28 4:01 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
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>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/l2cap_core.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
in addition I also applied this patch to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
2012-05-28 3:54 ` Marcel Holtmann
@ 2012-05-28 14:12 ` Andrei Emeltchenko
2012-05-28 16:23 ` Gustavo Padovan
1 sibling, 0 replies; 15+ messages in thread
From: Andrei Emeltchenko @ 2012-05-28 14:12 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Gustavo Padovan, linux-bluetooth, Gustavo Padovan
Hi Marcel and Gustavo,
On Mon, May 28, 2012 at 05:54:20AM +0200, Marcel Holtmann wrote:
...
> > @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> > .teardown = l2cap_sock_teardown_cb,
> > .state_change = l2cap_sock_state_change_cb,
> > .ready = l2cap_sock_ready_cb,
> > + .defer = l2cap_sock_defer_cb,
> > .alloc_skb = l2cap_sock_alloc_skb_cb,
> > };
>
> In addition can you explain to me how this is suppose to work if defer
> is not set. Then we never call ready.
For fixed channel we do not use ready so this would be OK.
> Also this is a pretty bad idea inside the code. We better do something
> like l2cap_sock_no_defer like the network subsystem does for general
> socket. Since otherwise you keep spreading if (defer) all over the
> places.
This look right, I will change my code following your proposal so Gustavo
can invoke those functions directly.
Best regards
Andrei Emeltchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
2012-05-28 3:54 ` Marcel Holtmann
2012-05-28 14:12 ` Andrei Emeltchenko
@ 2012-05-28 16:23 ` Gustavo Padovan
1 sibling, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 16:23 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, Gustavo Padovan
Hi Marcel,
* Marcel Holtmann <marcel@holtmann.org> [2012-05-28 05:54:20 +0200]:
> 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 0ea4cf1..6d0864e 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -535,6 +535,7 @@ struct l2cap_ops {
> > void (*state_change) (struct l2cap_chan *chan,
> > int state);
> > void (*ready) (struct l2cap_chan *chan);
> > + void (*defer) (struct l2cap_chan *chan);
> > 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 d0c1ebe..35c2c52 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)
>
> Coding style issue here.
>
> > + chan->ops->defer(chan->data);
> > } else {
> > __l2cap_state_change(chan, BT_CONFIG);
> > rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> > @@ -3381,7 +3379,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)
>
> And here.
>
> > + chan->ops->defer(chan->data);
> > } else {
> > __l2cap_state_change(chan, BT_CONFIG);
> > result = L2CAP_CR_SUCCESS;
> > @@ -5409,11 +5408,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)
>
> And here.
>
> > + 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 a421a4e..e81e910 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
> > release_sock(sk);
> > }
> >
> > +static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
> > +{
> > + struct sock *sk = chan->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,
> > @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> > .teardown = l2cap_sock_teardown_cb,
> > .state_change = l2cap_sock_state_change_cb,
> > .ready = l2cap_sock_ready_cb,
> > + .defer = l2cap_sock_defer_cb,
> > .alloc_skb = l2cap_sock_alloc_skb_cb,
> > };
> >
>
> In addition can you explain to me how this is suppose to work if defer
> is not set. Then we never call ready.
If you don't set defer_setup, you probably don't need to call ready. That was the
idea here.
> Also this is a pretty bad idea inside the code. We better do something
> like l2cap_sock_no_defer like the network subsystem does for general
> socket. Since otherwise you keep spreading if (defer) all over the
> places.
Sure, I agree, I'll remove all if (defer) from the code for now. And as
l2cap_sock is still the sole user of this we don't need to add
l2cap_sock_no_defer yet.
Gustavo
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-05-28 16:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-28 1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
2012-05-28 3:59 ` Marcel Holtmann
2012-05-28 1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
2012-05-28 3:54 ` Marcel Holtmann
2012-05-28 14:12 ` Andrei Emeltchenko
2012-05-28 16:23 ` Gustavo Padovan
2012-05-28 1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
2012-05-28 4:01 ` Marcel Holtmann
2012-05-28 1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
2012-05-28 1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).