[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 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