"packet types" wrong in libbluetooth

"packet types" wrong in libbluetooth

[Iain Hibbert]

Hi,

The HCI packet type functions ptypetostr() and strtoptype() in lib/hci.c
don't work correctly. This is because the HCI spec is messed up wrt the 2
and 3 Mbps packet types, which are indicated by inverted bit logic (see
"Create Connection Command" in core spec document)

I think (you might like to verify that) that the patch below will calm
this hienous situation before anybody needs to call the cops.

regards,
iain

--- lib/hci.c.orig	2009-01-09 20:11:59.000000000 +0000
+++ lib/hci.c	2009-01-09 20:12:02.000000000 +0000
@@ -232,12 +232,18 @@

 char *hci_ptypetostr(unsigned int ptype)
 {
+	ptype ^= (HCI_2DH1|HCI_2DH3|HCI_2DH5|HCI_3DH1|HCI_3DH3|HCI_3DH5);
 	return hci_bit2str(pkt_type_map, ptype);
 }

 int hci_strtoptype(char *str, unsigned int *val)
 {
-	return hci_str2bit(pkt_type_map, str, val);
+
+	if (hci_str2bit(pkt_type_map, str, val) == 0)
+		return 0;
+
+	*val ^= (HCI_2DH1|HCI_2DH3|HCI_2DH5|HCI_3DH1|HCI_3DH3|HCI_3DH5);
+	return 1;
 }

 char *hci_scoptypetostr(unsigned int ptype)

Re: "packet types" wrong in libbluetooth

[Marcel Holtmann]

Hi Iain,

> The HCI packet type functions ptypetostr() and strtoptype() in lib/hci.c
> don't work correctly. This is because the HCI spec is messed up wrt the 2
> and 3 Mbps packet types, which are indicated by inverted bit logic (see
> "Create Connection Command" in core spec document)

current situation is perfectly fine. It follows the spec. closely and
this means for EDR the types are inverted. Trying to make them looks
nice doesn't help. You should not mess with packet types at all. It is
the link managers' job to get this right.

Regards

Marcel

Re: "packet types" wrong in libbluetooth

[Iain Hibbert]

On Sat, 10 Jan 2009, Marcel Holtmann wrote:

> Hi Iain,
>
> > The HCI packet type functions ptypetostr() and strtoptype() in lib/hci.c
> > don't work correctly. This is because the HCI spec is messed up wrt the 2
> > and 3 Mbps packet types, which are indicated by inverted bit logic (see
> > "Create Connection Command" in core spec document)
>
> current situation is perfectly fine. It follows the spec. closely and
> this means for EDR the types are inverted.

since the spec is somewhat twisty I don't see how it follows it at all :)

> Trying to make them looks nice doesn't help. You should not mess with
> packet types at all. It is the link managers' job to get this right.

I am not messing with packet types at all. I apologise because I don't use
Linux or the BlueZ tools in general but it seems that hcidump displays
"Create Connection" packets (at least) incorrectly because of this. For
instance

	hci_ptypetostr(HCI_DH1)

will return

	"DH1 "

which seems normal. But, when this is part of a HCI packet (eg Create
Connection command), that result should actually be:

	"DH1 2-DH1 2-DH3 2-DH5 3-DH1 3-DH3 3-DH5 "

Should it be handled in hcidump instead, as below?

regards,
iain

--- parser/hci.c.orig	2009-01-10 15:51:51.000000000 +0000
+++ parser/hci.c	2009-01-10 15:51:53.000000000 +0000
@@ -718,6 +718,7 @@
 		addr, ptype, cp->role_switch,
 		clkoffset & 0x7fff, clkoffset & 0x8000 ? " (valid)" : "");

+	ptype ^= (HCI_2DH1|HCI_2DH3|HCI_2DH5|HCI_3DH1|HCI_3DH3|HCI_3DH5);
 	str = hci_ptypetostr(ptype);
 	if (str) {
 		p_indent(level, frm);
@@ -746,6 +747,7 @@
 	p_indent(level, frm);
 	printf("handle %d ptype 0x%4.4x\n", btohs(cp->handle), ptype);

+	ptype ^= (HCI_2DH1|HCI_2DH3|HCI_2DH5|HCI_3DH1|HCI_3DH3|HCI_3DH5);
 	str = hci_ptypetostr(ptype);
 	if (str) {
 		p_indent(level, frm);
@@ -2761,6 +2763,7 @@
 		p_indent(level, frm);
 		printf("Error: %s\n", status2str(evt->status));
 	} else {
+		ptype ^= (HCI_2DH1|HCI_2DH3|HCI_2DH5|HCI_3DH1|HCI_3DH3|HCI_3DH5);
 		str = hci_ptypetostr(ptype);
 		if (str) {
 			p_indent(level, frm);

Re: "packet types" wrong in libbluetooth

[Marcel Holtmann]

Hi Iain,

> > > The HCI packet type functions ptypetostr() and strtoptype() in lib/hci.c
> > > don't work correctly. This is because the HCI spec is messed up wrt the 2
> > > and 3 Mbps packet types, which are indicated by inverted bit logic (see
> > > "Create Connection Command" in core spec document)
> >
> > current situation is perfectly fine. It follows the spec. closely and
> > this means for EDR the types are inverted.
> 
> since the spec is somewhat twisty I don't see how it follows it at all :)
> 
> > Trying to make them looks nice doesn't help. You should not mess with
> > packet types at all. It is the link managers' job to get this right.
> 
> I am not messing with packet types at all. I apologise because I don't use
> Linux or the BlueZ tools in general but it seems that hcidump displays
> "Create Connection" packets (at least) incorrectly because of this. For
> instance
> 
> 	hci_ptypetostr(HCI_DH1)
> 
> will return
> 
> 	"DH1 "
> 
> which seems normal. But, when this is part of a HCI packet (eg Create
> Connection command), that result should actually be:
> 
> 	"DH1 2-DH1 2-DH3 2-DH5 3-DH1 3-DH3 3-DH5 "
> 
> Should it be handled in hcidump instead, as below?

the Create Connection displays the actual bits set. And that is how it
suppose to be. The above line means EDR disabled. You set the bitmask
and that the meaning of some bits is disabled instead of enabled is a
different story.

Regards

Marcel

Re: "packet types" wrong in libbluetooth

[Iain Hibbert]

On Sat, 10 Jan 2009, Marcel Holtmann wrote:
> the Create Connection displays the actual bits set. And that is how it
> suppose to be. The above line means EDR disabled. You set the bitmask
> and that the meaning of some bits is disabled instead of enabled is a
> different story.

Well I see what you are saying

< HCI Command: Create Connection (0x01|0x0005) plen 13
    bdaddr 00:19:63:B4:99:17 ptype 0xcc18 rswitch 0x01 clkoffset 0x0000
    Packet type: DM1 DM3 DM5 DH1 DH3 DH5

The actual bitwise value is clearly shown as "0xcc18". That the human
readable output does not show the correct meaning by design seems weird.

(imho of course :)
iain