skb_ensure_writable actually makes sure that the header of the skb is writable, and doesn't touch the payload. It doesn't need an skb_frags_readable check.
Removing this check restores DSCP functionality with unreadable skbs as it's called from dscp_tg.
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: 6d4e01d29d87356924f1521ca6df7a364e948f13
Trivial fix to a couple of outdated netmem comments. No code changes, just more accurately describing current code.
Signed-off-by: Mina Almasry almasrymina@google.com --- include/net/netmem.h | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/net/netmem.h b/include/net/netmem.h index 386164fb9c18..28ca931dc860 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -89,8 +89,7 @@ static inline unsigned int net_iov_idx(const struct net_iov *niov) * typedef netmem_ref - a nonexistent type marking a reference to generic * network memory. * - * A netmem_ref currently is always a reference to a struct page. This - * abstraction is introduced so support for new memory types can be added. + * A netmem_ref can be a struct page* or a struct net_iov* underneath. * * Use the supplied helpers to obtain the underlying memory pointer and fields. */ @@ -117,9 +116,6 @@ static inline struct page *__netmem_to_page(netmem_ref netmem) return (__force struct page *)netmem; }
-/* This conversion fails (returns NULL) if the netmem_ref is not struct page - * backed. - */ static inline struct page *netmem_to_page(netmem_ref netmem) { if (WARN_ON_ONCE(netmem_is_net_iov(netmem))) @@ -178,6 +174,21 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem) return page_to_pfn(netmem_to_page(netmem)); }
+/* __netmem_clear_lsb - clear the lsb of &netmem and return it as + * a net_iov*. + * @netmem: netmem reference to extract as net_iov. + * + * All the sub types of netmem_ref (page, net_iov) have the same pp, pp_magic, + * dma_addr, and pp_ref_count fields at the same offsets. Thus, we can access + * these fields without a type check to make sure that the underlying mem is + * net_iov or page. + * + * The resulting value of this function can only be used to access the fields + * that are NET_IOV_ASSERT_OFFSET'd. Accessing any other fields will result in + * undefined behavior. + * + * Return: the netmem_ref cast to net_iov* regardless of its underlying type. + */ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem) { return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
On Fri, 13 Jun 2025 04:28:02 +0000 Mina Almasry wrote:
+/* __netmem_clear_lsb - clear the lsb of &netmem and return it as
- a net_iov*.
nit: "clear the lsb" spells out the name of the function.. how about: convert netmem_ref to struct net_iov * for metadata access or some such ?
- @netmem: netmem reference to extract as net_iov.
Trivial fix to unused variable.
Signed-off-by: Mina Almasry almasrymina@google.com --- tools/testing/selftests/drivers/net/hw/ncdevmem.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 02e4d3d7ded2..cc9b40d9c5d5 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -852,7 +852,6 @@ static int do_client(struct memory_buffer *mem) ssize_t line_size = 0; struct cmsghdr *cmsg; char *line = NULL; - unsigned long mid; size_t len = 0; int socket_fd; __u32 ddmabuf;
Add ipv4 support to the recently added chunks tests, which was added as ipv6 only.
Signed-off-by: Mina Almasry almasrymina@google.com
--- tools/testing/selftests/drivers/net/hw/devmem.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/devmem.py b/tools/testing/selftests/drivers/net/hw/devmem.py index 7947650210a0..baa2f24240ba 100755 --- a/tools/testing/selftests/drivers/net/hw/devmem.py +++ b/tools/testing/selftests/drivers/net/hw/devmem.py @@ -51,15 +51,14 @@ def check_tx(cfg) -> None:
@ksft_disruptive def check_tx_chunks(cfg) -> None: - cfg.require_ipver("6") require_devmem(cfg)
port = rand_port() - listen_cmd = f"socat -U - TCP6-LISTEN:{port}" + listen_cmd = f"socat -U - TCP{cfg.addr_ipver}-LISTEN:{port}"
with bkg(listen_cmd, exit_wait=True) as socat: wait_port_listen(port) - cmd(f"echo -e "hello\nworld"| {cfg.bin_remote} -f {cfg.ifname} -s {cfg.addr_v['6']} -p {port} -z 3", host=cfg.remote, shell=True) + cmd(f"echo -e "hello\nworld"| {cfg.bin_remote} -f {cfg.ifname} -s {cfg.addr} -p {port} -z 3", host=cfg.remote, shell=True)
ksft_eq(socat.stdout.strip(), "hello\nworld")
On Fri, 13 Jun 2025 04:28:01 +0000 Mina Almasry wrote:
skb_ensure_writable actually makes sure that the header of the skb is writable, and doesn't touch the payload. It doesn't need an skb_frags_readable check.
"doesn't touch the payload" is a bit misleading. What I'd say instead is that the pskb_may_pull() call about ensures that write_len is within the head, and will already fail if write_len would dip into unreadable frags.
Removing this check restores DSCP functionality with unreadable skbs as it's called from dscp_tg.
Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
Signed-off-by: Mina Almasry almasrymina@google.com
Either no Fixes tag or send it to net. I think net is fine, this is clearly incorrect. By definition devmem should not prevent header modifications, so net?
nit: no empty lines between tags
linux-kselftest-mirror@lists.linaro.org
-
Jakub Kicinski -
Mina Almasry