Re: [PATCH v1] Bluetooth: L2CAP: add support for waiting disconnection resp

[Archie Pusaka <apusaka@google.com>]

[Re-sending in plain text]
Hi bluetooth maintainers,

May I ask for your review on this?

Thanks,
Archie


On Tue, 14 Apr 2020 at 16:09, Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> Whenever we disconnect a L2CAP connection, we would immediately
> report a disconnection event (EPOLLHUP) to the upper layer, without
> waiting for the response of the other device.
>
> This patch offers an option to wait until we receive a disconnection
> response before reporting disconnection event, by using the "how"
> parameter in l2cap_sock_shutdown(). Therefore, upper layer can opt
> to wait for disconnection response by shutdown(sock, SHUT_WR).
>
> This can be used to enforce proper disconnection order in HID,
> where the disconnection of the interrupt channel must be complete
> before attempting to disconnect the control channel.
>
> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> ---
>
>  net/bluetooth/l2cap_sock.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 1cea42ee1e922..a995d2c51fa7f 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1271,14 +1271,21 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
>         struct l2cap_conn *conn;
>         int err = 0;
>
> -       BT_DBG("sock %p, sk %p", sock, sk);
> +       BT_DBG("sock %p, sk %p, how %d", sock, sk, how);
> +
> +       /* 'how' parameter is mapped to sk_shutdown as follows:
> +        * SHUT_RD   (0) --> RCV_SHUTDOWN  (1)
> +        * SHUT_WR   (1) --> SEND_SHUTDOWN (2)
> +        * SHUT_RDWR (2) --> SHUTDOWN_MASK (3)
> +        */
> +       how++;
>
>         if (!sk)
>                 return 0;
>
>         lock_sock(sk);
>
> -       if (sk->sk_shutdown)
> +       if ((sk->sk_shutdown & how) == how)
>                 goto shutdown_already;
>
>         BT_DBG("Handling sock shutdown");
> @@ -1301,11 +1308,20 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
>                  * has already been actioned to close the L2CAP
>                  * link such as by l2cap_disconnection_req().
>                  */
> -               if (sk->sk_shutdown)
> -                       goto has_shutdown;
> +               if ((sk->sk_shutdown & how) == how)
> +                       goto shutdown_matched;
>         }
>
> -       sk->sk_shutdown = SHUTDOWN_MASK;
> +       /* Try setting the RCV_SHUTDOWN bit, return early if SEND_SHUTDOWN
> +        * is already set
> +        */
> +       if ((how & RCV_SHUTDOWN) && !(sk->sk_shutdown & RCV_SHUTDOWN)) {
> +               sk->sk_shutdown |= RCV_SHUTDOWN;
> +               if ((sk->sk_shutdown & how) == how)
> +                       goto shutdown_matched;
> +       }
> +
> +       sk->sk_shutdown |= SEND_SHUTDOWN;
>         release_sock(sk);
>
>         l2cap_chan_lock(chan);
> @@ -1335,7 +1351,7 @@ static int l2cap_sock_shutdown(struct socket *sock, int how)
>                 err = bt_sock_wait_state(sk, BT_CLOSED,
>                                          sk->sk_lingertime);
>
> -has_shutdown:
> +shutdown_matched:
>         l2cap_chan_put(chan);
>         sock_put(sk);
>
> @@ -1363,7 +1379,7 @@ static int l2cap_sock_release(struct socket *sock)
>
>         bt_sock_unlink(&l2cap_sk_list, sk);
>
> -       err = l2cap_sock_shutdown(sock, 2);
> +       err = l2cap_sock_shutdown(sock, SHUT_RDWR);
>         chan = l2cap_pi(sk)->chan;
>
>         l2cap_chan_hold(chan);
> --
> 2.26.0.110.g2183baf09c-goog
>