Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 08/15] Bluetooth: Don't write static address when privacy is enabled
From: Marcel Holtmann @ 2014-02-22 18:03 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-9-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> If privacy is enabled we'll be using RPAs instead of the static address.
> Therefore, only write the static address to HCI when the HCI_PRIVACY
> flag is not set.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 145254f84975..21d059968c32 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4609,7 +4609,8 @@ static int powered_update_hci(struct hci_dev *hdev)
> 
> 	if (lmp_le_capable(hdev)) {
> 		/* Set random address to static address if configured */
> -		if (bacmp(&hdev->static_addr, BDADDR_ANY))
> +		if (!test_bit(HCI_PRIVACY, &hdev->dev_flags) &&
> +		    bacmp(&hdev->static_addr, BDADDR_ANY))
> 			hci_req_add(&req, HCI_OP_LE_SET_RANDOM_ADDR, 6,
> 				    &hdev->static_addr);

since we are changing to code to always write the random address before enabling scanning, before enabling advertising and before creating a connection, lets remove this support here completely.

If we are not using privacy and have a static address, lets write it before it gets used. I added tracing of the currently configured random address. So we always know when we have to update the address itself or not. That makes it a bit more error free and gives us less headaches to figure static vs RPA.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 09/15] Bluetooth: Fix hdev->own_addr_type value when privacy is set
From: Marcel Holtmann @ 2014-02-22 18:06 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-10-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> When powering on the controller and deciding what value to set to
> hdev->own_address_time take also into account the HCI_PRIVACY flag (in
> which case random own address should be used).
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 964aa8deb009..753a73ba6764 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1477,14 +1477,16 @@ static void hci_init3_req(struct hci_request *req, unsigned long opt)
> 
> 	if (lmp_le_capable(hdev)) {
> 		/* If the controller has a public BD_ADDR, then by default
> -		 * use that one. If this is a LE only controller without
> -		 * a public address, default to the random address.
> +		 * use that one, unless Privacy is enabled. If this is a LE
> +		 * only controller without a public address, default to
> +		 * the random address.
> 		 *
> 		 * For debugging purposes it is possible to force
> 		 * controllers with a public address to use the
> 		 * random address instead.
> 		 */
> -		if (test_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags) ||
> +		if (test_bit(HCI_PRIVACY, &hdev->dev_flags) ||
> +		    test_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags) ||
> 		    !bacmp(&hdev->bdaddr, BDADDR_ANY))
> 			hdev->own_addr_type = ADDR_LE_DEV_RANDOM;

this patch is actually wrong. The hdev->own_addr_type should refer to the identity address. That one is either public address or static random address.

We need to check the address type we set for each command and set either hdev->own_addr_type if privacy is off or when privacy is on use ADDR_LE_DEV_RANDOM.

Please remember that we need to send the identity address type over SMP. So we need to know which one that is.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 10/15] Bluetooth: Add mechanism for updating the local RPA
From: Marcel Holtmann @ 2014-02-22 18:10 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-11-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> We need to have a way of updating the local RPA periodically. This patch
> adds a HCI_RPA_EXPIRED flag to track when the RPA needs to be
> regenerated as well as a delayed work structure and callback to do the
> updating periodically. By default the period for updating the RPA is set
> to 15 minutes.
> 
> When the RPA gets regenerated we need to disable and re-enable
> advertising if necessary. If there are any LE connections we do not
> update the RPA.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci.h      |  1 +
> include/net/bluetooth/hci_core.h |  7 +++++++
> net/bluetooth/hci_core.c         | 25 +++++++++++++++++++++++++
> net/bluetooth/mgmt.c             | 23 +++++++++++++++++++++++
> 4 files changed, 56 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5ff885ff29df..1bb45a47a78a 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -127,6 +127,7 @@ enum {
> 	HCI_SC_ENABLED,
> 	HCI_SC_ONLY,
> 	HCI_PRIVACY,
> +	HCI_RPA_EXPIRED,
> 	HCI_RPA_RESOLVING,
> 	HCI_HS_ENABLED,
> 	HCI_LE_ENABLED,
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 68bbcabdd9fd..94383dca6597 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -130,6 +130,9 @@ struct oob_data {
> 
> #define HCI_MAX_SHORT_NAME_LENGTH	10
> 
> +/* Default LE RPA expiry time, 15 minutes */
> +#define HCI_DEFAULT_RPA_TIMEOUT		(15 * 60)
> +
> struct amp_assoc {
> 	__u16	len;
> 	__u16	offset;
> @@ -304,6 +307,8 @@ struct hci_dev {
> 	__u8			scan_rsp_data_len;
> 
> 	__u8			irk[16];
> +	__u32			rpa_timeout;
> +	struct delayed_work	rpa_expired;
> 
> 	int (*open)(struct hci_dev *hdev);
> 	int (*close)(struct hci_dev *hdev);
> @@ -1252,6 +1257,8 @@ void hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max,
> void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
> 							__u8 ltk[16]);
> 
> +void hci_update_rpa(struct hci_request *req);
> +
> #define SCO_AIRMODE_MASK       0x0003
> #define SCO_AIRMODE_CVSD       0x0000
> #define SCO_AIRMODE_TRANSP     0x0003
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 753a73ba6764..eeeeda7e6b70 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2202,6 +2202,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> 
> 	cancel_delayed_work_sync(&hdev->le_scan_disable);
> 
> +	cancel_delayed_work_sync(&hdev->rpa_expired);
> +	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
> +

why on power down. Why not just set this on power up.

> 	hci_dev_lock(hdev);
> 	hci_inquiry_cache_flush(hdev);
> 	hci_conn_hash_flush(hdev);
> @@ -3276,6 +3279,26 @@ static void le_scan_disable_work(struct work_struct *work)
> 		BT_ERR("Disable LE scanning request failed: err %d", err);
> }
> 
> +void hci_update_rpa(struct hci_request *req)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +	bdaddr_t rpa;
> +
> +	if (smp_generate_rpa(hdev->tfm_aes, hdev->irk, &rpa) < 0) {
> +		BT_ERR("%s failed to generate new RPA", hdev->name);
> +		return;
> +	}
> +
> +	hci_req_add(req, HCI_OP_LE_SET_RANDOM_ADDR, 6, &rpa);
> +
> +	if (hdev->rpa_timeout) {
> +		int to;
> +
> +		to = msecs_to_jiffies(hdev->rpa_timeout * 1000);

I thought our rpa_timeout is in minutes? Are we allowing second granularity? I have nothing against it, but it will be rather useless to have a device that changes its RPA every second.

> +		queue_delayed_work(hdev->workqueue, &hdev->rpa_expired, to);
> +	}
> +}
> +
> /* Alloc HCI device */
> struct hci_dev *hci_alloc_dev(void)
> {
> @@ -3302,6 +3325,8 @@ struct hci_dev *hci_alloc_dev(void)
> 	hdev->le_conn_min_interval = 0x0028;
> 	hdev->le_conn_max_interval = 0x0038;
> 
> +	hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
> +
> 	mutex_init(&hdev->lock);
> 	mutex_init(&hdev->req_lock);
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 21d059968c32..248f116e3d08 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -882,12 +882,35 @@ static void service_cache_off(struct work_struct *work)
> 	hci_req_run(&req, NULL);
> }
> 
> +static void rpa_expired(struct work_struct *work)
> +{
> +	struct hci_dev *hdev = container_of(work, struct hci_dev,
> +					    rpa_expired.work);
> +	struct hci_request req;
> +
> +	BT_DBG("");
> +
> +	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
> +
> +	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags) ||
> +	    hci_conn_num(hdev, LE_LINK) > 0)
> +		return;
> +
> +	hci_req_init(&req, hdev);
> +
> +	disable_advertising(&req);
> +	enable_advertising(&req);
> +

might want to add a comment that the RPA gets regenerated when enabling advertising. Just to make it easy for everybody to get that this code does the right thing.

> +	hci_req_run(&req, NULL);
> +}
> +
> static void mgmt_init_hdev(struct sock *sk, struct hci_dev *hdev)
> {
> 	if (test_and_set_bit(HCI_MGMT, &hdev->dev_flags))
> 		return;
> 
> 	INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
> +	INIT_DELAYED_WORK(&hdev->rpa_expired, rpa_expired);
> 
> 	/* Non-mgmt controlled devices get this bit set
> 	 * implicitly so that pairing works for them, however

We should set the RPA_EXPIRED bit here.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 11/15] Bluetooth: Update local RPA if necessary when connecting LE
From: Marcel Holtmann @ 2014-02-22 18:12 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-12-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> When we initiate LE connections we need to regenerate the local RPA in
> case it has expired. This patch adds the necessary code for doing this
> in the hci_create_le_conn function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_conn.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index bd66c52eff95..325b42afc61e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -560,6 +560,9 @@ static int hci_create_le_conn(struct hci_conn *conn)
> 
> 	hci_req_init(&req, hdev);
> 
> +	if (test_and_clear_bit(HCI_RPA_EXPIRED, &hdev->dev_flags))
> +		hci_update_rpa(&req);
> +

and if privacy is not enabled, we should make sure the static address is the one that the controller currently knows about.

As explained in my other comment, this will make it long term a lot simpler. Maybe you want to introduce a helper for updating the random address. That why we just call it in certain places and it takes care of either writing a new RPA or it makes sure the static is set in case it differs.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 13/15] Bluetooth: Update local RPA if necessary before starting LE scan
From: Marcel Holtmann @ 2014-02-22 18:13 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-14-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> When we start LE scanning we need to regenerate the local RPA in case it
> has expired. This patch adds the necessary code for doing this in the
> start_discovery function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e3d8371a2493..2dd0c730f0e9 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3382,6 +3382,9 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
> 			goto failed;
> 		}
> 
> +		if (test_and_clear_bit(HCI_RPA_EXPIRED, &hdev->dev_flags))
> +			hci_update_rpa(&req);
> +

same comment here. We need to take care of the static address as well here.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 14/15] Bluetooth: Add debugfs entry for RPA regeneration timeout
From: Marcel Holtmann @ 2014-02-22 18:15 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-15-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> This patch adds a rpa_timeout debugfs entry which can be used to set the
> RPA regeneration timeout to something else than the default 15 minutes.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index eeeeda7e6b70..ecd147e51f45 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -492,6 +492,31 @@ static int idle_timeout_get(void *data, u64 *val)
> DEFINE_SIMPLE_ATTRIBUTE(idle_timeout_fops, idle_timeout_get,
> 			idle_timeout_set, "%llu\n");
> 
> +static int rpa_timeout_set(void *data, u64 val)
> +{
> +	struct hci_dev *hdev = data;
> +

if we are doing this in seconds, we might want to set some limits here. Don’t know which make sense, but something like at least 30 seconds and not more than 24 hours.

> +	hci_dev_lock(hdev);
> +	hdev->rpa_timeout = val;
> +	hci_dev_unlock(hdev);
> +
> +	return 0;
> +}

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 15/15] Bluetooth: Add full support for updating privacy state
From: Marcel Holtmann @ 2014-02-22 18:16 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-16-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> There are several things that need updating in addition to the
> HCI_PRIVACY flag when Set Privacy is called. This includes toggling the
> HCI_RPA_EXPIRED flag, setting the own_address_type variable, as well as
> restarting advertising if necessary.
> 
> This patch adds these missing pieces to the set_privacy function.

none of these should be needed if we use hdev->own_address_type as identity address type. And if we ensure that we update even the static address before each command that needs it.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 12/15] Bluetooth: Update local RPA if necessary before enabling advertising
From: Marcel Holtmann @ 2014-02-22 18:17 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1393088805-28658-13-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> When we enable advertising we need to regenerate the local RPA in case
> it has expired. This patch adds the necessary code for doing this in the
> enable_advertising function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 248f116e3d08..e3d8371a2493 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -842,6 +842,9 @@ static void enable_advertising(struct hci_request *req)
> 	struct hci_cp_le_set_adv_param cp;
> 	u8 enable = 0x01;
> 
> +	if (test_and_clear_bit(HCI_RPA_EXPIRED, &hdev->dev_flags))
> +		hci_update_rpa(req);
> +

same comment as the others. Lets also take the static address into account and make sure it up-to-date.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 10/15] Bluetooth: Add mechanism for updating the local RPA
From: Johan Hedberg @ 2014-02-22 18:37 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <30510655-D3CF-4168-A7E2-1DCB3496B606@holtmann.org>

Hi Marcel,

On Sat, Feb 22, 2014, Marcel Holtmann wrote:
> > @@ -2202,6 +2202,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> > 
> > 	cancel_delayed_work_sync(&hdev->le_scan_disable);
> > 
> > +	cancel_delayed_work_sync(&hdev->rpa_expired);
> > +	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
> > +
> 
> why on power down. Why not just set this on power up.

No reason. I can move it to power up.

> > @@ -3276,6 +3279,26 @@ static void le_scan_disable_work(struct work_struct *work)
> > 		BT_ERR("Disable LE scanning request failed: err %d", err);
> > }
> > 
> > +void hci_update_rpa(struct hci_request *req)
> > +{
> > +	struct hci_dev *hdev = req->hdev;
> > +	bdaddr_t rpa;
> > +
> > +	if (smp_generate_rpa(hdev->tfm_aes, hdev->irk, &rpa) < 0) {
> > +		BT_ERR("%s failed to generate new RPA", hdev->name);
> > +		return;
> > +	}
> > +
> > +	hci_req_add(req, HCI_OP_LE_SET_RANDOM_ADDR, 6, &rpa);
> > +
> > +	if (hdev->rpa_timeout) {
> > +		int to;
> > +
> > +		to = msecs_to_jiffies(hdev->rpa_timeout * 1000);
> 
> I thought our rpa_timeout is in minutes? Are we allowing second
> granularity? I have nothing against it, but it will be rather useless
> to have a device that changes its RPA every second.

Main reason is that we have other entries settable in seconds and it's
easier to verify that the timer is actually working if you don't have to
wait a full minute to change it. Not a big issue for me though, so I can
change it if you really want.

> > static void mgmt_init_hdev(struct sock *sk, struct hci_dev *hdev)
> > {
> > 	if (test_and_set_bit(HCI_MGMT, &hdev->dev_flags))
> > 		return;
> > 
> > 	INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
> > +	INIT_DELAYED_WORK(&hdev->rpa_expired, rpa_expired);
> > 
> > 	/* Non-mgmt controlled devices get this bit set
> > 	 * implicitly so that pairing works for them, however
> 
> We should set the RPA_EXPIRED bit here.

I don't think so. All the test_and_clear_bit(RPA_EXPIRED) places assume
that the flag will only be set if HCI_PRIVACY is also set. So I think
the right place to initialize it for the first time is when HCI_PRIVACY
is first set, i.e. in the set_privacy mgmt handler.

Johan

^ permalink raw reply

* Re: [PATCH 10/15] Bluetooth: Add mechanism for updating the local RPA
From: Marcel Holtmann @ 2014-02-22 18:40 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <20140222183750.GA30214@x220.p-661hnu-f1>

Hi Johan,

>>> @@ -2202,6 +2202,9 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>>> 
>>> 	cancel_delayed_work_sync(&hdev->le_scan_disable);
>>> 
>>> +	cancel_delayed_work_sync(&hdev->rpa_expired);
>>> +	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
>>> +
>> 
>> why on power down. Why not just set this on power up.
> 
> No reason. I can move it to power up.

lets move it there to make sure we always start out with a fresh one.

>>> @@ -3276,6 +3279,26 @@ static void le_scan_disable_work(struct work_struct *work)
>>> 		BT_ERR("Disable LE scanning request failed: err %d", err);
>>> }
>>> 
>>> +void hci_update_rpa(struct hci_request *req)
>>> +{
>>> +	struct hci_dev *hdev = req->hdev;
>>> +	bdaddr_t rpa;
>>> +
>>> +	if (smp_generate_rpa(hdev->tfm_aes, hdev->irk, &rpa) < 0) {
>>> +		BT_ERR("%s failed to generate new RPA", hdev->name);
>>> +		return;
>>> +	}
>>> +
>>> +	hci_req_add(req, HCI_OP_LE_SET_RANDOM_ADDR, 6, &rpa);
>>> +
>>> +	if (hdev->rpa_timeout) {
>>> +		int to;
>>> +
>>> +		to = msecs_to_jiffies(hdev->rpa_timeout * 1000);
>> 
>> I thought our rpa_timeout is in minutes? Are we allowing second
>> granularity? I have nothing against it, but it will be rather useless
>> to have a device that changes its RPA every second.
> 
> Main reason is that we have other entries settable in seconds and it's
> easier to verify that the timer is actually working if you don't have to
> wait a full minute to change it. Not a big issue for me though, so I can
> change it if you really want.

Keep it in seconds then and let us restrict it to a sensible range mentioned in the debugfs comment.

> 
>>> static void mgmt_init_hdev(struct sock *sk, struct hci_dev *hdev)
>>> {
>>> 	if (test_and_set_bit(HCI_MGMT, &hdev->dev_flags))
>>> 		return;
>>> 
>>> 	INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
>>> +	INIT_DELAYED_WORK(&hdev->rpa_expired, rpa_expired);
>>> 
>>> 	/* Non-mgmt controlled devices get this bit set
>>> 	 * implicitly so that pairing works for them, however
>> 
>> We should set the RPA_EXPIRED bit here.
> 
> I don't think so. All the test_and_clear_bit(RPA_EXPIRED) places assume
> that the flag will only be set if HCI_PRIVACY is also set. So I think
> the right place to initialize it for the first time is when HCI_PRIVACY
> is first set, i.e. in the set_privacy mgmt handler.

That might need changing. See my other comments. We should create a helper that can that check there.

Regards

Marcel


^ permalink raw reply

* [PATCH] shared: Fix not marking writer as active in mgmt
From: Szymon Janc @ 2014-02-22 18:40 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

---
 src/shared/mgmt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index ae90b89..bfc7258 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -209,8 +209,11 @@ static void wakeup_writer(struct mgmt *mgmt)
 	if (mgmt->writer_active)
 		return;
 
-	io_set_write_handler(mgmt->io, can_write_data, mgmt,
-						write_watch_destroy);
+	if (!io_set_write_handler(mgmt->io, can_write_data, mgmt,
+						write_watch_destroy))
+		return;
+
+	mgmt->writer_active = true;
 }
 
 struct opcode_index {
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 1/7] shared: Add support for disconnect handler to GLib based IO handling
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

From: Szymon Janc <szymon.janc@tieto.com>

---
 src/shared/io-glib.c | 76 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 9 deletions(-)

diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index 5bd9ac0..5490fc0 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -40,6 +40,10 @@ struct io {
 	io_callback_func_t write_callback;
 	io_destroy_func_t write_destroy;
 	void *write_data;
+	guint disconnect_watch;
+	io_callback_func_t disconnect_callback;
+	io_destroy_func_t disconnect_destroy;
+	void *disconnect_data;
 };
 
 static struct io *io_ref(struct io *io)
@@ -157,7 +161,7 @@ static gboolean read_callback(GIOChannel *channel, GIOCondition cond,
 	struct io *io = user_data;
 	bool result;
 
-	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
+	if (cond & (G_IO_ERR | G_IO_NVAL))
 		return FALSE;
 
 	if (io->read_callback)
@@ -183,9 +187,9 @@ bool io_set_read_handler(struct io *io, io_callback_func_t callback,
 		goto done;
 
 	io->read_watch = g_io_add_watch_full(io->channel, G_PRIORITY_DEFAULT,
-				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-				read_callback, io_ref(io),
-				read_watch_destroy);
+						G_IO_IN | G_IO_ERR | G_IO_NVAL,
+						read_callback, io_ref(io),
+						read_watch_destroy);
 	if (io->read_watch == 0)
 		return false;
 
@@ -218,7 +222,7 @@ static gboolean write_callback(GIOChannel *channel, GIOCondition cond,
 	struct io *io = user_data;
 	bool result;
 
-	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
+	if (cond & (G_IO_ERR | G_IO_NVAL))
 		return FALSE;
 
 	if (io->write_callback)
@@ -244,9 +248,9 @@ bool io_set_write_handler(struct io *io, io_callback_func_t callback,
 		goto done;
 
 	io->write_watch = g_io_add_watch_full(io->channel, G_PRIORITY_DEFAULT,
-				G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-				write_callback, io_ref(io),
-				write_watch_destroy);
+						G_IO_OUT | G_IO_ERR | G_IO_NVAL,
+						write_callback, io_ref(io),
+						write_watch_destroy);
 	if (io->write_watch == 0)
 		return false;
 
@@ -258,8 +262,62 @@ done:
 	return true;
 }
 
+static void disconnect_watch_destroy(gpointer user_data)
+{
+	struct io *io = user_data;
+
+	if (io->disconnect_destroy)
+		io->disconnect_destroy(io->disconnect_data);
+
+	io->disconnect_watch = 0;
+	io->disconnect_callback = NULL;
+	io->disconnect_destroy = NULL;
+	io->disconnect_data = NULL;
+
+	io_unref(io);
+}
+
+static gboolean disconnect_callback(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
+{
+	struct io *io = user_data;
+	bool result;
+
+	if (io->disconnect_callback)
+		result = io->disconnect_callback(io, io->disconnect_data);
+	else
+		result = false;
+
+	return result ? TRUE : FALSE;
+}
+
 bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
 				void *user_data, io_destroy_func_t destroy)
 {
-	return false;
+	if (!io)
+		return false;
+
+	if (io->disconnect_watch > 0) {
+		g_source_remove(io->disconnect_watch);
+		io->disconnect_watch = 0;
+	}
+
+	if (!callback)
+		goto done;
+
+	io->disconnect_watch = g_io_add_watch_full(io->channel,
+						G_PRIORITY_DEFAULT,
+						G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+						disconnect_callback, io_ref(io),
+						disconnect_watch_destroy);
+	if (io->disconnect_watch == 0)
+		return false;
+
+	io->disconnect_destroy = destroy;
+	io->disconnect_data = user_data;
+
+done:
+	io->disconnect_callback = callback;
+
+	return true;
 }
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 2/7] shared: Minor code style fixes in GLib based IO
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

---
 src/shared/io-glib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index 5490fc0..76f5a44 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -195,6 +195,7 @@ bool io_set_read_handler(struct io *io, io_callback_func_t callback,
 
 	io->read_destroy = destroy;
 	io->read_data = user_data;
+
 done:
 	io->read_callback = callback;
 
@@ -256,6 +257,7 @@ bool io_set_write_handler(struct io *io, io_callback_func_t callback,
 
 	io->write_destroy = destroy;
 	io->write_data = user_data;
+
 done:
 	io->write_callback = callback;
 
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 3/7] shared: Add support for disconnect handler to mainloop based IO
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

From: Szymon Janc <szymon.janc@tieto.com>

---
 src/shared/io-mainloop.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/shared/io-mainloop.c b/src/shared/io-mainloop.c
index d84d8d2..3fe1a88 100644
--- a/src/shared/io-mainloop.c
+++ b/src/shared/io-mainloop.c
@@ -42,6 +42,9 @@ struct io {
 	io_callback_func_t write_callback;
 	io_destroy_func_t write_destroy;
 	void *write_data;
+	io_callback_func_t disconnect_callback;
+	io_destroy_func_t disconnect_destroy;
+	void *disconnect_data;
 };
 
 static struct io *io_ref(struct io *io)
@@ -75,6 +78,9 @@ static void io_cleanup(void *user_data)
 	if (io->read_destroy)
 		io->read_destroy(io->read_data);
 
+	if (io->disconnect_destroy)
+		io->disconnect_destroy(io->disconnect_data);
+
 	if (io->close_on_destroy)
 		close(io->fd);
 
@@ -122,6 +128,21 @@ static void io_callback(int fd, uint32_t events, void *user_data)
 			mainloop_modify_fd(io->fd, io->events);
 		}
 	}
+
+	if ((events & EPOLLRDHUP) && io->disconnect_callback) {
+		if (!io->disconnect_callback(io, io->disconnect_data)) {
+			if (io->disconnect_destroy)
+				io->disconnect_destroy(io->disconnect_data);
+
+			io->disconnect_callback = NULL;
+			io->disconnect_destroy = NULL;
+			io->disconnect_data = NULL;
+
+			io->events &= ~EPOLLRDHUP;
+
+			mainloop_modify_fd(io->fd, io->events);
+		}
+	}
 }
 
 struct io *io_new(int fd)
@@ -246,5 +267,30 @@ bool io_set_write_handler(struct io *io, io_callback_func_t callback,
 bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
 				void *user_data, io_destroy_func_t destroy)
 {
-	return false;
+	uint32_t events;
+
+	if (!io || io->fd < 0)
+		return false;
+
+	if (io->disconnect_destroy)
+		io->disconnect_destroy(io->disconnect_data);
+
+	if (callback)
+		events = io->events | EPOLLRDHUP;
+	else
+		events = io->events & ~EPOLLRDHUP;
+
+	io->disconnect_callback = callback;
+	io->disconnect_destroy = destroy;
+	io->disconnect_data = user_data;
+
+	if (events == io->events)
+		return true;
+
+	if (mainloop_modify_fd(io->fd, events) < 0)
+		return false;
+
+	io->events = events;
+
+	return true;
 }
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 4/7] shared: Add support for disconnect handler in HFP
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

From: Szymon Janc <szymon.janc@tieto.com>

---
 src/shared/hfp.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/shared/hfp.h |  6 ++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 09fbe3e..98408ca 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -51,6 +51,13 @@ struct hfp_gw {
 	hfp_debug_func_t debug_callback;
 	hfp_destroy_func_t debug_destroy;
 	void *debug_data;
+
+	hfp_disconnect_func_t disconnect_callback;
+	hfp_destroy_func_t disconnect_destroy;
+	void *disconnect_data;
+
+	bool in_disconnect;
+	bool destroyed;
 };
 
 static void write_watch_destroy(void *user_data)
@@ -227,6 +234,7 @@ void hfp_gw_unref(struct hfp_gw *hfp)
 
 	io_set_write_handler(hfp->io, NULL, NULL, NULL);
 	io_set_read_handler(hfp->io, NULL, NULL, NULL);
+	io_set_disconnect_handler(hfp->io, NULL, NULL, NULL);
 
 	io_destroy(hfp->io);
 	hfp->io = NULL;
@@ -242,7 +250,12 @@ void hfp_gw_unref(struct hfp_gw *hfp)
 	ringbuf_free(hfp->write_buf);
 	hfp->write_buf = NULL;
 
-	free(hfp);
+	if (!hfp->in_disconnect) {
+		free(hfp);
+		return;
+	}
+
+	hfp->destroyed = true;
 }
 
 static void read_tracing(const void *buf, size_t count, void *user_data)
@@ -391,3 +404,54 @@ bool hfp_gw_set_command_handler(struct hfp_gw *hfp,
 
 	return true;
 }
+
+static void disconnect_watch_destroy(void *user_data)
+{
+	struct hfp_gw *hfp = user_data;
+
+	if (hfp->disconnect_destroy)
+		hfp->disconnect_destroy(hfp->disconnect_data);
+
+	if (hfp->destroyed)
+		free(hfp);
+}
+
+static bool io_disconnected(struct io *io, void *user_data)
+{
+	struct hfp_gw *hfp = user_data;
+
+	hfp->in_disconnect = true;
+
+	if (hfp->disconnect_callback)
+		hfp->disconnect_callback(hfp->disconnect_data);
+
+	hfp->in_disconnect = false;
+
+	return false;
+}
+
+bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
+					hfp_disconnect_func_t callback,
+					void *user_data,
+					hfp_destroy_func_t destroy)
+{
+	if (!hfp)
+		return false;
+
+	if (hfp->disconnect_destroy)
+		hfp->disconnect_destroy(hfp->disconnect_data);
+
+	if (!io_set_disconnect_handler(hfp->io, io_disconnected, hfp,
+						disconnect_watch_destroy)) {
+		hfp->disconnect_callback = NULL;
+		hfp->disconnect_destroy = NULL;
+		hfp->disconnect_data = NULL;
+		return false;
+	}
+
+	hfp->disconnect_callback = callback;
+	hfp->disconnect_destroy = destroy;
+	hfp->disconnect_data = user_data;
+
+	return true;
+}
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 836b166..6bb51bc 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -64,6 +64,7 @@ typedef void (*hfp_destroy_func_t)(void *user_data);
 typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
 
 typedef void (*hfp_command_func_t)(const char *command, void *user_data);
+typedef void (*hfp_disconnect_func_t)(void *user_data);
 
 struct hfp_gw;
 
@@ -86,3 +87,8 @@ bool hfp_gw_send_info(struct hfp_gw *hfp, const char *format, ...)
 bool hfp_gw_set_command_handler(struct hfp_gw *hfp,
 				hfp_command_func_t callback,
 				void *user_data, hfp_destroy_func_t destroy);
+
+bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
+					hfp_disconnect_func_t callback,
+					void *user_data,
+					hfp_destroy_func_t destroy);
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 5/7] shared: Add support for shutdown to IO
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

From: Szymon Janc <szymon.janc@tieto.com>

This allows to locally shutdown IO.
---
 src/shared/io-glib.c     | 9 +++++++++
 src/shared/io-mainloop.c | 9 +++++++++
 src/shared/io.h          | 2 ++
 3 files changed, 20 insertions(+)

diff --git a/src/shared/io-glib.c b/src/shared/io-glib.c
index 76f5a44..6316037 100644
--- a/src/shared/io-glib.c
+++ b/src/shared/io-glib.c
@@ -323,3 +323,12 @@ done:
 
 	return true;
 }
+
+bool io_shutdown(struct io *io)
+{
+	if (!io || !io->channel)
+		return false;
+
+	return g_io_channel_shutdown(io->channel, TRUE, NULL)
+							== G_IO_STATUS_NORMAL;
+}
diff --git a/src/shared/io-mainloop.c b/src/shared/io-mainloop.c
index 3fe1a88..2ea5eed 100644
--- a/src/shared/io-mainloop.c
+++ b/src/shared/io-mainloop.c
@@ -26,6 +26,7 @@
 #endif
 
 #include <unistd.h>
+#include <sys/socket.h>
 
 #include "monitor/mainloop.h"
 #include "src/shared/util.h"
@@ -294,3 +295,11 @@ bool io_set_disconnect_handler(struct io *io, io_callback_func_t callback,
 
 	return true;
 }
+
+bool io_shutdown(struct io *io)
+{
+	if (!io || io->fd < 0)
+		return false;
+
+	return shutdown(io->fd, SHUT_RDWR) == 0;
+}
diff --git a/src/shared/io.h b/src/shared/io.h
index 2e78c03..8897964 100644
--- a/src/shared/io.h
+++ b/src/shared/io.h
@@ -33,6 +33,8 @@ void io_destroy(struct io *io);
 int io_get_fd(struct io *io);
 bool io_set_close_on_destroy(struct io *io, bool do_close);
 
+bool io_shutdown(struct io *io);
+
 typedef bool (*io_callback_func_t)(struct io *io, void *user_data);
 
 bool io_set_read_handler(struct io *io, io_callback_func_t callback,
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 6/7] shared: Add support for local disconnect to HFP
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

From: Szymon Janc <szymon.janc@tieto.com>

This allows to locally trigger disconnection.
---
 src/shared/hfp.c | 8 ++++++++
 src/shared/hfp.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 98408ca..854cf46 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -455,3 +455,11 @@ bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
 
 	return true;
 }
+
+bool hfp_gw_disconnect(struct hfp_gw *hfp)
+{
+	if (!hfp)
+		return false;
+
+	return io_shutdown(hfp->io);
+}
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 6bb51bc..b0bd934 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -92,3 +92,5 @@ bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
 					hfp_disconnect_func_t callback,
 					void *user_data,
 					hfp_destroy_func_t destroy);
+
+bool hfp_gw_disconnect(struct hfp_gw *hfp);
-- 
1.9.0


^ permalink raw reply related

* [PATCH v2 7/7] android/handsfree: Use HFP code for connection handling
From: Szymon Janc @ 2014-02-22 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393103365-19908-1-git-send-email-szymon.janc@gmail.com>

From: Szymon Janc <szymon.janc@tieto.com>

HFP code is now able to handle disconnection on its own so just use
this instead of using own watches.
---
 android/handsfree.c | 50 +++++++++++++-------------------------------------
 1 file changed, 13 insertions(+), 37 deletions(-)

diff --git a/android/handsfree.c b/android/handsfree.c
index 680345b..60bff91 100644
--- a/android/handsfree.c
+++ b/android/handsfree.c
@@ -51,8 +51,6 @@
 static struct {
 	bdaddr_t bdaddr;
 	uint8_t state;
-	GIOChannel *io;
-	guint watch;
 	struct hfp_gw *gw;
 } device;
 
@@ -95,38 +93,23 @@ static void device_cleanup(void)
 		device.gw = NULL;
 	}
 
-	if (device.watch) {
-		g_source_remove(device.watch);
-		device.watch = 0;
-	}
-
-	if (device.io) {
-		g_io_channel_unref(device.io);
-		device.io = NULL;
-	}
-
 	device_set_state(HAL_EV_HANDSFREE_CONNECTION_STATE_DISCONNECTED);
 
 	memset(&device, 0, sizeof(device));
 }
 
-static gboolean watch_cb(GIOChannel *chan, GIOCondition cond,
-							gpointer user_data)
+static void at_command_handler(const char *command, void *user_data)
 {
-	DBG("");
-
-	device.watch = 0;
-
-	device_cleanup();
+	hfp_gw_send_result(device.gw, HFP_RESULT_ERROR);
 
-	return FALSE;
+	hfp_gw_disconnect(device.gw);
 }
 
-static void at_command_handler(const char *command, void *user_data)
+static void disconnect_watch(void *user_data)
 {
-	hfp_gw_send_result(device.gw, HFP_RESULT_ERROR);
+	DBG("");
 
-	g_io_channel_shutdown(device.io, TRUE, NULL);
+	device_cleanup();
 }
 
 static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
@@ -138,22 +121,15 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 		goto failed;
 	}
 
-	g_io_channel_set_close_on_unref(chan, TRUE);
-
 	device.gw = hfp_gw_new(g_io_channel_unix_get_fd(chan));
 	if (!device.gw)
 		goto failed;
 
+	g_io_channel_set_close_on_unref(chan, FALSE);
+
 	hfp_gw_set_close_on_unref(device.gw, true);
 	hfp_gw_set_command_handler(device.gw, at_command_handler, NULL, NULL);
-
-	device.watch = g_io_add_watch(chan,
-					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-					watch_cb, NULL);
-	if (device.watch == 0)
-		goto failed;
-
-	device.io = g_io_channel_ref(chan);
+	hfp_gw_set_disconnect_handler(device.gw, disconnect_watch, NULL, NULL);
 
 	device_set_state(HAL_EV_HANDSFREE_CONNECTION_STATE_CONNECTED);
 
@@ -331,11 +307,11 @@ static void handle_disconnect(const void *buf, uint16_t len)
 		goto failed;
 	}
 
-	if (device.io) {
-		device_set_state(HAL_EV_HANDSFREE_CONNECTION_STATE_DISCONNECTING);
-		g_io_channel_shutdown(device.io, TRUE, NULL);
-	} else {
+	if (device.state == HAL_EV_HANDSFREE_CONNECTION_STATE_CONNECTING) {
 		device_cleanup();
+	} else {
+		device_set_state(HAL_EV_HANDSFREE_CONNECTION_STATE_DISCONNECTING);
+		hfp_gw_disconnect(device.gw);
 	}
 
 	status = HAL_STATUS_SUCCESS;
-- 
1.9.0


^ permalink raw reply related

* Re: [PATCHv2 3/6] android/avrcp: Implement avrcp_get_avctp
From: Luiz Augusto von Dentz @ 2014-02-23 11:54 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1392996230-11789-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Feb 21, 2014 at 5:23 PM, Andrei Emeltchenko
<andrei.emeltchenko@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
>  android/avrcp-lib.c | 5 +++++
>  android/avrcp-lib.h | 3 +++
>  2 files changed, 8 insertions(+)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index ef48fa7..9180abd 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -39,6 +39,11 @@ struct avrcp {
>         struct avctp    *session;
>  };
>
> +struct avctp *avrcp_get_avctp(struct avrcp *session)
> +{
> +       return session->session;
> +}
> +
>  void avrcp_shutdown(struct avrcp *session)
>  {
>         if (session->session)
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index 7955d56..7bd8b25 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -21,8 +21,11 @@
>   *
>   */
>
> +struct avrcp;
> +
>  typedef void (*avrcp_destroy_cb_t) (void *user_data);
>
> +struct avctp *avrcp_get_avctp(struct avrcp *session);
>  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
>  void avrcp_shutdown(struct avrcp *session);
>  void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
> --
> 1.8.3.2

Not sure if you have seem my patch-set, this probably conflict with it
and a rather not have the AVCTP exposed.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCHv2 2/6] android/avctp: Add UNIT INFO command request
From: Luiz Augusto von Dentz @ 2014-02-23 11:57 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1392996230-11789-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Feb 21, 2014 at 5:23 PM, Andrei Emeltchenko
<andrei.emeltchenko@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
>  android/avctp.c | 13 +++++++++++++
>  android/avctp.h |  3 +++
>  2 files changed, 16 insertions(+)
>
> diff --git a/android/avctp.c b/android/avctp.c
> index 1e414d1..57a8c23 100644
> --- a/android/avctp.c
> +++ b/android/avctp.c
> @@ -1266,6 +1266,19 @@ int avctp_send_vendordep_req(struct avctp *session, uint8_t code,
>                                                 func, user_data);
>  }
>
> +int avctp_send_unit_info_req(struct avctp *session, avctp_rsp_cb func,
> +                                                               void *user_data)
> +{
> +       uint8_t operands[5];
> +
> +       memset(operands, 0xFF, sizeof(operands));
> +
> +       return avctp_send_req(session, AVC_CTYPE_STATUS,
> +                               AVC_SUBUNIT_UNIT, AVC_OP_UNITINFO,
> +                               operands, sizeof(operands),
> +                               func, user_data);
> +}
> +
>  unsigned int avctp_register_passthrough_handler(struct avctp *session,
>                                                 avctp_passthrough_cb cb,
>                                                 void *user_data)
> diff --git a/android/avctp.h b/android/avctp.h
> index dfa0ca6..7496e53 100644
> --- a/android/avctp.h
> +++ b/android/avctp.h
> @@ -46,6 +46,7 @@
>
>  /* subunits of interest */
>  #define AVC_SUBUNIT_PANEL              0x09
> +#define AVC_SUBUNIT_UNIT               0x1f
>
>  /* operands in passthrough commands */
>  #define AVC_SELECT                     0x00
> @@ -170,3 +171,5 @@ int avctp_send_vendordep_req(struct avctp *session, uint8_t code,
>  int avctp_send_browsing_req(struct avctp *session,
>                                 uint8_t *operands, size_t operand_count,
>                                 avctp_browsing_rsp_cb func, void *user_data);
> +int avctp_send_unit_info_req(struct avctp *session, avctp_rsp_cb func,
> +                                                       void *user_data);
> --
> 1.8.3.2

This probably needs to be internal and sent automatically once
connected specially if it is for a mandatory test.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCHv2 5/6] unit/avrcp: Add /TP/ICC/BV-01-I/CT test
From: Luiz Augusto von Dentz @ 2014-02-23 12:06 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1392996230-11789-5-git-send-email-Andrei.Emeltchenko.news@gmail.com>

HI Andrei,

On Fri, Feb 21, 2014 at 5:23 PM, Andrei Emeltchenko
<andrei.emeltchenko@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Test verifies that the Controller can collect information of Target
> by UNIT INFO command.
> ---
>  unit/test-avrcp.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index f1c0f46..2cb28b1 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -242,6 +242,37 @@ static void test_dummy(gconstpointer data)
>         destroy_context(context);
>  }
>
> +static void execute_context(struct context *context)
> +{
> +       g_main_loop_run(context->main_loop);
> +
> +       if (context->source > 0)
> +               g_source_remove(context->source);
> +
> +       avrcp_shutdown(context->session);
> +
> +       g_main_loop_unref(context->main_loop);
> +
> +       test_free(context->data);
> +       g_free(context);
> +}
> +
> +static void test_client(gconstpointer data)
> +{
> +       struct context *context = create_context(0x0100, data);
> +       struct avctp *session = avrcp_get_avctp(context->session);
> +       int ret = 0;
> +
> +       if (g_str_equal(context->data->test_name, "/TP/ICC/BV-01-I/CT"))
> +               ret = avctp_send_unit_info_req(session, NULL, NULL);
> +
> +       DBG("ret = %d", ret);
> +
> +       g_assert(!ret);
> +
> +       execute_context(context);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>         g_test_init(&argc, &argv, NULL);
> @@ -261,5 +292,11 @@ int main(int argc, char *argv[])
>         define_test("/TP/CRC/BV-01-I", test_dummy, raw_pdu(0x00));
>         define_test("/TP/CRC/BV-02-I", test_dummy, raw_pdu(0x00));
>
> +       /* Information collection for control tests */
> +
> +       define_test("/TP/ICC/BV-01-I/CT", test_client,
> +                       raw_pdu(0x00, 0x11, 0x0e, 0x01, 0xf8, 0x30,
> +                               0xff, 0xff, 0xff, 0xff, 0xff));
> +
>         return g_test_run();
>  }
> --
> 1.8.3.2

There is no such test in the test spec, there is only TP/ICC/BV-01-I,
perhaps you adding CT yourself just to be able to define the same test
for the TG but in that case I think it is better to separate with
different macros e.g. define_test_ct prefixing with "CT" to make
better distinction and don't mess with test names otherwise we may
confuse people looking at those tests.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH v2 00/10] LE Privacy support
From: johan.hedberg @ 2014-02-23 14:23 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Here's a new set of patches for the ones that didn't go through from the
initial set.

Johan

----------------------------------------------------------------
Johan Hedberg (10):
      Bluetooth: Set the correct address type in Identity Address Information
      Bluetooth: Add SMP function for generating RPAs
      Bluetooth: Add timer for regenerating local RPA
      Bluetooth: Add hci_get_own_address_type() convenience function
      Bluetooth: Use hci_get_own_address_type() when connecting LE
      Bluetooth: Use hci_get_own_address_type() for enabling advertising
      Bluetooth: Use hci_get_own_address_type() for initiating LE scan
      Bluetooth: Don't write static address during power on
      Bluetooth: Add debugfs entry for RPA regeneration timeout
      Bluetooth: Add support for Set Privacy command

 include/net/bluetooth/hci.h      |  1 +
 include/net/bluetooth/hci_core.h |  7 ++++
 net/bluetooth/hci_conn.c         |  2 +-
 net/bluetooth/hci_core.c         | 75 +++++++++++++++++++++++++++++++++
 net/bluetooth/mgmt.c             | 80 ++++++++++++++++++++++++++++++++----
 net/bluetooth/smp.c              | 21 +++++++++-
 net/bluetooth/smp.h              |  1 +
 7 files changed, 176 insertions(+), 11 deletions(-)


^ permalink raw reply

* [PATCH v2 01/10] Bluetooth: Set the correct address type in Identity Address Information
From: johan.hedberg @ 2014-02-23 14:23 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393165438-13610-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

The SMP Identity Address Information PDU contains our Identity Address.
The address type can either be static random or public and it is tracked
in the hdev->own_address_type variable.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 8ef50c790b96..ce2c461d07e3 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -1196,8 +1196,7 @@ int smp_distribute_keys(struct l2cap_conn *conn, __u8 force)
 
 		smp_send_cmd(conn, SMP_CMD_IDENT_INFO, sizeof(idinfo), &idinfo);
 
-		/* Just public address */
-		memset(&addrinfo, 0, sizeof(addrinfo));
+		addrinfo.addr_type = hdev->own_addr_type;
 		bacpy(&addrinfo.bdaddr, &hcon->src);
 
 		smp_send_cmd(conn, SMP_CMD_IDENT_ADDR_INFO, sizeof(addrinfo),
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 02/10] Bluetooth: Add SMP function for generating RPAs
From: johan.hedberg @ 2014-02-23 14:23 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393165438-13610-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

We need a function in smp.c to generate Resolvable Random Addresses in
order to support privacy. The local RPA will need to be generated before
advertising, scanning or connecting and regenerated at periodic
intervals. This patch adds the necessary function for RPA generation.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/smp.c | 18 ++++++++++++++++++
 net/bluetooth/smp.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index ce2c461d07e3..71a15233d1b6 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -124,6 +124,24 @@ bool smp_irk_matches(struct crypto_blkcipher *tfm, u8 irk[16],
 	return !memcmp(bdaddr->b, hash, 3);
 }
 
+int smp_generate_rpa(struct crypto_blkcipher *tfm, u8 irk[16], bdaddr_t *rpa)
+{
+	int err;
+
+	get_random_bytes(&rpa->b[3], 3);
+
+	rpa->b[5] &= 0x3f;
+	rpa->b[5] |= 0x40;
+
+	err = smp_ah(tfm, irk, &rpa->b[3], rpa->b);
+	if (err < 0)
+		return err;
+
+	BT_DBG("RPA %pMR", rpa);
+
+	return 0;
+}
+
 static int smp_c1(struct crypto_blkcipher *tfm, u8 k[16], u8 r[16],
 		  u8 preq[7], u8 pres[7], u8 _iat, bdaddr_t *ia,
 		  u8 _rat, bdaddr_t *ra, u8 res[16])
diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h
index d8cc543f523c..f32f1212f650 100644
--- a/net/bluetooth/smp.h
+++ b/net/bluetooth/smp.h
@@ -152,5 +152,6 @@ void smp_chan_destroy(struct l2cap_conn *conn);
 
 bool smp_irk_matches(struct crypto_blkcipher *tfm, u8 irk[16],
 		     bdaddr_t *bdaddr);
+int smp_generate_rpa(struct crypto_blkcipher *tfm, u8 irk[16], bdaddr_t *rpa);
 
 #endif /* __SMP_H */
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2 03/10] Bluetooth: Add timer for regenerating local RPA
From: johan.hedberg @ 2014-02-23 14:23 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393165438-13610-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

This patch adds a timer for updating the local RPA periodically. The
default timeout is set to 15 minutes.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci.h      |  1 +
 include/net/bluetooth/hci_core.h |  5 +++++
 net/bluetooth/hci_core.c         |  4 ++++
 net/bluetooth/mgmt.c             | 23 +++++++++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5ff885ff29df..1bb45a47a78a 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -127,6 +127,7 @@ enum {
 	HCI_SC_ENABLED,
 	HCI_SC_ONLY,
 	HCI_PRIVACY,
+	HCI_RPA_EXPIRED,
 	HCI_RPA_RESOLVING,
 	HCI_HS_ENABLED,
 	HCI_LE_ENABLED,
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 68bbcabdd9fd..6415514e4f17 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -130,6 +130,9 @@ struct oob_data {
 
 #define HCI_MAX_SHORT_NAME_LENGTH	10
 
+/* Default LE RPA expiry time, 15 minutes */
+#define HCI_DEFAULT_RPA_TIMEOUT		(15 * 60)
+
 struct amp_assoc {
 	__u16	len;
 	__u16	offset;
@@ -304,6 +307,8 @@ struct hci_dev {
 	__u8			scan_rsp_data_len;
 
 	__u8			irk[16];
+	__u32			rpa_timeout;
+	struct delayed_work	rpa_expired;
 
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 964aa8deb009..92d35811b61e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2102,6 +2102,7 @@ static int hci_dev_do_open(struct hci_dev *hdev)
 
 	if (!ret) {
 		hci_dev_hold(hdev);
+		set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
 		set_bit(HCI_UP, &hdev->flags);
 		hci_notify(hdev, HCI_DEV_UP);
 		if (!test_bit(HCI_SETUP, &hdev->dev_flags) &&
@@ -2199,6 +2200,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 		cancel_delayed_work(&hdev->service_cache);
 
 	cancel_delayed_work_sync(&hdev->le_scan_disable);
+	cancel_delayed_work_sync(&hdev->rpa_expired);
 
 	hci_dev_lock(hdev);
 	hci_inquiry_cache_flush(hdev);
@@ -3300,6 +3302,8 @@ struct hci_dev *hci_alloc_dev(void)
 	hdev->le_conn_min_interval = 0x0028;
 	hdev->le_conn_max_interval = 0x0038;
 
+	hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
+
 	mutex_init(&hdev->lock);
 	mutex_init(&hdev->req_lock);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 301b18a1c6a0..09316dd2cce1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -881,12 +881,35 @@ static void service_cache_off(struct work_struct *work)
 	hci_req_run(&req, NULL);
 }
 
+static void rpa_expired(struct work_struct *work)
+{
+	struct hci_dev *hdev = container_of(work, struct hci_dev,
+					    rpa_expired.work);
+	struct hci_request req;
+
+	BT_DBG("");
+
+	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
+
+	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags) ||
+	    hci_conn_num(hdev, LE_LINK) > 0)
+		return;
+
+	hci_req_init(&req, hdev);
+
+	disable_advertising(&req);
+	enable_advertising(&req);
+
+	hci_req_run(&req, NULL);
+}
+
 static void mgmt_init_hdev(struct sock *sk, struct hci_dev *hdev)
 {
 	if (test_and_set_bit(HCI_MGMT, &hdev->dev_flags))
 		return;
 
 	INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
+	INIT_DELAYED_WORK(&hdev->rpa_expired, rpa_expired);
 
 	/* Non-mgmt controlled devices get this bit set
 	 * implicitly so that pairing works for them, however
-- 
1.8.5.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox