From: Johan Hedberg <[email protected]>
The kernel needs to know whether it should connect to a device over
BR/EDR or over LE. This is particularly important in the future when
dual-mode device may be connectable also over LE. It is also important
if/when we decide to move the LE advertisement cache from the kernel
into user-space. Adding the type to the mgmt command also ensures
conformance with the latest mgmt API spec.
Signed-off-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/mgmt.h | 4 ++--
net/bluetooth/mgmt.c | 13 ++++++-------
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 8b07a83..bfdb04b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -172,11 +172,11 @@ struct mgmt_cp_set_io_capability {
#define MGMT_OP_PAIR_DEVICE 0x0014
struct mgmt_cp_pair_device {
- bdaddr_t bdaddr;
+ struct mgmt_addr_info addr;
__u8 io_cap;
} __packed;
struct mgmt_rp_pair_device {
- bdaddr_t bdaddr;
+ struct mgmt_addr_info addr;
__u8 status;
} __packed;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5562c21..555e31f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1333,7 +1333,8 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status)
struct mgmt_rp_pair_device rp;
struct hci_conn *conn = cmd->user_data;
- bacpy(&rp.bdaddr, &conn->dst);
+ bacpy(&rp.addr.bdaddr, &conn->dst);
+ rp.addr.type = link_to_mgmt(conn->type, conn->dst_type);
rp.status = status;
cmd_complete(cmd->sk, cmd->index, MGMT_OP_PAIR_DEVICE, &rp, sizeof(rp));
@@ -1366,7 +1367,6 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
struct hci_dev *hdev;
struct mgmt_cp_pair_device *cp;
struct pending_cmd *cmd;
- struct adv_entry *entry;
u8 sec_level, auth_type;
struct hci_conn *conn;
int err;
@@ -1390,12 +1390,11 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
else
auth_type = HCI_AT_DEDICATED_BONDING_MITM;
- entry = hci_find_adv_entry(hdev, &cp->bdaddr);
- if (entry)
- conn = hci_connect(hdev, LE_LINK, &cp->bdaddr, sec_level,
+ if (cp->addr.type == MGMT_ADDR_BREDR)
+ conn = hci_connect(hdev, ACL_LINK, &cp->addr.bdaddr, sec_level,
auth_type);
else
- conn = hci_connect(hdev, ACL_LINK, &cp->bdaddr, sec_level,
+ conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr, sec_level,
auth_type);
if (IS_ERR(conn)) {
@@ -1417,7 +1416,7 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
}
/* For LE, just connecting isn't a proof that the pairing finished */
- if (!entry)
+ if (cp->addr.type == MGMT_ADDR_BREDR)
conn->connect_cfm_cb = pairing_complete_cb;
conn->security_cfm_cb = pairing_complete_cb;
--
1.7.7.1
Hi Johan,
* [email protected] <[email protected]> [2011-11-11 00:07:35 +0200]:
> From: Johan Hedberg <[email protected]>
>
> When possible cmd_complete should be returned instead of cmd_status
> since it contains the remote address (this helps user-space track what
> exactly failed).
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/mgmt.c | 13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
Both patches were applied, thanks.
Gustavo
Hi Johan,
On 11/10/2011 2:33 PM, Johan Hedberg wrote:
> Hi Brian,
>
> On Thu, Nov 10, 2011, Brian Gix wrote:
>> On 11/10/2011 2:07 PM, [email protected] wrote:
>>>
>>> - entry = hci_find_adv_entry(hdev,&cp->bdaddr);
>>> - if (entry)
>>> - conn = hci_connect(hdev, LE_LINK,&cp->bdaddr, sec_level,
>>> + if (cp->addr.type == MGMT_ADDR_BREDR)
>>> + conn = hci_connect(hdev, ACL_LINK,&cp->addr.bdaddr, sec_level,
>>> auth_type);
>>> else
>>> - conn = hci_connect(hdev, ACL_LINK,&cp->bdaddr, sec_level,
>>> + conn = hci_connect(hdev, LE_LINK,&cp->addr.bdaddr, sec_level,
>>> auth_type);
>>>
>>
>> Are we differentiating between Dual Mode and BR/EDR here? If we
>> are, we may want to reverse the logic so that it connects with an
>> LE_LINK if the addr type == MGMT_ADDR_LE, and then connects to an
>> ACL_LINK otherwise (as the else).
>>
>> Unless this is being implimented as a bitmask, in which case the if
>> would be "if (cp->addr.type& MGMT_ADDR_BREDR)", at which point I
>> have no objection.
>>
>> Because of course Dual mode devices must use the ACL_LINK between
>> each other.
>
> The idea here isn't to push connection type selection to the kernel but
> to expect user-space to explicitly say how it wants to connect. If
> user-space wants to connect over LE it'll provide either ADDR_LE_PUBLIC
> or ADDR_LE_PRIVATE. So it doesn't really matter what way around the
> if-statement is formulated (I chose this way around since it meant one
> comparison instead of two).
OK, I'm fine with that.
--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Brian,
On Thu, Nov 10, 2011, Brian Gix wrote:
> On 11/10/2011 2:07 PM, [email protected] wrote:
> >
> >- entry = hci_find_adv_entry(hdev,&cp->bdaddr);
> >- if (entry)
> >- conn = hci_connect(hdev, LE_LINK,&cp->bdaddr, sec_level,
> >+ if (cp->addr.type == MGMT_ADDR_BREDR)
> >+ conn = hci_connect(hdev, ACL_LINK,&cp->addr.bdaddr, sec_level,
> > auth_type);
> > else
> >- conn = hci_connect(hdev, ACL_LINK,&cp->bdaddr, sec_level,
> >+ conn = hci_connect(hdev, LE_LINK,&cp->addr.bdaddr, sec_level,
> > auth_type);
> >
>
> Are we differentiating between Dual Mode and BR/EDR here? If we
> are, we may want to reverse the logic so that it connects with an
> LE_LINK if the addr type == MGMT_ADDR_LE, and then connects to an
> ACL_LINK otherwise (as the else).
>
> Unless this is being implimented as a bitmask, in which case the if
> would be "if (cp->addr.type & MGMT_ADDR_BREDR)", at which point I
> have no objection.
>
> Because of course Dual mode devices must use the ACL_LINK between
> each other.
The idea here isn't to push connection type selection to the kernel but
to expect user-space to explicitly say how it wants to connect. If
user-space wants to connect over LE it'll provide either ADDR_LE_PUBLIC
or ADDR_LE_PRIVATE. So it doesn't really matter what way around the
if-statement is formulated (I chose this way around since it meant one
comparison instead of two).
One thing missing here though is the pushing of cp->addr.type onward to
hci_connect so that it doesn't need to do a cache look-up anymore. I
left it out since it belongs to a separate patch and because I think the
other connection mechanisms (e.g. L2CAP sockets) also use hci_connect
and they don't (yet) get the address type from user-space.
Johan
Hi Johan,
On 11/10/2011 2:07 PM, [email protected] wrote:
>
> - entry = hci_find_adv_entry(hdev,&cp->bdaddr);
> - if (entry)
> - conn = hci_connect(hdev, LE_LINK,&cp->bdaddr, sec_level,
> + if (cp->addr.type == MGMT_ADDR_BREDR)
> + conn = hci_connect(hdev, ACL_LINK,&cp->addr.bdaddr, sec_level,
> auth_type);
> else
> - conn = hci_connect(hdev, ACL_LINK,&cp->bdaddr, sec_level,
> + conn = hci_connect(hdev, LE_LINK,&cp->addr.bdaddr, sec_level,
> auth_type);
>
Are we differentiating between Dual Mode and BR/EDR here? If we are, we
may want to reverse the logic so that it connects with an LE_LINK if the
addr type == MGMT_ADDR_LE, and then connects to an ACL_LINK otherwise
(as the else).
Unless this is being implimented as a bitmask, in which case the if
would be "if (cp->addr.type & MGMT_ADDR_BREDR)", at which point I have
no objection.
Because of course Dual mode devices must use the ACL_LINK between each
other.
--
Brian Gix
[email protected]
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
From: Johan Hedberg <[email protected]>
When possible cmd_complete should be returned instead of cmd_status
since it contains the remote address (this helps user-space track what
exactly failed).
Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/mgmt.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 555e31f..3d33168 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1366,6 +1366,7 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
{
struct hci_dev *hdev;
struct mgmt_cp_pair_device *cp;
+ struct mgmt_rp_pair_device rp;
struct pending_cmd *cmd;
u8 sec_level, auth_type;
struct hci_conn *conn;
@@ -1397,14 +1398,22 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len)
conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr, sec_level,
auth_type);
+ memset(&rp, 0, sizeof(rp));
+ bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
+ rp.addr.type = cp->addr.type;
+
if (IS_ERR(conn)) {
- err = PTR_ERR(conn);
+ rp.status = -PTR_ERR(conn);
+ err = cmd_complete(sk, index, MGMT_OP_PAIR_DEVICE,
+ &rp, sizeof(rp));
goto unlock;
}
if (conn->connect_cfm_cb) {
hci_conn_put(conn);
- err = cmd_status(sk, index, MGMT_OP_PAIR_DEVICE, EBUSY);
+ rp.status = EBUSY;
+ err = cmd_complete(sk, index, MGMT_OP_PAIR_DEVICE,
+ &rp, sizeof(rp));
goto unlock;
}
--
1.7.7.1