* [PATCH 20/22] Add EFS & EWS config parameters for ERTM & Streaming modes in l2cap_build_conf_req
From: haijun liu @ 2010-08-25 22:40 UTC (permalink / raw)
To: linux-bluetooth
>From 74b6581d7fdb0d3e5c0cd9546b15d58357c3758c Mon Sep 17 00:00:00 2001
From: haijun.liu <haijun.liu@atheros.com>
Date: Tue, 24 Aug 2010 15:52:49 +0800
Subject: [PATCH 20/22] Add EFS & EWS config parameters for ERTM &
Streaming modes in l2cap_build_conf_req.
---
net/bluetooth/l2cap.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e7f7067..73142ae 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2500,6 +2500,7 @@ static int l2cap_build_conf_req(struct sock *sk,
void *data)
struct l2cap_pinfo *pi = l2cap_pi(sk);
struct l2cap_conf_req *req = data;
struct l2cap_conf_rfc rfc = { .mode = pi->mode };
+ struct l2cap_conf_efs efs = { .service_type = L2CAP_SERVTYPE_BESTEFFORT };
void *ptr = req->data;
BT_DBG("sk %p", sk);
@@ -2510,10 +2511,25 @@ static int l2cap_build_conf_req(struct sock
*sk, void *data)
switch (pi->mode) {
case L2CAP_MODE_STREAMING:
case L2CAP_MODE_ERTM:
- if (pi->conf_state & L2CAP_CONF_STATE2_DEVICE)
- break;
+ pi->conf_state |= L2CAP_CONF_STATE2_DEVICE;
- /* fall through */
+ if (enable_highspeed && pi->hschan_req) {
+ pi->ext_flowspec_enable = 1;
+
+ pi->loc_efs.service_type = (!pi->guaranteed) ?
+ L2CAP_SERVTYPE_BESTEFFORT : L2CAP_SERVTYPE_GUARANTEED;
+ pi->loc_efs.max_sdu_size = L2CAP_DEFAULT_MAX_SDU_SIZE;
+ pi->loc_efs.sdu_inter_time = L2CAP_DEFAULT_SDU_ARRIVAL_TIME;
+ pi->loc_efs.access_latency = L2CAP_DEFAULT_ACCESS_LATENCY;
+ pi->loc_efs.flush_timeout = L2CAP_DEFAULT_FLUSH_TIMEOUT;
+ }
+ if (enable_highspeed && (pi->conn->feat_mask & L2CAP_FEAT_EXT_WINSIZE)) {
+ pi->extwin_enable = 1;
+ pi->extwin_size = L2CAP_DEFAULT_EXT_WINSIZE;
+ }
+ if (!l2cap_mode_supported(pi->mode, pi->conn->feat_mask))
+ l2cap_send_disconn_req(pi->conn, sk, ECONNRESET);
+ break;
default:
pi->mode = l2cap_select_mode(rfc.mode, pi->conn->feat_mask);
break;
@@ -2552,6 +2568,33 @@ done:
pi->fcs = L2CAP_FCS_NONE;
l2cap_add_conf_opt(&ptr, L2CAP_CONF_FCS, 1, pi->fcs);
}
+
+ if (pi->ext_flowspec_enable) {
+ efs.service_type = pi->loc_efs.service_type;
+ efs.max_sdu_size = cpu_to_le16(pi->loc_efs.max_sdu_size);
+ efs.sdu_inter_time = cpu_to_le32(pi->loc_efs.sdu_inter_time);
+ if (pi->loc_efs.service_type == L2CAP_SERVTYPE_BESTEFFORT) {
+ pi->loc_efs.id = 1;
+ efs.id = pi->loc_efs.id;
+ efs.access_latency = cpu_to_le32(0xFFFFFFFF);
+ efs.flush_timeout = cpu_to_le32(0xFFFFFFFF);
+ } else {
+ /* As spec, the ident shall be unique within
+ the scope of a physical link.
+ */
+ pi->loc_efs.id = l2cap_flowspec_ident(l2cap_pi(sk)->conn);
+ efs.id = pi->loc_efs.id;
+ efs.access_latency = cpu_to_le32(pi->loc_efs.access_latency);
+ efs.flush_timeout = cpu_to_le32(pi->loc_efs.flush_timeout);
+ }
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+ }
+
+ if (pi->extwin_enable) {
+ pi->extwin_size = max_t(__u16, pi->extwin_size, 0x01);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 4, pi->extwin_size);
+ }
break;
case L2CAP_MODE_STREAMING:
@@ -2572,6 +2615,24 @@ done:
pi->fcs = L2CAP_FCS_NONE;
l2cap_add_conf_opt(&ptr, L2CAP_CONF_FCS, 1, pi->fcs);
}
+
+ if (pi->ext_flowspec_enable) {
+ efs.id = 1;
+ efs.service_type = L2CAP_SERVTYPE_BESTEFFORT;
+ efs.max_sdu_size = cpu_to_le16(pi->loc_efs.max_sdu_size);
+ efs.sdu_inter_time = cpu_to_le32(pi->loc_efs.sdu_inter_time);
+ efs.access_latency = 0;
+ efs.flush_timeout = 0;
+
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+ }
+
+ if (pi->extwin_enable) {
+ pi->extwin_enable = 1;
+ pi->extwin_size = 0;
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 4, pi->extwin_size);
+ }
break;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 21/22] Parse EFS & EWS parameters for in l2cap_parse_conf_req
From: haijun liu @ 2010-08-25 22:40 UTC (permalink / raw)
To: linux-bluetooth
>From c3c9a97f3dfda527da2f9c4893302cde56e24198 Mon Sep 17 00:00:00 2001
From: haijun.liu <haijun.liu@atheros.com>
Date: Wed, 25 Aug 2010 21:59:19 +0800
Subject: [PATCH 21/22] Parse EFS & EWS parameters for in l2cap_parse_conf_req.
---
net/bluetooth/l2cap.c | 100 +++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 93 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 73142ae..d66d923 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2660,6 +2660,9 @@ static int l2cap_parse_conf_req(struct sock *sk,
void *data)
unsigned long val;
struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_BASIC };
u16 mtu = L2CAP_DEFAULT_MTU;
+ struct l2cap_conf_efs efs = { .service_type = L2CAP_SERVTYPE_BESTEFFORT };
+ u8 ext_flowspec_enable = 0;
+ u16 extwin_size = 0;
u16 result = L2CAP_CONF_SUCCESS;
BT_DBG("sk %p", sk);
@@ -2693,6 +2696,17 @@ static int l2cap_parse_conf_req(struct sock
*sk, void *data)
break;
+ case L2CAP_CONF_EFS:
+ /* handle the extended flow spec mode */
+ ext_flowspec_enable = 1;
+ if (olen == sizeof(efs))
+ memcpy(&efs, (void *) val, olen);
+ break;
+
+ case L2CAP_CONF_EWS:
+ extwin_size = val;
+ break;
+
default:
if (hint)
break;
@@ -2709,16 +2723,29 @@ static int l2cap_parse_conf_req(struct sock
*sk, void *data)
switch (pi->mode) {
case L2CAP_MODE_STREAMING:
case L2CAP_MODE_ERTM:
- if (!(pi->conf_state & L2CAP_CONF_STATE2_DEVICE)) {
- pi->mode = l2cap_select_mode(rfc.mode,
- pi->conn->feat_mask);
- break;
+ pi->conf_state |= L2CAP_CONF_STATE2_DEVICE;
+ if (enable_highspeed && ext_flowspec_enable && pi->hschan_req) {
+ pi->ext_flowspec_enable = 1;
+
+ pi->loc_efs.service_type = (!pi->guaranteed) ?
+ L2CAP_SERVTYPE_BESTEFFORT : L2CAP_SERVTYPE_GUARANTEED;
+ pi->loc_efs.max_sdu_size = L2CAP_DEFAULT_MAX_SDU_SIZE;
+ pi->loc_efs.sdu_inter_time = L2CAP_DEFAULT_SDU_ARRIVAL_TIME;
+ pi->loc_efs.access_latency = L2CAP_DEFAULT_ACCESS_LATENCY;
+ pi->loc_efs.flush_timeout = L2CAP_DEFAULT_FLUSH_TIMEOUT;
+ }
+ if (enable_highspeed && (pi->conn->feat_mask & L2CAP_FEAT_EXT_WINSIZE)) {
+ pi->extwin_enable = 1;
+ pi->extwin_size = L2CAP_DEFAULT_EXT_WINSIZE;
}
- if (pi->mode != rfc.mode)
+ if (!l2cap_mode_supported(pi->mode, pi->conn->feat_mask))
return -ECONNREFUSED;
break;
+ default:
+ pi->mode = l2cap_select_mode(sk, rfc.mode, pi->conn->feat_mask);
+ break;
}
done:
@@ -2733,8 +2760,36 @@ done:
sizeof(rfc), (unsigned long) &rfc);
}
+ /*
+ Add the ext flow spec and ext win size option parameters check here.
+ */
+
+ if (result == L2CAP_CONF_SUCCESS && ext_flowspec_enable) {
+ if (!pi->ext_flowspec_enable) {
+ /* remote support , local not support, refused */
+
+ result = L2CAP_CONF_REJECT;
+ return -ECONNREFUSED;
+ } else if (pi->loc_efs.service_type != L2CAP_SERVTYPE_NOTRAFIC &&
+ efs.service_type != L2CAP_SERVTYPE_NOTRAFIC &&
+ efs.service_type != pi->loc_efs.service_type) {
+
+ result = L2CAP_CONF_UNACCEPT;
+
+ if (pi->num_conf_req >= 1)
+ return -ECONNREFUSED;
+
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+
+ } else {
+ result = L2CAP_CONF_PENDING;
+ pi->conf_state |= L2CAP_CONF_LOCAL_PEND;
+ }
+ }
- if (result == L2CAP_CONF_SUCCESS) {
+
+ if (result == L2CAP_CONF_SUCCESS || result == L2CAP_CONF_PENDING) {
/* Configure output options and let the other side know
* which ones we don't like. */
@@ -2770,6 +2825,22 @@ done:
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC,
sizeof(rfc), (unsigned long) &rfc);
+ if (pi->ext_flowspec_enable) {
+ /* record the extend flow spec parameters */
+ pi->rem_efs.id = efs.id;
+ pi->rem_efs.service_type = efs.service_type;
+ pi->rem_efs.max_sdu_size = le16_to_cpu(efs.max_sdu_size);
+ pi->rem_efs.flush_timeout = le16_to_cpu(efs.flush_timeout);
+ pi->rem_efs.access_latency = le16_to_cpu(efs.access_latency);
+ pi->rem_efs.sdu_inter_time = le16_to_cpu(efs.sdu_inter_time);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+ }
+ if (extwin_size > 0) {
+ pi->rem_extwin_enable = 1;
+ pi->rem_extwin_size = extwin_size;
+ pi->conf_state |= L2CAP_CONF_EXTWINSIZE_DONE;
+ }
break;
case L2CAP_MODE_STREAMING:
@@ -2782,7 +2853,22 @@ done:
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC,
sizeof(rfc), (unsigned long) &rfc);
-
+ if (pi->ext_flowspec_enable) {
+ /* record the extend flow spec parameters */
+ pi->rem_efs.id = efs.id;
+ pi->rem_efs.service_type = efs.service_type;
+ pi->rem_efs.max_sdu_size = le16_to_cpu(efs.max_sdu_size);
+ pi->rem_efs.flush_timeout = le16_to_cpu(efs.flush_timeout);
+ pi->rem_efs.access_latency = le16_to_cpu(efs.access_latency);
+ pi->rem_efs.sdu_inter_time = le16_to_cpu(efs.sdu_inter_time);
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+ }
+ if (extwin_size == 0) {
+ pi->rem_extwin_enable = 1;
+ pi->rem_extwin_size = extwin_size;
+ pi->conf_state |= L2CAP_CONF_EXTWINSIZE_DONE;
+ }
break;
default:
--
1.6.3.3
^ permalink raw reply related
* [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp
From: haijun liu @ 2010-08-25 22:41 UTC (permalink / raw)
To: linux-bluetooth
>From 72afc40895eb272269156b98ab3024ac4f130b0d Mon Sep 17 00:00:00 2001
From: haijun.liu <haijun.liu@atheros.com>
Date: Wed, 25 Aug 2010 22:08:47 +0800
Subject: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp.
---
net/bluetooth/l2cap.c | 35 ++++++++++++++++++++++++++++++-----
1 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index d66d923..e21aed0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2896,6 +2896,7 @@ static int l2cap_parse_conf_rsp(struct sock *sk,
void *rsp, int len, void *data,
int type, olen;
unsigned long val;
struct l2cap_conf_rfc rfc;
+ struct l2cap_conf_efs efs;
BT_DBG("sk %p, rsp %p, len %d, req %p", sk, rsp, len, data);
@@ -2926,18 +2927,34 @@ static int l2cap_parse_conf_rsp(struct sock
*sk, void *rsp, int len, void *data,
rfc.mode != pi->mode)
return -ECONNREFUSED;
+ pi->mode = rfc.mode;
pi->fcs = 0;
l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC,
sizeof(rfc), (unsigned long) &rfc);
break;
- }
- }
- if (pi->mode == L2CAP_MODE_BASIC && pi->mode != rfc.mode)
- return -ECONNREFUSED;
+ case L2CAP_CONF_EFS:
+ if (olen == sizeof(efs))
+ memcpy(&efs, (void *)val, olen);
- pi->mode = rfc.mode;
+ if (pi->loc_efs.service_type != L2CAP_SERVTYPE_NOTRAFIC &&
+ efs.service_type != L2CAP_SERVTYPE_NOTRAFIC &&
+ efs.service_type != pi->loc_efs.service_type)
+ return -ECONNREFUSED;
+
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS,
+ sizeof(efs), (unsigned long) &efs);
+ break;
+
+ case L2CAP_CONF_EWS:
+ pi->extwin_size = val;
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS,
+ 2, pi->extwin_size);
+ break;
+
+ }
+ }
if (*result == L2CAP_CONF_SUCCESS) {
switch (rfc.mode) {
@@ -2946,6 +2963,14 @@ static int l2cap_parse_conf_rsp(struct sock
*sk, void *rsp, int len, void *data,
pi->retrans_timeout = le16_to_cpu(rfc.retrans_timeout);
pi->monitor_timeout = le16_to_cpu(rfc.monitor_timeout);
pi->mps = le16_to_cpu(rfc.max_pdu_size);
+ if (pi->ext_flowspec_enable) {
+ /* id, service type should not be changed */
+ pi->loc_efs.max_sdu_size = le16_to_cpu(efs.max_sdu_size);
+ pi->loc_efs.sdu_inter_time = le16_to_cpu(efs.sdu_inter_time);
+ pi->loc_efs.access_latency = le16_to_cpu(efs.access_latency);
+ pi->loc_efs.flush_timeout = le16_to_cpu(efs.flush_timeout);
+ }
+
break;
case L2CAP_MODE_STREAMING:
pi->mps = le16_to_cpu(rfc.max_pdu_size);
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp
From: Gustavo F. Padovan @ 2010-08-26 0:55 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQsNmOc-Mv30xA0x_NEpxvs4kXjGfuOfqHxEDP@mail.gmail.com>
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:41:02 +0800]:
> From 72afc40895eb272269156b98ab3024ac4f130b0d Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Wed, 25 Aug 2010 22:08:47 +0800
> Subject: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp.
>
> ---
> net/bluetooth/l2cap.c | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
For all patches here, please write a proper commit title and message in
your patches. The subject should always start with "Bluetooth:" and then
briefly describe your changes in the patch. It's recommended to not use
more than 50 letters in the subject and never use more than 72 letters.
And then you have the commit message, there you explain everything it's
needed to understand the patch.
Also you need a Signed-off-by line in the commit, and fixes your git
configuration to show your name instead of your email id.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 01/22] Add eL2cap signal macro definition
From: Gustavo F. Padovan @ 2010-08-26 0:57 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinpXB4ZXW=Oarn0qZAWWdRrr1JL1SB7mu9KSj08@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:22:27 +0800]:
> From 0ce1315590e4bfe0d344acad7e770710833795aa Mon Sep 17 00:00:00 2001
> From: haijun <haijun@haijun-laptop.(none)>
> Date: Wed, 18 Aug 2010 22:26:19 +0800
> Subject: [PATCH 01/22] Add eL2cap signal macro definition.
>
> ---
> include/net/bluetooth/l2cap.h | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
Please use L2CAP instead eL2CAP.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 02/22] Add eL2cap new features macro definition
From: Gustavo F. Padovan @ 2010-08-26 1:01 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=qOgovmY3s3PcS4tGb59HUhxx-Qydr2NyNs9Me@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:27:35 +0800]:
> From fb2a4d17435dbd6d8787e6928c1da4f12a839265 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Wed, 18 Aug 2010 22:33:22 +0800
> Subject: [PATCH 02/22] Add eL2cap new features macro definition.
>
> ---
> include/net/bluetooth/l2cap.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index bf0ea10..2e782d0 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -99,7 +99,11 @@ struct l2cap_conninfo {
> #define L2CAP_FEAT_ERTM 0x00000008
> #define L2CAP_FEAT_STREAMING 0x00000010
> #define L2CAP_FEAT_FCS 0x00000020
> +#define L2CAP_FEAT_EFS_BREDR 0x00000040
> #define L2CAP_FEAT_FIXED_CHAN 0x00000080
> +#define L2CAP_FEAT_EXT_WINSIZE 0x00000100
> +#define L2CAP_FEAT_UCDR 0x00000200
> +#define L2CAP_FEAT_RESERVED 0x80000000
We don't need the L2CAP_FEAT_RESERVED here.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 12/22] Add some default values for configuration process
From: Gustavo F. Padovan @ 2010-08-26 1:04 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikAZBO6dka+yN3tFTZekm9+1nzo3=OZEoxZ1Eab@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:35:34 +0800]:
> From 2d21459fa52b754ce0f666ccefc0175bc7f04420 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Sun, 22 Aug 2010 23:57:09 +0800
> Subject: [PATCH 12/22] Add some default values for configuration process.
>
> ---
> include/net/bluetooth/l2cap.h | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index fe411e2..d0ae9f5 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -36,6 +36,11 @@
> #define L2CAP_DEFAULT_MAX_PDU_SIZE 672
> #define L2CAP_DEFAULT_ACK_TO 200
> #define L2CAP_LOCAL_BUSY_TRIES 12
> +#define L2CAP_DEFAULT_MAX_SDU_SIZE 0xFFFF
> +#define L2CAP_DEFAULT_SDU_ARRIVAL_TIME 0xFFFFFFFF
> +#define L2CAP_DEFAULT_FLUSH_TIMEOUT 0xFFFFFFFF
We already have L2CAP_DEFAULT_FLUSH_TO with the same value.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt
From: Gustavo F. Padovan @ 2010-08-26 1:10 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=8xVks--xcLXEQ_=sHfP3fY+Wvyctu5K_kMDG-@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:36:05 +0800]:
> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> };
Here you are extending the API with the userspace, so we need a patch
for each option explaining why you need it and why the userspace should
care about that. It's a good idea here add this option to l2cap_options
in the same patch you implement the handling for it apply such patch hen
it is actually needed. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 14/22] Add three new options for l2cap_pinfo
From: Gustavo F. Padovan @ 2010-08-26 1:13 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikf=UeHUaT8wLnwUDnxgg0ouz1f6QkgYZvbr8t8@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:36:35 +0800]:
> From 06124ba36c1f6c9cf5bef861751ce749712cecb0 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:03:39 +0800
> Subject: [PATCH 14/22] Add three new options for l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 4f87aec..7a9bec4 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -367,6 +367,10 @@ struct l2cap_pinfo {
> __u8 num_conf_req;
> __u8 num_conf_rsp;
>
> + __u8 hschan_req;
> + __u8 guaranteed;
> + __u8 reconfig;
> +
The same here, you have to explain why you to increase struct l2cap_pinfo
size, and maybe only add them when you have a piece of code that uses
that.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
From: Gustavo F. Padovan @ 2010-08-26 1:21 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimnW0jk19cDG+r64Q+-HxdH2qdTBirTMB7ikr1W@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:37:30 +0800]:
> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:09:56 +0800
> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2d864d4..f2dd65d 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> __u16 remote_mps;
> __u16 mps;
>
> + __u8 ext_flowspec_enable;
> + struct ext_flow_spec loc_efs;
> + struct ext_flow_spec rem_efs;
> +
> + __u8 extwin_enable;
> + __u16 extwin_size;
> + __u8 rem_extwin_enable;
> + __u16 rem_extwin_size;
The same comment in 14/22 goes here and others patches in the patch set.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp
From: Gustavo F. Padovan @ 2010-08-26 1:26 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQsNmOc-Mv30xA0x_NEpxvs4kXjGfuOfqHxEDP@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:41:02 +0800]:
> From 72afc40895eb272269156b98ab3024ac4f130b0d Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Wed, 25 Aug 2010 22:08:47 +0800
> Subject: [PATCH 22/22] Parse EFS & EWS parameters for in l2cap_parse_conf_rsp.
>
> ---
> net/bluetooth/l2cap.c | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
For 20, 21 and 22. Create a separeted patches for EFS and EWS, they are
different stuff and do not overstep the column 80 when coding.
Fix that and the we can do another review ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 17/22] Add parameter enable_highspeed to control highspeed code in running phase
From: Gustavo F. Padovan @ 2010-08-26 1:28 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinaFobBK5OiFaV+fGsEe3JRTy7_dT-FFLjSt-Rq@mail.gmail.com>
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:38:27 +0800]:
> From c3fba9b5d7546576d9eb1d30eb45c453c8ee1dff Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:17:24 +0800
> Subject: [PATCH 17/22] Add parameter enable_highspeed to control
> highspeed code in running phase.
>
> ---
> net/bluetooth/l2cap.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 449cbdd..e7f7067 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -4851,6 +4851,9 @@ module_exit(l2cap_exit);
> module_param(enable_ertm, bool, 0644);
> MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");
>
> +module_param(enable_highspeed, bool, 0644);
> +MODULE_PARM_DESC(enable_highspeed, "Enable Bluetooth V3.0+HS
> highspeed feature");
I did not run this code, but I'm sure that this patch doesn't compile. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* New subscription request to list Bluez-devel from tang705@yahoo.com.cn
From: bluez-devel-owner @ 2010-08-26 2:57 UTC (permalink / raw)
To: bluez-devel-owner
Your authorization is required for a mailing list subscription request
approval:
For: tang705@yahoo.com.cn
List: bluez-devel@lists.sourceforge.net
At your convenience, visit:
https://lists.sourceforge.net/lists/admindb/bluez-devel
to process the request.
^ permalink raw reply
* Re: [PATCH 02/22] Add eL2cap new features macro definition
From: haijun liu @ 2010-08-26 6:59 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100826010116.GC16062@vigoh>
Hi Gustavo,
On Thu, Aug 26, 2010 at 9:01 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
>
> We don't need the L2CAP_FEAT_RESERVED here.
>
Thank you, we'll remove it.
^ permalink raw reply
* [PATCH] Fix handling empty fields in VCARDs
From: Lukasz Pawlik @ 2010-08-26 7:45 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-Fix-handling-empty-fields-in-VCARDs.patch --]
[-- Type: text/x-patch, Size: 5038 bytes --]
From 4bb29cdaf4a9923ab62973c38914ebb67f718d64 Mon Sep 17 00:00:00 2001
From: Lukasz Pawlik <lucas.pawlik@gmail.com>
Date: Thu, 26 Aug 2010 09:10:26 +0200
Subject: [PATCH] Fix handling empty fields in VCARDs
Previously even mandatory TEL field was not printed in VCARD if it was
empty. This patch fix this. Now tag TEL will be printed if phone number
is not set in phonebook. This patch also fix handling category string
for VCARDs in version 2.1 for url field.
---
plugins/vcard.c | 51 ++++++++++++++++++++++++++++++++-------------------
1 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/plugins/vcard.c b/plugins/vcard.c
index a20ab4d..a163d1b 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -256,7 +256,7 @@ static void vcard_printf_number(GString *vcards, uint8_t format,
vcard_printf(vcards, buf, number);
}
-static void vcard_printf_tag(GString *vcards, const char *tag,
+static void vcard_printf_tag(GString *vcards, uint8_t format, const char *tag,
const char *category, const char *fld)
{
char *separator = "", *type = "";
@@ -265,9 +265,10 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || strlen(fld) == 0)
+ if (fld == NULL || strlen(fld) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
-
+ }
if (category && strlen(category)) {
separator = ";";
type = "TYPE=";
@@ -280,8 +281,9 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
vcard_printf(vcards, "%s:%s", buf, fld);
}
-static void vcard_printf_slash_tag(GString *vcards, const char *tag,
- const char *category, const char *fld)
+static void vcard_printf_slash_tag(GString *vcards, uint8_t format,
+ const char *tag, const char *category,
+ const char *fld)
{
int len;
char *separator = "", *type = "";
@@ -290,9 +292,10 @@ static void vcard_printf_slash_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || (len = strlen(fld)) == 0)
+ if (fld == NULL || (len = strlen(fld)) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
-
+ }
if (category && strlen(category)) {
separator = ";";
type = "TYPE=";
@@ -314,9 +317,10 @@ static void vcard_printf_email(GString *vcards, uint8_t format,
char field[LEN_MAX];
int len = 0;
- if (!address || !(len = strlen(address)))
+ if (!address || !(len = strlen(address))){
+ vcard_printf(vcards, "EMAIL:");
return;
-
+ }
switch (category){
case EMAIL_TYPE_HOME:
if (format == FORMAT_VCARD21)
@@ -358,9 +362,10 @@ static gboolean org_fields_present(struct phonebook_contact *contact)
static void vcard_printf_org(GString *vcards,
struct phonebook_contact *contact)
{
- if (org_fields_present(contact) == FALSE)
+ if (org_fields_present(contact) == FALSE){
+ vcard_printf(vcards, "ORG:");
return;
-
+ }
vcard_printf(vcards, "ORG:%s;%s;%s", contact->company,
contact->department, contact->title);
}
@@ -428,9 +433,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_fullname(vcards, contact->fullname);
if (filter & FILTER_TEL) {
- GSList *l;
+ GSList *l = contact->numbers;
- for (l = contact->numbers; l; l = l->next) {
+ if (g_slist_length(l) == 0)
+ vcard_printf_number(vcards, format, NULL, 1,
+ TEL_TYPE_OTHER);
+
+ for (; l; l = l->next) {
struct phonebook_number *number = l->data;
vcard_printf_number(vcards, format, number->tel, 1,
@@ -439,9 +448,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
}
if (filter & FILTER_EMAIL) {
- GSList *l;
+ GSList *l = contact->emails;
+
+ if (g_slist_length(l) == 0)
+ vcard_printf_email(vcards, format, NULL,
+ EMAIL_TYPE_OTHER);
- for (l = contact->emails; l; l = l->next){
+ for (; l; l = l->next){
struct phonebook_email *email = l->data;
vcard_printf_email(vcards, format, email->address, email->type);
@@ -452,18 +465,18 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_adr(vcards, contact);
if (filter & FILTER_BDAY)
- vcard_printf_tag(vcards, "BDAY", NULL, contact->birthday);
+ vcard_printf_tag(vcards, format, "BDAY", NULL, contact->birthday);
if (filter & FILTER_NICKNAME)
- vcard_printf_slash_tag(vcards, "NICKNAME", NULL,
+ vcard_printf_slash_tag(vcards, format, "NICKNAME", NULL,
contact->nickname);
if (filter & FILTER_URL)
- vcard_printf_slash_tag(vcards, "URL", "INTERNET",
+ vcard_printf_slash_tag(vcards, format, "URL", "INTERNET",
contact->website);
if (filter & FILTER_PHOTO)
- vcard_printf_tag(vcards, "PHOTO", NULL, contact->photo);
+ vcard_printf_tag(vcards, format, "PHOTO", NULL, contact->photo);
if (filter & FILTER_ORG)
vcard_printf_org(vcards, contact);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix handling empty fields in VCARDs
From: Johan Hedberg @ 2010-08-26 8:15 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimB9Tvxt6zZKzfHwhGc_j8MnGTo1DbeH-pYoQpe@mail.gmail.com>
Hi Lukasz,
> From 4bb29cdaf4a9923ab62973c38914ebb67f718d64 Mon Sep 17 00:00:00 2001
> From: Lukasz Pawlik <lucas.pawlik@gmail.com>
> Date: Thu, 26 Aug 2010 09:10:26 +0200
> Subject: [PATCH] Fix handling empty fields in VCARDs
>
> Previously even mandatory TEL field was not printed in VCARD if it was
> empty. This patch fix this. Now tag TEL will be printed if phone number
> is not set in phonebook. This patch also fix handling category string
> for VCARDs in version 2.1 for url field.
> ---
> plugins/vcard.c | 51 ++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 32 insertions(+), 19 deletions(-)
In general the patch seems fine, but you need to fix your editor
settings when dealing with BlueZ code. It seems like you're using spaces
for indentation when the coding style is tabs-only:
> - for (l = contact->numbers; l; l = l->next) {
> + if (g_slist_length(l) == 0)
> + vcard_printf_number(vcards, format, NULL, 1,
> + TEL_TYPE_OTHER);
All of the above three added lines are indented with spaces.
> + for (; l; l = l->next) {
This added line is indented with tabs. How did you manage to get your
editor to produce such mixed results?
> + if (g_slist_length(l) == 0)
> + vcard_printf_email(vcards, format, NULL,
> + EMAIL_TYPE_OTHER);
The first line is tabs but the two others use spaces!?
> - for (l = contact->emails; l; l = l->next){
> + for (; l; l = l->next){
Again spaces here.
Please enable whitespace visualization and use proper editor settings to
avoid these issues in the future. Thanks.
Johan
^ permalink raw reply
* [PATCH] Fix handling empty fields in VCARDs
From: Lukasz Pawlik @ 2010-08-26 9:48 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-Fix-handling-empty-fields-in-VCARDs.patch --]
[-- Type: text/x-patch, Size: 5044 bytes --]
From c9a89b18acf88ac0546ba72d0a169d9c2f18656d Mon Sep 17 00:00:00 2001
From: Lukasz Pawlik <lucas.pawlik@gmail.com>
Date: Thu, 26 Aug 2010 11:33:33 +0200
Subject: [PATCH] Fix handling empty fields in VCARDs
Previously even mandatory TEL field was not printed in VCARD if it was
empty. This patch fix this. Now tag TEL will be printed if phone number
is not set in phonebook. This patch also fix handling category string
for VCARDs in version 2.1 for url field.
---
plugins/vcard.c | 54 ++++++++++++++++++++++++++++++++++++------------------
1 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/plugins/vcard.c b/plugins/vcard.c
index a20ab4d..c4c4c5a 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -256,7 +256,7 @@ static void vcard_printf_number(GString *vcards, uint8_t format,
vcard_printf(vcards, buf, number);
}
-static void vcard_printf_tag(GString *vcards, const char *tag,
+static void vcard_printf_tag(GString *vcards, uint8_t format, const char *tag,
const char *category, const char *fld)
{
char *separator = "", *type = "";
@@ -265,12 +265,15 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || strlen(fld) == 0)
+ if (fld == NULL || strlen(fld) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
+ }
if (category && strlen(category)) {
separator = ";";
- type = "TYPE=";
+ if (format == FORMAT_VCARD30)
+ type = "TYPE=";
} else {
category = "";
}
@@ -280,8 +283,9 @@ static void vcard_printf_tag(GString *vcards, const char *tag,
vcard_printf(vcards, "%s:%s", buf, fld);
}
-static void vcard_printf_slash_tag(GString *vcards, const char *tag,
- const char *category, const char *fld)
+static void vcard_printf_slash_tag(GString *vcards, uint8_t format,
+ const char *tag, const char *category,
+ const char *fld)
{
int len;
char *separator = "", *type = "";
@@ -290,12 +294,15 @@ static void vcard_printf_slash_tag(GString *vcards, const char *tag,
if (tag == NULL || strlen(tag) == 0)
return;
- if (fld == NULL || (len = strlen(fld)) == 0)
+ if (fld == NULL || (len = strlen(fld)) == 0){
+ vcard_printf(vcards, "%s:", tag);
return;
+ }
if (category && strlen(category)) {
separator = ";";
- type = "TYPE=";
+ if (format == FORMAT_VCARD30)
+ type = "TYPE=";
} else {
category = "";
}
@@ -314,9 +321,10 @@ static void vcard_printf_email(GString *vcards, uint8_t format,
char field[LEN_MAX];
int len = 0;
- if (!address || !(len = strlen(address)))
+ if (!address || !(len = strlen(address))){
+ vcard_printf(vcards, "EMAIL:");
return;
-
+ }
switch (category){
case EMAIL_TYPE_HOME:
if (format == FORMAT_VCARD21)
@@ -358,8 +366,10 @@ static gboolean org_fields_present(struct phonebook_contact *contact)
static void vcard_printf_org(GString *vcards,
struct phonebook_contact *contact)
{
- if (org_fields_present(contact) == FALSE)
+ if (org_fields_present(contact) == FALSE){
+ vcard_printf(vcards, "ORG:");
return;
+ }
vcard_printf(vcards, "ORG:%s;%s;%s", contact->company,
contact->department, contact->title);
@@ -428,9 +438,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_fullname(vcards, contact->fullname);
if (filter & FILTER_TEL) {
- GSList *l;
+ GSList *l = contact->numbers;
- for (l = contact->numbers; l; l = l->next) {
+ if (g_slist_length(l) == 0)
+ vcard_printf_number(vcards, format, NULL, 1,
+ TEL_TYPE_OTHER);
+
+ for (; l; l = l->next) {
struct phonebook_number *number = l->data;
vcard_printf_number(vcards, format, number->tel, 1,
@@ -439,9 +453,13 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
}
if (filter & FILTER_EMAIL) {
- GSList *l;
+ GSList *l = contact->emails;
+
+ if (g_slist_length(l) == 0)
+ vcard_printf_email(vcards, format, NULL,
+ EMAIL_TYPE_OTHER);
- for (l = contact->emails; l; l = l->next){
+ for (; l; l = l->next){
struct phonebook_email *email = l->data;
vcard_printf_email(vcards, format, email->address, email->type);
@@ -452,18 +470,18 @@ void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
vcard_printf_adr(vcards, contact);
if (filter & FILTER_BDAY)
- vcard_printf_tag(vcards, "BDAY", NULL, contact->birthday);
+ vcard_printf_tag(vcards, format, "BDAY", NULL, contact->birthday);
if (filter & FILTER_NICKNAME)
- vcard_printf_slash_tag(vcards, "NICKNAME", NULL,
+ vcard_printf_slash_tag(vcards, format, "NICKNAME", NULL,
contact->nickname);
if (filter & FILTER_URL)
- vcard_printf_slash_tag(vcards, "URL", "INTERNET",
+ vcard_printf_slash_tag(vcards, format, "URL", "INTERNET",
contact->website);
if (filter & FILTER_PHOTO)
- vcard_printf_tag(vcards, "PHOTO", NULL, contact->photo);
+ vcard_printf_tag(vcards, format, "PHOTO", NULL, contact->photo);
if (filter & FILTER_ORG)
vcard_printf_org(vcards, contact);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 13/22] Add three new options for l2cap_options which used in setsockopt & getsockopt
From: David Vrabel @ 2010-08-26 10:41 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=8xVks--xcLXEQ_=sHfP3fY+Wvyctu5K_kMDG-@mail.gmail.com>
haijun liu wrote:
> From 897b281d14ba4cf9a5fbbf5ba65b84c85e688737 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:00:26 +0800
> Subject: [PATCH 13/22] Add three new options for l2cap_options which
> used in setsockopt & getsockopt.
>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index d0ae9f5..4f87aec 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -65,6 +65,9 @@ struct l2cap_options {
> __u8 fcs;
> __u8 max_tx;
> __u16 txwin_size;
> + __u8 hschan_req;
This isn't the API that was agreed. See Mat Martineau's "Bluetooth:
Add socket option definitions for AMP" patch.
> + __u8 guaranteed;
Selecting guaranteed without being able to specify flow specs doesn't
seem useful.
I would like to see support for best-effort links merged first before
guaranteed links are considered.
> + __u8 reconfig;
I'd suggest removing this unless there's a profile/application that will
make use of it. I'd have thought that channel reconfiguration was
something the L2CAP stack does automatically when moving channels
between BR/EDR and AMP radios.
> };
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply
* Re: [PATCH] Fix handling empty fields in VCARDs
From: Johan Hedberg @ 2010-08-26 11:28 UTC (permalink / raw)
To: Lukasz Pawlik; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimN_hk1n7rDSBMpDP4-7ECV-4fm0NqHHwz0VBjo@mail.gmail.com>
Hi Lukasz,
On Thu, Aug 26, 2010, Lukasz Pawlik wrote:
> From c9a89b18acf88ac0546ba72d0a169d9c2f18656d Mon Sep 17 00:00:00 2001
> From: Lukasz Pawlik <lucas.pawlik@gmail.com>
> Date: Thu, 26 Aug 2010 11:33:33 +0200
> Subject: [PATCH] Fix handling empty fields in VCARDs
>
> Previously even mandatory TEL field was not printed in VCARD if it was
> empty. This patch fix this. Now tag TEL will be printed if phone number
> is not set in phonebook. This patch also fix handling category string
> for VCARDs in version 2.1 for url field.
> ---
> plugins/vcard.c | 54 ++++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 36 insertions(+), 18 deletions(-)
Thanks. The patch is now upstream. Btw, it's written vCard and not VCARD
(I fixed this for you manually).
Johan
^ permalink raw reply
* [PATCH] Bluetooth: BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar Rymarkiewicz @ 2010-08-27 11:44 UTC (permalink / raw)
To: linux-bluetooth
Cc: par-gunnar.p.hjalmdahl, joakim.xj.ceder, johan.hedberg,
Waldemar Rymarkiewicz
The security level BT_SECURITY_HIGH expects secure connection
and a minimum 16 digit pin code used for bonding. It's requitred by the
Sim Access Profile.
Patch on behalf of ST-Ericsson SA.
Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
include/net/bluetooth/hci.h | 17 +++++++
include/net/bluetooth/hci_core.h | 4 ++
net/bluetooth/hci_conn.c | 97 +++++++++++++++++++++++++++++++-------
net/bluetooth/hci_event.c | 4 ++
net/bluetooth/hci_sock.c | 3 +
net/bluetooth/rfcomm/core.c | 10 ++++-
6 files changed, 116 insertions(+), 19 deletions(-)
mode change 100644 => 100755 include/net/bluetooth/hci.h
mode change 100644 => 100755 include/net/bluetooth/hci_core.h
mode change 100644 => 100755 net/bluetooth/hci_conn.c
mode change 100644 => 100755 net/bluetooth/hci_event.c
mode change 100644 => 100755 net/bluetooth/rfcomm/core.c
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
old mode 100644
new mode 100755
index bcbdd6d..34a6fbf
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -99,6 +99,7 @@ enum {
#define HCISETLINKMODE _IOW('H', 226, int)
#define HCISETACLMTU _IOW('H', 227, int)
#define HCISETSCOMTU _IOW('H', 228, int)
+#define HCISETCONNINFO _IOW('H', 229, int)
#define HCIBLOCKADDR _IOW('H', 230, int)
#define HCIUNBLOCKADDR _IOW('H', 231, int)
@@ -224,6 +225,15 @@ enum {
#define HCI_AT_GENERAL_BONDING 0x04
#define HCI_AT_GENERAL_BONDING_MITM 0x05
+/* Link Key types */
+#define HCI_LK_COMBINATION 0x00
+#define HCI_LK_LOCAL_UNIT 0x01
+#define HCI_LK_REMOTE_UNIT 0x02
+#define HCI_LK_DEBUG_COMBINATION 0x03
+#define HCI_LK_UNAUTHENTICATED_COMBINATION 0x04
+#define HCI_LK_AUTHENTICATED_COMBINATION 0x05
+#define HCI_LK_CHANGEED_COMBINATION_KEY 0x06
+
/* ----- HCI Commands ---- */
#define HCI_OP_INQUIRY 0x0401
struct hci_cp_inquiry {
@@ -1022,9 +1032,16 @@ struct hci_conn_info_req {
struct hci_conn_info conn_info[0];
};
+struct hci_set_conn_info_req {
+ bdaddr_t bdaddr;
+ __u8 pin_len;
+ __u8 key_type;
+};
+
struct hci_auth_info_req {
bdaddr_t bdaddr;
__u8 type;
+ __u8 level;
};
struct hci_inquiry_req {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
old mode 100644
new mode 100755
index 4568b93..9eb2da3
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -183,6 +183,8 @@ struct hci_conn {
__u32 link_mode;
__u8 auth_type;
__u8 sec_level;
+ __u8 key_type;
+ __u8 pin_len;
__u8 power_save;
__u16 disc_timeout;
unsigned long pend;
@@ -430,6 +432,8 @@ int hci_get_dev_info(void __user *arg);
int hci_get_conn_list(void __user *arg);
int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
+int hci_set_conn_info(struct hci_dev *hdev, void __user *arg);
+
int hci_inquiry(void __user *arg);
struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev, bdaddr_t *bdaddr);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
old mode 100644
new mode 100755
index 0b1e460..ed7a007
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -233,6 +233,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
conn->mode = HCI_CM_ACTIVE;
conn->state = BT_OPEN;
conn->auth_type = HCI_AT_GENERAL_BONDING;
+ conn->key_type = 0xff;
+ conn->pin_len = 0;
conn->power_save = 1;
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
@@ -433,15 +435,11 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
EXPORT_SYMBOL(hci_conn_check_link_mode);
/* Authenticate remote device */
-static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
+static void hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
{
BT_DBG("conn %p", conn);
- if (sec_level > conn->sec_level)
- conn->sec_level = sec_level;
- else if (conn->link_mode & HCI_LM_AUTH)
- return 1;
-
+ conn->sec_level = sec_level;
conn->auth_type = auth_type;
if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
@@ -450,8 +448,20 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
hci_send_cmd(conn->hdev, HCI_OP_AUTH_REQUESTED,
sizeof(cp), &cp);
}
+}
- return 0;
+/* Encrypt the the link */
+static void hci_conn_encrypt(struct hci_conn *conn)
+{
+ BT_DBG("conn %p", conn);
+
+ if (!test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend)) {
+ struct hci_cp_set_conn_encrypt cp;
+ cp.handle = cpu_to_le16(conn->handle);
+ cp.encrypt = 1;
+ hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT,
+ sizeof(cp), &cp);
+ }
}
/* Enable security */
@@ -459,28 +469,54 @@ int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
{
BT_DBG("conn %p", conn);
+ /* For sdp we do not need the link key. */
if (sec_level == BT_SECURITY_SDP)
return 1;
+ /* For non 2.1 devices and low security level we do not need the
+ link key. */
if (sec_level == BT_SECURITY_LOW &&
(!conn->ssp_mode || !conn->hdev->ssp_mode))
return 1;
- if (conn->link_mode & HCI_LM_ENCRYPT)
- return hci_conn_auth(conn, sec_level, auth_type);
-
+ /* For other security levels we need link key. */
+ if (!(conn->link_mode & HCI_LM_AUTH))
+ goto do_auth;
+
+ /* An authenticated combination key has sufficient security for any
+ security level. */
+ if (conn->key_type == HCI_LK_AUTHENTICATED_COMBINATION)
+ goto do_encrypt;
+
+ /* An unauthenticated combination key has sufficient security for
+ security level 1 and 2. */
+ if (conn->key_type == HCI_LK_UNAUTHENTICATED_COMBINATION
+ && (sec_level == BT_SECURITY_MEDIUM
+ || sec_level == BT_SECURITY_LOW))
+ goto do_encrypt;
+
+ /* A combination key has always sufficient security for the security
+ levels 1 or 2. High security level requires that the combination key
+ was generated using the maximum PIN code length (16).
+ For pre 2.1 units. */
+ if ((conn->key_type == HCI_LK_COMBINATION))
+ if ((sec_level != BT_SECURITY_HIGH) || (conn->pin_len >= 16))
+ goto do_encrypt;
+
+do_auth:
if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
return 0;
- if (hci_conn_auth(conn, sec_level, auth_type)) {
- struct hci_cp_set_conn_encrypt cp;
- cp.handle = cpu_to_le16(conn->handle);
- cp.encrypt = 1;
- hci_send_cmd(conn->hdev, HCI_OP_SET_CONN_ENCRYPT,
- sizeof(cp), &cp);
- }
-
+ hci_conn_auth(conn, sec_level, auth_type);
return 0;
+
+do_encrypt:
+ if (conn->link_mode & HCI_LM_ENCRYPT)
+ return 1; /* sufficient link key */
+ else{
+ hci_conn_encrypt(conn);
+ return 0; /* auth pending */
+ }
}
EXPORT_SYMBOL(hci_conn_security);
@@ -713,6 +749,30 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg)
return copy_to_user(ptr, &ci, sizeof(ci)) ? -EFAULT : 0;
}
+int hci_set_conn_info(struct hci_dev *hdev, void __user *arg)
+{
+ struct hci_set_conn_info_req req;
+ struct hci_conn *conn;
+
+ if (copy_from_user(&req, arg, sizeof(req))) {
+ BT_DBG("copy from user failed");
+ return -EFAULT;
+ }
+
+ hci_dev_lock_bh(hdev);
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr);
+ if (conn) {
+ conn->pin_len = req.pin_len;
+ conn->key_type = req.key_type;
+ }
+ hci_dev_unlock_bh(hdev);
+
+ if (!conn)
+ return -ENOENT;
+
+ return 0;
+}
+
int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
{
struct hci_auth_info_req req;
@@ -725,6 +785,7 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr);
if (conn)
req.type = conn->auth_type;
+ req.level = conn->sec_level;
hci_dev_unlock_bh(hdev);
if (!conn)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
old mode 100644
new mode 100755
index bfef5ba..6d6b04c
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1521,6 +1521,10 @@ static inline void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
if (conn) {
hci_conn_hold(conn);
+ /* For Changed Combination Link Key the only link key has
+ * been changed, not link key type. */
+ if (conn->key_type != HCI_LK_CHANGEED_COMBINATION_KEY)
+ conn->key_type = ev->key_type;
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
hci_conn_put(conn);
}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 83acd16..502a7b0 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -269,6 +269,9 @@ static inline int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd, unsign
case HCIGETCONNINFO:
return hci_get_conn_info(hdev, (void __user *) arg);
+ case HCISETCONNINFO:
+ return hci_set_conn_info(hdev, (void __user *) arg);
+
case HCIGETAUTHINFO:
return hci_get_auth_info(hdev, (void __user *) arg);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
old mode 100644
new mode 100755
index 7dca91b..2e248d5
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2086,7 +2086,15 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
continue;
if (!status)
- set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ if (d->sec_level != BT_SECURITY_HIGH)
+ set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ else
+ if ((conn->key_type == HCI_LK_AUTHENTICATED_COMBINATION)
+ || (conn->key_type == HCI_LK_COMBINATION
+ && conn->pin_len >= 16))
+ set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+ else
+ set_bit(RFCOMM_AUTH_REJECT, &d->flags);
else
set_bit(RFCOMM_AUTH_REJECT, &d->flags);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar Rymarkiewicz @ 2010-08-27 11:45 UTC (permalink / raw)
To: linux-bluetooth
Cc: par-gunnar.p.hjalmdahl, joakim.xj.ceder, johan.hedberg,
Waldemar Rymarkiewicz
The security level BT_SECURITY_HIGH expects secure connection
and a minimum 16 digit pin code used for bonding. It's requitred by the
Sim Access Profile.
Patch on behalf of ST-Ericsson SA.
Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
---
lib/hci.h | 8 +++++++-
src/dbus-hci.c | 33 ++++++++++++++++++++++++++++-----
src/security.c | 38 ++++++++++++++++++++++++++++++++++----
3 files changed, 69 insertions(+), 10 deletions(-)
mode change 100644 => 100755 lib/bluetooth.h
mode change 100644 => 100755 lib/hci.h
mode change 100644 => 100755 lib/l2cap.h
mode change 100644 => 100755 lib/rfcomm.h
mode change 100644 => 100755 src/btio.c
mode change 100644 => 100755 src/btio.h
mode change 100644 => 100755 src/dbus-hci.c
mode change 100644 => 100755 src/security.c
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
old mode 100644
new mode 100755
diff --git a/lib/hci.h b/lib/hci.h
old mode 100644
new mode 100755
index 512dab9..a313929
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -96,7 +96,7 @@ enum {
#define HCISETLINKMODE _IOW('H', 226, int)
#define HCISETACLMTU _IOW('H', 227, int)
#define HCISETSCOMTU _IOW('H', 228, int)
-
+#define HCISETCONNINFO _IOW('H', 229, int)
#define HCIBLOCKADDR _IOW('H', 230, int)
#define HCIUNBLOCKADDR _IOW('H', 231, int)
@@ -2326,9 +2326,15 @@ struct hci_conn_info_req {
struct hci_conn_info conn_info[0];
};
+struct hci_set_conn_info_req {
+ bdaddr_t bdaddr;
+ uint8_t pin_len;
+ uint8_t key_type;
+};
struct hci_auth_info_req {
bdaddr_t bdaddr;
uint8_t type;
+ uint8_t level;
};
struct hci_inquiry_req {
diff --git a/lib/l2cap.h b/lib/l2cap.h
old mode 100644
new mode 100755
diff --git a/lib/rfcomm.h b/lib/rfcomm.h
old mode 100644
new mode 100755
diff --git a/src/btio.c b/src/btio.c
old mode 100644
new mode 100755
diff --git a/src/btio.h b/src/btio.h
old mode 100644
new mode 100755
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
old mode 100644
new mode 100755
index 9055ffe..177f536
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -165,6 +165,8 @@ static void pincode_cb(struct agent *agent, DBusError *err,
{
struct btd_adapter *adapter = device_get_adapter(device);
pin_code_reply_cp pr;
+ struct hci_auth_info_req ar;
+ struct hci_set_conn_info_req cr;
bdaddr_t sba, dba;
size_t len;
int dev;
@@ -180,13 +182,30 @@ static void pincode_cb(struct agent *agent, DBusError *err,
adapter_get_address(adapter, &sba);
device_get_address(device, &dba);
- if (err) {
- hci_send_cmd(dev, OGF_LINK_CTL,
- OCF_PIN_CODE_NEG_REPLY, 6, &dba);
- goto done;
- }
+ if (err)
+ goto reject;
len = strlen(pincode);
+ memset(&ar, 0, sizeof(ar));
+ bacpy(&ar.bdaddr, &dba);
+ if (ioctl(dev, HCIGETAUTHINFO, (unsigned long) &ar) < 0) {
+ error("Can't get auth info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
+
+ if (ar.level == BT_SECURITY_HIGH && len < 16) {
+ error("Not enough secure PIN (expected 16 digit PIN, but got \
+ %d digit).", len);
+ goto reject;
+ }
+
+ bacpy(&cr.bdaddr, &dba);
+ cr.pin_len = len;
+ cr.key_type = 0xff;
+ if (ioctl(dev, HCISETCONNINFO, (unsigned long) &cr) < 0) {
+ error("Can't set conn info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
set_pin_length(&sba, len);
@@ -196,7 +215,11 @@ static void pincode_cb(struct agent *agent, DBusError *err,
pr.pin_len = len;
hci_send_cmd(dev, OGF_LINK_CTL, OCF_PIN_CODE_REPLY,
PIN_CODE_REPLY_CP_SIZE, &pr);
+ goto done;
+reject:
+ hci_send_cmd(dev, OGF_LINK_CTL,
+ OCF_PIN_CODE_NEG_REPLY, 6, &dba);
done:
hci_close_dev(dev);
}
diff --git a/src/security.c b/src/security.c
old mode 100644
new mode 100755
index 667f1f1..b25d1e4
--- a/src/security.c
+++ b/src/security.c
@@ -309,6 +309,7 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
char sa[18], da[18];
uint8_t type;
int err;
+ int pinlen;
if (!get_adapter_and_device(sba, dba, &adapter, &device, FALSE))
device = NULL;
@@ -325,9 +326,11 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
DBG("HCIGETAUTHINFO failed %s (%d)",
strerror(errno), errno);
req.type = 0x00;
+ req.level = BT_SECURITY_LOW;
}
- DBG("kernel auth requirements = 0x%02x", req.type);
+ DBG("kernel auth requirements = 0x%02x and security level = 0x%02x", \
+ req.type, req.level);
if (main_opts.debug_keys && device && device_get_debug_key(device, key))
type = 0x03;
@@ -341,18 +344,34 @@ static void link_key_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
DBG("link key type = 0x%02x", type);
+ pinlen = read_pin_length(sba, dba);
+ DBG("stored link key type = 0x%02x pin_len = %d", type, pinlen);
+
/* Don't use unauthenticated combination keys if MITM is
- * required */
- if (type == 0x04 && req.type != 0xff && (req.type & 0x01))
+ * required and also don't use combination link keys authenticated with
+ * an PIN code len < 16 if security level BT_SECURITY_HIGH is required*/
+ if ((type == 0x04 && req.type != 0xff && (req.type & 0x01)) ||
+ (type == 0x00 && req.level == BT_SECURITY_HIGH && pinlen < 16))
hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
6, dba);
else {
link_key_reply_cp lr;
+ struct hci_set_conn_info_req cr;
memcpy(lr.link_key, key, 16);
bacpy(&lr.bdaddr, dba);
- hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_REPLY,
+ bacpy(&cr.bdaddr, dba);
+ cr.pin_len = pinlen;
+ cr.key_type = type;
+
+ if (ioctl(dev, HCISETCONNINFO, (unsigned long) &cr) < 0) {
+ error("Can't set conn info: %s (%d)", strerror(errno),
+ errno);
+ hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_NEG_REPLY,
+ 6, dba);
+ } else
+ hci_send_cmd(dev, OGF_LINK_CTL, OCF_LINK_KEY_REPLY,
LINK_KEY_REPLY_CP_SIZE, &lr);
}
}
@@ -523,6 +542,7 @@ static void pin_code_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
pin_code_reply_cp pr;
struct hci_conn_info_req *cr;
struct hci_conn_info *ci;
+ struct hci_auth_info_req ar;
char sa[18], da[18], pin[17];
int pinlen;
@@ -542,10 +562,20 @@ static void pin_code_request(int dev, bdaddr_t *sba, bdaddr_t *dba)
}
ci = cr->conn_info;
+ memset(&ar, 0, sizeof(ar));
+ bacpy(&ar.bdaddr, dba);
+ if (ioctl(dev, HCIGETAUTHINFO, (unsigned long) &ar) < 0) {
+ error("Can't get auth info: %s (%d)", strerror(errno), errno);
+ goto reject;
+ }
memset(pin, 0, sizeof(pin));
pinlen = read_pin_code(sba, dba, pin);
if (pinlen > 0) {
+ if (ar.level == BT_SECURITY_HIGH && pinlen < 16) {
+ error("Not 16 digit pin code.");
+ goto reject;
+ }
set_pin_length(sba, pinlen);
memcpy(pr.pin_code, pin, pinlen);
pr.pin_len = pinlen;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Bastien Nocera @ 2010-08-27 12:12 UTC (permalink / raw)
To: Waldemar Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg
In-Reply-To: <1282909526-19374-1-git-send-email-waldemar.rymarkiewicz@tieto.com>
On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
> The security level BT_SECURITY_HIGH expects secure connection
> and a minimum 16 digit pin code used for bonding. It's requitred by the
> Sim Access Profile.
How is user-space (meaning the pairing agent) supposed to handle that?
I'd need to make changes to gnome-bluetooth to use longer PIN codes for
the maximum security.
Cheers
^ permalink raw reply
* RE: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Waldemar.Rymarkiewicz @ 2010-08-27 12:26 UTC (permalink / raw)
To: hadess
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg, arunkr.singh
In-Reply-To: <1282911133.835.30.camel@localhost.localdomain>
Hi,
>-----Original Message-----
>From: Bastien Nocera [mailto:hadess@hadess.net]
>Sent: Friday, August 27, 2010 2:12 PM
>To: Rymarkiewicz Waldemar
>Cc: linux-bluetooth@vger.kernel.org;
>par-gunnar.p.hjalmdahl@stericsson.com;
>joakim.xj.ceder@stericsson.com; johan.hedberg@gmail.com
>Subject: Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
>
>On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
>> The security level BT_SECURITY_HIGH expects secure connection and a
>> minimum 16 digit pin code used for bonding. It's requitred
>by the Sim
>> Access Profile.
>
>How is user-space (meaning the pairing agent) supposed to handle that?
>I'd need to make changes to gnome-bluetooth to use longer PIN
>codes for the maximum security.
>
>Cheers
>
I assume that user will know that the 16 digit pin is requred, so should be enough to let the user type 16 digit in an agent I guess.
Usually a service that requires high security will generate right pin code.
Originaly the high security level was planned to require max pin code lenght as I know.
Thanks,
/Waldek
^ permalink raw reply
* RE: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Bastien Nocera @ 2010-08-27 12:32 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
johan.hedberg, arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480976E15A2C@EXMB04.eu.tieto.com>
On Fri, 2010-08-27 at 15:26 +0300, Waldemar.Rymarkiewicz@tieto.com
wrote:
> Hi,
>
> >-----Original Message-----
> >From: Bastien Nocera [mailto:hadess@hadess.net]
> >Sent: Friday, August 27, 2010 2:12 PM
> >To: Rymarkiewicz Waldemar
> >Cc: linux-bluetooth@vger.kernel.org;
> >par-gunnar.p.hjalmdahl@stericsson.com;
> >joakim.xj.ceder@stericsson.com; johan.hedberg@gmail.com
> >Subject: Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
> >
> >On Fri, 2010-08-27 at 13:45 +0200, Waldemar Rymarkiewicz wrote:
> >> The security level BT_SECURITY_HIGH expects secure connection and a
> >> minimum 16 digit pin code used for bonding. It's requitred
> >by the Sim
> >> Access Profile.
> >
> >How is user-space (meaning the pairing agent) supposed to handle that?
> >I'd need to make changes to gnome-bluetooth to use longer PIN
> >codes for the maximum security.
> >
> >Cheers
> >
>
> I assume that user will know that the 16 digit pin is requred, so
> should be enough to let the user type 16 digit in an agent I guess.
> Usually a service that requires high security will generate right pin
> code.
How would they know? Pairing the device isn't connecting to the
service... Furthermore, gnome-bluetooth's wizard takes a lot of care
generating pin codes for the user by default, so we'd need to know that
a 16 digit pin code is required.
Supporting 16 digits pin code would probably require interface changes.
And I was under the impression that the PIN code's length didn't come
into account for the creation of the encryption, just for the initial
challenge-response needed to verify the device is who it says it is.
> Originaly the high security level was planned to require max pin code
> lenght as I know.
^ permalink raw reply
* Re: [PATCH] BT_SECURITY_HIGH requires 16 digit pin code
From: Johan Hedberg @ 2010-08-27 12:45 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz
Cc: hadess, linux-bluetooth, par-gunnar.p.hjalmdahl, joakim.xj.ceder,
arunkr.singh
In-Reply-To: <99B09243E1A5DA4898CDD8B7001114480976E15A2C@EXMB04.eu.tieto.com>
Hi Waldemar,
On Fri, Aug 27, 2010, Waldemar.Rymarkiewicz@tieto.com wrote:
> I assume that user will know that the 16 digit pin is requred, so
> should be enough to let the user type 16 digit in an agent I guess.
> Usually a service that requires high security will generate right pin
> code.
I don't think "the user should know" is enough here, so we may need to
think about ways that we could relay this info to the agent (maybe a
special adapter property or an extra parameter in the PIN request agent
callback).
> Originaly the high security level was planned to require max pin code
> lenght as I know.
That's correct, however the UI side hasn't really been considered so
far.
In general about the patches, I suspect it'll take a bit longer than
usual to get them in since the code you're touching is one of the most
sensitive ones with respect to breakage. We've finetuned it multiple
times over the last few years to get rid of IOP issues, security
vulnerabilities and to make sure all qualification tests pass. Another
reason for an expected delay is that you're introducing changes to the
kernel interface and that always requires some extra reviewing.
So how well have you tested the patches? I.e. how confident are you that
you're not introducing any regressions? Scenarios that would need to be
tested before an upstream merge are (and I'm probably forgetting several
of them):
- legacy pairing acceptor & initiator
- security mode 3 acceptor & initiator
- ssp acceptor & initiator
- renewed link key handling for both debug and normal keys
- security level upgrading (i.e. connect first to a low security socket
and then over the same ACL to a higher security socket)
- complete and partial failure scenarios for all of the above
Additionally all these test should be done against several different
controllers due to differences in HCI interface behavior (event
ordering, error codes, etc). In that list I'd include at least one CSR,
and one Broadcom adapter and any other adapters from other manufacturers
that you can get hold of.
So how many of these tests do you already have covered? I'm not very
comfortable with pushing the patches upstream before most of the above
scenarios have been tested and verified not to introduce any
regressions.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox