zerocopy tx cleanups

PATCH Applied Pavel Begunkov 4 participants 15 messages Jun 27, 2024 – Jul 2, 2024

Subsystems (4)

Networking Network drivers io_uring Sockets

lore

Share & tools

lore

Pavel Begunkov

author

2024-06-27 12:59 UTC #0 lore raw

cover zerocopy tx cleanups

Assorted zerocopy send path cleanups, the main part of which is moving some net stack specific accounting out of io_uring back to net/ in Patch 4.

Pavel Begunkov (5):
net: always try to set ubuf in skb_zerocopy_iter_stream
net: split __zerocopy_sg_from_iter()
net: batch zerocopy_fill_skb_from_iter accounting io_uring/net: move charging socket out of zc io_uring
net: limit scope of a skb_zerocopy_iter_stream var

 include/linux/skbuff.h |  3 +++
 include/linux/socket.h |  2 +-
 io_uring/net.c         | 16 ++++----------
 net/core/datagram.c    | 47 +++++++++++++++++++++++++-----------------
 net/core/skbuff.c      |  6 +++---
 5 files changed, 39 insertions(+), 35 deletions(-)

Signature · 1 line

2.44.0

Pavel Begunkov

author

2024-06-27 12:59 UTC #1 lore raw

PATCH 1/5 net: always try to set ubuf in skb_zerocopy_iter_stream

skb_zcopy_set() does nothing if there is already a ubuf_info associated with an skb, and since ->link_skb should have set it several lines above the check here essentially does nothing and can be removed. It's also safer this way, because even if the callback is faulty we'll have it set.

Signed-off-by Pavel Begunkov <[email protected]>

---
 net/core/skbuff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

net/core/skbuff.c +1 -2

index 2315c088e91d..9f28822dde6f 100644
@@ -1884,8 +1884,7 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 		return err;
 	}
 
-	if (!uarg->ops->link_skb)
-		skb_zcopy_set(skb, uarg, NULL);
+	skb_zcopy_set(skb, uarg, NULL);
 	return skb->len - orig_len;
 }
 EXPORT_SYMBOL_GPL(skb_zerocopy_iter_stream);

Signature · 1 line

2.44.0

Willem de Bruijn

Reviewed-by on v1 1/5

2024-06-28 17:03 UTC #2 lore raw

Re: [PATCH net-next 1/5] net: always try to set ubuf in skb_zerocopy_iter_stream

Quoted 7 lines · Pavel Begunkov

skb_zcopy_set() does nothing if there is already a ubuf_info associated with an skb, and since ->link_skb should have set it several lines above the check here essentially does nothing and can be removed. It's also safer this way, because even if the callback is faulty we'll have it set.
Signed-off-by Pavel Begunkov <[email protected]>

Reviewed-by Willem de Bruijn <[email protected]>

Pavel Begunkov

author

2024-06-27 12:59 UTC #3 lore raw

PATCH 2/5 net: split __zerocopy_sg_from_iter()

In reply to Pavel Begunkov

Split a function out of __zerocopy_sg_from_iter() that only cares about the traditional path with refcounted pages and doesn't need to know about ->sg_from_iter. A preparation patch, we'll improve on the function later.

Signed-off-by Pavel Begunkov <[email protected]>

---
 net/core/datagram.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

net/core/datagram.c +13 -9

index 95f242591fd2..7f7d5da2e406 100644
@@ -610,16 +610,10 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
-			    struct sk_buff *skb, struct iov_iter *from,
-			    size_t length)
+static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
+					struct iov_iter *from, size_t length)
 {
-	int frag;
-
-	if (msg && msg->msg_ubuf && msg->sg_from_iter)
-		return msg->sg_from_iter(sk, skb, from, length);
-
-	frag = skb_shinfo(skb)->nr_frags;
+	int frag = skb_shinfo(skb)->nr_frags;
 
 	while (length && iov_iter_count(from)) {
 		struct page *head, *last_head = NULL;

@@ -692,6 +686,16 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
 	}
 	return 0;
 }
+
+int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
+			    struct sk_buff *skb, struct iov_iter *from,
+			    size_t length)
+{
+	if (msg && msg->msg_ubuf && msg->sg_from_iter)
+		return msg->sg_from_iter(sk, skb, from, length);
+	else
+		return zerocopy_fill_skb_from_iter(sk, skb, from, length);
+}
 EXPORT_SYMBOL(__zerocopy_sg_from_iter);
 
 /**

Signature · 1 line

2.44.0

Willem de Bruijn

Reviewed-by on v1 2/5

2024-06-28 17:03 UTC #4 lore raw

Re: [PATCH net-next 2/5] net: split __zerocopy_sg_from_iter()

Quoted 6 lines · Pavel Begunkov

Split a function out of __zerocopy_sg_from_iter() that only cares about the traditional path with refcounted pages and doesn't need to know about ->sg_from_iter. A preparation patch, we'll improve on the function later.
Signed-off-by Pavel Begunkov <[email protected]>

Reviewed-by Willem de Bruijn <[email protected]>

Pavel Begunkov

author

2024-06-27 12:59 UTC #5 lore raw

PATCH 3/5 net: batch zerocopy_fill_skb_from_iter accounting

In reply to Pavel Begunkov

Instead of accounting every page range against the socket separately, do it in batch based on the change in skb->truesize. It's also moved into __zerocopy_sg_from_iter(), so that zerocopy_fill_skb_from_iter() is simpler and responsible for setting frags but not the accounting.

Signed-off-by Pavel Begunkov <[email protected]>

---
 net/core/datagram.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

net/core/datagram.c +18 ~~-13~~

index 7f7d5da2e406..2b24d69b1e94 100644
@@ -610,7 +610,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
+static int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
 					struct iov_iter *from, size_t length)
 {
 	int frag = skb_shinfo(skb)->nr_frags;

@@ -621,7 +621,6 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
 		int refs, order, n = 0;
 		size_t start;
 		ssize_t copied;
-		unsigned long truesize;
 
 		if (frag == MAX_SKB_FRAGS)
 			return -EMSGSIZE;

@@ -633,17 +632,9 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
 
 		length -= copied;
 
-		truesize = PAGE_ALIGN(copied + start);
 		skb->data_len += copied;
 		skb->len += copied;
-		skb->truesize += truesize;
-		if (sk && sk->sk_type == SOCK_STREAM) {
-			sk_wmem_queued_add(sk, truesize);
-			if (!skb_zcopy_pure(skb))
-				sk_mem_charge(sk, truesize);
-		} else {
-			refcount_add(truesize, &skb->sk->sk_wmem_alloc);
-		}
+		skb->truesize += PAGE_ALIGN(copied + start);
 
 		head = compound_head(pages[n]);
 		order = compound_order(head);

@@ -691,10 +682,24 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
 			    struct sk_buff *skb, struct iov_iter *from,
 			    size_t length)
 {
+	unsigned long orig_size = skb->truesize;
+	unsigned long truesize;
+	int ret;
+
 	if (msg && msg->msg_ubuf && msg->sg_from_iter)
 		return msg->sg_from_iter(sk, skb, from, length);
-	else
-		return zerocopy_fill_skb_from_iter(sk, skb, from, length);
+
+	ret = zerocopy_fill_skb_from_iter(skb, from, length);
+	truesize = skb->truesize - orig_size;
+
+	if (sk && sk->sk_type == SOCK_STREAM) {
+		sk_wmem_queued_add(sk, truesize);
+		if (!skb_zcopy_pure(skb))
+			sk_mem_charge(sk, truesize);
+	} else {
+		refcount_add(truesize, &skb->sk->sk_wmem_alloc);
+	}
+	return ret;
 }
 EXPORT_SYMBOL(__zerocopy_sg_from_iter);

Signature · 1 line

2.44.0

Willem de Bruijn

Reviewed-by on v1 3/5

2024-06-28 17:06 UTC #6 lore raw

Re: [PATCH net-next 3/5] net: batch zerocopy_fill_skb_from_iter accounting

Quoted 6 lines · Pavel Begunkov

Instead of accounting every page range against the socket separately, do it in batch based on the change in skb->truesize. It's also moved into __zerocopy_sg_from_iter(), so that zerocopy_fill_skb_from_iter() is simpler and responsible for setting frags but not the accounting.
Signed-off-by Pavel Begunkov <[email protected]>

Reviewed-by Willem de Bruijn <[email protected]>

Quoted 18 lines

---
 net/core/datagram.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

net/core/datagram.c +1 -1

index 7f7d5da2e406..2b24d69b1e94 100644
@@ -610,7 +610,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
+static int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
 					struct iov_iter *from, size_t length)
 {
 	int frag = skb_shinfo(skb)->nr_frags;

net/core/datagram.c static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb, · line 626

Quoted 7 lines

net/core/datagram.c +0 -1

@@ -621,7 +621,6 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
 		int refs, order, n = 0;
 		size_t start;
 		ssize_t copied;
-		unsigned long truesize;
 
 		if (frag == MAX_SKB_FRAGS)
 			return -EMSGSIZE;

Does the existing code then incorrectly not unwind sk_wmem_queued_add and sk_mem_charge if returning with error from the second or later loop..

Quoted 49 lines

net/core/datagram.c +17 ~~-11~~

@@ -633,17 +632,9 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
 
 		length -= copied;
 
-		truesize = PAGE_ALIGN(copied + start);
 		skb->data_len += copied;
 		skb->len += copied;
-		skb->truesize += truesize;
-		if (sk && sk->sk_type == SOCK_STREAM) {
-			sk_wmem_queued_add(sk, truesize);
-			if (!skb_zcopy_pure(skb))
-				sk_mem_charge(sk, truesize);
-		} else {
-			refcount_add(truesize, &skb->sk->sk_wmem_alloc);
-		}
+		skb->truesize += PAGE_ALIGN(copied + start);
 
 		head = compound_head(pages[n]);
 		order = compound_order(head);

@@ -691,10 +682,24 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
 			    struct sk_buff *skb, struct iov_iter *from,
 			    size_t length)
 {
+	unsigned long orig_size = skb->truesize;
+	unsigned long truesize;
+	int ret;
+
 	if (msg && msg->msg_ubuf && msg->sg_from_iter)
 		return msg->sg_from_iter(sk, skb, from, length);
-	else
-		return zerocopy_fill_skb_from_iter(sk, skb, from, length);
+
+	ret = zerocopy_fill_skb_from_iter(skb, from, length);
+	truesize = skb->truesize - orig_size;
+
+	if (sk && sk->sk_type == SOCK_STREAM) {
+		sk_wmem_queued_add(sk, truesize);
+		if (!skb_zcopy_pure(skb))
+			sk_mem_charge(sk, truesize);
+	} else {
+		refcount_add(truesize, &skb->sk->sk_wmem_alloc);
+	}
+	return ret;
 }
 EXPORT_SYMBOL(__zerocopy_sg_from_iter);

Signature · 1 line

2.44.0

Pavel Begunkov

author

2024-07-01 11:11 UTC #7 lore raw

Re: [PATCH net-next 3/5] net: batch zerocopy_fill_skb_from_iter accounting

Quoted 9 lines · Willem de Bruijn

Quoted 6 lines · Pavel Begunkov
Instead of accounting every page range against the socket separately, do it in batch based on the change in skb->truesize. It's also moved into __zerocopy_sg_from_iter(), so that zerocopy_fill_skb_from_iter() is simpler and responsible for setting frags but not the accounting.
Signed-off-by Pavel Begunkov <[email protected]>
Reviewed-by Willem de Bruijn <[email protected]>

Thanks for reviews!

Quoted 23 lines

---
  net/core/datagram.c | 31 ++++++++++++++++++-------------
  1 file changed, 18 insertions(+), 13 deletions(-)

net/core/datagram.c +1 -2

index 7f7d5da2e406..2b24d69b1e94 100644
@@ -610,7 +610,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
  }
  EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
+static int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
  					struct iov_iter *from, size_t length)
  {
  	int frag = skb_shinfo(skb)->nr_frags;

@@ -621,7 +621,6 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
  		int refs, order, n = 0;
  		size_t start;
  		ssize_t copied;
-		unsigned long truesize;

Quoted 6 lines

net/core/datagram.c static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb, · line 626
Quoted 2 lines
net/core/datagram.c +0 -0
  		if (frag == MAX_SKB_FRAGS)
  			return -EMSGSIZE;
Does the existing code then incorrectly not unwind sk_wmem_queued_add and sk_mem_charge if returning with error from the second or later loop..

As long as ->truesize matches what's accounted to the socket, kfree_skb() -> sock_wfree()/->destructor() should take care of it. With sk_mem_charge() I assume __zerocopy_sg_from_iter -> ___pskb_trim() should do it, need to look it up, but if not, it sounds like a temporary over estimation until the skb is put down. I don't see anything concerning. Is that the scenario you're worried about?

Signature · 1 line

Pavel Begunkov

Willem de Bruijn

2024-07-01 18:28 UTC #8 lore raw

Re: [PATCH net-next 3/5] net: batch zerocopy_fill_skb_from_iter accounting

Quoted 43 lines · Pavel Begunkov

Quoted 9 lines · Willem de Bruijn
Quoted 6 lines · Pavel Begunkov
Instead of accounting every page range against the socket separately, do it in batch based on the change in skb->truesize. It's also moved into __zerocopy_sg_from_iter(), so that zerocopy_fill_skb_from_iter() is simpler and responsible for setting frags but not the accounting.
Signed-off-by Pavel Begunkov <[email protected]>
Reviewed-by Willem de Bruijn <[email protected]>
Thanks for reviews!
Quoted 23 lines
Quoted 23 lines
---
  net/core/datagram.c | 31 ++++++++++++++++++-------------
  1 file changed, 18 insertions(+), 13 deletions(-)
net/core/datagram.c +1 -2
index 7f7d5da2e406..2b24d69b1e94 100644
@@ -610,7 +610,7 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
  }
  EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
+static int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
  					struct iov_iter *from, size_t length)
  {
  	int frag = skb_shinfo(skb)->nr_frags;
@@ -621,7 +621,6 @@ static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb,
  		int refs, order, n = 0;
  		size_t start;
  		ssize_t copied;
-		unsigned long truesize;
 
Quoted 6 lines
net/core/datagram.c static int zerocopy_fill_skb_from_iter(struct sock *sk, struct sk_buff *skb, · line 626
Quoted 2 lines
net/core/datagram.c +0 -0
  		if (frag == MAX_SKB_FRAGS)
  			return -EMSGSIZE;
Does the existing code then incorrectly not unwind sk_wmem_queued_add and sk_mem_charge if returning with error from the second or later loop..

Quoted 6 lines

As long as ->truesize matches what's accounted to the socket, kfree_skb() -> sock_wfree()/->destructor() should take care of it. With sk_mem_charge() I assume __zerocopy_sg_from_iter -> ___pskb_trim() should do it, need to look it up, but if not, it sounds like a temporary over estimation until the skb is put down. I don't see anything concerning. Is that the scenario you're worried about?

Oh indeed. Thanks. I don't see ___pskb_trim adjusting except for the cases where it calls skb_condese, but neither does it adjust truesize. So indeed a temporary over estimation until e.g., tcp_wmem_free_skb. Sounds fine.

Pavel Begunkov

author

2024-06-27 12:59 UTC #9 lore raw

PATCH 4/5 io_uring/net: move charging socket out of zc io_uring

In reply to Pavel Begunkov

Currently, io_uring's io_sg_from_iter() duplicates the part of __zerocopy_sg_from_iter() charging pages to the socket. It'd be too easy to miss while changing it in net/, the chunk is not the most straightforward for outside users and full of internal implementation details. io_uring is not a good place to keep it, deduplicate it by moving out of the callback into __zerocopy_sg_from_iter().

Signed-off-by Pavel Begunkov <[email protected]>

---
 include/linux/skbuff.h |  3 +++
 include/linux/socket.h |  2 +-
 io_uring/net.c         | 16 ++++------------
 net/core/datagram.c    | 10 +++++-----
 4 files changed, 13 insertions(+), 18 deletions(-)

include/linux/skbuff.h +3 -0

index f4cda3fbdb75..9c29bdd5596d 100644
@@ -1703,6 +1703,9 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
 			    struct sk_buff *skb, struct iov_iter *from,
 			    size_t length);
 
+int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
+				struct iov_iter *from, size_t length);
+
 static inline int skb_zerocopy_iter_dgram(struct sk_buff *skb,
 					  struct msghdr *msg, int len)
 {

include/linux/socket.h +1 -1

index 89d16b90370b..2a1ff91d1914 100644
@@ -76,7 +76,7 @@ struct msghdr {
 	__kernel_size_t	msg_controllen;	/* ancillary data buffer length */
 	struct kiocb	*msg_iocb;	/* ptr to iocb for async requests */
 	struct ubuf_info *msg_ubuf;
-	int (*sg_from_iter)(struct sock *sk, struct sk_buff *skb,
+	int (*sg_from_iter)(struct sk_buff *skb,
 			    struct iov_iter *from, size_t length);
 };

io_uring/net.c +4 ~~-12~~

index 7c98c4d50946..84a7602bcef1 100644
@@ -1265,14 +1265,14 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	return io_sendmsg_prep_setup(req, req->opcode == IORING_OP_SENDMSG_ZC);
 }
 
-static int io_sg_from_iter_iovec(struct sock *sk, struct sk_buff *skb,
+static int io_sg_from_iter_iovec(struct sk_buff *skb,
 				 struct iov_iter *from, size_t length)
 {
 	skb_zcopy_downgrade_managed(skb);
-	return __zerocopy_sg_from_iter(NULL, sk, skb, from, length);
+	return zerocopy_fill_skb_from_iter(skb, from, length);
 }
 
-static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
+static int io_sg_from_iter(struct sk_buff *skb,
 			   struct iov_iter *from, size_t length)
 {
 	struct skb_shared_info *shinfo = skb_shinfo(skb);

@@ -1285,7 +1285,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
 	if (!frag)
 		shinfo->flags |= SKBFL_MANAGED_FRAG_REFS;
 	else if (unlikely(!skb_zcopy_managed(skb)))
-		return __zerocopy_sg_from_iter(NULL, sk, skb, from, length);
+		return zerocopy_fill_skb_from_iter(skb, from, length);
 
 	bi.bi_size = min(from->count, length);
 	bi.bi_bvec_done = from->iov_offset;

@@ -1312,14 +1312,6 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
 	skb->data_len += copied;
 	skb->len += copied;
 	skb->truesize += truesize;
-
-	if (sk && sk->sk_type == SOCK_STREAM) {
-		sk_wmem_queued_add(sk, truesize);
-		if (!skb_zcopy_pure(skb))
-			sk_mem_charge(sk, truesize);
-	} else {
-		refcount_add(truesize, &skb->sk->sk_wmem_alloc);
-	}
 	return ret;
 }

net/core/datagram.c +5 -5

index 2b24d69b1e94..554316c40883 100644
@@ -610,8 +610,8 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-static int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
-					struct iov_iter *from, size_t length)
+int zerocopy_fill_skb_from_iter(struct sk_buff *skb,
+				struct iov_iter *from, size_t length)
 {
 	int frag = skb_shinfo(skb)->nr_frags;

@@ -687,11 +687,11 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
 	int ret;
 
 	if (msg && msg->msg_ubuf && msg->sg_from_iter)
-		return msg->sg_from_iter(sk, skb, from, length);
+		ret = msg->sg_from_iter(skb, from, length);
+	else
+		ret = zerocopy_fill_skb_from_iter(skb, from, length);
 
-	ret = zerocopy_fill_skb_from_iter(skb, from, length);
 	truesize = skb->truesize - orig_size;
-
 	if (sk && sk->sk_type == SOCK_STREAM) {
 		sk_wmem_queued_add(sk, truesize);
 		if (!skb_zcopy_pure(skb))

Signature · 1 line

2.44.0

Willem de Bruijn

Reviewed-by on v1 4/5

2024-06-28 17:06 UTC #10 lore raw

Re: [PATCH net-next 4/5] io_uring/net: move charging socket out of zc io_uring

Quoted 8 lines · Pavel Begunkov

Currently, io_uring's io_sg_from_iter() duplicates the part of __zerocopy_sg_from_iter() charging pages to the socket. It'd be too easy to miss while changing it in net/, the chunk is not the most straightforward for outside users and full of internal implementation details. io_uring is not a good place to keep it, deduplicate it by moving out of the callback into __zerocopy_sg_from_iter().
Signed-off-by Pavel Begunkov <[email protected]>

Reviewed-by Willem de Bruijn <[email protected]>

Pavel Begunkov

author

2024-06-27 12:59 UTC #11 lore raw

PATCH 5/5 net: limit scope of a skb_zerocopy_iter_stream var

In reply to Pavel Begunkov

skb_zerocopy_iter_stream() only uses @orig_uarg in the !link_skb path, and we can move the local variable in the appropriate block.

Signed-off-by Pavel Begunkov <[email protected]>

---
 net/core/skbuff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

net/core/skbuff.c +2 -1

index 9f28822dde6f..9b71e4b8796a 100644
@@ -1856,7 +1856,6 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 			     struct msghdr *msg, int len,
 			     struct ubuf_info *uarg)
 {
-	struct ubuf_info *orig_uarg = skb_zcopy(skb);
 	int err, orig_len = skb->len;
 
 	if (uarg->ops->link_skb) {

@@ -1864,6 +1863,8 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 		if (err)
 			return err;
 	} else {
+		struct ubuf_info *orig_uarg = skb_zcopy(skb);
+
 		/* An skb can only point to one uarg. This edge case happens
 		 * when TCP appends to an skb, but zerocopy_realloc triggered
 		 * a new alloc.

Signature · 1 line

2.44.0

Willem de Bruijn

Reviewed-by on v1 5/5

2024-06-28 17:07 UTC #12 lore raw

Re: [PATCH net-next 5/5] net: limit scope of a skb_zerocopy_iter_stream var

Quoted 4 lines · Pavel Begunkov

skb_zerocopy_iter_stream() only uses @orig_uarg in the !link_skb path, and we can move the local variable in the appropriate block.
Signed-off-by Pavel Begunkov <[email protected]>

Reviewed-by Willem de Bruijn <[email protected]>

Jens Axboe

maintainer Reviewed-by on the v1 series

2024-06-28 17:09 UTC #13 lore raw

In reply to Pavel Begunkov

Quoted 3 lines · Pavel Begunkov

Assorted zerocopy send path cleanups, the main part of which is moving some net stack specific accounting out of io_uring back to net/ in Patch 4.

Looks good to me:

Reviewed-by Jens Axboe <[email protected]>

Signature · 1 line

Jens Axboe

[email protected]

CI Applied

2024-07-02 10:30 UTC #14 lore raw

In reply to Pavel Begunkov

This series was applied to netdev/net-next.git (main) by Paolo Abeni <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Paolo Abeni <[email protected]>:

Quoted 4 lines · On Thu, 27 Jun 2024 13:59:40 +0100 you wrote:

Assorted zerocopy send path cleanups, the main part of which is moving some net stack specific accounting out of io_uring back to net/ in Patch 4.

Quoted 8 lines

Pavel Begunkov (5):
net: always try to set ubuf in skb_zerocopy_iter_stream
net: split __zerocopy_sg_from_iter()
net: batch zerocopy_fill_skb_from_iter accounting io_uring/net: move charging socket out of zc io_uring
net: limit scope of a skb_zerocopy_iter_stream var
[...]

Here is the summary with links:
- [net-next,1/5] net: always try to set ubuf in skb_zerocopy_iter_stream

    https://git.kernel.org/netdev/net-next/c/9e2db9d3993e
  - [net-next,2/5] net: split __zerocopy_sg_from_iter()
    https://git.kernel.org/netdev/net-next/c/7fb05423fed4
  - [net-next,3/5] net: batch zerocopy_fill_skb_from_iter accounting
    https://git.kernel.org/netdev/net-next/c/aeb320fc05c7
  - [net-next,4/5] io_uring/net: move charging socket out of zc io_uring
    https://git.kernel.org/netdev/net-next/c/060f4ba6e403
  - [net-next,5/5] net: limit scope of a skb_zerocopy_iter_stream var
    https://git.kernel.org/netdev/net-next/c/2ca58ed21cef

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html