Hi all,
This patchset exports discovery and LE connection parameters via debugfs
filesystem. This might be useful for those who would like to do experiments
with different parameters.
BR,
Andre
Andre Guedes (3):
Bluetooth: Discovery parameters per hci_dev
Bluetooth: Export discovery parameters on debugfs
Bluetooth: Set discovery parameters
Vinicius Costa Gomes (3):
Bluetooth: Keep the LE connection parameters in its own structure
Bluetooth: Add LE connection parameters to debugfs
Bluetooth: Add support for setting LE connection parameters via
debugfs
include/net/bluetooth/hci_core.h | 23 ++++++
net/bluetooth/hci_conn.c | 11 +--
net/bluetooth/hci_core.c | 21 +++++-
net/bluetooth/hci_sysfs.c | 150 +++++++++++++++++++++++++++++++++++++++
net/bluetooth/mgmt.c | 15 ++--
5 files changed, 208 insertions(+), 12 deletions(-)
--
1.8.3.4
Hi Marcel,
On Wed, Aug 21, 2013 at 7:56 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Andre,
>
>>>> This patch adds support for setting discovery parameters through
>>>> debugfs filesystem.
>>>>
>>>> Signed-off-by: Andre Guedes <[email protected]>
>>>> ---
>>>> net/bluetooth/hci_sysfs.c | 45 +++++++++++++++++++++++++++++++++++++++=
+++++-
>>>> 1 file changed, 44 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>>>> index 90cb53b..17ebc4a 100644
>>>> --- a/net/bluetooth/hci_sysfs.c
>>>> +++ b/net/bluetooth/hci_sysfs.c
>>>> @@ -553,9 +553,52 @@ static int discovery_parameters_open(struct inode=
*inode, struct file *file)
>>>> return single_open(file, discovery_parameters_show, inode->i_priv=
ate);
>>>> }
>>>>
>>>> +static ssize_t discovery_parameters_write(struct file *file,
>>>> + const char __user *data,
>>>> + size_t count, loff_t *offset)
>>>> +{
>>>> + struct seq_file *sfile =3D file->private_data;
>>>> + struct hci_dev *hdev =3D sfile->private;
>>>> + struct discovery_param param;
>>>> + char *buf;
>>>> + int n;
>>>> + ssize_t res =3D 0;
>>>> +
>>>> + /* We don't allow partial writes */
>>>> + if (*offset !=3D 0)
>>>> + return 0;
>>>> +
>>>> + buf =3D kzalloc(count, GFP_KERNEL);
>>>> + if (!buf)
>>>> + return 0;
>>>> +
>>>> + if (copy_from_user(buf, data, count))
>>>> + goto out;
>>>> +
>>>> + memset(¶m, 0, sizeof(param));
>>>> +
>>>> + n =3D sscanf(buf, "%hhx %hx %hx %u %u %hhx %hhx",
>>>> + ¶m.scan_type, ¶m.scan_interval, ¶m.scan_w=
indow,
>>>> + ¶m.le_scan_duration, ¶m.interleaved_scan_dura=
tion,
>>>> + ¶m.interleaved_inquiry_length,
>>>> + ¶m.bredr_inquiry_length);
>>>
>>> I am not a huge fan of this crazy. I need a manual to know exactly what=
to do here. That is just silly. Unless things are a bit self-explanatory o=
n how you can tweak things, I am not really interested here.
>>
>> What do you think about having the following hierarchy:
>>
>> hciX/
>> |
>> |--> discovery_param/
>> | |--> scan_type
>> | |--> scan_window
>> | |--> scan_interval
>> | |--> le_scan_timeout
>> | |--> interleaved_scan_timeout
>> | |--> interleaved_inquiry_length
>> | |--> bredr_inquiry_length
>> |
>> |--> le_conn_param/
>> |--> scan_window
>> |--> scan_interval
>> |--> min_conn_interval
>> |--> max_conn_interval
>> |--> min_ce_length
>> |--> max_ce_length
>> |--> supervision_timeout
>> |--> conn_latency
>>
>> It sounds much self-explanatory to me.
>
> don't go crazy with the nesting here. Toplevel directory entries are just=
fine. The one tricky part you will run into is that things like the scan_w=
indow for connections suppose to be per device and not just global. Since y=
ou can learn many of these parameters from AD or GATT. And when connecting =
to one specific device, you suppose to use these ones and not some global c=
ommon value.
Ok, so I'll place them all in hciX/ and add the prefix "discov_" for
the discovery files and "le_conn_" for the LE connection ones.
About scan_window and scan_interval parameters, you are right, they
are supposed to be per device. That feature should be addressed in the
new LE connection approach we've discussed a while ago (which, BTW, I
already started the implementation). These exported parameters, on the
other hand, are the default values the kernel uses in case specific
parameters were not set for a given device.
The idea behind exporting LE connection parameters via debugfs is to
facilitate the implementation of experiments. Today the parameters are
hard coded in kernel and we have to recompile it each time we want
experiment new values. IMO, we should export scan_window and
scan_interval since they are very useful for running experiments.
>>> Also intermixing LE with BR/EDR is a bit pointless here. I would prefer=
if they are separate and not that BR/EDR only controller exports LE knobs =
and and LE only controller exports BR/EDR knobs.
>>
>> Sure. I'll fix this in the v2 patchset. We can also extend this to
>> others debugfs entries (e.g. inquiry_cache and auto_accept_delay files
>> should not be exported if controller is LE-only).
>
> Of course they should be separated.
I'll fix it.
>> In the current code, hdev is exported during device registration
>> (hci_register_dev). However, in hci_register_dev(), hdev has not been
>> initialized yet so we are not able to know is controller is BR/EDR, LE
>> or dual mode. In order to fix this, we need to postpone exporting hdev
>> by moving hci_add_sysfs call from hci_register_dev() to
>> hci_dev_open().
>>
>> As a side effect, the hciX/ directory will be added to debugfs once
>> the adapter is powered on and it will be removed once the adapter is
>> powered off. Do you see any problem with that?
>
> That is not acceptable and also not how the kernel actually works. We are=
initializing the adapter no matter what. So the information is available b=
efore power on.
Fair enough, let's forget about this approach.
I just realized we can fix this by exporting BR/EDR and LE specific
information once hdev is initialized. All we need to do is checking if
HCI_SETUP flag is set just after __hci_init call in hci_open_dev().
Thanks for your feedback,
Andre
Hi Andre,
>>> This patch adds support for setting discovery parameters through
>>> debugfs filesystem.
>>>
>>> Signed-off-by: Andre Guedes <[email protected]>
>>> ---
>>> net/bluetooth/hci_sysfs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>>> index 90cb53b..17ebc4a 100644
>>> --- a/net/bluetooth/hci_sysfs.c
>>> +++ b/net/bluetooth/hci_sysfs.c
>>> @@ -553,9 +553,52 @@ static int discovery_parameters_open(struct inode *inode, struct file *file)
>>> return single_open(file, discovery_parameters_show, inode->i_private);
>>> }
>>>
>>> +static ssize_t discovery_parameters_write(struct file *file,
>>> + const char __user *data,
>>> + size_t count, loff_t *offset)
>>> +{
>>> + struct seq_file *sfile = file->private_data;
>>> + struct hci_dev *hdev = sfile->private;
>>> + struct discovery_param param;
>>> + char *buf;
>>> + int n;
>>> + ssize_t res = 0;
>>> +
>>> + /* We don't allow partial writes */
>>> + if (*offset != 0)
>>> + return 0;
>>> +
>>> + buf = kzalloc(count, GFP_KERNEL);
>>> + if (!buf)
>>> + return 0;
>>> +
>>> + if (copy_from_user(buf, data, count))
>>> + goto out;
>>> +
>>> + memset(¶m, 0, sizeof(param));
>>> +
>>> + n = sscanf(buf, "%hhx %hx %hx %u %u %hhx %hhx",
>>> + ¶m.scan_type, ¶m.scan_interval, ¶m.scan_window,
>>> + ¶m.le_scan_duration, ¶m.interleaved_scan_duration,
>>> + ¶m.interleaved_inquiry_length,
>>> + ¶m.bredr_inquiry_length);
>>
>> I am not a huge fan of this crazy. I need a manual to know exactly what to do here. That is just silly. Unless things are a bit self-explanatory on how you can tweak things, I am not really interested here.
>
> What do you think about having the following hierarchy:
>
> hciX/
> |
> |--> discovery_param/
> | |--> scan_type
> | |--> scan_window
> | |--> scan_interval
> | |--> le_scan_timeout
> | |--> interleaved_scan_timeout
> | |--> interleaved_inquiry_length
> | |--> bredr_inquiry_length
> |
> |--> le_conn_param/
> |--> scan_window
> |--> scan_interval
> |--> min_conn_interval
> |--> max_conn_interval
> |--> min_ce_length
> |--> max_ce_length
> |--> supervision_timeout
> |--> conn_latency
>
> It sounds much self-explanatory to me.
don't go crazy with the nesting here. Toplevel directory entries are just fine. The one tricky part you will run into is that things like the scan_window for connections suppose to be per device and not just global. Since you can learn many of these parameters from AD or GATT. And when connecting to one specific device, you suppose to use these ones and not some global common value.
>> Also intermixing LE with BR/EDR is a bit pointless here. I would prefer if they are separate and not that BR/EDR only controller exports LE knobs and and LE only controller exports BR/EDR knobs.
>
> Sure. I'll fix this in the v2 patchset. We can also extend this to
> others debugfs entries (e.g. inquiry_cache and auto_accept_delay files
> should not be exported if controller is LE-only).
Of course they should be separated.
> In the current code, hdev is exported during device registration
> (hci_register_dev). However, in hci_register_dev(), hdev has not been
> initialized yet so we are not able to know is controller is BR/EDR, LE
> or dual mode. In order to fix this, we need to postpone exporting hdev
> by moving hci_add_sysfs call from hci_register_dev() to
> hci_dev_open().
>
> As a side effect, the hciX/ directory will be added to debugfs once
> the adapter is powered on and it will be removed once the adapter is
> powered off. Do you see any problem with that?
That is not acceptable and also not how the kernel actually works. We are initializing the adapter no matter what. So the information is available before power on.
Regards
Marcel
Hi Marcel,
On Sat, Aug 10, 2013 at 1:10 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Andre,
>
>> From: Vinicius Costa Gomes <[email protected]>
>>
>> Only reading the parameters is supported for now.
>
> don't come me with this short kernel commits. Write a proper explanation on what you are doing here and why.
According to what we discussed in the anterior patch, I'll completely
rework this one and write a proper explanation here.
>>
>> Signed-off-by: Vinicius Costa Gomes <[email protected]>
>> ---
>> net/bluetooth/hci_sysfs.c | 33 +++++++++++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>> index 17ebc4a..5993150 100644
>> --- a/net/bluetooth/hci_sysfs.c
>> +++ b/net/bluetooth/hci_sysfs.c
>> @@ -603,6 +603,35 @@ static const struct file_operations discovery_parameters_fops = {
>> .release = single_release,
>> };
>>
>> +static int le_conn_parameters_show(struct seq_file *f, void *p)
>> +{
>> + struct hci_dev *hdev = f->private;
>> + struct le_conn_params *params = &hdev->le_conn_params;
>> +
>> + hci_dev_lock(hdev);
>> +
>> + seq_printf(f, "0x%.4x 0x%.4x 0x%.4x 0x%.4x 0x%.4x\n",
>> + params->scan_interval, params->scan_window,
>> + params->conn_interval_min, params->conn_interval_max,
>> + params->supervision_timeout);
>> +
>> + hci_dev_unlock(hdev);
>> +
>> + return 0;
>> +}
>> +
>> +static int le_conn_parameters_open(struct inode *inode, struct file *file)
>> +{
>> + return single_open(file, le_conn_parameters_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations le_conn_parameters_fops = {
>> + .open = le_conn_parameters_open,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .release = single_release,
>> +};
>> +
>> void hci_init_sysfs(struct hci_dev *hdev)
>> {
>> struct device *dev = &hdev->dev;
>> @@ -647,6 +676,10 @@ int hci_add_sysfs(struct hci_dev *hdev)
>>
>> debugfs_create_file("discovery_parameters", 0644, hdev->debugfs, hdev,
>> &discovery_parameters_fops);
>> +
>> + debugfs_create_file("le_conn_parameters", 0644, hdev->debugfs, hdev,
>> + &le_conn_parameters_fops);
>> +
>> return 0;
>> }
>
> What is the point exactly of exposing this if you are BR/EDR only controller?
Yeah, there is no point. This will be fixed in v2.
BR,
Andre
Hi Marcel,
On Sat, Aug 10, 2013 at 1:09 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Andre,
>
>> This patch adds support for setting discovery parameters through
>> debugfs filesystem.
>>
>> Signed-off-by: Andre Guedes <[email protected]>
>> ---
>> net/bluetooth/hci_sysfs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>> index 90cb53b..17ebc4a 100644
>> --- a/net/bluetooth/hci_sysfs.c
>> +++ b/net/bluetooth/hci_sysfs.c
>> @@ -553,9 +553,52 @@ static int discovery_parameters_open(struct inode *inode, struct file *file)
>> return single_open(file, discovery_parameters_show, inode->i_private);
>> }
>>
>> +static ssize_t discovery_parameters_write(struct file *file,
>> + const char __user *data,
>> + size_t count, loff_t *offset)
>> +{
>> + struct seq_file *sfile = file->private_data;
>> + struct hci_dev *hdev = sfile->private;
>> + struct discovery_param param;
>> + char *buf;
>> + int n;
>> + ssize_t res = 0;
>> +
>> + /* We don't allow partial writes */
>> + if (*offset != 0)
>> + return 0;
>> +
>> + buf = kzalloc(count, GFP_KERNEL);
>> + if (!buf)
>> + return 0;
>> +
>> + if (copy_from_user(buf, data, count))
>> + goto out;
>> +
>> + memset(¶m, 0, sizeof(param));
>> +
>> + n = sscanf(buf, "%hhx %hx %hx %u %u %hhx %hhx",
>> + ¶m.scan_type, ¶m.scan_interval, ¶m.scan_window,
>> + ¶m.le_scan_duration, ¶m.interleaved_scan_duration,
>> + ¶m.interleaved_inquiry_length,
>> + ¶m.bredr_inquiry_length);
>
> I am not a huge fan of this crazy. I need a manual to know exactly what to do here. That is just silly. Unless things are a bit self-explanatory on how you can tweak things, I am not really interested here.
What do you think about having the following hierarchy:
hciX/
|
|--> discovery_param/
| |--> scan_type
| |--> scan_window
| |--> scan_interval
| |--> le_scan_timeout
| |--> interleaved_scan_timeout
| |--> interleaved_inquiry_length
| |--> bredr_inquiry_length
|
|--> le_conn_param/
|--> scan_window
|--> scan_interval
|--> min_conn_interval
|--> max_conn_interval
|--> min_ce_length
|--> max_ce_length
|--> supervision_timeout
|--> conn_latency
It sounds much self-explanatory to me.
> Also intermixing LE with BR/EDR is a bit pointless here. I would prefer if they are separate and not that BR/EDR only controller exports LE knobs and and LE only controller exports BR/EDR knobs.
Sure. I'll fix this in the v2 patchset. We can also extend this to
others debugfs entries (e.g. inquiry_cache and auto_accept_delay files
should not be exported if controller is LE-only).
In the current code, hdev is exported during device registration
(hci_register_dev). However, in hci_register_dev(), hdev has not been
initialized yet so we are not able to know is controller is BR/EDR, LE
or dual mode. In order to fix this, we need to postpone exporting hdev
by moving hci_add_sysfs call from hci_register_dev() to
hci_dev_open().
As a side effect, the hciX/ directory will be added to debugfs once
the adapter is powered on and it will be removed once the adapter is
powered off. Do you see any problem with that?
BR,
Andre
Hi Andre,
> From: Vinicius Costa Gomes <[email protected]>
>
> Only reading the parameters is supported for now.
don't come me with this short kernel commits. Write a proper explanation on what you are doing here and why.
>
> Signed-off-by: Vinicius Costa Gomes <[email protected]>
> ---
> net/bluetooth/hci_sysfs.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 17ebc4a..5993150 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -603,6 +603,35 @@ static const struct file_operations discovery_parameters_fops = {
> .release = single_release,
> };
>
> +static int le_conn_parameters_show(struct seq_file *f, void *p)
> +{
> + struct hci_dev *hdev = f->private;
> + struct le_conn_params *params = &hdev->le_conn_params;
> +
> + hci_dev_lock(hdev);
> +
> + seq_printf(f, "0x%.4x 0x%.4x 0x%.4x 0x%.4x 0x%.4x\n",
> + params->scan_interval, params->scan_window,
> + params->conn_interval_min, params->conn_interval_max,
> + params->supervision_timeout);
> +
> + hci_dev_unlock(hdev);
> +
> + return 0;
> +}
> +
> +static int le_conn_parameters_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, le_conn_parameters_show, inode->i_private);
> +}
> +
> +static const struct file_operations le_conn_parameters_fops = {
> + .open = le_conn_parameters_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> void hci_init_sysfs(struct hci_dev *hdev)
> {
> struct device *dev = &hdev->dev;
> @@ -647,6 +676,10 @@ int hci_add_sysfs(struct hci_dev *hdev)
>
> debugfs_create_file("discovery_parameters", 0644, hdev->debugfs, hdev,
> &discovery_parameters_fops);
> +
> + debugfs_create_file("le_conn_parameters", 0644, hdev->debugfs, hdev,
> + &le_conn_parameters_fops);
> +
> return 0;
> }
What is the point exactly of exposing this if you are BR/EDR only controller?
Regards
Marcel
Hi Andre,
> This patch adds support for setting discovery parameters through
> debugfs filesystem.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/hci_sysfs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 90cb53b..17ebc4a 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -553,9 +553,52 @@ static int discovery_parameters_open(struct inode *inode, struct file *file)
> return single_open(file, discovery_parameters_show, inode->i_private);
> }
>
> +static ssize_t discovery_parameters_write(struct file *file,
> + const char __user *data,
> + size_t count, loff_t *offset)
> +{
> + struct seq_file *sfile = file->private_data;
> + struct hci_dev *hdev = sfile->private;
> + struct discovery_param param;
> + char *buf;
> + int n;
> + ssize_t res = 0;
> +
> + /* We don't allow partial writes */
> + if (*offset != 0)
> + return 0;
> +
> + buf = kzalloc(count, GFP_KERNEL);
> + if (!buf)
> + return 0;
> +
> + if (copy_from_user(buf, data, count))
> + goto out;
> +
> + memset(¶m, 0, sizeof(param));
> +
> + n = sscanf(buf, "%hhx %hx %hx %u %u %hhx %hhx",
> + ¶m.scan_type, ¶m.scan_interval, ¶m.scan_window,
> + ¶m.le_scan_duration, ¶m.interleaved_scan_duration,
> + ¶m.interleaved_inquiry_length,
> + ¶m.bredr_inquiry_length);
I am not a huge fan of this crazy. I need a manual to know exactly what to do here. That is just silly. Unless things are a bit self-explanatory on how you can tweak things, I am not really interested here.
Also intermixing LE with BR/EDR is a bit pointless here. I would prefer if they are separate and not that BR/EDR only controller exports LE knobs and and LE only controller exports BR/EDR knobs.
Regards
Marcel
From: Vinicius Costa Gomes <[email protected]>
This will allow for easier parameterization of these fields. This is
the first step for allowing to change the parameters during runtime.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
include/net/bluetooth/hci_core.h | 10 ++++++++++
net/bluetooth/hci_conn.c | 11 ++++++-----
net/bluetooth/hci_core.c | 8 ++++++++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0a02fdb..24b91ae 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -117,6 +117,14 @@ struct oob_data {
u8 randomizer[16];
};
+struct le_conn_params {
+ u16 scan_interval;
+ u16 scan_window;
+ u16 conn_interval_min;
+ u16 conn_interval_max;
+ u16 supervision_timeout;
+};
+
#define HCI_MAX_SHORT_NAME_LENGTH 10
struct amp_assoc {
@@ -287,6 +295,8 @@ struct hci_dev {
struct delayed_work le_scan_disable;
+ struct le_conn_params le_conn_params;
+
__s8 adv_tx_power;
__u8 adv_data[HCI_MAX_AD_LENGTH];
__u8 adv_data_len;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 6c7f363..f944757 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -34,6 +34,7 @@
static void hci_le_create_connection(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
+ struct le_conn_params *params = &hdev->le_conn_params;
struct hci_cp_le_create_conn cp;
conn->state = BT_CONNECT;
@@ -42,13 +43,13 @@ static void hci_le_create_connection(struct hci_conn *conn)
conn->sec_level = BT_SECURITY_LOW;
memset(&cp, 0, sizeof(cp));
- cp.scan_interval = __constant_cpu_to_le16(0x0060);
- cp.scan_window = __constant_cpu_to_le16(0x0030);
+ cp.scan_interval = __cpu_to_le16(params->scan_interval);
+ cp.scan_window = __cpu_to_le16(params->scan_window);
bacpy(&cp.peer_addr, &conn->dst);
cp.peer_addr_type = conn->dst_type;
- cp.conn_interval_min = __constant_cpu_to_le16(0x0028);
- cp.conn_interval_max = __constant_cpu_to_le16(0x0038);
- cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
+ cp.conn_interval_min = __cpu_to_le16(params->conn_interval_min);
+ cp.conn_interval_max = __cpu_to_le16(params->conn_interval_max);
+ cp.supervision_timeout = __cpu_to_le16(params->supervision_timeout);
cp.min_ce_len = __constant_cpu_to_le16(0x0000);
cp.max_ce_len = __constant_cpu_to_le16(0x0000);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8750663..806d0c3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2079,6 +2079,7 @@ struct hci_dev *hci_alloc_dev(void)
{
struct hci_dev *hdev;
struct discovery_param *discov;
+ struct le_conn_params *conn_params;
hdev = kzalloc(sizeof(struct hci_dev), GFP_KERNEL);
if (!hdev)
@@ -2134,6 +2135,13 @@ struct hci_dev *hci_alloc_dev(void)
discov->interleaved_inquiry_length = DISCOV_INTERLEAVED_INQUIRY_LEN;
discov->bredr_inquiry_length = DISCOV_BREDR_INQUIRY_LEN;
+ conn_params = &hdev->le_conn_params;
+ conn_params->scan_interval = 0x0060;
+ conn_params->scan_window = 0x0030;
+ conn_params->conn_interval_min = 0x0028;
+ conn_params->conn_interval_max = 0x0038;
+ conn_params->supervision_timeout = 0x002a;
+
return hdev;
}
EXPORT_SYMBOL(hci_alloc_dev);
--
1.8.3.4
From: Vinicius Costa Gomes <[email protected]>
This will allow for much more pratical measures of the impact of
different connection parameters in different scenarios.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 5993150..f3bf419 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -620,6 +620,47 @@ static int le_conn_parameters_show(struct seq_file *f, void *p)
return 0;
}
+static ssize_t le_conn_parameters_write(struct file *filp,
+ const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ struct seq_file *seqf = filp->private_data;
+ struct hci_dev *hdev = seqf->private;
+ struct le_conn_params params;
+ char buffer[36];
+ int n;
+
+ /* No partial writes */
+ if (*ppos != 0)
+ return 0;
+
+ /* Format: '0x0000 0x0000 0x0000 0x0000 0x0000' length 35 */
+ if (cnt != 35)
+ return -ENOSPC;
+
+ memset(buffer, 0, sizeof(buffer));
+
+ if (copy_from_user(buffer, ubuf, cnt))
+ return 0;
+
+ memset(¶ms, 0, sizeof(params));
+
+ n = sscanf(buffer, "%hx %hx %hx %hx %hx", ¶ms.scan_interval,
+ ¶ms.scan_window, ¶ms.conn_interval_min,
+ ¶ms.conn_interval_max, ¶ms.supervision_timeout);
+
+ if (n < 5)
+ return -EINVAL;
+
+ hci_dev_lock(hdev);
+
+ memcpy(&hdev->le_conn_params, ¶ms, sizeof(params));
+
+ hci_dev_unlock(hdev);
+
+ return cnt;
+}
+
static int le_conn_parameters_open(struct inode *inode, struct file *file)
{
return single_open(file, le_conn_parameters_show, inode->i_private);
@@ -627,6 +668,7 @@ static int le_conn_parameters_open(struct inode *inode, struct file *file)
static const struct file_operations le_conn_parameters_fops = {
.open = le_conn_parameters_open,
+ .write = le_conn_parameters_write,
.read = seq_read,
.llseek = seq_lseek,
.release = single_release,
--
1.8.3.4
From: Vinicius Costa Gomes <[email protected]>
Only reading the parameters is supported for now.
Signed-off-by: Vinicius Costa Gomes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 17ebc4a..5993150 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -603,6 +603,35 @@ static const struct file_operations discovery_parameters_fops = {
.release = single_release,
};
+static int le_conn_parameters_show(struct seq_file *f, void *p)
+{
+ struct hci_dev *hdev = f->private;
+ struct le_conn_params *params = &hdev->le_conn_params;
+
+ hci_dev_lock(hdev);
+
+ seq_printf(f, "0x%.4x 0x%.4x 0x%.4x 0x%.4x 0x%.4x\n",
+ params->scan_interval, params->scan_window,
+ params->conn_interval_min, params->conn_interval_max,
+ params->supervision_timeout);
+
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+static int le_conn_parameters_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, le_conn_parameters_show, inode->i_private);
+}
+
+static const struct file_operations le_conn_parameters_fops = {
+ .open = le_conn_parameters_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void hci_init_sysfs(struct hci_dev *hdev)
{
struct device *dev = &hdev->dev;
@@ -647,6 +676,10 @@ int hci_add_sysfs(struct hci_dev *hdev)
debugfs_create_file("discovery_parameters", 0644, hdev->debugfs, hdev,
&discovery_parameters_fops);
+
+ debugfs_create_file("le_conn_parameters", 0644, hdev->debugfs, hdev,
+ &le_conn_parameters_fops);
+
return 0;
}
--
1.8.3.4
This patch exports discovery parameters thought debugfs filesystem.
The default values are:
$ cat discovery_parameters
0x01 0x0012 0x0012 10240 5120 0x04 0x08
The file format is the following:
Scanning type
Scanning interval
Scanning window
Scanning duration (in milliseconds) in LE discovery
Scanning duration (in milliseconds) in interleaved discovery
Inquiry length used in interleaved discovery
Inquiry length used in BR/EDR discovery
Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 7ad6ecf..90cb53b 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -531,6 +531,35 @@ static int auto_accept_delay_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(auto_accept_delay_fops, auto_accept_delay_get,
auto_accept_delay_set, "%llu\n");
+static int discovery_parameters_show(struct seq_file *f, void *ptr)
+{
+ struct hci_dev *hdev = f->private;
+ struct discovery_param *p = &hdev->discovery_param;
+
+ hci_dev_lock(hdev);
+
+ seq_printf(f, "0x%.2x 0x%.4x 0x%.4x %u %u 0x%.2x 0x%.2x\n",
+ p->scan_type, p->scan_interval, p->scan_window,
+ p->le_scan_duration, p->interleaved_scan_duration,
+ p->interleaved_inquiry_length, p->bredr_inquiry_length);
+
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+static int discovery_parameters_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, discovery_parameters_show, inode->i_private);
+}
+
+static const struct file_operations discovery_parameters_fops = {
+ .open = discovery_parameters_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
void hci_init_sysfs(struct hci_dev *hdev)
{
struct device *dev = &hdev->dev;
@@ -572,6 +601,9 @@ int hci_add_sysfs(struct hci_dev *hdev)
debugfs_create_file("auto_accept_delay", 0444, hdev->debugfs, hdev,
&auto_accept_delay_fops);
+
+ debugfs_create_file("discovery_parameters", 0444, hdev->debugfs, hdev,
+ &discovery_parameters_fops);
return 0;
}
--
1.8.3.4
This patch adds support for setting discovery parameters through
debugfs filesystem.
Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_sysfs.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 90cb53b..17ebc4a 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -553,9 +553,52 @@ static int discovery_parameters_open(struct inode *inode, struct file *file)
return single_open(file, discovery_parameters_show, inode->i_private);
}
+static ssize_t discovery_parameters_write(struct file *file,
+ const char __user *data,
+ size_t count, loff_t *offset)
+{
+ struct seq_file *sfile = file->private_data;
+ struct hci_dev *hdev = sfile->private;
+ struct discovery_param param;
+ char *buf;
+ int n;
+ ssize_t res = 0;
+
+ /* We don't allow partial writes */
+ if (*offset != 0)
+ return 0;
+
+ buf = kzalloc(count, GFP_KERNEL);
+ if (!buf)
+ return 0;
+
+ if (copy_from_user(buf, data, count))
+ goto out;
+
+ memset(¶m, 0, sizeof(param));
+
+ n = sscanf(buf, "%hhx %hx %hx %u %u %hhx %hhx",
+ ¶m.scan_type, ¶m.scan_interval, ¶m.scan_window,
+ ¶m.le_scan_duration, ¶m.interleaved_scan_duration,
+ ¶m.interleaved_inquiry_length,
+ ¶m.bredr_inquiry_length);
+ if (n != 7)
+ goto out;
+
+ hci_dev_lock(hdev);
+ memcpy(&hdev->discovery_param, ¶m, sizeof(param));
+ hci_dev_unlock(hdev);
+
+ res = count;
+out:
+ kfree(buf);
+ return res;
+}
+
static const struct file_operations discovery_parameters_fops = {
.open = discovery_parameters_open,
.read = seq_read,
+ .write = discovery_parameters_write,
.llseek = seq_lseek,
.release = single_release,
};
@@ -602,7 +645,7 @@ int hci_add_sysfs(struct hci_dev *hdev)
debugfs_create_file("auto_accept_delay", 0444, hdev->debugfs, hdev,
&auto_accept_delay_fops);
- debugfs_create_file("discovery_parameters", 0444, hdev->debugfs, hdev,
+ debugfs_create_file("discovery_parameters", 0644, hdev->debugfs, hdev,
&discovery_parameters_fops);
return 0;
}
--
1.8.3.4
This patch adds discovery parameters to struct hci_dev. This way, we
will be able to configure the discovery parameters per hci device.
Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci_core.h | 13 +++++++++++++
net/bluetooth/hci_core.c | 13 ++++++++++++-
net/bluetooth/mgmt.c | 15 +++++++++------
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f77885e..0a02fdb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -127,6 +127,17 @@ struct amp_assoc {
__u8 data[HCI_MAX_AMP_ASSOC_SIZE];
};
+struct discovery_param {
+ u8 scan_type;
+ u16 scan_interval;
+ u16 scan_window;
+ unsigned int interleaved_scan_duration;
+ unsigned int le_scan_duration;
+
+ u8 interleaved_inquiry_length;
+ u8 bredr_inquiry_length;
+};
+
#define HCI_MAX_PAGES 3
#define NUM_REASSEMBLY 4
@@ -280,6 +291,8 @@ struct hci_dev {
__u8 adv_data[HCI_MAX_AD_LENGTH];
__u8 adv_data_len;
+ struct discovery_param discovery_param;
+
int (*open)(struct hci_dev *hdev);
int (*close)(struct hci_dev *hdev);
int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b821b19..8750663 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2013,6 +2013,7 @@ static void le_scan_disable_work_complete(struct hci_dev *hdev, u8 status)
{
/* General inquiry access code (GIAC) */
u8 lap[3] = { 0x33, 0x8b, 0x9e };
+ struct discovery_param *discov = &hdev->discovery_param;
struct hci_request req;
struct hci_cp_inquiry cp;
int err;
@@ -2034,7 +2035,7 @@ static void le_scan_disable_work_complete(struct hci_dev *hdev, u8 status)
memset(&cp, 0, sizeof(cp));
memcpy(&cp.lap, lap, sizeof(cp.lap));
- cp.length = DISCOV_INTERLEAVED_INQUIRY_LEN;
+ cp.length = discov->interleaved_inquiry_length;
hci_req_add(&req, HCI_OP_INQUIRY, sizeof(cp), &cp);
hci_dev_lock(hdev);
@@ -2077,6 +2078,7 @@ static void le_scan_disable_work(struct work_struct *work)
struct hci_dev *hci_alloc_dev(void)
{
struct hci_dev *hdev;
+ struct discovery_param *discov;
hdev = kzalloc(sizeof(struct hci_dev), GFP_KERNEL);
if (!hdev)
@@ -2123,6 +2125,15 @@ struct hci_dev *hci_alloc_dev(void)
hci_init_sysfs(hdev);
discovery_init(hdev);
+ discov = &hdev->discovery_param;
+ discov->scan_type = LE_SCAN_ACTIVE;
+ discov->scan_interval = DISCOV_LE_SCAN_INT;
+ discov->scan_window = DISCOV_LE_SCAN_WIN;
+ discov->le_scan_duration = DISCOV_LE_TIMEOUT;
+ discov->interleaved_scan_duration = DISCOV_INTERLEAVED_TIMEOUT;
+ discov->interleaved_inquiry_length = DISCOV_INTERLEAVED_INQUIRY_LEN;
+ discov->bredr_inquiry_length = DISCOV_BREDR_INQUIRY_LEN;
+
return hdev;
}
EXPORT_SYMBOL(hci_alloc_dev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fedc539..2b4a3ba 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2642,6 +2642,8 @@ static int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
static void start_discovery_complete(struct hci_dev *hdev, u8 status)
{
+ struct discovery_param *discov = &hdev->discovery_param;
+
BT_DBG("status %d", status);
if (status) {
@@ -2658,12 +2660,12 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
switch (hdev->discovery.type) {
case DISCOV_TYPE_LE:
queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
- DISCOV_LE_TIMEOUT);
+ discov->le_scan_duration);
break;
case DISCOV_TYPE_INTERLEAVED:
queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
- DISCOV_INTERLEAVED_TIMEOUT);
+ discov->interleaved_scan_duration);
break;
case DISCOV_TYPE_BREDR:
@@ -2678,6 +2680,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len)
{
struct mgmt_cp_start_discovery *cp = data;
+ struct discovery_param *discov = &hdev->discovery_param;
struct pending_cmd *cmd;
struct hci_cp_le_set_scan_param param_cp;
struct hci_cp_le_set_scan_enable enable_cp;
@@ -2739,7 +2742,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
memset(&inq_cp, 0, sizeof(inq_cp));
memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
- inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;
+ inq_cp.length = discov->bredr_inquiry_length;
hci_req_add(&req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
break;
@@ -2775,9 +2778,9 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
}
memset(¶m_cp, 0, sizeof(param_cp));
- param_cp.type = LE_SCAN_ACTIVE;
- param_cp.interval = cpu_to_le16(DISCOV_LE_SCAN_INT);
- param_cp.window = cpu_to_le16(DISCOV_LE_SCAN_WIN);
+ param_cp.type = discov->scan_type;
+ param_cp.interval = cpu_to_le16(discov->scan_interval);
+ param_cp.window = cpu_to_le16(discov->scan_window);
hci_req_add(&req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
¶m_cp);
--
1.8.3.4