[PATCHv3 0/4] decode fixed channels

[PATCHv3 0/4] decode fixed channels

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Decode fixed channels in L2CAP Information Response

Changes:
	v2: split headers to different patch per request
	v3: separate patch adding 64 bit get_val, fixing issues
	with type conversion

Andrei Emeltchenko (4):
  add btohll macro
  add fixed channel definitions
  add 64 bit get_val support
  decode fixed channel list info rsp

 lib/bluetooth.h |    4 ++++
 lib/l2cap.h     |    5 +++++
 parser/l2cap.c  |   40 +++++++++++++++++++++++++++++-----------
 3 files changed, 38 insertions(+), 11 deletions(-)

-- 
1.7.4.1

[PATCHv3 1/4] add btohll macro

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

handle 64 bit swap
---
 lib/bluetooth.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 738e07a..b0680e2 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -93,13 +93,17 @@ enum {
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define htobs(d)  (d)
 #define htobl(d)  (d)
+#define htobll(d) (d)
 #define btohs(d)  (d)
 #define btohl(d)  (d)
+#define btohll(d) (d)
 #elif __BYTE_ORDER == __BIG_ENDIAN
 #define htobs(d)  bswap_16(d)
 #define htobl(d)  bswap_32(d)
+#define htobll(d) bswap_64(d)
 #define btohs(d)  bswap_16(d)
 #define btohl(d)  bswap_32(d)
+#define btohll(d) bswap_64(d)
 #else
 #error "Unknown byte order"
 #endif
-- 
1.7.4.1

[PATCHv3 2/4] add fixed channel definitions

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 lib/l2cap.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/lib/l2cap.h b/lib/l2cap.h
index 47b3dc3..3880551 100644
--- a/lib/l2cap.h
+++ b/lib/l2cap.h
@@ -139,6 +139,11 @@ struct l2cap_conninfo {
 
 #define L2CAP_SDULEN_SIZE	2
 
+/* L2CAP fixed channels */
+#define L2CAP_FC_L2CAP		0x02
+#define L2CAP_FC_CONNLESS	0x04
+#define L2CAP_FC_A2MP		0x08
+
 /* L2CAP structures */
 typedef struct {
 	uint16_t	len;
-- 
1.7.4.1

[PATCHv3 3/4] add 64 bit get_val support

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 parser/l2cap.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/parser/l2cap.c b/parser/l2cap.c
index 6a5a4b2..e1fc1f8 100644
--- a/parser/l2cap.c
+++ b/parser/l2cap.c
@@ -232,7 +232,7 @@ static uint8_t get_ext_ctrl(int in, uint16_t handle, uint16_t cid)
 	return 0;
 }
 
-static uint32_t get_val(uint8_t *ptr, uint8_t len)
+static uint64_t get_val(uint8_t *ptr, uint8_t len)
 {
 	switch (len) {
 	case 1:
@@ -241,6 +241,8 @@ static uint32_t get_val(uint8_t *ptr, uint8_t len)
 		return btohs(bt_get_unaligned((uint16_t *) ptr));
 	case 4:
 		return btohl(bt_get_unaligned((uint32_t *) ptr));
+	case 8:
+		return btohll(bt_get_unaligned((uint64_t *) ptr));
 	}
 	return 0;
 }
@@ -409,7 +411,7 @@ static inline void command_rej(int level, struct frame *frm)
 
 	switch (reason) {
 	case 0x0001:
-		printf(" mtu %d\n", get_val(frm->ptr + L2CAP_CMD_REJ_SIZE, 2));
+		printf(" mtu %d\n", (uint16_t) get_val(frm->ptr + L2CAP_CMD_REJ_SIZE, 2));
 		break;
 	case 0x0002:
 		cid = get_val(frm->ptr + L2CAP_CMD_REJ_SIZE, 4);
@@ -552,13 +554,13 @@ static void conf_opt(int level, void *ptr, int len, int in, uint16_t handle,
 			set_mode(in, handle, cid, 0x00);
 			printf("MTU");
 			if (h->len > 0)
-				printf(" %d", get_val(h->val, h->len));
+				printf(" %d", (uint16_t) get_val(h->val, h->len));
 			break;
 
 		case L2CAP_CONF_FLUSH_TO:
 			printf("FlushTO");
 			if (h->len > 0)
-				printf(" %d", get_val(h->val, h->len));
+				printf(" %d", (uint16_t) get_val(h->val, h->len));
 			break;
 
 		case L2CAP_CONF_QOS:
@@ -582,7 +584,7 @@ static void conf_opt(int level, void *ptr, int len, int in, uint16_t handle,
 		case L2CAP_CONF_EWS:
 			printf("EWS");
 			if (h->len > 0)
-				printf(" %d", get_val(h->val, h->len));
+				printf(" %d", (uint16_t) get_val(h->val, h->len));
 			set_ext_ctrl(in, handle, cid, 1);
 			break;
 
@@ -727,18 +729,18 @@ static inline void echo_rsp(int level, l2cap_cmd_hdr *cmd, struct frame *frm)
 
 static void info_opt(int level, int type, void *ptr, int len)
 {
-	uint32_t mask;
+	uint64_t mask;
 	int i;
 
 	p_indent(level, 0);
 
 	switch (type) {
 	case 0x0001:
-		printf("Connectionless MTU %d\n", get_val(ptr, len));
+		printf("Connectionless MTU %d\n", (uint16_t) get_val(ptr, len));
 		break;
 	case 0x0002:
 		mask = get_val(ptr, len);
-		printf("Extended feature mask 0x%4.4x\n", mask);
+		printf("Extended feature mask 0x%4.4x\n", (uint32_t) mask);
 		if (parser.flags & DUMP_VERBOSE)
 			for (i=0; l2cap_features[i].name; i++)
 				if (mask & l2cap_features[i].flag) {
-- 
1.7.4.1

[PATCHv3 4/4] decode fixed channel list info rsp

[Emeltchenko Andrei]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Decode fixed channels in information response

< ACL data: handle 1 flags 0x00 dlen 10
    L2CAP(s): Info req: type 3
> ACL data: handle 1 flags 0x02 dlen 20
    L2CAP(s): Info rsp: type 3 result 0
      Fixed channel list 0x0000000a
        L2CAP Signalling Channel
        AMP Manager Protocol
---
 parser/l2cap.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/parser/l2cap.c b/parser/l2cap.c
index e1fc1f8..3597058 100644
--- a/parser/l2cap.c
+++ b/parser/l2cap.c
@@ -61,10 +61,12 @@ static cid_info cid_table[2][CID_TABLE_SIZE];
 #define DCID cid_table[1]
 
 /* Can we move this to l2cap.h? */
-static struct {
+struct features {
 	char	*name;
 	int	flag;
-} l2cap_features[] = {
+};
+
+static struct features l2cap_features[] = {
 	{ "Flow control mode",			L2CAP_FEAT_FLOWCTL	},
 	{ "Retransmission mode",		L2CAP_FEAT_RETRANS	},
 	{ "Bi-directional QoS",			L2CAP_FEAT_BIDIR_QOS	},
@@ -78,6 +80,13 @@ static struct {
 	{ 0 }
 };
 
+static struct features l2cap_fix_chan[] = {
+	{ "L2CAP Signalling Channel",		L2CAP_FC_L2CAP		},
+	{ "L2CAP Connless",			L2CAP_FC_CONNLESS	},
+	{ "AMP Manager Protocol",		L2CAP_FC_A2MP		},
+	{ 0 }
+};
+
 static struct frame *add_handle(uint16_t handle)
 {
 	register handle_info *t = handle_table;
@@ -749,7 +758,14 @@ static void info_opt(int level, int type, void *ptr, int len)
 				}
 		break;
 	case 0x0003:
-		printf("Fixed channel list\n");
+		mask = get_val(ptr, len);
+		printf("Fixed channel list 0x%8.8llx\n", mask);
+		if (parser.flags & DUMP_VERBOSE)
+			for (i=0; l2cap_fix_chan[i].name; i++)
+				if (mask & l2cap_fix_chan[i].flag) {
+					p_indent(level + 1, 0);
+					printf("%s\n", l2cap_fix_chan[i].name);
+				}
 		break;
 	default:
 		printf("Unknown (len %d)\n", len);
-- 
1.7.4.1

Re: [PATCHv3 2/4] add fixed channel definitions

[Marcel Holtmann]

Hi Andrei,

>  lib/l2cap.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/l2cap.h b/lib/l2cap.h
> index 47b3dc3..3880551 100644
> --- a/lib/l2cap.h
> +++ b/lib/l2cap.h
> @@ -139,6 +139,11 @@ struct l2cap_conninfo {
>  
>  #define L2CAP_SDULEN_SIZE	2
>  
> +/* L2CAP fixed channels */
> +#define L2CAP_FC_L2CAP		0x02
> +#define L2CAP_FC_CONNLESS	0x04
> +#define L2CAP_FC_A2MP		0x08
> +

please get these added to bluez.git first and then send exactly the same
patch to add them here (once accepted).

Also please add also LE, LE signaling, AMP test manager as well. Just
make sure the list is complete according to 4.0 specification and
assigned numbers list.

Regards

Marcel

Re: [PATCHv3 3/4] add 64 bit get_val support

[Marcel Holtmann]

Hi Andrei,

>  parser/l2cap.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/parser/l2cap.c b/parser/l2cap.c
> index 6a5a4b2..e1fc1f8 100644
> --- a/parser/l2cap.c
> +++ b/parser/l2cap.c
> @@ -232,7 +232,7 @@ static uint8_t get_ext_ctrl(int in, uint16_t handle, uint16_t cid)
>  	return 0;
>  }
>  
> -static uint32_t get_val(uint8_t *ptr, uint8_t len)
> +static uint64_t get_val(uint8_t *ptr, uint8_t len)
>  {
>  	switch (len) {
>  	case 1:
> @@ -241,6 +241,8 @@ static uint32_t get_val(uint8_t *ptr, uint8_t len)
>  		return btohs(bt_get_unaligned((uint16_t *) ptr));
>  	case 4:
>  		return btohl(bt_get_unaligned((uint32_t *) ptr));
> +	case 8:
> +		return btohll(bt_get_unaligned((uint64_t *) ptr));
>  	}
>  	return 0;
>  }

turning everything into a uint64 is a bit heavy. Just change this into
broken down version. get_uint64, get_uint32, get_uint16 etc. Otherwise
you keep casting yourself crazy.

Regards

Marcel

Re: [PATCHv3 1/4] add btohll macro

[Marcel Holtmann]

Hi Andrei,

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> handle 64 bit swap

get this first merged into bluez.git and then backport it.

Regards

Marcel

Re: [PATCHv3 3/4] add 64 bit get_val support

[Emeltchenko Andrei]

Hi Marcel,

On Mon, Oct 17, 2011 at 08:31:08AM -0700, Marcel Holtmann wrote:
> Hi Andrei,
> 
> >  parser/l2cap.c |   18 ++++++++++--------
> >  1 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/parser/l2cap.c b/parser/l2cap.c
> > index 6a5a4b2..e1fc1f8 100644
> > --- a/parser/l2cap.c
> > +++ b/parser/l2cap.c
> > @@ -232,7 +232,7 @@ static uint8_t get_ext_ctrl(int in, uint16_t handle, uint16_t cid)
> >  	return 0;
> >  }
> >  
> > -static uint32_t get_val(uint8_t *ptr, uint8_t len)
> > +static uint64_t get_val(uint8_t *ptr, uint8_t len)
> >  {
> >  	switch (len) {
> >  	case 1:
> > @@ -241,6 +241,8 @@ static uint32_t get_val(uint8_t *ptr, uint8_t len)
> >  		return btohs(bt_get_unaligned((uint16_t *) ptr));
> >  	case 4:
> >  		return btohl(bt_get_unaligned((uint32_t *) ptr));
> > +	case 8:
> > +		return btohll(bt_get_unaligned((uint64_t *) ptr));
> >  	}
> >  	return 0;
> >  }
> 
> turning everything into a uint64 is a bit heavy. Just change this into
> broken down version. get_uint64, get_uint32, get_uint16 etc. Otherwise
> you keep casting yourself crazy.

Yes, casting does not look nice. What about if I keep everything AS IS and
just use btohll(bt_get_unaligned((ptr)) to access 64 bit value.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv3 3/4] add 64 bit get_val support

[Marcel Holtmann]

Hi Andrei,

> > >  parser/l2cap.c |   18 ++++++++++--------
> > >  1 files changed, 10 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/parser/l2cap.c b/parser/l2cap.c
> > > index 6a5a4b2..e1fc1f8 100644
> > > --- a/parser/l2cap.c
> > > +++ b/parser/l2cap.c
> > > @@ -232,7 +232,7 @@ static uint8_t get_ext_ctrl(int in, uint16_t handle, uint16_t cid)
> > >  	return 0;
> > >  }
> > >  
> > > -static uint32_t get_val(uint8_t *ptr, uint8_t len)
> > > +static uint64_t get_val(uint8_t *ptr, uint8_t len)
> > >  {
> > >  	switch (len) {
> > >  	case 1:
> > > @@ -241,6 +241,8 @@ static uint32_t get_val(uint8_t *ptr, uint8_t len)
> > >  		return btohs(bt_get_unaligned((uint16_t *) ptr));
> > >  	case 4:
> > >  		return btohl(bt_get_unaligned((uint32_t *) ptr));
> > > +	case 8:
> > > +		return btohll(bt_get_unaligned((uint64_t *) ptr));
> > >  	}
> > >  	return 0;
> > >  }
> > 
> > turning everything into a uint64 is a bit heavy. Just change this into
> > broken down version. get_uint64, get_uint32, get_uint16 etc. Otherwise
> > you keep casting yourself crazy.
> 
> Yes, casting does not look nice. What about if I keep everything AS IS and
> just use btohll(bt_get_unaligned((ptr)) to access 64 bit value.

this does not make it more readable. Create helpers like get_uint64 for
it.

Regards

Marcel

Re: [PATCHv3 3/4] add 64 bit get_val support

[Emeltchenko Andrei]

Hi Marcel,

On Tue, Oct 18, 2011 at 08:53:51AM -0700, Marcel Holtmann wrote:
> > > > +	case 8:
> > > > +		return btohll(bt_get_unaligned((uint64_t *) ptr));
> > > >  	}
> > > >  	return 0;
> > > >  }
> > > 
> > > turning everything into a uint64 is a bit heavy. Just change this into
> > > broken down version. get_uint64, get_uint32, get_uint16 etc. Otherwise
> > > you keep casting yourself crazy.
> > 
> > Yes, casting does not look nice. What about if I keep everything AS IS and
> > just use btohll(bt_get_unaligned((ptr)) to access 64 bit value.
> 
> this does not make it more readable. Create helpers like get_uint64 for
> it.

Is this OK:

--- a/parser/parser.h
+++ b/parser/parser.h
...
+static inline uint64_t get_uint64(void *ptr)
+{
+       return btohll(bt_get_unaligned((uint64_t *) ptr));
+}
...

--- a/parser/l2cap.c
+++ b/parser/l2cap.c
...
        case 0x0003:
-               fc_mask = btohll(bt_get_unaligned((uint64_t *) ptr));
+               fc_mask = get_uint64(ptr);
...

Best regards 
Andrei Emeltchenko 
 

Re: [PATCHv3 3/4] add 64 bit get_val support

[Marcel Holtmann]

Hi Andrei,

> > > > > +	case 8:
> > > > > +		return btohll(bt_get_unaligned((uint64_t *) ptr));
> > > > >  	}
> > > > >  	return 0;
> > > > >  }
> > > > 
> > > > turning everything into a uint64 is a bit heavy. Just change this into
> > > > broken down version. get_uint64, get_uint32, get_uint16 etc. Otherwise
> > > > you keep casting yourself crazy.
> > > 
> > > Yes, casting does not look nice. What about if I keep everything AS IS and
> > > just use btohll(bt_get_unaligned((ptr)) to access 64 bit value.
> > 
> > this does not make it more readable. Create helpers like get_uint64 for
> > it.
> 
> Is this OK:
> 
> --- a/parser/parser.h
> +++ b/parser/parser.h
> ...
> +static inline uint64_t get_uint64(void *ptr)
> +{
> +       return btohll(bt_get_unaligned((uint64_t *) ptr));
> +}
> ...

that would be fine with me.

And if you wanna go all out, then you could fix the weird SDP big-endian
conversion/handling that is lingering around the source code and all
parsers for a while now.

Maybe just something like get_le64, get_le32, get_be32 etc. Think about
it a little bit and if you are up for some major cleanup task.

Regards

Marcel