* [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF @ 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 ` (5 more replies) 0 siblings, 6 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-16 20:10 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> 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(-) diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index 55b0d177375b..99de17922bda 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -57,6 +57,7 @@ struct hci_pinfo { unsigned long flags; __u32 cookie; char comm[TASK_COMM_LEN]; + __u16 mtu; }; static struct hci_dev *hci_hdev_from_sock(struct sock *sk) @@ -1374,6 +1375,10 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, break; } + /* Default MTU to HCI_MAX_FRAME_SIZE if not set */ + if (!hci_pi(sk)->mtu) + hci_pi(sk)->mtu = HCI_MAX_FRAME_SIZE; + sk->sk_state = BT_BOUND; done: @@ -1719,7 +1724,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT)) return -EINVAL; - if (len < 4 || len > HCI_MAX_FRAME_SIZE) + if (len < 4 || len > hci_pi(sk)->mtu) return -EINVAL; buf = kmalloc(len, GFP_KERNEL); @@ -1849,8 +1854,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, goto done; } -static int hci_sock_setsockopt(struct socket *sock, int level, int optname, - sockptr_t optval, unsigned int len) +static int hci_sock_setsockopt_old(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int len) { struct hci_ufilter uf = { .opcode = 0 }; struct sock *sk = sock->sk; @@ -1858,9 +1863,6 @@ static int hci_sock_setsockopt(struct socket *sock, int level, int optname, BT_DBG("sk %p, opt %d", sk, optname); - if (level != SOL_HCI) - return -ENOPROTOOPT; - lock_sock(sk); if (hci_pi(sk)->channel != HCI_CHANNEL_RAW) { @@ -1935,18 +1937,63 @@ static int hci_sock_setsockopt(struct socket *sock, int level, int optname, return err; } -static int hci_sock_getsockopt(struct socket *sock, int level, int optname, - char __user *optval, int __user *optlen) +static int hci_sock_setsockopt(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int len) { - struct hci_ufilter uf; struct sock *sk = sock->sk; - int len, opt, err = 0; + int err = 0, opt = 0; BT_DBG("sk %p, opt %d", sk, optname); - if (level != SOL_HCI) + if (level == SOL_HCI) + return hci_sock_setsockopt_old(sock, level, optname, optval, + len); + + if (level != SOL_BLUETOOTH) return -ENOPROTOOPT; + lock_sock(sk); + + switch (optname) { + case BT_SNDMTU: + case BT_RCVMTU: + switch (hci_pi(sk)->channel) { + /* Don't allow changing MTU for channels that are meant for HCI + * traffic only. + */ + case HCI_CHANNEL_RAW: + case HCI_CHANNEL_USER: + err = -ENOPROTOOPT; + goto done; + } + + if (copy_from_sockptr(&opt, optval, sizeof(u16))) { + err = -EFAULT; + break; + } + + hci_pi(sk)->mtu = opt; + break; + + default: + err = -ENOPROTOOPT; + break; + } + +done: + release_sock(sk); + return err; +} + +static int hci_sock_getsockopt_old(struct socket *sock, int level, int optname, + char __user *optval, int __user *optlen) +{ + struct hci_ufilter uf; + struct sock *sk = sock->sk; + int len, opt, err = 0; + + BT_DBG("sk %p, opt %d", sk, optname); + if (get_user(len, optlen)) return -EFAULT; @@ -2004,6 +2051,39 @@ static int hci_sock_getsockopt(struct socket *sock, int level, int optname, return err; } +static int hci_sock_getsockopt(struct socket *sock, int level, int optname, + char __user *optval, int __user *optlen) +{ + struct sock *sk = sock->sk; + int err = 0; + + BT_DBG("sk %p, opt %d", sk, optname); + + if (level == SOL_HCI) + return hci_sock_getsockopt_old(sock, level, optname, optval, + optlen); + + if (level != SOL_BLUETOOTH) + return -ENOPROTOOPT; + + lock_sock(sk); + + switch (optname) { + case BT_SNDMTU: + case BT_RCVMTU: + if (put_user(hci_pi(sk)->mtu, (u16 __user *)optval)) + err = -EFAULT; + break; + + default: + err = -ENOPROTOOPT; + break; + } + + release_sock(sk); + return err; +} + static const struct proto_ops hci_sock_ops = { .family = PF_BLUETOOTH, .owner = THIS_MODULE, -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v6 2/4] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF Luiz Augusto von Dentz @ 2021-09-16 20:10 ` Luiz Augusto von Dentz 2021-09-16 20:10 ` [PATCH v6 3/4] Bluetooth: Fix passing NULL to PTR_ERR Luiz Augusto von Dentz ` (4 subsequent siblings) 5 siblings, 0 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-16 20:10 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. Tested-by: Tedd Ho-Jeong An <tedd.an@intel.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- net/bluetooth/hci_sock.c | 100 +++++++++++++++------------------------ 1 file changed, 37 insertions(+), 63 deletions(-) diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index 99de17922bda..d0dad1fafe07 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -1510,7 +1510,8 @@ static int hci_sock_recvmsg(struct socket *sock, struct msghdr *msg, return err ? : copied; } -static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, size_t msglen) +static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, + struct sk_buff *skb) { u8 *cp; struct mgmt_hdr *hdr; @@ -1520,31 +1521,31 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, bool var_len, no_hdev; int err; - BT_DBG("got %zu bytes", msglen); + BT_DBG("got %d bytes", skb->len); - if (msglen < sizeof(*hdr)) + if (skb->len < sizeof(*hdr)) return -EINVAL; - hdr = buf; + hdr = (void *)skb->data; opcode = __le16_to_cpu(hdr->opcode); index = __le16_to_cpu(hdr->index); len = __le16_to_cpu(hdr->len); - if (len != msglen - sizeof(*hdr)) { + if (len != skb->len - sizeof(*hdr)) { err = -EINVAL; goto done; } 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); } } @@ -1609,13 +1610,13 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, if (hdev && chan->hdev_init) chan->hdev_init(sk, hdev); - cp = buf + sizeof(*hdr); + cp = skb->data + sizeof(*hdr); err = handler->func(sk, hdev, cp, len); if (err < 0) goto done; - err = msglen; + err = skb->len; done: if (hdev) @@ -1624,10 +1625,10 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, return err; } -static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int flags) +static int hci_logging_frame(struct sock *sk, struct sk_buff *skb, + unsigned int flags) { struct hci_mon_hdr *hdr; - struct sk_buff *skb; struct hci_dev *hdev; u16 index; int err; @@ -1636,21 +1637,13 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f * the priority byte, the ident length byte and at least one string * terminator NUL byte. Anything shorter are invalid packets. */ - if (len < sizeof(*hdr) + 3) + if (skb->len < sizeof(*hdr) + 3) return -EINVAL; - skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err); - if (!skb) - return err; - - memcpy(skb_put(skb, len), buf, len); - hdr = (void *)skb->data; - if (__le16_to_cpu(hdr->len) != len - sizeof(*hdr)) { - err = -EINVAL; - goto drop; - } + if (__le16_to_cpu(hdr->len) != skb->len - sizeof(*hdr)) + return -EINVAL; if (__le16_to_cpu(hdr->opcode) == 0x0000) { __u8 priority = skb->data[sizeof(*hdr)]; @@ -1669,25 +1662,20 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f * The message follows the ident string (if present) and * must be NUL terminated. Otherwise it is not a valid packet. */ - if (priority > 7 || skb->data[len - 1] != 0x00 || - ident_len > len - sizeof(*hdr) - 3 || - skb->data[sizeof(*hdr) + ident_len + 1] != 0x00) { - err = -EINVAL; - goto drop; - } + if (priority > 7 || skb->data[skb->len - 1] != 0x00 || + ident_len > skb->len - sizeof(*hdr) - 3 || + skb->data[sizeof(*hdr) + ident_len + 1] != 0x00) + return -EINVAL; } else { - err = -EINVAL; - goto drop; + return -EINVAL; } index = __le16_to_cpu(hdr->index); if (index != MGMT_INDEX_NONE) { hdev = hci_dev_get(index); - if (!hdev) { - err = -ENODEV; - goto drop; - } + if (!hdev) + return -ENODEV; } else { hdev = NULL; } @@ -1695,13 +1683,11 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f hdr->opcode = cpu_to_le16(HCI_MON_USER_LOGGING); hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL); - err = len; + err = skb->len; if (hdev) hci_dev_put(hdev); -drop: - kfree_skb(skb); return err; } @@ -1713,7 +1699,6 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, struct hci_dev *hdev; struct sk_buff *skb; int err; - void *buf; const unsigned int flags = msg->msg_flags; BT_DBG("sock %p sk %p", sock, sk); @@ -1727,13 +1712,9 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, if (len < 4 || len > hci_pi(sk)->mtu) return -EINVAL; - 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, len, 0, 0); + if (IS_ERR(skb)) + return PTR_ERR(skb); lock_sock(sk); @@ -1743,39 +1724,33 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, break; case HCI_CHANNEL_MONITOR: err = -EOPNOTSUPP; - goto done; + goto drop; case HCI_CHANNEL_LOGGING: - err = hci_logging_frame(sk, buf, len, flags); - goto done; + err = hci_logging_frame(sk, skb, flags); + goto drop; default: mutex_lock(&mgmt_chan_list_lock); chan = __hci_mgmt_chan_find(hci_pi(sk)->channel); if (chan) - err = hci_mgmt_cmd(chan, sk, buf, len); + err = hci_mgmt_cmd(chan, sk, skb); else err = -EINVAL; mutex_unlock(&mgmt_chan_list_lock); - goto done; + goto drop; } hdev = hci_hdev_from_sock(sk); if (IS_ERR(hdev)) { err = PTR_ERR(hdev); - goto done; + goto drop; } if (!test_bit(HCI_UP, &hdev->flags)) { err = -ENETDOWN; - goto done; + goto drop; } - skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err); - if (!skb) - goto done; - - memcpy(skb_put(skb, len), buf, len); - hci_skb_pkt_type(skb) = skb->data[0]; skb_pull(skb, 1); @@ -1846,7 +1821,6 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, done: release_sock(sk); - kfree(buf); return err; drop: -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v6 3/4] Bluetooth: Fix passing NULL to PTR_ERR 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 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 ` 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 ` (3 subsequent siblings) 5 siblings, 0 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-16 20:10 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Passing NULL to PTR_ERR will result in 0 (success), also since the likes of bt_skb_sendmsg does never return NULL it is safe to replace the instances of IS_ERR_OR_NULL with IS_ERR when checking its return. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Tested-by: Tedd Ho-Jeong An <tedd.an@intel.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- include/net/bluetooth/bluetooth.h | 2 +- net/bluetooth/rfcomm/sock.c | 2 +- net/bluetooth/sco.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index aa221c1a27c6..3271870fd85e 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -496,7 +496,7 @@ static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk, struct sk_buff *tmp; tmp = bt_skb_sendmsg(sk, msg, len, mtu, headroom, tailroom); - if (IS_ERR_OR_NULL(tmp)) { + if (IS_ERR(tmp)) { kfree_skb(skb); return tmp; } diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c index 5938af3e9936..4bf4ea6cbb5e 100644 --- a/net/bluetooth/rfcomm/sock.c +++ b/net/bluetooth/rfcomm/sock.c @@ -583,7 +583,7 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg, skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE, RFCOMM_SKB_TAIL_RESERVE); - if (IS_ERR_OR_NULL(skb)) + if (IS_ERR(skb)) return PTR_ERR(skb); sent = rfcomm_dlc_send(d, skb); diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 446f871f11ed..f51399d1b9cb 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -733,7 +733,7 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg, return -EOPNOTSUPP; skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0); - if (IS_ERR_OR_NULL(skb)) + if (IS_ERR(skb)) return PTR_ERR(skb); lock_sock(sk); -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v6 4/4] Bluetooth: SCO: Fix sco_send_frame returning skb->len 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 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 ` 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 ` (2 subsequent siblings) 5 siblings, 0 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-16 20:10 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> The skb in modified by hci_send_sco which pushes SCO headers thus changing skb->len causing sco_sock_sendmsg to fail. Fixes: 0771cbb3b97d ("Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg") Tested-by: Tedd Ho-Jeong An <tedd.an@intel.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> --- net/bluetooth/sco.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index f51399d1b9cb..8eabf41b2993 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -284,16 +284,17 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk) static int sco_send_frame(struct sock *sk, struct sk_buff *skb) { struct sco_conn *conn = sco_pi(sk)->conn; + int len = skb->len; /* Check outgoing MTU */ - if (skb->len > conn->mtu) + if (len > conn->mtu) return -EINVAL; - BT_DBG("sk %p len %d", sk, skb->len); + BT_DBG("sk %p len %d", sk, len); hci_send_sco(conn->hcon, skb); - return skb->len; + return len; } static void sco_recv_frame(struct sco_conn *conn, struct sk_buff *skb) @@ -744,7 +745,8 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg, err = -ENOTCONN; release_sock(sk); - if (err) + + if (err < 0) kfree_skb(skb); return err; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [v6,1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF Luiz Augusto von Dentz ` (2 preceding siblings ...) 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 ` bluez.test.bot 2021-09-18 2:06 ` bluez.test.bot 2021-09-21 8:46 ` [PATCH v6 1/4] " Marcel Holtmann 5 siblings, 0 replies; 10+ messages in thread From: bluez.test.bot @ 2021-09-16 21:02 UTC (permalink / raw) To: linux-bluetooth, luiz.dentz [-- Attachment #1: Type: text/plain, Size: 1982 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=548423 ---Test result--- Test Summary: CheckPatch FAIL 1.53 seconds GitLint FAIL 1.20 seconds BuildKernel PASS 568.47 seconds TestRunner: Setup PASS 372.38 seconds TestRunner: l2cap-tester PASS 2.81 seconds TestRunner: bnep-tester PASS 2.05 seconds TestRunner: mgmt-tester PASS 32.54 seconds TestRunner: rfcomm-tester PASS 2.30 seconds TestRunner: sco-tester PASS 2.27 seconds TestRunner: smp-tester PASS 2.28 seconds TestRunner: userchan-tester PASS 2.10 seconds Details ############################## Test: CheckPatch - FAIL - 1.53 seconds Run checkpatch.pl script with rule in .checkpatch.conf [v6,3/4] Bluetooth: Fix passing NULL to PTR_ERR\WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #92: bt_skb_sendmsg does never return NULL it is safe to replace the instances of total: 0 errors, 1 warnings, 0 checks, 24 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/12500201.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ############################## Test: GitLint - FAIL - 1.20 seconds Run gitlint with rule in .gitlint [v6,4/4] Bluetooth: SCO: Fix sco_send_frame returning skb->len 6: B1 Line exceeds max length (90>80): "Fixes: 0771cbb3b97d ("Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg")" --- Regards, Linux Bluetooth [-- Attachment #2: l2cap-tester.log --] [-- Type: application/octet-stream, Size: 51566 bytes --] [-- Attachment #3: bnep-tester.log --] [-- Type: application/octet-stream, Size: 3935 bytes --] [-- Attachment #4: mgmt-tester.log --] [-- Type: application/octet-stream, Size: 626845 bytes --] [-- Attachment #5: rfcomm-tester.log --] [-- Type: application/octet-stream, Size: 14791 bytes --] [-- Attachment #6: sco-tester.log --] [-- Type: application/octet-stream, Size: 13045 bytes --] [-- Attachment #7: smp-tester.log --] [-- Type: application/octet-stream, Size: 11858 bytes --] [-- Attachment #8: userchan-tester.log --] [-- Type: application/octet-stream, Size: 6515 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [v6,1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF Luiz Augusto von Dentz ` (3 preceding siblings ...) 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 5 siblings, 0 replies; 10+ messages in thread From: bluez.test.bot @ 2021-09-18 2:06 UTC (permalink / raw) To: linux-bluetooth, luiz.dentz [-- Attachment #1: Type: text/plain, Size: 1998 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=548423 ---Test result--- Test Summary: CheckPatch FAIL 2.55 seconds GitLint FAIL 1.19 seconds BuildKernel PASS 684.73 seconds TestRunner: Setup PASS 444.11 seconds TestRunner: l2cap-tester PASS 3.33 seconds TestRunner: bnep-tester PASS 2.30 seconds TestRunner: mgmt-tester PASS 34.90 seconds TestRunner: rfcomm-tester PASS 2.74 seconds TestRunner: smp-tester PASS 2.61 seconds TestRunner: userchan-tester PASS 2.34 seconds Details ############################## Test: CheckPatch - FAIL - 2.55 seconds Run checkpatch.pl script with rule in .checkpatch.conf [v6,3/4] Bluetooth: Fix passing NULL to PTR_ERR\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #92: bt_skb_sendmsg does never return NULL it is safe to replace the instances of total: 0 errors, 1 warnings, 0 checks, 24 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/12500201.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ############################## Test: GitLint - FAIL - 1.19 seconds Run gitlint with rule in .gitlint [v6,4/4] Bluetooth: SCO: Fix sco_send_frame returning skb->len 6: B1 Line exceeds max length (90>80): "Fixes: 0771cbb3b97d ("Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg")" --- Regards, Linux Bluetooth [-- Attachment #2: l2cap-tester.log --] [-- Type: application/octet-stream, Size: 51567 bytes --] [-- Attachment #3: bnep-tester.log --] [-- Type: application/octet-stream, Size: 3934 bytes --] [-- Attachment #4: mgmt-tester.log --] [-- Type: application/octet-stream, Size: 626845 bytes --] [-- Attachment #5: rfcomm-tester.log --] [-- Type: application/octet-stream, Size: 14790 bytes --] [-- Attachment #6: smp-tester.log --] [-- Type: application/octet-stream, Size: 11859 bytes --] [-- Attachment #7: userchan-tester.log --] [-- Type: application/octet-stream, Size: 7768 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF Luiz Augusto von Dentz ` (4 preceding siblings ...) 2021-09-18 2:06 ` bluez.test.bot @ 2021-09-21 8:46 ` Marcel Holtmann 2021-09-21 18:03 ` Luiz Augusto von Dentz 5 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2021-09-21 8:46 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth 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. Regards Marcel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 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 0 siblings, 1 reply; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-21 18:03 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Tue, Sep 21, 2021 at 1:46 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. > Regards > > Marcel > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 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 0 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2021-09-22 14:19 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth 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. Regards Marcel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 2021-09-22 14:19 ` Marcel Holtmann @ 2021-09-23 21:04 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 10+ messages in thread From: Luiz Augusto von Dentz @ 2021-09-23 21:04 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-23 21:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-16 20:10 [PATCH v6 1/4] Bluetooth: hci_sock: Add support for BT_{SND,RCV}BUF 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 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.