skb_ensure_writable should succeed when it's trying to write to the
header of the unreadable skbs, so it doesn't need an unconditional
skb_frags_readable check. The preceding pskb_may_pull() call will
succeed if write_len is within the head and fail if we're trying to
write to the unreadable payload, so we don't need an additional check.

Removing this check restores DSCP functionality with unreadable skbs as
it's called from dscp_tg.

Cc: willemb@google.com
Cc: sdf@fomichev.me
Cc: asml.silence@gmail.com
Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/skbuff.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 85fc82f72d26..d6420b74ea9c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6261,9 +6261,6 @@ int skb_ensure_writable(struct sk_buff *skb, unsigned int write_len)
 	if (!pskb_may_pull(skb, write_len))
 		return -ENOMEM;
 
-	if (!skb_frags_readable(skb))
-		return -EFAULT;
-
 	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
 		return 0;
 

base-commit: 5466491c9e3309ed5c7adbb8fad6e93fcc9a8fe9
-- 
2.50.0.rc1.591.g9c95f17f64-goog

On 06/15, Mina Almasry wrote:
> skb_ensure_writable should succeed when it's trying to write to the
> header of the unreadable skbs, so it doesn't need an unconditional
> skb_frags_readable check. The preceding pskb_may_pull() call will
> succeed if write_len is within the head and fail if we're trying to
> write to the unreadable payload, so we don't need an additional check.
> 
> Removing this check restores DSCP functionality with unreadable skbs as
> it's called from dscp_tg.

Can you share more info on which use-case (or which call sites) you're
trying to fix?

On Mon, Jun 16, 2025 at 7:38 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
>
> On 06/15, Mina Almasry wrote:
> > skb_ensure_writable should succeed when it's trying to write to the
> > header of the unreadable skbs, so it doesn't need an unconditional
> > skb_frags_readable check. The preceding pskb_may_pull() call will
> > succeed if write_len is within the head and fail if we're trying to
> > write to the unreadable payload, so we don't need an additional check.
> >
> > Removing this check restores DSCP functionality with unreadable skbs as
> > it's called from dscp_tg.
>
> Can you share more info on which use-case (or which call sites) you're
> trying to fix?

Hi Stan,

It's the use case of setting a DSCP header, and the call site is
dscp_tg() -> skb_ensure_writable.

Repro steps should roughly be:

# Set DSCP header
sudo iptables -tmangle -A POSTROUTING -p tcp -m comment --comment
"foo" -j DSCP --set-dscp 0x08

# then run some unreadable netmem workload.

Before this change you should see 0 throughput, after this change the
unreadable netmem workload should work as expected.

-- 
Thanks,
Mina

On 06/16, Mina Almasry wrote:
> On Mon, Jun 16, 2025 at 7:38 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
> >
> > On 06/15, Mina Almasry wrote:
> > > skb_ensure_writable should succeed when it's trying to write to the
> > > header of the unreadable skbs, so it doesn't need an unconditional
> > > skb_frags_readable check. The preceding pskb_may_pull() call will
> > > succeed if write_len is within the head and fail if we're trying to
> > > write to the unreadable payload, so we don't need an additional check.
> > >
> > > Removing this check restores DSCP functionality with unreadable skbs as
> > > it's called from dscp_tg.
> >
> > Can you share more info on which use-case (or which call sites) you're
> > trying to fix?
> 
> Hi Stan,
> 
> It's the use case of setting a DSCP header, and the call site is
> dscp_tg() -> skb_ensure_writable.
> 
> Repro steps should roughly be:
> 
> # Set DSCP header
> sudo iptables -tmangle -A POSTROUTING -p tcp -m comment --comment
> "foo" -j DSCP --set-dscp 0x08
> 
> # then run some unreadable netmem workload.
> 
> Before this change you should see 0 throughput, after this change the
> unreadable netmem workload should work as expected.

Ah, so this is basically all netfilter, makes sense, thanks!

Acked-by: Stanislav Fomichev <sdf@fomichev.me>