[PATCHv6 hcidump 0/4] decode fixed channels

[PATCHv6 hcidump 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
	v4: typecast => format specifier (%d -> %llu)
	v5: remove typecasts and use direct access to 64 bit value.
	v6: add get_uint64 helper

Andrei Emeltchenko (4):
  add btohll macro
  add fixed channel definitions
  get_uint64 helper
  decode fixed channel list info rsp

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

-- 
1.7.4.1

[PATCHv6 hcidump 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

[PATCHv6 hcidump 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

Re: [PATCHv6 hcidump 2/4] add fixed channel definitions

[Johan Hedberg]

Hi Andrei,

On Thu, Oct 20, 2011, Emeltchenko Andrei wrote:
> ---
>  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;

The first two patches have been applied.

For the rest, like Marcel suggested, I think it'd be a good idea to have
proper get_{le,be}{16,32,64} functions. Since we can use these both in
hcidump and bluez I suppose libbluetooth is the appropriate place for
them, however in that case (e.g. if you put them in lib/bluetooth.h) a
_bt prefix should be added.

Johan

Re: [PATCHv6 hcidump 2/4] add fixed channel definitions

[Emeltchenko Andrei]

Hi Johan,

On Fri, Oct 21, 2011 at 11:36:53AM +0300, Johan Hedberg wrote:
> Hi Andrei,
> 
> On Thu, Oct 20, 2011, Emeltchenko Andrei wrote:
> > ---
> >  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;
> 
> The first two patches have been applied.
> 
> For the rest, like Marcel suggested, I think it'd be a good idea to have
> proper get_{le,be}{16,32,64} functions. Since we can use these both in
> hcidump and bluez I suppose libbluetooth is the appropriate place for
> them, however in that case (e.g. if you put them in lib/bluetooth.h) a
> _bt prefix should be added.

Would the following code be OK?

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index b0680e2..158103b 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -125,6 +125,30 @@ do {                                               \
        __p->__v = (val);                       \
 } while(0)
 
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+inline uint64_t bt_get_le64(void *ptr)
+{
+       return bt_get_unaligned((uint64_t *) ptr);
+}
+
+inline uint64_t bt_get_be64(void *ptr)
+{
+       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
+}
+#elif __BYTE_ORDER == __BIG_ENDIAN
+inline uint64_t bt_get_le64(void *ptr)
+{
+       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
+}
+
+inline uint64_t bt_get_be64(void *ptr)
+{
+       return bt_get_unaligned((uint64_t *) ptr);
+}
+#else
+#error "Unknown byte order"
+#endif
+
 /* BD Address */
 typedef struct {
        uint8_t b[6];


Regards,
Andrei

Re: [PATCHv6 hcidump 2/4] add fixed channel definitions

[Johan Hedberg]

Hi Andrei,

On Fri, Oct 21, 2011, Emeltchenko Andrei wrote:
> Would the following code be OK?
> 
> diff --git a/lib/bluetooth.h b/lib/bluetooth.h
> index b0680e2..158103b 100644
> --- a/lib/bluetooth.h
> +++ b/lib/bluetooth.h
> @@ -125,6 +125,30 @@ do {                                               \
>         __p->__v = (val);                       \
>  } while(0)
>  
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> +inline uint64_t bt_get_le64(void *ptr)
> +{
> +       return bt_get_unaligned((uint64_t *) ptr);
> +}
> +
> +inline uint64_t bt_get_be64(void *ptr)
> +{
> +       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> +}
> +#elif __BYTE_ORDER == __BIG_ENDIAN
> +inline uint64_t bt_get_le64(void *ptr)
> +{
> +       return bswap_64(bt_get_unaligned((uint64_t *) ptr));
> +}
> +
> +inline uint64_t bt_get_be64(void *ptr)
> +{
> +       return bt_get_unaligned((uint64_t *) ptr);
> +}
> +#else
> +#error "Unknown byte order"
> +#endif
> +

Yes, I think that would be fine, but please also add the same for 32 and
16 bit integers.

Johan

[PATCHv6 hcidump 3/4] get_uint64 helper

[Emeltchenko Andrei]

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

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

diff --git a/parser/parser.h b/parser/parser.h
index e975808..25aee09 100644
--- a/parser/parser.h
+++ b/parser/parser.h
@@ -193,6 +193,11 @@ static inline uint64_t get_u64(struct frame *frm)
 	return u64;
 }
 
+static inline uint64_t get_uint64(void *ptr)
+{
+	return btohll(bt_get_unaligned((uint64_t *) ptr));
+}
+
 static inline void get_u128(struct frame *frm, uint64_t *l, uint64_t *h)
 {
 	*h = get_u64(frm);
-- 
1.7.4.1

[PATCHv6 hcidump 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 |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/parser/l2cap.c b/parser/l2cap.c
index 6a5a4b2..4fc77e3 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;
@@ -728,6 +737,7 @@ 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 fc_mask;
 	int i;
 
 	p_indent(level, 0);
@@ -747,7 +757,14 @@ static void info_opt(int level, int type, void *ptr, int len)
 				}
 		break;
 	case 0x0003:
-		printf("Fixed channel list\n");
+		fc_mask = get_uint64(ptr);
+		printf("Fixed channel list 0x%8.8llx\n", fc_mask);
+		if (parser.flags & DUMP_VERBOSE)
+			for (i=0; l2cap_fix_chan[i].name; i++)
+				if (fc_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