Re: [PATCH 10/15] Bluetooth: Add mechanism for updating the local RPA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux