From: Gustavo Padovan <[email protected]>
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
Hi Marcel,
* Marcel Holtmann <[email protected]> [2012-05-28 05:48:48 +0200]:
> Hi Gustavo,
>
> > > > Remove another socket usage from l2cap_core.c
> > > >
> > > > Signed-off-by: Gustavo Padovan <[email protected]>
> > > > ---
> > > > 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
Hi Marcel and Gustavo,
On Sun, May 27, 2012 at 07:00:28AM +0200, Marcel Holtmann wrote:
> Hi Gustavo,
>
> > From: Gustavo Padovan <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > Signed-off-by: Gustavo Padovan <[email protected]>
>
> 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
Hi Gustavo,
> > > Remove another socket usage from l2cap_core.c
> > >
> > > Signed-off-by: Gustavo Padovan <[email protected]>
> > > ---
> > > 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
Hi Marcel,
* Marcel Holtmann <[email protected]> [2012-05-27 07:07:15 +0200]:
> Hi Gustavo,
>
> > Remove another socket usage from l2cap_core.c
> >
> > Signed-off-by: Gustavo Padovan <[email protected]>
> > ---
> > 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
Hi Gustavo,
> > On Fri, May 25, 2012 at 09:31:04AM -0300, Gustavo Padovan wrote:
> > > From: Gustavo Padovan <[email protected]>
> > >
> > > This keep l2cap chan ops functions parameters in sync.
> > >
> > > Signed-off-by: Gustavo Padovan <[email protected]>
> >
> > ...
> >
> > > 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
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 <[email protected]>
> Acked-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
Hi Gustavo,
> When DEFER_SETUP is set defer() will trigger an authorization request
> to the userspace.
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> ---
> 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
Hi Gustavo,
> Remove another socket usage from l2cap_core.c
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> ---
> 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
Hi Gustavo,
> These vars are kept in sync so we can use chan->state here.
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> ---
> 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
Hi Gustavo,
> This replace code in l2cap_le_conn_ready() by a similar code in
> l2cap_chan_ready().
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
Hi Gustavo,
> From: Gustavo Padovan <[email protected]>
>
> This move socket specific code to l2cap_sock.c.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Signed-off-by: Gustavo Padovan <[email protected]>
same here. If this is Andrei's patch, then he deserves the author line.
Regards
Marcel
Hi Gustavo,
> From: Gustavo Padovan <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Signed-off-by: Gustavo Padovan <[email protected]>
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
Hi Gustavo,
> This is already performed inside l2cap_chan_ready(), so we don't need it
> here again.
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> Acked-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 3 ---
> 1 file changed, 3 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
On Fri, 25 May 2012, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [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 <[email protected]>
> >
> > This keep l2cap chan ops functions parameters in sync.
> >
> > Signed-off-by: Gustavo Padovan <[email protected]>
>
> ...
>
> > 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
Hi Gustavo,
On Fri, May 25, 2012 at 09:31:04AM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> This keep l2cap chan ops functions parameters in sync.
>
> Signed-off-by: Gustavo Padovan <[email protected]>
...
> 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
From: Gustavo Padovan <[email protected]>
This keep l2cap chan ops functions parameters in sync.
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
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 <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
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 <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
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 <[email protected]>
Acked-by: Andrei Emeltchenko <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
When DEFER_SETUP is set defer() will trigger an authorization request
to the userspace.
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
Remove another socket usage from l2cap_core.c
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
Remove socket specific code from l2cap_core.c
Signed-off-by: Gustavo Padovan <[email protected]>
Acked-by: Andrei Emeltchenko <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
These vars are kept in sync so we can use chan->state here.
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
This replace code in l2cap_le_conn_ready() by a similar code in
l2cap_chan_ready().
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
This move socket specific code to l2cap_sock.c.
Signed-off-by: Andrei Emeltchenko <[email protected]>
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
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 <[email protected]>
Signed-off-by: Andrei Emeltchenko <[email protected]>
Signed-off-by: Gustavo Padovan <[email protected]>
---
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
From: Gustavo Padovan <[email protected]>
This is already performed inside l2cap_chan_ready(), so we don't need it
here again.
Signed-off-by: Gustavo Padovan <[email protected]>
Acked-by: Andrei Emeltchenko <[email protected]>
---
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