Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200430145105.Bluez.v1.1.I86c1e0e8933d92d8d76dcd95036bf325cfaeced0@changeid>
In-Reply-To: <20200430145105.Bluez.v1.1.I86c1e0e8933d92d8d76dcd95036bf325cfaeced0@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 30 Apr 2020 10:22:52 -0700
Message-ID: <CABBYNZL_rOVYs-3W8DZn_ugh-c23r+H4qjmBi55t3Rd-iZCW-Q@mail.gmail.com>
Subject: Re: [Bluez PATCH v1] tools/l2cap-tester: Add test for waiting
 disconnection response
To:     Archie Pusaka <apusaka@google.com>
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Marcel Holtmann <marcel@holtmann.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Archie,

On Wed, Apr 29, 2020 at 11:52 PM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> 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