From: Gustavo Padovan <[email protected]>
Hi,
This patchset does a bit more of clean up in l2cap core, it was inially based
on the two patches Andrei has sent to the list today.
Vinicus, you might want check patch 4/8 especifically.
Comments are welcome.
Gustavo Padovan (8):
Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c
Bluetooth: Remove extra l2cap_state_change(BT_CONNECTED)
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->authorize
include/net/bluetooth/l2cap.h | 4 ++
net/bluetooth/l2cap_core.c | 121 +++++++++--------------------------------
net/bluetooth/l2cap_sock.c | 111 ++++++++++++++++++++++++++++++++++++-
3 files changed, 139 insertions(+), 97 deletions(-)
--
1.7.10.1
Hi,
On Thu, May 24, 2012 at 10:18:07AM -0700, Mat Martineau wrote:
> >>On Thu, May 24, 2012 at 04:00:16AM -0300, Gustavo Padovan wrote:
> >>>- lock_sock(sk);
> >>>- __l2cap_state_change(chan, BT_DISCONN);
> >>>- __l2cap_chan_set_err(chan, err);
> >>>- release_sock(sk);
> >>>+ l2cap_state_change(chan, BT_DISCONN);
> >>>+ if(chan->ops->set_err)
> >>>+ chan->ops->set_err(chan->data, err);
> >>
> >>I do not know can it be done somehow better, currently we lock and unlock
> >>sockets for each operation.
> >
> >I'm having trouble to get your point, could you please explain?
>
> What I see is that the lock used to be acquired once, with both
> state_change and set_err happening while locked. With this change,
> the lock is acquired twice, once for state_change and once for
> set_err. I'm not sure it's good to release the lock between
> state_change and set_err.
Maybe we add parameter err to l2cap_state_change and if set set error.
Then we do not lock/unlock for each operation.
Best regards
Andrei Emeltchenko
Hi Andrei,
On 14:30 Thu 24 May, Andrei Emeltchenko wrote:
> Hi Ulisses,
>
> On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
> > >> > + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
> > >>
> > >> Again, why void *data ?
> > >
> > > I mean here that for fixed channels we do not need this function at this
> > > point since initialization is different.
> >
> > So? What do you mean? This needs to be generic, I think. It's an
> > abstraction after all.
>
> Fixed channels do not have configuration phase, they are created
> CONNECTED (at least A2MP).
You are right, but having this abstraction seems useful.
In LE, for example, we (ab)use the CONFIG phase for blocking the channel
while SMP is running (negotiating the encryption of the link), so any
applications are not able to send data in a lower security level than
what they requested.
>
> Best regards
> Andrei Emeltchenko
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
Gustavo -
On Thu, 24 May 2012, Andrei Emeltchenko wrote:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:36PM -0300, Gustavo Padovan wrote:
>> From: Gustavo Padovan <[email protected]>
>>
>> Hi,
>>
>> This patchset does a bit more of clean up in l2cap core, it was inially based
>> on the two patches Andrei has sent to the list today.
>
> Can this RFC be merged with other 2 RFC since otherwise I got lost.
>
> I also found that my mail reader indent your RFC in a strange way so I
> thought that they are in the same series.
I agree -- using "--chain-reply-to" has made this group of RFC patches
very difficult to read in a threaded email view. Would you consider
using --no-chain-reply-to with git send-email?
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Mat,
On Thu, May 24, 2012 at 2:48 PM, Mat Martineau <[email protected]> wrote:
>
> On Thu, 24 May 2012, Andrei Emeltchenko wrote:
>
>> Hi Ulisses,
>>
>> On Thu, May 24, 2012 at 08:31:15AM -0300, Ulisses Furquim wrote:
>>>
>>> Hi Andrei,
>>>
>>> On Thu, May 24, 2012 at 8:30 AM, Andrei Emeltchenko
>>> <[email protected]> wrote:
>>>>
>>>> Hi Ulisses,
>>>>
>>>> On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
>>>>>>>>
>>>>>>>> + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
>>>>>>>
>>>>>>>
>>>>>>> Again, why void *data ?
>>>>>>
>>>>>>
>>>>>> I mean here that for fixed channels we do not need this function at
>>>>>> this
>>>>>> point since initialization is different.
>>>>>
>>>>>
>>>>> So? What do you mean? This needs to be generic, I think. It's an
>>>>> abstraction after all.
>>>>
>>>>
>>>> Fixed channels do not have configuration phase, they are created
>>>> CONNECTED (at least A2MP).
>>>
>>>
>>> And your proposal is?
>>
>>
>> Fox fixed channels ready is not defined (at least now) so we can just use
>> exact type, see my patch in a minute.
>
>
> For l2cap_ops right now, every callback takes a void* except for alloc_skb.
> ?alloc_skb could get by with a void* by using l2cap_pi(sk)->chan.
IMO that should be changed to void*.
> In any case, I think we can agree that some consistency in l2cap_ops would
> be good! ?Either way will work because the void* can give the chan*, and
> with the chan* you have chan->data. ?Let's pick either void* or l2cap_chan*
> for the callbacks and stick with it.
Exactly. I'm ok with either void* or chan* for the same reason Mat
said. Let's have some consistency in l2cap_ops, please.
Best regards,
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
On Thu, 24 May 2012, Andrei Emeltchenko wrote:
> Hi Ulisses,
>
> On Thu, May 24, 2012 at 08:31:15AM -0300, Ulisses Furquim wrote:
>> Hi Andrei,
>>
>> On Thu, May 24, 2012 at 8:30 AM, Andrei Emeltchenko
>> <[email protected]> wrote:
>>> Hi Ulisses,
>>>
>>> On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
>>>>>>> + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
>>>>>>
>>>>>> Again, why void *data ?
>>>>>
>>>>> I mean here that for fixed channels we do not need this function at this
>>>>> point since initialization is different.
>>>>
>>>> So? What do you mean? This needs to be generic, I think. It's an
>>>> abstraction after all.
>>>
>>> Fixed channels do not have configuration phase, they are created
>>> CONNECTED (at least A2MP).
>>
>> And your proposal is?
>
> Fox fixed channels ready is not defined (at least now) so we can just use
> exact type, see my patch in a minute.
For l2cap_ops right now, every callback takes a void* except for
alloc_skb. alloc_skb could get by with a void* by using
l2cap_pi(sk)->chan.
In any case, I think we can agree that some consistency in l2cap_ops
would be good! Either way will work because the void* can give the
chan*, and with the chan* you have chan->data. Let's pick either
void* or l2cap_chan* for the callbacks and stick with it.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
On Thu, 24 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().
>
> 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 c9de4f5..5ff294f9 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);
> -
Can you explain in the commit message why it is ok to move
bt_accept_enqueue before the other initialization code in
l2cap_connect_req? It looks like some important setup happens there,
but maybe the parent socket lock keeps things serialized.
> __l2cap_chan_add(conn, chan);
>
> dcid = chan->scid;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index d2a81f1..f4c3e2c 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -915,6 +915,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
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi,
On 14:14 Thu 24 May, Gustavo Padovan wrote:
> * Gustavo Padovan <[email protected]> [2012-05-24 13:56:11 -0300]:
>
> > > >
> > > > +static void l2cap_sock_finalize_cb(void *data, int err)
> > > > +{
> > > > + struct sock *sk = data;
> > >
> > > Why we cannot just use l2cap_sock_finalize_cb(struct sock *sk, int err)
> > > what is the reason for extra assignment?
> >
> > Because we want to be generic here, we don't know who else will use finalize()
> > so we need to keep a void * here.
> >
> > >
> > > I also think finalize is not good name since it does not mean what is
> > > actually finalized.
> >
> > And it is not, I failed to come with a better name here. The initial one was
> > delete, even worse.
>
> Maybe we can go with prepare_finalize() or prepare_close() here.
That sounds better, there's "teardown" too.
>
> Gustavo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
Gustavo -
On Thu, 24 May 2012, Gustavo Padovan wrote:
> Hi Andrei,
>
> * Andrei Emeltchenko <[email protected]> [2012-05-24 11:26:04 +0300]:
>
>> Hi Gustavo,
>>
>> On Thu, May 24, 2012 at 04:00:16AM -0300, Gustavo Padovan wrote:
>>> - lock_sock(sk);
>>> - __l2cap_state_change(chan, BT_DISCONN);
>>> - __l2cap_chan_set_err(chan, err);
>>> - release_sock(sk);
>>> + l2cap_state_change(chan, BT_DISCONN);
>>> + if(chan->ops->set_err)
>>> + chan->ops->set_err(chan->data, err);
>>
>> I do not know can it be done somehow better, currently we lock and unlock
>> sockets for each operation.
>
> I'm having trouble to get your point, could you please explain?
What I see is that the lock used to be acquired once, with both
state_change and set_err happening while locked. With this change,
the lock is acquired twice, once for state_change and once for
set_err. I'm not sure it's good to release the lock between
state_change and set_err.
--
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-24 12:24:26 +0300]:
> Hi Gustavo,
>
> On Thu, May 24, 2012 at 03:02:53AM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > bt_accept_enqueue() can be easily placed at the end of
> > l2cap_sock_new_connection_cb().
> >
> > 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 c9de4f5..5ff294f9 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);
> > -
>
> Shall we also move to l2cap_sock_new_connection_cb bacpys above?
I don't think so, bacpy and bacmp are used everywhere, they need to fixed
differently.
Gustavo
* Gustavo Padovan <[email protected]> [2012-05-24 13:56:11 -0300]:
> > >
> > > +static void l2cap_sock_finalize_cb(void *data, int err)
> > > +{
> > > + struct sock *sk = data;
> >
> > Why we cannot just use l2cap_sock_finalize_cb(struct sock *sk, int err)
> > what is the reason for extra assignment?
>
> Because we want to be generic here, we don't know who else will use finalize()
> so we need to keep a void * here.
>
> >
> > I also think finalize is not good name since it does not mean what is
> > actually finalized.
>
> And it is not, I failed to come with a better name here. The initial one was
> delete, even worse.
Maybe we can go with prepare_finalize() or prepare_close() here.
Gustavo
Gustavo -
On Wed, 23 May 2012, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> When DEFER_SETUP is set authorize() 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 4f81d08..94e375a 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -531,6 +531,7 @@ struct l2cap_ops {
> struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan,
> unsigned long len, int nb);
> void (*ready) (void *data);
> + void (*authorize) (void *data);
> };
I'm trying to think of a name more specific to the deferred connection
feature than "authorize"... There's a lot of use of "auth" to refer
to "authentication", so it could be confusing to throw "authorize" in
to the mix.
"deferred_connect"?
"pending_connection"?
"defer"?
>
> struct l2cap_conn {
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 1305b3b..22ba699 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 = cpu_to_le16(L2CAP_CR_PEND);
> rsp.status = cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> - if (parent)
> - parent->sk_data_ready(parent, 0);
> -
> + if(chan->ops->authorize)
> + chan->ops->authorize(chan->data);
> } else {
> __l2cap_state_change(chan, BT_CONFIG);
> rsp.result = 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->authorize)
> + chan->ops->authorize(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->authorize)
> + chan->ops->authorize(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 dac7b2c..d2a81f1 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1047,6 +1047,17 @@ static void l2cap_sock_ready_cb(void *data)
> release_sock(sk);
> }
>
> +static void l2cap_sock_authorize_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,
> @@ -1056,6 +1067,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,
> + .authorize = l2cap_sock_authorize_cb,
> };
>
> static void l2cap_sock_destruct(struct sock *sk)
> --
> 1.7.10.1
--
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-24 12:55:53 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:44PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > When DEFER_SETUP is set authorize() will trigger an authorization request
> > to the userspace.
>
> Looks good.
>
> > +static void l2cap_sock_authorize_cb(void *data)
> > +{
> > + struct sock *sk = data;
> > + struct sock *parent;
> > +
> > + parent = bt_sk(sk)->parent;
>
> But why not:
>
> l2cap_sock_authorize_cb(struct sock *sk)
> {
> struct sock *parent = bt_sk(sk)->parent;
We need to be generic, I said that in the other patch already. The chan->sk
should disappear and the only thing l2cap_core.c will see is chan->data void *
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 11:30:45 +0300]:
> Hi Gustavo,
>
> On Thu, May 24, 2012 at 04:00:17AM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > If the is not oriented we should call sk_state_change() just after set the
> > channel state to BT_CONNECTED.
> >
> > Signed-off-by: Gustavo Padovan <[email protected]>
> > ---
> > net/bluetooth/l2cap_core.c | 6 +-----
> > net/bluetooth/l2cap_sock.c | 4 ++++
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 0d22695..6137b48 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -1176,12 +1176,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> > l2cap_chan_ready(chan);
> >
> > } else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
> > - struct sock *sk = chan->sk;
> > __clear_chan_timer(chan);
> > - lock_sock(sk);
> > - __l2cap_state_change(chan, BT_CONNECTED);
> > - sk->sk_state_change(sk);
> > - release_sock(sk);
> > + l2cap_state_change(chan, BT_CONNECTED);
> >
> > } else if (chan->state == BT_CONNECT)
> > l2cap_do_start(chan);
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 0a1a827..9fd9412 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1018,6 +1018,10 @@ static void l2cap_sock_state_change_cb(void *data, int state)
> > struct sock *sk = data;
> >
> > sk->sk_state = state;
> > +
> > + if (state == BT_CONNECTED &&
> > + l2cap_pi(sk)->chan->chan_type != L2CAP_CHAN_CONN_ORIENTED)
> > + sk->sk_state_change(sk);
>
> Somehow I feel that there is too much logic here.
Yes, there is, I failed to find something better.
>BTW: can we use
> sk_state_change for every state_change callback?
No, it would be a bit expensive call sk_state_change() everytime here.
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 11:26:04 +0300]:
> Hi Gustavo,
>
> On Thu, May 24, 2012 at 04:00:16AM -0300, Gustavo Padovan wrote:
> > - lock_sock(sk);
> > - __l2cap_state_change(chan, BT_DISCONN);
> > - __l2cap_chan_set_err(chan, err);
> > - release_sock(sk);
> > + l2cap_state_change(chan, BT_DISCONN);
> > + if(chan->ops->set_err)
> > + chan->ops->set_err(chan->data, err);
>
> I do not know can it be done somehow better, currently we lock and unlock
> sockets for each operation.
I'm having trouble to get your point, could you please explain?
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 12:50:06 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:43PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > Remove another socket usage from l2cap_core.c
>
> Good idea.
>
> ...
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -623,10 +623,15 @@ 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);
>
> Do we now have 2 similar flags?
Yes, and there is no other way to do this, BT_SK_DEFER_SETUP is for socket
related usage (mostly in af_bluetooth.c) and CONF_DEFER_SETUP is for
l2cap_core.c
>
> > + }
> > +
> > +
>
> Remove extra line
Sure.
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 12:45:48 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:42PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > Remove socket specific code from l2cap_core.c
> >
> > Signed-off-by: Gustavo Padovan <[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 9416c60..46e6fb7 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 8a3620b..bc409a2 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;
> > + }
> > +
>
> Ok I see where comes from this change. I feel that this can be merged with
> some other patch removing parent sk.
Let keep this patch as is, the other patch changes locking and I prefer a
separated patch for it.
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 12:35:59 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:37PM -0300, Gustavo Padovan wrote:
> > 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->finalize)
> > + chan->ops->finalize(chan->data, 0);
>
> Looks like you are handling all cases so this can be put outside of
> switch.
No, I'm not, look to the BT_CONNECTED/BT_CONFIG option.
>
> > 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->finalize)
> > + chan->ops->finalize(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->finalize)
> > + chan->ops->finalize(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 4d36605..0302cb4 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_finalize_cb(void *data, int err)
> > +{
> > + struct sock *sk = data;
>
> Why we cannot just use l2cap_sock_finalize_cb(struct sock *sk, int err)
> what is the reason for extra assignment?
Because we want to be generic here, we don't know who else will use finalize()
so we need to keep a void * here.
>
> I also think finalize is not good name since it does not mean what is
> actually finalized.
And it is not, I failed to come with a better name here. The initial one was
delete, even worse.
Gustavo
Hi Andrei,
* Andrei Emeltchenko <[email protected]> [2012-05-24 13:02:29 +0300]:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:36PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > Hi,
> >
> > This patchset does a bit more of clean up in l2cap core, it was inially based
> > on the two patches Andrei has sent to the list today.
>
> Can this RFC be merged with other 2 RFC since otherwise I got lost.
>
> I also found that my mail reader indent your RFC in a strange way so I
> thought that they are in the same series.
And actually they the same series, I just thought twice that I was done with
this patchset, but then I just found more work to do.
Gustavo
Hi Ulisses,
On Thu, May 24, 2012 at 08:31:15AM -0300, Ulisses Furquim wrote:
> Hi Andrei,
>
> On Thu, May 24, 2012 at 8:30 AM, Andrei Emeltchenko
> <[email protected]> wrote:
> > Hi Ulisses,
> >
> > On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
> >> >> > + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
> >> >>
> >> >> Again, why void *data ?
> >> >
> >> > I mean here that for fixed channels we do not need this function at this
> >> > point since initialization is different.
> >>
> >> So? What do you mean? This needs to be generic, I think. It's an
> >> abstraction after all.
> >
> > Fixed channels do not have configuration phase, they are created
> > CONNECTED (at least A2MP).
>
> And your proposal is?
Fox fixed channels ready is not defined (at least now) so we can just use
exact type, see my patch in a minute.
Best regards
Andrei Emeltchenko
Hi Andrei,
On Thu, May 24, 2012 at 8:30 AM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi Ulisses,
>
> On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
>> >> > + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
>> >>
>> >> Again, why void *data ?
>> >
>> > I mean here that for fixed channels we do not need this function at this
>> > point since initialization is different.
>>
>> So? What do you mean? This needs to be generic, I think. It's an
>> abstraction after all.
>
> Fixed channels do not have configuration phase, they are created
> CONNECTED (at least A2MP).
And your proposal is?
Regards,
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
Hi Ulisses,
On Thu, May 24, 2012 at 08:17:23AM -0300, Ulisses Furquim wrote:
> >> > + ? void ? ? ? ? ? ? ? ? ? ?(*ready) (void *data);
> >>
> >> Again, why void *data ?
> >
> > I mean here that for fixed channels we do not need this function at this
> > point since initialization is different.
>
> So? What do you mean? This needs to be generic, I think. It's an
> abstraction after all.
Fixed channels do not have configuration phase, they are created
CONNECTED (at least A2MP).
Best regards
Andrei Emeltchenko
Hi Andrei,
On Thu, May 24, 2012 at 7:23 AM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi,
>
> On Thu, May 24, 2012 at 12:39:59PM +0300, Andrei Emeltchenko wrote:
>> Hi Gustavo,
>>
>> On Wed, May 23, 2012 at 10:12:39PM -0300, Gustavo Padovan wrote:
>> > 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 29f7b06..d1f6819 100644
>> > --- a/include/net/bluetooth/l2cap.h
>> > +++ b/include/net/bluetooth/l2cap.h
>> > @@ -530,6 +530,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);
>>
>> Again, why void *data ?
>
> I mean here that for fixed channels we do not need this function at this
> point since initialization is different.
So? What do you mean? This needs to be generic, I think. It's an
abstraction after all.
Regards,
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
Hi,
On Thu, May 24, 2012 at 12:39:59PM +0300, Andrei Emeltchenko wrote:
> Hi Gustavo,
>
> On Wed, May 23, 2012 at 10:12:39PM -0300, Gustavo Padovan wrote:
> > 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 29f7b06..d1f6819 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -530,6 +530,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);
>
> Again, why void *data ?
I mean here that for fixed channels we do not need this function at this
point since initialization is different.
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:36PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> Hi,
>
> This patchset does a bit more of clean up in l2cap core, it was inially based
> on the two patches Andrei has sent to the list today.
Can this RFC be merged with other 2 RFC since otherwise I got lost.
I also found that my mail reader indent your RFC in a strange way so I
thought that they are in the same series.
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:44PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> When DEFER_SETUP is set authorize() will trigger an authorization request
> to the userspace.
Looks good.
> +static void l2cap_sock_authorize_cb(void *data)
> +{
> + struct sock *sk = data;
> + struct sock *parent;
> +
> + parent = bt_sk(sk)->parent;
But why not:
l2cap_sock_authorize_cb(struct sock *sk)
{
struct sock *parent = bt_sk(sk)->parent;
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:43PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> Remove another socket usage from l2cap_core.c
Good idea.
...
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -623,10 +623,15 @@ 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);
Do we now have 2 similar flags?
> + }
> +
> +
Remove extra line
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:42PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> Remove socket specific code from l2cap_core.c
>
> Signed-off-by: Gustavo Padovan <[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 9416c60..46e6fb7 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 8a3620b..bc409a2 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;
> + }
> +
Ok I see where comes from this change. I feel that this can be merged with
some other patch removing parent sk.
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:39PM -0300, Gustavo Padovan wrote:
> 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 29f7b06..d1f6819 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -530,6 +530,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);
Again, why void *data ?
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:38PM -0300, Gustavo Padovan wrote:
> 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 e7a598c..f77b134 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3608,8 +3608,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);
> @@ -3740,7 +3738,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Gustavo,
On Wed, May 23, 2012 at 10:12:37PM -0300, Gustavo Padovan wrote:
> 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->finalize)
> + chan->ops->finalize(chan->data, 0);
Looks like you are handling all cases so this can be put outside of
switch.
> 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->finalize)
> + chan->ops->finalize(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->finalize)
> + chan->ops->finalize(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 4d36605..0302cb4 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_finalize_cb(void *data, int err)
> +{
> + struct sock *sk = data;
Why we cannot just use l2cap_sock_finalize_cb(struct sock *sk, int err)
what is the reason for extra assignment?
I also think finalize is not good name since it does not mean what is
actually finalized.
Best regards
Andrei Emeltchenko
> + 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);
> +}
> +
Hi Gustavo,
On Thu, May 24, 2012 at 03:02:52AM -0300, Gustavo Padovan wrote:
> 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]>
> ---
> 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 22ba699..c9de4f5 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->finalize)
> - chan->ops->finalize(chan->data, 0);
What is finalize? Is this the first patch? We do not have finalize yet.
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Thu, May 24, 2012 at 03:02:53AM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> bt_accept_enqueue() can be easily placed at the end of
> l2cap_sock_new_connection_cb().
>
> 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 c9de4f5..5ff294f9 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);
> -
Shall we also move to l2cap_sock_new_connection_cb bacpys above?
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Thu, May 24, 2012 at 03:02:54AM -0300, Gustavo Padovan wrote:
> @@ -900,16 +900,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;
> }
You seems to forget to post patch adding sk_acceptq_is_full to
l2cap_sock_new_connection_cb
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Thu, May 24, 2012 at 04:00:17AM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <[email protected]>
>
> If the is not oriented we should call sk_state_change() just after set the
> channel state to BT_CONNECTED.
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 +-----
> net/bluetooth/l2cap_sock.c | 4 ++++
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 0d22695..6137b48 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1176,12 +1176,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> l2cap_chan_ready(chan);
>
> } else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
> - struct sock *sk = chan->sk;
> __clear_chan_timer(chan);
> - lock_sock(sk);
> - __l2cap_state_change(chan, BT_CONNECTED);
> - sk->sk_state_change(sk);
> - release_sock(sk);
> + l2cap_state_change(chan, BT_CONNECTED);
>
> } else if (chan->state == BT_CONNECT)
> l2cap_do_start(chan);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 0a1a827..9fd9412 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1018,6 +1018,10 @@ static void l2cap_sock_state_change_cb(void *data, int state)
> struct sock *sk = data;
>
> sk->sk_state = state;
> +
> + if (state == BT_CONNECTED &&
> + l2cap_pi(sk)->chan->chan_type != L2CAP_CHAN_CONN_ORIENTED)
> + sk->sk_state_change(sk);
Somehow I feel that there is too much logic here. BTW: can we use
sk_state_change for every state_change callback?
Best regards
Andrei Emeltchenko
Hi Gustavo,
On Thu, May 24, 2012 at 04:00:16AM -0300, Gustavo Padovan wrote:
> - lock_sock(sk);
> - __l2cap_state_change(chan, BT_DISCONN);
> - __l2cap_chan_set_err(chan, err);
> - release_sock(sk);
> + l2cap_state_change(chan, BT_DISCONN);
> + if(chan->ops->set_err)
> + chan->ops->set_err(chan->data, err);
I do not know can it be done somehow better, currently we lock and unlock
sockets for each operation.
Best regards
Andrei Emeltchenko
From: Gustavo Padovan <[email protected]>
If the is not oriented we should call sk_state_change() just after set the
channel state to BT_CONNECTED.
Signed-off-by: Gustavo Padovan <[email protected]>
---
net/bluetooth/l2cap_core.c | 6 +-----
net/bluetooth/l2cap_sock.c | 4 ++++
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 0d22695..6137b48 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1176,12 +1176,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
l2cap_chan_ready(chan);
} else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
- struct sock *sk = chan->sk;
__clear_chan_timer(chan);
- lock_sock(sk);
- __l2cap_state_change(chan, BT_CONNECTED);
- sk->sk_state_change(sk);
- release_sock(sk);
+ l2cap_state_change(chan, BT_CONNECTED);
} else if (chan->state == BT_CONNECT)
l2cap_do_start(chan);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 0a1a827..9fd9412 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1018,6 +1018,10 @@ static void l2cap_sock_state_change_cb(void *data, int state)
struct sock *sk = data;
sk->sk_state = state;
+
+ if (state == BT_CONNECTED &&
+ l2cap_pi(sk)->chan->chan_type != L2CAP_CHAN_CONN_ORIENTED)
+ sk->sk_state_change(sk);
}
static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
--
1.7.10.1
From: Gustavo Padovan <[email protected]>
Remove more sk occurrences from l2cap_core by enabling
us to set sk_err via chan ops.
Signed-off-by: Gustavo Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/l2cap_core.c | 34 ++++++++++------------------------
net/bluetooth/l2cap_sock.c | 10 ++++++++++
3 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 94e375a..e734572 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -532,6 +532,7 @@ struct l2cap_ops {
unsigned long len, int nb);
void (*ready) (void *data);
void (*authorize) (void *data);
+ void (*set_err) (void *data, int err);
};
struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 314ce74..0d22695 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -192,22 +192,6 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state)
release_sock(sk);
}
-static inline void __l2cap_chan_set_err(struct l2cap_chan *chan, int err)
-{
- struct sock *sk = chan->sk;
-
- sk->sk_err = err;
-}
-
-static inline void l2cap_chan_set_err(struct l2cap_chan *chan, int err)
-{
- struct sock *sk = chan->sk;
-
- lock_sock(sk);
- __l2cap_chan_set_err(chan, err);
- release_sock(sk);
-}
-
static void __set_retrans_timer(struct l2cap_chan *chan)
{
if (!delayed_work_pending(&chan->monitor_timer) &&
@@ -989,7 +973,6 @@ static inline int l2cap_mode_supported(__u8 mode, __u32 feat_mask)
static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *chan, int err)
{
- struct sock *sk = chan->sk;
struct l2cap_disconn_req req;
if (!conn)
@@ -1006,10 +989,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c
l2cap_send_cmd(conn, l2cap_get_ident(conn),
L2CAP_DISCONN_REQ, sizeof(req), &req);
- lock_sock(sk);
- __l2cap_state_change(chan, BT_DISCONN);
- __l2cap_chan_set_err(chan, err);
- release_sock(sk);
+ l2cap_state_change(chan, BT_DISCONN);
+ if(chan->ops->set_err)
+ chan->ops->set_err(chan->data, err);
}
/* ---- L2CAP connections ---- */
@@ -1220,8 +1202,11 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err)
mutex_lock(&conn->chan_lock);
list_for_each_entry(chan, &conn->chan_l, list) {
- if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags))
- __l2cap_chan_set_err(chan, err);
+ if (!test_bit(FLAG_FORCE_RELIABLE, &chan->flags))
+ continue;
+
+ if(chan->ops->set_err)
+ chan->ops->set_err(chan->data, err);
}
mutex_unlock(&conn->chan_lock);
@@ -3682,7 +3667,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
}
default:
- l2cap_chan_set_err(chan, ECONNRESET);
+ if(chan->ops->set_err)
+ chan->ops->set_err(chan->data, err);
__set_chan_timer(chan, L2CAP_DISC_REJ_TIMEOUT);
l2cap_send_disconn_req(conn, chan, ECONNRESET);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index be3e934..0a1a827 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1067,6 +1067,15 @@ static void l2cap_sock_authorize_cb(void *data)
parent->sk_data_ready(parent, 0);
}
+static void l2cap_sock_set_err_cb(void *data, int err)
+{
+ struct sock *sk = data;
+
+ lock_sock(sk);
+ sk->sk_err = err;
+ release_sock(sk);
+}
+
static struct l2cap_ops l2cap_chan_ops = {
.name = "L2CAP Socket Interface",
.new_connection = l2cap_sock_new_connection_cb,
@@ -1077,6 +1086,7 @@ static struct l2cap_ops l2cap_chan_ops = {
.alloc_skb = l2cap_sock_alloc_skb_cb,
.ready = l2cap_sock_ready_cb,
.authorize = l2cap_sock_authorize_cb,
+ .set_err = l2cap_sock_set_err_cb,
};
static void l2cap_sock_destruct(struct sock *sk)
--
1.7.10.1
This is Bluetooth: and not Blueooth:
* Gustavo Padovan <[email protected]> [2012-05-24 03:02:54 -0300]:
> 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 5ff294f9..314ce74 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 != cpu_to_le16(0x0001) &&
> @@ -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 f4c3e2c..be3e934 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -900,16 +900,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);
>
> @@ -917,6 +922,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
>
Gustavo
* Gustavo Padovan <[email protected]> [2012-05-23 22:12:36 -0300]:
> From: Gustavo Padovan <[email protected]>
>
> Hi,
>
> This patchset does a bit more of clean up in l2cap core, it was inially based
> on the two patches Andrei has sent to the list today.
For those interested in this work, a good measure of how good we are is:
$ egrep -rn "\<sk\>" net/bluetooth/l2cap_core.c | wc -l
Before this patchset of 11 patches we were counting 113 here, now we down to 77
occurrences of "sk" in the file with bt_sk(sk)->{src,dst} and socket locks
being the biggest users.
Gustavo
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 5ff294f9..314ce74 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 != cpu_to_le16(0x0001) &&
@@ -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 f4c3e2c..be3e934 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -900,16 +900,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);
@@ -917,6 +922,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().
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 c9de4f5..5ff294f9 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 d2a81f1..f4c3e2c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -915,6 +915,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]>
---
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 22ba699..c9de4f5 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->finalize)
- chan->ops->finalize(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 authorize() 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 4f81d08..94e375a 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -531,6 +531,7 @@ struct l2cap_ops {
struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan,
unsigned long len, int nb);
void (*ready) (void *data);
+ void (*authorize) (void *data);
};
struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1305b3b..22ba699 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 = cpu_to_le16(L2CAP_CR_PEND);
rsp.status = cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
- if (parent)
- parent->sk_data_ready(parent, 0);
-
+ if(chan->ops->authorize)
+ chan->ops->authorize(chan->data);
} else {
__l2cap_state_change(chan, BT_CONFIG);
rsp.result = 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->authorize)
+ chan->ops->authorize(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->authorize)
+ chan->ops->authorize(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 dac7b2c..d2a81f1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1047,6 +1047,17 @@ static void l2cap_sock_ready_cb(void *data)
release_sock(sk);
}
+static void l2cap_sock_authorize_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,
@@ -1056,6 +1067,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,
+ .authorize = l2cap_sock_authorize_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 | 9 +++++++--
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d1f6819..4f81d08 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -595,6 +595,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 46e6fb7..1305b3b 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 = cpu_to_le16(L2CAP_CR_PEND);
rsp.status = 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 bc409a2..dac7b2c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -623,10 +623,15 @@ 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]>
---
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 9416c60..46e6fb7 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 8a3620b..bc409a2 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 15ee8a5..9416c60 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 7d671ad..15ee8a5 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 29f7b06..d1f6819 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -530,6 +530,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 f77b134..7d671ad 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 0302cb4..8a3620b 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 = {
.finalize = l2cap_sock_finalize_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 is already performed inside l2cap_chan_ready(), so we don't need it
here again.
Signed-off-by: Gustavo Padovan <[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 e7a598c..f77b134 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3608,8 +3608,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);
@@ -3740,7 +3738,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
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 0142257..29f7b06 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -526,6 +526,7 @@ struct l2cap_ops {
struct l2cap_chan *(*new_connection) (void *data);
int (*recv) (void *data, struct sk_buff *skb);
void (*close) (void *data);
+ void (*finalize) (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 e31b005..e7a598c 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->finalize)
+ chan->ops->finalize(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->finalize)
+ chan->ops->finalize(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->finalize)
+ chan->ops->finalize(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->finalize)
+ chan->ops->finalize(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 4d36605..0302cb4 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_finalize_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,
+ .finalize = l2cap_sock_finalize_cb,
.state_change = l2cap_sock_state_change_cb,
.alloc_skb = l2cap_sock_alloc_skb_cb,
};
--
1.7.10.1