* [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 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
` (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 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
* 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