2011-06-02 16:15:24

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 1/6] Bluetooth: Add l2cap_chan_ops abstraction

Add an abstraction layer between L2CAP core and its users (only
l2cap_sock.c now). The first function implemented is new_connection() that
replaces calls to l2cap_sock_alloc() in l2cap_core.c

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 12 +++++++++---
net/bluetooth/l2cap_core.c | 17 +++++++----------
net/bluetooth/l2cap_sock.c | 29 +++++++++++++++++++++++++++--
3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a2dcbff..95e44df 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -353,6 +353,15 @@ struct l2cap_chan {

struct list_head list;
struct list_head global_l;
+
+ struct l2cap_ops *ops;
+};
+
+struct l2cap_ops {
+ char *name;
+ void *data;
+
+ struct l2cap_chan *(*new_connection) (void *data);
};

struct l2cap_conn {
@@ -459,9 +468,6 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm);
int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid);

void l2cap_sock_kill(struct sock *sk);
-void l2cap_sock_init(struct sock *sk, struct sock *parent);
-struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
- int proto, gfp_t prio);

struct l2cap_chan *l2cap_chan_create(struct sock *sk);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 233ecdc..6849e74 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -837,18 +837,16 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
goto clean;
}

- sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, GFP_ATOMIC);
- if (!sk)
+ chan = pchan->ops->new_connection(pchan->ops->data);
+ if (!chan)
goto clean;

- chan = l2cap_pi(sk)->chan;
+ sk = chan->sk;

write_lock_bh(&conn->chan_lock);

hci_conn_hold(conn->hcon);

- l2cap_sock_init(sk, parent);
-
bacpy(&bt_sk(sk)->src, conn->src);
bacpy(&bt_sk(sk)->dst, conn->dst);

@@ -2323,10 +2321,12 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
goto response;
}

- sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, GFP_ATOMIC);
- if (!sk)
+ chan = pchan->ops->new_connection(pchan->ops->data);
+ if (!chan)
goto response;

+ sk = chan->sk;
+
write_lock_bh(&conn->chan_lock);

/* Check if we already have channel with that dcid */
@@ -2339,9 +2339,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd

hci_conn_hold(conn->hcon);

- chan = l2cap_pi(sk)->chan;
-
- l2cap_sock_init(sk, parent);
bacpy(&bt_sk(sk)->src, conn->src);
bacpy(&bt_sk(sk)->dst, conn->dst);
chan->psm = psm;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1337811..79926a4 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -31,6 +31,8 @@
#include <net/bluetooth/l2cap.h>

static const struct proto_ops l2cap_sock_ops;
+static void l2cap_sock_init(struct sock *sk, struct sock *parent);
+static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio);

static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
{
@@ -739,6 +741,25 @@ static int l2cap_sock_release(struct socket *sock)
return err;
}

+static struct l2cap_chan *l2cap_chan_new_connection_cb(void *data)
+{
+ struct sock *sk, *parent = data;
+
+ sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
+ GFP_ATOMIC);
+ if (!sk)
+ return NULL;
+
+ l2cap_sock_init(sk, parent);
+
+ return l2cap_pi(sk)->chan;
+}
+
+static struct l2cap_ops l2cap_chan_ops = {
+ .name = "L2CAP Socket Interface",
+ .new_connection = l2cap_chan_new_connection_cb,
+};
+
static void l2cap_sock_destruct(struct sock *sk)
{
BT_DBG("sk %p", sk);
@@ -747,7 +768,7 @@ static void l2cap_sock_destruct(struct sock *sk)
skb_queue_purge(&sk->sk_write_queue);
}

-void l2cap_sock_init(struct sock *sk, struct sock *parent)
+static void l2cap_sock_init(struct sock *sk, struct sock *parent)
{
struct l2cap_pinfo *pi = l2cap_pi(sk);
struct l2cap_chan *chan = pi->chan;
@@ -772,6 +793,7 @@ void l2cap_sock_init(struct sock *sk, struct sock *parent)
chan->role_switch = pchan->role_switch;
chan->force_reliable = pchan->force_reliable;
chan->flushable = pchan->flushable;
+ chan->ops = pchan->ops;
} else {

switch (sk->sk_type) {
@@ -802,6 +824,9 @@ void l2cap_sock_init(struct sock *sk, struct sock *parent)
chan->role_switch = 0;
chan->force_reliable = 0;
chan->flushable = BT_FLUSHABLE_OFF;
+
+ l2cap_chan_ops.data = sk;
+ chan->ops = &l2cap_chan_ops;
}

/* Default config options */
@@ -814,7 +839,7 @@ static struct proto l2cap_proto = {
.obj_size = sizeof(struct l2cap_pinfo)
};

-struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio)
+static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio)
{
struct sock *sk;
struct l2cap_chan *chan;
--
1.7.5.1



2011-06-02 21:50:26

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops

Hi Mat,

* Mat Martineau <[email protected]> [2011-06-02 14:30:41 -0700]:

>
> Hi Gustavo -
>
>
> On Thu, 2 Jun 2011, Gustavo F. Padovan wrote:
>
> >close() calls l2cap_sock_kill() on l2cap_sock.c
> >
> >Signed-off-by: Gustavo F. Padovan <[email protected]>
> >---
> >include/net/bluetooth/l2cap.h | 3 +--
> >net/bluetooth/l2cap_core.c | 17 +++++++++--------
> >net/bluetooth/l2cap_sock.c | 10 +++++++++-
> >3 files changed, 19 insertions(+), 11 deletions(-)
>
> <snip>
>
> >--- a/net/bluetooth/l2cap_sock.c
> >+++ b/net/bluetooth/l2cap_sock.c
> >@@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
> >/* Kill socket (only if zapped and orphan)
> > * Must be called on unlocked socket.
> > */
> >-void l2cap_sock_kill(struct sock *sk)
> >+static void l2cap_sock_kill(struct sock *sk)
> >{
> > if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> > return;
> >@@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
> > return sock_queue_rcv_skb(sk, skb);
> >}
> >
> >+static void l2cap_chan_close_cb(void *data)
> >+{
> >+ struct sock *sk = data;
> >+
> >+ l2cap_sock_kill(sk);
> >+}
> >+
> >static struct l2cap_ops l2cap_chan_ops = {
> > .name = "L2CAP Socket Interface",
> > .new_connection = l2cap_chan_new_connection_cb,
> > .recv = l2cap_chan_recv_cb,
> >+ .close = l2cap_chan_close_cb,
> >};
> >
> >static void l2cap_sock_destruct(struct sock *sk)
>
>
> It looks like we'll have l2cap_ops for sock, rfcomm, and AMP's A2MP
> channel -- so I would suggest using unique names for the callbacks
> for each of those modules:
>
> l2cap_sock_new_connection_cb
> l2cap_sock_recv_cb
> l2cap_sock_close_cb
>
> l2cap_a2mp_new_connection_cb
> l2cap_a2mp_recv_cb
> l2cap_a2mp_close_cb
>
> etc., instead of "l2cap_chan_*_cb"
>
> Even though the functions are (or will be) static within their
> respective files, it will be less confusing if their names tie them
> to the specific module.

I'm not sure, what I'm trying to avoid here is create a false relation between
l2cap_sock_close and l2cap_sock_close_cb for example, thus I called it
l2cap_chan_close_cb. But unique names are indeed a good idea.

Maybe, l2cap_sock_chan_new_cb, l2cap_sock_chan_close_cb,
l2cap_sock_chan_recv_cb. But those are a bit big.

>
> I hope you can check these in to bluetooth-next soon, it would help
> with the patch set I'm working on.

Btw, I really want to fix this L2CAP mess before push any A2MP stuff into the
tree. A2MP code shouldn't touch sockets at all. I don't want to bring more
issues to the stack like the many we have for RFCOMM.
I intend to finish this soon. Patches are welcome. :)

--
Gustavo F. Padovan
http://profusion.mobi

2011-06-02 21:30:41

by Mat Martineau

[permalink] [raw]
Subject: Re: [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops


Hi Gustavo -


On Thu, 2 Jun 2011, Gustavo F. Padovan wrote:

> close() calls l2cap_sock_kill() on l2cap_sock.c
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 3 +--
> net/bluetooth/l2cap_core.c | 17 +++++++++--------
> net/bluetooth/l2cap_sock.c | 10 +++++++++-
> 3 files changed, 19 insertions(+), 11 deletions(-)

<snip>

> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
> /* Kill socket (only if zapped and orphan)
> * Must be called on unlocked socket.
> */
> -void l2cap_sock_kill(struct sock *sk)
> +static void l2cap_sock_kill(struct sock *sk)
> {
> if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> return;
> @@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
> return sock_queue_rcv_skb(sk, skb);
> }
>
> +static void l2cap_chan_close_cb(void *data)
> +{
> + struct sock *sk = data;
> +
> + l2cap_sock_kill(sk);
> +}
> +
> static struct l2cap_ops l2cap_chan_ops = {
> .name = "L2CAP Socket Interface",
> .new_connection = l2cap_chan_new_connection_cb,
> .recv = l2cap_chan_recv_cb,
> + .close = l2cap_chan_close_cb,
> };
>
> static void l2cap_sock_destruct(struct sock *sk)


It looks like we'll have l2cap_ops for sock, rfcomm, and AMP's A2MP
channel -- so I would suggest using unique names for the callbacks for
each of those modules:

l2cap_sock_new_connection_cb
l2cap_sock_recv_cb
l2cap_sock_close_cb

l2cap_a2mp_new_connection_cb
l2cap_a2mp_recv_cb
l2cap_a2mp_close_cb

etc., instead of "l2cap_chan_*_cb"

Even though the functions are (or will be) static within their
respective files, it will be less confusing if their names tie them
to the specific module.

I hope you can check these in to bluetooth-next soon, it would help
with the patch set I'm working on.

Regards,

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

2011-06-02 16:15:29

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 6/6] Bluetooth: keep reference if any ERTM timer is enabled

ERTM use the generic L2CAP timer functions to keep a reference to the
channel. This is useful for avoiding crashes.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 15 ++++++++-----
net/bluetooth/l2cap_core.c | 44 ++++++++++++++++++++--------------------
2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e3e58f9..c4799da2 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -437,12 +437,15 @@ struct l2cap_pinfo {

#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
-#define __mod_retrans_timer() mod_timer(&chan->retrans_timer, \
- jiffies + msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
-#define __mod_monitor_timer() mod_timer(&chan->monitor_timer, \
- jiffies + msecs_to_jiffies(L2CAP_DEFAULT_MONITOR_TO));
-#define __mod_ack_timer() mod_timer(&chan->ack_timer, \
- jiffies + msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
+#define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \
+ L2CAP_DEFAULT_RETRANS_TO);
+#define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer)
+#define __set_monitor_timer(c) l2cap_set_timer(c, &c->monitor_timer, \
+ L2CAP_DEFAULT_MONITOR_TO);
+#define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer)
+#define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \
+ L2CAP_DEFAULT_ACK_TO);
+#define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)

static inline int l2cap_tx_window_full(struct l2cap_chan *ch)
{
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8586077..7042f6e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -380,9 +380,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
if (chan->mode == L2CAP_MODE_ERTM) {
struct srej_list *l, *tmp;

- del_timer(&chan->retrans_timer);
- del_timer(&chan->monitor_timer);
- del_timer(&chan->ack_timer);
+ __clear_retrans_timer(chan);
+ __clear_monitor_timer(chan);
+ __clear_ack_timer(chan);

skb_queue_purge(&chan->srej_q);
skb_queue_purge(&chan->busy_q);
@@ -686,9 +686,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c
sk = chan->sk;

if (chan->mode == L2CAP_MODE_ERTM) {
- del_timer(&chan->retrans_timer);
- del_timer(&chan->monitor_timer);
- del_timer(&chan->ack_timer);
+ __clear_retrans_timer(chan);
+ __clear_monitor_timer(chan);
+ __clear_ack_timer(chan);
}

req.dcid = cpu_to_le16(chan->dcid);
@@ -1166,7 +1166,7 @@ static void l2cap_monitor_timeout(unsigned long arg)
}

chan->retry_count++;
- __mod_monitor_timer();
+ __set_monitor_timer(chan);

l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL);
bh_unlock_sock(sk);
@@ -1181,7 +1181,7 @@ static void l2cap_retrans_timeout(unsigned long arg)

bh_lock_sock(sk);
chan->retry_count = 1;
- __mod_monitor_timer();
+ __set_monitor_timer(chan);

chan->conn_state |= L2CAP_CONN_WAIT_F;

@@ -1205,7 +1205,7 @@ static void l2cap_drop_acked_frames(struct l2cap_chan *chan)
}

if (!chan->unacked_frames)
- del_timer(&chan->retrans_timer);
+ __clear_retrans_timer(chan);
}

void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
@@ -1332,7 +1332,7 @@ int l2cap_ertm_send(struct l2cap_chan *chan)

l2cap_do_send(chan, tx_skb);

- __mod_retrans_timer();
+ __set_retrans_timer(chan);

bt_cb(skb)->tx_seq = chan->next_tx_seq;
chan->next_tx_seq = (chan->next_tx_seq + 1) % 64;
@@ -3249,8 +3249,8 @@ static int l2cap_try_push_rx_skb(struct l2cap_chan *chan)
l2cap_send_sframe(chan, control);
chan->retry_count = 1;

- del_timer(&chan->retrans_timer);
- __mod_monitor_timer();
+ __clear_retrans_timer(chan);
+ __set_monitor_timer(chan);

chan->conn_state |= L2CAP_CONN_WAIT_F;

@@ -3341,7 +3341,7 @@ static int l2cap_push_rx_skb(struct l2cap_chan *chan, struct sk_buff *skb, u16 c

chan->conn_state |= L2CAP_CONN_RNR_SENT;

- del_timer(&chan->ack_timer);
+ __clear_ack_timer(chan);

queue_work(_busy_wq, &chan->busy_work);

@@ -3510,9 +3510,9 @@ static inline int l2cap_data_channel_iframe(struct l2cap_chan *chan, u16 rx_cont

if (L2CAP_CTRL_FINAL & rx_control &&
chan->conn_state & L2CAP_CONN_WAIT_F) {
- del_timer(&chan->monitor_timer);
+ __clear_monitor_timer(chan);
if (chan->unacked_frames > 0)
- __mod_retrans_timer();
+ __set_retrans_timer(chan);
chan->conn_state &= ~L2CAP_CONN_WAIT_F;
}

@@ -3593,7 +3593,7 @@ static inline int l2cap_data_channel_iframe(struct l2cap_chan *chan, u16 rx_cont

l2cap_send_srejframe(chan, tx_seq);

- del_timer(&chan->ack_timer);
+ __clear_ack_timer(chan);
}
return 0;

@@ -3618,7 +3618,7 @@ expected:
l2cap_retransmit_frames(chan);
}

- __mod_ack_timer();
+ __set_ack_timer(chan);

chan->num_acked = (chan->num_acked + 1) % num_to_ack;
if (chan->num_acked == num_to_ack - 1)
@@ -3644,7 +3644,7 @@ static inline void l2cap_data_channel_rrframe(struct l2cap_chan *chan, u16 rx_co
if (chan->conn_state & L2CAP_CONN_SREJ_SENT) {
if ((chan->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
(chan->unacked_frames > 0))
- __mod_retrans_timer();
+ __set_retrans_timer(chan);

chan->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
l2cap_send_srejtail(chan);
@@ -3663,7 +3663,7 @@ static inline void l2cap_data_channel_rrframe(struct l2cap_chan *chan, u16 rx_co
} else {
if ((chan->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
(chan->unacked_frames > 0))
- __mod_retrans_timer();
+ __set_retrans_timer(chan);

chan->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
if (chan->conn_state & L2CAP_CONN_SREJ_SENT)
@@ -3746,7 +3746,7 @@ static inline void l2cap_data_channel_rnrframe(struct l2cap_chan *chan, u16 rx_c
chan->conn_state |= L2CAP_CONN_SEND_FBIT;

if (!(chan->conn_state & L2CAP_CONN_SREJ_SENT)) {
- del_timer(&chan->retrans_timer);
+ __clear_retrans_timer(chan);
if (rx_control & L2CAP_CTRL_POLL)
l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_FINAL);
return;
@@ -3764,9 +3764,9 @@ static inline int l2cap_data_channel_sframe(struct l2cap_chan *chan, u16 rx_cont

if (L2CAP_CTRL_FINAL & rx_control &&
chan->conn_state & L2CAP_CONN_WAIT_F) {
- del_timer(&chan->monitor_timer);
+ __clear_monitor_timer(chan);
if (chan->unacked_frames > 0)
- __mod_retrans_timer();
+ __set_retrans_timer(chan);
chan->conn_state &= ~L2CAP_CONN_WAIT_F;
}

--
1.7.5.1


2011-06-02 16:15:28

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 5/6] Bluetooth: Make timer functions generic

We now plan to use l2cap_set_timer and l2cap_clear_timer in ERTM timers.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 +
net/bluetooth/l2cap_core.c | 59 +++++++++++++++++++++--------------------
2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c620a92..e3e58f9 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -435,6 +435,8 @@ struct l2cap_pinfo {
#define L2CAP_CONN_RNR_SENT 0x0200
#define L2CAP_CONN_SAR_RETRY 0x0400

+#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
+#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
#define __mod_retrans_timer() mod_timer(&chan->retrans_timer, \
jiffies + msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
#define __mod_monitor_timer() mod_timer(&chan->monitor_timer, \
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index dd8da43..8586077 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -220,19 +220,19 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn)
return 0;
}

-static void l2cap_chan_set_timer(struct l2cap_chan *chan, long timeout)
+static void l2cap_set_timer(struct l2cap_chan *chan, struct timer_list *timer, long timeout)
{
BT_DBG("chan %p state %d timeout %ld", chan->sk, chan->sk->sk_state,
timeout);
- if (!mod_timer(&chan->chan_timer, jiffies + timeout))
+ if (!mod_timer(timer, jiffies + timeout))
chan_hold(chan);
}

-static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
+static void l2cap_clear_timer(struct l2cap_chan *chan, struct timer_list *timer)
{
BT_DBG("chan %p state %d", chan, chan->sk->sk_state);

- if (timer_pending(&chan->chan_timer) && del_timer(&chan->chan_timer))
+ if (timer_pending(timer) && del_timer(timer))
chan_put(chan);
}

@@ -248,7 +248,7 @@ static void l2cap_chan_timeout(unsigned long arg)

if (sock_owned_by_user(sk)) {
/* sk is owned by user. Try again later */
- l2cap_chan_set_timer(chan, HZ / 5);
+ __set_chan_timer(chan, HZ / 5);
bh_unlock_sock(sk);
chan_put(chan);
return;
@@ -345,7 +345,7 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
struct l2cap_conn *conn = chan->conn;
struct sock *parent = bt_sk(sk)->parent;

- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);

BT_DBG("chan %p, conn %p, err %d", chan, conn, err);

@@ -403,7 +403,8 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
/* Close not yet accepted channels */
while ((sk = bt_accept_dequeue(parent, NULL))) {
struct l2cap_chan *chan = l2cap_pi(sk)->chan;
- l2cap_chan_clear_timer(chan);
+
+ __clear_chan_timer(chan);
lock_sock(sk);
l2cap_chan_close(chan, ECONNRESET);
release_sock(sk);
@@ -430,8 +431,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
case BT_CONFIG:
if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED &&
conn->hcon->type == ACL_LINK) {
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, sk->sk_sndtimeo);
l2cap_send_disconn_req(conn, chan, reason);
} else
l2cap_chan_del(chan, reason);
@@ -866,7 +867,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)

__l2cap_chan_add(conn, chan);

- l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+ __set_chan_timer(chan, sk->sk_sndtimeo);

sk->sk_state = BT_CONNECTED;
parent->sk_data_ready(parent, 0);
@@ -894,13 +895,13 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
bh_lock_sock(sk);

if (conn->hcon->type == LE_LINK) {
- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
}

if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
} else if (sk->sk_state == BT_CONNECT)
@@ -1099,11 +1100,11 @@ int l2cap_chan_connect(struct l2cap_chan *chan)
l2cap_chan_add(conn, chan);

sk->sk_state = BT_CONNECT;
- l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+ __set_chan_timer(chan, sk->sk_sndtimeo);

if (hcon->state == BT_CONNECTED) {
if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);
if (l2cap_check_security(chan))
sk->sk_state = BT_CONNECTED;
} else
@@ -1667,7 +1668,7 @@ static void l2cap_chan_ready(struct sock *sk)
BT_DBG("sk %p, parent %p", sk, parent);

chan->conf_state = 0;
- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);

if (!parent) {
/* Outgoing channel.
@@ -2362,7 +2363,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd

dcid = chan->scid;

- l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+ __set_chan_timer(chan, sk->sk_sndtimeo);

chan->ident = cmd->ident;

@@ -2479,8 +2480,8 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
/* don't delete l2cap channel if sk is owned by user */
if (sock_owned_by_user(sk)) {
sk->sk_state = BT_DISCONN;
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, HZ / 5);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, HZ / 5);
break;
}

@@ -2653,7 +2654,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr

default:
sk->sk_err = ECONNRESET;
- l2cap_chan_set_timer(chan, HZ * 5);
+ __set_chan_timer(chan, HZ * 5);
l2cap_send_disconn_req(conn, chan, ECONNRESET);
goto done;
}
@@ -2709,8 +2710,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
/* don't delete l2cap channel if sk is owned by user */
if (sock_owned_by_user(sk)) {
sk->sk_state = BT_DISCONN;
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, HZ / 5);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, HZ / 5);
bh_unlock_sock(sk);
return 0;
}
@@ -2743,8 +2744,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
/* don't delete l2cap channel if sk is owned by user */
if (sock_owned_by_user(sk)) {
sk->sk_state = BT_DISCONN;
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, HZ / 5);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, HZ / 5);
bh_unlock_sock(sk);
return 0;
}
@@ -4140,13 +4141,13 @@ static inline void l2cap_check_encryption(struct l2cap_chan *chan, u8 encrypt)

if (encrypt == 0x00) {
if (chan->sec_level == BT_SECURITY_MEDIUM) {
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, HZ * 5);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, HZ * 5);
} else if (chan->sec_level == BT_SECURITY_HIGH)
l2cap_chan_close(chan, ECONNREFUSED);
} else {
if (chan->sec_level == BT_SECURITY_MEDIUM)
- l2cap_chan_clear_timer(chan);
+ __clear_chan_timer(chan);
}
}

@@ -4191,8 +4192,8 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
l2cap_send_cmd(conn, chan->ident,
L2CAP_CONN_REQ, sizeof(req), &req);
} else {
- l2cap_chan_clear_timer(chan);
- l2cap_chan_set_timer(chan, HZ / 10);
+ __clear_chan_timer(chan);
+ __set_chan_timer(chan, HZ / 10);
}
} else if (sk->sk_state == BT_CONNECT2) {
struct l2cap_conn_rsp rsp;
@@ -4203,7 +4204,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
result = L2CAP_CR_SUCCESS;
} else {
sk->sk_state = BT_DISCONN;
- l2cap_chan_set_timer(chan, HZ / 10);
+ __set_chan_timer(chan, HZ / 10);
result = L2CAP_CR_SEC_BLOCK;
}

--
1.7.5.1


2011-06-02 16:15:27

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan

struct l2cap_chan has now its own refcnt that is compatible with the
socket refcnt, i.e., we won't see sk_refcnt = 0 and chan->refcnt > 0.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/l2cap_core.c | 29 ++++++++++++++++++++---------
2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index af2f171..c620a92 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -287,6 +287,8 @@ struct l2cap_chan {

struct l2cap_conn *conn;

+ atomic_t refcnt;
+
__le16 psm;
__u16 dcid;
__u16 scid;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c645e03..dd8da43 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -78,6 +78,18 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);

/* ---- L2CAP channels ---- */
+
+static inline void chan_hold(struct l2cap_chan *c)
+{
+ atomic_inc(&c->refcnt);
+}
+
+static inline void chan_put(struct l2cap_chan *c)
+{
+ if (atomic_dec_and_test(&c->refcnt))
+ kfree(c);
+}
+
static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
{
struct l2cap_chan *c;
@@ -213,7 +225,7 @@ static void l2cap_chan_set_timer(struct l2cap_chan *chan, long timeout)
BT_DBG("chan %p state %d timeout %ld", chan->sk, chan->sk->sk_state,
timeout);
if (!mod_timer(&chan->chan_timer, jiffies + timeout))
- sock_hold(chan->sk);
+ chan_hold(chan);
}

static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
@@ -221,7 +233,7 @@ static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
BT_DBG("chan %p state %d", chan, chan->sk->sk_state);

if (timer_pending(&chan->chan_timer) && del_timer(&chan->chan_timer))
- __sock_put(chan->sk);
+ chan_put(chan);
}

static void l2cap_chan_timeout(unsigned long arg)
@@ -238,7 +250,7 @@ static void l2cap_chan_timeout(unsigned long arg)
/* sk is owned by user. Try again later */
l2cap_chan_set_timer(chan, HZ / 5);
bh_unlock_sock(sk);
- sock_put(sk);
+ chan_put(chan);
return;
}

@@ -255,7 +267,7 @@ static void l2cap_chan_timeout(unsigned long arg)
bh_unlock_sock(sk);

chan->ops->close(chan->ops->data);
- sock_put(sk);
+ chan_put(chan);
}

struct l2cap_chan *l2cap_chan_create(struct sock *sk)
@@ -274,6 +286,8 @@ struct l2cap_chan *l2cap_chan_create(struct sock *sk)

setup_timer(&chan->chan_timer, l2cap_chan_timeout, (unsigned long) chan);

+ atomic_set(&chan->refcnt, 1);
+
return chan;
}

@@ -283,13 +297,11 @@ void l2cap_chan_destroy(struct l2cap_chan *chan)
list_del(&chan->global_l);
write_unlock_bh(&chan_list_lock);

- kfree(chan);
+ chan_put(chan);
}

static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
{
- struct sock *sk = chan->sk;
-
BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn,
chan->psm, chan->dcid);

@@ -320,7 +332,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
chan->omtu = L2CAP_DEFAULT_MTU;
}

- sock_hold(sk);
+ chan_hold(chan);

list_add(&chan->list, &conn->chan_l);
}
@@ -342,7 +354,6 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
write_lock_bh(&conn->chan_lock);
list_del(&chan->list);
write_unlock_bh(&conn->chan_lock);
- __sock_put(sk);

chan->conn = NULL;
hci_conn_put(conn->hcon);
--
1.7.5.1


2011-06-02 16:15:26

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops

close() calls l2cap_sock_kill() on l2cap_sock.c

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 3 +--
net/bluetooth/l2cap_core.c | 17 +++++++++--------
net/bluetooth/l2cap_sock.c | 10 +++++++++-
3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e625f09..af2f171 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -363,6 +363,7 @@ struct l2cap_ops {

struct l2cap_chan *(*new_connection) (void *data);
int (*recv) (void *data, struct sk_buff *skb);
+ void (*close) (void *data);
};

struct l2cap_conn {
@@ -468,8 +469,6 @@ int __l2cap_wait_ack(struct sock *sk);
int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm);
int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid);

-void l2cap_sock_kill(struct sock *sk);
-
struct l2cap_chan *l2cap_chan_create(struct sock *sk);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
void l2cap_chan_destroy(struct l2cap_chan *chan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b5327f3..c645e03 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -254,7 +254,7 @@ static void l2cap_chan_timeout(unsigned long arg)

bh_unlock_sock(sk);

- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
sock_put(sk);
}

@@ -391,11 +391,12 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)

/* Close not yet accepted channels */
while ((sk = bt_accept_dequeue(parent, NULL))) {
- l2cap_chan_clear_timer(l2cap_pi(sk)->chan);
+ struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+ l2cap_chan_clear_timer(chan);
lock_sock(sk);
- l2cap_chan_close(l2cap_pi(sk)->chan, ECONNRESET);
+ l2cap_chan_close(chan, ECONNRESET);
release_sock(sk);
- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
}

parent->sk_state = BT_CLOSED;
@@ -988,7 +989,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
bh_lock_sock(sk);
l2cap_chan_del(chan, err);
bh_unlock_sock(sk);
- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
}

if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
@@ -2333,7 +2334,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
if (__l2cap_get_chan_by_dcid(conn, scid)) {
write_unlock_bh(&conn->chan_lock);
sock_set_flag(sk, SOCK_ZAPPED);
- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
goto response;
}

@@ -2706,7 +2707,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
l2cap_chan_del(chan, ECONNRESET);
bh_unlock_sock(sk);

- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
return 0;
}

@@ -2740,7 +2741,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
l2cap_chan_del(chan, 0);
bh_unlock_sock(sk);

- l2cap_sock_kill(sk);
+ chan->ops->close(chan->ops->data);
return 0;
}

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index ba6d4f1..23cf902 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
/* Kill socket (only if zapped and orphan)
* Must be called on unlocked socket.
*/
-void l2cap_sock_kill(struct sock *sk)
+static void l2cap_sock_kill(struct sock *sk)
{
if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
return;
@@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
return sock_queue_rcv_skb(sk, skb);
}

+static void l2cap_chan_close_cb(void *data)
+{
+ struct sock *sk = data;
+
+ l2cap_sock_kill(sk);
+}
+
static struct l2cap_ops l2cap_chan_ops = {
.name = "L2CAP Socket Interface",
.new_connection = l2cap_chan_new_connection_cb,
.recv = l2cap_chan_recv_cb,
+ .close = l2cap_chan_close_cb,
};

static void l2cap_sock_destruct(struct sock *sk)
--
1.7.5.1


2011-06-02 16:15:25

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 2/6] Bluetooth: add recv() callback to l2cap_chan_ops

This abstracts the call to sock_queue_recv_skb() into
l2cap_chan_ops->recv().

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/l2cap_core.c | 16 ++++++++--------
net/bluetooth/l2cap_sock.c | 8 ++++++++
3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 95e44df..e625f09 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -362,6 +362,7 @@ struct l2cap_ops {
void *data;

struct l2cap_chan *(*new_connection) (void *data);
+ int (*recv) (void *data, struct sk_buff *skb);
};

struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6849e74..b5327f3 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1692,7 +1692,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
if (!nskb)
continue;

- if (sock_queue_rcv_skb(sk, nskb))
+ if (chan->ops->recv(chan->ops->data, nskb))
kfree_skb(nskb);
}
read_unlock(&conn->chan_lock);
@@ -3118,7 +3118,7 @@ static int l2cap_ertm_reassembly_sdu(struct l2cap_chan *chan, struct sk_buff *sk
if (chan->conn_state & L2CAP_CONN_SAR_SDU)
goto drop;

- return sock_queue_rcv_skb(chan->sk, skb);
+ return chan->ops->recv(chan->ops->data, skb);

case L2CAP_SDU_START:
if (chan->conn_state & L2CAP_CONN_SAR_SDU)
@@ -3184,7 +3184,7 @@ static int l2cap_ertm_reassembly_sdu(struct l2cap_chan *chan, struct sk_buff *sk
return -ENOMEM;
}

- err = sock_queue_rcv_skb(chan->sk, _skb);
+ err = chan->ops->recv(chan->ops->data, _skb);
if (err < 0) {
kfree_skb(_skb);
chan->conn_state |= L2CAP_CONN_SAR_RETRY;
@@ -3352,7 +3352,7 @@ static int l2cap_streaming_reassembly_sdu(struct l2cap_chan *chan, struct sk_buf
break;
}

- err = sock_queue_rcv_skb(chan->sk, skb);
+ err = chan->ops->recv(chan->ops->data, skb);
if (!err)
return 0;

@@ -3413,7 +3413,7 @@ static int l2cap_streaming_reassembly_sdu(struct l2cap_chan *chan, struct sk_buf

if (chan->partial_sdu_len == chan->sdu_len) {
_skb = skb_clone(chan->sdu, GFP_ATOMIC);
- err = sock_queue_rcv_skb(chan->sk, _skb);
+ err = chan->ops->recv(chan->ops->data, _skb);
if (err < 0)
kfree_skb(_skb);
}
@@ -3880,7 +3880,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (chan->imtu < skb->len)
goto drop;

- if (!sock_queue_rcv_skb(sk, skb))
+ if (!chan->ops->recv(chan->ops->data, skb))
goto done;
break;

@@ -3958,7 +3958,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
if (l2cap_pi(sk)->chan->imtu < skb->len)
goto drop;

- if (!sock_queue_rcv_skb(sk, skb))
+ if (!chan->ops->recv(chan->ops->data, skb))
goto done;

drop:
@@ -3991,7 +3991,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, __le16 cid, struct
if (l2cap_pi(sk)->chan->imtu < skb->len)
goto drop;

- if (!sock_queue_rcv_skb(sk, skb))
+ if (!chan->ops->recv(chan->ops->data, skb))
goto done;

drop:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 79926a4..ba6d4f1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -755,9 +755,17 @@ static struct l2cap_chan *l2cap_chan_new_connection_cb(void *data)
return l2cap_pi(sk)->chan;
}

+static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
+{
+ struct sock *sk = data;
+
+ return sock_queue_rcv_skb(sk, skb);
+}
+
static struct l2cap_ops l2cap_chan_ops = {
.name = "L2CAP Socket Interface",
.new_connection = l2cap_chan_new_connection_cb,
+ .recv = l2cap_chan_recv_cb,
};

static void l2cap_sock_destruct(struct sock *sk)
--
1.7.5.1