2020-04-30 06:53:01

by Archie Pusaka

[permalink] [raw]
Subject: [Bluez PATCH v1] tools/l2cap-tester: Add test for waiting disconnection response

From: Archie Pusaka <[email protected]>

This is to test the behaviour of L2CAP channel when closed with
shut_down(sock, SHUT_WR). In this case, we should wait until we
receive a disconnection response before raising G_IO_HUP.
---

emulator/bthost.c | 17 ++++++++++-
emulator/bthost.h | 5 +++-
tools/l2cap-tester.c | 65 ++++++++++++++++++++++++++++++++++++++-----
tools/rfcomm-tester.c | 2 +-
4 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/emulator/bthost.c b/emulator/bthost.c
index 0fa283464..71d5f97bb 100644
--- a/emulator/bthost.c
+++ b/emulator/bthost.c
@@ -180,6 +180,7 @@ struct l2cap_pending_req {
struct l2cap_conn_cb_data {
uint16_t psm;
bthost_l2cap_connect_cb func;
+ bthost_l2cap_disconnect_cb disconn_func;
void *user_data;
struct l2cap_conn_cb_data *next;
};
@@ -1510,7 +1511,9 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
uint8_t ident, const void *data, uint16_t len)
{
const struct bt_l2cap_pdu_disconn_req *req = data;
+ struct l2cap_conn_cb_data *cb_data;
struct bt_l2cap_pdu_disconn_rsp rsp;
+ struct l2conn *l2conn;

if (len < sizeof(*req))
return false;
@@ -1522,6 +1525,15 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
l2cap_sig_send(bthost, conn, BT_L2CAP_PDU_DISCONN_RSP, ident, &rsp,
sizeof(rsp));

+ l2conn = btconn_find_l2cap_conn_by_scid(conn, rsp.scid);
+ if (!l2conn)
+ return true;
+
+ cb_data = bthost_find_l2cap_cb_by_psm(bthost, l2conn->psm);
+
+ if (cb_data && cb_data->disconn_func)
+ cb_data->disconn_func(cb_data->user_data);
+
return true;
}

@@ -2553,7 +2565,9 @@ uint64_t bthost_conn_get_fixed_chan(struct bthost *bthost, uint16_t handle)
}

void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
- bthost_l2cap_connect_cb func, void *user_data)
+ bthost_l2cap_connect_cb func,
+ bthost_l2cap_disconnect_cb disconn_func,
+ void *user_data)
{
struct l2cap_conn_cb_data *data;

@@ -2564,6 +2578,7 @@ void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
data->psm = psm;
data->user_data = user_data;
data->func = func;
+ data->disconn_func = disconn_func;
data->next = bthost->new_l2cap_conn_data;

bthost->new_l2cap_conn_data = data;
diff --git a/emulator/bthost.h b/emulator/bthost.h
index b5f36964d..2b347136f 100644
--- a/emulator/bthost.h
+++ b/emulator/bthost.h
@@ -100,9 +100,12 @@ void bthost_le_start_encrypt(struct bthost *bthost, uint16_t handle,
const uint8_t ltk[16]);
typedef void (*bthost_l2cap_connect_cb) (uint16_t handle, uint16_t cid,
void *user_data);
+typedef void (*bthost_l2cap_disconnect_cb) (void *user_data);

void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
- bthost_l2cap_connect_cb func, void *user_data);
+ bthost_l2cap_connect_cb func,
+ bthost_l2cap_disconnect_cb disconn_func,
+ void *user_data);

void bthost_set_sc_support(struct bthost *bthost, bool enable);

diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
index 844ff2b46..1ef4e9213 100644
--- a/tools/l2cap-tester.c
+++ b/tools/l2cap-tester.c
@@ -55,6 +55,7 @@ struct test_data {
uint16_t dcid;
int sk;
int sk2;
+ bool host_disconnected;
};

struct l2cap_data {
@@ -93,6 +94,8 @@ struct l2cap_data {
bool server_not_advertising;
bool direct_advertising;
bool close_1;
+
+ bool shut_sock_wr;
};

static void mgmt_debug(const char *str, void *user_data)
@@ -316,6 +319,12 @@ static const struct l2cap_data client_connect_write_success_test = {
.data_len = sizeof(l2_data),
};

+static const struct l2cap_data client_connect_shut_wr_success_test = {
+ .client_psm = 0x1001,
+ .server_psm = 0x1001,
+ .shut_sock_wr = true,
+};
+
static const struct l2cap_data client_connect_nval_psm_test_1 = {
.client_psm = 0x1001,
.expect_err = ECONNREFUSED,
@@ -967,6 +976,27 @@ static void server_bthost_received_data(const void *buf, uint16_t len,
tester_test_passed();
}

+static gboolean socket_closed_cb(GIOChannel *io, GIOCondition cond,
+ gpointer user_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct l2cap_data *l2data = data->test_data;
+
+ if (l2data->shut_sock_wr) {
+ /* if socket is closed using SHUT_WR, L2CAP disconnection
+ * response must be received first before G_IO_HUP event.
+ */
+ if (data->host_disconnected)
+ tester_test_passed();
+ else {
+ tester_warn("G_IO_HUP received before receiving L2CAP disconnection");
+ tester_test_failed();
+ }
+ }
+
+ return FALSE;
+}
+
static bool check_mtu(struct test_data *data, int sk)
{
const struct l2cap_data *l2data = data->test_data;
@@ -1062,6 +1092,11 @@ static gboolean l2cap_connect_cb(GIOChannel *io, GIOCondition cond,
tester_test_failed();
}

+ return FALSE;
+ } else if (l2data->shut_sock_wr) {
+ g_io_add_watch(io, G_IO_HUP, socket_closed_cb, NULL);
+ shutdown(sk, SHUT_WR);
+
return FALSE;
}

@@ -1214,6 +1249,13 @@ static void client_l2cap_connect_cb(uint16_t handle, uint16_t cid,
data->handle = handle;
}

+static void client_l2cap_disconnect_cb(void *user_data)
+{
+ struct test_data *data = user_data;
+
+ data->host_disconnected = true;
+}
+
static void direct_adv_cmd_complete(uint16_t opcode, const void *param,
uint8_t len, void *user_data)
{
@@ -1254,13 +1296,18 @@ static void test_connect(const void *test_data)

if (l2data->server_psm) {
struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+ bthost_l2cap_connect_cb host_connect_cb = NULL;
+ bthost_l2cap_disconnect_cb host_disconnect_cb = NULL;

- if (!l2data->data_len)
- bthost_add_l2cap_server(bthost, l2data->server_psm,
- NULL, NULL);
- else
- bthost_add_l2cap_server(bthost, l2data->server_psm,
- client_l2cap_connect_cb, data);
+ if (l2data->data_len)
+ host_connect_cb = client_l2cap_connect_cb;
+
+ if (l2data->shut_sock_wr)
+ host_disconnect_cb = client_l2cap_disconnect_cb;
+
+ bthost_add_l2cap_server(bthost, l2data->server_psm,
+ host_connect_cb, host_disconnect_cb,
+ data);
}

if (l2data->direct_advertising)
@@ -1639,7 +1686,7 @@ static void test_connect_2(const void *test_data)

if (!l2data->data_len)
bthost_add_l2cap_server(bthost, l2data->server_psm,
- NULL, NULL);
+ NULL, NULL, NULL);
}

defer = (l2data->mode == BT_MODE_EXT_FLOWCTL);
@@ -1927,6 +1974,10 @@ int main(int argc, char *argv[])
&client_connect_nval_psm_test_3,
setup_powered_client, test_connect);

+ test_l2cap_bredr("L2CAP BR/EDR Client - Socket Shut WR Success",
+ &client_connect_shut_wr_success_test,
+ setup_powered_client, test_connect);
+
test_l2cap_bredr("L2CAP BR/EDR Server - Success",
&l2cap_server_success_test,
setup_powered_server, test_server);
diff --git a/tools/rfcomm-tester.c b/tools/rfcomm-tester.c
index b20d70d58..2d3be27d8 100644
--- a/tools/rfcomm-tester.c
+++ b/tools/rfcomm-tester.c
@@ -548,7 +548,7 @@ static void test_connect(const void *test_data)
GIOChannel *io;
int sk;

- bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL);
+ bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL, NULL);
bthost_add_rfcomm_server(bthost, cli->server_channel,
rfcomm_connect_cb, NULL);

--
2.26.2.303.gf8c07b1a785-goog


2020-04-30 06:56:49

by Archie Pusaka

[permalink] [raw]
Subject: Re: [Bluez PATCH v1] tools/l2cap-tester: Add test for waiting disconnection response

Hi Marcel,

I add a new test in l2cap-tester that specifically tests for "when
closing socket, wait until we receive disconnection reply before
closing". This will only work if we close with SHUT_WR and after the
corresponding kernel patch (https://lkml.org/lkml/2020/4/22/1564) is
applied.

Is this the test that you have in mind?

On Thu, 30 Apr 2020 at 14:52, Archie Pusaka <[email protected]> wrote:
>
> From: Archie Pusaka <[email protected]>
>
> This is to test the behaviour of L2CAP channel when closed with
> shut_down(sock, SHUT_WR). In this case, we should wait until we
> receive a disconnection response before raising G_IO_HUP.
> ---
>
> emulator/bthost.c | 17 ++++++++++-
> emulator/bthost.h | 5 +++-
> tools/l2cap-tester.c | 65 ++++++++++++++++++++++++++++++++++++++-----
> tools/rfcomm-tester.c | 2 +-
> 4 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/emulator/bthost.c b/emulator/bthost.c
> index 0fa283464..71d5f97bb 100644
> --- a/emulator/bthost.c
> +++ b/emulator/bthost.c
> @@ -180,6 +180,7 @@ struct l2cap_pending_req {
> struct l2cap_conn_cb_data {
> uint16_t psm;
> bthost_l2cap_connect_cb func;
> + bthost_l2cap_disconnect_cb disconn_func;
> void *user_data;
> struct l2cap_conn_cb_data *next;
> };
> @@ -1510,7 +1511,9 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
> uint8_t ident, const void *data, uint16_t len)
> {
> const struct bt_l2cap_pdu_disconn_req *req = data;
> + struct l2cap_conn_cb_data *cb_data;
> struct bt_l2cap_pdu_disconn_rsp rsp;
> + struct l2conn *l2conn;
>
> if (len < sizeof(*req))
> return false;
> @@ -1522,6 +1525,15 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
> l2cap_sig_send(bthost, conn, BT_L2CAP_PDU_DISCONN_RSP, ident, &rsp,
> sizeof(rsp));
>
> + l2conn = btconn_find_l2cap_conn_by_scid(conn, rsp.scid);
> + if (!l2conn)
> + return true;
> +
> + cb_data = bthost_find_l2cap_cb_by_psm(bthost, l2conn->psm);
> +
> + if (cb_data && cb_data->disconn_func)
> + cb_data->disconn_func(cb_data->user_data);
> +
> return true;
> }
>
> @@ -2553,7 +2565,9 @@ uint64_t bthost_conn_get_fixed_chan(struct bthost *bthost, uint16_t handle)
> }
>
> void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> - bthost_l2cap_connect_cb func, void *user_data)
> + bthost_l2cap_connect_cb func,
> + bthost_l2cap_disconnect_cb disconn_func,
> + void *user_data)
> {
> struct l2cap_conn_cb_data *data;
>
> @@ -2564,6 +2578,7 @@ void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> data->psm = psm;
> data->user_data = user_data;
> data->func = func;
> + data->disconn_func = disconn_func;
> data->next = bthost->new_l2cap_conn_data;
>
> bthost->new_l2cap_conn_data = data;
> diff --git a/emulator/bthost.h b/emulator/bthost.h
> index b5f36964d..2b347136f 100644
> --- a/emulator/bthost.h
> +++ b/emulator/bthost.h
> @@ -100,9 +100,12 @@ void bthost_le_start_encrypt(struct bthost *bthost, uint16_t handle,
> const uint8_t ltk[16]);
> typedef void (*bthost_l2cap_connect_cb) (uint16_t handle, uint16_t cid,
> void *user_data);
> +typedef void (*bthost_l2cap_disconnect_cb) (void *user_data);
>
> void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> - bthost_l2cap_connect_cb func, void *user_data);
> + bthost_l2cap_connect_cb func,
> + bthost_l2cap_disconnect_cb disconn_func,
> + void *user_data);
>
> void bthost_set_sc_support(struct bthost *bthost, bool enable);
>
> diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
> index 844ff2b46..1ef4e9213 100644
> --- a/tools/l2cap-tester.c
> +++ b/tools/l2cap-tester.c
> @@ -55,6 +55,7 @@ struct test_data {
> uint16_t dcid;
> int sk;
> int sk2;
> + bool host_disconnected;
> };
>
> struct l2cap_data {
> @@ -93,6 +94,8 @@ struct l2cap_data {
> bool server_not_advertising;
> bool direct_advertising;
> bool close_1;
> +
> + bool shut_sock_wr;
> };
>
> static void mgmt_debug(const char *str, void *user_data)
> @@ -316,6 +319,12 @@ static const struct l2cap_data client_connect_write_success_test = {
> .data_len = sizeof(l2_data),
> };
>
> +static const struct l2cap_data client_connect_shut_wr_success_test = {
> + .client_psm = 0x1001,
> + .server_psm = 0x1001,
> + .shut_sock_wr = true,
> +};
> +
> static const struct l2cap_data client_connect_nval_psm_test_1 = {
> .client_psm = 0x1001,
> .expect_err = ECONNREFUSED,
> @@ -967,6 +976,27 @@ static void server_bthost_received_data(const void *buf, uint16_t len,
> tester_test_passed();
> }
>
> +static gboolean socket_closed_cb(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct test_data *data = tester_get_data();
> + const struct l2cap_data *l2data = data->test_data;
> +
> + if (l2data->shut_sock_wr) {
> + /* if socket is closed using SHUT_WR, L2CAP disconnection
> + * response must be received first before G_IO_HUP event.
> + */
> + if (data->host_disconnected)
> + tester_test_passed();
> + else {
> + tester_warn("G_IO_HUP received before receiving L2CAP disconnection");
> + tester_test_failed();
> + }
> + }
> +
> + return FALSE;
> +}
> +
> static bool check_mtu(struct test_data *data, int sk)
> {
> const struct l2cap_data *l2data = data->test_data;
> @@ -1062,6 +1092,11 @@ static gboolean l2cap_connect_cb(GIOChannel *io, GIOCondition cond,
> tester_test_failed();
> }
>
> + return FALSE;
> + } else if (l2data->shut_sock_wr) {
> + g_io_add_watch(io, G_IO_HUP, socket_closed_cb, NULL);
> + shutdown(sk, SHUT_WR);
> +
> return FALSE;
> }
>
> @@ -1214,6 +1249,13 @@ static void client_l2cap_connect_cb(uint16_t handle, uint16_t cid,
> data->handle = handle;
> }
>
> +static void client_l2cap_disconnect_cb(void *user_data)
> +{
> + struct test_data *data = user_data;
> +
> + data->host_disconnected = true;
> +}
> +
> static void direct_adv_cmd_complete(uint16_t opcode, const void *param,
> uint8_t len, void *user_data)
> {
> @@ -1254,13 +1296,18 @@ static void test_connect(const void *test_data)
>
> if (l2data->server_psm) {
> struct bthost *bthost = hciemu_client_get_host(data->hciemu);
> + bthost_l2cap_connect_cb host_connect_cb = NULL;
> + bthost_l2cap_disconnect_cb host_disconnect_cb = NULL;
>
> - if (!l2data->data_len)
> - bthost_add_l2cap_server(bthost, l2data->server_psm,
> - NULL, NULL);
> - else
> - bthost_add_l2cap_server(bthost, l2data->server_psm,
> - client_l2cap_connect_cb, data);
> + if (l2data->data_len)
> + host_connect_cb = client_l2cap_connect_cb;
> +
> + if (l2data->shut_sock_wr)
> + host_disconnect_cb = client_l2cap_disconnect_cb;
> +
> + bthost_add_l2cap_server(bthost, l2data->server_psm,
> + host_connect_cb, host_disconnect_cb,
> + data);
> }
>
> if (l2data->direct_advertising)
> @@ -1639,7 +1686,7 @@ static void test_connect_2(const void *test_data)
>
> if (!l2data->data_len)
> bthost_add_l2cap_server(bthost, l2data->server_psm,
> - NULL, NULL);
> + NULL, NULL, NULL);
> }
>
> defer = (l2data->mode == BT_MODE_EXT_FLOWCTL);
> @@ -1927,6 +1974,10 @@ int main(int argc, char *argv[])
> &client_connect_nval_psm_test_3,
> setup_powered_client, test_connect);
>
> + test_l2cap_bredr("L2CAP BR/EDR Client - Socket Shut WR Success",
> + &client_connect_shut_wr_success_test,
> + setup_powered_client, test_connect);
> +
> test_l2cap_bredr("L2CAP BR/EDR Server - Success",
> &l2cap_server_success_test,
> setup_powered_server, test_server);
> diff --git a/tools/rfcomm-tester.c b/tools/rfcomm-tester.c
> index b20d70d58..2d3be27d8 100644
> --- a/tools/rfcomm-tester.c
> +++ b/tools/rfcomm-tester.c
> @@ -548,7 +548,7 @@ static void test_connect(const void *test_data)
> GIOChannel *io;
> int sk;
>
> - bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL);
> + bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL, NULL);
> bthost_add_rfcomm_server(bthost, cli->server_channel,
> rfcomm_connect_cb, NULL);
>
> --
> 2.26.2.303.gf8c07b1a785-goog
>

2020-04-30 07:08:35

by bluez.test.bot

[permalink] [raw]
Subject: RE: [Bluez,v1] tools/l2cap-tester: Add test for waiting disconnection response


This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.


Test Result:
Checkbuild Failed

Patch Series:
[Bluez,v1] tools/l2cap-tester: Add test for waiting disconnection response


Outputs:
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
android/tester-main.c: In function ‘emu_add_l2cap_server_action’:
android/tester-main.c:2869:2: error: too few arguments to function ‘bthost_add_l2cap_server’
2869 | bthost_add_l2cap_server(bthost, l2cap_data->psm, l2cap_data->func,
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from android/tester-main.c:33:
./emulator/bthost.h:105:6: note: declared here
105 | void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
| ^~~~~~~~~~~~~~~~~~~~~~~
make[1]: *** [Makefile:7165: android/android_tester-tester-main.o] Error 1
make: *** [Makefile:4010: all] Error 2



---
Regards,
Linux Bluetooth

2020-04-30 17:25:36

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [Bluez PATCH v1] tools/l2cap-tester: Add test for waiting disconnection response

Hi Archie,

On Wed, Apr 29, 2020 at 11:52 PM Archie Pusaka <[email protected]> wrote:
>
> From: Archie Pusaka <[email protected]>
>
> This is to test the behaviour of L2CAP channel when closed with
> shut_down(sock, SHUT_WR). In this case, we should wait until we
> receive a disconnection response before raising G_IO_HUP.
> ---
>
> emulator/bthost.c | 17 ++++++++++-
> emulator/bthost.h | 5 +++-
> tools/l2cap-tester.c | 65 ++++++++++++++++++++++++++++++++++++++-----
> tools/rfcomm-tester.c | 2 +-
> 4 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/emulator/bthost.c b/emulator/bthost.c
> index 0fa283464..71d5f97bb 100644
> --- a/emulator/bthost.c
> +++ b/emulator/bthost.c
> @@ -180,6 +180,7 @@ struct l2cap_pending_req {
> struct l2cap_conn_cb_data {
> uint16_t psm;
> bthost_l2cap_connect_cb func;
> + bthost_l2cap_disconnect_cb disconn_func;
> void *user_data;
> struct l2cap_conn_cb_data *next;
> };
> @@ -1510,7 +1511,9 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
> uint8_t ident, const void *data, uint16_t len)
> {
> const struct bt_l2cap_pdu_disconn_req *req = data;
> + struct l2cap_conn_cb_data *cb_data;
> struct bt_l2cap_pdu_disconn_rsp rsp;
> + struct l2conn *l2conn;
>
> if (len < sizeof(*req))
> return false;
> @@ -1522,6 +1525,15 @@ static bool l2cap_disconn_req(struct bthost *bthost, struct btconn *conn,
> l2cap_sig_send(bthost, conn, BT_L2CAP_PDU_DISCONN_RSP, ident, &rsp,
> sizeof(rsp));
>
> + l2conn = btconn_find_l2cap_conn_by_scid(conn, rsp.scid);
> + if (!l2conn)
> + return true;
> +
> + cb_data = bthost_find_l2cap_cb_by_psm(bthost, l2conn->psm);
> +
> + if (cb_data && cb_data->disconn_func)
> + cb_data->disconn_func(cb_data->user_data);
> +
> return true;
> }
>
> @@ -2553,7 +2565,9 @@ uint64_t bthost_conn_get_fixed_chan(struct bthost *bthost, uint16_t handle)
> }
>
> void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> - bthost_l2cap_connect_cb func, void *user_data)
> + bthost_l2cap_connect_cb func,
> + bthost_l2cap_disconnect_cb disconn_func,
> + void *user_data)
> {
> struct l2cap_conn_cb_data *data;
>
> @@ -2564,6 +2578,7 @@ void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> data->psm = psm;
> data->user_data = user_data;
> data->func = func;
> + data->disconn_func = disconn_func;
> data->next = bthost->new_l2cap_conn_data;
>
> bthost->new_l2cap_conn_data = data;
> diff --git a/emulator/bthost.h b/emulator/bthost.h
> index b5f36964d..2b347136f 100644
> --- a/emulator/bthost.h
> +++ b/emulator/bthost.h
> @@ -100,9 +100,12 @@ void bthost_le_start_encrypt(struct bthost *bthost, uint16_t handle,
> const uint8_t ltk[16]);
> typedef void (*bthost_l2cap_connect_cb) (uint16_t handle, uint16_t cid,
> void *user_data);
> +typedef void (*bthost_l2cap_disconnect_cb) (void *user_data);
>
> void bthost_add_l2cap_server(struct bthost *bthost, uint16_t psm,
> - bthost_l2cap_connect_cb func, void *user_data);
> + bthost_l2cap_connect_cb func,
> + bthost_l2cap_disconnect_cb disconn_func,
> + void *user_data);
>
> void bthost_set_sc_support(struct bthost *bthost, bool enable);
>
> diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
> index 844ff2b46..1ef4e9213 100644
> --- a/tools/l2cap-tester.c
> +++ b/tools/l2cap-tester.c
> @@ -55,6 +55,7 @@ struct test_data {
> uint16_t dcid;
> int sk;
> int sk2;
> + bool host_disconnected;
> };
>
> struct l2cap_data {
> @@ -93,6 +94,8 @@ struct l2cap_data {
> bool server_not_advertising;
> bool direct_advertising;
> bool close_1;
> +
> + bool shut_sock_wr;
> };
>
> static void mgmt_debug(const char *str, void *user_data)
> @@ -316,6 +319,12 @@ static const struct l2cap_data client_connect_write_success_test = {
> .data_len = sizeof(l2_data),
> };
>
> +static const struct l2cap_data client_connect_shut_wr_success_test = {
> + .client_psm = 0x1001,
> + .server_psm = 0x1001,
> + .shut_sock_wr = true,
> +};
> +
> static const struct l2cap_data client_connect_nval_psm_test_1 = {
> .client_psm = 0x1001,
> .expect_err = ECONNREFUSED,
> @@ -967,6 +976,27 @@ static void server_bthost_received_data(const void *buf, uint16_t len,
> tester_test_passed();
> }
>
> +static gboolean socket_closed_cb(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct test_data *data = tester_get_data();
> + const struct l2cap_data *l2data = data->test_data;
> +
> + if (l2data->shut_sock_wr) {
> + /* if socket is closed using SHUT_WR, L2CAP disconnection
> + * response must be received first before G_IO_HUP event.
> + */
> + if (data->host_disconnected)
> + tester_test_passed();
> + else {
> + tester_warn("G_IO_HUP received before receiving L2CAP disconnection");
> + tester_test_failed();
> + }
> + }
> +
> + return FALSE;
> +}
> +
> static bool check_mtu(struct test_data *data, int sk)
> {
> const struct l2cap_data *l2data = data->test_data;
> @@ -1062,6 +1092,11 @@ static gboolean l2cap_connect_cb(GIOChannel *io, GIOCondition cond,
> tester_test_failed();
> }
>
> + return FALSE;
> + } else if (l2data->shut_sock_wr) {
> + g_io_add_watch(io, G_IO_HUP, socket_closed_cb, NULL);
> + shutdown(sk, SHUT_WR);

Id make the test abort if cannot be disconnected properly, but it
looks like we don't have any means to detect if the kernel has support
for it or not, but perhaps it is fine since this is trying to test the
right behavior.

> +
> return FALSE;
> }
>
> @@ -1214,6 +1249,13 @@ static void client_l2cap_connect_cb(uint16_t handle, uint16_t cid,
> data->handle = handle;
> }
>
> +static void client_l2cap_disconnect_cb(void *user_data)
> +{
> + struct test_data *data = user_data;
> +
> + data->host_disconnected = true;
> +}
> +
> static void direct_adv_cmd_complete(uint16_t opcode, const void *param,
> uint8_t len, void *user_data)
> {
> @@ -1254,13 +1296,18 @@ static void test_connect(const void *test_data)
>
> if (l2data->server_psm) {
> struct bthost *bthost = hciemu_client_get_host(data->hciemu);
> + bthost_l2cap_connect_cb host_connect_cb = NULL;
> + bthost_l2cap_disconnect_cb host_disconnect_cb = NULL;
>
> - if (!l2data->data_len)
> - bthost_add_l2cap_server(bthost, l2data->server_psm,
> - NULL, NULL);
> - else
> - bthost_add_l2cap_server(bthost, l2data->server_psm,
> - client_l2cap_connect_cb, data);
> + if (l2data->data_len)
> + host_connect_cb = client_l2cap_connect_cb;
> +
> + if (l2data->shut_sock_wr)
> + host_disconnect_cb = client_l2cap_disconnect_cb;
> +
> + bthost_add_l2cap_server(bthost, l2data->server_psm,
> + host_connect_cb, host_disconnect_cb,
> + data);
> }
>
> if (l2data->direct_advertising)
> @@ -1639,7 +1686,7 @@ static void test_connect_2(const void *test_data)
>
> if (!l2data->data_len)
> bthost_add_l2cap_server(bthost, l2data->server_psm,
> - NULL, NULL);
> + NULL, NULL, NULL);
> }
>
> defer = (l2data->mode == BT_MODE_EXT_FLOWCTL);
> @@ -1927,6 +1974,10 @@ int main(int argc, char *argv[])
> &client_connect_nval_psm_test_3,
> setup_powered_client, test_connect);
>
> + test_l2cap_bredr("L2CAP BR/EDR Client - Socket Shut WR Success",
> + &client_connect_shut_wr_success_test,
> + setup_powered_client, test_connect);
> +
> test_l2cap_bredr("L2CAP BR/EDR Server - Success",
> &l2cap_server_success_test,
> setup_powered_server, test_server);
> diff --git a/tools/rfcomm-tester.c b/tools/rfcomm-tester.c
> index b20d70d58..2d3be27d8 100644
> --- a/tools/rfcomm-tester.c
> +++ b/tools/rfcomm-tester.c
> @@ -548,7 +548,7 @@ static void test_connect(const void *test_data)
> GIOChannel *io;
> int sk;
>
> - bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL);
> + bthost_add_l2cap_server(bthost, 0x0003, NULL, NULL, NULL);
> bthost_add_rfcomm_server(bthost, cli->server_channel,
> rfcomm_connect_cb, NULL);
>
> --
> 2.26.2.303.gf8c07b1a785-goog
>


--
Luiz Augusto von Dentz

2020-05-05 04:37:58

by Archie Pusaka

[permalink] [raw]
Subject: Re: [Bluez PATCH v1] tools/l2cap-tester: Add test for waiting disconnection response

Hi Luiz,

> Id make the test abort if cannot be disconnected properly, but it
> looks like we don't have any means to detect if the kernel has support
> for it or not, but perhaps it is fine since this is trying to test the
> right behavior.
>
Yeah, currently we have no way of knowing whether the kernel supports
this behavior or not. If we really want to, perhaps we can add
something to query that, but I doubt that is worth the hassle.