From: Michał Narajowski <[email protected]>
This patch enables appending local name to scan response data. If
currently advertised instance has name flag set it is expired
immediately.
Signed-off-by: Michał Narajowski <[email protected]>
Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/hci_request.c | 28 +++++++++++++++++++--------
net/bluetooth/mgmt.c | 46 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 64 insertions(+), 10 deletions(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 9566ff8..0ce6cdd 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -971,14 +971,14 @@ void __hci_req_enable_advertising(struct hci_request *req)
hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
}
-static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
+static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
{
- u8 ad_len = 0;
size_t name_len;
+ int max_len;
+ max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
name_len = strlen(hdev->dev_name);
- if (name_len > 0) {
- size_t max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
+ if (name_len > 0 && max_len > 0) {
if (name_len > max_len) {
name_len = max_len;
@@ -997,22 +997,34 @@ static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
return ad_len;
}
+static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
+{
+ return append_local_name(hdev, ptr, 0);
+}
+
static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instance,
u8 *ptr)
{
struct adv_info *adv_instance;
+ u32 instance_flags;
+ u8 scan_rsp_len = 0;
adv_instance = hci_find_adv_instance(hdev, instance);
if (!adv_instance)
return 0;
- /* TODO: Set the appropriate entries based on advertising instance flags
- * here once flags other than 0 are supported.
- */
+ instance_flags = adv_instance->flags;
+
memcpy(ptr, adv_instance->scan_rsp_data,
adv_instance->scan_rsp_len);
- return adv_instance->scan_rsp_len;
+ scan_rsp_len += adv_instance->scan_rsp_len;
+ ptr += adv_instance->scan_rsp_len;
+
+ if (instance_flags & MGMT_ADV_FLAG_LOCAL_NAME)
+ scan_rsp_len = append_local_name(hdev, ptr, scan_rsp_len);
+
+ return scan_rsp_len;
}
void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instance)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74179b9..fa1f1c7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3014,6 +3014,35 @@ static int user_passkey_neg_reply(struct sock *sk, struct hci_dev *hdev,
HCI_OP_USER_PASSKEY_NEG_REPLY, 0);
}
+static void adv_expire(struct hci_dev *hdev, u32 flags)
+{
+ struct adv_info *adv_instance;
+ struct hci_request req;
+ int err;
+
+ adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
+ if (!adv_instance)
+ return;
+
+ /* stop if current instance doesn't need to be changed */
+ if (!(adv_instance->flags & flags))
+ return;
+
+ cancel_adv_timeout(hdev);
+
+ adv_instance = hci_get_next_instance(hdev, adv_instance->instance);
+ if (!adv_instance)
+ return;
+
+ hci_req_init(&req, hdev);
+ err = __hci_req_schedule_adv_instance(&req, adv_instance->instance,
+ true);
+ if (err)
+ return;
+
+ hci_req_run(&req, NULL);
+}
+
static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
{
struct mgmt_cp_set_local_name *cp;
@@ -3029,13 +3058,17 @@ static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
cp = cmd->param;
- if (status)
+ if (status) {
mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME,
mgmt_status(status));
- else
+ } else {
mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME, 0,
cp, sizeof(*cp));
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV))
+ adv_expire(hdev, MGMT_ADV_FLAG_LOCAL_NAME);
+ }
+
mgmt_pending_remove(cmd);
unlock:
@@ -5887,6 +5920,7 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
flags |= MGMT_ADV_FLAG_DISCOV;
flags |= MGMT_ADV_FLAG_LIMITED_DISCOV;
flags |= MGMT_ADV_FLAG_MANAGED_FLAGS;
+ flags |= MGMT_ADV_FLAG_LOCAL_NAME;
if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
flags |= MGMT_ADV_FLAG_TX_POWER;
@@ -5963,6 +5997,10 @@ static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
tx_power_managed = true;
max_len -= 3;
}
+ } else {
+ /* at least 1 byte of name should fit in */
+ if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
+ max_len -= 3;
}
if (len > max_len)
@@ -6295,6 +6333,10 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
max_len -= 3;
+ } else {
+ /* at least 1 byte of name should fit in */
+ if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
+ max_len -= 3;
}
return max_len;
--
2.7.4
Hi Szymon,
> This patch enables appending local name to scan response data. If
> currently advertised instance has name flag set it is expired
> immediately.
>
> Signed-off-by: Michał Narajowski <[email protected]>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/hci_request.c | 28 +++++++++++++++++++--------
> net/bluetooth/mgmt.c | 46 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 64 insertions(+), 10 deletions(-)
all 6 patches have been applied to bluetooth-next tree.
Regards
Marcel
Hi Szymon,
>>> This patch enables appending local name to scan response data. If
>>> currently advertised instance has name flag set it is expired
>>> immediately.
>>>
>>> Signed-off-by: Michał Narajowski <[email protected]>
>>> Signed-off-by: Szymon Janc <[email protected]>
>>> ---
>>> net/bluetooth/hci_request.c | 28 +++++++++++++++++++--------
>>> net/bluetooth/mgmt.c | 46
>>> +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64
>>> insertions(+), 10 deletions(-)
>>>
>>> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
>>> index 9566ff8..0ce6cdd 100644
>>> --- a/net/bluetooth/hci_request.c
>>> +++ b/net/bluetooth/hci_request.c
>>> @@ -971,14 +971,14 @@ void __hci_req_enable_advertising(struct hci_request
>>> *req)>
>>> hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
>>>
>>> }
>>>
>>> -static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
>>> +static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
>>> {
>>> - u8 ad_len = 0;
>>>
>>> size_t name_len;
>>>
>>> + int max_len;
>>>
>>> + max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
>>>
>>> name_len = strlen(hdev->dev_name);
>>>
>>> - if (name_len > 0) {
>>> - size_t max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
>>> + if (name_len > 0 && max_len > 0) {
>>>
>>> if (name_len > max_len) {
>>>
>>> name_len = max_len;
>>>
>>> @@ -997,22 +997,34 @@ static u8 create_default_scan_rsp_data(struct
>>> hci_dev *hdev, u8 *ptr)>
>>> return ad_len;
>>>
>>> }
>>>
>>> +static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
>>> +{
>>> + return append_local_name(hdev, ptr, 0);
>>> +}
>>> +
>>> static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instance,
>>>
>>> u8 *ptr)
>>>
>>> {
>>>
>>> struct adv_info *adv_instance;
>>>
>>> + u32 instance_flags;
>>> + u8 scan_rsp_len = 0;
>>>
>>> adv_instance = hci_find_adv_instance(hdev, instance);
>>> if (!adv_instance)
>>>
>>> return 0;
>>>
>>> - /* TODO: Set the appropriate entries based on advertising instance
> flags
>>> - * here once flags other than 0 are supported.
>>> - */
>>> + instance_flags = adv_instance->flags;
>>> +
>>>
>>> memcpy(ptr, adv_instance->scan_rsp_data,
>>>
>>> adv_instance->scan_rsp_len);
>>>
>>> - return adv_instance->scan_rsp_len;
>>> + scan_rsp_len += adv_instance->scan_rsp_len;
>>> + ptr += adv_instance->scan_rsp_len;
>>> +
>>> + if (instance_flags & MGMT_ADV_FLAG_LOCAL_NAME)
>>> + scan_rsp_len = append_local_name(hdev, ptr, scan_rsp_len);
>>> +
>>> + return scan_rsp_len;
>>> }
>>>
>>> void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instance)
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index 74179b9..fa1f1c7 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -3014,6 +3014,35 @@ static int user_passkey_neg_reply(struct sock *sk,
>>> struct hci_dev *hdev,>
>>> HCI_OP_USER_PASSKEY_NEG_REPLY, 0);
>>>
>>> }
>>>
>>> +static void adv_expire(struct hci_dev *hdev, u32 flags)
>>> +{
>>> + struct adv_info *adv_instance;
>>> + struct hci_request req;
>>> + int err;
>>> +
>>> + adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
>>> + if (!adv_instance)
>>> + return;
>>> +
>>> + /* stop if current instance doesn't need to be changed */
>>> + if (!(adv_instance->flags & flags))
>>> + return;
>>
>> are you sure this is correct? Isn't this !(current_flags ^ new_flags) what
>> you are looking for.
>
> I want to expire current advertising only if it has specific flag(s) set. Eg
> if name is changed, only expire if current advertising has name flag, if
> apperance is change, only expire if apprearance flag is set etc.
fair enough.
>> Also we should really have mgmt-tester test cases for this. We want to make
>> sure that this works for active instances and also the default instance 0
>> (aka Set Advertising).
>
> Yeap, will send updated mgmt-tester patches next week.
>
>>> +
>>> + cancel_adv_timeout(hdev);
>>> +
>>> + adv_instance = hci_get_next_instance(hdev, adv_instance->instance);
>>> + if (!adv_instance)
>>> + return;
>>> +
>>> + hci_req_init(&req, hdev);
>>> + err = __hci_req_schedule_adv_instance(&req, adv_instance->instance,
>>> + true);
>>> + if (err)
>>> + return;
>>> +
>>> + hci_req_run(&req, NULL);
>>> +}
>>> +
>>> static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
>>> {
>>>
>>> struct mgmt_cp_set_local_name *cp;
>>>
>>> @@ -3029,13 +3058,17 @@ static void set_name_complete(struct hci_dev
>>> *hdev, u8 status, u16 opcode)>
>>> cp = cmd->param;
>>>
>>> - if (status)
>>> + if (status) {
>>>
>>> mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME,
>>>
>>> mgmt_status(status));
>>>
>>> - else
>>> + } else {
>>>
>>> mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME, 0,
>>>
>>> cp, sizeof(*cp));
>>>
>>> + if (hci_dev_test_flag(hdev, HCI_LE_ADV))
>>> + adv_expire(hdev, MGMT_ADV_FLAG_LOCAL_NAME);
>>> + }
>>> +
>>>
>>> mgmt_pending_remove(cmd);
>>>
>>> unlock:
>>> @@ -5887,6 +5920,7 @@ static u32 get_supported_adv_flags(struct hci_dev
>>> *hdev)>
>>> flags |= MGMT_ADV_FLAG_DISCOV;
>>> flags |= MGMT_ADV_FLAG_LIMITED_DISCOV;
>>> flags |= MGMT_ADV_FLAG_MANAGED_FLAGS;
>>>
>>> + flags |= MGMT_ADV_FLAG_LOCAL_NAME;
>>>
>>> if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
>>>
>>> flags |= MGMT_ADV_FLAG_TX_POWER;
>>>
>>> @@ -5963,6 +5997,10 @@ static bool tlv_data_is_valid(struct hci_dev *hdev,
>>> u32 adv_flags, u8 *data,>
>>> tx_power_managed = true;
>>> max_len -= 3;
>>>
>>> }
>>>
>>> + } else {
>>> + /* at least 1 byte of name should fit in */
>>> + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
>>> + max_len -= 3;
>>>
>>> }
>>>
>>> if (len > max_len)
>>>
>>> @@ -6295,6 +6333,10 @@ static u8 tlv_data_max_len(u32 adv_flags, bool
>>> is_adv_data)>
>>> if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
>>>
>>> max_len -= 3;
>>>
>>> + } else {
>>> + /* at least 1 byte of name should fit in */
>>> + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
>>> + max_len -= 3;
>>>
>>> }
>>
>> So I do not get the math here. Either the short name will fit or the long
>> name. Especially the Get Advertising Size Information command should return
>> the accurate leftover size. I mean if the name utilizes all the space, then
>> this should return 0.
>>
>> Also the short name is maximal 11 + 2 octets. Meaning it will always fit
>> together with appearance no matter what. So if long name is too large, then
>> we pick the short name and return the leftover space.
>
> That was my thinking as well initially but...shortened name is never used for
> HCI (only copied to mgmt events so I assumed this is legacy cruft).
> What is done is that name is shortened if it doesn't fit (which make sense
> since shortened name should always be full name substring). But seems like max
> shortened name length is not respected eg for EIR shortened name is used if
> name length is >48 chars....
That sounds like an oversight on our part. While for EIR we might be able to argue this, but for AD we should start making use of the shortened name. However if none is set (which might be current bluetoothd default behavior), that is another story. In that case we might introduce a new error allowing to indicate this. Play with it and see how that works. However one thing I want is that userspace can make good decisions. That means we need to know how much of the name it includes.
> I can work on that but I think this can be seprate patchset that would clean
> all of above issues.
Feel free to send cleanup patches first. I personally do not care about the order, but yes, this needs some cleanups.
Regards
Marcel
Hi Marcel,
On Sunday, 18 September 2016 16:28:11 CEST Marcel Holtmann wrote:
> Hi Szymon,
>=20
> > This patch enables appending local name to scan response data. If
> > currently advertised instance has name flag set it is expired
> > immediately.
> >=20
> > Signed-off-by: Micha=C5=82 Narajowski <[email protected]>
> > Signed-off-by: Szymon Janc <[email protected]>
> > ---
> > net/bluetooth/hci_request.c | 28 +++++++++++++++++++--------
> > net/bluetooth/mgmt.c | 46
> > +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64
> > insertions(+), 10 deletions(-)
> >=20
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index 9566ff8..0ce6cdd 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -971,14 +971,14 @@ void __hci_req_enable_advertising(struct hci_requ=
est
> > *req)>=20
> > hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
> >=20
> > }
> >=20
> > -static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> > +static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
> > {
> > - u8 ad_len =3D 0;
> >=20
> > size_t name_len;
> >=20
> > + int max_len;
> >=20
> > + max_len =3D HCI_MAX_AD_LENGTH - ad_len - 2;
> >=20
> > name_len =3D strlen(hdev->dev_name);
> >=20
> > - if (name_len > 0) {
> > - size_t max_len =3D HCI_MAX_AD_LENGTH - ad_len - 2;
> > + if (name_len > 0 && max_len > 0) {
> >=20
> > if (name_len > max_len) {
> > =09
> > name_len =3D max_len;
> >=20
> > @@ -997,22 +997,34 @@ static u8 create_default_scan_rsp_data(struct
> > hci_dev *hdev, u8 *ptr)>=20
> > return ad_len;
> >=20
> > }
> >=20
> > +static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> > +{
> > + return append_local_name(hdev, ptr, 0);
> > +}
> > +
> > static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instan=
ce,
> >=20
> > u8 *ptr)
> >=20
> > {
> >=20
> > struct adv_info *adv_instance;
> >=20
> > + u32 instance_flags;
> > + u8 scan_rsp_len =3D 0;
> >=20
> > adv_instance =3D hci_find_adv_instance(hdev, instance);
> > if (!adv_instance)
> > =09
> > return 0;
> >=20
> > - /* TODO: Set the appropriate entries based on advertising instance=20
flags
> > - * here once flags other than 0 are supported.
> > - */
> > + instance_flags =3D adv_instance->flags;
> > +
> >=20
> > memcpy(ptr, adv_instance->scan_rsp_data,
> > =09
> > adv_instance->scan_rsp_len);
> >=20
> > - return adv_instance->scan_rsp_len;
> > + scan_rsp_len +=3D adv_instance->scan_rsp_len;
> > + ptr +=3D adv_instance->scan_rsp_len;
> > +
> > + if (instance_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> > + scan_rsp_len =3D append_local_name(hdev, ptr, scan_rsp_len);
> > +
> > + return scan_rsp_len;
> > }
> >=20
> > void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instanc=
e)
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 74179b9..fa1f1c7 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -3014,6 +3014,35 @@ static int user_passkey_neg_reply(struct sock *s=
k,
> > struct hci_dev *hdev,>=20
> > HCI_OP_USER_PASSKEY_NEG_REPLY, 0);
> >=20
> > }
> >=20
> > +static void adv_expire(struct hci_dev *hdev, u32 flags)
> > +{
> > + struct adv_info *adv_instance;
> > + struct hci_request req;
> > + int err;
> > +
> > + adv_instance =3D hci_find_adv_instance(hdev, hdev->cur_adv_instance);
> > + if (!adv_instance)
> > + return;
> > +
> > + /* stop if current instance doesn't need to be changed */
> > + if (!(adv_instance->flags & flags))
> > + return;
>=20
> are you sure this is correct? Isn't this !(current_flags ^ new_flags) what
> you are looking for.
I want to expire current advertising only if it has specific flag(s) set. E=
g=20
if name is changed, only expire if current advertising has name flag, if=20
apperance is change, only expire if apprearance flag is set etc.
>=20
> Also we should really have mgmt-tester test cases for this. We want to ma=
ke
> sure that this works for active instances and also the default instance 0
> (aka Set Advertising).
Yeap, will send updated mgmt-tester patches next week.
> > +
> > + cancel_adv_timeout(hdev);
> > +
> > + adv_instance =3D hci_get_next_instance(hdev, adv_instance->instance);
> > + if (!adv_instance)
> > + return;
> > +
> > + hci_req_init(&req, hdev);
> > + err =3D __hci_req_schedule_adv_instance(&req, adv_instance->instance,
> > + true);
> > + if (err)
> > + return;
> > +
> > + hci_req_run(&req, NULL);
> > +}
> > +
> > static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opco=
de)
> > {
> >=20
> > struct mgmt_cp_set_local_name *cp;
> >=20
> > @@ -3029,13 +3058,17 @@ static void set_name_complete(struct hci_dev
> > *hdev, u8 status, u16 opcode)>=20
> > cp =3D cmd->param;
> >=20
> > - if (status)
> > + if (status) {
> >=20
> > mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME,
> > =09
> > mgmt_status(status));
> >=20
> > - else
> > + } else {
> >=20
> > mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME, 0,
> > =09
> > cp, sizeof(*cp));
> >=20
> > + if (hci_dev_test_flag(hdev, HCI_LE_ADV))
> > + adv_expire(hdev, MGMT_ADV_FLAG_LOCAL_NAME);
> > + }
> > +
> >=20
> > mgmt_pending_remove(cmd);
> >=20
> > unlock:
> > @@ -5887,6 +5920,7 @@ static u32 get_supported_adv_flags(struct hci_dev
> > *hdev)>=20
> > flags |=3D MGMT_ADV_FLAG_DISCOV;
> > flags |=3D MGMT_ADV_FLAG_LIMITED_DISCOV;
> > flags |=3D MGMT_ADV_FLAG_MANAGED_FLAGS;
> >=20
> > + flags |=3D MGMT_ADV_FLAG_LOCAL_NAME;
> >=20
> > if (hdev->adv_tx_power !=3D HCI_TX_POWER_INVALID)
> > =09
> > flags |=3D MGMT_ADV_FLAG_TX_POWER;
> >=20
> > @@ -5963,6 +5997,10 @@ static bool tlv_data_is_valid(struct hci_dev *hd=
ev,
> > u32 adv_flags, u8 *data,>=20
> > tx_power_managed =3D true;
> > max_len -=3D 3;
> > =09
> > }
> >=20
> > + } else {
> > + /* at least 1 byte of name should fit in */
> > + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> > + max_len -=3D 3;
> >=20
> > }
> > =09
> > if (len > max_len)
> >=20
> > @@ -6295,6 +6333,10 @@ static u8 tlv_data_max_len(u32 adv_flags, bool
> > is_adv_data)>=20
> > if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
> > =09
> > max_len -=3D 3;
> >=20
> > + } else {
> > + /* at least 1 byte of name should fit in */
> > + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> > + max_len -=3D 3;
> >=20
> > }
>=20
> So I do not get the math here. Either the short name will fit or the long
> name. Especially the Get Advertising Size Information command should retu=
rn
> the accurate leftover size. I mean if the name utilizes all the space, th=
en
> this should return 0.
>=20
> Also the short name is maximal 11 + 2 octets. Meaning it will always fit
> together with appearance no matter what. So if long name is too large, th=
en
> we pick the short name and return the leftover space.
That was my thinking as well initially but...shortened name is never used f=
or=20
HCI (only copied to mgmt events so I assumed this is legacy cruft).
What is done is that name is shortened if it doesn't fit (which make sense=
=20
since shortened name should always be full name substring). But seems like =
max=20
shortened name length is not respected eg for EIR shortened name is used if=
=20
name length is >48 chars....
I can work on that but I think this can be seprate patchset that would clea=
n=20
all of above issues.
>=20
> Regards
>=20
> Marcel
=2D-=20
pozdrawiam
Szymon Janc
Hi Szymon,
> This patch enables appending local name to scan response data. If
> currently advertised instance has name flag set it is expired
> immediately.
>
> Signed-off-by: Michał Narajowski <[email protected]>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/hci_request.c | 28 +++++++++++++++++++--------
> net/bluetooth/mgmt.c | 46 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 9566ff8..0ce6cdd 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -971,14 +971,14 @@ void __hci_req_enable_advertising(struct hci_request *req)
> hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
> }
>
> -static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> +static u8 append_local_name(struct hci_dev *hdev, u8 *ptr, u8 ad_len)
> {
> - u8 ad_len = 0;
> size_t name_len;
> + int max_len;
>
> + max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
> name_len = strlen(hdev->dev_name);
> - if (name_len > 0) {
> - size_t max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
> + if (name_len > 0 && max_len > 0) {
>
> if (name_len > max_len) {
> name_len = max_len;
> @@ -997,22 +997,34 @@ static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> return ad_len;
> }
>
> +static u8 create_default_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
> +{
> + return append_local_name(hdev, ptr, 0);
> +}
> +
> static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instance,
> u8 *ptr)
> {
> struct adv_info *adv_instance;
> + u32 instance_flags;
> + u8 scan_rsp_len = 0;
>
> adv_instance = hci_find_adv_instance(hdev, instance);
> if (!adv_instance)
> return 0;
>
> - /* TODO: Set the appropriate entries based on advertising instance flags
> - * here once flags other than 0 are supported.
> - */
> + instance_flags = adv_instance->flags;
> +
> memcpy(ptr, adv_instance->scan_rsp_data,
> adv_instance->scan_rsp_len);
>
> - return adv_instance->scan_rsp_len;
> + scan_rsp_len += adv_instance->scan_rsp_len;
> + ptr += adv_instance->scan_rsp_len;
> +
> + if (instance_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> + scan_rsp_len = append_local_name(hdev, ptr, scan_rsp_len);
> +
> + return scan_rsp_len;
> }
>
> void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instance)
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 74179b9..fa1f1c7 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3014,6 +3014,35 @@ static int user_passkey_neg_reply(struct sock *sk, struct hci_dev *hdev,
> HCI_OP_USER_PASSKEY_NEG_REPLY, 0);
> }
>
> +static void adv_expire(struct hci_dev *hdev, u32 flags)
> +{
> + struct adv_info *adv_instance;
> + struct hci_request req;
> + int err;
> +
> + adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);
> + if (!adv_instance)
> + return;
> +
> + /* stop if current instance doesn't need to be changed */
> + if (!(adv_instance->flags & flags))
> + return;
are you sure this is correct? Isn't this !(current_flags ^ new_flags) what you are looking for.
Also we should really have mgmt-tester test cases for this. We want to make sure that this works for active instances and also the default instance 0 (aka Set Advertising).
> +
> + cancel_adv_timeout(hdev);
> +
> + adv_instance = hci_get_next_instance(hdev, adv_instance->instance);
> + if (!adv_instance)
> + return;
> +
> + hci_req_init(&req, hdev);
> + err = __hci_req_schedule_adv_instance(&req, adv_instance->instance,
> + true);
> + if (err)
> + return;
> +
> + hci_req_run(&req, NULL);
> +}
> +
> static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
> {
> struct mgmt_cp_set_local_name *cp;
> @@ -3029,13 +3058,17 @@ static void set_name_complete(struct hci_dev *hdev, u8 status, u16 opcode)
>
> cp = cmd->param;
>
> - if (status)
> + if (status) {
> mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME,
> mgmt_status(status));
> - else
> + } else {
> mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_SET_LOCAL_NAME, 0,
> cp, sizeof(*cp));
>
> + if (hci_dev_test_flag(hdev, HCI_LE_ADV))
> + adv_expire(hdev, MGMT_ADV_FLAG_LOCAL_NAME);
> + }
> +
> mgmt_pending_remove(cmd);
>
> unlock:
> @@ -5887,6 +5920,7 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
> flags |= MGMT_ADV_FLAG_DISCOV;
> flags |= MGMT_ADV_FLAG_LIMITED_DISCOV;
> flags |= MGMT_ADV_FLAG_MANAGED_FLAGS;
> + flags |= MGMT_ADV_FLAG_LOCAL_NAME;
>
> if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
> flags |= MGMT_ADV_FLAG_TX_POWER;
> @@ -5963,6 +5997,10 @@ static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
> tx_power_managed = true;
> max_len -= 3;
> }
> + } else {
> + /* at least 1 byte of name should fit in */
> + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> + max_len -= 3;
> }
>
> if (len > max_len)
> @@ -6295,6 +6333,10 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
>
> if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
> max_len -= 3;
> + } else {
> + /* at least 1 byte of name should fit in */
> + if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
> + max_len -= 3;
> }
So I do not get the math here. Either the short name will fit or the long name. Especially the Get Advertising Size Information command should return the accurate leftover size. I mean if the name utilizes all the space, then this should return 0.
Also the short name is maximal 11 + 2 octets. Meaning it will always fit together with appearance no matter what. So if long name is too large, then we pick the short name and return the leftover space.
Regards
Marcel
Increment the mgmt revision due to the recently added
Read Extended Controller Information and Set Appearance commands.
Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/mgmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 61e8153..b4075d1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -38,7 +38,7 @@
#include "mgmt_util.h"
#define MGMT_VERSION 1
-#define MGMT_REVISION 13
+#define MGMT_REVISION 14
static const u16 mgmt_commands[] = {
MGMT_OP_READ_INDEX_LIST,
--
2.7.4
Flags are not allowed in Scan Response.
Signed-off-by: Szymon Janc <[email protected]>
---
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 cdc88f4..61e8153 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6067,7 +6067,8 @@ static bool tlv_data_is_valid(u32 adv_flags, u8 *data, u8 len, bool is_adv_data)
for (i = 0, cur_len = 0; i < len; i += (cur_len + 1)) {
cur_len = data[i];
- if (data[i + 1] == EIR_FLAGS && flags_managed(adv_flags))
+ if (data[i + 1] == EIR_FLAGS &&
+ (!is_adv_data || flags_managed(adv_flags)))
return false;
if (data[i + 1] == EIR_TX_POWER && tx_power_managed(adv_flags))
--
2.7.4
This unifies max length and TLV validity checks.
Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/mgmt.c | 85 +++++++++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0751195..cdc88f4 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6007,34 +6007,59 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
return err;
}
-static bool tlv_data_is_valid(u32 adv_flags, u8 *data, u8 len, bool is_adv_data)
+static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
{
u8 max_len = HCI_MAX_AD_LENGTH;
- int i, cur_len;
- bool flags_managed = false;
- bool tx_power_managed = false;
if (is_adv_data) {
if (adv_flags & (MGMT_ADV_FLAG_DISCOV |
MGMT_ADV_FLAG_LIMITED_DISCOV |
- MGMT_ADV_FLAG_MANAGED_FLAGS)) {
- flags_managed = true;
+ MGMT_ADV_FLAG_MANAGED_FLAGS))
max_len -= 3;
- }
- if (adv_flags & MGMT_ADV_FLAG_TX_POWER) {
- tx_power_managed = true;
+ if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
max_len -= 3;
- }
} else {
/* at least 1 byte of name should fit in */
if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
max_len -= 3;
- if (adv_flags & MGMT_ADV_FLAG_APPEARANCE)
+ if (adv_flags & (MGMT_ADV_FLAG_APPEARANCE))
max_len -= 4;
}
+ return max_len;
+}
+
+static bool flags_managed(u32 adv_flags)
+{
+ return adv_flags & (MGMT_ADV_FLAG_DISCOV |
+ MGMT_ADV_FLAG_LIMITED_DISCOV |
+ MGMT_ADV_FLAG_MANAGED_FLAGS);
+}
+
+static bool tx_power_managed(u32 adv_flags)
+{
+ return adv_flags & MGMT_ADV_FLAG_TX_POWER;
+}
+
+static bool name_managed(u32 adv_flags)
+{
+ return adv_flags & MGMT_ADV_FLAG_LOCAL_NAME;
+}
+
+static bool appearance_managed(u32 adv_flags)
+{
+ return adv_flags & MGMT_ADV_FLAG_APPEARANCE;
+}
+
+static bool tlv_data_is_valid(u32 adv_flags, u8 *data, u8 len, bool is_adv_data)
+{
+ int i, cur_len;
+ u8 max_len;
+
+ max_len = tlv_data_max_len(adv_flags, is_adv_data);
+
if (len > max_len)
return false;
@@ -6042,10 +6067,20 @@ static bool tlv_data_is_valid(u32 adv_flags, u8 *data, u8 len, bool is_adv_data)
for (i = 0, cur_len = 0; i < len; i += (cur_len + 1)) {
cur_len = data[i];
- if (flags_managed && data[i + 1] == EIR_FLAGS)
+ if (data[i + 1] == EIR_FLAGS && flags_managed(adv_flags))
+ return false;
+
+ if (data[i + 1] == EIR_TX_POWER && tx_power_managed(adv_flags))
+ return false;
+
+ if (data[i + 1] == EIR_NAME_COMPLETE && name_managed(adv_flags))
+ return false;
+
+ if (data[i + 1] == EIR_NAME_SHORT && name_managed(adv_flags))
return false;
- if (tx_power_managed && data[i + 1] == EIR_TX_POWER)
+ if (data[i + 1] == EIR_APPEARANCE &&
+ appearance_managed(adv_flags))
return false;
/* If the current field length would exceed the total data
@@ -6353,30 +6388,6 @@ unlock:
return err;
}
-static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
-{
- u8 max_len = HCI_MAX_AD_LENGTH;
-
- if (is_adv_data) {
- if (adv_flags & (MGMT_ADV_FLAG_DISCOV |
- MGMT_ADV_FLAG_LIMITED_DISCOV |
- MGMT_ADV_FLAG_MANAGED_FLAGS))
- max_len -= 3;
-
- if (adv_flags & MGMT_ADV_FLAG_TX_POWER)
- max_len -= 3;
- } else {
- /* at least 1 byte of name should fit in */
- if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
- max_len -= 3;
-
- if (adv_flags & (MGMT_ADV_FLAG_APPEARANCE))
- max_len -= 4;
- }
-
- return max_len;
-}
-
static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
--
2.7.4
hdev parameter is not used in function.
Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/mgmt.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b9c997c..0751195 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6007,8 +6007,7 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
return err;
}
-static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
- u8 len, bool is_adv_data)
+static bool tlv_data_is_valid(u32 adv_flags, u8 *data, u8 len, bool is_adv_data)
{
u8 max_len = HCI_MAX_AD_LENGTH;
int i, cur_len;
@@ -6170,8 +6169,8 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
goto unlock;
}
- if (!tlv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len, true) ||
- !tlv_data_is_valid(hdev, flags, cp->data + cp->adv_data_len,
+ if (!tlv_data_is_valid(flags, cp->data, cp->adv_data_len, true) ||
+ !tlv_data_is_valid(flags, cp->data + cp->adv_data_len,
cp->scan_rsp_len, false)) {
err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
MGMT_STATUS_INVALID_PARAMS);
--
2.7.4
From: Michał Narajowski <[email protected]>
This patch enables prepending appearance value to scan response data.
It also adds support for setting appearance value through mgmt command.
If currently advertised instance has apperance flag set it is expired
immediately.
Signed-off-by: Michał Narajowski <[email protected]>
Signed-off-by: Szymon Janc <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/mgmt.h | 6 ++++++
net/bluetooth/hci_request.c | 8 ++++++++
net/bluetooth/mgmt.c | 37 +++++++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a48f71d..f00bf66 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -211,6 +211,7 @@ struct hci_dev {
__u8 dev_name[HCI_MAX_NAME_LENGTH];
__u8 short_name[HCI_MAX_SHORT_NAME_LENGTH];
__u8 eir[HCI_MAX_EIR_LENGTH];
+ __u16 appearance;
__u8 dev_class[3];
__u8 major_class;
__u8 minor_class;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 611b243..72a456b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -598,6 +598,12 @@ struct mgmt_rp_read_ext_info {
__u8 eir[0];
} __packed;
+#define MGMT_OP_SET_APPEARANCE 0x0043
+struct mgmt_cp_set_appearance {
+ __u16 appearance;
+} __packed;
+#define MGMT_SET_APPEARANCE_SIZE 2
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 0ce6cdd..c813568 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1015,6 +1015,14 @@ static u8 create_instance_scan_rsp_data(struct hci_dev *hdev, u8 instance,
instance_flags = adv_instance->flags;
+ if ((instance_flags & MGMT_ADV_FLAG_APPEARANCE) && hdev->appearance) {
+ ptr[0] = 3;
+ ptr[1] = EIR_APPEARANCE;
+ put_unaligned_le16(hdev->appearance, ptr + 2);
+ scan_rsp_len += 4;
+ ptr += 4;
+ }
+
memcpy(ptr, adv_instance->scan_rsp_data,
adv_instance->scan_rsp_len);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fa1f1c7..b9c997c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -105,6 +105,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_GET_ADV_SIZE_INFO,
MGMT_OP_START_LIMITED_DISCOVERY,
MGMT_OP_READ_EXT_INFO,
+ MGMT_OP_SET_APPEARANCE,
};
static const u16 mgmt_events[] = {
@@ -3145,6 +3146,34 @@ failed:
return err;
}
+static int set_appearance(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 len)
+{
+ struct mgmt_cp_set_appearance *cp = data;
+ u16 apperance;
+ int err;
+
+ BT_DBG("");
+
+ apperance = le16_to_cpu(cp->appearance);
+
+ hci_dev_lock(hdev);
+
+ if (hdev->appearance != apperance) {
+ hdev->appearance = apperance;
+
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV))
+ adv_expire(hdev, MGMT_ADV_FLAG_APPEARANCE);
+ }
+
+ err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_APPEARANCE, 0, NULL,
+ 0);
+
+ hci_dev_unlock(hdev);
+
+ return err;
+}
+
static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
u16 opcode, struct sk_buff *skb)
{
@@ -5920,6 +5949,7 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
flags |= MGMT_ADV_FLAG_DISCOV;
flags |= MGMT_ADV_FLAG_LIMITED_DISCOV;
flags |= MGMT_ADV_FLAG_MANAGED_FLAGS;
+ flags |= MGMT_ADV_FLAG_APPEARANCE;
flags |= MGMT_ADV_FLAG_LOCAL_NAME;
if (hdev->adv_tx_power != HCI_TX_POWER_INVALID)
@@ -6001,6 +6031,9 @@ static bool tlv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *data,
/* at least 1 byte of name should fit in */
if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
max_len -= 3;
+
+ if (adv_flags & MGMT_ADV_FLAG_APPEARANCE)
+ max_len -= 4;
}
if (len > max_len)
@@ -6337,6 +6370,9 @@ static u8 tlv_data_max_len(u32 adv_flags, bool is_adv_data)
/* at least 1 byte of name should fit in */
if (adv_flags & MGMT_ADV_FLAG_LOCAL_NAME)
max_len -= 3;
+
+ if (adv_flags & (MGMT_ADV_FLAG_APPEARANCE))
+ max_len -= 4;
}
return max_len;
@@ -6472,6 +6508,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ start_limited_discovery, MGMT_START_DISCOVERY_SIZE },
{ read_ext_controller_info,MGMT_READ_EXT_INFO_SIZE,
HCI_MGMT_UNTRUSTED },
+ { set_appearance, MGMT_SET_APPEARANCE_SIZE },
};
void mgmt_index_added(struct hci_dev *hdev)
--
2.7.4