2014-01-25 01:54:00

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi,

These patches extend existing tests in l2cap-tester to exercise two common
operations on LE sockets:

* getsockopt(BT_SECURITY) on client side (to set security level)
* setsockopt(L2CAP_OPTIONS) on server side (called by bt_io_get())

At the moment, there is a regression on kernel introduced by commit
a5a1e0e6b9c1dea3696192b5ec153d03917eb7b8 ("Bluetooth: Switch ATT channels to
use L2CAP_CHAN_FIXED") which is affecting these two operations. I'm currently
implementing and testing fixes for this regression.

PS: For some reason, the following tests are failing on my system, even using
an older kernel:

L2CAP LE Client - Success
L2CAP LE Client SMP - Success
L2CAP LE Client - Command Reject
L2CAP LE Client - Invalid PSM
L2CAP LE Server - Success

connect() is returning EINVAL. Any ideas for why this may be happening?

Best Regards,
Anderson Lizardo

Anderson Lizardo (2):
tools/l2cap-tester: Test setsockopt(BT_SECURITY) on LE client
tools/l2cap-tester: Test getsockopt(L2CAP_OPTIONS) on LE server

tools/l2cap-tester.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)

--
1.8.3.2



2014-01-25 23:16:18

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi Johan,

On Sat, Jan 25, 2014 at 7:13 PM, Johan Hedberg <[email protected]> wrote:
>> What about the suggestion to make l2cap-tester check if LE CoC is
>> enabled on the kernel and skip the the tests that depend on it? Right
>> now, the tests simply fail which is misleading (at first I thought it
>> was a regression).
>>
>> Another option is to change the test name to mention that they require
>> enabling LE CoC.
>
> This has never bothered me much since the assumption is that mainly
> developers would be running these tools and therefore know the
> implications and requirements. I have a feeling that adding this kind of
> checks might be a bit overkill, especially since this debugfs entry will
> disappear as soon as we do one more test run at the UPF next week. After
> that LE CoC support will always be there if you've got a new enough
> kernel.

If the are plans to have this option removed real soon, no problem for me then.

Best Regards,
--
Anderson Lizardo
INdT - Manaus - Brazil

2014-01-25 23:13:09

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi Lizardo,

On Sat, Jan 25, 2014, Anderson Lizardo wrote:
> On Sat, Jan 25, 2014 at 6:12 PM, Johan Hedberg <[email protected]> wrote:
> > I pushed your first patch and a slightly different version of the
> > second one which is more thorough in its checks. I also sent fixes for
> > the kernel side so you don't need to spend more time on that (unless you
> > disagree with the fixes I sent).
>
> Thanks! I looked at the patches and I have no comments. Will do tests
> and report problems (if any).
>
> What about the suggestion to make l2cap-tester check if LE CoC is
> enabled on the kernel and skip the the tests that depend on it? Right
> now, the tests simply fail which is misleading (at first I thought it
> was a regression).
>
> Another option is to change the test name to mention that they require
> enabling LE CoC.

This has never bothered me much since the assumption is that mainly
developers would be running these tools and therefore know the
implications and requirements. I have a feeling that adding this kind of
checks might be a bit overkill, especially since this debugfs entry will
disappear as soon as we do one more test run at the UPF next week. After
that LE CoC support will always be there if you've got a new enough
kernel.

Johan

2014-01-25 22:48:01

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi Johan,

On Sat, Jan 25, 2014 at 6:12 PM, Johan Hedberg <[email protected]> wrote:
> I pushed your first patch and a slightly different version of the
> second one which is more thorough in its checks. I also sent fixes for
> the kernel side so you don't need to spend more time on that (unless you
> disagree with the fixes I sent).

Thanks! I looked at the patches and I have no comments. Will do tests
and report problems (if any).

What about the suggestion to make l2cap-tester check if LE CoC is
enabled on the kernel and skip the the tests that depend on it? Right
now, the tests simply fail which is misleading (at first I thought it
was a regression).

Another option is to change the test name to mention that they require
enabling LE CoC.

Best Regards,
--
Anderson Lizardo
INdT - Manaus - Brazil

2014-01-25 22:12:03

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi Lizardo,

On Fri, Jan 24, 2014, Anderson Lizardo wrote:
> These patches extend existing tests in l2cap-tester to exercise two common
> operations on LE sockets:
>
> * getsockopt(BT_SECURITY) on client side (to set security level)
> * setsockopt(L2CAP_OPTIONS) on server side (called by bt_io_get())
>
> At the moment, there is a regression on kernel introduced by commit
> a5a1e0e6b9c1dea3696192b5ec153d03917eb7b8 ("Bluetooth: Switch ATT channels to
> use L2CAP_CHAN_FIXED") which is affecting these two operations. I'm currently
> implementing and testing fixes for this regression.

I pushed your first patch and a slightly different version of the
second one which is more thorough in its checks. I also sent fixes for
the kernel side so you don't need to spend more time on that (unless you
disagree with the fixes I sent).

Johan

2014-01-25 02:50:23

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi again,

On Fri, Jan 24, 2014 at 10:00 PM, Anderson Lizardo
<[email protected]> wrote:
> Actually I know why they fail: the tests try to use a PSM (and not
> CID) on a single mode LE adapter. I just don't understand why this is
> expected to work.

I now realise that this has to do with LE CoC. Would it be okay to
make l2cap-tester check for
/sys/module/bluetooth/parameters/enable_lecoc at test setup and if it
contains "N", skip the test?

Best Regards,
--
Anderson Lizardo
INdT - Manaus - Brazil

2014-01-25 02:00:26

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/2] Basic [sg]etsockopt() testing in l2cap-tester

Hi,

On Fri, Jan 24, 2014 at 9:54 PM, Anderson Lizardo
<[email protected]> wrote:
> PS: For some reason, the following tests are failing on my system, even using
> an older kernel:
>
> L2CAP LE Client - Success
> L2CAP LE Client SMP - Success
> L2CAP LE Client - Command Reject
> L2CAP LE Client - Invalid PSM
> L2CAP LE Server - Success
>
> connect() is returning EINVAL. Any ideas for why this may be happening?

Actually I know why they fail: the tests try to use a PSM (and not
CID) on a single mode LE adapter. I just don't understand why this is
expected to work.

Best Regards,
--
Anderson Lizardo
INdT - Manaus - Brazil

2014-01-25 01:54:01

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH BlueZ 1/2] tools/l2cap-tester: Test setsockopt(BT_SECURITY) on LE client

Although setting a security level using setsockopt(BT_SECURITY) is
optional for LE sockets (it will default to doing
unencrypted/unauthenticated connection), it is a common operation and it
is done by BlueZ daemon and some tools.
---
tools/l2cap-tester.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
index 05202bd..d541f1b 100644
--- a/tools/l2cap-tester.c
+++ b/tools/l2cap-tester.c
@@ -461,6 +461,7 @@ static const struct l2cap_data le_server_success_test = {

static const struct l2cap_data le_att_client_connect_success_test_1 = {
.cid = 0x0004,
+ .sec_level = BT_SECURITY_LOW,
};

static const struct l2cap_data le_att_server_success_test_1 = {
--
1.8.3.2


2014-01-25 01:54:02

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH BlueZ 2/2] tools/l2cap-tester: Test getsockopt(L2CAP_OPTIONS) on LE server

The btio layer always performs this operation when bt_io_get() is
called. Also check for getsockopt(BT_RCVMTU), which is supported on
kernels with CoC enabled.
---
tools/l2cap-tester.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
index d541f1b..78e7ce6 100644
--- a/tools/l2cap-tester.c
+++ b/tools/l2cap-tester.c
@@ -1034,7 +1034,9 @@ static gboolean l2cap_listen_cb(GIOChannel *io, GIOCondition cond,
{
struct test_data *data = tester_get_data();
const struct l2cap_data *l2data = data->test_data;
- int sk, new_sk;
+ struct l2cap_options l2o;
+ int sk, new_sk, err;
+ socklen_t len;

data->io_id = 0;

@@ -1047,6 +1049,28 @@ static gboolean l2cap_listen_cb(GIOChannel *io, GIOCondition cond,
return FALSE;
}

+ len = sizeof(l2o);
+ memset(&l2o, 0, len);
+
+ /* LE CoC enabled kernels should support BT_RCVMTU */
+ err = getsockopt(new_sk, SOL_BLUETOOTH, BT_RCVMTU, &l2o.imtu, &len);
+ if (err < 0 && errno != EPROTONOSUPPORT && errno != ENOPROTOOPT) {
+ tester_warn("getsockopt(BT_RCVMTU) failed: %s (%d)",
+ strerror(errno), errno);
+ tester_test_failed();
+ return FALSE;
+ }
+
+ /* For non-LE CoC enabled kernels we need to fall back to L2CAP_OPTIONS,
+ * so test support for it as well */
+ err = getsockopt(new_sk, SOL_L2CAP, L2CAP_OPTIONS, &l2o, &len);
+ if (err < 0) {
+ tester_warn("getsockopt(L2CAP_OPTIONS) failed: %s (%d)",
+ strerror(errno), errno);
+ tester_test_failed();
+ return FALSE;
+ }
+
if (l2data->read_data) {
struct bthost *bthost;
GIOChannel *new_io;
--
1.8.3.2