[PATCH v1 net-next] af_unix: Don't check last_len in unix_stream_data_wait().

[PATCH v1 net-next] af_unix: Don't check last_len in unix_stream_data_wait().

[Kuniyuki Iwashima]

When commit 869e7c62486e ("net: af_unix: implement stream sendpage
support") added sendpage() support, data could be appended to the last
skb in the receiver's queue.

That's why we needed to check if the length of the last skb was changed
while waiting for new data in unix_stream_data_wait().

However, commit a0dbf5f818f9 ("af_unix: Support MSG_SPLICE_PAGES") and
commit 57d44a354a43 ("unix: Convert unix_stream_sendpage() to use
MSG_SPLICE_PAGES") refactored sendmsg(), and now data is always added
to a new skb.

Now we no longer need to check the length of the last skb, so let's
remove the dead logic in unix_stream_data_wait().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e4af6616e1df..6b710043dd41 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2505,11 +2505,9 @@ static int unix_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
  *	Sleep until more data has arrived. But check for races..
  */
 static long unix_stream_data_wait(struct sock *sk, long timeo,
-				  struct sk_buff *last, unsigned int last_len,
-				  bool freezable)
+				  struct sk_buff *last, bool freezable)
 {
 	unsigned int state = TASK_INTERRUPTIBLE | freezable * TASK_FREEZABLE;
-	struct sk_buff *tail;
 	DEFINE_WAIT(wait);
 
 	unix_state_lock(sk);
@@ -2517,9 +2515,7 @@ static long unix_stream_data_wait(struct sock *sk, long timeo,
 	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, state);
 
-		tail = skb_peek_tail(&sk->sk_receive_queue);
-		if (tail != last ||
-		    (tail && tail->len != last_len) ||
+		if (skb_peek_tail(&sk->sk_receive_queue) != last ||
 		    sk->sk_err ||
 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
 		    signal_pending(current) ||
@@ -2671,7 +2667,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 	long timeo;
 	int skip;
 	size_t size = state->size;
-	unsigned int last_len;
 
 	if (unlikely(sk->sk_state != TCP_ESTABLISHED)) {
 		err = -EINVAL;
@@ -2710,7 +2705,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 			goto unlock;
 		}
 		last = skb = skb_peek(&sk->sk_receive_queue);
-		last_len = last ? last->len : 0;
 
 again:
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
@@ -2744,8 +2738,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 
 			mutex_unlock(&u->iolock);
 
-			timeo = unix_stream_data_wait(sk, timeo, last,
-						      last_len, freezable);
+			timeo = unix_stream_data_wait(sk, timeo, last, freezable);
 
 			if (signal_pending(current)) {
 				err = sock_intr_errno(timeo);
@@ -2763,7 +2756,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		while (skip >= unix_skb_len(skb)) {
 			skip -= unix_skb_len(skb);
 			last = skb;
-			last_len = skb->len;
+
 			skb = skb_peek_next(skb, &sk->sk_receive_queue);
 			if (!skb)
 				goto again;
@@ -2854,7 +2847,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 
 			skip = 0;
 			last = skb;
-			last_len = skb->len;
+
 			unix_state_lock(sk);
 			skb = skb_peek_next(skb, &sk->sk_receive_queue);
 			if (skb)
-- 
2.30.2

Re: [PATCH v1 net-next] af_unix: Don't check last_len in unix_stream_data_wait().

[Simon Horman]

On Thu, May 30, 2024 at 09:42:56AM -0700, Kuniyuki Iwashima wrote:
> When commit 869e7c62486e ("net: af_unix: implement stream sendpage
> support") added sendpage() support, data could be appended to the last
> skb in the receiver's queue.
> 
> That's why we needed to check if the length of the last skb was changed
> while waiting for new data in unix_stream_data_wait().
> 
> However, commit a0dbf5f818f9 ("af_unix: Support MSG_SPLICE_PAGES") and
> commit 57d44a354a43 ("unix: Convert unix_stream_sendpage() to use
> MSG_SPLICE_PAGES") refactored sendmsg(), and now data is always added
> to a new skb.
> 
> Now we no longer need to check the length of the last skb, so let's
> remove the dead logic in unix_stream_data_wait().
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

...

> @@ -2744,8 +2738,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>  
>  			mutex_unlock(&u->iolock);
>  
> -			timeo = unix_stream_data_wait(sk, timeo, last,
> -						      last_len, freezable);
> +			timeo = unix_stream_data_wait(sk, timeo, last, freezable);

Hi Iwashima-san,

A minor nit from my side. In the case that you have to reason perhaps
keep the line above to <= 80 columns wide.

...