Hi,
This set of patches contains some cleanups and fixes for future L2CAP
work. The L2CAP usage in coming core spec versions will no longer be as
constrined as it has so far with extensions to the LE signalling
channel etc. This means that we really do need separate code paths for
LE signalling instead of sharing BR/EDR related signalling code. The
patches to split this up keep the same functionality as before (for now)
but ensure that we have dedicated functions and structs for LE.
Johan
----------------------------------------------------------------
Johan Hedberg (13):
Bluetooth: Handle LE L2CAP signalling in its own function
Bluetooth: Create independent LE signalling defines and structs
Bluetooth: Rename L2CAP_CID_LE_DATA to L2CAP_CID_ATT
Bluetooth: Fix LE vs BR/EDR selection when connecting
Bluetooth: Fix EBUSY condition test in l2cap_chan_connect
Bluetooth: Fix hardcoding ATT CID in __l2cap_chan_add()
Bluetooth: Add clarifying comment to l2cap_conn_ready()
Bluetooth: Fix duplicate call to l2cap_chan_ready()
Bluetooth: Remove useless sk variable in l2cap_le_conn_ready
Bluetooth: Remove unnecessary L2CAP channel state check
Bluetooth: Simplify hci_conn_hold/drop logic for L2CAP
Bluetooth: Remove useless hci_conn disc_timeout setting
Bluetooth: Fix multiple LE socket handling
include/net/bluetooth/l2cap.h | 36 ++++++++--
net/bluetooth/l2cap_core.c | 145 ++++++++++++++++++++++++++---------------
net/bluetooth/l2cap_sock.c | 4 +-
3 files changed, 126 insertions(+), 59 deletions(-)
Hi Johan,
* Johan Hedberg <[email protected]> [2013-04-29 19:35:45 +0300]:
> From: Johan Hedberg <[email protected]>
>
> The LE ATT server socket needs to be superseded by any ATT client
> sockets. Previously this was done by looking at the hcon->out variable
> (indicating whether the connection is outgoing or incoming) which is a
> too crude way of determining whether the server socket needs to be
> picked or not (an outgoing connection doesn't necessarily mean that an
> ATT client socket has triggered it).
>
> This patch extends the ATT server socket lookup function
> (l2cap_le_conn_ready) to be used for all LE connections (regardless of
> the hcon->out value) and adds an internal check into the function for
> the existence of any ATT client sockets (in which case the server socket
> should be skipped). For this to work reliably all lookups must be done
> while the l2cap_conn->chan_lock is held, meaning also that the call to
> l2cap_chan_add needs to be changed to its lockless __l2cap_chan_add
> counterpart.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
All 13 patches have been applied to bluetooth-next. Thanks.
Gustavo
Hi,
On 10:51 Tue 30 Apr, Marcel Holtmann wrote:
> Hi Vinicius,
>
> >> The choice between LE and BR/EDR should be made on the destination
> >> address type instead of the destination CID. This is particularly
> >> important when in the future more than one CID will be allowed for LE.
> >>
> >> Signed-off-by: Johan Hedberg <[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 d5e4404..95d46cd 100644
> >> --- a/net/bluetooth/l2cap_core.c
> >> +++ b/net/bluetooth/l2cap_core.c
> >> @@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> >>
> >> auth_type = l2cap_get_auth_type(chan);
> >>
> >> - if (chan->dcid == L2CAP_CID_ATT)
> >> + if (bdaddr_type_is_le(dst_type))
> >
> > Wouldn't this break applications that rely on the CID for making a LE
> > connection?
>
> which application are these. They are all from the time where enable_le=0 was the default. We introduced the BD_ADDR address type before we enabled LE by default.
I was thinking "old" BlueZ releases, 4.99 would have this issue, 4.101 is
fine.
Fair enough. That argument about the address type being added before LE being
enabled by default convinced me.
>
> Regards
>
> Marcel
>
Cheers,
--
Vinicius
Hi Vinicius,
On Tue, Apr 30, 2013, Vinicius Costa Gomes wrote:
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> >
> > auth_type = l2cap_get_auth_type(chan);
> >
> > - if (chan->dcid == L2CAP_CID_ATT)
> > + if (bdaddr_type_is_le(dst_type))
>
> Wouldn't this break applications that rely on the CID for making a LE
> connection?
An application that wants to make an LE connection but doesn't provide
an LE address type in the l2_bdaddr_type member of sockaddr_l2 is
broken.
Johan
Hi Vinicius,
>> The choice between LE and BR/EDR should be made on the destination
>> address type instead of the destination CID. This is particularly
>> important when in the future more than one CID will be allowed for LE.
>>
>> Signed-off-by: Johan Hedberg <[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 d5e4404..95d46cd 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>>
>> auth_type = l2cap_get_auth_type(chan);
>>
>> - if (chan->dcid == L2CAP_CID_ATT)
>> + if (bdaddr_type_is_le(dst_type))
>
> Wouldn't this break applications that rely on the CID for making a LE
> connection?
which application are these. They are all from the time where enable_le=0 was the default. We introduced the BD_ADDR address type before we enabled LE by default.
Regards
Marcel
Hi Johan,
On 19:35 Mon 29 Apr, Johan Hedberg wrote:
> From: Johan Hedberg <[email protected]>
>
> The choice between LE and BR/EDR should be made on the destination
> address type instead of the destination CID. This is particularly
> important when in the future more than one CID will be allowed for LE.
>
> Signed-off-by: Johan Hedberg <[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 d5e4404..95d46cd 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>
> auth_type = l2cap_get_auth_type(chan);
>
> - if (chan->dcid == L2CAP_CID_ATT)
> + if (bdaddr_type_is_le(dst_type))
Wouldn't this break applications that rely on the CID for making a LE
connection?
> hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
> chan->sec_level, auth_type);
> else
> --
> 1.7.10.4
>
> --
> 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
Hi Johan,
> This set of patches contains some cleanups and fixes for future L2CAP
> work. The L2CAP usage in coming core spec versions will no longer be as
> constrined as it has so far with extensions to the LE signalling
> channel etc. This means that we really do need separate code paths for
> LE signalling instead of sharing BR/EDR related signalling code. The
> patches to split this up keep the same functionality as before (for now)
> but ensure that we have dedicated functions and structs for LE.
>
> Johan
>
> ----------------------------------------------------------------
> Johan Hedberg (13):
> Bluetooth: Handle LE L2CAP signalling in its own function
> Bluetooth: Create independent LE signalling defines and structs
> Bluetooth: Rename L2CAP_CID_LE_DATA to L2CAP_CID_ATT
> Bluetooth: Fix LE vs BR/EDR selection when connecting
> Bluetooth: Fix EBUSY condition test in l2cap_chan_connect
> Bluetooth: Fix hardcoding ATT CID in __l2cap_chan_add()
> Bluetooth: Add clarifying comment to l2cap_conn_ready()
> Bluetooth: Fix duplicate call to l2cap_chan_ready()
> Bluetooth: Remove useless sk variable in l2cap_le_conn_ready
> Bluetooth: Remove unnecessary L2CAP channel state check
> Bluetooth: Simplify hci_conn_hold/drop logic for L2CAP
> Bluetooth: Remove useless hci_conn disc_timeout setting
> Bluetooth: Fix multiple LE socket handling
>
> include/net/bluetooth/l2cap.h | 36 ++++++++--
> net/bluetooth/l2cap_core.c | 145 ++++++++++++++++++++++++++---------------
> net/bluetooth/l2cap_sock.c | 4 +-
> 3 files changed, 126 insertions(+), 59 deletions(-)
the whole set looks fine to me.
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
From: Johan Hedberg <[email protected]>
The L2CAP code has been incrementing the hci_conn reference for each
l2cap_chan instance in the l2cap_conn list. Likewise, the reference is
dropped each time an l2cap_chan is removed from the list. The reference
counting policy with respect to removal has been clear and explicit in
the l2cap_chan_drop function, however for addition the function
calling 2cap_chan_add has always had to do a separate hci_conn_hold
call.
What made the counting even more hard to follow is that the
hci_connect() procedure increments the reference and the L2CAP layer
making this call took advantage of it to use it as its own reference.
This patch aims to clarify things by having the call to hci_conn_hold
inside __l2cap_chan_add, thereby removing the need to do it in the
functions calling __l2cap_chan_add. The reference count for hci_connect
is still kept as it's necessary for users such as mgmt_pair_device,
however for the L2CAP layer it means that an extra call to hci_conn_drop
must be performed once l2cap_chan_add has been done.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e373b9d..c78a510 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -545,6 +545,8 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
l2cap_chan_hold(chan);
+ hci_conn_hold(conn->hcon);
+
list_add(&chan->list, &conn->chan_l);
}
@@ -1361,7 +1363,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
chan->dcid = L2CAP_CID_ATT;
- hci_conn_hold(conn->hcon);
conn->hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
bacpy(&bt_sk(chan->sk)->src, conn->src);
@@ -1827,6 +1828,9 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
l2cap_chan_add(conn, chan);
l2cap_chan_lock(chan);
+ /* l2cap_chan_add takes its own ref so we can drop this one */
+ hci_conn_drop(hcon);
+
l2cap_state_change(chan, BT_CONNECT);
__set_chan_timer(chan, sk->sk_sndtimeo);
@@ -3741,8 +3745,6 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
sk = chan->sk;
- hci_conn_hold(conn->hcon);
-
bacpy(&bt_sk(sk)->src, conn->src);
bacpy(&bt_sk(sk)->dst, conn->dst);
chan->psm = psm;
--
1.7.10.4
From: Johan Hedberg <[email protected]>
The sk variable is of quite little use since it's only used to simplify
access in the two bt_sk() calls.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 881be61..aa4bc70 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1340,7 +1340,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 *parent;
struct l2cap_chan *chan, *pchan;
BT_DBG("");
@@ -1361,13 +1361,11 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
chan->dcid = L2CAP_CID_ATT;
- sk = chan->sk;
-
hci_conn_hold(conn->hcon);
conn->hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
- bacpy(&bt_sk(sk)->src, conn->src);
- bacpy(&bt_sk(sk)->dst, conn->dst);
+ bacpy(&bt_sk(chan->sk)->src, conn->src);
+ bacpy(&bt_sk(chan->sk)->dst, conn->dst);
l2cap_chan_add(conn, chan);
--
1.7.10.4
From: Johan Hedberg <[email protected]>
The LE ATT server socket needs to be superseded by any ATT client
sockets. Previously this was done by looking at the hcon->out variable
(indicating whether the connection is outgoing or incoming) which is a
too crude way of determining whether the server socket needs to be
picked or not (an outgoing connection doesn't necessarily mean that an
ATT client socket has triggered it).
This patch extends the ATT server socket lookup function
(l2cap_le_conn_ready) to be used for all LE connections (regardless of
the hcon->out value) and adds an internal check into the function for
the existence of any ATT client sockets (in which case the server socket
should be skipped). For this to work reliably all lookups must be done
while the l2cap_conn->chan_lock is held, meaning also that the call to
l2cap_chan_add needs to be changed to its lockless __l2cap_chan_add
counterpart.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d510550..05e6255 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1353,6 +1353,10 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
if (!pchan)
return;
+ /* Client ATT sockets should override the server one */
+ if (__l2cap_get_chan_by_dcid(conn, L2CAP_CID_ATT))
+ return;
+
parent = pchan->sk;
lock_sock(parent);
@@ -1366,7 +1370,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
bacpy(&bt_sk(chan->sk)->src, conn->src);
bacpy(&bt_sk(chan->sk)->dst, conn->dst);
- l2cap_chan_add(conn, chan);
+ __l2cap_chan_add(conn, chan);
clean:
release_sock(parent);
@@ -1379,9 +1383,6 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
BT_DBG("conn %p", conn);
- if (!hcon->out && hcon->type == LE_LINK)
- l2cap_le_conn_ready(conn);
-
/* For outgoing pairing which doesn't necessarily have an
* associated socket (e.g. mgmt_pair_device).
*/
@@ -1390,6 +1391,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
mutex_lock(&conn->chan_lock);
+ if (hcon->type == LE_LINK)
+ l2cap_le_conn_ready(conn);
+
list_for_each_entry(chan, &conn->chan_l, list) {
l2cap_chan_lock(chan);
--
1.7.10.4
From: Johan Hedberg <[email protected]>
In l2cap_att_channel() we're only interested in the BT_CONNECTED state
so this state can directly be passed to l2cap_global_chan_by_scid().
This way there's no need to do any additional state check later.
Signed-off-by: Johan Hedberg <[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 aa4bc70..e373b9d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6357,16 +6357,13 @@ static void l2cap_att_channel(struct l2cap_conn *conn,
{
struct l2cap_chan *chan;
- chan = l2cap_global_chan_by_scid(0, L2CAP_CID_ATT,
+ chan = l2cap_global_chan_by_scid(BT_CONNECTED, L2CAP_CID_ATT,
conn->src, conn->dst);
if (!chan)
goto drop;
BT_DBG("chan %p, len %d", chan, skb->len);
- if (chan->state != BT_BOUND && chan->state != BT_CONNECTED)
- goto drop;
-
if (chan->imtu < skb->len)
goto drop;
--
1.7.10.4
From: Johan Hedberg <[email protected]>
There's no need to reset disc_timeout in l2cap_le_conn_ready since
HCI_DISCONN_TIMEOUT is the default when the hci_conn is created and
there should be no way for it to get changed between creation and
l2cap_le_conn_ready being called.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c78a510..d510550 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1363,8 +1363,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
chan->dcid = L2CAP_CID_ATT;
- conn->hcon->disc_timeout = HCI_DISCONN_TIMEOUT;
-
bacpy(&bt_sk(chan->sk)->src, conn->src);
bacpy(&bt_sk(chan->sk)->dst, conn->dst);
--
1.7.10.4
From: Johan Hedberg <[email protected]>
In l2cap_le_conn_ready() after doing l2cap_chann_add() the LE channel is
part of the list which is subsequently iterated in l2cap_conn_ready() in
this loop each channel will get l2cap_chan_ready() called which would
result in trying to set the channel two times into BT_CONNECTED state.
Instead it makes sense to just add the channel but not call chan_ready
in l2cap_le_conn_ready, which is what this patch does.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 60d13bc..881be61 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1371,8 +1371,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
l2cap_chan_add(conn, chan);
- l2cap_chan_ready(chan);
-
clean:
release_sock(parent);
}
--
1.7.10.4
From: Johan Hedberg <[email protected]>
There is an extra call to smp_conn_security() for outgoing LE
connections from l2cap_conn_ready() but the reason for this call is far
from clear. After a bit of commit history research and using git blame I
found out that this extra call is for socket-less pairing processes
added by commit 160dc6ac1. This patch adds a clarifying comment to the
code for this.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index dd7f9b3..60d13bc 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1387,6 +1387,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
if (!hcon->out && hcon->type == LE_LINK)
l2cap_le_conn_ready(conn);
+ /* For outgoing pairing which doesn't necessarily have an
+ * associated socket (e.g. mgmt_pair_device).
+ */
if (hcon->out && hcon->type == LE_LINK)
smp_conn_security(hcon, hcon->pending_sec_level);
--
1.7.10.4
From: Johan Hedberg <[email protected]>
Since in the future more than the ATT CID may be permissible we should
not be hardcoding it for all LE connections in __l2cap_chan_add().
Instead, the source ATT CID should only be set if the destination is
also ATT, and in other cases we should just use the existing dynamic CID
allocation function.
Assigning scid based on dcid means that whenever __l2cap_chan_add() is
called that chan->dcid is properly initialized. l2cap_le_conn_ready()
wasn't initializing is properly so this is also taken care of in this
patch.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fd8d3059..dd7f9b3 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -504,8 +504,10 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
if (conn->hcon->type == LE_LINK) {
/* LE connection */
chan->omtu = L2CAP_DEFAULT_MTU;
- chan->scid = L2CAP_CID_ATT;
- chan->dcid = L2CAP_CID_ATT;
+ if (chan->dcid == L2CAP_CID_ATT)
+ chan->scid = L2CAP_CID_ATT;
+ else
+ chan->scid = l2cap_alloc_cid(conn);
} else {
/* Alloc CID for connection-oriented socket */
chan->scid = l2cap_alloc_cid(conn);
@@ -1357,6 +1359,8 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
if (!chan)
goto clean;
+ chan->dcid = L2CAP_CID_ATT;
+
sk = chan->sk;
hci_conn_hold(conn->hcon);
--
1.7.10.4
From: Johan Hedberg <[email protected]>
The current test in l2cap_chan_connect is intended to protect against
multiple conflicting connect attempts. However, it assumes that there
will ever only be a single CID that is connected to, which is not true.
We do need to check for conflicts with connect attempts to the same
destination CID but this check is not in anyway specific to LE but can
be applied to BR/EDR as well.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 95d46cd..fd8d3059 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1811,16 +1811,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
goto done;
}
- if (hcon->type == LE_LINK) {
- err = 0;
-
- if (!list_empty(&conn->chan_l)) {
- err = -EBUSY;
- hci_conn_drop(hcon);
- }
-
- if (err)
- goto done;
+ if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
+ hci_conn_drop(hcon);
+ err = -EBUSY;
+ goto done;
}
/* Update source addr of the socket */
--
1.7.10.4
From: Johan Hedberg <[email protected]>
The choice between LE and BR/EDR should be made on the destination
address type instead of the destination CID. This is particularly
important when in the future more than one CID will be allowed for LE.
Signed-off-by: Johan Hedberg <[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 d5e4404..95d46cd 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
auth_type = l2cap_get_auth_type(chan);
- if (chan->dcid == L2CAP_CID_ATT)
+ if (bdaddr_type_is_le(dst_type))
hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
chan->sec_level, auth_type);
else
--
1.7.10.4
From: Johan Hedberg <[email protected]>
In future Core Specification versions the ATT CID will be just one of
many possible CIDs that can be used for data transfer. Therefore, it
makes sense to rename the define for the ATT CID to something less
ambigous.
Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 +-
net/bluetooth/l2cap_core.c | 14 +++++++-------
net/bluetooth/l2cap_sock.c | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 2f52a28..c31ac21 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -247,7 +247,7 @@ struct l2cap_conn_rsp {
#define L2CAP_CID_SIGNALING 0x0001
#define L2CAP_CID_CONN_LESS 0x0002
#define L2CAP_CID_A2MP 0x0003
-#define L2CAP_CID_LE_DATA 0x0004
+#define L2CAP_CID_ATT 0x0004
#define L2CAP_CID_LE_SIGNALING 0x0005
#define L2CAP_CID_SMP 0x0006
#define L2CAP_CID_DYN_START 0x0040
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6bf5d19..d5e4404 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -504,8 +504,8 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
if (conn->hcon->type == LE_LINK) {
/* LE connection */
chan->omtu = L2CAP_DEFAULT_MTU;
- chan->scid = L2CAP_CID_LE_DATA;
- chan->dcid = L2CAP_CID_LE_DATA;
+ chan->scid = L2CAP_CID_ATT;
+ chan->dcid = L2CAP_CID_ATT;
} else {
/* Alloc CID for connection-oriented socket */
chan->scid = l2cap_alloc_cid(conn);
@@ -1344,7 +1344,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
BT_DBG("");
/* Check if we have socket listening on cid */
- pchan = l2cap_global_chan_by_scid(BT_LISTEN, L2CAP_CID_LE_DATA,
+ pchan = l2cap_global_chan_by_scid(BT_LISTEN, L2CAP_CID_ATT,
conn->src, conn->dst);
if (!pchan)
return;
@@ -1792,7 +1792,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
auth_type = l2cap_get_auth_type(chan);
- if (chan->dcid == L2CAP_CID_LE_DATA)
+ if (chan->dcid == L2CAP_CID_ATT)
hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
chan->sec_level, auth_type);
else
@@ -6360,7 +6360,7 @@ static void l2cap_att_channel(struct l2cap_conn *conn,
{
struct l2cap_chan *chan;
- chan = l2cap_global_chan_by_scid(0, L2CAP_CID_LE_DATA,
+ chan = l2cap_global_chan_by_scid(0, L2CAP_CID_ATT,
conn->src, conn->dst);
if (!chan)
goto drop;
@@ -6411,7 +6411,7 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
l2cap_conless_channel(conn, psm, skb);
break;
- case L2CAP_CID_LE_DATA:
+ case L2CAP_CID_ATT:
l2cap_att_channel(conn, skb);
break;
@@ -6537,7 +6537,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
continue;
}
- if (chan->scid == L2CAP_CID_LE_DATA) {
+ if (chan->scid == L2CAP_CID_ATT) {
if (!status && encrypt) {
chan->sec_level = hcon->sec_level;
l2cap_chan_ready(chan);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 141e7b0..16b3e51 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -466,7 +466,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
static bool l2cap_valid_mtu(struct l2cap_chan *chan, u16 mtu)
{
switch (chan->scid) {
- case L2CAP_CID_LE_DATA:
+ case L2CAP_CID_ATT:
if (mtu < L2CAP_LE_MIN_MTU)
return false;
break;
@@ -630,7 +630,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
conn = chan->conn;
/*change security for LE channels */
- if (chan->scid == L2CAP_CID_LE_DATA) {
+ if (chan->scid == L2CAP_CID_ATT) {
if (!conn->hcon->out) {
err = -EINVAL;
break;
--
1.7.10.4
From: Johan Hedberg <[email protected]>
Since the LE signalling channel is independent from the BR/EDR
signalling channel and may experience changes incompatible with the
BR/EDR channel it makes sense from the start to have independent defines
and structs for it.
Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/l2cap.h | 34 ++++++++++++++++++++++++++++++----
net/bluetooth/l2cap_core.c | 24 ++++++++++++------------
2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index fb94cf1..2f52a28 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -110,8 +110,13 @@ struct l2cap_conninfo {
#define L2CAP_MOVE_CHAN_RSP 0x0f
#define L2CAP_MOVE_CHAN_CFM 0x10
#define L2CAP_MOVE_CHAN_CFM_RSP 0x11
-#define L2CAP_CONN_PARAM_UPDATE_REQ 0x12
-#define L2CAP_CONN_PARAM_UPDATE_RSP 0x13
+
+/* L2CAP LE command codes */
+#define L2CAP_LE_COMMAND_REJ 0x01
+#define L2CAP_LE_DISCONN_REQ 0x06
+#define L2CAP_LE_DISCONN_RSP 0x07
+#define L2CAP_LE_CONN_PARAM_UPDATE_REQ 0x12
+#define L2CAP_LE_CONN_PARAM_UPDATE_RSP 0x13
/* L2CAP extended feature mask */
#define L2CAP_FEAT_FLOWCTL 0x00000001
@@ -406,14 +411,35 @@ struct l2cap_move_chan_cfm_rsp {
#define L2CAP_IR_SUCCESS 0x0000
#define L2CAP_IR_NOTSUPP 0x0001
-struct l2cap_conn_param_update_req {
+struct l2cap_le_cmd_hdr {
+ __u8 code;
+ __u8 ident;
+ __le16 len;
+} __packed;
+#define L2CAP_LE_CMD_HDR_SIZE 4
+
+struct l2cap_le_cmd_rej_unk {
+ __le16 reason;
+} __packed;
+
+struct l2cap_le_disconn_req {
+ __le16 dcid;
+ __le16 scid;
+} __packed;
+
+struct l2cap_le_disconn_rsp {
+ __le16 dcid;
+ __le16 scid;
+} __packed;
+
+struct l2cap_le_conn_param_update_req {
__le16 min;
__le16 max;
__le16 latency;
__le16 to_multiplier;
} __packed;
-struct l2cap_conn_param_update_rsp {
+struct l2cap_le_conn_param_update_rsp {
__le16 result;
} __packed;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1adda11..6bf5d19 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5113,13 +5113,13 @@ static inline int l2cap_check_conn_param(u16 min, u16 max, u16 latency,
return 0;
}
-static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
- struct l2cap_cmd_hdr *cmd,
- u8 *data)
+static inline int l2cap_le_conn_param_update_req(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd,
+ u8 *data)
{
struct hci_conn *hcon = conn->hcon;
- struct l2cap_conn_param_update_req *req;
- struct l2cap_conn_param_update_rsp rsp;
+ struct l2cap_le_conn_param_update_req *req;
+ struct l2cap_le_conn_param_update_rsp rsp;
u16 min, max, latency, to_multiplier, cmd_len;
int err;
@@ -5127,10 +5127,10 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
return -EINVAL;
cmd_len = __le16_to_cpu(cmd->len);
- if (cmd_len != sizeof(struct l2cap_conn_param_update_req))
+ if (cmd_len != sizeof(struct l2cap_le_conn_param_update_req))
return -EPROTO;
- req = (struct l2cap_conn_param_update_req *) data;
+ req = (struct l2cap_le_conn_param_update_req *) data;
min = __le16_to_cpu(req->min);
max = __le16_to_cpu(req->max);
latency = __le16_to_cpu(req->latency);
@@ -5147,7 +5147,7 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
else
rsp.result = __constant_cpu_to_le16(L2CAP_CONN_PARAM_ACCEPTED);
- l2cap_send_cmd(conn, cmd->ident, L2CAP_CONN_PARAM_UPDATE_RSP,
+ l2cap_send_cmd(conn, cmd->ident, L2CAP_LE_CONN_PARAM_UPDATE_RSP,
sizeof(rsp), &rsp);
if (!err)
@@ -5240,13 +5240,13 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u8 *data)
{
switch (cmd->code) {
- case L2CAP_COMMAND_REJ:
+ case L2CAP_LE_COMMAND_REJ:
return 0;
- case L2CAP_CONN_PARAM_UPDATE_REQ:
- return l2cap_conn_param_update_req(conn, cmd, data);
+ case L2CAP_LE_CONN_PARAM_UPDATE_REQ:
+ return l2cap_le_conn_param_update_req(conn, cmd, data);
- case L2CAP_CONN_PARAM_UPDATE_RSP:
+ case L2CAP_LE_CONN_PARAM_UPDATE_RSP:
return 0;
default:
--
1.7.10.4
From: Johan Hedberg <[email protected]>
The LE L2CAP signalling channel follows its own rules and will continue
to evolve independently from the BR/EDR signalling channel. Therefore,
it makes sense to have a clear split from BR/EDR by having a dedicated
function for handling LE signalling commands.
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_core.c | 53 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a76d1ac..1adda11 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5255,6 +5255,51 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
}
}
+static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
+ struct sk_buff *skb)
+{
+ u8 *data = skb->data;
+ int len = skb->len;
+ struct l2cap_cmd_hdr cmd;
+ int err;
+
+ l2cap_raw_recv(conn, skb);
+
+ while (len >= L2CAP_CMD_HDR_SIZE) {
+ u16 cmd_len;
+ memcpy(&cmd, data, L2CAP_CMD_HDR_SIZE);
+ data += L2CAP_CMD_HDR_SIZE;
+ len -= L2CAP_CMD_HDR_SIZE;
+
+ cmd_len = le16_to_cpu(cmd.len);
+
+ BT_DBG("code 0x%2.2x len %d id 0x%2.2x", cmd.code, cmd_len,
+ cmd.ident);
+
+ if (cmd_len > len || !cmd.ident) {
+ BT_DBG("corrupted command");
+ break;
+ }
+
+ err = l2cap_le_sig_cmd(conn, &cmd, data);
+ if (err) {
+ struct l2cap_cmd_rej_unk rej;
+
+ BT_ERR("Wrong link type (%d)", err);
+
+ /* FIXME: Map err to a valid reason */
+ rej.reason = __constant_cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
+ l2cap_send_cmd(conn, cmd.ident, L2CAP_COMMAND_REJ,
+ sizeof(rej), &rej);
+ }
+
+ data += cmd_len;
+ len -= cmd_len;
+ }
+
+ kfree_skb(skb);
+}
+
static inline void l2cap_sig_channel(struct l2cap_conn *conn,
struct sk_buff *skb)
{
@@ -5281,11 +5326,7 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
break;
}
- if (conn->hcon->type == LE_LINK)
- err = l2cap_le_sig_cmd(conn, &cmd, data);
- else
- err = l2cap_bredr_sig_cmd(conn, &cmd, cmd_len, data);
-
+ err = l2cap_bredr_sig_cmd(conn, &cmd, cmd_len, data);
if (err) {
struct l2cap_cmd_rej_unk rej;
@@ -6358,6 +6399,8 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
switch (cid) {
case L2CAP_CID_LE_SIGNALING:
+ l2cap_le_sig_channel(conn, skb);
+ break;
case L2CAP_CID_SIGNALING:
l2cap_sig_channel(conn, skb);
break;
--
1.7.10.4