splice from half-closed socket returning -EAGAIN

splice from half-closed socket returning -EAGAIN

[Lennert Buytenhek]

Hi,

When doing a nonblocking splice() from a half-closed TCP socket
(POLLRDHUP, FIN sent by the other side), I get -EAGAIN, while I was
expecting that to return zero.  Non-blocking read/readv/recv/recvfrom/
recvmsg on a half-closed socket also returns zero AFAIK, and in
general, a zero return on a read operation meaning "EOF" seems to be
well-established.

Is there any reason why splice() doesn't do this?  -EAGAIN hints that
the operation might succeed in the future, but since the socket is
half-closed, this won't ever happen.  Also, it's kind of annoying to
have to poll for POLLRDHUP separately, as this isn't needed when using
"copyful" socket I/O -- and there doesn't seem to be any other way of
telling that a half-close has occured on the source socket.

(Test app attached, tested on 2.6.27.5 and 2.6.28.)


thanks,
Lennert




#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <string.h>
#include <unistd.h>

int listening_fd;
int connecting_fd;
int accepted_fd;


int main()
{
	struct sockaddr_in addr;
	socklen_t addrlen;
	int pfd[2];
	int ret;

	/*
	 * Step 1: set up listening socket.
	 */
	listening_fd = socket(AF_INET, SOCK_STREAM, 0);
	if (listening_fd < 0) {
		perror("socket");
		abort();
	}

	addr.sin_family = AF_INET;
	addr.sin_addr.s_addr = htonl(INADDR_ANY);
	addr.sin_port = htons(6667);
	if (bind(listening_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
		perror("bind");
		abort();
	}

	listen(listening_fd, 1);


	/*
	 * Step 2: set up socket to connect to listening socket
	 * created in step 1.
 	 */
	connecting_fd = socket(AF_INET, SOCK_STREAM, 0);
	if (connecting_fd < 0) {
		perror("socket");
		exit(-1);
	}

	addr.sin_family = AF_INET;
	addr.sin_addr.s_addr = htonl(0x7f000001);
	addr.sin_port = htons(6667);
	if (connect(connecting_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
		perror("connect");
		exit(-1);
	}


	/*
	 * Step 3: accept the connection created in step 2.
	 */
	addrlen = sizeof(addr);
	accepted_fd = accept(listening_fd, (struct sockaddr *)&addr, &addrlen);
	if (accepted_fd < 0) {
		perror("accept");
		exit(-1);
	}


	/*
	 * Step 4: close the other half of the connection, and try
	 * to splice into a pipe.
	 */
	close(connecting_fd);

	if (pipe(pfd) < 0) {
		perror("pipe");
		exit(-1);
	}

	ret = splice(accepted_fd, NULL, pfd[1], NULL, 4096, SPLICE_F_NONBLOCK);
	printf("splice returned %d\n", ret);
	if (ret < 0)
		perror("splice");

	return 0;
}

Re: splice from half-closed socket returning -EAGAIN

[Lennert Buytenhek]

On Sun, Jan 04, 2009 at 02:30:54PM +0100, Lennert Buytenhek wrote:

> When doing a nonblocking splice() from a half-closed TCP socket
> (POLLRDHUP, FIN sent by the other side), I get -EAGAIN, while I was
> expecting that to return zero.  Non-blocking read/readv/recv/recvfrom/
> recvmsg on a half-closed socket also returns zero AFAIK, and in
> general, a zero return on a read operation meaning "EOF" seems to be
> well-established.
> 
> Is there any reason why splice() doesn't do this?  -EAGAIN hints that
> the operation might succeed in the future, but since the socket is
> half-closed, this won't ever happen.  Also, it's kind of annoying to
> have to poll for POLLRDHUP separately, as this isn't needed when using
> "copyful" socket I/O -- and there doesn't seem to be any other way of
> telling that a half-close has occured on the source socket.
> 
> (Test app attached, tested on 2.6.27.5 and 2.6.28.)

How about something like this?


From: Lennert Buytenhek <buytenh@wantsofly.org>
Subject: tcp: don't mask EOF and socket errors on nonblocking splice receive

Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
results in masking of EOF (RDHUP) and error conditions on the socket
by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
to be after the EOF and error checks to fix this.

Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c5aca0b..9e31f91 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -571,41 +571,41 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 	lock_sock(sk);
 
 	timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
 	while (tss.len) {
 		ret = __tcp_splice_read(sk, &tss);
 		if (ret < 0)
 			break;
 		else if (!ret) {
 			if (spliced)
 				break;
-			if (flags & SPLICE_F_NONBLOCK) {
-				ret = -EAGAIN;
-				break;
-			}
 			if (sock_flag(sk, SOCK_DONE))
 				break;
 			if (sk->sk_err) {
 				ret = sock_error(sk);
 				break;
 			}
 			if (sk->sk_shutdown & RCV_SHUTDOWN)
 				break;
 			if (sk->sk_state == TCP_CLOSE) {
 				/*
 				 * This occurs when user tries to read
 				 * from never connected socket.
 				 */
 				if (!sock_flag(sk, SOCK_DONE))
 					ret = -ENOTCONN;
 				break;
 			}
+			if (flags & SPLICE_F_NONBLOCK) {
+				ret = -EAGAIN;
+				break;
+			}
 			if (!timeo) {
 				ret = -EAGAIN;
 				break;
 			}
 			sk_wait_data(sk, &timeo);
 			if (signal_pending(current)) {
 				ret = sock_intr_errno(timeo);
 				break;
 			}
 			continue;

Re: splice from half-closed socket returning -EAGAIN

[David Miller]

From: Lennert Buytenhek <buytenh@wantstofly.org>
Date: Mon, 5 Jan 2009 07:47:29 +0100

> tcp: don't mask EOF and socket errors on nonblocking splice receive
> 
> Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> results in masking of EOF (RDHUP) and error conditions on the socket
> by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> to be after the EOF and error checks to fix this.
> 
> Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>

This change looks like the perfect fix for this problem.

I'll apply this, thanks Lennert!

Re: splice from half-closed socket returning -EAGAIN

[Jens Axboe]

On Sun, Jan 04 2009, David Miller wrote:
> From: Lennert Buytenhek <buytenh@wantstofly.org>
> Date: Mon, 5 Jan 2009 07:47:29 +0100
> 
> > tcp: don't mask EOF and socket errors on nonblocking splice receive
> > 
> > Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> > results in masking of EOF (RDHUP) and error conditions on the socket
> > by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> > to be after the EOF and error checks to fix this.
> > 
> > Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> 
> This change looks like the perfect fix for this problem.
> 
> I'll apply this, thanks Lennert!

Indeed, you can add my acked-by as well if you want.

-- 
Jens Axboe

Re: splice from half-closed socket returning -EAGAIN

[Jarek Poplawski]

On Sun, Jan 04, 2009 at 11:59:46PM -0800, David Miller wrote:
> From: Lennert Buytenhek <buytenh@wantstofly.org>
> Date: Mon, 5 Jan 2009 07:47:29 +0100
> 
> > tcp: don't mask EOF and socket errors on nonblocking splice receive
> > 
> > Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> > results in masking of EOF (RDHUP) and error conditions on the socket
> > by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> > to be after the EOF and error checks to fix this.
> > 
> > Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> 
> This change looks like the perfect fix for this problem.
> 

Actually, I wonder why this "if (flags & SPLICE_F_NONBLOCK)" can't
be skipped at all. Isn't "if (!timeo)" enough now?

Jarek P.

Re: splice from half-closed socket returning -EAGAIN

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 5 Jan 2009 08:11:37 +0000

> On Sun, Jan 04, 2009 at 11:59:46PM -0800, David Miller wrote:
> > From: Lennert Buytenhek <buytenh@wantstofly.org>
> > Date: Mon, 5 Jan 2009 07:47:29 +0100
> > 
> > > tcp: don't mask EOF and socket errors on nonblocking splice receive
> > > 
> > > Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> > > results in masking of EOF (RDHUP) and error conditions on the socket
> > > by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> > > to be after the EOF and error checks to fix this.
> > > 
> > > Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> > 
> > This change looks like the perfect fix for this problem.
> > 
> 
> Actually, I wonder why this "if (flags & SPLICE_F_NONBLOCK)" can't
> be skipped at all. Isn't "if (!timeo)" enough now?

Is it really the same condition in the end?

Re: splice from half-closed socket returning -EAGAIN

[Jarek Poplawski]

On Mon, Jan 05, 2009 at 12:14:01AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 5 Jan 2009 08:11:37 +0000
> 
> > On Sun, Jan 04, 2009 at 11:59:46PM -0800, David Miller wrote:
> > > From: Lennert Buytenhek <buytenh@wantstofly.org>
> > > Date: Mon, 5 Jan 2009 07:47:29 +0100
> > > 
> > > > tcp: don't mask EOF and socket errors on nonblocking splice receive
> > > > 
> > > > Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> > > > results in masking of EOF (RDHUP) and error conditions on the socket
> > > > by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> > > > to be after the EOF and error checks to fix this.
> > > > 
> > > > Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> > > 
> > > This change looks like the perfect fix for this problem.
> > > 
> > 
> > Actually, I wonder why this "if (flags & SPLICE_F_NONBLOCK)" can't
> > be skipped at all. Isn't "if (!timeo)" enough now?
> 
> Is it really the same condition in the end?

Yes!?

Jarek P.

Re: splice from half-closed socket returning -EAGAIN

[Lennert Buytenhek]

On Mon, Jan 05, 2009 at 12:14:01AM -0800, David Miller wrote:

> > > > tcp: don't mask EOF and socket errors on nonblocking splice receive
> > > > 
> > > > Currently, setting SPLICE_F_NONBLOCK on splice from a TCP socket
> > > > results in masking of EOF (RDHUP) and error conditions on the socket
> > > > by an -EAGAIN return.  Move the NONBLOCK check in tcp_splice_read()
> > > > to be after the EOF and error checks to fix this.
> > > > 
> > > > Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> > > 
> > > This change looks like the perfect fix for this problem.
> > > 
> > 
> > Actually, I wonder why this "if (flags & SPLICE_F_NONBLOCK)" can't
> > be skipped at all. Isn't "if (!timeo)" enough now?
> 
> Is it really the same condition in the end?

It _seems_ to be?  This:

        timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);

sets 'timeo' to zero if SPLICE_F_NONBLOCK is set.  And when we get
to the timeo / SPLICE_F_NONBLOCK checks, it must be in the first
iteration of the loop, because if 'spliced' was nonzero, it would
have broken out of the loop earlier, and we wouldn't have reached
the check at all.

So it would seem that the SPLICE_F_NONBLOCK check can be deleted
entirely.

Re: splice from half-closed socket returning -EAGAIN

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 5 Jan 2009 08:22:52 +0000

> On Mon, Jan 05, 2009 at 12:14:01AM -0800, David Miller wrote:
> > Is it really the same condition in the end?
> 
> Yes!?

Indeed, as you explained in your follow-up, this splice
non-block flag is used to compute the socket timeout value
and thus it is in fact totally unnecessary.

I'll toss these checks in the net-2.6 tree, thanks!

Re: splice from half-closed socket returning -EAGAIN

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 05 Jan 2009 00:52:49 -0800 (PST)

> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 5 Jan 2009 08:22:52 +0000
> 
> > On Mon, Jan 05, 2009 at 12:14:01AM -0800, David Miller wrote:
> > > Is it really the same condition in the end?
> > 
> > Yes!?
> 
> Indeed, as you explained in your follow-up, this splice
             ^^^

I meant Lennert here, of course.