linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	"linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] Bluetooth: use helper function for monitor's open/close notifications
Date: Mon, 26 Jul 2021 10:40:14 -0700	[thread overview]
Message-ID: <CABBYNZLw-1xofLwsNYEcb7auN6KorPTLh3ZOWFR4V6n4knOT-g@mail.gmail.com> (raw)
In-Reply-To: <20210724140331.3465-1-penguin-kernel@I-love.SAKURA.ne.jp>

Hi Tetsuo,

On Sat, Jul 24, 2021 at 7:03 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> hci_sock.c has many
> hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> calls. Use helper functions and replace skb with sk.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  net/bluetooth/hci_sock.c | 96 ++++++++++++++++------------------------
>  1 file changed, 37 insertions(+), 59 deletions(-)
>
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 786a06a232fd..fc2336855dab 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -295,6 +295,11 @@ void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
>         read_unlock(&hci_sk_list.lock);
>  }
>
> +static void __hci_send_to_monitor(struct sk_buff *skb)
> +{

We can probably have the checks of NULL skb added directly here and
perhaps kfree_skb as well since it seems it is always a copy that is
sent here, the hci_send_to_monitor don't have __ prefix so I wonder
why you have chosen to use it?

> +       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> +}
> +
>  /* Send frame to monitor socket */
>  void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>  {
> @@ -350,8 +355,7 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>         hdr->index = cpu_to_le16(hdev->id);
>         hdr->len = cpu_to_le16(skb->len);
>
> -       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb_copy,
> -                           HCI_SOCK_TRUSTED, NULL);
> +       __hci_send_to_monitor(skb_copy);
>         kfree_skb(skb_copy);
>  }
>
> @@ -545,6 +549,16 @@ static struct sk_buff *create_monitor_ctrl_open(struct sock *sk)
>         return skb;
>  }
>
> +static void hci_monitor_ctrl_open(struct sock *sk)
> +{
> +       struct sk_buff *skb = create_monitor_ctrl_open(sk);
> +
> +       if (skb) {
> +               __hci_send_to_monitor(skb);
> +               kfree_skb(skb);
> +       }
> +}
> +
>  static struct sk_buff *create_monitor_ctrl_close(struct sock *sk)
>  {
>         struct hci_mon_hdr *hdr;
> @@ -583,6 +597,16 @@ static struct sk_buff *create_monitor_ctrl_close(struct sock *sk)
>         return skb;
>  }
>
> +static void hci_monitor_ctrl_close(struct sock *sk)
> +{
> +       struct sk_buff *skb = create_monitor_ctrl_close(sk);
> +
> +       if (skb) {
> +               __hci_send_to_monitor(skb);
> +               kfree_skb(skb);
> +       }
> +}
> +
>  static struct sk_buff *create_monitor_ctrl_command(struct sock *sk, u16 index,
>                                                    u16 opcode, u16 len,
>                                                    const void *buf)
> @@ -741,8 +765,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)
>                 /* Send event to monitor */
>                 skb = create_monitor_event(hdev, event);
>                 if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> +                       __hci_send_to_monitor(skb);
>                         kfree_skb(skb);
>                 }
>         }
> @@ -859,7 +882,6 @@ static int hci_sock_release(struct socket *sock)
>  {
>         struct sock *sk = sock->sk;
>         struct hci_dev *hdev;
> -       struct sk_buff *skb;
>
>         BT_DBG("sock %p sk %p", sock, sk);
>
> @@ -876,12 +898,7 @@ static int hci_sock_release(struct socket *sock)
>         case HCI_CHANNEL_USER:
>         case HCI_CHANNEL_CONTROL:
>                 /* Send event to monitor */
> -               skb = create_monitor_ctrl_close(sk);
> -               if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> -                       kfree_skb(skb);
> -               }
> +               hci_monitor_ctrl_close(sk);
>
>                 hci_sock_free_cookie(sk);
>                 break;
> @@ -1021,18 +1038,11 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,
>          * of a given socket.
>          */
>         if (hci_sock_gen_cookie(sk)) {
> -               struct sk_buff *skb;
> -
>                 if (capable(CAP_NET_ADMIN))
>                         hci_sock_set_flag(sk, HCI_SOCK_TRUSTED);
>
>                 /* Send event to monitor */
> -               skb = create_monitor_ctrl_open(sk);
> -               if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> -                       kfree_skb(skb);
> -               }
> +               hci_monitor_ctrl_open(sk);
>         }
>
>         release_sock(sk);
> @@ -1114,7 +1124,6 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>         struct sockaddr_hci haddr;
>         struct sock *sk = sock->sk;
>         struct hci_dev *hdev = NULL;
> -       struct sk_buff *skb;
>         int len, err = 0;
>
>         BT_DBG("sock %p sk %p", sock, sk);
> @@ -1162,12 +1171,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>                          * notification. Send a close notification first to
>                          * allow the state transition to bounded.
>                          */
> -                       skb = create_monitor_ctrl_close(sk);
> -                       if (skb) {
> -                               hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                                   HCI_SOCK_TRUSTED, NULL);
> -                               kfree_skb(skb);
> -                       }
> +                       hci_monitor_ctrl_close(sk);
>                 }
>
>                 if (capable(CAP_NET_ADMIN))
> @@ -1176,12 +1180,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>                 hci_pi(sk)->hdev = hdev;
>
>                 /* Send event to monitor */
> -               skb = create_monitor_ctrl_open(sk);
> -               if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> -                       kfree_skb(skb);
> -               }
> +               hci_monitor_ctrl_open(sk);
>                 break;
>
>         case HCI_CHANNEL_USER:
> @@ -1251,12 +1250,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>                          * a user channel socket. For a clean transition, send
>                          * the close notification first.
>                          */
> -                       skb = create_monitor_ctrl_close(sk);
> -                       if (skb) {
> -                               hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                                   HCI_SOCK_TRUSTED, NULL);
> -                               kfree_skb(skb);
> -                       }
> +                       hci_monitor_ctrl_close(sk);
>                 }
>
>                 /* The user channel is restricted to CAP_NET_ADMIN
> @@ -1267,12 +1261,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>                 hci_pi(sk)->hdev = hdev;
>
>                 /* Send event to monitor */
> -               skb = create_monitor_ctrl_open(sk);
> -               if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> -                       kfree_skb(skb);
> -               }
> +               hci_monitor_ctrl_open(sk);
>
>                 atomic_inc(&hdev->promisc);
>                 break;
> @@ -1359,21 +1348,11 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
>                                  * allow for a clean transition, send the
>                                  * close notification first.
>                                  */
> -                               skb = create_monitor_ctrl_close(sk);
> -                               if (skb) {
> -                                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                                           HCI_SOCK_TRUSTED, NULL);
> -                                       kfree_skb(skb);
> -                               }
> +                               hci_monitor_ctrl_close(sk);
>                         }
>
>                         /* Send event to monitor */
> -                       skb = create_monitor_ctrl_open(sk);
> -                       if (skb) {
> -                               hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                                   HCI_SOCK_TRUSTED, NULL);
> -                               kfree_skb(skb);
> -                       }
> +                       hci_monitor_ctrl_open(sk);
>
>                         hci_sock_set_flag(sk, HCI_MGMT_INDEX_EVENTS);
>                         hci_sock_set_flag(sk, HCI_MGMT_UNCONF_INDEX_EVENTS);
> @@ -1559,8 +1538,7 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk,
>                 skb = create_monitor_ctrl_command(sk, index, opcode, len,
>                                                   buf + sizeof(*hdr));
>                 if (skb) {
> -                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> -                                           HCI_SOCK_TRUSTED, NULL);
> +                       __hci_send_to_monitor(skb);
>                         kfree_skb(skb);
>                 }
>         }
> @@ -1715,7 +1693,7 @@ static int hci_logging_frame(struct sock *sk, struct msghdr *msg, int len)
>
>         hdr->opcode = cpu_to_le16(HCI_MON_USER_LOGGING);
>
> -       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> +       __hci_send_to_monitor(skb);
>         err = len;
>
>         if (hdev)
> --
> 2.18.4
>


-- 
Luiz Augusto von Dentz

  reply	other threads:[~2021-07-26 17:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-24 14:03 Tetsuo Handa
2021-07-26 17:40 ` Luiz Augusto von Dentz [this message]
2021-07-26 20:39   ` Tetsuo Handa
2021-07-26 21:26     ` Luiz Augusto von Dentz
2021-07-27  2:09 ` bluez.test.bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABBYNZLw-1xofLwsNYEcb7auN6KorPTLh3ZOWFR4V6n4knOT-g@mail.gmail.com \
    --to=luiz.dentz@gmail.com \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --subject='Re: [PATCH] Bluetooth: use helper function for monitor'\''s open/close notifications' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).