2021-04-14 12:30:48

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ] monitor: Fix the incorrect vendor name

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the vendor name is alwasy shown as "Microsoft" even
though a different vendor.

< HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
Type: Data fragment (0x01)
> HCI Event: Command Complete (0x0e) plen 4
Microsoft Secure Send (0x3f|0x0009) ncmd 31
Status: Success (0x00)
---
monitor/packet.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/monitor/packet.c b/monitor/packet.c
index d729a01cc..91d2294ff 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -9325,18 +9325,12 @@ static const char *get_supported_command(int bit)

static const char *current_vendor_str(void)
{
- uint16_t manufacturer, msft_opcode;
+ uint16_t manufacturer;

- if (index_current < MAX_INDEX) {
+ if (index_current < MAX_INDEX)
manufacturer = index_list[index_current].manufacturer;
- msft_opcode = index_list[index_current].msft_opcode;
- } else {
+ else
manufacturer = fallback_manufacturer;
- msft_opcode = BT_HCI_CMD_NOP;
- }
-
- if (msft_opcode != BT_HCI_CMD_NOP)
- return "Microsoft";

switch (manufacturer) {
case 2:
--
2.25.1


2021-04-14 12:36:51

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ] monitor: Fix the incorrect vendor name

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=466779

---Test result---

Test Summary:
CheckPatch PASS 0.39 seconds
GitLint PASS 0.10 seconds
Prep - Setup ELL PASS 44.99 seconds
Build - Prep PASS 0.10 seconds
Build - Configure PASS 7.84 seconds
Build - Make PASS 187.18 seconds
Make Check PASS 9.45 seconds
Make Dist PASS 12.13 seconds
Make Dist - Configure PASS 4.91 seconds
Make Dist - Make PASS 79.26 seconds
Build w/ext ELL - Configure PASS 7.94 seconds
Build w/ext ELL - Make PASS 181.20 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Dist - PASS
Desc: Run 'make dist' and build the distribution tarball

##############################
Test: Make Dist - Configure - PASS
Desc: Configure the source from distribution tarball

##############################
Test: Make Dist - Make - PASS
Desc: Build the source from distribution tarball

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth

2021-04-14 22:41:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [BlueZ] monitor: Fix the incorrect vendor name

Hi Tedd,

> This patch fixes the vendor name is alwasy shown as "Microsoft" even
> though a different vendor.
>
> < HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
> Type: Data fragment (0x01)
>> HCI Event: Command Complete (0x0e) plen 4
> Microsoft Secure Send (0x3f|0x0009) ncmd 31
> Status: Success (0x00)
> ---
> monitor/packet.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/monitor/packet.c b/monitor/packet.c
> index d729a01cc..91d2294ff 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -9325,18 +9325,12 @@ static const char *get_supported_command(int bit)
>
> static const char *current_vendor_str(void)
> {
> - uint16_t manufacturer, msft_opcode;
> + uint16_t manufacturer;
>
> - if (index_current < MAX_INDEX) {
> + if (index_current < MAX_INDEX)
> manufacturer = index_list[index_current].manufacturer;
> - msft_opcode = index_list[index_current].msft_opcode;
> - } else {
> + else
> manufacturer = fallback_manufacturer;
> - msft_opcode = BT_HCI_CMD_NOP;
> - }
> -
> - if (msft_opcode != BT_HCI_CMD_NOP)
> - return "Microsoft";

seems we have a bug here, but the fix can not be correct either. If we are running on Intel firmware and the Microsoft extension is used, it should show Microsoft and not Intel for the vendor commands.

Regards

Marcel

2021-04-15 02:27:23

by An, Tedd

[permalink] [raw]
Subject: Re: [BlueZ] monitor: Fix the incorrect vendor name

Hi Marcel,

On 4/14/21, 3:09 AM, "Marcel Holtmann" <[email protected]> wrote:

Hi Tedd,

> This patch fixes the vendor name is alwasy shown as "Microsoft" even
> though a different vendor.
>
> < HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
> Type: Data fragment (0x01)
>> HCI Event: Command Complete (0x0e) plen 4
> Microsoft Secure Send (0x3f|0x0009) ncmd 31
> Status: Success (0x00)
> ---
> monitor/packet.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/monitor/packet.c b/monitor/packet.c
> index d729a01cc..91d2294ff 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -9325,18 +9325,12 @@ static const char *get_supported_command(int bit)
>
> static const char *current_vendor_str(void)
> {
> - uint16_t manufacturer, msft_opcode;
> + uint16_t manufacturer;
>
> - if (index_current < MAX_INDEX) {
> + if (index_current < MAX_INDEX)
> manufacturer = index_list[index_current].manufacturer;
> - msft_opcode = index_list[index_current].msft_opcode;
> - } else {
> + else
> manufacturer = fallback_manufacturer;
> - msft_opcode = BT_HCI_CMD_NOP;
> - }
> -
> - if (msft_opcode != BT_HCI_CMD_NOP)
> - return "Microsoft";

seems we have a bug here, but the fix can not be correct either. If we are running on Intel firmware and the Microsoft extension is used, it should show Microsoft and not Intel for the vendor commands.

I submitted v2 and I think I took care of the msft_opcode handling but I realized that the msft_event_opcode is also like msft_opcode - each vendor will have a different value.
I know the msft_event_code for Intel, which is 0x50, but don't know for Realtek. (Do you happen to know?)

I changed the v2 to RFC for your further comments.

Regards

Marcel

Regards,
Tedd

2021-04-15 03:50:07

by Archie Pusaka

[permalink] [raw]
Subject: Re: [BlueZ] monitor: Fix the incorrect vendor name

Hi Tedd,

On Thu, 15 Apr 2021 at 10:26, An, Tedd <[email protected]> wrote:
>
> Hi Marcel,
>
> On 4/14/21, 3:09 AM, "Marcel Holtmann" <[email protected]> wrote:
>
> Hi Tedd,
>
> > This patch fixes the vendor name is alwasy shown as "Microsoft" even
> > though a different vendor.
> >
> > < HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
> > Type: Data fragment (0x01)
> >> HCI Event: Command Complete (0x0e) plen 4
> > Microsoft Secure Send (0x3f|0x0009) ncmd 31
> > Status: Success (0x00)
> > ---
> > monitor/packet.c | 12 +++---------
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/monitor/packet.c b/monitor/packet.c
> > index d729a01cc..91d2294ff 100644
> > --- a/monitor/packet.c
> > +++ b/monitor/packet.c
> > @@ -9325,18 +9325,12 @@ static const char *get_supported_command(int bit)
> >
> > static const char *current_vendor_str(void)
> > {
> > - uint16_t manufacturer, msft_opcode;
> > + uint16_t manufacturer;
> >
> > - if (index_current < MAX_INDEX) {
> > + if (index_current < MAX_INDEX)
> > manufacturer = index_list[index_current].manufacturer;
> > - msft_opcode = index_list[index_current].msft_opcode;
> > - } else {
> > + else
> > manufacturer = fallback_manufacturer;
> > - msft_opcode = BT_HCI_CMD_NOP;
> > - }
> > -
> > - if (msft_opcode != BT_HCI_CMD_NOP)
> > - return "Microsoft";
>
> seems we have a bug here, but the fix can not be correct either. If we are running on Intel firmware and the Microsoft extension is used, it should show Microsoft and not Intel for the vendor commands.
>
> I submitted v2 and I think I took care of the msft_opcode handling but I realized that the msft_event_opcode is also like msft_opcode - each vendor will have a different value.
> I know the msft_event_code for Intel, which is 0x50, but don't know for Realtek. (Do you happen to know?)

On my Realtek device the msft_event_code is 8 bytes long: 0x23 0x79
0x54 0x33 0x77 0x88 0x97 0x68.

localhost ~ # hcitool cmd 0x3f 0xf0 0x00
< HCI Command: ogf 0x3f, ocf 0x00f0, plen 1
00
> HCI Event: 0x0e plen 22
02 F0 FC 00 00 3F 00 00 00 00 00 00 00 08 23 79 54 33 77 88
97 68

> I changed the v2 to RFC for your further comments.
>
> Regards
>
> Marcel
>
> Regards,
> Tedd
>

Cheers,
Archie

2021-04-15 04:36:47

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: Re: [BlueZ] monitor: Fix the incorrect vendor name

Hi Archie,

On Thu, 2021-04-15 at 11:47 +0800, Archie Pusaka wrote:
> Hi Tedd,
>
> On Thu, 15 Apr 2021 at 10:26, An, Tedd <[email protected]> wrote:
> > Hi Marcel,
> >
> > On 4/14/21, 3:09 AM, "Marcel Holtmann" <[email protected]> wrote:
> >
> > Hi Tedd,
> >
> > > This patch fixes the vendor name is alwasy shown as "Microsoft" even
> > > though a different vendor.
> > >
> > > < HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
> > > Type: Data fragment (0x01)
> > >> HCI Event: Command Complete (0x0e) plen 4
> > > Microsoft Secure Send (0x3f|0x0009) ncmd 31
> > > Status: Success (0x00)
> > > ---
> > > monitor/packet.c | 12 +++---------
> > > 1 file changed, 3 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/monitor/packet.c b/monitor/packet.c
> > > index d729a01cc..91d2294ff 100644
> > > --- a/monitor/packet.c
> > > +++ b/monitor/packet.c
> > > @@ -9325,18 +9325,12 @@ static const char *get_supported_command(int
> > bit)
> > >
> > > static const char *current_vendor_str(void)
> > > {
> > > - uint16_t manufacturer, msft_opcode;
> > > + uint16_t manufacturer;
> > >
> > > - if (index_current < MAX_INDEX) {
> > > + if (index_current < MAX_INDEX)
> > > manufacturer = index_list[index_current].manufacturer;
> > > - msft_opcode = index_list[index_current].msft_opcode;
> > > - } else {
> > > + else
> > > manufacturer = fallback_manufacturer;
> > > - msft_opcode = BT_HCI_CMD_NOP;
> > > - }
> > > -
> > > - if (msft_opcode != BT_HCI_CMD_NOP)
> > > - return "Microsoft";
> >
> > seems we have a bug here, but the fix can not be correct either. If we
> > are running on Intel firmware and the Microsoft extension is used, it should
> > show Microsoft and not Intel for the vendor commands.
> >
> > I submitted v2 and I think I took care of the msft_opcode handling but I
> > realized that the msft_event_opcode is also like msft_opcode - each vendor
> > will have a different value.
> > I know the msft_event_code for Intel, which is 0x50, but don't know for
> > Realtek. (Do you happen to know?)
>
> On my Realtek device the msft_event_code is 8 bytes long: 0x23 0x79
> 0x54 0x33 0x77 0x88 0x97 0x68.
>
> localhost ~ # hcitool cmd 0x3f 0xf0 0x00
> < HCI Command: ogf 0x3f, ocf 0x00f0, plen 1
> 00
> > HCI Event: 0x0e plen 22
> 02 F0 FC 00 00 3F 00 00 00 00 00 00 00 08 23 79 54 33 77 88
> 97 68
>

Thanks for the info.
I am going to change the pat to support "variable" length of msft_event_code.

> > I changed the v2 to RFC for your further comments.
> >
> > Regards
> >
> > Marcel
> >
> > Regards,
> > Tedd
> >
>
> Cheers,
> Archie

2021-04-16 05:58:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [BlueZ] monitor: Fix the incorrect vendor name

Hi Archie,

>>> This patch fixes the vendor name is alwasy shown as "Microsoft" even
>>> though a different vendor.
>>>
>>> < HCI Command: Microsoft Secure Send (0x3f|0x0009) plen 249
>>> Type: Data fragment (0x01)
>>>> HCI Event: Command Complete (0x0e) plen 4
>>> Microsoft Secure Send (0x3f|0x0009) ncmd 31
>>> Status: Success (0x00)
>>> ---
>>> monitor/packet.c | 12 +++---------
>>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>> index d729a01cc..91d2294ff 100644
>>> --- a/monitor/packet.c
>>> +++ b/monitor/packet.c
>>> @@ -9325,18 +9325,12 @@ static const char *get_supported_command(int bit)
>>>
>>> static const char *current_vendor_str(void)
>>> {
>>> - uint16_t manufacturer, msft_opcode;
>>> + uint16_t manufacturer;
>>>
>>> - if (index_current < MAX_INDEX) {
>>> + if (index_current < MAX_INDEX)
>>> manufacturer = index_list[index_current].manufacturer;
>>> - msft_opcode = index_list[index_current].msft_opcode;
>>> - } else {
>>> + else
>>> manufacturer = fallback_manufacturer;
>>> - msft_opcode = BT_HCI_CMD_NOP;
>>> - }
>>> -
>>> - if (msft_opcode != BT_HCI_CMD_NOP)
>>> - return "Microsoft";
>>
>> seems we have a bug here, but the fix can not be correct either. If we are running on Intel firmware and the Microsoft extension is used, it should show Microsoft and not Intel for the vendor commands.
>>
>> I submitted v2 and I think I took care of the msft_opcode handling but I realized that the msft_event_opcode is also like msft_opcode - each vendor will have a different value.
>> I know the msft_event_code for Intel, which is 0x50, but don't know for Realtek. (Do you happen to know?)
>
> On my Realtek device the msft_event_code is 8 bytes long: 0x23 0x79
> 0x54 0x33 0x77 0x88 0x97 0x68.


I remember having seen different event prefixes for Realtek controllers. However after re-testing it seems to be the same.

My latest 5.1 dongle has this:

> HCI Event: Command Complete (0x0e) plen 22
Microsoft Extension (0x3f|0x00f0) ncmd 2
Read Supported Features (0x00)
Status: Success (0x00)
Features: 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00
RSSI Monitoring feature for BR/EDR
RSSI Monitoring feature for LE connections
RSSI Monitoring of LE advertisements
Advertising Monitoring of LE advertisements
Verifying the validity of P-192 and P-256 keys
Continuous Advertising Monitoring
Event prefix length: 8
23 79 54 33 77 88 97 68

And my older 4.2 dongle has this:

> HCI Event: Command Complete (0x0e) plen 22
Microsoft Extension (0x3f|0x00f0) ncmd 2
Read Supported Features (0x00)
Status: Success (0x00)
Features: 0x0f 0x00 0x00 0x00 0x00 0x00 0x00 0x00
RSSI Monitoring feature for BR/EDR
RSSI Monitoring feature for LE connections
RSSI Monitoring of LE advertisements
Advertising Monitoring of LE advertisements
Event prefix length: 8
23 79 54 33 77 88 97 68

Regards

Marcel