All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Marcel Holtmann <marcel@holtmann.org>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF
Date: Thu, 23 Sep 2021 14:04:02 -0700	[thread overview]
Message-ID: <CABBYNZ+ubDjUpm21BesXmm1oH08KH_d=QppZu_iZnY==ArDsrA@mail.gmail.com> (raw)
In-Reply-To: <53BDC874-DDC8-4700-BB26-2BAF21403D73@holtmann.org>

Hi Marcel,

On Wed, Sep 22, 2021 at 7:19 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >>> This adds support for BT_{SND,RCV}BUF so userspace can set MTU based on
> >>> the channel usage.
> >>>
> >>> Fixes: https://github.com/bluez/bluez/issues/201
> >>>
> >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>> ---
> >>> net/bluetooth/hci_sock.c | 102 ++++++++++++++++++++++++++++++++++-----
> >>> 1 file changed, 91 insertions(+), 11 deletions(-)
> >>
> >> so I applied patches 1, 3 and 4 to bluetooth-next tree.
> >>
> >> The patch 2 needs a more details review when I have time since I remember there were cases where the SKB copy was really needed.
> >
> > Is that something not covered by CI testing? Note we still have a copy
> > done internally with create_monitor_ctrl_command.
>
> there are cases where the SKB handed down is in hdev->send() and that one is owned by the driver to make whatever modifications to headroom it pleases. So if the stack needs to send it out via other sockets, it needs a copy.
>
> We can optimize here, but need to be careful.

I guess you are referring to instances of hci_send_to_channel, which
appears there only one instance that is changed in the diff bellow:

        if (chan->channel == HCI_CHANNEL_CONTROL) {
-               struct sk_buff *skb;
+               struct sk_buff *cmd;

                /* Send event to monitor */
-               skb = create_monitor_ctrl_command(sk, index, opcode, len,
-                                                 buf + sizeof(*hdr));
-               if (skb) {
-                       hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
+               cmd = create_monitor_ctrl_command(sk, index, opcode, len,
+                                                 skb->data + sizeof(*hdr));
+               if (cmd) {
+                       hci_send_to_channel(HCI_CHANNEL_MONITOR, cmd,
                                            HCI_SOCK_TRUSTED, NULL);
-                       kfree_skb(skb);
+                       kfree_skb(cmd);
                }
        }

Ive only changed the variable name since the original code use skb
which is now used as the original data instead of buf (extra copy), so
nothing has changed in this regard, also running this with KSAN, etc,
doesn't seem to produce any traces nor there seems to be anything
wrong in btmon either.

Note with this we are eliminating the extra copy on buf:

msg -> skb (new) vs msg-> buf -> skb (old)

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

      reply	other threads:[~2021-09-23 21:04 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16 20:10 Luiz Augusto von Dentz
2021-09-16 20:10 ` [PATCH v6 2/4] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
2021-09-16 20:10 ` [PATCH v6 3/4] Bluetooth: Fix passing NULL to PTR_ERR Luiz Augusto von Dentz
2021-09-16 20:10 ` [PATCH v6 4/4] Bluetooth: SCO: Fix sco_send_frame returning skb->len Luiz Augusto von Dentz
2021-09-16 21:02 ` [v6,1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF bluez.test.bot
2021-09-18  2:06 ` bluez.test.bot
2021-09-21  8:46 ` [PATCH v6 1/4] " Marcel Holtmann
2021-09-21 18:03   ` Luiz Augusto von Dentz
2021-09-22 14:19     ` Marcel Holtmann
2021-09-23 21:04       ` Luiz Augusto von Dentz [this message]

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='CABBYNZ+ubDjUpm21BesXmm1oH08KH_d=QppZu_iZnY==ArDsrA@mail.gmail.com' \
    --to=luiz.dentz@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --subject='Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF' \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.