[PATCH v2 0/7] Introduce APIs to set advertising data

[PATCH v2 0/7] Introduce APIs to set advertising data

[Arman Uguray]

*v2: - Addressed Marcel's comments on v1.

*v1: - Addressed Marcel's comments.
     - Included support for setting scan response data and timeouts.

Arman Uguray (7):
  Bluetooth: Add definitions for Add/Remove Advertising API
  Bluetooth: Introduce HCI_ADVERTISING_INSTANCE setting and add AD flags
  Bluetooth: Add data structure for advertising instance
  Bluetooth: Implement the Add Advertising command
  Bluetooth: Implement the Remove Advertising command
  Bluetooth: Add support for instance scan response
  Bluetooth: Add support for adv instance timeout

 include/net/bluetooth/hci.h      |   2 +
 include/net/bluetooth/hci_core.h |  18 ++
 include/net/bluetooth/mgmt.h     |  34 +++
 net/bluetooth/hci_core.c         |   1 +
 net/bluetooth/mgmt.c             | 491 +++++++++++++++++++++++++++++++++++++--
 5 files changed, 533 insertions(+), 13 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 1/7] Bluetooth: Add definitions for Add/Remove Advertising API

[Arman Uguray]

This patch adds definitions for the Add Advertising and Remove
Advertising MGMT commands and events.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 include/net/bluetooth/mgmt.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index a1a6867..68abd4b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -539,6 +539,30 @@ struct mgmt_rp_read_adv_features {
 	__u8   instance[0];
 } __packed;
 
+#define MGMT_OP_ADD_ADVERTISING		0x003E
+struct mgmt_cp_add_advertising {
+	__u8	instance;
+	__le32	flags;
+	__le16	duration;
+	__le16	timeout;
+	__u8	adv_data_len;
+	__u8	scan_rsp_len;
+	__u8	data[0];
+} __packed;
+#define MGMT_ADD_ADVERTISING_SIZE	11
+struct mgmt_rp_add_advertising {
+	__u8	instance;
+} __packed;
+
+#define MGMT_OP_REMOVE_ADVERTISING	0x003F
+struct mgmt_cp_remove_advertising {
+	__u8	instance;
+} __packed;
+#define MGMT_REMOVE_ADVERTISING_SIZE	1
+struct mgmt_rp_remove_advertising {
+	__u8	instance;
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	__le16	opcode;
@@ -742,3 +766,13 @@ struct mgmt_ev_local_oob_data_updated {
 	__le16	eir_len;
 	__u8	eir[0];
 } __packed;
+
+#define MGMT_EV_ADVERTISING_ADDED	0x0023
+struct mgmt_ev_advertising_added {
+	__u8    instance;
+} __packed;
+
+#define MGMT_EV_ADVERTISING_REMOVED	0x0024
+struct mgmt_ev_advertising_removed {
+	__u8    instance;
+} __packed;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 2/7] Bluetooth: Introduce HCI_ADVERTISING_INSTANCE setting and add AD flags

[Arman Uguray]

This patch introduces the HCI_ADVERTISING_INSTANCE setting, which is set
when an at least one advertising instance has been added using the
"Add Advertising" mgmt command. This patch also adds a macro definition
for the EIR_APPEARANCE field type.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 include/net/bluetooth/hci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 06e7eee..3acecf3 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -227,6 +227,7 @@ enum {
 	HCI_LE_ENABLED,
 	HCI_ADVERTISING,
 	HCI_ADVERTISING_CONNECTABLE,
+	HCI_ADVERTISING_INSTANCE,
 	HCI_CONNECTABLE,
 	HCI_DISCOVERABLE,
 	HCI_LIMITED_DISCOVERABLE,
@@ -465,6 +466,7 @@ enum {
 #define EIR_SSP_HASH_C		0x0E /* Simple Pairing Hash C */
 #define EIR_SSP_RAND_R		0x0F /* Simple Pairing Randomizer R */
 #define EIR_DEVICE_ID		0x10 /* device ID */
+#define EIR_APPEARANCE		0x19 /* Device appearance */
 #define EIR_LE_BDADDR		0x1B /* LE Bluetooth device address */
 #define EIR_LE_ROLE		0x1C /* LE role */
 #define EIR_LE_SC_CONFIRM	0x22 /* LE SC Confirmation Value */
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 3/7] Bluetooth: Add data structure for advertising instance

[Arman Uguray]

This patch introduces a new data structure to represent advertising
instances that were added using the "Add Advertising" mgmt command.
Initially an hci_dev structure will support only one of these instances
at a time, so the current instance is simply stored as a direct member
of hci_dev.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 include/net/bluetooth/hci_core.h | 16 ++++++++++++++++
 net/bluetooth/hci_core.c         |  1 +
 2 files changed, 17 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b65c53d..2d853d8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -155,6 +155,15 @@ struct oob_data {
 	u8 rand256[16];
 };
 
+struct adv_info {
+	__u8 instance;
+	__u32 flags;
+	__u16 adv_data_len;
+	__u8 adv_data[HCI_MAX_AD_LENGTH];
+	__u16 scan_rsp_len;
+	__u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
+};
+
 #define HCI_MAX_SHORT_NAME_LENGTH	10
 
 /* Default LE RPA expiry time, 15 minutes */
@@ -364,6 +373,8 @@ struct hci_dev {
 	__u8			scan_rsp_data[HCI_MAX_AD_LENGTH];
 	__u8			scan_rsp_data_len;
 
+	struct adv_info		adv_instance;
+
 	__u8			irk[16];
 	__u32			rpa_timeout;
 	struct delayed_work	rpa_expired;
@@ -550,6 +561,11 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
 	hdev->discovery.scan_duration = 0;
 }
 
+static inline void adv_info_init(struct hci_dev *hdev)
+{
+	memset(&hdev->adv_instance, 0, sizeof(struct adv_info));
+}
+
 bool hci_discovery_active(struct hci_dev *hdev);
 
 void hci_discovery_set_state(struct hci_dev *hdev, int state);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 773f216..db3dd0c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3125,6 +3125,7 @@ struct hci_dev *hci_alloc_dev(void)
 
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
+	adv_info_init(hdev);
 
 	return hdev;
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 4/7] Bluetooth: Implement the Add Advertising command

[Arman Uguray]

This patch adds the most basic implementation for the
"Add Advertisement" command. All state updates between the
various HCI settings (POWERED, ADVERTISING, ADVERTISING_INSTANCE,
and LE_ENABLED) has been implemented. The command currently
supports only setting the advertising data fields, with no flags
and no scan response data.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 net/bluetooth/mgmt.c | 283 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 273 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 8c771e7..ea18675 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -100,6 +100,7 @@ static const u16 mgmt_commands[] = {
 	MGMT_OP_READ_LOCAL_OOB_EXT_DATA,
 	MGMT_OP_READ_EXT_INDEX_LIST,
 	MGMT_OP_READ_ADV_FEATURES,
+	MGMT_OP_ADD_ADVERTISING,
 };
 
 static const u16 mgmt_events[] = {
@@ -135,6 +136,8 @@ static const u16 mgmt_events[] = {
 	MGMT_EV_EXT_INDEX_ADDED,
 	MGMT_EV_EXT_INDEX_REMOVED,
 	MGMT_EV_LOCAL_OOB_DATA_UPDATED,
+	MGMT_EV_ADVERTISING_ADDED,
+	MGMT_EV_ADVERTISING_REMOVED,
 };
 
 #define CACHE_TIMEOUT	msecs_to_jiffies(2 * 1000)
@@ -864,7 +867,7 @@ static u8 get_adv_discov_flags(struct hci_dev *hdev)
 	return 0;
 }
 
-static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
+static u8 create_default_adv_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0, flags = 0;
 
@@ -896,7 +899,18 @@ static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
 	return ad_len;
 }
 
-static void update_adv_data(struct hci_request *req)
+static u8 create_instance_adv_data(struct hci_dev *hdev, u8 *ptr)
+{
+	/* TODO: Set the appropriate entries based on advertising instance flags
+	 * here once flags other than 0 are supported.
+	 */
+	memcpy(ptr, hdev->adv_instance.adv_data,
+	       hdev->adv_instance.adv_data_len);
+
+	return hdev->adv_instance.adv_data_len;
+}
+
+static void update_adv_data_for_instance(struct hci_request *req, u8 instance)
 {
 	struct hci_dev *hdev = req->hdev;
 	struct hci_cp_le_set_adv_data cp;
@@ -907,8 +921,12 @@ static void update_adv_data(struct hci_request *req)
 
 	memset(&cp, 0, sizeof(cp));
 
-	len = create_adv_data(hdev, cp.data);
+	if (instance)
+		len = create_instance_adv_data(hdev, cp.data);
+	else
+		len = create_default_adv_data(hdev, 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;
@@ -921,6 +939,25 @@ static void update_adv_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
 }
 
+static void update_adv_data(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	u8 instance;
+
+	/* The "Set Advertising" setting supercedes the "Add Advertising"
+	 * setting. Here we set the advertising data based on which
+	 * setting was set. When neither apply, default to the global settings,
+	 * represented by instance "0".
+	 */
+	if (hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE) &&
+	    !hci_dev_test_flag(hdev, HCI_ADVERTISING))
+		instance = 1;
+	else
+		instance = 0;
+
+	update_adv_data_for_instance(req, instance);
+}
+
 int mgmt_update_adv_data(struct hci_dev *hdev)
 {
 	struct hci_request req;
@@ -4374,10 +4411,17 @@ static int set_device_id(struct sock *sk, struct hci_dev *hdev, void *data,
 	return err;
 }
 
+static void enable_advertising_instance(struct hci_dev *hdev, u8 status,
+					u16 opcode)
+{
+	BT_DBG("status %d", status);
+}
+
 static void set_advertising_complete(struct hci_dev *hdev, u8 status,
 				     u16 opcode)
 {
 	struct cmd_lookup match = { NULL, hdev };
+	struct hci_request req;
 
 	hci_dev_lock(hdev);
 
@@ -4402,6 +4446,21 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status,
 	if (match.sk)
 		sock_put(match.sk);
 
+	/* If "Set Advertising" was just disabled and instance advertising was
+	 * set up earlier, then enable the advertising instance.
+	 */
+	if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
+	    !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
+		goto unlock;
+
+	hci_req_init(&req, hdev);
+
+	update_adv_data(&req);
+	enable_advertising(&req);
+
+	if (hci_req_run(&req, enable_advertising_instance) < 0)
+		BT_ERR("Failed to re-configure advertising");
+
 unlock:
 	hci_dev_unlock(hdev);
 }
@@ -4484,10 +4543,13 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
 	else
 		hci_dev_clear_flag(hdev, HCI_ADVERTISING_CONNECTABLE);
 
-	if (val)
+	if (val) {
+		/* Switch to instance "0" for the Set Advertising setting. */
+		update_adv_data_for_instance(&req, 0);
 		enable_advertising(&req);
-	else
+	} else {
 		disable_advertising(&req);
+	}
 
 	err = hci_req_run(&req, set_advertising_complete);
 	if (err < 0)
@@ -6299,12 +6361,21 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
 	struct mgmt_rp_read_adv_features *rp;
 	size_t rp_len;
 	int err;
+	bool instance;
 
 	BT_DBG("%s", hdev->name);
 
 	hci_dev_lock(hdev);
 
 	rp_len = sizeof(*rp);
+
+	/* Currently only one instance is supported, so just add 1 to the
+	 * response length.
+	 */
+	instance = hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE);
+	if (instance)
+		rp_len++;
+
 	rp = kmalloc(rp_len, GFP_ATOMIC);
 	if (!rp) {
 		hci_dev_unlock(hdev);
@@ -6314,8 +6385,17 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
 	rp->supported_flags = cpu_to_le32(0);
 	rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
 	rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
-	rp->max_instances = 0;
-	rp->num_instances = 0;
+	rp->max_instances = 1;
+
+	/* Currently only one instance is supported, so simply return the
+	 * current instance number.
+	 */
+	if (instance) {
+		rp->num_instances = 1;
+		rp->instance[0] = 1;
+	} else {
+		rp->num_instances = 0;
+	}
 
 	hci_dev_unlock(hdev);
 
@@ -6327,6 +6407,179 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
+static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
+			      u8 adv_data_len)
+{
+	u8 max_adv_len = HCI_MAX_AD_LENGTH;
+	int i, cur_len;
+
+	/* TODO: Correctly reduce adv_len based on adv_flags. */
+
+	if (adv_data_len > max_adv_len)
+		return false;
+
+	/* Make sure that adv_data is correctly formatted. */
+	for (i = 0, cur_len = 0; i < adv_data_len; i += (cur_len + 1)) {
+		cur_len = adv_data[i];
+
+		/* If the current field length would exceed the total data
+		 * length, then it's invalid.
+		 */
+		if (i + cur_len >= adv_data_len)
+			return false;
+	}
+
+	return true;
+}
+
+static void advertising_added(struct sock *sk, struct hci_dev *hdev,
+			      u8 instance)
+{
+	struct mgmt_ev_advertising_added ev;
+
+	ev.instance = instance;
+
+	mgmt_event(MGMT_EV_ADVERTISING_ADDED, hdev, &ev, sizeof(ev), sk);
+}
+
+static void advertising_removed(struct sock *sk, struct hci_dev *hdev,
+				u8 instance)
+{
+	struct mgmt_ev_advertising_removed ev;
+
+	ev.instance = instance;
+
+	mgmt_event(MGMT_EV_ADVERTISING_REMOVED, hdev, &ev, sizeof(ev), sk);
+}
+
+static void add_advertising_complete(struct hci_dev *hdev, u8 status,
+				     u16 opcode)
+{
+	struct mgmt_pending_cmd *cmd;
+	struct mgmt_rp_add_advertising rp;
+
+	BT_DBG("status %d", status);
+
+	hci_dev_lock(hdev);
+
+	cmd = pending_find(MGMT_OP_ADD_ADVERTISING, hdev);
+
+	if (status) {
+		hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
+		memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
+		advertising_removed(cmd ? cmd->sk : NULL, hdev, 1);
+	}
+
+	if (!cmd)
+		goto unlock;
+
+	rp.instance = 0x01;
+
+	if (status)
+		mgmt_cmd_status(cmd->sk, cmd->index, cmd->opcode,
+				mgmt_status(status));
+	else
+		mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
+				  mgmt_status(status), &rp, sizeof(rp));
+
+	mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
+}
+
+static int add_advertising(struct sock *sk, struct hci_dev *hdev,
+			   void *data, u16 data_len)
+{
+	struct mgmt_cp_add_advertising *cp = data;
+	struct mgmt_rp_add_advertising rp;
+	u32 flags;
+	u8 status;
+	int err;
+	struct mgmt_pending_cmd *cmd;
+	struct hci_request req;
+
+	BT_DBG("%s", hdev->name);
+
+	status = mgmt_le_support(hdev);
+	if (status)
+		return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+				       status);
+
+	flags = __le32_to_cpu(cp->flags);
+
+	/* The current implementation only supports adding one instance and
+	 * doesn't support flags.
+	 */
+	if (cp->instance != 0x01 || flags)
+		return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+				       MGMT_STATUS_INVALID_PARAMS);
+
+	hci_dev_lock(hdev);
+
+	if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+	    pending_find(MGMT_OP_SET_LE, hdev)) {
+		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+				      MGMT_STATUS_BUSY);
+		goto unlock;
+	}
+
+	if (!adv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len)) {
+		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+				      MGMT_STATUS_INVALID_PARAMS);
+		goto unlock;
+	}
+
+	hdev->adv_instance.flags = flags;
+	hdev->adv_instance.adv_data_len = cp->adv_data_len;
+	hdev->adv_instance.scan_rsp_len = cp->scan_rsp_len;
+
+	if (cp->adv_data_len)
+		memcpy(hdev->adv_instance.adv_data, cp->data, cp->adv_data_len);
+
+	if (cp->scan_rsp_len)
+		memcpy(hdev->adv_instance.scan_rsp_data,
+		       cp->data + cp->adv_data_len, cp->scan_rsp_len);
+
+	if (!hci_dev_test_and_set_flag(hdev, HCI_ADVERTISING_INSTANCE))
+		advertising_added(sk, hdev, 1);
+
+	/* If the HCI_ADVERTISING flag is set or the device isn't powered then
+	 * we have no HCI communication to make. Simply return.
+	 */
+	if (!hdev_is_powered(hdev) ||
+	    hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
+		rp.instance = 0x01;
+		err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+					MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+		goto unlock;
+	}
+
+	/* We're good to go, update advertising data, parameters, and start
+	 * advertising.
+	 */
+	cmd = mgmt_pending_add(sk, MGMT_OP_ADD_ADVERTISING, hdev, data,
+			       data_len);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	hci_req_init(&req, hdev);
+
+	update_adv_data(&req);
+	enable_advertising(&req);
+
+	err = hci_req_run(&req, add_advertising_complete);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
+
+	return err;
+}
+
 static const struct hci_mgmt_handler mgmt_handlers[] = {
 	{ NULL }, /* 0x0000 (no command) */
 	{ read_version,            MGMT_READ_VERSION_SIZE,
@@ -6411,6 +6664,8 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
 						HCI_MGMT_NO_HDEV |
 						HCI_MGMT_UNTRUSTED },
 	{ read_adv_features,       MGMT_READ_ADV_FEATURES_SIZE },
+	{ add_advertising,	   MGMT_ADD_ADVERTISING_SIZE,
+						HCI_MGMT_VAR_LEN },
 };
 
 void mgmt_index_added(struct hci_dev *hdev)
@@ -6582,7 +6837,8 @@ static int powered_update_hci(struct hci_dev *hdev)
 			update_scan_rsp_data(&req);
 		}
 
-		if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+		if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
+		    hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
 			enable_advertising(&req);
 
 		restart_le_actions(&req);
@@ -6694,7 +6950,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 			    sizeof(scan), &scan);
 	}
 	update_class(&req);
-	update_adv_data(&req);
+
+	/* Advertising instances don't use the global discoverable setting, so
+	 * only update AD if advertising was enabled using Set Advertising.
+	 */
+	if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+		update_adv_data(&req);
+
 	hci_req_run(&req, NULL);
 
 	hdev->discov_timeout = 0;
@@ -7595,7 +7857,8 @@ void mgmt_reenable_advertising(struct hci_dev *hdev)
 {
 	struct hci_request req;
 
-	if (!hci_dev_test_flag(hdev, HCI_ADVERTISING))
+	if (!hci_dev_test_flag(hdev, HCI_ADVERTISING) &&
+	    !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
 		return;
 
 	hci_req_init(&req, hdev);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 5/7] Bluetooth: Implement the Remove Advertising command

[Arman Uguray]

This patch implements the "Remove Advertising" mgmt command.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 net/bluetooth/mgmt.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ea18675..3991380 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -101,6 +101,7 @@ static const u16 mgmt_commands[] = {
 	MGMT_OP_READ_EXT_INDEX_LIST,
 	MGMT_OP_READ_ADV_FEATURES,
 	MGMT_OP_ADD_ADVERTISING,
+	MGMT_OP_REMOVE_ADVERTISING,
 };
 
 static const u16 mgmt_events[] = {
@@ -6518,6 +6519,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 	hci_dev_lock(hdev);
 
 	if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+	    pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev) ||
 	    pending_find(MGMT_OP_SET_LE, hdev)) {
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
 				      MGMT_STATUS_BUSY);
@@ -6580,6 +6582,106 @@ unlock:
 	return err;
 }
 
+static void remove_advertising_complete(struct hci_dev *hdev, u8 status,
+					u16 opcode)
+{
+	struct mgmt_pending_cmd *cmd;
+	struct mgmt_rp_remove_advertising rp;
+
+	BT_DBG("status %d", status);
+
+	hci_dev_lock(hdev);
+
+	/* A failure status here only means that we failed to disable
+	 * advertising. Otherwise, the advertising instance has been removed,
+	 * so report success.
+	 */
+	cmd = pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev);
+	if (!cmd)
+		goto unlock;
+
+	rp.instance = 1;
+
+	mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode, MGMT_STATUS_SUCCESS,
+			  &rp, sizeof(rp));
+	mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
+}
+
+static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
+			      void *data, u16 data_len)
+{
+	struct mgmt_cp_remove_advertising *cp = data;
+	struct mgmt_rp_remove_advertising rp;
+	int err;
+	struct mgmt_pending_cmd *cmd;
+	struct hci_request req;
+
+	BT_DBG("%s", hdev->name);
+
+	/* The current implementation only allows modifying instance no 1. A
+	 * value of 0 indicates that all instances should be cleared.
+	 */
+	if (cp->instance > 1)
+		return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+				       MGMT_STATUS_INVALID_PARAMS);
+
+	hci_dev_lock(hdev);
+
+	if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+	    pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev) ||
+	    pending_find(MGMT_OP_SET_LE, hdev)) {
+		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+				      MGMT_STATUS_BUSY);
+		goto unlock;
+	}
+
+	if (!hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE)) {
+		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+				      MGMT_STATUS_INVALID_PARAMS);
+		goto unlock;
+	}
+
+	memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
+
+	advertising_removed(sk, hdev, 1);
+
+	hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
+
+	/* If the HCI_ADVERTISING flag is set or the device isn't powered then
+	 * we have no HCI communication to make. Simply return.
+	 */
+	if (!hdev_is_powered(hdev) ||
+	    hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
+		rp.instance = 1;
+		err = mgmt_cmd_complete(sk, hdev->id,
+					MGMT_OP_REMOVE_ADVERTISING,
+					MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+		goto unlock;
+	}
+
+	cmd = mgmt_pending_add(sk, MGMT_OP_REMOVE_ADVERTISING, hdev, data,
+			       data_len);
+	if (!cmd) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	hci_req_init(&req, hdev);
+	disable_advertising(&req);
+
+	err = hci_req_run(&req, remove_advertising_complete);
+	if (err < 0)
+		mgmt_pending_remove(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
+
+	return err;
+}
+
 static const struct hci_mgmt_handler mgmt_handlers[] = {
 	{ NULL }, /* 0x0000 (no command) */
 	{ read_version,            MGMT_READ_VERSION_SIZE,
@@ -6666,6 +6768,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
 	{ read_adv_features,       MGMT_READ_ADV_FEATURES_SIZE },
 	{ add_advertising,	   MGMT_ADD_ADVERTISING_SIZE,
 						HCI_MGMT_VAR_LEN },
+	{ remove_advertising,	   MGMT_REMOVE_ADVERTISING_SIZE },
 };
 
 void mgmt_index_added(struct hci_dev *hdev)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 6/7] Bluetooth: Add support for instance scan response

[Arman Uguray]

This patch implements setting the Scan Response data provided as part
of an advertising instance through the Add Advertising command.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3991380..280346f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -793,7 +793,7 @@ static struct mgmt_pending_cmd *pending_find_data(u16 opcode,
 	return mgmt_pending_find_data(HCI_CHANNEL_CONTROL, opcode, hdev, data);
 }
 
-static u8 create_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
+static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0;
 	size_t name_len;
@@ -819,7 +819,19 @@ static u8 create_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
 	return ad_len;
 }
 
-static void update_scan_rsp_data(struct hci_request *req)
+static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
+{
+	/* TODO: Set the appropriate entries based on advertising instance flags
+	 * here once flags other than 0 are supported.
+	 */
+	memcpy(ptr, hdev->adv_instance.scan_rsp_data,
+	       hdev->adv_instance.scan_rsp_len);
+
+	return hdev->adv_instance.scan_rsp_len;
+}
+
+static void update_scan_rsp_data_for_instance(struct hci_request *req,
+					      u8 instance)
 {
 	struct hci_dev *hdev = req->hdev;
 	struct hci_cp_le_set_scan_rsp_data cp;
@@ -830,7 +842,10 @@ static void update_scan_rsp_data(struct hci_request *req)
 
 	memset(&cp, 0, sizeof(cp));
 
-	len = create_scan_rsp_data(hdev, cp.data);
+	if (instance)
+		len = create_instance_scan_rsp_data(hdev, 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) == 0)
@@ -844,6 +859,25 @@ static void update_scan_rsp_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
 }
 
+static void update_scan_rsp_data(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	u8 instance;
+
+	/* The "Set Advertising" setting supersedes the "Add Advertising"
+	 * setting. Here we set the scan response data based on which
+	 * setting was set. When neither apply, default to the global settings,
+	 * represented by instance "0".
+	 */
+	if (hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE) &&
+	    !hci_dev_test_flag(hdev, HCI_ADVERTISING))
+		instance = 1;
+	else
+		instance = 0;
+
+	update_scan_rsp_data_for_instance(req, instance);
+}
+
 static u8 get_adv_discov_flags(struct hci_dev *hdev)
 {
 	struct mgmt_pending_cmd *cmd;
@@ -945,7 +979,7 @@ static void update_adv_data(struct hci_request *req)
 	struct hci_dev *hdev = req->hdev;
 	u8 instance;
 
-	/* The "Set Advertising" setting supercedes the "Add Advertising"
+	/* The "Set Advertising" setting supersedes the "Add Advertising"
 	 * setting. Here we set the advertising data based on which
 	 * setting was set. When neither apply, default to the global settings,
 	 * represented by instance "0".
@@ -4547,6 +4581,7 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
 	if (val) {
 		/* Switch to instance "0" for the Set Advertising setting. */
 		update_adv_data_for_instance(&req, 0);
+		update_scan_rsp_data_for_instance(&req, 0);
 		enable_advertising(&req);
 	} else {
 		disable_advertising(&req);
@@ -6408,25 +6443,25 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
-static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
-			      u8 adv_data_len)
+static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
+			      u8 len)
 {
-	u8 max_adv_len = HCI_MAX_AD_LENGTH;
+	u8 max_len = HCI_MAX_AD_LENGTH;
 	int i, cur_len;
 
-	/* TODO: Correctly reduce adv_len based on adv_flags. */
+	/* TODO: Correctly reduce len based on adv_flags. */
 
-	if (adv_data_len > max_adv_len)
+	if (len > max_len)
 		return false;
 
-	/* Make sure that adv_data is correctly formatted. */
-	for (i = 0, cur_len = 0; i < adv_data_len; i += (cur_len + 1)) {
-		cur_len = adv_data[i];
+	/* Make sure that the data is correctly formatted. */
+	for (i = 0, cur_len = 0; i < len; i += (cur_len + 1)) {
+		cur_len = data[i];
 
 		/* If the current field length would exceed the total data
 		 * length, then it's invalid.
 		 */
-		if (i + cur_len >= adv_data_len)
+		if (i + cur_len >= len)
 			return false;
 	}
 
@@ -6526,7 +6561,9 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 		goto unlock;
 	}
 
-	if (!adv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len)) {
+	if (!tlv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len) ||
+	    !tlv_data_is_valid(hdev, flags, cp->data + cp->adv_data_len,
+			       cp->scan_rsp_len)) {
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
 				      MGMT_STATUS_INVALID_PARAMS);
 		goto unlock;
@@ -6570,6 +6607,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 	hci_req_init(&req, hdev);
 
 	update_adv_data(&req);
+	update_scan_rsp_data(&req);
 	enable_advertising(&req);
 
 	err = hci_req_run(&req, add_advertising_complete);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH v2 7/7] Bluetooth: Add support for adv instance timeout

[Arman Uguray]

This patch implements support for the timeout parameter of the
Add Advertising command.

Signed-off-by: Arman Uguray <armansito@chromium.org>
---
 include/net/bluetooth/hci_core.h |   2 +
 net/bluetooth/mgmt.c             | 101 +++++++++++++++++++++++++++++++--------
 2 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 2d853d8..f97ea79 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -156,8 +156,10 @@ struct oob_data {
 };
 
 struct adv_info {
+	struct delayed_work timeout_exp;
 	__u8 instance;
 	__u32 flags;
+	__u16 timeout;
 	__u16 adv_data_len;
 	__u8 adv_data[HCI_MAX_AD_LENGTH];
 	__u16 scan_rsp_len;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 280346f..e09ac01 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1336,6 +1336,49 @@ static bool hci_stop_discovery(struct hci_request *req)
 	return false;
 }
 
+static void advertising_added(struct sock *sk, struct hci_dev *hdev,
+			      u8 instance)
+{
+	struct mgmt_ev_advertising_added ev;
+
+	ev.instance = instance;
+
+	mgmt_event(MGMT_EV_ADVERTISING_ADDED, hdev, &ev, sizeof(ev), sk);
+}
+
+static void advertising_removed(struct sock *sk, struct hci_dev *hdev,
+				u8 instance)
+{
+	struct mgmt_ev_advertising_removed ev;
+
+	ev.instance = instance;
+
+	mgmt_event(MGMT_EV_ADVERTISING_REMOVED, hdev, &ev, sizeof(ev), sk);
+}
+
+static void clear_adv_instance(struct hci_dev *hdev)
+{
+	struct hci_request req;
+
+	if (!hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
+		return;
+
+	if (hdev->adv_instance.timeout)
+		cancel_delayed_work(&hdev->adv_instance.timeout_exp);
+
+	memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
+	advertising_removed(NULL, hdev, 1);
+	hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
+
+	if (!hdev_is_powered(hdev) ||
+	    hci_dev_test_flag(hdev, HCI_ADVERTISING))
+		return;
+
+	hci_req_init(&req, hdev);
+	disable_advertising(&req);
+	hci_req_run(&req, NULL);
+}
+
 static int clean_up_hci_state(struct hci_dev *hdev)
 {
 	struct hci_request req;
@@ -1351,6 +1394,9 @@ static int clean_up_hci_state(struct hci_dev *hdev)
 		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 	}
 
+	if (hdev->adv_instance.timeout)
+		clear_adv_instance(hdev);
+
 	if (hci_dev_test_flag(hdev, HCI_LE_ADV))
 		disable_advertising(&req);
 
@@ -6468,26 +6514,6 @@ static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
 	return true;
 }
 
-static void advertising_added(struct sock *sk, struct hci_dev *hdev,
-			      u8 instance)
-{
-	struct mgmt_ev_advertising_added ev;
-
-	ev.instance = instance;
-
-	mgmt_event(MGMT_EV_ADVERTISING_ADDED, hdev, &ev, sizeof(ev), sk);
-}
-
-static void advertising_removed(struct sock *sk, struct hci_dev *hdev,
-				u8 instance)
-{
-	struct mgmt_ev_advertising_removed ev;
-
-	ev.instance = instance;
-
-	mgmt_event(MGMT_EV_ADVERTISING_REMOVED, hdev, &ev, sizeof(ev), sk);
-}
-
 static void add_advertising_complete(struct hci_dev *hdev, u8 status,
 				     u16 opcode)
 {
@@ -6524,6 +6550,18 @@ unlock:
 	hci_dev_unlock(hdev);
 }
 
+static void adv_timeout_expired(struct work_struct *work)
+{
+	struct hci_dev *hdev = container_of(work, struct hci_dev,
+					    adv_instance.timeout_exp.work);
+
+	hdev->adv_instance.timeout = 0;
+
+	hci_dev_lock(hdev);
+	clear_adv_instance(hdev);
+	hci_dev_unlock(hdev);
+}
+
 static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 			   void *data, u16 data_len)
 {
@@ -6531,6 +6569,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 	struct mgmt_rp_add_advertising rp;
 	u32 flags;
 	u8 status;
+	u16 timeout;
 	int err;
 	struct mgmt_pending_cmd *cmd;
 	struct hci_request req;
@@ -6543,6 +6582,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 				       status);
 
 	flags = __le32_to_cpu(cp->flags);
+	timeout = __le16_to_cpu(cp->timeout);
 
 	/* The current implementation only supports adding one instance and
 	 * doesn't support flags.
@@ -6553,6 +6593,12 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 
 	hci_dev_lock(hdev);
 
+	if (timeout && !hdev_is_powered(hdev)) {
+		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+				      MGMT_STATUS_REJECTED);
+		goto unlock;
+	}
+
 	if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
 	    pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev) ||
 	    pending_find(MGMT_OP_SET_LE, hdev)) {
@@ -6569,6 +6615,8 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 		goto unlock;
 	}
 
+	INIT_DELAYED_WORK(&hdev->adv_instance.timeout_exp, adv_timeout_expired);
+
 	hdev->adv_instance.flags = flags;
 	hdev->adv_instance.adv_data_len = cp->adv_data_len;
 	hdev->adv_instance.scan_rsp_len = cp->scan_rsp_len;
@@ -6580,6 +6628,16 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
 		memcpy(hdev->adv_instance.scan_rsp_data,
 		       cp->data + cp->adv_data_len, cp->scan_rsp_len);
 
+	if (hdev->adv_instance.timeout)
+		cancel_delayed_work(&hdev->adv_instance.timeout_exp);
+
+	hdev->adv_instance.timeout = timeout;
+
+	if (timeout)
+		queue_delayed_work(hdev->workqueue,
+				   &hdev->adv_instance.timeout_exp,
+				   msecs_to_jiffies(timeout * 1000));
+
 	if (!hci_dev_test_and_set_flag(hdev, HCI_ADVERTISING_INSTANCE))
 		advertising_added(sk, hdev, 1);
 
@@ -6682,6 +6740,9 @@ static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
 		goto unlock;
 	}
 
+	if (hdev->adv_instance.timeout)
+		cancel_delayed_work(&hdev->adv_instance.timeout_exp);
+
 	memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
 
 	advertising_removed(sk, hdev, 1);
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH v2 3/7] Bluetooth: Add data structure for advertising instance

[Marcel Holtmann]

Hi Arman,

> This patch introduces a new data structure to represent advertising
> instances that were added using the "Add Advertising" mgmt command.
> Initially an hci_dev structure will support only one of these instances
> at a time, so the current instance is simply stored as a direct member
> of hci_dev.
> 
> Signed-off-by: Arman Uguray <armansito@chromium.org>
> ---
> include/net/bluetooth/hci_core.h | 16 ++++++++++++++++
> net/bluetooth/hci_core.c         |  1 +
> 2 files changed, 17 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b65c53d..2d853d8 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -155,6 +155,15 @@ struct oob_data {
> 	u8 rand256[16];
> };
> 
> +struct adv_info {
> +	__u8 instance;
> +	__u32 flags;
> +	__u16 adv_data_len;
> +	__u8 adv_data[HCI_MAX_AD_LENGTH];
> +	__u16 scan_rsp_len;
> +	__u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
> +};
> +

seems I missed this one in the previous review. Please align the field names.

	__u8  instance;
	__u32 flags;
	__16  adv_data_len;
	__u8  adv_data[..


> #define HCI_MAX_SHORT_NAME_LENGTH	10
> 
> /* Default LE RPA expiry time, 15 minutes */
> @@ -364,6 +373,8 @@ struct hci_dev {
> 	__u8			scan_rsp_data[HCI_MAX_AD_LENGTH];
> 	__u8			scan_rsp_data_len;
> 
> +	struct adv_info		adv_instance;
> +
> 	__u8			irk[16];
> 	__u32			rpa_timeout;
> 	struct delayed_work	rpa_expired;
> @@ -550,6 +561,11 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
> 	hdev->discovery.scan_duration = 0;
> }
> 
> +static inline void adv_info_init(struct hci_dev *hdev)
> +{
> +	memset(&hdev->adv_instance, 0, sizeof(struct adv_info));
> +}

I wonder if we should just set adv_instance->instance = 0xff to make sure it is not accidentally the level 0 instance.

> +
> bool hci_discovery_active(struct hci_dev *hdev);
> 
> void hci_discovery_set_state(struct hci_dev *hdev, int state);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 773f216..db3dd0c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3125,6 +3125,7 @@ struct hci_dev *hci_alloc_dev(void)
> 
> 	hci_init_sysfs(hdev);
> 	discovery_init(hdev);
> +	adv_info_init(hdev);
> 
> 	return hdev;
> }

Regards

Marcel

Re: [PATCH v2 6/7] Bluetooth: Add support for instance scan response

[Marcel Holtmann]

Hi Arman,

> This patch implements setting the Scan Response data provided as part
> of an advertising instance through the Add Advertising command.
> 
> Signed-off-by: Arman Uguray <armansito@chromium.org>
> ---
> net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 3991380..280346f 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -793,7 +793,7 @@ static struct mgmt_pending_cmd *pending_find_data(u16 opcode,
> 	return mgmt_pending_find_data(HCI_CHANNEL_CONTROL, opcode, hdev, data);
> }
> 
> -static u8 create_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> +static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> {
> 	u8 ad_len = 0;
> 	size_t name_len;
> @@ -819,7 +819,19 @@ static u8 create_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> 	return ad_len;
> }
> 
> -static void update_scan_rsp_data(struct hci_request *req)
> +static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> +{
> +	/* TODO: Set the appropriate entries based on advertising instance flags
> +	 * here once flags other than 0 are supported.
> +	 */
> +	memcpy(ptr, hdev->adv_instance.scan_rsp_data,
> +	       hdev->adv_instance.scan_rsp_len);
> +
> +	return hdev->adv_instance.scan_rsp_len;
> +}
> +
> +static void update_scan_rsp_data_for_instance(struct hci_request *req,
> +					      u8 instance)
> {
> 	struct hci_dev *hdev = req->hdev;
> 	struct hci_cp_le_set_scan_rsp_data cp;
> @@ -830,7 +842,10 @@ static void update_scan_rsp_data(struct hci_request *req)
> 
> 	memset(&cp, 0, sizeof(cp));
> 
> -	len = create_scan_rsp_data(hdev, cp.data);
> +	if (instance)
> +		len = create_instance_scan_rsp_data(hdev, 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) == 0)

Use !memcmp here. We are trying to remove the == 0 usages. There might be some still left in the code in multiple places, but for new code we try not to use it anymore.

> @@ -844,6 +859,25 @@ static void update_scan_rsp_data(struct hci_request *req)
> 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
> }
> 
> +static void update_scan_rsp_data(struct hci_request *req)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +	u8 instance;
> +
> +	/* The "Set Advertising" setting supersedes the "Add Advertising"
> +	 * setting. Here we set the scan response data based on which
> +	 * setting was set. When neither apply, default to the global settings,
> +	 * represented by instance "0".
> +	 */
> +	if (hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE) &&
> +	    !hci_dev_test_flag(hdev, HCI_ADVERTISING))
> +		instance = 1;
> +	else
> +		instance = 0;

here I would also use 0x01 and 0x00.

> +
> +	update_scan_rsp_data_for_instance(req, instance);
> +}
> +
> static u8 get_adv_discov_flags(struct hci_dev *hdev)
> {
> 	struct mgmt_pending_cmd *cmd;
> @@ -945,7 +979,7 @@ static void update_adv_data(struct hci_request *req)
> 	struct hci_dev *hdev = req->hdev;
> 	u8 instance;
> 
> -	/* The "Set Advertising" setting supercedes the "Add Advertising"
> +	/* The "Set Advertising" setting supersedes the "Add Advertising"

So this one should be just fixed in your previous patch ;)

> 	 * setting. Here we set the advertising data based on which
> 	 * setting was set. When neither apply, default to the global settings,
> 	 * represented by instance "0".
> @@ -4547,6 +4581,7 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
> 	if (val) {
> 		/* Switch to instance "0" for the Set Advertising setting. */
> 		update_adv_data_for_instance(&req, 0);
> +		update_scan_rsp_data_for_instance(&req, 0);
> 		enable_advertising(&req);
> 	} else {
> 		disable_advertising(&req);
> @@ -6408,25 +6443,25 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> 	return err;
> }
> 
> -static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
> -			      u8 adv_data_len)
> +static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
> +			      u8 len)
> {
> -	u8 max_adv_len = HCI_MAX_AD_LENGTH;
> +	u8 max_len = HCI_MAX_AD_LENGTH;
> 	int i, cur_len;
> 
> -	/* TODO: Correctly reduce adv_len based on adv_flags. */
> +	/* TODO: Correctly reduce len based on adv_flags. */
> 
> -	if (adv_data_len > max_adv_len)
> +	if (len > max_len)
> 		return false;
> 
> -	/* Make sure that adv_data is correctly formatted. */
> -	for (i = 0, cur_len = 0; i < adv_data_len; i += (cur_len + 1)) {
> -		cur_len = adv_data[i];
> +	/* Make sure that the data is correctly formatted. */
> +	for (i = 0, cur_len = 0; i < len; i += (cur_len + 1)) {
> +		cur_len = data[i];
> 
> 		/* If the current field length would exceed the total data
> 		 * length, then it's invalid.
> 		 */
> -		if (i + cur_len >= adv_data_len)
> +		if (i + cur_len >= len)
> 			return false;
> 	}
> 
> @@ -6526,7 +6561,9 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> 		goto unlock;
> 	}
> 
> -	if (!adv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len)) {
> +	if (!tlv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len) ||
> +	    !tlv_data_is_valid(hdev, flags, cp->data + cp->adv_data_len,
> +			       cp->scan_rsp_len)) {
> 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> 				      MGMT_STATUS_INVALID_PARAMS);
> 		goto unlock;
> @@ -6570,6 +6607,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> 	hci_req_init(&req, hdev);
> 
> 	update_adv_data(&req);
> +	update_scan_rsp_data(&req);
> 	enable_advertising(&req);
> 
> 	err = hci_req_run(&req, add_advertising_complete);

Regards

Marcel

Re: [PATCH v2 3/7] Bluetooth: Add data structure for advertising instance

[Arman Uguray]

Hi Marcel,

> On Mon, Mar 23, 2015 at 11:32 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Arman,
>
>> This patch introduces a new data structure to represent advertising
>> instances that were added using the "Add Advertising" mgmt command.
>> Initially an hci_dev structure will support only one of these instances
>> at a time, so the current instance is simply stored as a direct member
>> of hci_dev.
>>
>> Signed-off-by: Arman Uguray <armansito@chromium.org>
>> ---
>> include/net/bluetooth/hci_core.h | 16 ++++++++++++++++
>> net/bluetooth/hci_core.c         |  1 +
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index b65c53d..2d853d8 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -155,6 +155,15 @@ struct oob_data {
>>       u8 rand256[16];
>> };
>>
>> +struct adv_info {
>> +     __u8 instance;
>> +     __u32 flags;
>> +     __u16 adv_data_len;
>> +     __u8 adv_data[HCI_MAX_AD_LENGTH];
>> +     __u16 scan_rsp_len;
>> +     __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
>> +};
>> +
>
> seems I missed this one in the previous review. Please align the field names.
>
>         __u8  instance;
>         __u32 flags;
>         __16  adv_data_len;
>         __u8  adv_data[..
>
>
>> #define HCI_MAX_SHORT_NAME_LENGTH     10
>>
>> /* Default LE RPA expiry time, 15 minutes */
>> @@ -364,6 +373,8 @@ struct hci_dev {
>>       __u8                    scan_rsp_data[HCI_MAX_AD_LENGTH];
>>       __u8                    scan_rsp_data_len;
>>
>> +     struct adv_info         adv_instance;
>> +
>>       __u8                    irk[16];
>>       __u32                   rpa_timeout;
>>       struct delayed_work     rpa_expired;
>> @@ -550,6 +561,11 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
>>       hdev->discovery.scan_duration = 0;
>> }
>>
>> +static inline void adv_info_init(struct hci_dev *hdev)
>> +{
>> +     memset(&hdev->adv_instance, 0, sizeof(struct adv_info));
>> +}
>
> I wonder if we should just set adv_instance->instance = 0xff to make sure it is not accidentally the level 0 instance.
>

Probably makes no difference at the moment, since level 0 parameters
are read directly from the global settings. Eventually we'll want to
maintain a list of adv_info so that we can reuse the same data
structure and code paths to store level 0 and greater instances.
Unifying the code paths will be work on its own, so I'd rather do that
later in a separate set.

>> +
>> bool hci_discovery_active(struct hci_dev *hdev);
>>
>> void hci_discovery_set_state(struct hci_dev *hdev, int state);
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 773f216..db3dd0c 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3125,6 +3125,7 @@ struct hci_dev *hci_alloc_dev(void)
>>
>>       hci_init_sysfs(hdev);
>>       discovery_init(hdev);
>> +     adv_info_init(hdev);
>>
>>       return hdev;
>> }
>
> Regards
>
> Marcel
>

Thanks,
Arman