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
next prev parent 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 [PATCH] Bluetooth: use helper function for monitor's open/close notifications 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 \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).