Hi all,
In order to establish a LE connection, we need to know the address type
(public or random) from the remote device. Since this information was not
exposed to user-space and we were not sure about changing the Bluetooth ABI
at that time, we came up with a in-kernel solution to enable LE connections.
We store sensitive information (bdaddr and bdaddr_type) gathered from LE
advertising report events. Once we get a connection request from user-space,
we search the destination address in the advertising cache to get its type
and then start the connection establishment.
Nevertheless, today, the remote device address type is exposed to user-space
through management interface events. We can use this information to establish
LE connection and drop the advertising cache in kernel.
To achieve that, we may add the address type field to struct sockaddr_l2.
This new field would be used for LE connection only. BR/EDR sockets would just
ignore it. We wouldn't even need to set this field for BR/EDR sockets.
These changes would be taken in four steps:
1. Kernel: add address type info to struct sockaddr_l2
2. User-space: set the address type field for LE connections
3. Kernel: use address type info from user-space instead of checking
advertising cache
4. Kernel: remove advertising cache code
This RFC series implements steps 1 and 3. User-space work is still under
development and we'll send a RFC soon. Step 4 would be started as soon as
we have steps 1, 2 and 3 done.
These RFC patches are just to you guys take a look how the code looks like
and provide some comments. They are still under testing.
Feedback are very welcome.
Regards,
Andre Guedes
Andre Guedes (3):
Bluetooth: Add address type to struct sockaddr_l2
Bluetooth: Add dst_type parameter to hci_connect
Bluetooth: Use address type info from User-space
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/l2cap.h | 3 ++-
net/bluetooth/hci_conn.c | 12 ++++--------
net/bluetooth/l2cap_core.c | 11 ++++++-----
net/bluetooth/l2cap_sock.c | 2 +-
net/bluetooth/mgmt.c | 8 ++++----
net/bluetooth/sco.c | 3 ++-
7 files changed, 20 insertions(+), 21 deletions(-)
--
1.7.9.4
Hi Marcel,
On Thu, Mar 29, 2012 at 2:31 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Andre,
>
>> > On Tue, Mar 27, 2012, Andre Guedes wrote:
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 le->dst_type =3D (dst_type =3D=3D MGMT_ADDR=
_LE_RANDOM) ?
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ADDR_LE_DEV=
_RANDOM : ADDR_LE_DEV_PUBLIC;
>> >
>> > You might want to make a simple helper function for the type conversio=
n.
>> >
>> > Also, I'm not so sure it's a good idea to directly reuse mgmt API
>> > defines for the L2CAP socket interface. The values may in the end be t=
he
>> > same but probably there should be separate defines in l2cap.h.
>>
>> I'm not sure too.
>>
>> I think it would be better we define address type macros in
>> bluetooth.h and replace its prefix by BDADDR_TYPE_. So we would have
>> something like this in bluetooth.h:
>>
>> +/* BD Address type */
>> +#define BDADDR_TYPE_BREDR =A0 =A0 =A00x00
>> +#define BDADDR_TYPE_LE_PUBLIC =A00x01
>> +#define BDADDR_TYPE_LE_RANDOM =A00x02
>> +#define BDADDR_TYPE_INVALID =A0 =A00xff
>
> what is INVALID for? That seems like a pointless value to have.
Management interface defines a invalid address type macro, but I think
we can easily remove it.
>> What do you think?
>
> I also get the feeling that these are a bit long. What is the benefit of
> using BDADDR_TYPE_ namespace. Would not something like this be better:
>
> =A0 =A0 =A0 =A0BDADDR_BREDR =A0 =A0 0x00
> =A0 =A0 =A0 =A0BDADDR_LE_PUBLIC 0x01
> =A0 =A0 =A0 =A0BDADDR_LE_RANDOM 0x02
Yes, it would be better. I'll work on a patch for this and add it to
the RFC v2 series.
BR,
Andre
Hi Andre,
> > On Tue, Mar 27, 2012, Andre Guedes wrote:
> >> + le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
> >> + ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
> >
> > You might want to make a simple helper function for the type conversion.
> >
> > Also, I'm not so sure it's a good idea to directly reuse mgmt API
> > defines for the L2CAP socket interface. The values may in the end be the
> > same but probably there should be separate defines in l2cap.h.
>
> I'm not sure too.
>
> I think it would be better we define address type macros in
> bluetooth.h and replace its prefix by BDADDR_TYPE_. So we would have
> something like this in bluetooth.h:
>
> +/* BD Address type */
> +#define BDADDR_TYPE_BREDR 0x00
> +#define BDADDR_TYPE_LE_PUBLIC 0x01
> +#define BDADDR_TYPE_LE_RANDOM 0x02
> +#define BDADDR_TYPE_INVALID 0xff
what is INVALID for? That seems like a pointless value to have.
> What do you think?
I also get the feeling that these are a bit long. What is the benefit of
using BDADDR_TYPE_ namespace. Would not something like this be better:
BDADDR_BREDR 0x00
BDADDR_LE_PUBLIC 0x01
BDADDR_LE_RANDOM 0x02
Regards
Marcel
Hi Johan,
On Thu, Mar 29, 2012 at 6:12 AM, Johan Hedberg <[email protected]> wrote:
> Hi Andre,
>
> On Tue, Mar 27, 2012, Andre Guedes wrote:
>> + ? ? ? ? ? ? le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
>
> You might want to make a simple helper function for the type conversion.
>
> Also, I'm not so sure it's a good idea to directly reuse mgmt API
> defines for the L2CAP socket interface. The values may in the end be the
> same but probably there should be separate defines in l2cap.h.
I'm not sure too.
I think it would be better we define address type macros in
bluetooth.h and replace its prefix by BDADDR_TYPE_. So we would have
something like this in bluetooth.h:
+/* BD Address type */
+#define BDADDR_TYPE_BREDR 0x00
+#define BDADDR_TYPE_LE_PUBLIC 0x01
+#define BDADDR_TYPE_LE_RANDOM 0x02
+#define BDADDR_TYPE_INVALID 0xff
What do you think?
Thanks,
Andre
Hi Andre,
On Tue, Mar 27, 2012, Andre Guedes wrote:
> + le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
> + ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
You might want to make a simple helper function for the type conversion.
Also, I'm not so sure it's a good idea to directly reuse mgmt API
defines for the L2CAP socket interface. The values may in the end be the
same but probably there should be separate defines in l2cap.h.
Johan
Hi Andrei,
On Wed, Mar 28, 2012 at 11:26 AM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi Andre,
>
> On Wed, Mar 28, 2012 at 11:05:47AM -0300, Andre Guedes wrote:
>> >> -int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
>> >> +int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>> >> + ? ? ? ? ? ? ? ? ? ? bdaddr_t *dst, u8 dst_type)
>> >> ?{
>> >> ? ? ? struct sock *sk = chan->sk;
>> >> ? ? ? bdaddr_t *src = &bt_sk(sk)->src;
>> >> @@ -1141,8 +1142,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
>> >> ? ? ? __u8 auth_type;
>> >> ? ? ? int err;
>> >>
>> >> - ? ? BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
>> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __le16_to_cpu(chan->psm));
>> >> + ? ? BT_DBG("%s -> %s (type %u) psm 0x%2.2x", batostr(src), batostr(dst),
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dst_type, __le16_to_cpu(chan->psm));
>> >
>> > If you change style for other functions you might change it here as well.
>>
>> Sorry, didn't follow you here.
>
> In l2cap_chan_connect you seems align second line at "(". Maybe we need
> then align the same for debug statement?
I see now, I guess we are supposed to align debug too. I'll fix this.
Thanks,
Andre
Hi Andre,
On Wed, Mar 28, 2012 at 11:05:47AM -0300, Andre Guedes wrote:
> >> -int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
> >> +int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> >> + ? ? ? ? ? ? ? ? ? ? bdaddr_t *dst, u8 dst_type)
> >> ?{
> >> ? ? ? struct sock *sk = chan->sk;
> >> ? ? ? bdaddr_t *src = &bt_sk(sk)->src;
> >> @@ -1141,8 +1142,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
> >> ? ? ? __u8 auth_type;
> >> ? ? ? int err;
> >>
> >> - ? ? BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __le16_to_cpu(chan->psm));
> >> + ? ? BT_DBG("%s -> %s (type %u) psm 0x%2.2x", batostr(src), batostr(dst),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dst_type, __le16_to_cpu(chan->psm));
> >
> > If you change style for other functions you might change it here as well.
>
> Sorry, didn't follow you here.
In l2cap_chan_connect you seems align second line at "(". Maybe we need
then align the same for debug statement?
Best regards
Andrei Emeltchenko
Hi Andrei,
On Wed, Mar 28, 2012 at 5:56 AM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi Andre,
>
> On Tue, Mar 27, 2012 at 08:59:36PM -0300, Andre Guedes wrote:
>> In order to establish a LE connection we need the address type
>> information. User-space already pass this information to kernel
>> through struct sockaddr_l2.
>>
>> This patch adds the dst_type parameter to l2cap_chan_connect so we
>> are able to pass the address type info from user-space down to
>> hci_conn layer.
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> ?include/net/bluetooth/l2cap.h | ? ?2 +-
>> ?net/bluetooth/l2cap_core.c ? ?| ? 11 ++++++-----
>> ?net/bluetooth/l2cap_sock.c ? ?| ? ?2 +-
>> ?3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index d14967e..262628c 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -911,7 +911,7 @@ struct l2cap_chan *l2cap_chan_create(void);
>> ?void l2cap_chan_close(struct l2cap_chan *chan, int reason);
>> ?void l2cap_chan_destroy(struct l2cap_chan *chan);
>> ?int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bdaddr_t *dst);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bdaddr_t *dst, u8 dst_type);
>> ?int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u32 priority);
>> ?void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 91b02de..e0dc4ca 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -1131,7 +1131,8 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr
>> ? ? ? return c1;
>> ?}
>>
>> -int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
>> +int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
>> + ? ? ? ? ? ? ? ? ? ? bdaddr_t *dst, u8 dst_type)
>> ?{
>> ? ? ? struct sock *sk = chan->sk;
>> ? ? ? bdaddr_t *src = &bt_sk(sk)->src;
>> @@ -1141,8 +1142,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
>> ? ? ? __u8 auth_type;
>> ? ? ? int err;
>>
>> - ? ? BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __le16_to_cpu(chan->psm));
>> + ? ? BT_DBG("%s -> %s (type %u) psm 0x%2.2x", batostr(src), batostr(dst),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dst_type, __le16_to_cpu(chan->psm));
>
> If you change style for other functions you might change it here as well.
Sorry, didn't follow you here.
>>
>> ? ? ? hdev = hci_get_route(dst, src);
>> ? ? ? if (!hdev)
>> @@ -1216,10 +1217,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
>> ? ? ? auth_type = l2cap_get_auth_type(chan);
>>
>> ? ? ? if (chan->dcid == L2CAP_CID_LE_DATA)
>> - ? ? ? ? ? ? hcon = hci_connect(hdev, LE_LINK, dst, MGMT_ADDR_LE_RANDOM,
>> + ? ? ? ? ? ? hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chan->sec_level, auth_type);
>> ? ? ? else
>> - ? ? ? ? ? ? hcon = hci_connect(hdev, ACL_LINK, dst, MGMT_ADDR_BREDR,
>> + ? ? ? ? ? ? hcon = hci_connect(hdev, ACL_LINK, dst, dst_type,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chan->sec_level, auth_type);
>>
>> ? ? ? if (IS_ERR(hcon)) {
>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> index 53e563f..4b38cf8 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -124,7 +124,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
>> ? ? ? ? ? ? ? return -EINVAL;
>>
>> ? ? ? err = l2cap_chan_connect(chan, la.l2_psm, __le16_to_cpu(la.l2_cid),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? &la.l2_bdaddr);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? &la.l2_bdaddr, la.l2_bdaddr_type);
>
> Would it make sense to pass la as a parameter?
I guess so. If we all agree this is a valid refactoring, I take care
of it in a separated patch.
Thanks,
Andre
Hi Andrei,
On Wed, Mar 28, 2012 at 5:48 AM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi Andre,
>
> On Tue, Mar 27, 2012 at 08:59:35PM -0300, Andre Guedes wrote:
>> This patch adds the dst_type parameter to hci_connect. This way, we
>> use the address type information from upper layer instead of searching
>> it on the advertising cache.
>>
>> The dst_type parameter is ignored for BR/EDR connection establishment.
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> ?include/net/bluetooth/hci_core.h | ? ?2 +-
>> ?net/bluetooth/hci_conn.c ? ? ? ? | ? 12 ++++--------
>> ?net/bluetooth/l2cap_core.c ? ? ? | ? ?4 ++--
>> ?net/bluetooth/mgmt.c ? ? ? ? ? ? | ? ?8 ++++----
>> ?net/bluetooth/sco.c ? ? ? ? ? ? ?| ? ?3 ++-
>> ?5 files changed, 13 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index fa2c778..178eaa1 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -574,7 +574,7 @@ int hci_chan_del(struct hci_chan *chan);
>> ?void hci_chan_list_flush(struct hci_conn *conn);
>>
>> ?struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u8 sec_level, __u8 auth_type);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u8 dst_type, __u8 sec_level, __u8 auth_type);
>> ?int hci_conn_check_link_mode(struct hci_conn *conn);
>> ?int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
>> ?int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 947172b..4f24d3f 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -514,7 +514,8 @@ EXPORT_SYMBOL(hci_get_route);
>>
>> ?/* Create SCO, ACL or LE connection.
>> ? * Device _must_ be locked */
>> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
>> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __u8 dst_type, __u8 sec_level, __u8 auth_type)
>> ?{
>> ? ? ? struct hci_conn *acl;
>> ? ? ? struct hci_conn *sco;
>> @@ -523,21 +524,16 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>> ? ? ? BT_DBG("%s dst %s", hdev->name, batostr(dst));
>>
>> ? ? ? if (type == LE_LINK) {
>> - ? ? ? ? ? ? struct adv_entry *entry;
>> -
>> ? ? ? ? ? ? ? le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
>> ? ? ? ? ? ? ? if (le)
>> ? ? ? ? ? ? ? ? ? ? ? return ERR_PTR(-EBUSY);
>>
>> - ? ? ? ? ? ? entry = hci_find_adv_entry(hdev, dst);
>> - ? ? ? ? ? ? if (!entry)
>> - ? ? ? ? ? ? ? ? ? ? return ERR_PTR(-EHOSTUNREACH);
>> -
>> ? ? ? ? ? ? ? le = hci_conn_add(hdev, LE_LINK, dst);
>> ? ? ? ? ? ? ? if (!le)
>> ? ? ? ? ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);
>>
>> - ? ? ? ? ? ? le->dst_type = entry->bdaddr_type;
>> + ? ? ? ? ? ? le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
>
> Is this checking validity of dst_type? Just split to check and then
> assign.
It is not a validity check. dst_type parameter is in MGMT address type
format and we need to translate it to HCI address type format before
the assignment.
Thanks,
Andre
Hi Andre,
On Tue, Mar 27, 2012 at 08:59:36PM -0300, Andre Guedes wrote:
> In order to establish a LE connection we need the address type
> information. User-space already pass this information to kernel
> through struct sockaddr_l2.
>
> This patch adds the dst_type parameter to l2cap_chan_connect so we
> are able to pass the address type info from user-space down to
> hci_conn layer.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> net/bluetooth/l2cap_core.c | 11 ++++++-----
> net/bluetooth/l2cap_sock.c | 2 +-
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d14967e..262628c 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -911,7 +911,7 @@ struct l2cap_chan *l2cap_chan_create(void);
> void l2cap_chan_close(struct l2cap_chan *chan, int reason);
> void l2cap_chan_destroy(struct l2cap_chan *chan);
> int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> - bdaddr_t *dst);
> + bdaddr_t *dst, u8 dst_type);
> int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> u32 priority);
> void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 91b02de..e0dc4ca 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1131,7 +1131,8 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr
> return c1;
> }
>
> -int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
> +int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> + bdaddr_t *dst, u8 dst_type)
> {
> struct sock *sk = chan->sk;
> bdaddr_t *src = &bt_sk(sk)->src;
> @@ -1141,8 +1142,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
> __u8 auth_type;
> int err;
>
> - BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
> - __le16_to_cpu(chan->psm));
> + BT_DBG("%s -> %s (type %u) psm 0x%2.2x", batostr(src), batostr(dst),
> + dst_type, __le16_to_cpu(chan->psm));
If you change style for other functions you might change it here as well.
>
> hdev = hci_get_route(dst, src);
> if (!hdev)
> @@ -1216,10 +1217,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
> auth_type = l2cap_get_auth_type(chan);
>
> if (chan->dcid == L2CAP_CID_LE_DATA)
> - hcon = hci_connect(hdev, LE_LINK, dst, MGMT_ADDR_LE_RANDOM,
> + hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
> chan->sec_level, auth_type);
> else
> - hcon = hci_connect(hdev, ACL_LINK, dst, MGMT_ADDR_BREDR,
> + hcon = hci_connect(hdev, ACL_LINK, dst, dst_type,
> chan->sec_level, auth_type);
>
> if (IS_ERR(hcon)) {
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 53e563f..4b38cf8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -124,7 +124,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
> return -EINVAL;
>
> err = l2cap_chan_connect(chan, la.l2_psm, __le16_to_cpu(la.l2_cid),
> - &la.l2_bdaddr);
> + &la.l2_bdaddr, la.l2_bdaddr_type);
Would it make sense to pass la as a parameter?
Best regards
Andrei Emeltchenko
Hi Andre,
On Tue, Mar 27, 2012 at 08:59:35PM -0300, Andre Guedes wrote:
> This patch adds the dst_type parameter to hci_connect. This way, we
> use the address type information from upper layer instead of searching
> it on the advertising cache.
>
> The dst_type parameter is ignored for BR/EDR connection establishment.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_conn.c | 12 ++++--------
> net/bluetooth/l2cap_core.c | 4 ++--
> net/bluetooth/mgmt.c | 8 ++++----
> net/bluetooth/sco.c | 3 ++-
> 5 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fa2c778..178eaa1 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -574,7 +574,7 @@ int hci_chan_del(struct hci_chan *chan);
> void hci_chan_list_flush(struct hci_conn *conn);
>
> struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> - __u8 sec_level, __u8 auth_type);
> + __u8 dst_type, __u8 sec_level, __u8 auth_type);
> int hci_conn_check_link_mode(struct hci_conn *conn);
> int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
> int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 947172b..4f24d3f 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -514,7 +514,8 @@ EXPORT_SYMBOL(hci_get_route);
>
> /* Create SCO, ACL or LE connection.
> * Device _must_ be locked */
> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
> + __u8 dst_type, __u8 sec_level, __u8 auth_type)
> {
> struct hci_conn *acl;
> struct hci_conn *sco;
> @@ -523,21 +524,16 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
> BT_DBG("%s dst %s", hdev->name, batostr(dst));
>
> if (type == LE_LINK) {
> - struct adv_entry *entry;
> -
> le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> if (le)
> return ERR_PTR(-EBUSY);
>
> - entry = hci_find_adv_entry(hdev, dst);
> - if (!entry)
> - return ERR_PTR(-EHOSTUNREACH);
> -
> le = hci_conn_add(hdev, LE_LINK, dst);
> if (!le)
> return ERR_PTR(-ENOMEM);
>
> - le->dst_type = entry->bdaddr_type;
> + le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
> + ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
Is this checking validity of dst_type? Just split to check and then
assign.
Best regards
Andrei Emeltchenko
In order to establish a LE connection we need the address type
information. User-space already pass this information to kernel
through struct sockaddr_l2.
This patch adds the dst_type parameter to l2cap_chan_connect so we
are able to pass the address type info from user-space down to
hci_conn layer.
Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 +-
net/bluetooth/l2cap_core.c | 11 ++++++-----
net/bluetooth/l2cap_sock.c | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d14967e..262628c 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -911,7 +911,7 @@ struct l2cap_chan *l2cap_chan_create(void);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
void l2cap_chan_destroy(struct l2cap_chan *chan);
int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
- bdaddr_t *dst);
+ bdaddr_t *dst, u8 dst_type);
int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
u32 priority);
void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 91b02de..e0dc4ca 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1131,7 +1131,8 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr
return c1;
}
-int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
+int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
+ bdaddr_t *dst, u8 dst_type)
{
struct sock *sk = chan->sk;
bdaddr_t *src = &bt_sk(sk)->src;
@@ -1141,8 +1142,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
__u8 auth_type;
int err;
- BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
- __le16_to_cpu(chan->psm));
+ BT_DBG("%s -> %s (type %u) psm 0x%2.2x", batostr(src), batostr(dst),
+ dst_type, __le16_to_cpu(chan->psm));
hdev = hci_get_route(dst, src);
if (!hdev)
@@ -1216,10 +1217,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
auth_type = l2cap_get_auth_type(chan);
if (chan->dcid == L2CAP_CID_LE_DATA)
- hcon = hci_connect(hdev, LE_LINK, dst, MGMT_ADDR_LE_RANDOM,
+ hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
chan->sec_level, auth_type);
else
- hcon = hci_connect(hdev, ACL_LINK, dst, MGMT_ADDR_BREDR,
+ hcon = hci_connect(hdev, ACL_LINK, dst, dst_type,
chan->sec_level, auth_type);
if (IS_ERR(hcon)) {
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 53e563f..4b38cf8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -124,7 +124,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
return -EINVAL;
err = l2cap_chan_connect(chan, la.l2_psm, __le16_to_cpu(la.l2_cid),
- &la.l2_bdaddr);
+ &la.l2_bdaddr, la.l2_bdaddr_type);
if (err)
return err;
--
1.7.9.4
This patch adds the dst_type parameter to hci_connect. This way, we
use the address type information from upper layer instead of searching
it on the advertising cache.
The dst_type parameter is ignored for BR/EDR connection establishment.
Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_conn.c | 12 ++++--------
net/bluetooth/l2cap_core.c | 4 ++--
net/bluetooth/mgmt.c | 8 ++++----
net/bluetooth/sco.c | 3 ++-
5 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fa2c778..178eaa1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -574,7 +574,7 @@ int hci_chan_del(struct hci_chan *chan);
void hci_chan_list_flush(struct hci_conn *conn);
struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u8 sec_level, __u8 auth_type);
+ __u8 dst_type, __u8 sec_level, __u8 auth_type);
int hci_conn_check_link_mode(struct hci_conn *conn);
int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 947172b..4f24d3f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -514,7 +514,8 @@ EXPORT_SYMBOL(hci_get_route);
/* Create SCO, ACL or LE connection.
* Device _must_ be locked */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
+struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
+ __u8 dst_type, __u8 sec_level, __u8 auth_type)
{
struct hci_conn *acl;
struct hci_conn *sco;
@@ -523,21 +524,16 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
BT_DBG("%s dst %s", hdev->name, batostr(dst));
if (type == LE_LINK) {
- struct adv_entry *entry;
-
le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
if (le)
return ERR_PTR(-EBUSY);
- entry = hci_find_adv_entry(hdev, dst);
- if (!entry)
- return ERR_PTR(-EHOSTUNREACH);
-
le = hci_conn_add(hdev, LE_LINK, dst);
if (!le)
return ERR_PTR(-ENOMEM);
- le->dst_type = entry->bdaddr_type;
+ le->dst_type = (dst_type == MGMT_ADDR_LE_RANDOM) ?
+ ADDR_LE_DEV_RANDOM : ADDR_LE_DEV_PUBLIC;
hci_le_connect(le);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3caff27..91b02de 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1216,10 +1216,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d
auth_type = l2cap_get_auth_type(chan);
if (chan->dcid == L2CAP_CID_LE_DATA)
- hcon = hci_connect(hdev, LE_LINK, dst,
+ hcon = hci_connect(hdev, LE_LINK, dst, MGMT_ADDR_LE_RANDOM,
chan->sec_level, auth_type);
else
- hcon = hci_connect(hdev, ACL_LINK, dst,
+ hcon = hci_connect(hdev, ACL_LINK, dst, MGMT_ADDR_BREDR,
chan->sec_level, auth_type);
if (IS_ERR(hcon)) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5a56e0c..7fae5e3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1911,11 +1911,11 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
auth_type = HCI_AT_DEDICATED_BONDING_MITM;
if (cp->addr.type == MGMT_ADDR_BREDR)
- conn = hci_connect(hdev, ACL_LINK, &cp->addr.bdaddr, sec_level,
- auth_type);
+ conn = hci_connect(hdev, ACL_LINK, &cp->addr.bdaddr,
+ cp->addr.type, sec_level, auth_type);
else
- conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr, sec_level,
- auth_type);
+ conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr,
+ cp->addr.type, sec_level, auth_type);
memset(&rp, 0, sizeof(rp));
bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8bf26d1..a48de4c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -196,7 +196,8 @@ static int sco_connect(struct sock *sk)
else
type = SCO_LINK;
- hcon = hci_connect(hdev, type, dst, BT_SECURITY_LOW, HCI_AT_NO_BONDING);
+ hcon = hci_connect(hdev, type, dst, MGMT_ADDR_BREDR, BT_SECURITY_LOW,
+ HCI_AT_NO_BONDING);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
--
1.7.9.4
This patch adds the address type info to struct sockaddr_l2 so
user-space can inform the remote device address type required
to establish a LE connection.
Soon, instead of looking the advertising cache up to discover the
address type, we'll use this address type info to establish LE
connections.
Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/l2cap.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f6f0500..d14967e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -57,6 +57,7 @@ struct sockaddr_l2 {
__le16 l2_psm;
bdaddr_t l2_bdaddr;
__le16 l2_cid;
+ __u8 l2_bdaddr_type;
};
/* L2CAP socket options */
--
1.7.9.4