* [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-05 11:14 ` Luiz Augusto von Dentz
2017-12-04 8:07 ` [RFC 2/9] Bluetooth: Impmlement extended adv enable Jaganath Kanakkassery
` (7 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch reads the number of advertising sets in the controller
during init and save it in hdev.
Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
Instance 0 is reserved for "Set Advertising" which means that
multi adv is supported only for total sets - 1.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 7 +++++++
include/net/bluetooth/hci_core.h | 4 ++++
net/bluetooth/hci_core.c | 11 +++++++++--
net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
net/bluetooth/mgmt.c | 6 +++---
5 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index f0868df..59df823 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -398,6 +398,7 @@ enum {
#define HCI_LE_SLAVE_FEATURES 0x08
#define HCI_LE_PING 0x10
#define HCI_LE_DATA_LEN_EXT 0x20
+#define HCI_LE_EXT_ADV 0x10
#define HCI_LE_EXT_SCAN_POLICY 0x80
/* Connection modes */
@@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
__le16 max_ce_len;
} __packed;
+#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
+struct hci_rp_le_read_num_supported_adv_sets {
+ __u8 status;
+ __u8 num_of_sets;
+} __packed;
+
/* ---- HCI Events ---- */
#define HCI_EV_INQUIRY_COMPLETE 0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 554671c..4a7a4ae 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -219,6 +219,7 @@ struct hci_dev {
__u8 features[HCI_MAX_PAGES][8];
__u8 le_features[8];
__u8 le_white_list_size;
+ __u8 le_no_of_adv_sets;
__u8 le_states[8];
__u8 commands[64];
__u8 hci_ver;
@@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
hci_dev_test_flag(dev, HCI_SC_ENABLED))
+/* Extended advertising support */
+#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
+
/* ----- HCI protocols ----- */
#define HCI_PROTO_DEFER 0x01
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a91a09a..3472572 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
}
+ if (ext_adv_capable(hdev)) {
+ /* Read LE Number of Supported Advertising Sets */
+ hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
+ 0, NULL);
+ }
+
hci_set_le_support(req);
}
@@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
memset(adv_instance->scan_rsp_data, 0,
sizeof(adv_instance->scan_rsp_data));
} else {
- if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
- instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
+ if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
+ instance < 1 || instance > hdev->le_no_of_adv_sets)
return -EOVERFLOW;
adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
@@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
hdev->le_max_tx_time = 0x0148;
hdev->le_max_rx_len = 0x001b;
hdev->le_max_rx_time = 0x0148;
+ hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a26ae80..06d8c1b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
le_set_scan_enable_complete(hdev, cp->enable);
}
+static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
+
+ BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
+ rp->num_of_sets);
+
+ if (rp->status)
+ return;
+
+ /* Instance 0 is reserved for Set Advertising */
+ hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
+ HCI_MAX_ADV_INSTANCES);
+}
+
static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
struct sk_buff *skb)
{
@@ -3128,6 +3144,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_le_set_ext_scan_enable(hdev, skb);
break;
+ case HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS:
+ hci_cc_le_read_num_adv_sets(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 07a3cc2..ffd5f7b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5998,7 +5998,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
rp->supported_flags = cpu_to_le32(supported_flags);
rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
- rp->max_instances = HCI_MAX_ADV_INSTANCES;
+ rp->max_instances = hdev->le_no_of_adv_sets;
rp->num_instances = hdev->adv_instance_cnt;
instance = rp->instance;
@@ -6187,7 +6187,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
status);
- if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
+ if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
MGMT_STATUS_INVALID_PARAMS);
@@ -6422,7 +6422,7 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
MGMT_STATUS_REJECTED);
- if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
+ if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
MGMT_STATUS_INVALID_PARAMS);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-04 8:07 ` [RFC 1/9] Bluetooth: Read no of adv sets during init Jaganath Kanakkassery
@ 2017-12-05 11:14 ` Luiz Augusto von Dentz
2017-12-07 7:57 ` Jaganath K
2017-12-08 8:40 ` Marcel Holtmann
0 siblings, 2 replies; 23+ messages in thread
From: Luiz Augusto von Dentz @ 2017-12-05 11:14 UTC (permalink / raw)
To: Jaganath Kanakkassery
Cc: linux-bluetooth@vger.kernel.org, Jaganath Kanakkassery
Hi Jaganath,
On Mon, Dec 4, 2017 at 10:07 AM, Jaganath Kanakkassery
<jaganath.k.os@gmail.com> wrote:
> This patch reads the number of advertising sets in the controller
> during init and save it in hdev.
>
> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>
> Instance 0 is reserved for "Set Advertising" which means that
> multi adv is supported only for total sets - 1.
>
> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
> ---
> include/net/bluetooth/hci.h | 7 +++++++
> include/net/bluetooth/hci_core.h | 4 ++++
> net/bluetooth/hci_core.c | 11 +++++++++--
> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
> net/bluetooth/mgmt.c | 6 +++---
> 5 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index f0868df..59df823 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -398,6 +398,7 @@ enum {
> #define HCI_LE_SLAVE_FEATURES 0x08
> #define HCI_LE_PING 0x10
> #define HCI_LE_DATA_LEN_EXT 0x20
> +#define HCI_LE_EXT_ADV 0x10
> #define HCI_LE_EXT_SCAN_POLICY 0x80
>
> /* Connection modes */
> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
> __le16 max_ce_len;
> } __packed;
>
> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
> +struct hci_rp_le_read_num_supported_adv_sets {
> + __u8 status;
> + __u8 num_of_sets;
> +} __packed;
> +
> /* ---- HCI Events ---- */
> #define HCI_EV_INQUIRY_COMPLETE 0x01
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 554671c..4a7a4ae 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -219,6 +219,7 @@ struct hci_dev {
> __u8 features[HCI_MAX_PAGES][8];
> __u8 le_features[8];
> __u8 le_white_list_size;
> + __u8 le_no_of_adv_sets;
I was expecting some flag that indicates if the instances would be
maintained by the controller or not, but perhaps this is covered in
other patches.
> __u8 le_states[8];
> __u8 commands[64];
> __u8 hci_ver;
> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>
> +/* Extended advertising support */
> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
> +
> /* ----- HCI protocols ----- */
> #define HCI_PROTO_DEFER 0x01
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index a91a09a..3472572 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
> }
>
> + if (ext_adv_capable(hdev)) {
> + /* Read LE Number of Supported Advertising Sets */
> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
> + 0, NULL);
> + }
> +
> hci_set_le_support(req);
> }
>
> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
> memset(adv_instance->scan_rsp_data, 0,
> sizeof(adv_instance->scan_rsp_data));
> } else {
> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
> + instance < 1 || instance > hdev->le_no_of_adv_sets)
> return -EOVERFLOW;
>
> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
> hdev->le_max_tx_time = 0x0148;
> hdev->le_max_rx_len = 0x001b;
> hdev->le_max_rx_time = 0x0148;
> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>
> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a26ae80..06d8c1b 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
> le_set_scan_enable_complete(hdev, cp->enable);
> }
>
> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
> +
> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
> + rp->num_of_sets);
> +
> + if (rp->status)
> + return;
> +
> + /* Instance 0 is reserved for Set Advertising */
> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
> + HCI_MAX_ADV_INSTANCES);
Id say if the controller cannot support as many instances as the host
stack then we should keep using the software based scheduler, another
option would be to let the user select what scheduler it wants to use
since with host based scheduler it may get a much more consistent
behavior than with controller based which is especially important for
beacons.
> +}
> +
> static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
> struct sk_buff *skb)
> {
> @@ -3128,6 +3144,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
> hci_cc_le_set_ext_scan_enable(hdev, skb);
> break;
>
> + case HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS:
> + hci_cc_le_read_num_adv_sets(hdev, skb);
> + break;
> +
> default:
> BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
> break;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 07a3cc2..ffd5f7b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5998,7 +5998,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> rp->supported_flags = cpu_to_le32(supported_flags);
> rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
> rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
> - rp->max_instances = HCI_MAX_ADV_INSTANCES;
> + rp->max_instances = hdev->le_no_of_adv_sets;
> rp->num_instances = hdev->adv_instance_cnt;
>
> instance = rp->instance;
> @@ -6187,7 +6187,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> status);
>
> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> MGMT_STATUS_INVALID_PARAMS);
>
> @@ -6422,7 +6422,7 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
> MGMT_STATUS_REJECTED);
>
> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
> MGMT_STATUS_INVALID_PARAMS);
>
> --
> 2.7.4
>
> --
> 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
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-05 11:14 ` Luiz Augusto von Dentz
@ 2017-12-07 7:57 ` Jaganath K
2017-12-07 10:42 ` Luiz Augusto von Dentz
2017-12-08 8:40 ` Marcel Holtmann
1 sibling, 1 reply; 23+ messages in thread
From: Jaganath K @ 2017-12-07 7:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: linux-bluetooth@vger.kernel.org, Jaganath Kanakkassery
Hi Luiz,
> Hi Jaganath,
>
> On Mon, Dec 4, 2017 at 10:07 AM, Jaganath Kanakkassery
> <jaganath.k.os@gmail.com> wrote:
>> This patch reads the number of advertising sets in the controller
>> during init and save it in hdev.
>>
>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>
>> Instance 0 is reserved for "Set Advertising" which means that
>> multi adv is supported only for total sets - 1.
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>> ---
>> include/net/bluetooth/hci.h | 7 +++++++
>> include/net/bluetooth/hci_core.h | 4 ++++
>> net/bluetooth/hci_core.c | 11 +++++++++--
>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>> net/bluetooth/mgmt.c | 6 +++---
>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index f0868df..59df823 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -398,6 +398,7 @@ enum {
>> #define HCI_LE_SLAVE_FEATURES 0x08
>> #define HCI_LE_PING 0x10
>> #define HCI_LE_DATA_LEN_EXT 0x20
>> +#define HCI_LE_EXT_ADV 0x10
>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>
>> /* Connection modes */
>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>> __le16 max_ce_len;
>> } __packed;
>>
>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>> +struct hci_rp_le_read_num_supported_adv_sets {
>> + __u8 status;
>> + __u8 num_of_sets;
>> +} __packed;
>> +
>> /* ---- HCI Events ---- */
>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 554671c..4a7a4ae 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -219,6 +219,7 @@ struct hci_dev {
>> __u8 features[HCI_MAX_PAGES][8];
>> __u8 le_features[8];
>> __u8 le_white_list_size;
>> + __u8 le_no_of_adv_sets;
>
> I was expecting some flag that indicates if the instances would be
> maintained by the controller or not, but perhaps this is covered in
> other patches.
Actually le_no_of_adv_sets is initialized to HCI_MAX_ADV_INSTANCES
and if extended adv is supported it will be overwritten with the no of instances
supported by the controller.
>
>> __u8 le_states[8];
>> __u8 commands[64];
>> __u8 hci_ver;
>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>
>> +/* Extended advertising support */
>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>> +
>> /* ----- HCI protocols ----- */
>> #define HCI_PROTO_DEFER 0x01
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index a91a09a..3472572 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
>> }
>>
>> + if (ext_adv_capable(hdev)) {
>> + /* Read LE Number of Supported Advertising Sets */
>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>> + 0, NULL);
>> + }
>> +
>> hci_set_le_support(req);
>> }
>>
>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
>> memset(adv_instance->scan_rsp_data, 0,
>> sizeof(adv_instance->scan_rsp_data));
>> } else {
>> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>> return -EOVERFLOW;
>>
>> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>> hdev->le_max_tx_time = 0x0148;
>> hdev->le_max_rx_len = 0x001b;
>> hdev->le_max_rx_time = 0x0148;
>> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>>
>> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
>> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index a26ae80..06d8c1b 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
>> le_set_scan_enable_complete(hdev, cp->enable);
>> }
>>
>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>> + struct sk_buff *skb)
>> +{
>> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
>> +
>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
>> + rp->num_of_sets);
>> +
>> + if (rp->status)
>> + return;
>> +
>> + /* Instance 0 is reserved for Set Advertising */
>> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
>> + HCI_MAX_ADV_INSTANCES);
>
> Id say if the controller cannot support as many instances as the host
> stack then we should keep using the software based scheduler, another
> option would be to let the user select what scheduler it wants to use
> since with host based scheduler it may get a much more consistent
> behavior than with controller based which is especially important for
> beacons.
>
So do we need a new mgmt API for selecting the scheduler (host or controller) ?
>> +}
>> +
>> static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
>> struct sk_buff *skb)
>> {
>> @@ -3128,6 +3144,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>> hci_cc_le_set_ext_scan_enable(hdev, skb);
>> break;
>>
>> + case HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS:
>> + hci_cc_le_read_num_adv_sets(hdev, skb);
>> + break;
>> +
>> default:
>> BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
>> break;
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 07a3cc2..ffd5f7b 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -5998,7 +5998,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
>> rp->supported_flags = cpu_to_le32(supported_flags);
>> rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
>> rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
>> - rp->max_instances = HCI_MAX_ADV_INSTANCES;
>> + rp->max_instances = hdev->le_no_of_adv_sets;
>> rp->num_instances = hdev->adv_instance_cnt;
>>
>> instance = rp->instance;
>> @@ -6187,7 +6187,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>> status);
>>
>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>> MGMT_STATUS_INVALID_PARAMS);
>>
>> @@ -6422,7 +6422,7 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>> MGMT_STATUS_REJECTED);
>>
>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>> MGMT_STATUS_INVALID_PARAMS);
>>
Thanks,
Jaganath
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-07 7:57 ` Jaganath K
@ 2017-12-07 10:42 ` Luiz Augusto von Dentz
2017-12-07 10:59 ` Jaganath K
0 siblings, 1 reply; 23+ messages in thread
From: Luiz Augusto von Dentz @ 2017-12-07 10:42 UTC (permalink / raw)
To: Jaganath K; +Cc: linux-bluetooth@vger.kernel.org, Jaganath Kanakkassery
Hi Jaganath,
On Thu, Dec 7, 2017 at 5:57 AM, Jaganath K <jaganath.k.os@gmail.com> wrote:
> Hi Luiz,
>
>> Hi Jaganath,
>>
>> On Mon, Dec 4, 2017 at 10:07 AM, Jaganath Kanakkassery
>> <jaganath.k.os@gmail.com> wrote:
>>> This patch reads the number of advertising sets in the controller
>>> during init and save it in hdev.
>>>
>>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>>
>>> Instance 0 is reserved for "Set Advertising" which means that
>>> multi adv is supported only for total sets - 1.
>>>
>>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>>> ---
>>> include/net/bluetooth/hci.h | 7 +++++++
>>> include/net/bluetooth/hci_core.h | 4 ++++
>>> net/bluetooth/hci_core.c | 11 +++++++++--
>>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>>> net/bluetooth/mgmt.c | 6 +++---
>>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index f0868df..59df823 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -398,6 +398,7 @@ enum {
>>> #define HCI_LE_SLAVE_FEATURES 0x08
>>> #define HCI_LE_PING 0x10
>>> #define HCI_LE_DATA_LEN_EXT 0x20
>>> +#define HCI_LE_EXT_ADV 0x10
>>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>>
>>> /* Connection modes */
>>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>>> __le16 max_ce_len;
>>> } __packed;
>>>
>>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>>> +struct hci_rp_le_read_num_supported_adv_sets {
>>> + __u8 status;
>>> + __u8 num_of_sets;
>>> +} __packed;
>>> +
>>> /* ---- HCI Events ---- */
>>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index 554671c..4a7a4ae 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -219,6 +219,7 @@ struct hci_dev {
>>> __u8 features[HCI_MAX_PAGES][8];
>>> __u8 le_features[8];
>>> __u8 le_white_list_size;
>>> + __u8 le_no_of_adv_sets;
>>
>> I was expecting some flag that indicates if the instances would be
>> maintained by the controller or not, but perhaps this is covered in
>> other patches.
>
> Actually le_no_of_adv_sets is initialized to HCI_MAX_ADV_INSTANCES
> and if extended adv is supported it will be overwritten with the no of instances
> supported by the controller.
Which is exactly what Ive commented, since you overwrite how can you
tell what scheduler shall be used? For instance if the command fails
le_no_of_adv_sets would be left with default but those instances are
from the host not the controller.
>>
>>> __u8 le_states[8];
>>> __u8 commands[64];
>>> __u8 hci_ver;
>>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>>
>>> +/* Extended advertising support */
>>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>>> +
>>> /* ----- HCI protocols ----- */
>>> #define HCI_PROTO_DEFER 0x01
>>>
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index a91a09a..3472572 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
>>> }
>>>
>>> + if (ext_adv_capable(hdev)) {
>>> + /* Read LE Number of Supported Advertising Sets */
>>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>>> + 0, NULL);
>>> + }
>>> +
>>> hci_set_le_support(req);
>>> }
>>>
>>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
>>> memset(adv_instance->scan_rsp_data, 0,
>>> sizeof(adv_instance->scan_rsp_data));
>>> } else {
>>> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
>>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>>> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
>>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>>> return -EOVERFLOW;
>>>
>>> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
>>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>>> hdev->le_max_tx_time = 0x0148;
>>> hdev->le_max_rx_len = 0x001b;
>>> hdev->le_max_rx_time = 0x0148;
>>> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>>>
>>> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
>>> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>> index a26ae80..06d8c1b 100644
>>> --- a/net/bluetooth/hci_event.c
>>> +++ b/net/bluetooth/hci_event.c
>>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
>>> le_set_scan_enable_complete(hdev, cp->enable);
>>> }
>>>
>>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>>> + struct sk_buff *skb)
>>> +{
>>> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
>>> +
>>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
>>> + rp->num_of_sets);
>>> +
>>> + if (rp->status)
>>> + return;
>>> +
>>> + /* Instance 0 is reserved for Set Advertising */
>>> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
>>> + HCI_MAX_ADV_INSTANCES);
>>
>> Id say if the controller cannot support as many instances as the host
>> stack then we should keep using the software based scheduler, another
>> option would be to let the user select what scheduler it wants to use
>> since with host based scheduler it may get a much more consistent
>> behavior than with controller based which is especially important for
>> beacons.
>>
>
> So do we need a new mgmt API for selecting the scheduler (host or controller) ?
I think it could possible work by having a global preference setting
rather than having it per instance, which means we would have an entry
in main.conf telling if the platforms wants to use the controller
implementation or not which we may use not only for advertising API
but other controller features that may have bugs or vary on behavior
from vendor to vendor.
It is really a pity that there is no duration in the HCI commands
since otherwise we could maintain the existing instance and just
rotate them, without it seems impossible to tell if each instance got
any air time to rotate to next set.
>>> +}
>>> +
>>> static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
>>> struct sk_buff *skb)
>>> {
>>> @@ -3128,6 +3144,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>>> hci_cc_le_set_ext_scan_enable(hdev, skb);
>>> break;
>>>
>>> + case HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS:
>>> + hci_cc_le_read_num_adv_sets(hdev, skb);
>>> + break;
>>> +
>>> default:
>>> BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
>>> break;
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index 07a3cc2..ffd5f7b 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -5998,7 +5998,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
>>> rp->supported_flags = cpu_to_le32(supported_flags);
>>> rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
>>> rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
>>> - rp->max_instances = HCI_MAX_ADV_INSTANCES;
>>> + rp->max_instances = hdev->le_no_of_adv_sets;
>>> rp->num_instances = hdev->adv_instance_cnt;
>>>
>>> instance = rp->instance;
>>> @@ -6187,7 +6187,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>>> status);
>>>
>>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>>> MGMT_STATUS_INVALID_PARAMS);
>>>
>>> @@ -6422,7 +6422,7 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>>> MGMT_STATUS_REJECTED);
>>>
>>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>>> MGMT_STATUS_INVALID_PARAMS);
>>>
>
> Thanks,
> Jaganath
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-07 10:42 ` Luiz Augusto von Dentz
@ 2017-12-07 10:59 ` Jaganath K
0 siblings, 0 replies; 23+ messages in thread
From: Jaganath K @ 2017-12-07 10:59 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: linux-bluetooth@vger.kernel.org, Jaganath Kanakkassery
Hi Luiz,
> Hi Jaganath,
>
> On Thu, Dec 7, 2017 at 5:57 AM, Jaganath K <jaganath.k.os@gmail.com> wrote:
>> Hi Luiz,
>>
>>> Hi Jaganath,
>>>
>>> On Mon, Dec 4, 2017 at 10:07 AM, Jaganath Kanakkassery
>>> <jaganath.k.os@gmail.com> wrote:
>>>> This patch reads the number of advertising sets in the controller
>>>> during init and save it in hdev.
>>>>
>>>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>>>
>>>> Instance 0 is reserved for "Set Advertising" which means that
>>>> multi adv is supported only for total sets - 1.
>>>>
>>>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>>>> ---
>>>> include/net/bluetooth/hci.h | 7 +++++++
>>>> include/net/bluetooth/hci_core.h | 4 ++++
>>>> net/bluetooth/hci_core.c | 11 +++++++++--
>>>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>>>> net/bluetooth/mgmt.c | 6 +++---
>>>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>> index f0868df..59df823 100644
>>>> --- a/include/net/bluetooth/hci.h
>>>> +++ b/include/net/bluetooth/hci.h
>>>> @@ -398,6 +398,7 @@ enum {
>>>> #define HCI_LE_SLAVE_FEATURES 0x08
>>>> #define HCI_LE_PING 0x10
>>>> #define HCI_LE_DATA_LEN_EXT 0x20
>>>> +#define HCI_LE_EXT_ADV 0x10
>>>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>>>
>>>> /* Connection modes */
>>>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>>>> __le16 max_ce_len;
>>>> } __packed;
>>>>
>>>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>>>> +struct hci_rp_le_read_num_supported_adv_sets {
>>>> + __u8 status;
>>>> + __u8 num_of_sets;
>>>> +} __packed;
>>>> +
>>>> /* ---- HCI Events ---- */
>>>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>>>
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index 554671c..4a7a4ae 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -219,6 +219,7 @@ struct hci_dev {
>>>> __u8 features[HCI_MAX_PAGES][8];
>>>> __u8 le_features[8];
>>>> __u8 le_white_list_size;
>>>> + __u8 le_no_of_adv_sets;
>>>
>>> I was expecting some flag that indicates if the instances would be
>>> maintained by the controller or not, but perhaps this is covered in
>>> other patches.
>>
>> Actually le_no_of_adv_sets is initialized to HCI_MAX_ADV_INSTANCES
>> and if extended adv is supported it will be overwritten with the no of instances
>> supported by the controller.
>
> Which is exactly what Ive commented, since you overwrite how can you
> tell what scheduler shall be used? For instance if the command fails
> le_no_of_adv_sets would be left with default but those instances are
> from the host not the controller.
>
Yeah...current implementation always choose controller scheduler if extended
adv is supported.
>>>
>>>> __u8 le_states[8];
>>>> __u8 commands[64];
>>>> __u8 hci_ver;
>>>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>>>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>>>
>>>> +/* Extended advertising support */
>>>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>>>> +
>>>> /* ----- HCI protocols ----- */
>>>> #define HCI_PROTO_DEFER 0x01
>>>>
>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>> index a91a09a..3472572 100644
>>>> --- a/net/bluetooth/hci_core.c
>>>> +++ b/net/bluetooth/hci_core.c
>>>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>>>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
>>>> }
>>>>
>>>> + if (ext_adv_capable(hdev)) {
>>>> + /* Read LE Number of Supported Advertising Sets */
>>>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>>>> + 0, NULL);
>>>> + }
>>>> +
>>>> hci_set_le_support(req);
>>>> }
>>>>
>>>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
>>>> memset(adv_instance->scan_rsp_data, 0,
>>>> sizeof(adv_instance->scan_rsp_data));
>>>> } else {
>>>> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
>>>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>>>> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
>>>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>>>> return -EOVERFLOW;
>>>>
>>>> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
>>>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>>>> hdev->le_max_tx_time = 0x0148;
>>>> hdev->le_max_rx_len = 0x001b;
>>>> hdev->le_max_rx_time = 0x0148;
>>>> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>>>>
>>>> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
>>>> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
>>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>>> index a26ae80..06d8c1b 100644
>>>> --- a/net/bluetooth/hci_event.c
>>>> +++ b/net/bluetooth/hci_event.c
>>>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
>>>> le_set_scan_enable_complete(hdev, cp->enable);
>>>> }
>>>>
>>>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>>>> + struct sk_buff *skb)
>>>> +{
>>>> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
>>>> +
>>>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
>>>> + rp->num_of_sets);
>>>> +
>>>> + if (rp->status)
>>>> + return;
>>>> +
>>>> + /* Instance 0 is reserved for Set Advertising */
>>>> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
>>>> + HCI_MAX_ADV_INSTANCES);
>>>
>>> Id say if the controller cannot support as many instances as the host
>>> stack then we should keep using the software based scheduler, another
>>> option would be to let the user select what scheduler it wants to use
>>> since with host based scheduler it may get a much more consistent
>>> behavior than with controller based which is especially important for
>>> beacons.
>>>
>>
>> So do we need a new mgmt API for selecting the scheduler (host or controller) ?
>
> I think it could possible work by having a global preference setting
> rather than having it per instance, which means we would have an entry
> in main.conf telling if the platforms wants to use the controller
> implementation or not which we may use not only for advertising API
> but other controller features that may have bugs or vary on behavior
> from vendor to vendor.
>
> It is really a pity that there is no duration in the HCI commands
> since otherwise we could maintain the existing instance and just
> rotate them, without it seems impossible to tell if each instance got
> any air time to rotate to next set.
>
So i think first thing we should do is to just replace legacy with
extended adv and then
we can go for choosing scheduler later on. I will modify patches accordingly.
>>>> +}
>>>> +
>>>> static void hci_cc_le_read_white_list_size(struct hci_dev *hdev,
>>>> struct sk_buff *skb)
>>>> {
>>>> @@ -3128,6 +3144,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
>>>> hci_cc_le_set_ext_scan_enable(hdev, skb);
>>>> break;
>>>>
>>>> + case HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS:
>>>> + hci_cc_le_read_num_adv_sets(hdev, skb);
>>>> + break;
>>>> +
>>>> default:
>>>> BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
>>>> break;
>>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>>> index 07a3cc2..ffd5f7b 100644
>>>> --- a/net/bluetooth/mgmt.c
>>>> +++ b/net/bluetooth/mgmt.c
>>>> @@ -5998,7 +5998,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
>>>> rp->supported_flags = cpu_to_le32(supported_flags);
>>>> rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
>>>> rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
>>>> - rp->max_instances = HCI_MAX_ADV_INSTANCES;
>>>> + rp->max_instances = hdev->le_no_of_adv_sets;
>>>> rp->num_instances = hdev->adv_instance_cnt;
>>>>
>>>> instance = rp->instance;
>>>> @@ -6187,7 +6187,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
>>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>>>> status);
>>>>
>>>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>>>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
>>>> MGMT_STATUS_INVALID_PARAMS);
>>>>
>>>> @@ -6422,7 +6422,7 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
>>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>>>> MGMT_STATUS_REJECTED);
>>>>
>>>> - if (cp->instance < 1 || cp->instance > HCI_MAX_ADV_INSTANCES)
>>>> + if (cp->instance < 1 || cp->instance > hdev->le_no_of_adv_sets)
>>>> return mgmt_cmd_status(sk, hdev->id, MGMT_OP_GET_ADV_SIZE_INFO,
>>>> MGMT_STATUS_INVALID_PARAMS);
>>>>
>>
>> Thanks,
>> Jaganath
>
>
>
> --
> Luiz Augusto von Dentz
Thanks,
Jaganath
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-05 11:14 ` Luiz Augusto von Dentz
2017-12-07 7:57 ` Jaganath K
@ 2017-12-08 8:40 ` Marcel Holtmann
2017-12-08 11:57 ` Jaganath K
1 sibling, 1 reply; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 8:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Jaganath Kanakkassery, linux-bluetooth@vger.kernel.org,
Jaganath Kanakkassery
Hi Luiz,
>> This patch reads the number of advertising sets in the controller
>> during init and save it in hdev.
>>
>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>
>> Instance 0 is reserved for "Set Advertising" which means that
>> multi adv is supported only for total sets - 1.
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>> ---
>> include/net/bluetooth/hci.h | 7 +++++++
>> include/net/bluetooth/hci_core.h | 4 ++++
>> net/bluetooth/hci_core.c | 11 +++++++++--
>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>> net/bluetooth/mgmt.c | 6 +++---
>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index f0868df..59df823 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -398,6 +398,7 @@ enum {
>> #define HCI_LE_SLAVE_FEATURES 0x08
>> #define HCI_LE_PING 0x10
>> #define HCI_LE_DATA_LEN_EXT 0x20
>> +#define HCI_LE_EXT_ADV 0x10
>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>
>> /* Connection modes */
>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>> __le16 max_ce_len;
>> } __packed;
>>
>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>> +struct hci_rp_le_read_num_supported_adv_sets {
>> + __u8 status;
>> + __u8 num_of_sets;
>> +} __packed;
>> +
>> /* ---- HCI Events ---- */
>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 554671c..4a7a4ae 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -219,6 +219,7 @@ struct hci_dev {
>> __u8 features[HCI_MAX_PAGES][8];
>> __u8 le_features[8];
>> __u8 le_white_list_size;
>> + __u8 le_no_of_adv_sets;
>
> I was expecting some flag that indicates if the instances would be
> maintained by the controller or not, but perhaps this is covered in
> other patches.
>
>> __u8 le_states[8];
>> __u8 commands[64];
>> __u8 hci_ver;
>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>
>> +/* Extended advertising support */
>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>> +
>> /* ----- HCI protocols ----- */
>> #define HCI_PROTO_DEFER 0x01
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index a91a09a..3472572 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
>> }
>>
>> + if (ext_adv_capable(hdev)) {
>> + /* Read LE Number of Supported Advertising Sets */
>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>> + 0, NULL);
>> + }
>> +
>> hci_set_le_support(req);
>> }
>>
>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
>> memset(adv_instance->scan_rsp_data, 0,
>> sizeof(adv_instance->scan_rsp_data));
>> } else {
>> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>> return -EOVERFLOW;
>>
>> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>> hdev->le_max_tx_time = 0x0148;
>> hdev->le_max_rx_len = 0x001b;
>> hdev->le_max_rx_time = 0x0148;
>> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>>
>> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
>> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index a26ae80..06d8c1b 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
>> le_set_scan_enable_complete(hdev, cp->enable);
>> }
>>
>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>> + struct sk_buff *skb)
>> +{
>> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
>> +
>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
>> + rp->num_of_sets);
>> +
>> + if (rp->status)
>> + return;
>> +
>> + /* Instance 0 is reserved for Set Advertising */
>> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
>> + HCI_MAX_ADV_INSTANCES);
>
> Id say if the controller cannot support as many instances as the host
> stack then we should keep using the software based scheduler, another
> option would be to let the user select what scheduler it wants to use
> since with host based scheduler it may get a much more consistent
> behavior than with controller based which is especially important for
> beacons.
frankly this will not help either. The max instances reported from the controller is not a fixed guaranteed number. It is the most likely case. However a controller can run out of resources and decide to refuse an advertising instance.
However having an extra flag for permanent / continuous offload would be interesting. If not specified, then the kernel will rotate. For 4.x controllers it will rotate based on a single instance, for 5.x it will rotate with n instances. The extra flag could then indicate, please do not include me into the rotation and keep me always active. Which is something that instance 0 would always inherit.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-08 8:40 ` Marcel Holtmann
@ 2017-12-08 11:57 ` Jaganath K
2017-12-08 18:34 ` Marcel Holtmann
0 siblings, 1 reply; 23+ messages in thread
From: Jaganath K @ 2017-12-08 11:57 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org,
Jaganath Kanakkassery
Hi Marcel,
> Hi Luiz,
>
>>> This patch reads the number of advertising sets in the controller
>>> during init and save it in hdev.
>>>
>>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>>
>>> Instance 0 is reserved for "Set Advertising" which means that
>>> multi adv is supported only for total sets - 1.
>>>
>>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>>> ---
>>> include/net/bluetooth/hci.h | 7 +++++++
>>> include/net/bluetooth/hci_core.h | 4 ++++
>>> net/bluetooth/hci_core.c | 11 +++++++++--
>>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>>> net/bluetooth/mgmt.c | 6 +++---
>>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index f0868df..59df823 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -398,6 +398,7 @@ enum {
>>> #define HCI_LE_SLAVE_FEATURES 0x08
>>> #define HCI_LE_PING 0x10
>>> #define HCI_LE_DATA_LEN_EXT 0x20
>>> +#define HCI_LE_EXT_ADV 0x10
>>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>>
>>> /* Connection modes */
>>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>>> __le16 max_ce_len;
>>> } __packed;
>>>
>>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>>> +struct hci_rp_le_read_num_supported_adv_sets {
>>> + __u8 status;
>>> + __u8 num_of_sets;
>>> +} __packed;
>>> +
>>> /* ---- HCI Events ---- */
>>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/h=
ci_core.h
>>> index 554671c..4a7a4ae 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -219,6 +219,7 @@ struct hci_dev {
>>> __u8 features[HCI_MAX_PAGES][8];
>>> __u8 le_features[8];
>>> __u8 le_white_list_size;
>>> + __u8 le_no_of_adv_sets;
>>
>> I was expecting some flag that indicates if the instances would be
>> maintained by the controller or not, but perhaps this is covered in
>> other patches.
>>
>>> __u8 le_states[8];
>>> __u8 commands[64];
>>> __u8 hci_ver;
>>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>>
>>> +/* Extended advertising support */
>>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>>> +
>>> /* ----- HCI protocols ----- */
>>> #define HCI_PROTO_DEFER 0x01
>>>
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index a91a09a..3472572 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, =
unsigned long opt)
>>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0,=
NULL);
>>> }
>>>
>>> + if (ext_adv_capable(hdev)) {
>>> + /* Read LE Number of Supported Advertising Sets=
*/
>>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_A=
DV_SETS,
>>> + 0, NULL);
>>> + }
>>> +
>>> hci_set_le_support(req);
>>> }
>>>
>>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8=
instance, u32 flags,
>>> memset(adv_instance->scan_rsp_data, 0,
>>> sizeof(adv_instance->scan_rsp_data));
>>> } else {
>>> - if (hdev->adv_instance_cnt >=3D HCI_MAX_ADV_INSTANCES |=
|
>>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>>> + if (hdev->adv_instance_cnt >=3D hdev->le_no_of_adv_sets=
||
>>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>>> return -EOVERFLOW;
>>>
>>> adv_instance =3D kzalloc(sizeof(*adv_instance), GFP_KERN=
EL);
>>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>>> hdev->le_max_tx_time =3D 0x0148;
>>> hdev->le_max_rx_len =3D 0x001b;
>>> hdev->le_max_rx_time =3D 0x0148;
>>> + hdev->le_no_of_adv_sets =3D HCI_MAX_ADV_INSTANCES;
>>>
>>> hdev->rpa_timeout =3D HCI_DEFAULT_RPA_TIMEOUT;
>>> hdev->discov_interleaved_timeout =3D DISCOV_INTERLEAVED_TIMEOUT;
>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>> index a26ae80..06d8c1b 100644
>>> --- a/net/bluetooth/hci_event.c
>>> +++ b/net/bluetooth/hci_event.c
>>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct=
hci_dev *hdev,
>>> le_set_scan_enable_complete(hdev, cp->enable);
>>> }
>>>
>>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>>> + struct sk_buff *skb)
>>> +{
>>> + struct hci_rp_le_read_num_supported_adv_sets *rp =3D (void *) s=
kb->data;
>>> +
>>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->s=
tatus,
>>> + rp->num_of_sets);
>>> +
>>> + if (rp->status)
>>> + return;
>>> +
>>> + /* Instance 0 is reserved for Set Advertising */
>>> + hdev->le_no_of_adv_sets =3D min_t(u8, rp->num_of_sets - 1,
>>> + HCI_MAX_ADV_INSTANCES);
>>
>> Id say if the controller cannot support as many instances as the host
>> stack then we should keep using the software based scheduler, another
>> option would be to let the user select what scheduler it wants to use
>> since with host based scheduler it may get a much more consistent
>> behavior than with controller based which is especially important for
>> beacons.
>
> frankly this will not help either. The max instances reported from the co=
ntroller is not a fixed guaranteed number. It is the most likely case. Howe=
ver a controller can run out of resources and decide to refuse an advertisi=
ng instance.
>
Does it mean that we need to retrieve no of supported adv sets every time
we enable advertising?
or try enabling based on the initial no_of_sets and handle it for eg
if adv_set_param
failed with "Limit Reached" error?
> However having an extra flag for permanent / continuous offload would be =
interesting. If not specified, then the kernel will rotate. For 4.x control=
lers it will rotate based on a single instance, for 5.x it will rotate with=
n instances. The extra flag could then indicate, please do not include me =
into the rotation and keep me always active. Which is something that instan=
ce 0 would always inherit.
>
So you want to still rotate adv instances by kernel wherein at a time
n adv sets/instances
would be enabled, where n is no of supported sets? and timer should be
running for the
min of scheduled adv instances duration and once timer expires then
replace it with the
next unscheduled instance.
Thanks,
Jaganath
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-08 11:57 ` Jaganath K
@ 2017-12-08 18:34 ` Marcel Holtmann
2018-03-05 11:56 ` Jaganath K
0 siblings, 1 reply; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 18:34 UTC (permalink / raw)
To: Jaganath K
Cc: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org,
Jaganath Kanakkassery
Hi Jaganath,
>>>> This patch reads the number of advertising sets in the controller
>>>> during init and save it in hdev.
>>>>
>>>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>>>
>>>> Instance 0 is reserved for "Set Advertising" which means that
>>>> multi adv is supported only for total sets - 1.
>>>>
>>>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>>>> ---
>>>> include/net/bluetooth/hci.h | 7 +++++++
>>>> include/net/bluetooth/hci_core.h | 4 ++++
>>>> net/bluetooth/hci_core.c | 11 +++++++++--
>>>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>>>> net/bluetooth/mgmt.c | 6 +++---
>>>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>> index f0868df..59df823 100644
>>>> --- a/include/net/bluetooth/hci.h
>>>> +++ b/include/net/bluetooth/hci.h
>>>> @@ -398,6 +398,7 @@ enum {
>>>> #define HCI_LE_SLAVE_FEATURES 0x08
>>>> #define HCI_LE_PING 0x10
>>>> #define HCI_LE_DATA_LEN_EXT 0x20
>>>> +#define HCI_LE_EXT_ADV 0x10
>>>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>>>
>>>> /* Connection modes */
>>>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>>>> __le16 max_ce_len;
>>>> } __packed;
>>>>
>>>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>>>> +struct hci_rp_le_read_num_supported_adv_sets {
>>>> + __u8 status;
>>>> + __u8 num_of_sets;
>>>> +} __packed;
>>>> +
>>>> /* ---- HCI Events ---- */
>>>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>>>
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index 554671c..4a7a4ae 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -219,6 +219,7 @@ struct hci_dev {
>>>> __u8 features[HCI_MAX_PAGES][8];
>>>> __u8 le_features[8];
>>>> __u8 le_white_list_size;
>>>> + __u8 le_no_of_adv_sets;
>>>
>>> I was expecting some flag that indicates if the instances would be
>>> maintained by the controller or not, but perhaps this is covered in
>>> other patches.
>>>
>>>> __u8 le_states[8];
>>>> __u8 commands[64];
>>>> __u8 hci_ver;
>>>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>>>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>>>
>>>> +/* Extended advertising support */
>>>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV)
>>>> +
>>>> /* ----- HCI protocols ----- */
>>>> #define HCI_PROTO_DEFER 0x01
>>>>
>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>> index a91a09a..3472572 100644
>>>> --- a/net/bluetooth/hci_core.c
>>>> +++ b/net/bluetooth/hci_core.c
>>>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>>>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0, NULL);
>>>> }
>>>>
>>>> + if (ext_adv_capable(hdev)) {
>>>> + /* Read LE Number of Supported Advertising Sets */
>>>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>>>> + 0, NULL);
>>>> + }
>>>> +
>>>> hci_set_le_support(req);
>>>> }
>>>>
>>>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
>>>> memset(adv_instance->scan_rsp_data, 0,
>>>> sizeof(adv_instance->scan_rsp_data));
>>>> } else {
>>>> - if (hdev->adv_instance_cnt >= HCI_MAX_ADV_INSTANCES ||
>>>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>>>> + if (hdev->adv_instance_cnt >= hdev->le_no_of_adv_sets ||
>>>> + instance < 1 || instance > hdev->le_no_of_adv_sets)
>>>> return -EOVERFLOW;
>>>>
>>>> adv_instance = kzalloc(sizeof(*adv_instance), GFP_KERNEL);
>>>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>>>> hdev->le_max_tx_time = 0x0148;
>>>> hdev->le_max_rx_len = 0x001b;
>>>> hdev->le_max_rx_time = 0x0148;
>>>> + hdev->le_no_of_adv_sets = HCI_MAX_ADV_INSTANCES;
>>>>
>>>> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
>>>> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
>>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>>> index a26ae80..06d8c1b 100644
>>>> --- a/net/bluetooth/hci_event.c
>>>> +++ b/net/bluetooth/hci_event.c
>>>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(struct hci_dev *hdev,
>>>> le_set_scan_enable_complete(hdev, cp->enable);
>>>> }
>>>>
>>>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>>>> + struct sk_buff *skb)
>>>> +{
>>>> + struct hci_rp_le_read_num_supported_adv_sets *rp = (void *) skb->data;
>>>> +
>>>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp->status,
>>>> + rp->num_of_sets);
>>>> +
>>>> + if (rp->status)
>>>> + return;
>>>> +
>>>> + /* Instance 0 is reserved for Set Advertising */
>>>> + hdev->le_no_of_adv_sets = min_t(u8, rp->num_of_sets - 1,
>>>> + HCI_MAX_ADV_INSTANCES);
>>>
>>> Id say if the controller cannot support as many instances as the host
>>> stack then we should keep using the software based scheduler, another
>>> option would be to let the user select what scheduler it wants to use
>>> since with host based scheduler it may get a much more consistent
>>> behavior than with controller based which is especially important for
>>> beacons.
>>
>> frankly this will not help either. The max instances reported from the controller is not a fixed guaranteed number. It is the most likely case. However a controller can run out of resources and decide to refuse an advertising instance.
>>
>
> Does it mean that we need to retrieve no of supported adv sets every time
> we enable advertising?
>
> or try enabling based on the initial no_of_sets and handle it for eg
> if adv_set_param
> failed with "Limit Reached" error?
we need to handle the error case correctly.
>> However having an extra flag for permanent / continuous offload would be interesting. If not specified, then the kernel will rotate. For 4.x controllers it will rotate based on a single instance, for 5.x it will rotate with n instances. The extra flag could then indicate, please do not include me into the rotation and keep me always active. Which is something that instance 0 would always inherit.
>>
>
> So you want to still rotate adv instances by kernel wherein at a time
> n adv sets/instances
> would be enabled, where n is no of supported sets? and timer should be
> running for the
> min of scheduled adv instances duration and once timer expires then
> replace it with the
> next unscheduled instance.
It would be some sort of round-robin. If we have more instances than sets, then timer has to run, and then we rotate, the oldest one moves out, the next moves in.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 1/9] Bluetooth: Read no of adv sets during init
2017-12-08 18:34 ` Marcel Holtmann
@ 2018-03-05 11:56 ` Jaganath K
0 siblings, 0 replies; 23+ messages in thread
From: Jaganath K @ 2018-03-05 11:56 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org,
Jaganath Kanakkassery
Hi Marcel,
On Sat, Dec 9, 2017 at 12:04 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Jaganath,
>
>>>>> This patch reads the number of advertising sets in the controller
>>>>> during init and save it in hdev.
>>>>>
>>>>> Currently host only supports upto HCI_MAX_ADV_INSTANCES (5).
>>>>>
>>>>> Instance 0 is reserved for "Set Advertising" which means that
>>>>> multi adv is supported only for total sets - 1.
>>>>>
>>>>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.co=
m>
>>>>> ---
>>>>> include/net/bluetooth/hci.h | 7 +++++++
>>>>> include/net/bluetooth/hci_core.h | 4 ++++
>>>>> net/bluetooth/hci_core.c | 11 +++++++++--
>>>>> net/bluetooth/hci_event.c | 20 ++++++++++++++++++++
>>>>> net/bluetooth/mgmt.c | 6 +++---
>>>>> 5 files changed, 43 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.=
h
>>>>> index f0868df..59df823 100644
>>>>> --- a/include/net/bluetooth/hci.h
>>>>> +++ b/include/net/bluetooth/hci.h
>>>>> @@ -398,6 +398,7 @@ enum {
>>>>> #define HCI_LE_SLAVE_FEATURES 0x08
>>>>> #define HCI_LE_PING 0x10
>>>>> #define HCI_LE_DATA_LEN_EXT 0x20
>>>>> +#define HCI_LE_EXT_ADV 0x10
>>>>> #define HCI_LE_EXT_SCAN_POLICY 0x80
>>>>>
>>>>> /* Connection modes */
>>>>> @@ -1543,6 +1544,12 @@ struct hci_cp_le_ext_conn_param {
>>>>> __le16 max_ce_len;
>>>>> } __packed;
>>>>>
>>>>> +#define HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS 0x203b
>>>>> +struct hci_rp_le_read_num_supported_adv_sets {
>>>>> + __u8 status;
>>>>> + __u8 num_of_sets;
>>>>> +} __packed;
>>>>> +
>>>>> /* ---- HCI Events ---- */
>>>>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>>>>
>>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth=
/hci_core.h
>>>>> index 554671c..4a7a4ae 100644
>>>>> --- a/include/net/bluetooth/hci_core.h
>>>>> +++ b/include/net/bluetooth/hci_core.h
>>>>> @@ -219,6 +219,7 @@ struct hci_dev {
>>>>> __u8 features[HCI_MAX_PAGES][8];
>>>>> __u8 le_features[8];
>>>>> __u8 le_white_list_size;
>>>>> + __u8 le_no_of_adv_sets;
>>>>
>>>> I was expecting some flag that indicates if the instances would be
>>>> maintained by the controller or not, but perhaps this is covered in
>>>> other patches.
>>>>
>>>>> __u8 le_states[8];
>>>>> __u8 commands[64];
>>>>> __u8 hci_ver;
>>>>> @@ -1154,6 +1155,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>>>> #define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
>>>>> hci_dev_test_flag(dev, HCI_SC_ENABLED))
>>>>>
>>>>> +/* Extended advertising support */
>>>>> +#define ext_adv_capable(dev) ((dev)->le_features[1] & HCI_LE_EXT_ADV=
)
>>>>> +
>>>>> /* ----- HCI protocols ----- */
>>>>> #define HCI_PROTO_DEFER 0x01
>>>>>
>>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>>> index a91a09a..3472572 100644
>>>>> --- a/net/bluetooth/hci_core.c
>>>>> +++ b/net/bluetooth/hci_core.c
>>>>> @@ -716,6 +716,12 @@ static int hci_init3_req(struct hci_request *req=
, unsigned long opt)
>>>>> hci_req_add(req, HCI_OP_LE_READ_DEF_DATA_LEN, 0=
, NULL);
>>>>> }
>>>>>
>>>>> + if (ext_adv_capable(hdev)) {
>>>>> + /* Read LE Number of Supported Advertising Se=
ts */
>>>>> + hci_req_add(req, HCI_OP_LE_READ_NUM_SUPPORTED=
_ADV_SETS,
>>>>> + 0, NULL);
>>>>> + }
>>>>> +
>>>>> hci_set_le_support(req);
>>>>> }
>>>>>
>>>>> @@ -2676,8 +2682,8 @@ int hci_add_adv_instance(struct hci_dev *hdev, =
u8 instance, u32 flags,
>>>>> memset(adv_instance->scan_rsp_data, 0,
>>>>> sizeof(adv_instance->scan_rsp_data));
>>>>> } else {
>>>>> - if (hdev->adv_instance_cnt >=3D HCI_MAX_ADV_INSTANCES=
||
>>>>> - instance < 1 || instance > HCI_MAX_ADV_INSTANCES)
>>>>> + if (hdev->adv_instance_cnt >=3D hdev->le_no_of_adv_se=
ts ||
>>>>> + instance < 1 || instance > hdev->le_no_of_adv_set=
s)
>>>>> return -EOVERFLOW;
>>>>>
>>>>> adv_instance =3D kzalloc(sizeof(*adv_instance), GFP_KER=
NEL);
>>>>> @@ -2972,6 +2978,7 @@ struct hci_dev *hci_alloc_dev(void)
>>>>> hdev->le_max_tx_time =3D 0x0148;
>>>>> hdev->le_max_rx_len =3D 0x001b;
>>>>> hdev->le_max_rx_time =3D 0x0148;
>>>>> + hdev->le_no_of_adv_sets =3D HCI_MAX_ADV_INSTANCES;
>>>>>
>>>>> hdev->rpa_timeout =3D HCI_DEFAULT_RPA_TIMEOUT;
>>>>> hdev->discov_interleaved_timeout =3D DISCOV_INTERLEAVED_TIMEOUT=
;
>>>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>>>> index a26ae80..06d8c1b 100644
>>>>> --- a/net/bluetooth/hci_event.c
>>>>> +++ b/net/bluetooth/hci_event.c
>>>>> @@ -1243,6 +1243,22 @@ static void hci_cc_le_set_ext_scan_enable(stru=
ct hci_dev *hdev,
>>>>> le_set_scan_enable_complete(hdev, cp->enable);
>>>>> }
>>>>>
>>>>> +static void hci_cc_le_read_num_adv_sets(struct hci_dev *hdev,
>>>>> + struct sk_buff *skb)
>>>>> +{
>>>>> + struct hci_rp_le_read_num_supported_adv_sets *rp =3D (void *)=
skb->data;
>>>>> +
>>>>> + BT_DBG("%s status 0x%2.2x No of Adv sets %u", hdev->name, rp-=
>status,
>>>>> + rp->num_of_sets);
>>>>> +
>>>>> + if (rp->status)
>>>>> + return;
>>>>> +
>>>>> + /* Instance 0 is reserved for Set Advertising */
>>>>> + hdev->le_no_of_adv_sets =3D min_t(u8, rp->num_of_sets - 1,
>>>>> + HCI_MAX_ADV_INSTANCES);
>>>>
>>>> Id say if the controller cannot support as many instances as the host
>>>> stack then we should keep using the software based scheduler, another
>>>> option would be to let the user select what scheduler it wants to use
>>>> since with host based scheduler it may get a much more consistent
>>>> behavior than with controller based which is especially important for
>>>> beacons.
>>>
>>> frankly this will not help either. The max instances reported from the =
controller is not a fixed guaranteed number. It is the most likely case. Ho=
wever a controller can run out of resources and decide to refuse an adverti=
sing instance.
>>>
>>
>> Does it mean that we need to retrieve no of supported adv sets every tim=
e
>> we enable advertising?
>>
>> or try enabling based on the initial no_of_sets and handle it for eg
>> if adv_set_param
>> failed with "Limit Reached" error?
>
> we need to handle the error case correctly.
>
>>> However having an extra flag for permanent / continuous offload would b=
e interesting. If not specified, then the kernel will rotate. For 4.x contr=
ollers it will rotate based on a single instance, for 5.x it will rotate wi=
th n instances. The extra flag could then indicate, please do not include m=
e into the rotation and keep me always active. Which is something that inst=
ance 0 would always inherit.
>>>
>>
>> So you want to still rotate adv instances by kernel wherein at a time
>> n adv sets/instances
>> would be enabled, where n is no of supported sets? and timer should be
>> running for the
>> min of scheduled adv instances duration and once timer expires then
>> replace it with the
>> next unscheduled instance.
>
> It would be some sort of round-robin. If we have more instances than sets=
, then timer has to run, and then we rotate, the oldest one moves out, the =
next moves in.
>
I was thinking of implementing this and we can have two approaches.
First as you said oldest one move out and next one moves in which adds and
removes one instance during each rotation. So basically if there are 2 sets
and 5 instances then it would be like 12 23 34 45 51 etc.
Another approach is disable all the current sets and schedule the new ones.
So in above scenario it would be like 12, 34, 45, 12 etc. I think with this=
all
instances will be scheduled in lesser time.
Plz let me know your opinion.
Thanks,
Jaganath
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 2/9] Bluetooth: Impmlement extended adv enable
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 1/9] Bluetooth: Read no of adv sets during init Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports Jaganath Kanakkassery
` (6 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch implements ext adv set parameter and enable functions.
Introduced __hci_req_start_ext_adv() which can activate a single
instance or all instances based on the parameter passed.
If atleast one instance is enabled then HCI_LE_ADV flag is set in
hdev. State is added for each instance to check whether the instance
is newly created (which means that it should be programmed to the
controller), enabled or disabled state.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 37 ++++++
include/net/bluetooth/hci_core.h | 7 ++
net/bluetooth/hci_event.c | 87 +++++++++++++++
net/bluetooth/hci_request.c | 235 ++++++++++++++++++++++++++++++++++-----
net/bluetooth/hci_request.h | 5 +
net/bluetooth/mgmt.c | 96 ++++++++++------
6 files changed, 407 insertions(+), 60 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 59df823..dd6b9cb 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1550,6 +1550,43 @@ struct hci_rp_le_read_num_supported_adv_sets {
__u8 num_of_sets;
} __packed;
+#define HCI_OP_LE_SET_EXT_ADV_PARAMS 0x2036
+struct hci_cp_le_set_ext_adv_params {
+ __u8 handle;
+ __le16 evt_properties;
+ __u8 min_interval[3];
+ __u8 max_interval[3];
+ __u8 channel_map;
+ __u8 own_addr_type;
+ __u8 peer_addr_type;
+ bdaddr_t peer_addr;
+ __u8 filter_policy;
+ __u8 tx_power;
+ __u8 primary_phy;
+ __u8 secondary_max_skip;
+ __u8 secondary_phy;
+ __u8 sid;
+ __u8 notif_enable;
+} __packed;
+
+struct hci_rp_le_set_ext_adv_params {
+ __u8 status;
+ __u8 tx_power;
+} __attribute__ ((packed));
+
+#define HCI_OP_LE_SET_EXT_ADV_ENABLE 0x2039
+struct hci_cp_le_set_ext_adv_enable {
+ __u8 enable;
+ __u8 num_of_sets;
+ __u8 data[0];
+} __packed;
+
+struct hci_cp_ext_adv_set {
+ __u8 handle;
+ __le16 duration;
+ __u8 max_events;
+} __packed;
+
/* ---- HCI Events ---- */
#define HCI_EV_INQUIRY_COMPLETE 0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4a7a4ae..2abeabb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -169,6 +169,13 @@ struct adv_info {
__u8 adv_data[HCI_MAX_AD_LENGTH];
__u16 scan_rsp_len;
__u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
+ __u8 addr_type;
+ unsigned long state;
+};
+
+/* Adv instance states */
+enum {
+ ADV_INST_ENABLED,
};
#define HCI_MAX_ADV_INSTANCES 5
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 06d8c1b..724c668 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1076,6 +1076,56 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
hci_dev_unlock(hdev);
}
+static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_cp_le_set_ext_adv_enable *cp;
+ struct hci_cp_ext_adv_set *adv_set;
+ __u8 status = *((__u8 *) skb->data);
+ struct adv_info *adv_instance;
+ int i;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+ if (status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_ENABLE);
+ if (!cp)
+ return;
+
+ adv_set = (void *) cp->data;
+
+ hci_dev_lock(hdev);
+
+ if (cp->enable) {
+ struct hci_conn *conn;
+
+ /* Set HCI_LE_ADV if atleast one instance is enabled */
+ hci_dev_set_flag(hdev, HCI_LE_ADV);
+
+ /* If we're doing connection initiation as peripheral. Set a
+ * timeout in case something goes wrong.
+ */
+ conn = hci_lookup_le_connect(hdev);
+ if (conn)
+ queue_delayed_work(hdev->workqueue,
+ &conn->le_conn_timeout,
+ conn->conn_timeout);
+
+ for (i = 0; i < cp->num_of_sets; i++) {
+ adv_instance = hci_find_adv_instance(hdev,
+ adv_set->handle);
+ if (adv_instance)
+ set_bit(ADV_INST_ENABLED, &adv_instance->state);
+
+ adv_set++;
+ }
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_cp_le_set_scan_param *cp;
@@ -1438,6 +1488,35 @@ static void hci_cc_set_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
hci_dev_unlock(hdev);
}
+static void hci_cc_set_ext_adv_param(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_rp_le_set_ext_adv_params *rp = (void *) skb->data;
+ struct hci_cp_le_set_ext_adv_params *cp;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS);
+ if (!cp)
+ return;
+
+ hci_dev_lock(hdev);
+
+ if (!cp->handle) {
+ hdev->adv_addr_type = cp->own_addr_type;
+ } else {
+ struct adv_info *adv_instance;
+
+ adv_instance = hci_find_adv_instance(hdev, cp->handle);
+ if (adv_instance)
+ adv_instance->addr_type = cp->own_addr_type;
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cc_read_rssi(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_rp_read_rssi *rp = (void *) skb->data;
@@ -3148,6 +3227,14 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_le_read_num_adv_sets(hdev, skb);
break;
+ case HCI_OP_LE_SET_EXT_ADV_PARAMS:
+ hci_cc_set_ext_adv_param(hdev, skb);
+ break;
+
+ case HCI_OP_LE_SET_EXT_ADV_ENABLE:
+ hci_cc_le_set_ext_adv_enable(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
break;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 62a7b94..05f1388 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -890,6 +890,24 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
hdev->le_scan_window, own_addr_type, filter_policy);
}
+static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
+{
+ struct adv_info *adv_instance;
+
+ /* Ignore instance 0 */
+ if (instance == 0x00)
+ return 0;
+
+ adv_instance = hci_find_adv_instance(hdev, instance);
+ if (!adv_instance)
+ return 0;
+
+ /* TODO: Take into account the "appearance" and "local-name" flags here.
+ * These are currently being ignored as they are not supported.
+ */
+ return adv_instance->scan_rsp_len;
+}
+
static u8 get_cur_adv_instance_scan_rsp_len(struct hci_dev *hdev)
{
u8 instance = hdev->cur_adv_instance;
@@ -1258,13 +1276,22 @@ void hci_req_reenable_advertising(struct hci_dev *hdev)
hci_req_init(&req, hdev);
- if (hdev->cur_adv_instance) {
- __hci_req_schedule_adv_instance(&req, hdev->cur_adv_instance,
- true);
+ if (ext_adv_capable(hdev)) {
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ __hci_req_start_ext_adv(&req, 0, false, false, 0);
+ else
+ __hci_req_start_ext_adv(&req, 0, true, false, 0);
} else {
- __hci_req_update_adv_data(&req, 0x00);
- __hci_req_update_scan_rsp_data(&req, 0x00);
- __hci_req_enable_advertising(&req);
+
+ if (hdev->cur_adv_instance) {
+ __hci_req_schedule_adv_instance(&req,
+ hdev->cur_adv_instance,
+ true);
+ } else {
+ __hci_req_update_adv_data(&req, 0x00);
+ __hci_req_update_scan_rsp_data(&req, 0x00);
+ __hci_req_enable_advertising(&req);
+ }
}
hci_req_run(&req, adv_enable_complete);
@@ -1301,6 +1328,133 @@ unlock:
hci_dev_unlock(hdev);
}
+static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
+ u8 instance)
+{
+ struct hci_cp_le_set_ext_adv_params cp;
+ struct hci_dev *hdev = req->hdev;
+ bool connectable;
+ u32 flags;
+ /* In ext adv set param interval is 3 octets */
+ const u8 adv_interval[3] = { 0x00, 0x08, 0x00 };
+
+ flags = get_adv_instance_flags(hdev, instance);
+
+ /* If the "connectable" instance flag was not set, then choose between
+ * ADV_IND and ADV_NONCONN_IND based on the global connectable setting.
+ */
+ connectable = (flags & MGMT_ADV_FLAG_CONNECTABLE) ||
+ mgmt_get_connectable(hdev);
+
+ memset(&cp, 0, sizeof(cp));
+
+ memcpy(cp.min_interval, adv_interval, sizeof(cp.min_interval));
+ memcpy(cp.max_interval, adv_interval, sizeof(cp.max_interval));
+
+ if (connectable)
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
+ else if (get_adv_instance_scan_rsp_len(hdev, instance))
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_SCAN_IND);
+ else
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_NONCONN_IND);
+
+ cp.own_addr_type = BDADDR_LE_PUBLIC;
+ cp.channel_map = hdev->le_adv_channel_map;
+ cp.tx_power = 127;
+ cp.primary_phy = LE_PHY_1M;
+ cp.secondary_phy = LE_PHY_1M;
+ cp.handle = instance;
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
+}
+
+void __hci_req_enable_ext_advertising(struct hci_request *req, u8 instance,
+ bool all_instances)
+{
+ struct hci_cp_le_set_ext_adv_enable *cp;
+ struct hci_cp_ext_adv_set *adv_set;
+ struct hci_dev *hdev = req->hdev;
+ u8 data[sizeof(*cp) + sizeof(*adv_set) * HCI_MAX_ADV_INSTANCES];
+ struct adv_info *adv_instance;
+ u16 duration;
+
+ cp = (void *) data;
+ adv_set = (void *) cp->data;
+
+ memset(cp, 0, sizeof(*cp));
+
+ cp->enable = 0x01;
+
+ if (all_instances) {
+ cp->num_of_sets = 0;
+
+ list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
+ /* duration = timeout_in_ms / 10 */
+ duration = adv_instance->timeout * 100;
+
+ memset(adv_set, 0, sizeof(*adv_set));
+
+ adv_set->handle = adv_instance->instance;
+ adv_set->duration = cpu_to_le16(duration);
+
+ adv_set++;
+
+ cp->num_of_sets++;
+ }
+ } else {
+ cp->num_of_sets = 0x01;
+
+ memset(adv_set, 0, sizeof(*adv_set));
+
+ adv_set->handle = instance;
+
+ if (instance > 0) {
+ adv_instance = hci_find_adv_instance(hdev, instance);
+ if (!adv_instance)
+ return;
+
+ /* duration = timeout_in_ms / 10 */
+ duration = adv_instance->timeout * 100;
+ adv_set->duration = cpu_to_le16(duration);
+ }
+ }
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_ENABLE,
+ sizeof(*cp) + sizeof(*adv_set) * cp->num_of_sets,
+ data);
+}
+
+int __hci_req_start_ext_adv(struct hci_request *req, u8 instance,
+ bool all_instances, bool check_flag, u32 flags)
+{
+ struct hci_dev *hdev = req->hdev;
+ struct adv_info *adv_instance;
+
+ if (hci_conn_num(hdev, LE_LINK) > 0)
+ return -EPERM;
+
+ if (all_instances) {
+ if (list_empty(&hdev->adv_instances))
+ return -EPERM;
+
+ list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
+ /* If current instance doesn't need to be changed */
+ if (check_flag && !(adv_instance->flags & flags))
+ continue;
+
+ __hci_req_setup_ext_adv_instance(req,
+ adv_instance->instance);
+ }
+
+ __hci_req_enable_ext_advertising(req, 0, true);
+ } else {
+ __hci_req_setup_ext_adv_instance(req, instance);
+ __hci_req_enable_ext_advertising(req, instance, false);
+ }
+
+ return 0;
+}
+
int __hci_req_schedule_adv_instance(struct hci_request *req, u8 instance,
bool force)
{
@@ -1618,17 +1772,26 @@ static int connectable_update(struct hci_request *req, unsigned long opt)
__hci_req_update_scan(req);
- /* If BR/EDR is not enabled and we disable advertising as a
- * by-product of disabling connectable, we need to update the
- * advertising flags.
- */
- if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
- __hci_req_update_adv_data(req, hdev->cur_adv_instance);
- /* Update the advertising parameters if necessary */
- if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
- !list_empty(&hdev->adv_instances))
- __hci_req_enable_advertising(req);
+ if (ext_adv_capable(hdev)) {
+ /* Update adv flags and adv params */
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ __hci_req_start_ext_adv(req, 0, false, false, 0);
+ else
+ __hci_req_start_ext_adv(req, 0, true, false, 0);
+ } else {
+ /* If BR/EDR is not enabled and we disable advertising as a
+ * by-product of disabling connectable, we need to update the
+ * advertising flags.
+ */
+ if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
+ __hci_req_update_adv_data(req, hdev->cur_adv_instance);
+
+ /* Update the advertising parameters if necessary */
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
+ !list_empty(&hdev->adv_instances))
+ __hci_req_enable_advertising(req);
+ }
__hci_update_background_scan(req);
@@ -1737,8 +1900,13 @@ static int discoverable_update(struct hci_request *req, unsigned long opt)
/* Discoverable mode affects the local advertising
* address in limited privacy mode.
*/
- if (hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
- __hci_req_enable_advertising(req);
+ if (hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY)) {
+ if (ext_adv_capable(hdev))
+ __hci_req_start_ext_adv(req, 0, false,
+ false,0);
+ else
+ __hci_req_enable_advertising(req);
+ }
}
hci_dev_unlock(hdev);
@@ -2327,16 +2495,29 @@ static int powered_update_hci(struct hci_request *req, unsigned long opt)
__hci_req_update_adv_data(req, 0x00);
__hci_req_update_scan_rsp_data(req, 0x00);
- if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
- __hci_req_enable_advertising(req);
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
+ if (ext_adv_capable(hdev))
+ __hci_req_start_ext_adv(req, 0, false,
+ false, 0);
+ else
+ __hci_req_enable_advertising(req);
+ }
} else if (!list_empty(&hdev->adv_instances)) {
- struct adv_info *adv_instance;
-
- adv_instance = list_first_entry(&hdev->adv_instances,
- struct adv_info, list);
- __hci_req_schedule_adv_instance(req,
- adv_instance->instance,
- true);
+ if (ext_adv_capable(hdev)) {
+ __hci_req_start_ext_adv(req, 0, true, false, 0);
+ } else {
+ struct adv_info *adv_instance;
+ struct list_head *head = &hdev->adv_instances;
+ u8 instance;
+
+ adv_instance = list_first_entry(head,
+ struct adv_info,
+ list);
+ instance = adv_instance->instance;
+ __hci_req_schedule_adv_instance(req,
+ instance,
+ true);
+ }
}
}
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 702beb1..2f2dfad 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -80,6 +80,11 @@ void hci_req_clear_adv_instance(struct hci_dev *hdev, struct sock *sk,
struct hci_request *req, u8 instance,
bool force);
+int __hci_req_start_ext_adv(struct hci_request *req, u8 instance,
+ bool all_instances, bool check_flag, u32 flags);
+void __hci_req_enable_ext_advertising(struct hci_request *req, u8 instance,
+ bool all_instances);
+
void __hci_req_update_class(struct hci_request *req);
/* Returns true if HCI commands were queued */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ffd5f7b..2575aff 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -817,7 +817,10 @@ static void rpa_expired(struct work_struct *work)
* function.
*/
hci_req_init(&req, hdev);
- __hci_req_enable_advertising(&req);
+ if (ext_adv_capable(hdev))
+ __hci_req_start_ext_adv(&req, 0, false, false, 0);
+ else
+ __hci_req_enable_advertising(&req);
hci_req_run(&req, NULL);
}
@@ -3025,27 +3028,39 @@ static void adv_expire(struct hci_dev *hdev, u32 flags)
struct hci_request req;
int err;
- adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
- if (!adv_instance)
- return;
+ if (ext_adv_capable(hdev)) {
+ hci_req_init(&req, hdev);
- /* stop if current instance doesn't need to be changed */
- if (!(adv_instance->flags & flags))
- return;
+ __hci_req_start_ext_adv(&req, 0, true, true, flags);
- cancel_adv_timeout(hdev);
+ if (!skb_queue_empty(&req.cmd_q))
+ hci_req_run(&req, NULL);
+ } else {
+ adv_instance = hci_find_adv_instance(hdev,
+ hdev->cur_adv_instance);
+ if (!adv_instance)
+ return;
- adv_instance = hci_get_next_instance(hdev, adv_instance->instance);
- if (!adv_instance)
- return;
+ /* stop if current instance doesn't need to be changed */
+ if (!(adv_instance->flags & flags))
+ return;
- hci_req_init(&req, hdev);
- err = __hci_req_schedule_adv_instance(&req, adv_instance->instance,
- true);
- if (err)
- return;
+ cancel_adv_timeout(hdev);
- hci_req_run(&req, NULL);
+ adv_instance = hci_get_next_instance(hdev,
+ adv_instance->instance);
+ if (!adv_instance)
+ return;
+
+ hci_req_init(&req, hdev);
+ err = __hci_req_schedule_adv_instance(&req,
+ adv_instance->instance,
+ true);
+ if (err)
+ return;
+
+ hci_req_run(&req, NULL);
+ }
}
static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
@@ -3925,19 +3940,25 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status,
list_empty(&hdev->adv_instances))
goto unlock;
- instance = hdev->cur_adv_instance;
- if (!instance) {
- adv_instance = list_first_entry_or_null(&hdev->adv_instances,
- struct adv_info, list);
- if (!adv_instance)
- goto unlock;
+ hci_req_init(&req, hdev);
- instance = adv_instance->instance;
- }
+ if (ext_adv_capable(hdev)) {
+ err = __hci_req_start_ext_adv(&req, 0, true, false, 0);
+ } else {
+ struct list_head *instances = &hdev->adv_instances;
+ instance = hdev->cur_adv_instance;
+ if (!instance) {
+ adv_instance = list_first_entry_or_null(instances,
+ struct adv_info,
+ list);
+ if (!adv_instance)
+ goto unlock;
- hci_req_init(&req, hdev);
+ instance = adv_instance->instance;
+ }
- err = __hci_req_schedule_adv_instance(&req, instance, true);
+ err = __hci_req_schedule_adv_instance(&req, instance, true);
+ }
if (!err)
err = hci_req_run(&req, enable_advertising_instance);
@@ -4035,10 +4056,14 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
* We cannot use update_[adv|scan_rsp]_data() here as the
* HCI_ADVERTISING flag is not yet set.
*/
- hdev->cur_adv_instance = 0x00;
- __hci_req_update_adv_data(&req, 0x00);
- __hci_req_update_scan_rsp_data(&req, 0x00);
- __hci_req_enable_advertising(&req);
+ if (ext_adv_capable(hdev)) {
+ __hci_req_start_ext_adv(&req, 0, false, false, 0);
+ } else {
+ hdev->cur_adv_instance = 0x00;
+ __hci_req_update_adv_data(&req, 0x00);
+ __hci_req_update_scan_rsp_data(&req, 0x00);
+ __hci_req_enable_advertising(&req);
+ }
} else {
__hci_req_disable_advertising(&req);
}
@@ -6248,7 +6273,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
if (hdev->adv_instance_cnt > prev_instance_cnt)
mgmt_advertising_added(sk, hdev, cp->instance);
- if (hdev->cur_adv_instance == cp->instance) {
+ if (hdev->cur_adv_instance == cp->instance && !ext_adv_capable(hdev)) {
/* If the currently advertised instance is being changed then
* cancel the current advertising and schedule the next
* instance. If there is only one instance then the overridden
@@ -6291,7 +6316,12 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
hci_req_init(&req, hdev);
- err = __hci_req_schedule_adv_instance(&req, schedule_instance, true);
+ if (ext_adv_capable(hdev))
+ err = __hci_req_start_ext_adv(&req, schedule_instance, false,
+ false, 0);
+ else
+ err = __hci_req_schedule_adv_instance(&req, schedule_instance,
+ true);
if (!err)
err = hci_req_run(&req, add_advertising_complete);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 1/9] Bluetooth: Read no of adv sets during init Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 2/9] Bluetooth: Impmlement extended adv enable Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-08 8:46 ` Marcel Holtmann
2017-12-04 8:07 ` [RFC 4/9] Bluetooth: Implement disable and removal of adv instance Jaganath Kanakkassery
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch implements Set Ext Adv data and Set Ext Scan rsp data
if controller support extended advertising.
Currently the operation is set as Complete data and fragment
preference is set as no fragment
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 29 +++++++
include/net/bluetooth/hci_core.h | 6 +-
net/bluetooth/hci_core.c | 8 +-
net/bluetooth/hci_request.c | 177 ++++++++++++++++++++++++++++++++-------
net/bluetooth/mgmt.c | 18 +++-
5 files changed, 201 insertions(+), 37 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index dd6b9cb..997995d 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1587,6 +1587,30 @@ struct hci_cp_ext_adv_set {
__u8 max_events;
} __packed;
+#define HCI_MAX_EXT_AD_LENGTH 251
+
+#define HCI_OP_LE_SET_EXT_ADV_DATA 0x2037
+struct hci_cp_le_set_ext_adv_data {
+ __u8 handle;
+ __u8 operation;
+ __u8 fragment_preference;
+ __u8 length;
+ __u8 data[HCI_MAX_EXT_AD_LENGTH];
+} __packed;
+
+#define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038
+struct hci_cp_le_set_ext_scan_rsp_data {
+ __u8 handle;
+ __u8 operation;
+ __u8 fragment_preference;
+ __u8 length;
+ __u8 data[HCI_MAX_EXT_AD_LENGTH];
+} __packed;
+
+#define LE_SET_ADV_DATA_OP_COMPLETE 0x03
+
+#define LE_SET_ADV_DATA_NO_FRAG 0x01
+
/* ---- HCI Events ---- */
#define HCI_EV_INQUIRY_COMPLETE 0x01
@@ -1984,6 +2008,11 @@ struct hci_ev_le_conn_complete {
#define LE_LEGACY_SCAN_RSP_ADV 0x001b
#define LE_LEGACY_SCAN_RSP_ADV_SCAN 0x001a
+/* Extended Advertising event types */
+#define LE_EXT_ADV_NON_CONN_IND 0x0000
+#define LE_EXT_ADV_CONN_IND 0x0001
+#define LE_EXT_ADV_SCAN_IND 0x0002
+
#define ADDR_LE_DEV_PUBLIC 0x00
#define ADDR_LE_DEV_RANDOM 0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 2abeabb..610172a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -166,9 +166,9 @@ struct adv_info {
__u16 remaining_time;
__u16 duration;
__u16 adv_data_len;
- __u8 adv_data[HCI_MAX_AD_LENGTH];
+ __u8 adv_data[HCI_MAX_EXT_AD_LENGTH];
__u16 scan_rsp_len;
- __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
+ __u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH];
__u8 addr_type;
unsigned long state;
};
@@ -176,6 +176,8 @@ struct adv_info {
/* Adv instance states */
enum {
ADV_INST_ENABLED,
+ ADV_INST_ADV_DATA_CHANGED,
+ ADV_INST_SCAN_DATA_CHANGED,
};
#define HCI_MAX_ADV_INSTANCES 5
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3472572..6c22ed6 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2700,12 +2700,16 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
adv_instance->adv_data_len = adv_data_len;
adv_instance->scan_rsp_len = scan_rsp_len;
- if (adv_data_len)
+ if (adv_data_len) {
memcpy(adv_instance->adv_data, adv_data, adv_data_len);
+ set_bit(ADV_INST_ADV_DATA_CHANGED, &adv_instance->state);
+ }
- if (scan_rsp_len)
+ if (scan_rsp_len) {
memcpy(adv_instance->scan_rsp_data,
scan_rsp_data, scan_rsp_len);
+ set_bit(ADV_INST_SCAN_DATA_CHANGED, &adv_instance->state);
+ }
adv_instance->timeout = timeout;
adv_instance->remaining_time = timeout;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 05f1388..9c45cbf 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1132,29 +1132,81 @@ static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instance,
void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instance)
{
struct hci_dev *hdev = req->hdev;
- struct hci_cp_le_set_scan_rsp_data cp;
u8 len;
+ struct adv_info *adv_inst;
if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
return;
- memset(&cp, 0, sizeof(cp));
+ if (ext_adv_capable(hdev)) {
+ struct hci_cp_le_set_ext_scan_rsp_data cp;
- if (instance)
- len = create_instance_scan_rsp_data(hdev, instance, cp.data);
- else
- len = create_default_scan_rsp_data(hdev, cp.data);
+ memset(&cp, 0, sizeof(cp));
- if (hdev->scan_rsp_data_len == len &&
- !memcmp(cp.data, hdev->scan_rsp_data, len))
- return;
+ if (instance)
+ len = create_instance_scan_rsp_data(hdev, instance,
+ cp.data);
+ else
+ len = create_default_scan_rsp_data(hdev, cp.data);
- memcpy(hdev->scan_rsp_data, cp.data, sizeof(cp.data));
- hdev->scan_rsp_data_len = len;
+ if (!instance) {
+ /* There's nothing to do if the data hasn't changed */
+ if (hdev->scan_rsp_data_len == len &&
+ memcmp(cp.data, hdev->scan_rsp_data, len) == 0)
+ return;
- cp.length = len;
+ memcpy(hdev->scan_rsp_data, cp.data, sizeof(cp.data));
+ memcpy(hdev->scan_rsp_data, cp.data,
+ sizeof(hdev->scan_rsp_data));
+ hdev->scan_rsp_data_len = len;
+ } else {
+ adv_inst = hci_find_adv_instance(hdev, instance);
+ if (!adv_inst)
+ return;
- hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
+ /* There's nothing to do if the data hasn't changed and
+ * scan rsp data is not changed by user
+ */
+ if (!test_and_clear_bit(ADV_INST_SCAN_DATA_CHANGED,
+ &adv_inst->state) &&
+ adv_inst->scan_rsp_len == len &&
+ memcmp(cp.data, adv_inst->scan_rsp_data, len) == 0)
+ return;
+
+ memcpy(adv_inst->scan_rsp_data, cp.data,
+ sizeof(cp.data));
+ adv_inst->scan_rsp_len = len;
+ }
+
+ cp.handle = instance;
+ cp.length = len;
+ cp.operation = LE_SET_ADV_DATA_OP_COMPLETE;
+ cp.fragment_preference = LE_SET_ADV_DATA_NO_FRAG;
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_SCAN_RSP_DATA, sizeof(cp),
+ &cp);
+ } else {
+ struct hci_cp_le_set_scan_rsp_data cp;
+
+ memset(&cp, 0, sizeof(cp));
+
+ if (instance)
+ len = create_instance_scan_rsp_data(hdev, instance,
+ cp.data);
+ else
+ len = create_default_scan_rsp_data(hdev, cp.data);
+
+ if (hdev->scan_rsp_data_len == len &&
+ !memcmp(cp.data, hdev->scan_rsp_data, len))
+ return;
+
+ memcpy(hdev->scan_rsp_data, cp.data, sizeof(cp.data));
+ hdev->scan_rsp_data_len = len;
+
+ cp.length = len;
+
+ hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
+ }
}
static u8 create_instance_adv_data(struct hci_dev *hdev, u8 instance, u8 *ptr)
@@ -1228,27 +1280,70 @@ static u8 create_instance_adv_data(struct hci_dev *hdev, u8 instance, u8 *ptr)
void __hci_req_update_adv_data(struct hci_request *req, u8 instance)
{
struct hci_dev *hdev = req->hdev;
- struct hci_cp_le_set_adv_data cp;
u8 len;
+ struct adv_info *adv_instance;
if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
return;
- memset(&cp, 0, sizeof(cp));
+ if (ext_adv_capable(hdev)) {
+ struct hci_cp_le_set_ext_adv_data cp;
+
+ memset(&cp, 0, sizeof(cp));
- len = create_instance_adv_data(hdev, instance, cp.data);
+ len = create_instance_adv_data(hdev, instance, cp.data);
- /* There's nothing to do if the data hasn't changed */
- if (hdev->adv_data_len == len &&
- memcmp(cp.data, hdev->adv_data, len) == 0)
- return;
+ if (!instance) {
+ /* There's nothing to do if the data hasn't changed */
+ if (hdev->adv_data_len == len &&
+ memcmp(cp.data, hdev->adv_data, len) == 0)
+ return;
- memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
- hdev->adv_data_len = len;
+ memcpy(hdev->adv_data, cp.data, sizeof(hdev->adv_data));
+ hdev->adv_data_len = len;
+ } else {
+ adv_instance = hci_find_adv_instance(hdev, instance);
+ if (!adv_instance)
+ return;
- cp.length = len;
+ /* There's nothing to do if the data hasn't changed and
+ * adv data is not changed by user
+ */
+ if (!test_and_clear_bit(ADV_INST_ADV_DATA_CHANGED,
+ &adv_instance->state) &&
+ adv_instance->adv_data_len == len &&
+ memcmp(cp.data, adv_instance->adv_data, len) == 0)
+ return;
+
+ memcpy(adv_instance->adv_data, cp.data, sizeof(cp.data));
+ adv_instance->adv_data_len = len;
+ }
+
+ cp.length = len;
+ cp.handle = instance;
+ cp.operation = LE_SET_ADV_DATA_OP_COMPLETE;
+ cp.fragment_preference = LE_SET_ADV_DATA_NO_FRAG;
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_DATA, sizeof(cp), &cp);
+ } else {
+ struct hci_cp_le_set_adv_data cp;
+
+ memset(&cp, 0, sizeof(cp));
+
+ len = create_instance_adv_data(hdev, instance, cp.data);
+
+ /* There's nothing to do if the data hasn't changed */
+ if (hdev->adv_data_len == len &&
+ memcmp(cp.data, hdev->adv_data, len) == 0)
+ return;
- hci_req_add(req, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+ memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
+ hdev->adv_data_len = len;
+
+ cp.length = len;
+
+ hci_req_add(req, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+ }
}
int hci_req_update_adv_data(struct hci_dev *hdev, u8 instance)
@@ -1337,6 +1432,8 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
u32 flags;
/* In ext adv set param interval is 3 octets */
const u8 adv_interval[3] = { 0x00, 0x08, 0x00 };
+ struct adv_info *adv_inst;
+ bool secondary_adv;
flags = get_adv_instance_flags(hdev, instance);
@@ -1351,12 +1448,29 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
memcpy(cp.min_interval, adv_interval, sizeof(cp.min_interval));
memcpy(cp.max_interval, adv_interval, sizeof(cp.max_interval));
- if (connectable)
- cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
- else if (get_adv_instance_scan_rsp_len(hdev, instance))
- cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_SCAN_IND);
- else
- cp.evt_properties = cpu_to_le16(LE_LEGACY_NONCONN_IND);
+ adv_inst = hci_find_adv_instance(hdev, instance);
+ if (!adv_inst)
+ return;
+
+ secondary_adv = (adv_inst->adv_data_len > HCI_MAX_AD_LENGTH ||
+ adv_inst->scan_rsp_len > HCI_MAX_AD_LENGTH);
+
+ if (connectable) {
+ if (secondary_adv)
+ cp.evt_properties = cpu_to_le16(LE_EXT_ADV_CONN_IND);
+ else
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
+ } else if (get_adv_instance_scan_rsp_len(hdev, instance)) {
+ if (secondary_adv)
+ cp.evt_properties = cpu_to_le16(LE_EXT_ADV_SCAN_IND);
+ else
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_SCAN_IND);
+ } else {
+ if (secondary_adv)
+ cp.evt_properties = cpu_to_le16(LE_EXT_ADV_NON_CONN_IND);
+ else
+ cp.evt_properties = cpu_to_le16(LE_LEGACY_NONCONN_IND);
+ }
cp.own_addr_type = BDADDR_LE_PUBLIC;
cp.channel_map = hdev->le_adv_channel_map;
@@ -1366,6 +1480,9 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
cp.handle = instance;
hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
+
+ __hci_req_update_adv_data(req, instance);
+ __hci_req_update_scan_rsp_data(req, instance);
}
void __hci_req_enable_ext_advertising(struct hci_request *req, u8 instance,
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2575aff..65e5131 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6021,8 +6021,15 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
supported_flags = get_supported_adv_flags(hdev);
rp->supported_flags = cpu_to_le32(supported_flags);
- rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
- rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
+
+ if (ext_adv_capable(hdev)) {
+ rp->max_adv_data_len = HCI_MAX_EXT_AD_LENGTH;
+ rp->max_scan_rsp_len = HCI_MAX_EXT_AD_LENGTH;
+ } else {
+ rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
+ rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
+ }
+
rp->max_instances = hdev->le_no_of_adv_sets;
rp->num_instances = hdev->adv_instance_cnt;
@@ -6052,7 +6059,12 @@ static u8 calculate_name_len(struct hci_dev *hdev)
static u8 tlv_data_max_len(struct hci_dev *hdev, u32 adv_flags,
bool is_adv_data)
{
- u8 max_len = HCI_MAX_AD_LENGTH;
+ u8 max_len;
+
+ if (ext_adv_capable(hdev))
+ max_len = HCI_MAX_EXT_AD_LENGTH;
+ else
+ max_len = HCI_MAX_AD_LENGTH;
if (is_adv_data) {
if (adv_flags & (MGMT_ADV_FLAG_DISCOV |
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports
2017-12-04 8:07 ` [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports Jaganath Kanakkassery
@ 2017-12-08 8:46 ` Marcel Holtmann
2017-12-08 12:02 ` Jaganath K
0 siblings, 1 reply; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 8:46 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth, Jaganath Kanakkassery
Hi Jaganath,
> This patch implements Set Ext Adv data and Set Ext Scan rsp data
> if controller support extended advertising.
>
> Currently the operation is set as Complete data and fragment
> preference is set as no fragment
>
> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
> ---
> include/net/bluetooth/hci.h | 29 +++++++
> include/net/bluetooth/hci_core.h | 6 +-
> net/bluetooth/hci_core.c | 8 +-
> net/bluetooth/hci_request.c | 177 ++++++++++++++++++++++++++++++++-------
> net/bluetooth/mgmt.c | 18 +++-
> 5 files changed, 201 insertions(+), 37 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index dd6b9cb..997995d 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1587,6 +1587,30 @@ struct hci_cp_ext_adv_set {
> __u8 max_events;
> } __packed;
>
> +#define HCI_MAX_EXT_AD_LENGTH 251
> +
> +#define HCI_OP_LE_SET_EXT_ADV_DATA 0x2037
> +struct hci_cp_le_set_ext_adv_data {
> + __u8 handle;
> + __u8 operation;
> + __u8 fragment_preference;
> + __u8 length;
> + __u8 data[HCI_MAX_EXT_AD_LENGTH];
> +} __packed;
> +
> +#define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038
> +struct hci_cp_le_set_ext_scan_rsp_data {
> + __u8 handle;
> + __u8 operation;
> + __u8 fragment_preference;
> + __u8 length;
> + __u8 data[HCI_MAX_EXT_AD_LENGTH];
> +} __packed;
> +
> +#define LE_SET_ADV_DATA_OP_COMPLETE 0x03
> +
> +#define LE_SET_ADV_DATA_NO_FRAG 0x01
> +
> /* ---- HCI Events ---- */
> #define HCI_EV_INQUIRY_COMPLETE 0x01
>
> @@ -1984,6 +2008,11 @@ struct hci_ev_le_conn_complete {
> #define LE_LEGACY_SCAN_RSP_ADV 0x001b
> #define LE_LEGACY_SCAN_RSP_ADV_SCAN 0x001a
>
> +/* Extended Advertising event types */
> +#define LE_EXT_ADV_NON_CONN_IND 0x0000
> +#define LE_EXT_ADV_CONN_IND 0x0001
> +#define LE_EXT_ADV_SCAN_IND 0x0002
> +
> #define ADDR_LE_DEV_PUBLIC 0x00
> #define ADDR_LE_DEV_RANDOM 0x01
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 2abeabb..610172a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -166,9 +166,9 @@ struct adv_info {
> __u16 remaining_time;
> __u16 duration;
> __u16 adv_data_len;
> - __u8 adv_data[HCI_MAX_AD_LENGTH];
> + __u8 adv_data[HCI_MAX_EXT_AD_LENGTH];
> __u16 scan_rsp_len;
> - __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
> + __u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH];
I don’t think you can do it this way. The legacy advertising is its own instance. So you need extra adv_data_ext[] field here since you need to track both. I would need to double check if some ext adv set number is magically matched to the legacy advertising, but I do not remember that.
Even if you use the new HCI commands to set up legacy advertising, the length does not magically increase.
Frankly I would like to see first that we use the new HCI commands to set up legacy advertising. That way we improve using multiple legacy advertising instances at the same time via controller offload and avoid rotation. That part should be figured out first and implemented before adding the extra length of PHY details.
And btw. we need mgmt-tester patches to test the correct behavior. The advertising API is rather complex and powerful.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports
2017-12-08 8:46 ` Marcel Holtmann
@ 2017-12-08 12:02 ` Jaganath K
0 siblings, 0 replies; 23+ messages in thread
From: Jaganath K @ 2017-12-08 12:02 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: open list:BLUETOOTH DRIVERS, Jaganath Kanakkassery
Hi Marcel,
> Hi Jaganath,
>
>> This patch implements Set Ext Adv data and Set Ext Scan rsp data
>> if controller support extended advertising.
>>
>> Currently the operation is set as Complete data and fragment
>> preference is set as no fragment
>>
>> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
>> ---
>> include/net/bluetooth/hci.h | 29 +++++++
>> include/net/bluetooth/hci_core.h | 6 +-
>> net/bluetooth/hci_core.c | 8 +-
>> net/bluetooth/hci_request.c | 177 ++++++++++++++++++++++++++++++++-=
------
>> net/bluetooth/mgmt.c | 18 +++-
>> 5 files changed, 201 insertions(+), 37 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index dd6b9cb..997995d 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -1587,6 +1587,30 @@ struct hci_cp_ext_adv_set {
>> __u8 max_events;
>> } __packed;
>>
>> +#define HCI_MAX_EXT_AD_LENGTH 251
>> +
>> +#define HCI_OP_LE_SET_EXT_ADV_DATA 0x2037
>> +struct hci_cp_le_set_ext_adv_data {
>> + __u8 handle;
>> + __u8 operation;
>> + __u8 fragment_preference;
>> + __u8 length;
>> + __u8 data[HCI_MAX_EXT_AD_LENGTH];
>> +} __packed;
>> +
>> +#define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038
>> +struct hci_cp_le_set_ext_scan_rsp_data {
>> + __u8 handle;
>> + __u8 operation;
>> + __u8 fragment_preference;
>> + __u8 length;
>> + __u8 data[HCI_MAX_EXT_AD_LENGTH];
>> +} __packed;
>> +
>> +#define LE_SET_ADV_DATA_OP_COMPLETE 0x03
>> +
>> +#define LE_SET_ADV_DATA_NO_FRAG 0x01
>> +
>> /* ---- HCI Events ---- */
>> #define HCI_EV_INQUIRY_COMPLETE 0x01
>>
>> @@ -1984,6 +2008,11 @@ struct hci_ev_le_conn_complete {
>> #define LE_LEGACY_SCAN_RSP_ADV 0x001b
>> #define LE_LEGACY_SCAN_RSP_ADV_SCAN 0x001a
>>
>> +/* Extended Advertising event types */
>> +#define LE_EXT_ADV_NON_CONN_IND 0x0000
>> +#define LE_EXT_ADV_CONN_IND 0x0001
>> +#define LE_EXT_ADV_SCAN_IND 0x0002
>> +
>> #define ADDR_LE_DEV_PUBLIC 0x00
>> #define ADDR_LE_DEV_RANDOM 0x01
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index 2abeabb..610172a 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -166,9 +166,9 @@ struct adv_info {
>> __u16 remaining_time;
>> __u16 duration;
>> __u16 adv_data_len;
>> - __u8 adv_data[HCI_MAX_AD_LENGTH];
>> + __u8 adv_data[HCI_MAX_EXT_AD_LENGTH];
>> __u16 scan_rsp_len;
>> - __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
>> + __u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH];
>
> I don=E2=80=99t think you can do it this way. The legacy advertising is i=
ts own instance. So you need extra adv_data_ext[] field here since you need=
to track both. I would need to double check if some ext adv set number is =
magically matched to the legacy advertising, but I do not remember that.
>
> Even if you use the new HCI commands to set up legacy advertising, the le=
ngth does not magically increase.
>
I think we need to use new HCI commands always (if supported) since as
i understood from spec
it says both legacy and new commands cannot be used together in one BT
on session.
Pz see "3.1.1 Legacy and Extended Advertising"
> Frankly I would like to see first that we use the new HCI commands to set=
up legacy advertising. That way we improve using multiple legacy advertisi=
ng instances at the same time via controller offload and avoid rotation. Th=
at part should be figured out first and implemented before adding the extra=
length of PHY details.
>
> And btw. we need mgmt-tester patches to test the correct behavior. The ad=
vertising API is rather complex and powerful.
>
> Regards
>
> Marcel
>
Thanks,
Jaganath
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 4/9] Bluetooth: Implement disable and removal of adv instance
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (2 preceding siblings ...)
2017-12-04 8:07 ` [RFC 3/9] Bluetooth: Use Set ext adv/scan rsp data if controller supports Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 5/9] Bluetooth: Process Adv-Set Terminate event Jaganath Kanakkassery
` (4 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch implements hci_req_clear_ext_adv_instance() whose semantics
is same as hci_req_clear_adv_instance().
Also handles disable case of set scan enable complete.
Adv sets can be removed from the controller using two commands,
clear - which removes all the sets, remove - which removes only
the given set.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 7 +++
net/bluetooth/hci_event.c | 33 +++++++++++
net/bluetooth/hci_request.c | 141 ++++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/hci_request.h | 3 +
net/bluetooth/mgmt.c | 17 ++++--
5 files changed, 197 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 997995d..65d2124 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1611,6 +1611,13 @@ struct hci_cp_le_set_ext_scan_rsp_data {
#define LE_SET_ADV_DATA_NO_FRAG 0x01
+#define HCI_OP_LE_REMOVE_ADV_SET 0x203c
+struct hci_cp_le_remove_adv_set {
+ __u8 handle;
+} __packed;
+
+#define HCI_OP_LE_CLEAR_ADV_SETS 0x203d
+
/* ---- HCI Events ---- */
#define HCI_EV_INQUIRY_COMPLETE 0x01
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 724c668..64873dd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1121,8 +1121,41 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev,
adv_set++;
}
+ } else {
+ /* If all instances are disabled */
+ if (!cp->num_of_sets) {
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
+
+ list_for_each_entry(adv_instance, &hdev->adv_instances,
+ list)
+ clear_bit(ADV_INST_ENABLED,
+ &adv_instance->state);
+
+ goto unlock;
+ }
+
+ for (i = 0; i < cp->num_of_sets; i++) {
+ adv_instance = hci_find_adv_instance(hdev,
+ adv_set->handle);
+ if (adv_instance)
+ clear_bit(ADV_INST_ENABLED,
+ &adv_instance->state);
+
+ adv_set++;
+ }
+
+ list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
+ /* Dont clear HCI_LE_ADV if atleast one instance is
+ * enabled
+ */
+ if (test_bit(ADV_INST_ENABLED, &adv_instance->state))
+ goto unlock;
+ }
+
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
}
+unlock:
hci_dev_unlock(hdev);
}
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 9c45cbf..ca235eb 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1423,6 +1423,55 @@ unlock:
hci_dev_unlock(hdev);
}
+void __hci_req_remove_ext_adv_set(struct hci_request *req, u8 instance)
+{
+ struct hci_cp_le_remove_adv_set cp;
+
+ memset(&cp, 0, sizeof(cp));
+
+ cp.handle = instance;
+
+ hci_req_add(req, HCI_OP_LE_REMOVE_ADV_SET, sizeof(cp), &cp);
+}
+
+void __hci_req_clear_ext_adv_sets(struct hci_request *req)
+{
+ hci_req_add(req, HCI_OP_LE_REMOVE_ADV_SET, 0, NULL);
+}
+
+void __hci_req_stop_ext_adv(struct hci_request *req, u8 instance,
+ bool all_instances)
+{
+ if (all_instances) {
+ struct hci_cp_le_set_ext_adv_enable cp;
+
+ cp.enable = 0x00;
+ /* Disable all adv sets */
+ cp.num_of_sets = 0x00;
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_ENABLE, sizeof(cp), &cp);
+ } else {
+ struct hci_cp_le_set_ext_adv_enable *cp;
+ struct hci_cp_ext_adv_set *cp_adv_set;
+ u8 data[sizeof(*cp) + sizeof(*cp_adv_set)];
+
+ cp = (void *) data;
+ cp_adv_set = (void *) cp->data;
+
+ memset(cp, 0, sizeof(*cp));
+
+ cp->enable = 0x00;
+ cp->num_of_sets = 0x01;
+
+ memset(cp_adv_set, 0, sizeof(*cp_adv_set));
+
+ cp_adv_set->handle = instance;
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_ENABLE, sizeof(data),
+ data);
+ }
+}
+
static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
u8 instance)
{
@@ -1554,6 +1603,10 @@ int __hci_req_start_ext_adv(struct hci_request *req, u8 instance,
if (list_empty(&hdev->adv_instances))
return -EPERM;
+ /* If atleast one adv is enabled then disable all */
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV))
+ __hci_req_stop_ext_adv(req, 0, true);
+
list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
/* If current instance doesn't need to be changed */
if (check_flag && !(adv_instance->flags & flags))
@@ -1565,6 +1618,27 @@ int __hci_req_start_ext_adv(struct hci_request *req, u8 instance,
__hci_req_enable_ext_advertising(req, 0, true);
} else {
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
+ /* If instance 0 is going to be enabled then disable
+ * all other instances. This is to be aligned with
+ * the current design and and is not actually required
+ * in case of extended adv where in all the instances
+ * including 0 can be enabled at the same time
+ */
+ if (!instance)
+ __hci_req_stop_ext_adv(req, 0, true);
+ else {
+ adv_instance = hci_find_adv_instance(hdev,
+ instance);
+ if (!adv_instance)
+ return 0;
+
+ if (test_bit(ADV_INST_ENABLED,
+ &adv_instance->state))
+ __hci_req_stop_ext_adv(req, instance,
+ false);
+ }
+ }
__hci_req_setup_ext_adv_instance(req, instance);
__hci_req_enable_ext_advertising(req, instance, false);
}
@@ -1649,6 +1723,68 @@ static void cancel_adv_timeout(struct hci_dev *hdev)
* For instance == 0x00:
* - force == true: All instances will be removed regardless of their timeout
* setting.
+ * - force == false: All the instances will be disabled and only instances that
+ * have a timeout will be removed. Instance state will be set to IDLE.
+ */
+static void hci_req_clear_ext_adv_instance(struct hci_dev *hdev,
+ struct hci_request *req,
+ struct sock *sk, u8 instance,
+ bool force)
+{
+ struct adv_info *adv_instance, *n;
+ int err;
+ u8 rem_inst;
+
+ if (instance == 0x00) {
+ /* Disable and remove all instances from the controller */
+ if (req) {
+ __hci_req_stop_ext_adv(req, 0, true);
+ __hci_req_clear_ext_adv_sets(req);
+ }
+
+ /* Remove the instances which has a timeout */
+ list_for_each_entry_safe(adv_instance, n, &hdev->adv_instances,
+ list) {
+ if (force || adv_instance->timeout) {
+ rem_inst = adv_instance->instance;
+
+ err = hci_remove_adv_instance(hdev, rem_inst);
+ if (!err)
+ mgmt_advertising_removed(sk, hdev,
+ rem_inst);
+ } else {
+ /* Set state to idle */
+ adv_instance->state = 0;
+ }
+ }
+ } else {
+ adv_instance = hci_find_adv_instance(hdev, instance);
+ if (!adv_instance)
+ return;
+
+ if (test_bit(ADV_INST_ENABLED, &adv_instance->state) && req)
+ __hci_req_stop_ext_adv(req, instance, false);
+
+ if (force) {
+ if (req)
+ __hci_req_remove_ext_adv_set(req, instance);
+
+ err = hci_remove_adv_instance(hdev, instance);
+ if (!err)
+ mgmt_advertising_removed(sk, hdev, instance);
+ }
+ }
+}
+
+/* For a single instance:
+ * - force == true: The instance will be removed even when its remaining
+ * lifetime is not zero.
+ * - force == false: the instance will be deactivated but kept stored unless
+ * the remaining lifetime is zero.
+ *
+ * For instance == 0x00:
+ * - force == true: All instances will be removed regardless of their timeout
+ * setting.
* - force == false: Only instances that have a timeout will be removed.
*/
void hci_req_clear_adv_instance(struct hci_dev *hdev, struct sock *sk,
@@ -1659,6 +1795,11 @@ void hci_req_clear_adv_instance(struct hci_dev *hdev, struct sock *sk,
int err;
u8 rem_inst;
+ if (ext_adv_capable(hdev)) {
+ hci_req_clear_ext_adv_instance(hdev, req, sk, instance, force);
+ return;
+ }
+
/* Cancel any timeout concerning the removed instance(s). */
if (!instance || hdev->cur_adv_instance == instance)
cancel_adv_timeout(hdev);
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 2f2dfad..936f6c5 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -84,6 +84,9 @@ int __hci_req_start_ext_adv(struct hci_request *req, u8 instance,
bool all_instances, bool check_flag, u32 flags);
void __hci_req_enable_ext_advertising(struct hci_request *req, u8 instance,
bool all_instances);
+void __hci_req_stop_ext_adv(struct hci_request *req, u8 instance,
+ bool all_instances);
+void __hci_req_clear_ext_adv_sets(struct hci_request *req);
void __hci_req_update_class(struct hci_request *req);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 65e5131..120da46 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1824,8 +1824,14 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
hci_cp.le = val;
hci_cp.simul = 0x00;
} else {
- if (hci_dev_test_flag(hdev, HCI_LE_ADV))
- __hci_req_disable_advertising(&req);
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
+ if (ext_adv_capable(hdev)) {
+ __hci_req_stop_ext_adv(&req, 0, true);
+ __hci_req_clear_ext_adv_sets(&req);
+ } else {
+ __hci_req_disable_advertising(&req);
+ }
+ }
}
hci_req_add(&req, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
@@ -4065,7 +4071,10 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
__hci_req_enable_advertising(&req);
}
} else {
- __hci_req_disable_advertising(&req);
+ if (ext_adv_capable(hdev))
+ __hci_req_stop_ext_adv(&req, 0, false);
+ else
+ __hci_req_disable_advertising(&req);
}
err = hci_req_run(&req, set_advertising_complete);
@@ -6415,7 +6424,7 @@ static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
hci_req_clear_adv_instance(hdev, sk, &req, cp->instance, true);
- if (list_empty(&hdev->adv_instances))
+ if (list_empty(&hdev->adv_instances) && !ext_adv_capable(hdev))
__hci_req_disable_advertising(&req);
/* If no HCI commands have been collected so far or the HCI_ADVERTISING
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 5/9] Bluetooth: Process Adv-Set Terminate event
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (3 preceding siblings ...)
2017-12-04 8:07 ` [RFC 4/9] Bluetooth: Implement disable and removal of adv instance Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-08 8:51 ` Marcel Holtmann
2017-12-04 8:07 ` [RFC 6/9] Bluetooth: Use ext adv for directed adv Jaganath Kanakkassery
` (3 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch enables and process Advertising Set Terminate event.
This event can come mainly in two scenarios - Connection and
timeout. In case of connection - adv instance state will be
changed to disabled and in timeout - adv instance will be removed.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 8 ++++++
net/bluetooth/hci_core.c | 6 +++++
net/bluetooth/hci_event.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 65d2124..2ee16f7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2115,6 +2115,14 @@ struct hci_ev_le_enh_conn_complete {
__u8 clk_accurancy;
} __packed;
+#define HCI_EV_LE_ADV_SET_TERM 0x12
+struct hci_ev_le_adv_set_term {
+ __u8 status;
+ __u8 handle;
+ __le16 conn_handle;
+ __u8 num_evts;
+} __packed;
+
/* Internal events generated by Bluetooth stack */
#define HCI_EV_STACK_INTERNAL 0xfd
struct hci_ev_stack_internal {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6c22ed6..7f17325 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -689,6 +689,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
if (hdev->commands[37] & 0x80)
events[1] |= 0x02; /* LE Enhanced conn complete */
+ /* If the controller supports the LE Extended advertising
+ * enable the corresponding event.
+ */
+ if (ext_adv_capable(hdev))
+ events[2] |= 0x02; /* LE Adv Set Terminated */
+
hci_req_add(req, HCI_OP_LE_SET_EVENT_MASK, sizeof(events),
events);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 64873dd..8a118c1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4879,6 +4879,62 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
le16_to_cpu(ev->supervision_timeout));
}
+static void hci_le_adv_set_terminated_evt(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_ev_le_adv_set_term *ev = (void *) skb->data;
+ struct adv_info *adv_instance = NULL;
+ int err;
+
+ hci_dev_lock(hdev);
+
+ if (ev->handle) {
+ adv_instance = hci_find_adv_instance(hdev, ev->handle);
+ if (!adv_instance)
+ goto unlock;
+ }
+
+ /* If this is because of connection */
+ if (!ev->status) {
+ if (!ev->handle)
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
+ else
+ clear_bit(ADV_INST_ENABLED, &adv_instance->state);
+ } else if (ev->handle) {
+ /* Remove the instance in all other cases */
+ err = hci_remove_adv_instance(hdev, ev->handle);
+ if (!err)
+ mgmt_advertising_removed(NULL, hdev, ev->handle);
+ }
+
+ /* If instance 0 was advertiing then others would be already disabled */
+ if (!ev->handle)
+ goto unlock;
+
+ /* Check state of other instances and modify flags accordingly */
+ list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
+ if (test_bit(ADV_INST_ENABLED, &adv_instance->state)) {
+ if (!ev->status) {
+ struct hci_request req;
+
+ /* If it is a connection and another instance
+ * is enabled then disable all, to be
+ * alligned with the existing design
+ */
+ hci_req_init(&req, hdev);
+ __hci_req_stop_ext_adv(&req, 0, true);
+ hci_req_run(&req, NULL);
+ }
+ goto unlock;
+ }
+ }
+
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static void hci_le_conn_update_complete_evt(struct hci_dev *hdev,
struct sk_buff *skb)
{
@@ -5492,6 +5548,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_le_enh_conn_complete_evt(hdev, skb);
break;
+ case HCI_EV_LE_ADV_SET_TERM:
+ hci_le_adv_set_terminated_evt(hdev, skb);
+ break;
+
default:
break;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 5/9] Bluetooth: Process Adv-Set Terminate event
2017-12-04 8:07 ` [RFC 5/9] Bluetooth: Process Adv-Set Terminate event Jaganath Kanakkassery
@ 2017-12-08 8:51 ` Marcel Holtmann
0 siblings, 0 replies; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 8:51 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth, Jaganath Kanakkassery
Hi Jaganath,
> This patch enables and process Advertising Set Terminate event.
> This event can come mainly in two scenarios - Connection and
> timeout. In case of connection - adv instance state will be
> changed to disabled and in timeout - adv instance will be removed.
>
> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
> ---
> include/net/bluetooth/hci.h | 8 ++++++
> net/bluetooth/hci_core.c | 6 +++++
> net/bluetooth/hci_event.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 74 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 65d2124..2ee16f7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -2115,6 +2115,14 @@ struct hci_ev_le_enh_conn_complete {
> __u8 clk_accurancy;
> } __packed;
>
> +#define HCI_EV_LE_ADV_SET_TERM 0x12
> +struct hci_ev_le_adv_set_term {
> + __u8 status;
> + __u8 handle;
> + __le16 conn_handle;
> + __u8 num_evts;
> +} __packed;
please get the formatting right here.
> +
> /* Internal events generated by Bluetooth stack */
> #define HCI_EV_STACK_INTERNAL 0xfd
> struct hci_ev_stack_internal {
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 6c22ed6..7f17325 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -689,6 +689,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> if (hdev->commands[37] & 0x80)
> events[1] |= 0x02; /* LE Enhanced conn complete */
>
> + /* If the controller supports the LE Extended advertising
> + * enable the corresponding event.
Here we also have extra whitespaces.
> + */
> + if (ext_adv_capable(hdev))
> + events[2] |= 0x02; /* LE Adv Set Terminated */
> +
> hci_req_add(req, HCI_OP_LE_SET_EVENT_MASK, sizeof(events),
> events);
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 64873dd..8a118c1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4879,6 +4879,62 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
> le16_to_cpu(ev->supervision_timeout));
> }
>
> +static void hci_le_adv_set_terminated_evt(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + struct hci_ev_le_adv_set_term *ev = (void *) skb->data;
> + struct adv_info *adv_instance = NULL;
> + int err;
> +
> + hci_dev_lock(hdev);
> +
> + if (ev->handle) {
> + adv_instance = hci_find_adv_instance(hdev, ev->handle);
> + if (!adv_instance)
> + goto unlock;
> + }
> +
> + /* If this is because of connection */
> + if (!ev->status) {
> + if (!ev->handle)
> + hci_dev_clear_flag(hdev, HCI_LE_ADV);
> + else
> + clear_bit(ADV_INST_ENABLED, &adv_instance->state);
This needs to be done all super carefully since we now will have multiple active advertising sets and connections can happen on more than one. So just clearing LE_ADV flag is dangerous.
> + } else if (ev->handle) {
> + /* Remove the instance in all other cases */
> + err = hci_remove_adv_instance(hdev, ev->handle);
> + if (!err)
> + mgmt_advertising_removed(NULL, hdev, ev->handle);
> + }
> +
> + /* If instance 0 was advertiing then others would be already disabled */
> + if (!ev->handle)
> + goto unlock;
> +
> + /* Check state of other instances and modify flags accordingly */
> + list_for_each_entry(adv_instance, &hdev->adv_instances, list) {
> + if (test_bit(ADV_INST_ENABLED, &adv_instance->state)) {
> + if (!ev->status) {
> + struct hci_request req;
> +
> + /* If it is a connection and another instance
> + * is enabled then disable all, to be
> + * alligned with the existing design
> + */
> + hci_req_init(&req, hdev);
> + __hci_req_stop_ext_adv(&req, 0, true);
> + hci_req_run(&req, NULL);
> + }
> + goto unlock;
> + }
> + }
> +
> + hci_dev_clear_flag(hdev, HCI_LE_ADV);
> +
> +unlock:
> + hci_dev_unlock(hdev);
> +}
> +
> static void hci_le_conn_update_complete_evt(struct hci_dev *hdev,
> struct sk_buff *skb)
> {
> @@ -5492,6 +5548,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
> hci_le_enh_conn_complete_evt(hdev, skb);
> break;
>
> + case HCI_EV_LE_ADV_SET_TERM:
> + hci_le_adv_set_terminated_evt(hdev, skb);
> + break;
> +
> default:
> break;
> }
I have the feeling we really need to store the connection handle in the instance data. So that when we get the connection complete event that we can clearly identify where things belong to. So dies the connection complete or set terminated comes first. I think that btmon traces in the commit messages would be good here.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 6/9] Bluetooth: Use ext adv for directed adv
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (4 preceding siblings ...)
2017-12-04 8:07 ` [RFC 5/9] Bluetooth: Process Adv-Set Terminate event Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-08 8:56 ` Marcel Holtmann
2017-12-04 8:07 ` [RFC 7/9] Bluetooth: Implement Set ADV set random address Jaganath Kanakkassery
` (2 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch does extended advertising for directed advertising
if the controller supportes. Instance 0 is used for directed
advertising.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
net/bluetooth/hci_conn.c | 67 ++++++++++++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 22 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9459311..789a91a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -827,35 +827,58 @@ static void hci_req_directed_advertising(struct hci_request *req,
struct hci_conn *conn)
{
struct hci_dev *hdev = req->hdev;
- struct hci_cp_le_set_adv_param cp;
u8 own_addr_type;
u8 enable;
- /* Clear the HCI_LE_ADV bit temporarily so that the
- * hci_update_random_address knows that it's safe to go ahead
- * and write a new random address. The flag will be set back on
- * as soon as the SET_ADV_ENABLE HCI command completes.
- */
- hci_dev_clear_flag(hdev, HCI_LE_ADV);
+ if (ext_adv_capable(hdev)) {
+ struct hci_cp_le_set_ext_adv_params cp;
- /* Set require_privacy to false so that the remote device has a
- * chance of identifying us.
- */
- if (hci_update_random_address(req, false, conn_use_rpa(conn),
- &own_addr_type) < 0)
- return;
+ memset(&cp, 0, sizeof(cp));
- memset(&cp, 0, sizeof(cp));
- cp.type = LE_ADV_DIRECT_IND;
- cp.own_address_type = own_addr_type;
- cp.direct_addr_type = conn->dst_type;
- bacpy(&cp.direct_addr, &conn->dst);
- cp.channel_map = hdev->le_adv_channel_map;
+ cp.evt_properties = LE_LEGACY_ADV_DIRECT_IND;
+ cp.own_addr_type = own_addr_type;
+ cp.channel_map = hdev->le_adv_channel_map;
+ cp.tx_power = 127;
+ cp.primary_phy = LE_PHY_1M;
+ cp.secondary_phy = LE_PHY_1M;
+ cp.handle = 0; /* Use instance 0 for directed adv */
+ cp.own_addr_type = own_addr_type;
+ cp.peer_addr_type = conn->dst_type;
+ bacpy(&cp.peer_addr, &conn->dst);
+
+ hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
+
+ __hci_req_enable_ext_advertising(req, 0, false);
+ } else {
+ struct hci_cp_le_set_adv_param cp;
+
+ /* Clear the HCI_LE_ADV bit temporarily so that the
+ * hci_update_random_address knows that it's safe to go ahead
+ * and write a new random address. The flag will be set back on
+ * as soon as the SET_ADV_ENABLE HCI command completes.
+ */
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
+
+ /* Set require_privacy to false so that the remote device has a
+ * chance of identifying us.
+ */
+ if (hci_update_random_address(req, false, conn_use_rpa(conn),
+ &own_addr_type) < 0)
+ return;
- hci_req_add(req, HCI_OP_LE_SET_ADV_PARAM, sizeof(cp), &cp);
+ memset(&cp, 0, sizeof(cp));
+ cp.type = LE_ADV_DIRECT_IND;
+ cp.own_address_type = own_addr_type;
+ cp.direct_addr_type = conn->dst_type;
+ bacpy(&cp.direct_addr, &conn->dst);
+ cp.channel_map = hdev->le_adv_channel_map;
+
+ hci_req_add(req, HCI_OP_LE_SET_ADV_PARAM, sizeof(cp), &cp);
- enable = 0x01;
- hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
+ enable = 0x01;
+ hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable),
+ &enable);
+ }
conn->state = BT_CONNECT;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 6/9] Bluetooth: Use ext adv for directed adv
2017-12-04 8:07 ` [RFC 6/9] Bluetooth: Use ext adv for directed adv Jaganath Kanakkassery
@ 2017-12-08 8:56 ` Marcel Holtmann
0 siblings, 0 replies; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 8:56 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth, Jaganath Kanakkassery
Hi Jaganath,
> This patch does extended advertising for directed advertising
> if the controller supportes. Instance 0 is used for directed
> advertising.
>
> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
> ---
> net/bluetooth/hci_conn.c | 67 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 45 insertions(+), 22 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9459311..789a91a 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -827,35 +827,58 @@ static void hci_req_directed_advertising(struct hci_request *req,
> struct hci_conn *conn)
> {
> struct hci_dev *hdev = req->hdev;
> - struct hci_cp_le_set_adv_param cp;
> u8 own_addr_type;
> u8 enable;
>
> - /* Clear the HCI_LE_ADV bit temporarily so that the
> - * hci_update_random_address knows that it's safe to go ahead
> - * and write a new random address. The flag will be set back on
> - * as soon as the SET_ADV_ENABLE HCI command completes.
> - */
> - hci_dev_clear_flag(hdev, HCI_LE_ADV);
> + if (ext_adv_capable(hdev)) {
> + struct hci_cp_le_set_ext_adv_params cp;
>
> - /* Set require_privacy to false so that the remote device has a
> - * chance of identifying us.
> - */
> - if (hci_update_random_address(req, false, conn_use_rpa(conn),
> - &own_addr_type) < 0)
> - return;
> + memset(&cp, 0, sizeof(cp));
>
> - memset(&cp, 0, sizeof(cp));
> - cp.type = LE_ADV_DIRECT_IND;
> - cp.own_address_type = own_addr_type;
> - cp.direct_addr_type = conn->dst_type;
> - bacpy(&cp.direct_addr, &conn->dst);
> - cp.channel_map = hdev->le_adv_channel_map;
> + cp.evt_properties = LE_LEGACY_ADV_DIRECT_IND;
> + cp.own_addr_type = own_addr_type;
> + cp.channel_map = hdev->le_adv_channel_map;
> + cp.tx_power = 127;
> + cp.primary_phy = LE_PHY_1M;
> + cp.secondary_phy = LE_PHY_1M;
> + cp.handle = 0; /* Use instance 0 for directed adv */
> + cp.own_addr_type = own_addr_type;
> + cp.peer_addr_type = conn->dst_type;
> + bacpy(&cp.peer_addr, &conn->dst);
> +
> + hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
> +
> + __hci_req_enable_ext_advertising(req, 0, false);
I think this misses setting the random address for set 0 in case we have privacy enabled or are using static address.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC 7/9] Bluetooth: Implement Set ADV set random address
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (5 preceding siblings ...)
2017-12-04 8:07 ` [RFC 6/9] Bluetooth: Use ext adv for directed adv Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 8/9] Bluetooth: Handle incoming connection to an adv set Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 9/9] Bluetooth: Implement secondary advertising on different PHYs Jaganath Kanakkassery
8 siblings, 0 replies; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This basically sets the random address for an instance.
Random address can be set only if the instance is created which
is done in Set ext adv param.
This introduces a hci_get_random_address() which returns the
own address type and random address (rpa, nrpa or static) based
on the instance flags and hdev flags.
Also introduces random address and address type to adv instance
structure which can be used when a connection gets created
to a particular adv set.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 6 +++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 23 ++++++++
net/bluetooth/hci_event.c | 33 ++++++++++++
net/bluetooth/hci_request.c | 110 ++++++++++++++++++++++++++++++++++++++-
net/bluetooth/hci_request.h | 3 ++
6 files changed, 175 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 2ee16f7..1337fbf 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1618,6 +1618,12 @@ struct hci_cp_le_remove_adv_set {
#define HCI_OP_LE_CLEAR_ADV_SETS 0x203d
+#define HCI_OP_LE_SET_ADV_SET_RAND_ADDR 0x2035
+struct hci_cp_le_set_adv_set_rand_addr {
+ __u8 handle;
+ bdaddr_t bdaddr;
+} __packed;
+
/* ---- HCI Events ---- */
#define HCI_EV_INQUIRY_COMPLETE 0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 610172a..da7609b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -171,6 +171,7 @@ struct adv_info {
__u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH];
__u8 addr_type;
unsigned long state;
+ bdaddr_t random_addr;
};
/* Adv instance states */
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 789a91a..d7e6b0e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -832,6 +832,14 @@ static void hci_req_directed_advertising(struct hci_request *req,
if (ext_adv_capable(hdev)) {
struct hci_cp_le_set_ext_adv_params cp;
+ bdaddr_t random_addr;
+
+ /* Set require_privacy to false so that the remote device has a
+ * chance of identifying us.
+ */
+ if (hci_get_random_address(hdev, false, conn_use_rpa(conn),
+ &own_addr_type, &random_addr) < 0)
+ return;
memset(&cp, 0, sizeof(cp));
@@ -848,6 +856,21 @@ static void hci_req_directed_advertising(struct hci_request *req,
hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
+ if (own_addr_type == ADDR_LE_DEV_RANDOM &&
+ bacmp(&random_addr, BDADDR_ANY) &&
+ bacmp(&random_addr, &hdev->random_addr)) {
+ struct hci_cp_le_set_adv_set_rand_addr cp;
+
+ memset(&cp, 0, sizeof(cp));
+
+ cp.handle = 0;
+ bacpy(&cp.bdaddr, &random_addr);
+
+ hci_req_add(req,
+ HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
+ sizeof(cp), &cp);
+ }
+
__hci_req_enable_ext_advertising(req, 0, false);
} else {
struct hci_cp_le_set_adv_param cp;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8a118c1..67081ab 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1041,6 +1041,35 @@ static void hci_cc_le_set_random_addr(struct hci_dev *hdev, struct sk_buff *skb)
hci_dev_unlock(hdev);
}
+static void hci_cc_le_set_adv_set_random_addr(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ __u8 status = *((__u8 *) skb->data);
+ struct hci_cp_le_set_adv_set_rand_addr *cp;
+ struct adv_info *adv_instance;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+ if (status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_SET_RAND_ADDR);
+ if (!cp)
+ return;
+
+ hci_dev_lock(hdev);
+
+ if (!cp->handle) {
+ bacpy(&hdev->random_addr, &cp->bdaddr);
+ } else {
+ adv_instance = hci_find_adv_instance(hdev, cp->handle);
+ if (adv_instance)
+ bacpy(&adv_instance->random_addr, &cp->bdaddr);
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 *sent, status = *((__u8 *) skb->data);
@@ -3268,6 +3297,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_le_set_ext_adv_enable(hdev, skb);
break;
+ case HCI_OP_LE_SET_ADV_SET_RAND_ADDR:
+ hci_cc_le_set_adv_set_random_addr(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
break;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ca235eb..9d0a940 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1423,6 +1423,83 @@ unlock:
hci_dev_unlock(hdev);
}
+int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
+ bool use_rpa, u8 *own_addr_type, bdaddr_t *rand_addr)
+{
+ int err;
+
+ bacpy(rand_addr, BDADDR_ANY);
+
+ /* If privacy is enabled use a resolvable private address. If
+ * current RPA has expired then generate a new one.
+ */
+ if (use_rpa) {
+ *own_addr_type = ADDR_LE_DEV_RANDOM;
+
+ err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
+ if (err < 0) {
+ BT_ERR("%s failed to generate new RPA", hdev->name);
+ return err;
+ }
+
+ bacpy(rand_addr, &hdev->rpa);
+
+ return 0;
+ }
+
+ /* In case of required privacy without resolvable private address,
+ * use an non-resolvable private address. This is useful for active
+ * scanning and non-connectable advertising.
+ */
+ if (require_privacy) {
+ bdaddr_t nrpa;
+
+ while (true) {
+ /* The non-resolvable private address is generated
+ * from random six bytes with the two most significant
+ * bits cleared.
+ */
+ get_random_bytes(&nrpa, 6);
+ nrpa.b[5] &= 0x3f;
+
+ /* The non-resolvable private address shall not be
+ * equal to the public address.
+ */
+ if (bacmp(&hdev->bdaddr, &nrpa))
+ break;
+ }
+
+ *own_addr_type = ADDR_LE_DEV_RANDOM;
+ bacpy(rand_addr, &nrpa);
+
+ return 0;
+ }
+
+ /* If forcing static address is in use or there is no public
+ * address use the static address as random address
+ *
+ * In case BR/EDR has been disabled on a dual-mode controller
+ * and a static address has been configured, then use that
+ * address instead of the public BR/EDR address.
+ */
+ if (hci_dev_test_flag(hdev, HCI_FORCE_STATIC_ADDR) ||
+ !bacmp(&hdev->bdaddr, BDADDR_ANY) ||
+ (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED) &&
+ bacmp(&hdev->static_addr, BDADDR_ANY))) {
+ *own_addr_type = ADDR_LE_DEV_RANDOM;
+ bacpy(rand_addr, &hdev->static_addr);
+
+ return 0;
+ }
+
+ /* Neither privacy nor static address is being used so use a
+ * public address.
+ */
+ *own_addr_type = ADDR_LE_DEV_PUBLIC;
+
+ return 0;
+}
+
void __hci_req_remove_ext_adv_set(struct hci_request *req, u8 instance)
{
struct hci_cp_le_remove_adv_set cp;
@@ -1483,6 +1560,8 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
const u8 adv_interval[3] = { 0x00, 0x08, 0x00 };
struct adv_info *adv_inst;
bool secondary_adv;
+ bdaddr_t random_addr, *current_addr;
+ u8 own_addr_type;
flags = get_adv_instance_flags(hdev, instance);
@@ -1492,6 +1571,15 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
connectable = (flags & MGMT_ADV_FLAG_CONNECTABLE) ||
mgmt_get_connectable(hdev);
+ /* Set require_privacy to true only when non-connectable
+ * advertising is used. In that case it is fine to use a
+ * non-resolvable private address.
+ */
+ if (hci_get_random_address(hdev, !connectable,
+ adv_use_rpa(hdev, flags),
+ &own_addr_type, &random_addr) < 0)
+ return;
+
memset(&cp, 0, sizeof(cp));
memcpy(cp.min_interval, adv_interval, sizeof(cp.min_interval));
@@ -1521,7 +1609,7 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
cp.evt_properties = cpu_to_le16(LE_LEGACY_NONCONN_IND);
}
- cp.own_addr_type = BDADDR_LE_PUBLIC;
+ cp.own_addr_type = own_addr_type;
cp.channel_map = hdev->le_adv_channel_map;
cp.tx_power = 127;
cp.primary_phy = LE_PHY_1M;
@@ -1530,6 +1618,26 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
+ if (!instance)
+ current_addr = &hdev->random_addr;
+ else
+ current_addr = &adv_inst->random_addr;
+
+ if (own_addr_type == ADDR_LE_DEV_RANDOM &&
+ bacmp(&random_addr, BDADDR_ANY) &&
+ bacmp(&random_addr, current_addr)) {
+ struct hci_cp_le_set_adv_set_rand_addr cp;
+
+ memset(&cp, 0, sizeof(cp));
+
+ cp.handle = instance;
+ bacpy(&cp.bdaddr, &random_addr);
+
+ hci_req_add(req,
+ HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
+ sizeof(cp), &cp);
+ }
+
__hci_req_update_adv_data(req, instance);
__hci_req_update_scan_rsp_data(req, instance);
}
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 936f6c5..0e7c760 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -87,6 +87,9 @@ void __hci_req_enable_ext_advertising(struct hci_request *req, u8 instance,
void __hci_req_stop_ext_adv(struct hci_request *req, u8 instance,
bool all_instances);
void __hci_req_clear_ext_adv_sets(struct hci_request *req);
+int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
+ bool use_rpa, u8 *own_addr_type,
+ bdaddr_t *rand_addr);
void __hci_req_update_class(struct hci_request *req);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 8/9] Bluetooth: Handle incoming connection to an adv set
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (6 preceding siblings ...)
2017-12-04 8:07 ` [RFC 7/9] Bluetooth: Implement Set ADV set random address Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-04 8:07 ` [RFC 9/9] Bluetooth: Implement secondary advertising on different PHYs Jaganath Kanakkassery
8 siblings, 0 replies; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch basically set responder address to the hci conn from
the adv instance to which connection is established.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
net/bluetooth/hci_event.c | 47 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 67081ab..92b8308 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4737,8 +4737,11 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
/* All controllers implicitly stop advertising in the event of a
* connection, so ensure that the state bit is cleared.
+ * If ext adv is supported then it would be handled in adv terminated
+ * event.
*/
- hci_dev_clear_flag(hdev, HCI_LE_ADV);
+ if (!ext_adv_capable(hdev))
+ hci_dev_clear_flag(hdev, HCI_LE_ADV);
conn = hci_lookup_le_connect(hdev);
if (!conn) {
@@ -4775,14 +4778,17 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
}
if (!conn->out) {
- /* Set the responder (our side) address type based on
- * the advertising address type.
- */
- conn->resp_addr_type = hdev->adv_addr_type;
- if (hdev->adv_addr_type == ADDR_LE_DEV_RANDOM)
- bacpy(&conn->resp_addr, &hdev->random_addr);
- else
- bacpy(&conn->resp_addr, &hdev->bdaddr);
+ /* In ext adv, resp addr will be set in adv terminated event */
+ if (!ext_adv_capable(hdev)) {
+ /* Set the responder (our side) address type based on
+ * the advertising address type.
+ */
+ conn->resp_addr_type = hdev->adv_addr_type;
+ if (hdev->adv_addr_type == ADDR_LE_DEV_RANDOM)
+ bacpy(&conn->resp_addr, &hdev->random_addr);
+ else
+ bacpy(&conn->resp_addr, &hdev->bdaddr);
+ }
conn->init_addr_type = bdaddr_type;
bacpy(&conn->init_addr, bdaddr);
@@ -4929,10 +4935,33 @@ static void hci_le_adv_set_terminated_evt(struct hci_dev *hdev,
/* If this is because of connection */
if (!ev->status) {
+ struct hci_conn *conn;
+
if (!ev->handle)
hci_dev_clear_flag(hdev, HCI_LE_ADV);
else
clear_bit(ADV_INST_ENABLED, &adv_instance->state);
+
+ conn = hci_conn_hash_lookup_handle(hdev,
+ le16_to_cpu(ev->conn_handle));
+ if (conn) {
+ /* Set the responder (our side) address type based on
+ * the advertising address type.
+ */
+ if (!ev->handle) {
+ conn->resp_addr_type = hdev->adv_addr_type;
+ if (hdev->adv_addr_type == ADDR_LE_DEV_RANDOM)
+ bacpy(&conn->resp_addr, &hdev->random_addr);
+ else
+ bacpy(&conn->resp_addr, &hdev->bdaddr);
+ } else {
+ conn->resp_addr_type = adv_instance->addr_type;
+ if (adv_instance->addr_type == ADDR_LE_DEV_RANDOM)
+ bacpy(&conn->resp_addr, &adv_instance->random_addr);
+ else
+ bacpy(&conn->resp_addr, &hdev->bdaddr);
+ }
+ }
} else if (ev->handle) {
/* Remove the instance in all other cases */
err = hci_remove_adv_instance(hdev, ev->handle);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC 9/9] Bluetooth: Implement secondary advertising on different PHYs
2017-12-04 8:07 [RFC 0/9] Extended Adv Jaganath Kanakkassery
` (7 preceding siblings ...)
2017-12-04 8:07 ` [RFC 8/9] Bluetooth: Handle incoming connection to an adv set Jaganath Kanakkassery
@ 2017-12-04 8:07 ` Jaganath Kanakkassery
2017-12-08 9:00 ` Marcel Holtmann
8 siblings, 1 reply; 23+ messages in thread
From: Jaganath Kanakkassery @ 2017-12-04 8:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jaganath Kanakkassery
This patch adds support for advertising in primary and secondary
channel on different PHYs. User can add the phy preference in
the flag based on which phy type will be added in extended
advertising parameter would be set.
Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
---
include/net/bluetooth/hci.h | 6 +++++-
include/net/bluetooth/mgmt.h | 6 ++++++
net/bluetooth/hci_request.c | 18 +++++++++++++++---
net/bluetooth/mgmt.c | 18 ++++++++++++++++--
4 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 1337fbf..40a22e2 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -398,6 +398,8 @@ enum {
#define HCI_LE_SLAVE_FEATURES 0x08
#define HCI_LE_PING 0x10
#define HCI_LE_DATA_LEN_EXT 0x20
+#define HCI_LE_PHY_2M 0x01
+#define HCI_LE_PHY_CODED 0x08
#define HCI_LE_EXT_ADV 0x10
#define HCI_LE_EXT_SCAN_POLICY 0x80
@@ -1507,7 +1509,9 @@ struct hci_cp_le_set_ext_scan_params {
__u8 data[0];
} __packed;
-#define LE_PHY_1M 0x01
+#define LE_PHY_1M 0x01
+#define LE_PHY_2M 0x02
+#define LE_PHY_CODED 0x03
struct hci_cp_le_scan_phy_params {
__u8 type;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 72a456b..f1055dd 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -561,6 +561,12 @@ struct mgmt_rp_add_advertising {
#define MGMT_ADV_FLAG_TX_POWER BIT(4)
#define MGMT_ADV_FLAG_APPEARANCE BIT(5)
#define MGMT_ADV_FLAG_LOCAL_NAME BIT(6)
+#define MGMT_ADV_FLAG_SEC_1M BIT(7)
+#define MGMT_ADV_FLAG_SEC_2M BIT(8)
+#define MGMT_ADV_FLAG_SEC_CODED BIT(9)
+
+#define MGMT_ADV_FLAG_SEC_MASK (MGMT_ADV_FLAG_SEC_1M | MGMT_ADV_FLAG_SEC_2M | \
+ MGMT_ADV_FLAG_SEC_CODED)
#define MGMT_OP_REMOVE_ADVERTISING 0x003F
struct mgmt_cp_remove_advertising {
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 9d0a940..68c2f18 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1590,7 +1590,8 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
return;
secondary_adv = (adv_inst->adv_data_len > HCI_MAX_AD_LENGTH ||
- adv_inst->scan_rsp_len > HCI_MAX_AD_LENGTH);
+ adv_inst->scan_rsp_len > HCI_MAX_AD_LENGTH ||
+ flags & MGMT_ADV_FLAG_SEC_MASK);
if (connectable) {
if (secondary_adv)
@@ -1612,8 +1613,19 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
cp.own_addr_type = own_addr_type;
cp.channel_map = hdev->le_adv_channel_map;
cp.tx_power = 127;
- cp.primary_phy = LE_PHY_1M;
- cp.secondary_phy = LE_PHY_1M;
+
+ if (flags & MGMT_ADV_FLAG_SEC_2M){
+ cp.primary_phy = LE_PHY_1M;
+ cp.secondary_phy = LE_PHY_2M;
+ } else if (flags & MGMT_ADV_FLAG_SEC_CODED) {
+ cp.primary_phy = LE_PHY_CODED;
+ cp.secondary_phy = LE_PHY_CODED;
+ } else {
+ /* In all other cases use 1M */
+ cp.primary_phy = LE_PHY_1M;
+ cp.secondary_phy = LE_PHY_1M;
+ }
+
cp.handle = instance;
hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 120da46..68cf080 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5996,6 +5996,16 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
flags |= MGMT_ADV_FLAG_APPEARANCE;
flags |= MGMT_ADV_FLAG_LOCAL_NAME;
+ if (ext_adv_capable(hdev)) {
+ flags |= MGMT_ADV_FLAG_SEC_1M;
+
+ if (hdev->le_features[1] & HCI_LE_PHY_2M)
+ flags |= MGMT_ADV_FLAG_SEC_2M;
+
+ if (hdev->le_features[1] & HCI_LE_PHY_CODED)
+ flags |= MGMT_ADV_FLAG_SEC_CODED;
+ }
+
if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
flags |= MGMT_ADV_FLAG_TX_POWER;
@@ -6246,10 +6256,14 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
duration = __le16_to_cpu(cp->duration);
/* The current implementation only supports a subset of the specified
- * flags.
+ * flags. Also need to check mutual exclusiveness of sec flags.
*/
supported_flags = get_supported_adv_flags(hdev);
- if (flags & ~supported_flags)
+ if (flags & ~supported_flags ||
+ ((flags & MGMT_ADV_FLAG_SEC_MASK) != 0 &&
+ (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_1M &&
+ (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_2M &&
+ (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_CODED))
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
MGMT_STATUS_INVALID_PARAMS);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC 9/9] Bluetooth: Implement secondary advertising on different PHYs
2017-12-04 8:07 ` [RFC 9/9] Bluetooth: Implement secondary advertising on different PHYs Jaganath Kanakkassery
@ 2017-12-08 9:00 ` Marcel Holtmann
0 siblings, 0 replies; 23+ messages in thread
From: Marcel Holtmann @ 2017-12-08 9:00 UTC (permalink / raw)
To: Jaganath Kanakkassery; +Cc: linux-bluetooth, Jaganath Kanakkassery
Hi Jaganath,
> This patch adds support for advertising in primary and secondary
> channel on different PHYs. User can add the phy preference in
> the flag based on which phy type will be added in extended
> advertising parameter would be set.
>
> Signed-off-by: Jaganath Kanakkassery <jaganathx.kanakkassery@intel.com>
> ---
> include/net/bluetooth/hci.h | 6 +++++-
> include/net/bluetooth/mgmt.h | 6 ++++++
> net/bluetooth/hci_request.c | 18 +++++++++++++++---
> net/bluetooth/mgmt.c | 18 ++++++++++++++++--
> 4 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 1337fbf..40a22e2 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -398,6 +398,8 @@ enum {
> #define HCI_LE_SLAVE_FEATURES 0x08
> #define HCI_LE_PING 0x10
> #define HCI_LE_DATA_LEN_EXT 0x20
> +#define HCI_LE_PHY_2M 0x01
> +#define HCI_LE_PHY_CODED 0x08
> #define HCI_LE_EXT_ADV 0x10
> #define HCI_LE_EXT_SCAN_POLICY 0x80
>
> @@ -1507,7 +1509,9 @@ struct hci_cp_le_set_ext_scan_params {
> __u8 data[0];
> } __packed;
>
> -#define LE_PHY_1M 0x01
> +#define LE_PHY_1M 0x01
> +#define LE_PHY_2M 0x02
> +#define LE_PHY_CODED 0x03
>
> struct hci_cp_le_scan_phy_params {
> __u8 type;
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 72a456b..f1055dd 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -561,6 +561,12 @@ struct mgmt_rp_add_advertising {
> #define MGMT_ADV_FLAG_TX_POWER BIT(4)
> #define MGMT_ADV_FLAG_APPEARANCE BIT(5)
> #define MGMT_ADV_FLAG_LOCAL_NAME BIT(6)
> +#define MGMT_ADV_FLAG_SEC_1M BIT(7)
> +#define MGMT_ADV_FLAG_SEC_2M BIT(8)
> +#define MGMT_ADV_FLAG_SEC_CODED BIT(9)
> +
> +#define MGMT_ADV_FLAG_SEC_MASK (MGMT_ADV_FLAG_SEC_1M | MGMT_ADV_FLAG_SEC_2M | \
> + MGMT_ADV_FLAG_SEC_CODED)
>
> #define MGMT_OP_REMOVE_ADVERTISING 0x003F
> struct mgmt_cp_remove_advertising {
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 9d0a940..68c2f18 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1590,7 +1590,8 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
> return;
>
> secondary_adv = (adv_inst->adv_data_len > HCI_MAX_AD_LENGTH ||
> - adv_inst->scan_rsp_len > HCI_MAX_AD_LENGTH);
> + adv_inst->scan_rsp_len > HCI_MAX_AD_LENGTH ||
> + flags & MGMT_ADV_FLAG_SEC_MASK);
>
> if (connectable) {
> if (secondary_adv)
> @@ -1612,8 +1613,19 @@ static void __hci_req_setup_ext_adv_instance(struct hci_request *req,
> cp.own_addr_type = own_addr_type;
> cp.channel_map = hdev->le_adv_channel_map;
> cp.tx_power = 127;
> - cp.primary_phy = LE_PHY_1M;
> - cp.secondary_phy = LE_PHY_1M;
> +
> + if (flags & MGMT_ADV_FLAG_SEC_2M){
> + cp.primary_phy = LE_PHY_1M;
> + cp.secondary_phy = LE_PHY_2M;
> + } else if (flags & MGMT_ADV_FLAG_SEC_CODED) {
> + cp.primary_phy = LE_PHY_CODED;
> + cp.secondary_phy = LE_PHY_CODED;
> + } else {
> + /* In all other cases use 1M */
> + cp.primary_phy = LE_PHY_1M;
> + cp.secondary_phy = LE_PHY_1M;
> + }
> +
> cp.handle = instance;
>
> hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(cp), &cp);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 120da46..68cf080 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5996,6 +5996,16 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
> flags |= MGMT_ADV_FLAG_APPEARANCE;
> flags |= MGMT_ADV_FLAG_LOCAL_NAME;
>
> + if (ext_adv_capable(hdev)) {
> + flags |= MGMT_ADV_FLAG_SEC_1M;
> +
> + if (hdev->le_features[1] & HCI_LE_PHY_2M)
> + flags |= MGMT_ADV_FLAG_SEC_2M;
> +
> + if (hdev->le_features[1] & HCI_LE_PHY_CODED)
> + flags |= MGMT_ADV_FLAG_SEC_CODED;
> + }
> +
> if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
> flags |= MGMT_ADV_FLAG_TX_POWER;
>
> @@ -6246,10 +6256,14 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> duration = __le16_to_cpu(cp->duration);
>
> /* The current implementation only supports a subset of the specified
> - * flags.
> + * flags. Also need to check mutual exclusiveness of sec flags.
> */
> supported_flags = get_supported_adv_flags(hdev);
> - if (flags & ~supported_flags)
> + if (flags & ~supported_flags ||
> + ((flags & MGMT_ADV_FLAG_SEC_MASK) != 0 &&
> + (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_1M &&
> + (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_2M &&
> + (flags & MGMT_ADV_FLAG_SEC_MASK) != MGMT_ADV_FLAG_SEC_CODED))
define a mask value for this and do some bit trickery. I think there are even CPU macros for bit counting in the worst case. However a simple XOR and then AND with the mask value checked against 0 should do the trick.
Regards
Marcel
^ permalink raw reply [flat|nested] 23+ messages in thread