[RFC PATCH] net: skb: on zero-copy formatted output to skb

[RFC PATCH] net: skb: on zero-copy formatted output to skb

[Dmitry Antipov]

Some code, most notably the Bluetooth drivers, uses something like
the following:

char buf[80];
snprintf(buf, sizeof(buf), "Driver: %s\n", driver_name);
skb_put_data(skb, buf, strlen(buf));

This looks suboptimal at least because:

1) It yields in BUG() just in case the developer underestimates
   the size of an skb being used;
2) It requires extra data copy from an external buffer;
3) It uses 'strlen()' redundantly because actual data length
   is calculated by 'snprintf()' itself.

So introduce 'skb_printf()' which aims to address all of these
issues. As usual, thoughts and comments are highly appreciated.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 include/linux/skbuff.h |  1 +
 net/core/skbuff.c      | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2bcf78a4de7b..fb4ef55a8f86 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4292,6 +4292,7 @@ int skb_mpls_update_lse(struct sk_buff *skb, __be32 mpls_lse);
 int skb_mpls_dec_ttl(struct sk_buff *skb);
 struct sk_buff *pskb_extract(struct sk_buff *skb, int off, int to_copy,
 			     gfp_t gfp);
+int skb_printf(struct sk_buff *skb, const char *fmt, ...);
 
 static inline int memcpy_from_msg(void *data, struct msghdr *msg, int len)
 {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7dad68e3b518..051ab4f28c75 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6992,6 +6992,24 @@ struct sk_buff *pskb_extract(struct sk_buff *skb, int off,
 }
 EXPORT_SYMBOL(pskb_extract);
 
+int skb_printf(struct sk_buff *skb, const char *fmt, ...)
+{
+	int len, size = skb_availroom(skb);
+	va_list args;
+
+	va_start(args, fmt);
+	len = vsnprintf(skb_tail_pointer(skb), size, fmt, args);
+	va_end(args);
+
+	if (unlikely(len >= size))
+		return -ENOSPC;
+
+	skb->tail += len;
+	skb->len += len;
+	return len;
+}
+EXPORT_SYMBOL(skb_printf);
+
 /**
  * skb_condense - try to get rid of fragments/frag_list if possible
  * @skb: buffer
-- 
2.53.0

RE: [RFC] net: skb: on zero-copy formatted output to skb

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 3771 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=1084557

---Test result---

Test Summary:
CheckPatch                    PASS      0.82 seconds
GitLint                       PASS      0.24 seconds
SubjectPrefix                 FAIL      0.09 seconds
BuildKernel                   PASS      26.07 seconds
CheckAllWarning               PASS      33.93 seconds
CheckSparse                   PASS      27.78 seconds
BuildKernel32                 PASS      26.43 seconds
TestRunnerSetup               PASS      578.11 seconds
TestRunner_l2cap-tester       PASS      28.36 seconds
TestRunner_iso-tester         PASS      46.37 seconds
TestRunner_bnep-tester        PASS      6.63 seconds
TestRunner_mgmt-tester        FAIL      115.81 seconds
TestRunner_rfcomm-tester      PASS      9.54 seconds
TestRunner_sco-tester         PASS      14.67 seconds
TestRunner_ioctl-tester       PASS      10.66 seconds
TestRunner_mesh-tester        FAIL      12.30 seconds
TestRunner_smp-tester         PASS      8.84 seconds
TestRunner_userchan-tester    PASS      7.13 seconds
TestRunner_6lowpan-tester     FAIL      8.54 seconds
IncrementalBuild              PASS      25.21 seconds

Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.115 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.732 seconds
Mesh - Send cancel - 2                               Timed out    1.996 seconds
##############################
Test: TestRunner_6lowpan-tester - FAIL
Desc: Run 6lowpan-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
7.0.0-rc7-g9a5a979a67e1 #1 Not tainted
------------------------------------------------------
kworker/0:1/13 is trying to acquire lock:
ffff88800233f140 ((wq_completion)hci0#2){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x75/0x180

but task is already holding lock:
ffffffff9264faa0 (rtnl_mutex){+.+.}-{4:4}, at: lowpan_unregister_netdev+0xd/0x30

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #4 (rtnl_mutex){+.+.}-{4:4}:
       lock_acquire+0xf7/0x2c0
       __mutex_lock+0x16b/0x1fc0
       lowpan_register_netdev+0x11/0x30
       chan_ready_cb+0x836/0xd00
       l2cap_recv_frame+0x61bb/0x88e0
       l2cap_recv_acldata+0x790/0xdf0
       hci_rx_work+0x500/0xd00
       process_scheduled_works+0xba7/0x1a90
       worker_thread+0x514/0xbb0
       kthread+0x368/0x490
       ret_from_fork+0x498/0x7e0
       ret_from_fork_asm+0x19/0x30

-> #3 (&chan->lock#3/1){+.+.}-{4:4}:
       lock_acquire+0xf7/0x2c0
       __mutex_lock+0x16b/0x1fc0
       l2cap_chan_connect+0x74e/0x1980
       lowpan_control_write+0x523/0x660
       full_proxy_write+0x10b/0x190
       vfs_write+0x1c0/0xf60
       ksys_write+0xf1/0x1d0
       do_syscall_64+0xa0/0x570
       entry_SYSCALL_64_after_hwframe+0x74/0x7c

-> #2 (&conn->lock){+.+.}-{4:4}:
...
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0


https://github.com/bluez/bluetooth-next/pull/117

---
Regards,
Linux Bluetooth