[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

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

[Andrew Lunn]

On Thu, Apr 23, 2026 at 10:36:38AM +0300, Dmitry Antipov wrote:
> 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.

Please always include at least one user of a new helper.

Do you plan to modify all the bluetooth drivers to use this?

	Andrew

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

[Andrew Lunn]

> +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;

A quick look in drivers/bluetooth suggests that none of them care
about truncation, at least they don't check the return code. Maybe it
would be better to truncate than return an error?

I would also try to make the behaviour consistent with the normal
sprintf(), or snprintf(). You want to give users some idea what it is
doing based on its name. It is well known that snprintf() will
truncate, and does not return an error code. sprintf() will just
overwrite the end of the buffer and not return an error code etc.

However calling this skb_snprintf() would be odd, since you don't
actually pass size. But it hints at what it does.

So please add some kerneldoc describing what it actually does.

Please also annotate the const char *fmt, so the compiler can do
format string checks, parameter counting etc.

	Andrew

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

[Dmitry Antipov]

On Thu, 2026-04-23 at 14:40 +0200, Andrew Lunn wrote:

> 
> A quick look in drivers/bluetooth suggests that none of them care
> about truncation, at least they don't check the return code. Maybe it
> would be better to truncate than return an error?

Hm... I'm not a fan of (silent?) truncation and assume that user who wants to
detect it should be able to do so. If a (bluetooth) driver, or a subsystem,
or whatever else, doesn't care too much, it can wrap it with something like

#define bt_skb_printf(skb, fmt, ...)				\
({								\
	if (unlikely(skb_printf(skb, fmt, __VA_ARGS__) < 0)	\
		bt_warn("output to skb@%p truncated\n", skb));	\
})

> I would also try to make the behaviour consistent with the normal
> sprintf(), or snprintf(). You want to give users some idea what it is
> doing based on its name. It is well known that snprintf() will
> truncate, and does not return an error code. sprintf() will just
> overwrite the end of the buffer and not return an error code etc.
> 
> However calling this skb_snprintf() would be odd, since you don't
> actually pass size. But it hints at what it does.
> 
> So please add some kerneldoc describing what it actually does.

OK, some more neutral name like 'skb_fmt(...)' may be better indeed.

> Please also annotate the const char *fmt, so the compiler can do
> format string checks, parameter counting etc.

Surely '__printf(2, 3)' should be used here.

Dmitry

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

[Andrew Lunn]

> > So please add some kerneldoc describing what it actually does.
> 
> OK, some more neutral name like 'skb_fmt(...)' may be better indeed.

Sure. Just document what it actually does, what the return value
means, etc.

> > Please also annotate the const char *fmt, so the compiler can do
> > format string checks, parameter counting etc.
> 
> Surely '__printf(2, 3)' should be used here.

Yep.

Thanks
	Andrew