* Re: [PATCH 3/8] Bluetooth: Validate PSM values in calls to connect() and bind().
From: Gustavo F. Padovan @ 2010-08-02 21:24 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Mat Martineau, linux-bluetooth, rshaffer, linux-arm-msm
In-Reply-To: <1280777991.12579.39.camel@localhost.localdomain>
Hi Mat,
* Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:39:51 -0700]:
> Hi Mat,
>
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> > net/bluetooth/l2cap.c | 12 ++++++++++++
> > 1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index aed72f2..44bc6ee 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -1008,6 +1008,12 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> > goto done;
> > }
> >
> > + /* If specified, PSM must be odd and lsb of upper byte must be 0 */
> > + if (la.l2_psm && (__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
> > + err = -EINVAL;
> > + goto done;
> > + }
> > +
>
> this was a nice feature of L2CAP in BlueZ actually ;)
And please add a commit message for this patch.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH v4 0/5] Enhanced support for extended inquiry response
From: ingas @ 2010-08-02 21:24 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100802185542.GA28641@jh-x301>
Hi Johan,
Thanks for your reply.
> Hi Inga,
>
> First of all, sorry for the slight delay in my reply. I've been on
> holiday last week with quite limted email processing capability.
>
> On Wed, Jul 28, 2010, ingas@codeaurora.org wrote:
>> In the current implementation, EIR write is keyed off HCI Command
>> Complete events for a set of commands (in security.c).
>> - When first registering an OPP service with sdptool, the
>> update_svcclass_list() (sdpd-service.c) calls manager_update_svc()
>> (manager.c) which in turn calls adapter_update() (adapter.c). BTW, why
>> this level of indirection, since manager_update_svc() is called only
>> from
>> one place in the code and calling adapter_update() is all it does?
>> Probably some earlier architectural decision I am not aware of…
>
> That looks pretty stupid to me too. Might be some historical artifact.
> Anyway, feel free to send a patch to remove this indirection (i.e. call
> adapter_update directly).
>
Will do once i get through the initial submission.
>> The current implementation may be extended as following:
>> Since update_svcclass_list() is being called throughout sdpd-service.c
>> in
>> all the places where EIR needs to be updated as well, I propose to
>> modify
>> the corresponding function adapter_set_service_classes() (adapter.c) :
>>
>> **********************
>> Before:
>> /* If we already have the CoD we want or the cache is enabled or an
>> * existing CoD write is in progress just bail out */
>> if (adapter->current_cod == adapter->wanted_cod ||
>> adapter->cache_enable || adapter->pending_cod)
>> return 0;
>>
>>
>> ***********************
>>
>> After:
>> /* If we already have the CoD we want or the cache is enabled or an
>> * existing CoD write is in progress just bail out */
>> if (adapter->cache_enable || adapter->pending_cod)
>> return 0;
>>
>> /* If we already have the CoD we want, update EIR and return */
>> if (adapter->current_cod == adapter->wanted_cod ) {
>> update_ext_inquiry_response(adapter);
>> return 0;
>> }
>>
>> **********************
>> Will that be acceptable? I tested this modification and it’s working
>> with
>> both d-bus methods and sdptool.
>
> Seems good to me.
>
I just submitted an updated patch set for EIR (hopefully, not messing up
my tabs an spaces in the process). This is certainly a learning process
and your patience is appreciated :)
Regards,
Inga
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Gustavo F. Padovan @ 2010-08-02 21:20 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Mat Martineau, linux-bluetooth, rshaffer, linux-arm-msm
In-Reply-To: <1280777912.12579.37.camel@localhost.localdomain>
Hi Mat,
* Marcel Holtmann <marcel@holtmann.org> [2010-08-02 12:38:32 -0700]:
> Hi Mat,
>
> > Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> > ---
> > net/bluetooth/l2cap.c | 12 ++++++++----
> > 1 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 9ba1e8e..aed72f2 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> > goto unlock;
> >
> > if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> > - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> > - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> > + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> > + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> > + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> > + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> > l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
>
> this becomes unreadable and my brain starts to throw a core dump. So it
> clearly needs to be put into a helper inline function.
Actually we don't need that, since the code that deals with Basic Mode
never check and use the l2cap_pi(sk)->fcs. So we don't care about FCS
value in the Basic Mode.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH 2/2] ebook backend: return all TEL attribs from vcard in string
From: Johan Hedberg @ 2010-08-02 20:06 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201007281324.o6SDO9YB010621@joshua.mesa.nl>
Hi,
On Wed, Jul 28, 2010, Marcel Mol wrote:
> evcard_to_string() only took one TEL attrib from a vcard. But
> multiple TEL attribs are supported. This patch will convert
> all TEL attribs to strings.
> ---
> plugins/phonebook-ebook.c | 24 +++++++++++++-----------
> 1 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
> index 089b956..04b7900 100644
> --- a/plugins/phonebook-ebook.c
> +++ b/plugins/phonebook-ebook.c
> @@ -97,6 +97,7 @@ static char *evcard_to_string(EVCard *evcard, unsigned int format,
> uint64_t filter)
> {
> EVCard *evcard2;
> + GList *l;
> char *vcard;
> unsigned int i;
>
> @@ -109,18 +110,19 @@ static char *evcard_to_string(EVCard *evcard, unsigned int format,
> */
> filter = format == EVC_FORMAT_VCARD_30 ? filter | 0x87: filter | 0x85;
>
> + l = e_vcard_get_attributes(evcard);
> evcard2 = e_vcard_new();
> - for (i = 0; i < 29; i++) {
> - EVCardAttribute *attrib;
> -
> - if (!(filter & (1 << i)))
> - continue;
> -
> - attrib = e_vcard_get_attribute(evcard, attribute_mask[i]);
> - if (!attrib)
> - continue;
> -
> - e_vcard_add_attribute(evcard2, e_vcard_attribute_copy(attrib));
> + for (; l; l = g_list_next(l)) {
> + EVCardAttribute *attrib = l->data;
> + if (attrib) {
> + const char *name = e_vcard_attribute_get_name(attrib);
> + for (i = 0; i < 29; i++) {
> + if (!(filter & (1 << i)))
> + continue;
> + if (!strcmp(name, attribute_mask[i]))
> + e_vcard_add_attribute(evcard2, e_vcard_attribute_copy(attrib));
> + }
> + }
> }
This needs to be cleaned up a bit in order not to be so deeply nested
which additionally causes you problems with producing over 80-character
long lines. You can avoid that with keeping the same
if (!attrib)
continue;
trick as the original code had as well as
if (g_strcmp0(name, attribute_mask[i]) != 0)
continue;
Johan
^ permalink raw reply
* Re: [PATCH 7/8] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
From: Marcel Holtmann @ 2010-08-02 19:53 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-8-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 582975b..8e9fa51 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1923,6 +1923,7 @@ done:
> static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags)
> {
> struct sock *sk = sock->sk;
> + int len_or_err;
>
> lock_sock(sk);
>
> @@ -1956,7 +1957,13 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
>
> release_sock(sk);
>
> - return bt_sock_recvmsg(iocb, sock, msg, len, flags);
> + if (sock->type == SOCK_STREAM)
> + len_or_err = bt_sock_stream_recvmsg(iocb, sock, msg, len,
> + flags);
> + else
> + len_or_err = bt_sock_recvmsg(iocb, sock, msg, len, flags);
> +
> + return len_or_err;
> }
I don't like this variable name. Just call it "len" or just use "ret"
here. Check what other parts of net/bluetooth/ are using.
Also since you are not checking is value anyway, why not return right
away from the if.
if (sock->type == SOCK_STREAM)
return bt_sock_stream_recvmsg(...);
return bt_sock_recvmsg(...);
That way the compiler should also not complain. And in addition your
patch looks dead simple ;)
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 0/8] Bluetooth: L2CAP updates for PSM validation and ERTM.
From: Marcel Holtmann @ 2010-08-02 19:50 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> This is a collection of ERTM-related patches, including some L2CAP
> configuration fixes and allowing partial-frame reads from L2CAP
> SOCK_STREAM sockets. PSM validation is also included, although that
> is not restricted to ERTM.
so the merge window for 2.6.36 is basically closed and these patches
came in too late. If they fix a bug, I like you to explain that in
detail in the commit message so I can pick them out and include them for
submission to John.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 8/8] Bluetooth: Use 3-DH5 payload size for default ERTM max PDU size.
From: Marcel Holtmann @ 2010-08-02 19:48 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-9-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
this should have a commit message with technical details to explain why
it is a good idea.
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 16e412f..93aba17 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -35,7 +35,7 @@
> #define L2CAP_DEFAULT_MAX_TX 3
> #define L2CAP_DEFAULT_RETRANS_TO 2000 /* 2 seconds */
> #define L2CAP_DEFAULT_MONITOR_TO 12000 /* 12 seconds */
> -#define L2CAP_DEFAULT_MAX_PDU_SIZE 672
> +#define L2CAP_DEFAULT_MAX_PDU_SIZE 1011 /* Sized for 3-DH5 packet */
> #define L2CAP_DEFAULT_ACK_TO 200
> #define L2CAP_LOCAL_BUSY_TRIES 12
Also the default PDU size for basic mode should stay with 672. And only
for ERTM move it to 1010. Is this ensured?
And then again here. Is that explained in the commit message?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 6/8] Bluetooth: Move stream-oriented recvmsg code so it can be used by L2CAP.
From: Marcel Holtmann @ 2010-08-02 19:46 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-7-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> include/net/bluetooth/bluetooth.h | 2 +
> net/bluetooth/af_bluetooth.c | 107 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/rfcomm/sock.c | 104 ++---------------------------------
> 3 files changed, 115 insertions(+), 98 deletions(-)
looks all like a good idea, but I really have to insist that the commit
message explain everything in detail. Give a reason why the code is
similar or the same and why this makes sense.
Also splitting these into two or more patches makes sense. One adding
the stream receive and the other modifying L2CAP and RFCOMM to use it.
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 27a902d..08b6c2a 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -126,6 +126,8 @@ int bt_sock_unregister(int proto);
> void bt_sock_link(struct bt_sock_list *l, struct sock *s);
> void bt_sock_unlink(struct bt_sock_list *l, struct sock *s);
> int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags);
> +int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
> + struct msghdr *msg, size_t len, int flags);
> uint bt_sock_poll(struct file * file, struct socket *sock, poll_table *wait);
> int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 421c45b..73047f5 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -265,6 +265,113 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
> }
> EXPORT_SYMBOL(bt_sock_recvmsg);
>
> +static long bt_sock_data_wait(struct sock *sk, long timeo)
> +{
> + DECLARE_WAITQUEUE(wait, current);
> +
> + add_wait_queue(sk_sleep(sk), &wait);
> + for (;;) {
> + set_current_state(TASK_INTERRUPTIBLE);
> +
> + if (!skb_queue_empty(&sk->sk_receive_queue) ||
> + sk->sk_err ||
> + (sk->sk_shutdown & RCV_SHUTDOWN) ||
> + signal_pending(current) ||
> + !timeo)
> + break;
This makes my brain hurt. Please lets do this readable and with proper
coding style and/or a helper function.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] Fix vcard filter setting
From: Johan Hedberg @ 2010-08-02 19:45 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201007281324.o6SDO2oH010611@joshua.mesa.nl>
Hi,
On Tue, Jul 27, 2010, Marcel Mol wrote:
> The bits for VCARD version 2.1 and version 3.0 were wrong:
> oin case of 2.1 the FN bit was on and for 3.0 it is off.
> The comment state that 3.0 needs FN while 2.1 does not.
> This patch fixes that.
> ---
> plugins/phonebook-ebook.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Thanks for the patch. It's now in the upstream obexd tree.
Johan
^ permalink raw reply
* Re: [PATCH 5/8] Bluetooth: Don't modify remote_tx_win when receiving a config response. Only config requests should set remote_tx_win.
From: Marcel Holtmann @ 2010-08-02 19:42 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-6-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
please write a proper commit message. Your subject is not meant to be
used for explaining what is wrong and what is right and how you did it.
It is called a SUBJECT.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 4/8] Bluetooth: Do endianness conversion on MPS configuration value before doing comparisons.
From: Marcel Holtmann @ 2010-08-02 19:41 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-5-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
same for subject line vs commit message. And please explain why and if
this fixes a bug.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/8] Bluetooth: Change default ERTM retransmit timeout to 2 seconds, as the spec requires.
From: Marcel Holtmann @ 2010-08-02 19:40 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-3-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
I do expect some more details commit messages. The subject line is
suppose to around 50 chars and give an idea what the patch is about. The
actual commit message should have the full blown explanation why etc.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/8] Bluetooth: Validate PSM values in calls to connect() and bind().
From: Marcel Holtmann @ 2010-08-02 19:39 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-4-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index aed72f2..44bc6ee 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1008,6 +1008,12 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> goto done;
> }
>
> + /* If specified, PSM must be odd and lsb of upper byte must be 0 */
> + if (la.l2_psm && (__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
> + err = -EINVAL;
> + goto done;
> + }
> +
this was a nice feature of L2CAP in BlueZ actually ;)
I am fine with forbidding this. However, please send a patch that fixes
l2test's default PSM first. Otherwise you gonna break that one.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Marcel Holtmann @ 2010-08-02 19:38 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, rshaffer, linux-arm-msm
In-Reply-To: <1280776810-18213-2-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 9ba1e8e..aed72f2 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> goto unlock;
>
> if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
> - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
> + if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
> + l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
> + (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
> + l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
> l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
this becomes unreadable and my brain starts to throw a core dump. So it
clearly needs to be put into a helper inline function.
Regards
Marcel
^ permalink raw reply
* [PATCH 8/8] Bluetooth: Use 3-DH5 payload size for default ERTM max PDU size.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/l2cap.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 16e412f..93aba17 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -35,7 +35,7 @@
#define L2CAP_DEFAULT_MAX_TX 3
#define L2CAP_DEFAULT_RETRANS_TO 2000 /* 2 seconds */
#define L2CAP_DEFAULT_MONITOR_TO 12000 /* 12 seconds */
-#define L2CAP_DEFAULT_MAX_PDU_SIZE 672
+#define L2CAP_DEFAULT_MAX_PDU_SIZE 1011 /* Sized for 3-DH5 packet */
#define L2CAP_DEFAULT_ACK_TO 200
#define L2CAP_LOCAL_BUSY_TRIES 12
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 7/8] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 582975b..8e9fa51 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1923,6 +1923,7 @@ done:
static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags)
{
struct sock *sk = sock->sk;
+ int len_or_err;
lock_sock(sk);
@@ -1956,7 +1957,13 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
release_sock(sk);
- return bt_sock_recvmsg(iocb, sock, msg, len, flags);
+ if (sock->type == SOCK_STREAM)
+ len_or_err = bt_sock_stream_recvmsg(iocb, sock, msg, len,
+ flags);
+ else
+ len_or_err = bt_sock_recvmsg(iocb, sock, msg, len, flags);
+
+ return len_or_err;
}
static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __user *optval, unsigned int optlen)
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 6/8] Bluetooth: Move stream-oriented recvmsg code so it can be used by L2CAP.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/bluetooth.h | 2 +
net/bluetooth/af_bluetooth.c | 107 +++++++++++++++++++++++++++++++++++++
net/bluetooth/rfcomm/sock.c | 104 ++---------------------------------
3 files changed, 115 insertions(+), 98 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..08b6c2a 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -126,6 +126,8 @@ int bt_sock_unregister(int proto);
void bt_sock_link(struct bt_sock_list *l, struct sock *s);
void bt_sock_unlink(struct bt_sock_list *l, struct sock *s);
int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg, size_t len, int flags);
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t len, int flags);
uint bt_sock_poll(struct file * file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 421c45b..73047f5 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -265,6 +265,113 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
}
EXPORT_SYMBOL(bt_sock_recvmsg);
+static long bt_sock_data_wait(struct sock *sk, long timeo)
+{
+ DECLARE_WAITQUEUE(wait, current);
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ if (!skb_queue_empty(&sk->sk_receive_queue) ||
+ sk->sk_err ||
+ (sk->sk_shutdown & RCV_SHUTDOWN) ||
+ signal_pending(current) ||
+ !timeo)
+ break;
+
+ set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ release_sock(sk);
+ timeo = schedule_timeout(timeo);
+ lock_sock(sk);
+ clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+ }
+
+ __set_current_state(TASK_RUNNING);
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return timeo;
+}
+
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ size_t target, copied = 0;
+ long timeo;
+
+ if (flags & MSG_OOB)
+ return -EOPNOTSUPP;
+
+ msg->msg_namelen = 0;
+
+ BT_DBG("sk %p size %zu", sk, size);
+
+ lock_sock(sk);
+
+ target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
+ timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
+ do {
+ struct sk_buff *skb;
+ int chunk;
+
+ skb = skb_dequeue(&sk->sk_receive_queue);
+ if (!skb) {
+ if (copied >= target)
+ break;
+
+ if ((err = sock_error(sk)) != 0)
+ break;
+ if (sk->sk_shutdown & RCV_SHUTDOWN)
+ break;
+
+ err = -EAGAIN;
+ if (!timeo)
+ break;
+
+ timeo = bt_sock_data_wait(sk, timeo);
+
+ if (signal_pending(current)) {
+ err = sock_intr_errno(timeo);
+ goto out;
+ }
+ continue;
+ }
+
+ chunk = min_t(unsigned int, skb->len, size);
+ if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ if (!copied)
+ copied = -EFAULT;
+ break;
+ }
+ copied += chunk;
+ size -= chunk;
+
+ sock_recv_ts_and_drops(msg, sk, skb);
+
+ if (!(flags & MSG_PEEK)) {
+ skb_pull(skb, chunk);
+ if (skb->len) {
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ kfree_skb(skb);
+
+ } else {
+ /* put message back and return */
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ break;
+ }
+ } while (size);
+
+out:
+ release_sock(sk);
+ return copied ? : err;
+}
+EXPORT_SYMBOL(bt_sock_stream_recvmsg);
+
static inline unsigned int bt_accept_poll(struct sock *parent)
{
struct list_head *p, *n;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..5c92929 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -617,121 +617,29 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
return sent;
}
-static long rfcomm_sock_data_wait(struct sock *sk, long timeo)
-{
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue(sk_sleep(sk), &wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
-
- if (!skb_queue_empty(&sk->sk_receive_queue) ||
- sk->sk_err ||
- (sk->sk_shutdown & RCV_SHUTDOWN) ||
- signal_pending(current) ||
- !timeo)
- break;
-
- set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- release_sock(sk);
- timeo = schedule_timeout(timeo);
- lock_sock(sk);
- clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
- }
-
- __set_current_state(TASK_RUNNING);
- remove_wait_queue(sk_sleep(sk), &wait);
- return timeo;
-}
-
static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
struct msghdr *msg, size_t size, int flags)
{
struct sock *sk = sock->sk;
struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
- int err = 0;
- size_t target, copied = 0;
- long timeo;
+ int len_or_err;
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
rfcomm_dlc_accept(d);
return 0;
}
- if (flags & MSG_OOB)
- return -EOPNOTSUPP;
-
- msg->msg_namelen = 0;
-
- BT_DBG("sk %p size %zu", sk, size);
+ len_or_err = bt_sock_stream_recvmsg(iocb, sock, msg, size, flags);
lock_sock(sk);
+ if (!(flags & MSG_PEEK) && len_or_err > 0)
+ atomic_sub(len_or_err, &sk->sk_rmem_alloc);
- target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
- timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
-
- do {
- struct sk_buff *skb;
- int chunk;
-
- skb = skb_dequeue(&sk->sk_receive_queue);
- if (!skb) {
- if (copied >= target)
- break;
-
- if ((err = sock_error(sk)) != 0)
- break;
- if (sk->sk_shutdown & RCV_SHUTDOWN)
- break;
-
- err = -EAGAIN;
- if (!timeo)
- break;
-
- timeo = rfcomm_sock_data_wait(sk, timeo);
-
- if (signal_pending(current)) {
- err = sock_intr_errno(timeo);
- goto out;
- }
- continue;
- }
-
- chunk = min_t(unsigned int, skb->len, size);
- if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- if (!copied)
- copied = -EFAULT;
- break;
- }
- copied += chunk;
- size -= chunk;
-
- sock_recv_ts_and_drops(msg, sk, skb);
-
- if (!(flags & MSG_PEEK)) {
- atomic_sub(chunk, &sk->sk_rmem_alloc);
-
- skb_pull(skb, chunk);
- if (skb->len) {
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- kfree_skb(skb);
-
- } else {
- /* put message back and return */
- skb_queue_head(&sk->sk_receive_queue, skb);
- break;
- }
- } while (size);
-
-out:
if (atomic_read(&sk->sk_rmem_alloc) <= (sk->sk_rcvbuf >> 2))
rfcomm_dlc_unthrottle(rfcomm_pi(sk)->dlc);
-
release_sock(sk);
- return copied ? : err;
+
+ return len_or_err;
}
static int rfcomm_sock_setsockopt_old(struct socket *sock, int optname, char __user *optval, unsigned int optlen)
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 5/8] Bluetooth: Don't modify remote_tx_win when receiving a config response. Only config requests should set remote_tx_win.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9780ab0..582975b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2808,7 +2808,6 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
if (*result == L2CAP_CONF_SUCCESS) {
switch (rfc.mode) {
case L2CAP_MODE_ERTM:
- pi->remote_tx_win = rfc.txwin_size;
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);
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 4/8] Bluetooth: Do endianness conversion on MPS configuration value before doing comparisons.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 44bc6ee..9780ab0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2708,10 +2708,10 @@ done:
case L2CAP_MODE_ERTM:
pi->remote_tx_win = rfc.txwin_size;
pi->remote_max_tx = rfc.max_transmit;
- if (rfc.max_pdu_size > pi->conn->mtu - 10)
- rfc.max_pdu_size = le16_to_cpu(pi->conn->mtu - 10);
pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
+ if (pi->remote_mps > pi->conn->mtu - 10)
+ pi->remote_mps = pi->conn->mtu - 10;
rfc.retrans_timeout =
le16_to_cpu(L2CAP_DEFAULT_RETRANS_TO);
@@ -2726,10 +2726,9 @@ done:
break;
case L2CAP_MODE_STREAMING:
- if (rfc.max_pdu_size > pi->conn->mtu - 10)
- rfc.max_pdu_size = le16_to_cpu(pi->conn->mtu - 10);
-
pi->remote_mps = le16_to_cpu(rfc.max_pdu_size);
+ if (pi->remote_mps > pi->conn->mtu - 10)
+ pi->remote_mps = pi->conn->mtu - 10;
pi->conf_state |= L2CAP_CONF_MODE_DONE;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 3/8] Bluetooth: Validate PSM values in calls to connect() and bind().
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index aed72f2..44bc6ee 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1008,6 +1008,12 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
goto done;
}
+ /* If specified, PSM must be odd and lsb of upper byte must be 0 */
+ if (la.l2_psm && (__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
if (la.l2_psm && __le16_to_cpu(la.l2_psm) < 0x1001 &&
!capable(CAP_NET_BIND_SERVICE)) {
err = -EACCES;
@@ -1190,6 +1196,12 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
goto done;
}
+ /* PSM must be odd and lsb of upper byte must be 0 */
+ if ((__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
/* Set destination address and psm */
bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 2/8] Bluetooth: Change default ERTM retransmit timeout to 2 seconds, as the spec requires.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
include/net/bluetooth/l2cap.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 636724b..16e412f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -33,7 +33,7 @@
#define L2CAP_DEFAULT_FLUSH_TO 0xffff
#define L2CAP_DEFAULT_TX_WINDOW 63
#define L2CAP_DEFAULT_MAX_TX 3
-#define L2CAP_DEFAULT_RETRANS_TO 1000 /* 1 second */
+#define L2CAP_DEFAULT_RETRANS_TO 2000 /* 2 seconds */
#define L2CAP_DEFAULT_MONITOR_TO 12000 /* 12 seconds */
#define L2CAP_DEFAULT_MAX_PDU_SIZE 672
#define L2CAP_DEFAULT_ACK_TO 200
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 1/8] Bluetooth: Make sure the L2CAP FCS is only enabled for ERTM or streaming.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm, Mat Martineau
In-Reply-To: <1280776810-18213-1-git-send-email-mathewm@codeaurora.org>
Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
---
net/bluetooth/l2cap.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9ba1e8e..aed72f2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3127,8 +3127,10 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
goto unlock;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
+ if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
+ l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
+ (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
+ l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
sk->sk_state = BT_CONNECTED;
@@ -3217,8 +3219,10 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_OUTPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
+ if ((l2cap_pi(sk)->mode == L2CAP_MODE_ERTM ||
+ l2cap_pi(sk)->mode == L2CAP_MODE_STREAMING) &&
+ (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
+ l2cap_pi(sk)->fcs != L2CAP_FCS_NONE))
l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
sk->sk_state = BT_CONNECTED;
--
1.7.1
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply related
* [PATCH 0/8] Bluetooth: L2CAP updates for PSM validation and ERTM.
From: Mat Martineau @ 2010-08-02 19:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, gustavo, rshaffer, linux-arm-msm
This is a collection of ERTM-related patches, including some L2CAP
configuration fixes and allowing partial-frame reads from L2CAP
SOCK_STREAM sockets. PSM validation is also included, although that
is not restricted to ERTM.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: Getting link quality or RSSI
From: Marcel Holtmann @ 2010-08-02 18:58 UTC (permalink / raw)
To: Davide Pesavento; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimQeQ-XoM5yuWQYx5_1-xWf1H0ax_J=BbtDfG2T@mail.gmail.com>
Hi Davide,
> >> I'm writing an application to retrieve the current link quality (or
> >> RSSI) between my laptop's Bluetooth adapter and a specific remote
> >> device (my mobile phone). In order to do that I'm of course using the
> >> latest version of BlueZ, but I've encountered several issues so far.
> >>
> >> The dbus API exposed by bluetoothd doesn't have any GetRSSI or
> >> GetLinkQuality methods. Would it be possible to export these methods
> >> in the public API?
> >
> > the link quality is vendor specific according to the specification and
> > the RSSI of an existing connection is rather useless. So we don't bother
> > to export those.
> >
>
> Does this mean you're going to reject a patch which adds those methods
> to the dbus API?
constantly polling them via D-Bus, yes I would reject such a patch. To
make this proper you would need a kernel patch first that polls the RSSI
and/or link quality when a ACL is established and not in power saving
mode. Then you need to use this data to send async signals via D-Bus.
I have done both, let me assure you that some chips don't provide proper
RSSI values. Then link quality is vendor specific and we can't do
anything real useful with it (except it is a CSR chip). So I have don't
the whole exercise and figured out that it is rather useless feature of
Bluetooth.
> >> Then I looked at the bluetooth HCI library that comes with bluez.
> >> First of all, is there any reasons why it's totally undocumented?
> >> Anyway, I noticed hci_read_link_quality() and hci_read_rssi() in
> >> hci_lib.h and tried using them. Since they seem to require an
> >> established connection, I also used hci_create_connection(). However I
> >> soon discovered that creating a connection requires root privileges,
> >> is that right or did I do something wrong?
> >>
> >> Furthermore, even when running the program as root, the connection
> >> gets established only for a few seconds and then it disconnects from
> >> the remote device. Is this behaviour intended? How can I specify to
> >> keep the connection alive indefinitely?
> >
> > If you don't have an active connection that is used, the kernel will
> > terminate any idle ones. So using hcitool for this is rather pointless
> > unless you have a profile already using that connection.
> >
>
> "using a connection" means pushing some traffic over it?
> Is there no other way to prevent the kernel from terminating idle connections?
You don't need to transfer data, but you need a reference count on the
ACL link. And that can only happen by opening a L2CAP socket.
> By the way, I've read in the Bluetooth specs that there exists an
> extended inquiry mode which allows the host to gather the RSSI of
> available devices too. How can I perform such kind of inquiry from my
> application using bluez?
Nice idea, but RSSI from inquiry result and RSSI from an ACL are not the
same. You can't compare them properly to make sense out of them. I tried
that as well. Please read the specification again to see their
difference when it comes to power control on the low level baseband.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v4 0/5] Enhanced support for extended inquiry response
From: Johan Hedberg @ 2010-08-02 18:55 UTC (permalink / raw)
To: ingas; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <feb3c412e1be4c1b7b42efdbc581aabb.squirrel@www.codeaurora.org>
Hi Inga,
First of all, sorry for the slight delay in my reply. I've been on
holiday last week with quite limted email processing capability.
On Wed, Jul 28, 2010, ingas@codeaurora.org wrote:
> In the current implementation, EIR write is keyed off HCI Command
> Complete events for a set of commands (in security.c).
> - When first registering an OPP service with sdptool, the
> update_svcclass_list() (sdpd-service.c) calls manager_update_svc()
> (manager.c) which in turn calls adapter_update() (adapter.c). BTW, why
> this level of indirection, since manager_update_svc() is called only from
> one place in the code and calling adapter_update() is all it does?
> Probably some earlier architectural decision I am not aware of…
That looks pretty stupid to me too. Might be some historical artifact.
Anyway, feel free to send a patch to remove this indirection (i.e. call
adapter_update directly).
> The current implementation may be extended as following:
> Since update_svcclass_list() is being called throughout sdpd-service.c in
> all the places where EIR needs to be updated as well, I propose to modify
> the corresponding function adapter_set_service_classes() (adapter.c) :
>
> **********************
> Before:
> /* If we already have the CoD we want or the cache is enabled or an
> * existing CoD write is in progress just bail out */
> if (adapter->current_cod == adapter->wanted_cod ||
> adapter->cache_enable || adapter->pending_cod)
> return 0;
>
>
> ***********************
>
> After:
> /* If we already have the CoD we want or the cache is enabled or an
> * existing CoD write is in progress just bail out */
> if (adapter->cache_enable || adapter->pending_cod)
> return 0;
>
> /* If we already have the CoD we want, update EIR and return */
> if (adapter->current_cod == adapter->wanted_cod ) {
> update_ext_inquiry_response(adapter);
> return 0;
> }
>
> **********************
> Will that be acceptable? I tested this modification and it’s working with
> both d-bus methods and sdptool.
Seems good to me.
> As a side note, in future it might be a good idea to expose EIR writes to
> an application layer providing external API. The intent of EIR is to allow
> quick scan of the surroundings to find a particular service that a device
> is interested at the moment (and in turn, exposing the services that the
> device choses to expose at the moment) without actually going into full
> blown connection mode. EIR space is at a premium for sophisticated
> devices with multiple services. Not every single service needs to be
> exposed in EIR. It would be nice to be able to pick which one to add. For
> example, if a device supports both headset and handsfree profiles (as all
> of the phones do), it might not be necessary to advertise headset uuid in
> EIR since the headsets out there are mostly legacy devices that cannot
> perform EIR anyway… Also, as more new services that use uuid128 are
> introduced, the EIR buffer pretty quickly if every single one of them is
> added without discretion. Just something to throw out there :)
Good point (not that any concrete examples of such a device/situation
would have come accross yet). One possibility to solve the issue without
directly exposing this to applications would be to have a (rather
static) setting in main.conf of UUIDs to be prioritized over others in
the EIR data.
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