* [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper
@ 2021-08-31 1:15 Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 2/4] Bluetooth: Add bt_skb_sendmmsg helper Luiz Augusto von Dentz
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-31 1:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
bt_skb_sendmsg helps takes care of allocation the skb and copying the
the contents of msg over to the skb while checking for possible errors
so it should be safe to call it without holding lock_sock.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
include/net/bluetooth/bluetooth.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9125effbf448..1bfb5d3d24dd 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -420,6 +420,29 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
return NULL;
}
+/* Shall not be called with lock_sock held */
+static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
+ struct msghdr *msg,
+ size_t len, int reserve)
+{
+ struct sk_buff *skb;
+ int err;
+
+ skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
+ if (!skb)
+ return ERR_PTR(err);
+
+ if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
+ kfree_skb(skb);
+ return ERR_PTR(-EFAULT);
+ }
+
+ skb_reserve(skb, reserve);
+ skb->priority = sk->sk_priority;
+
+ return skb;
+}
+
int bt_to_errno(u16 code);
void hci_sock_set_flag(struct sock *sk, int nr);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 2/4] Bluetooth: Add bt_skb_sendmmsg helper
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
@ 2021-08-31 1:15 ` Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 3/4] Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-31 1:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This works similarly to bt_skb_sendmsg but can split the msg into
multiple skb fragments which is useful for stream sockets.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
include/net/bluetooth/bluetooth.h | 35 +++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 1bfb5d3d24dd..1d66bccdf539 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -443,6 +443,41 @@ static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
return skb;
}
+/* Similar to bt_skb_sendmsg but can split the msg into multiple fragments
+ * accourding to the MTU.
+ */
+static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk,
+ struct msghdr *msg,
+ size_t len, size_t mtu,
+ int reserve)
+{
+ struct sk_buff *skb, **frag;
+ size_t size = min_t(size_t, len, mtu);
+
+ skb = bt_skb_sendmsg(sk, msg, size, reserve);
+ if (IS_ERR_OR_NULL(skb))
+ return skb;
+
+ len -= size;
+ if (!len)
+ return skb;
+
+ /* Add remaining data over MTU as continuation fragments */
+ frag = &skb_shinfo(skb)->frag_list;
+ while (len) {
+ *frag = bt_skb_sendmsg(sk, msg, size, reserve);
+ if (IS_ERR_OR_NULL(*frag)) {
+ kfree_skb(skb);
+ return *frag;
+ }
+
+ len -= (*frag)->len;
+ frag = &(*frag)->next;
+ }
+
+ return skb;
+}
+
int bt_to_errno(u16 code);
void hci_sock_set_flag(struct sock *sk, int nr);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 3/4] Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 2/4] Bluetooth: Add bt_skb_sendmmsg helper Luiz Augusto von Dentz
@ 2021-08-31 1:15 ` Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg Luiz Augusto von Dentz
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-31 1:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This makes use of bt_skb_sendmsg instead of allocating a different
buffer to be used with memcpy_from_msg which cause one extra copy.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/sco.c | 34 +++++++++++-----------------------
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index b62c91c627e2..8a262fbc8aa8 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -280,27 +280,19 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk)
return err;
}
-static int sco_send_frame(struct sock *sk, void *buf, int len,
- unsigned int msg_flags)
+static int sco_send_frame(struct sock *sk, struct sk_buff *skb)
{
struct sco_conn *conn = sco_pi(sk)->conn;
- struct sk_buff *skb;
- int err;
/* Check outgoing MTU */
- if (len > conn->mtu)
+ if (skb->len > conn->mtu)
return -EINVAL;
- BT_DBG("sk %p len %d", sk, len);
+ BT_DBG("sk %p len %d", sk, skb->len);
- skb = bt_skb_send_alloc(sk, len, msg_flags & MSG_DONTWAIT, &err);
- if (!skb)
- return err;
-
- memcpy(skb_put(skb, len), buf, len);
hci_send_sco(conn->hcon, skb);
- return len;
+ return skb->len;
}
static void sco_recv_frame(struct sco_conn *conn, struct sk_buff *skb)
@@ -722,7 +714,7 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
size_t len)
{
struct sock *sk = sock->sk;
- void *buf;
+ struct sk_buff *skb;
int err;
BT_DBG("sock %p, sk %p", sock, sk);
@@ -734,24 +726,20 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (msg->msg_flags & MSG_OOB)
return -EOPNOTSUPP;
- buf = kmalloc(len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- if (memcpy_from_msg(buf, msg, len)) {
- kfree(buf);
- return -EFAULT;
- }
+ skb = bt_skb_sendmsg(sk, msg, len, 0);
+ if (IS_ERR_OR_NULL(skb))
+ return PTR_ERR(skb);
lock_sock(sk);
if (sk->sk_state == BT_CONNECTED)
- err = sco_send_frame(sk, buf, len, msg->msg_flags);
+ err = sco_send_frame(sk, skb);
else
err = -ENOTCONN;
release_sock(sk);
- kfree(buf);
+ if (err)
+ kfree_skb(skb);
return err;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 2/4] Bluetooth: Add bt_skb_sendmmsg helper Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 3/4] Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
@ 2021-08-31 1:15 ` Luiz Augusto von Dentz
2021-08-31 3:14 ` kernel test robot
2021-08-31 12:30 ` kernel test robot
2021-08-31 8:35 ` [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Marcel Holtmann
2021-08-31 19:11 ` [RFC,1/4] " bluez.test.bot
4 siblings, 2 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-31 1:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This makes use of bt_skb_sendmmsg instead using memcpy_from_msg which
is not considered safe to be used when lock_sock is held.
Also make rfcomm_dlc_send handle skb with fragments and queue them all
atomically.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/rfcomm/core.c | 44 ++++++++++++++++++++++++++++------
net/bluetooth/rfcomm/sock.c | 48 ++++++++++---------------------------
2 files changed, 49 insertions(+), 43 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index f2bacb464ccf..361a60b4a670 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -549,22 +549,52 @@ struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
return dlc;
}
+static int rfcomm_dlc_send_frag(struct rfcomm_dlc *d, struct sk_buff *frag)
+{
+ int len = frag->len;
+
+ BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
+
+ if (len > d->mtu)
+ return -EINVAL;
+
+ rfcomm_make_uih(frag, d->addr);
+ __skb_queue_tail(&d->tx_queue, frag);
+
+ return len;
+}
+
int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
{
- int len = skb->len;
+ struct sk_buff *frag;
+ int len;
if (d->state != BT_CONNECTED)
return -ENOTCONN;
- BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
+ /* Queue all fragments atomically. */
+ spin_lock_bh(&d->tx_queue.lock);
- if (len > d->mtu)
- return -EINVAL;
+ len = rfcomm_dlc_send_frag(d, skb);
+ if (len < 0)
+ goto unlock;
- rfcomm_make_uih(skb, d->addr);
- skb_queue_tail(&d->tx_queue, skb);
+ skb_walk_frags(skb, frag) {
+ int ret;
+
+ ret = rfcomm_dlc_send_frag(d, frag);
+ if (ret < 0)
+ break;
+
+ len += ret;
+ }
+
+ skb_shinfo(skb)->frag_list = NULL;
+
+unlock:
+ spin_unlock_bh(&d->tx_queue.lock);
- if (!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
+ if (len > 0 && !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
rfcomm_schedule();
return len;
}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 2c95bb58f901..9dec2ff510be 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -558,6 +558,7 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
{
struct sock *sk = sock->sk;
struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
+ size_t size;
struct sk_buff *skb;
int sent;
@@ -574,47 +575,22 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
lock_sock(sk);
- sent = bt_sock_wait_ready(sk, msg->msg_flags);
- if (sent)
- goto done;
+ size = min_t(size_t, len, d->mtu);
- while (len) {
- size_t size = min_t(size_t, len, d->mtu);
- int err;
-
- skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
- msg->msg_flags & MSG_DONTWAIT, &err);
- if (!skb) {
- if (sent == 0)
- sent = err;
- break;
- }
- skb_reserve(skb, RFCOMM_SKB_HEAD_RESERVE);
-
- err = memcpy_from_msg(skb_put(skb, size), msg, size);
- if (err) {
- kfree_skb(skb);
- if (sent == 0)
- sent = err;
- break;
- }
+ sent = bt_sock_wait_ready(sk, msg->msg_flags);
- skb->priority = sk->sk_priority;
+ release_sock(sk);
- err = rfcomm_dlc_send(d, skb);
- if (err < 0) {
- kfree_skb(skb);
- if (sent == 0)
- sent = err;
- break;
- }
+ if (sent)
+ return sent;
- sent += size;
- len -= size;
- }
+ skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_RESERVE);
+ if (IS_ERR_OR_NULL(skb))
+ return PTR_ERR(skb);
-done:
- release_sock(sk);
+ sent = rfcomm_dlc_send(d, skb);
+ if (sent < 0)
+ kfree_skb(skb);
return sent;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg
2021-08-31 1:15 ` [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg Luiz Augusto von Dentz
@ 2021-08-31 3:14 ` kernel test robot
2021-08-31 12:30 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-08-31 3:14 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3028 bytes --]
Hi Luiz,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bluetooth-next/master]
[cannot apply to bluetooth/master v5.14 next-20210830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a2468415c162f27c7def3dd53acdb62cf084afdd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
git checkout a2468415c162f27c7def3dd53acdb62cf084afdd
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/bluetooth/rfcomm/sock.c: In function 'rfcomm_sock_sendmsg':
>> net/bluetooth/rfcomm/sock.c:561:16: warning: variable 'size' set but not used [-Wunused-but-set-variable]
561 | size_t size;
| ^~~~
vim +/size +561 net/bluetooth/rfcomm/sock.c
555
556 static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
557 size_t len)
558 {
559 struct sock *sk = sock->sk;
560 struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
> 561 size_t size;
562 struct sk_buff *skb;
563 int sent;
564
565 if (test_bit(RFCOMM_DEFER_SETUP, &d->flags))
566 return -ENOTCONN;
567
568 if (msg->msg_flags & MSG_OOB)
569 return -EOPNOTSUPP;
570
571 if (sk->sk_shutdown & SEND_SHUTDOWN)
572 return -EPIPE;
573
574 BT_DBG("sock %p, sk %p", sock, sk);
575
576 lock_sock(sk);
577
578 size = min_t(size_t, len, d->mtu);
579
580 sent = bt_sock_wait_ready(sk, msg->msg_flags);
581
582 release_sock(sk);
583
584 if (sent)
585 return sent;
586
587 skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_RESERVE);
588 if (IS_ERR_OR_NULL(skb))
589 return PTR_ERR(skb);
590
591 sent = rfcomm_dlc_send(d, skb);
592 if (sent < 0)
593 kfree_skb(skb);
594
595 return sent;
596 }
597
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 60756 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
` (2 preceding siblings ...)
2021-08-31 1:15 ` [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg Luiz Augusto von Dentz
@ 2021-08-31 8:35 ` Marcel Holtmann
2021-09-01 0:20 ` Luiz Augusto von Dentz
2021-08-31 19:11 ` [RFC,1/4] " bluez.test.bot
4 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2021-08-31 8:35 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
> bt_skb_sendmsg helps takes care of allocation the skb and copying the
> the contents of msg over to the skb while checking for possible errors
> so it should be safe to call it without holding lock_sock.
>
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/bluetooth.h | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9125effbf448..1bfb5d3d24dd 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -420,6 +420,29 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
> return NULL;
> }
>
> +/* Shall not be called with lock_sock held */
> +static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
> + struct msghdr *msg,
> + size_t len, int reserve)
> +{
> + struct sk_buff *skb;
> + int err;
> +
> + skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
> + if (!skb)
> + return ERR_PTR(err);
> +
> + if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
> + kfree_skb(skb);
> + return ERR_PTR(-EFAULT);
> + }
> +
> + skb_reserve(skb, reserve);
> + skb->priority = sk->sk_priority;
have you tested this? I remember vaguely (really vaguely actually) that skb_reserve needs to be called before you do any changes to the buffer via skb_put and others. Especially since bt_skb_send_alloc already calls skb_reserve in the first place.
Regards
Marcel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg
2021-08-31 1:15 ` [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg Luiz Augusto von Dentz
@ 2021-08-31 12:30 ` kernel test robot
2021-08-31 12:30 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-08-31 12:30 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: llvm, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]
Hi Luiz,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20210831]
[cannot apply to bluetooth/master v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: i386-randconfig-a013-20210831 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a2468415c162f27c7def3dd53acdb62cf084afdd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
git checkout a2468415c162f27c7def3dd53acdb62cf084afdd
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/bluetooth/rfcomm/sock.c:561:9: warning: variable 'size' set but not used [-Wunused-but-set-variable]
size_t size;
^
1 warning generated.
vim +/size +561 net/bluetooth/rfcomm/sock.c
555
556 static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
557 size_t len)
558 {
559 struct sock *sk = sock->sk;
560 struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
> 561 size_t size;
562 struct sk_buff *skb;
563 int sent;
564
565 if (test_bit(RFCOMM_DEFER_SETUP, &d->flags))
566 return -ENOTCONN;
567
568 if (msg->msg_flags & MSG_OOB)
569 return -EOPNOTSUPP;
570
571 if (sk->sk_shutdown & SEND_SHUTDOWN)
572 return -EPIPE;
573
574 BT_DBG("sock %p, sk %p", sock, sk);
575
576 lock_sock(sk);
577
578 size = min_t(size_t, len, d->mtu);
579
580 sent = bt_sock_wait_ready(sk, msg->msg_flags);
581
582 release_sock(sk);
583
584 if (sent)
585 return sent;
586
587 skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_RESERVE);
588 if (IS_ERR_OR_NULL(skb))
589 return PTR_ERR(skb);
590
591 sent = rfcomm_dlc_send(d, skb);
592 if (sent < 0)
593 kfree_skb(skb);
594
595 return sent;
596 }
597
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48341 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg
@ 2021-08-31 12:30 ` kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-08-31 12:30 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
Hi Luiz,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20210831]
[cannot apply to bluetooth/master v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: i386-randconfig-a013-20210831 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a2468415c162f27c7def3dd53acdb62cf084afdd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-Add-bt_skb_sendmsg-helper/20210831-091735
git checkout a2468415c162f27c7def3dd53acdb62cf084afdd
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/bluetooth/rfcomm/sock.c:561:9: warning: variable 'size' set but not used [-Wunused-but-set-variable]
size_t size;
^
1 warning generated.
vim +/size +561 net/bluetooth/rfcomm/sock.c
555
556 static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
557 size_t len)
558 {
559 struct sock *sk = sock->sk;
560 struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
> 561 size_t size;
562 struct sk_buff *skb;
563 int sent;
564
565 if (test_bit(RFCOMM_DEFER_SETUP, &d->flags))
566 return -ENOTCONN;
567
568 if (msg->msg_flags & MSG_OOB)
569 return -EOPNOTSUPP;
570
571 if (sk->sk_shutdown & SEND_SHUTDOWN)
572 return -EPIPE;
573
574 BT_DBG("sock %p, sk %p", sock, sk);
575
576 lock_sock(sk);
577
578 size = min_t(size_t, len, d->mtu);
579
580 sent = bt_sock_wait_ready(sk, msg->msg_flags);
581
582 release_sock(sk);
583
584 if (sent)
585 return sent;
586
587 skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_RESERVE);
588 if (IS_ERR_OR_NULL(skb))
589 return PTR_ERR(skb);
590
591 sent = rfcomm_dlc_send(d, skb);
592 if (sent < 0)
593 kfree_skb(skb);
594
595 return sent;
596 }
597
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 48341 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RFC,1/4] Bluetooth: Add bt_skb_sendmsg helper
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
` (3 preceding siblings ...)
2021-08-31 8:35 ` [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Marcel Holtmann
@ 2021-08-31 19:11 ` bluez.test.bot
4 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2021-08-31 19:11 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
[-- Attachment #1: Type: text/plain, Size: 2808 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=539493
---Test result---
Test Summary:
CheckPatch PASS 2.15 seconds
GitLint PASS 0.48 seconds
BuildKernel PASS 674.53 seconds
TestRunner: Setup PASS 449.35 seconds
TestRunner: l2cap-tester PASS 3.06 seconds
TestRunner: bnep-tester PASS 2.17 seconds
TestRunner: mgmt-tester PASS 34.17 seconds
TestRunner: rfcomm-tester FAIL 2.41 seconds
TestRunner: sco-tester PASS 2.38 seconds
TestRunner: smp-tester PASS 2.53 seconds
TestRunner: userchan-tester PASS 2.34 seconds
Details
##############################
Test: CheckPatch - PASS - 2.15 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
##############################
Test: GitLint - PASS - 0.48 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 674.53 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 449.35 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 3.06 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.17 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 34.17 seconds
Run test-runner with mgmt-tester
Total: 452, Passed: 452 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: rfcomm-tester - FAIL - 2.41 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 7 (77.8%), Failed: 2, Not Run: 0
Failed Test Cases
Basic RFCOMM Socket Client - Write Success Failed 0.022 seconds
Basic RFCOMM Socket Server - Write Success Failed 0.020 seconds
##############################
Test: TestRunner: sco-tester - PASS - 2.38 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - PASS - 2.53 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: userchan-tester - PASS - 2.34 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 44386 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3593 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 622561 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11712 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9948 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11859 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5489 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper
2021-08-31 8:35 ` [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Marcel Holtmann
@ 2021-09-01 0:20 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-01 0:20 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Tue, Aug 31, 2021 at 1:35 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > bt_skb_sendmsg helps takes care of allocation the skb and copying the
> > the contents of msg over to the skb while checking for possible errors
> > so it should be safe to call it without holding lock_sock.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/bluetooth.h | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 9125effbf448..1bfb5d3d24dd 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -420,6 +420,29 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
> > return NULL;
> > }
> >
> > +/* Shall not be called with lock_sock held */
> > +static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
> > + struct msghdr *msg,
> > + size_t len, int reserve)
> > +{
> > + struct sk_buff *skb;
> > + int err;
> > +
> > + skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
> > + if (!skb)
> > + return ERR_PTR(err);
> > +
> > + if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
> > + kfree_skb(skb);
> > + return ERR_PTR(-EFAULT);
> > + }
> > +
> > + skb_reserve(skb, reserve);
> > + skb->priority = sk->sk_priority;
>
> have you tested this? I remember vaguely (really vaguely actually) that skb_reserve needs to be called before you do any changes to the buffer via skb_put and others. Especially since bt_skb_send_alloc already calls skb_reserve in the first place.
Yep, that and the fact that we need both a header and footer size to
be given separately since the len should only account for the payload
itself, anyway I will send an update shortly and Ive now tested with
rfcomm-tester and they are all passing.
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-01 0:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 1:15 [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 2/4] Bluetooth: Add bt_skb_sendmmsg helper Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 3/4] Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
2021-08-31 1:15 ` [RFC 4/4] Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg Luiz Augusto von Dentz
2021-08-31 3:14 ` kernel test robot
2021-08-31 12:30 ` kernel test robot
2021-08-31 12:30 ` kernel test robot
2021-08-31 8:35 ` [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper Marcel Holtmann
2021-09-01 0:20 ` Luiz Augusto von Dentz
2021-08-31 19:11 ` [RFC,1/4] " bluez.test.bot
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.