Hi Jakub,
>>>> This patch adds the opcode and structure for Start Service Discovery
>>>> operation.
>>>>
>>>> Signed-off-by: Jakub Pawlowski <jpawlowski@xxxxxxxxxx>
>>>> Signed-off-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
>>>> ---
>>>> include/net/bluetooth/mgmt.h | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>>>> index 9b382ea34fd9..95c34d5180fa 100644
>>>> --- a/include/net/bluetooth/mgmt.h
>>>> +++ b/include/net/bluetooth/mgmt.h
>>>> @@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
>>>> } __packed;
>>>> #define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
>>>>
>>>> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
>>>> +struct mgmt_cp_start_service_discovery {
>>>> + __u8 type;
>>>
>>> Maybe we should get rid of type ? service discovery based on
>>> advertisement content makes sense only for LE
>>
>> this works perfectly fine for BR/EDR or BR/EDR + LE discovery. It is up to the call to decide what to do. And we have kept the type in place even for commands that are currently only supported on one transport. It allows us to extend the API without breaking it.
>
> Ok, that sounds reasonable.
>
> I've merged my filtering patch on top of your patches, then run
> everything and did some manual tests - all works great!
> I've just send updated patches.
I did re-work your patches a little bit actually.
The filter should be applied independently on EIR, advertising data and/or scan response data. Running it over the combined data has the danger that if some device are broken it becomes hard to detect wrong scan response data. So I decided to run this manually. And I also decided to skip the scan response data in case we already found a match in the advertising data.
There is also the funny case if either of this data is empty, then we should consider no matching and need to handle that one as well.
You also had some uint8_t in there which I assume you copied from userspace code. We do not use that in kernel code and use u8 instead. So I fixed that as well. If I counted correctly, the the code also had one off-by-one for the EIR field length check. That should be fixed as well now.
The only missing piece is now the LE restart handling. That is the one patch I have actually not yet looked at.
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
