Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1622981 > unrolled thread
| Started by | "Felipe F. Tonello" <eu@felipetonello.com> |
|---|---|
| First post | 2017-04-13 14:30 +0200 |
| Last post | 2017-04-13 20:50 +0200 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v5 BlueZ 4/4] Bluetooth: Handle Slave Connection Interval Range AD "Felipe F. Tonello" <eu@felipetonello.com> - 2017-04-13 14:30 +0200
Re: [PATCH v5 BlueZ 4/4] Bluetooth: Handle Slave Connection Interval Range AD Vinicius Costa Gomes <vinicius.gomes@intel.com> - 2017-04-13 20:30 +0200
Re: [PATCH v5 BlueZ 4/4] Bluetooth: Handle Slave Connection Interval Range AD Luiz Augusto von Dentz <luiz.dentz@gmail.com> - 2017-04-13 20:50 +0200
| From | "Felipe F. Tonello" <eu@felipetonello.com> |
|---|---|
| Date | 2017-04-13 14:30 +0200 |
| Subject | [PATCH v5 BlueZ 4/4] Bluetooth: Handle Slave Connection Interval Range AD |
| Message-ID | <tvM2R-2Wl-9@gated-at.bofh.it> |
The Slave Connection Interval Range data type contains the Peripheral's
preferred connection interval range, for all logical connections.
It is useful to parse it in the Kernel so there is no multiple calls to
MGMT interface to update the device connection parameters and subsequent
connection command call to this device will use proper connection
parameters. This saves context-switches and eliminates user-space to
update the connection parameters each time a device is found or
bluetoothd is restarted and so on. Also, there is no need for the
user-space to know care about it because if the slave device wishes to
persist with these parameters, it should use the L2CAP connection
parameters upade request upon a completed connection.
Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
net/bluetooth/mgmt.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1fba2a03f8ae..ea5d6c85f173 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7442,6 +7442,46 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
return true;
}
+static bool has_eir_slave_conn_int(const u8 *eir_data, u8 eir_len,
+ u16 *min_conn, u16 *max_conn)
+{
+ u16 len = 0;
+ const u8 EIR_SLAVE_CONN_INT = 0x12; /* Slave Connection Interval Range */
+
+ while (len < eir_len - 1) {
+ u8 field_len = eir_data[0];
+ const u8 *data;
+ u8 data_len;
+
+ /* Check for the end of EIR */
+ if (field_len == 0)
+ break;
+
+ len += field_len + 1;
+
+ /* Do not continue EIR Data parsing if got
+ * incorrect length
+ */
+ if (len > eir_len)
+ break;
+
+ data = &eir_data[2];
+ data_len = field_len - 1;
+
+ if (eir_data[1] == EIR_SLAVE_CONN_INT) {
+ if (data_len < 4)
+ break;
+ *min_conn = le16_to_cpu(&data[0]);
+ *max_conn = le16_to_cpu(&data[2]);
+ return true;
+ }
+
+ eir_data += field_len + 1;
+ }
+
+ return false;
+}
+
void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
@@ -7449,6 +7489,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
char buf[512];
struct mgmt_ev_device_found *ev = (void *)buf;
size_t ev_size;
+ struct hci_conn *hcon;
/* Don't send events for a non-kernel initiated discovery. With
* LE one exception is if we have pend_le_reports > 0 in which
@@ -7521,6 +7562,18 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
+ /* Search for Slave Connection Interval AD */
+ hcon = hci_conn_hash_lookup_le(hdev, bdaddr, addr_type);
+ if (hcon) {
+ u16 min_conn_int, max_conn_int;
+
+ if (has_eir_slave_conn_int(ev->eir, ev->eir_len,
+ &min_conn_int, &max_conn_int)) {
+ hcon->le_conn_min_interval = min_conn_int;
+ hcon->le_conn_max_interval = max_conn_int;
+ }
+ }
+
mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
}
--
2.12.2
[toc] | [next] | [standalone]
| From | Vinicius Costa Gomes <vinicius.gomes@intel.com> |
|---|---|
| Date | 2017-04-13 20:30 +0200 |
| Message-ID | <tvRFf-6Or-11@gated-at.bofh.it> |
| In reply to | #1622981 |
Hi Felipe,
"Felipe F. Tonello" <eu@felipetonello.com> writes:
> The Slave Connection Interval Range data type contains the Peripheral's
> preferred connection interval range, for all logical connections.
>
> It is useful to parse it in the Kernel so there is no multiple calls to
> MGMT interface to update the device connection parameters and subsequent
> connection command call to this device will use proper connection
> parameters. This saves context-switches and eliminates user-space to
> update the connection parameters each time a device is found or
> bluetoothd is restarted and so on. Also, there is no need for the
> user-space to know care about it because if the slave device wishes to
> persist with these parameters, it should use the L2CAP connection
> parameters upade request upon a completed connection.
nitpick: upade -> update
>
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> net/bluetooth/mgmt.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 1fba2a03f8ae..ea5d6c85f173 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -7442,6 +7442,46 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
> return true;
> }
>
> +static bool has_eir_slave_conn_int(const u8 *eir_data, u8 eir_len,
> + u16 *min_conn, u16 *max_conn)
> +{
> + u16 len = 0;
> + const u8 EIR_SLAVE_CONN_INT = 0x12; /* Slave Connection Interval Range */
> +
> + while (len < eir_len - 1) {
> + u8 field_len = eir_data[0];
> + const u8 *data;
> + u8 data_len;
> +
> + /* Check for the end of EIR */
> + if (field_len == 0)
> + break;
> +
> + len += field_len + 1;
> +
> + /* Do not continue EIR Data parsing if got
> + * incorrect length
> + */
> + if (len > eir_len)
> + break;
> +
> + data = &eir_data[2];
> + data_len = field_len - 1;
> +
> + if (eir_data[1] == EIR_SLAVE_CONN_INT) {
> + if (data_len < 4)
> + break;
> + *min_conn = le16_to_cpu(&data[0]);
> + *max_conn = le16_to_cpu(&data[2]);
> + return true;
> + }
> +
> + eir_data += field_len + 1;
> + }
> +
> + return false;
> +}
> +
> void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
> u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
> @@ -7449,6 +7489,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> char buf[512];
> struct mgmt_ev_device_found *ev = (void *)buf;
> size_t ev_size;
> + struct hci_conn *hcon;
>
> /* Don't send events for a non-kernel initiated discovery. With
> * LE one exception is if we have pend_le_reports > 0 in which
> @@ -7521,6 +7562,18 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
> ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
>
> + /* Search for Slave Connection Interval AD */
> + hcon = hci_conn_hash_lookup_le(hdev, bdaddr, addr_type);
> + if (hcon) {
> + u16 min_conn_int, max_conn_int;
> +
> + if (has_eir_slave_conn_int(ev->eir, ev->eir_len,
> + &min_conn_int, &max_conn_int)) {
> + hcon->le_conn_min_interval = min_conn_int;
> + hcon->le_conn_max_interval = max_conn_int;
> + }
> + }
> +
It's been some time that I looked at this code, so I could be missing
something, but I got the feeling that this part would make more sense if
it was at process_adv_report(), there's even the check for a pending
connection, so no need to redo that here.
Apart from this, the series looks good.
> mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
> }
>
> --
> 2.12.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
[toc] | [prev] | [next] | [standalone]
| From | Luiz Augusto von Dentz <luiz.dentz@gmail.com> |
|---|---|
| Date | 2017-04-13 20:50 +0200 |
| Subject | Re: [PATCH v5 BlueZ 4/4] Bluetooth: Handle Slave Connection Interval Range AD |
| Message-ID | <tvRYB-6WU-5@gated-at.bofh.it> |
| In reply to | #1623246 |
Hi,
On Thu, Apr 13, 2017 at 9:24 PM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi Felipe,
>
> "Felipe F. Tonello" <eu@felipetonello.com> writes:
>
>> The Slave Connection Interval Range data type contains the Peripheral's
>> preferred connection interval range, for all logical connections.
>>
>> It is useful to parse it in the Kernel so there is no multiple calls to
>> MGMT interface to update the device connection parameters and subsequent
>> connection command call to this device will use proper connection
>> parameters. This saves context-switches and eliminates user-space to
>> update the connection parameters each time a device is found or
>> bluetoothd is restarted and so on. Also, there is no need for the
>> user-space to know care about it because if the slave device wishes to
>> persist with these parameters, it should use the L2CAP connection
>> parameters upade request upon a completed connection.
>
> nitpick: upade -> update
>
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>> net/bluetooth/mgmt.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 1fba2a03f8ae..ea5d6c85f173 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -7442,6 +7442,46 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
>> return true;
>> }
>>
>> +static bool has_eir_slave_conn_int(const u8 *eir_data, u8 eir_len,
>> + u16 *min_conn, u16 *max_conn)
>> +{
>> + u16 len = 0;
>> + const u8 EIR_SLAVE_CONN_INT = 0x12; /* Slave Connection Interval Range */
>> +
>> + while (len < eir_len - 1) {
>> + u8 field_len = eir_data[0];
>> + const u8 *data;
>> + u8 data_len;
>> +
>> + /* Check for the end of EIR */
>> + if (field_len == 0)
>> + break;
>> +
>> + len += field_len + 1;
>> +
>> + /* Do not continue EIR Data parsing if got
>> + * incorrect length
>> + */
>> + if (len > eir_len)
>> + break;
>> +
>> + data = &eir_data[2];
>> + data_len = field_len - 1;
>> +
>> + if (eir_data[1] == EIR_SLAVE_CONN_INT) {
>> + if (data_len < 4)
>> + break;
>> + *min_conn = le16_to_cpu(&data[0]);
>> + *max_conn = le16_to_cpu(&data[2]);
>> + return true;
>> + }
>> +
>> + eir_data += field_len + 1;
>> + }
>> +
>> + return false;
>> +}
>> +
>> void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
>> u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
>> @@ -7449,6 +7489,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> char buf[512];
>> struct mgmt_ev_device_found *ev = (void *)buf;
>> size_t ev_size;
>> + struct hci_conn *hcon;
>>
>> /* Don't send events for a non-kernel initiated discovery. With
>> * LE one exception is if we have pend_le_reports > 0 in which
>> @@ -7521,6 +7562,18 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
>> ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
>>
>> + /* Search for Slave Connection Interval AD */
>> + hcon = hci_conn_hash_lookup_le(hdev, bdaddr, addr_type);
>> + if (hcon) {
>> + u16 min_conn_int, max_conn_int;
>> +
>> + if (has_eir_slave_conn_int(ev->eir, ev->eir_len,
>> + &min_conn_int, &max_conn_int)) {
>> + hcon->le_conn_min_interval = min_conn_int;
>> + hcon->le_conn_max_interval = max_conn_int;
>> + }
>> + }
>> +
>
> It's been some time that I looked at this code, so I could be missing
> something, but I got the feeling that this part would make more sense if
> it was at process_adv_report(), there's even the check for a pending
> connection, so no need to redo that here.
Actually I would use the AD only in case the device is marked for
auto-connect or there is a connection pending, so the parameters are
only used for the connection alone and are not persisted.
> Apart from this, the series looks good.
>
>
>> mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
>> }
>>
>> --
>> 2.12.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Cheers,
> --
> Vinicius
--
Luiz Augusto von Dentz
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web