[PATCH net-next] Squash-to: "selftests: mptcp: mptfo Initiator/Listener"

Paolo Abeni posted 1 patch 4 weeks, 1 day ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../selftests/net/mptcp/mptcp_connect.c       | 118 ++++++++----------
1 file changed, 55 insertions(+), 63 deletions(-)
[PATCH net-next] Squash-to: "selftests: mptcp: mptfo Initiator/Listener"
Posted by Paolo Abeni 4 weeks, 1 day ago
The TFO is allowed to send less data than the provided one,
we need to track more status info to properly allow the next sendmsg()
starting from the next to send byte.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 .../selftests/net/mptcp/mptcp_connect.c       | 118 ++++++++----------
 1 file changed, 55 insertions(+), 63 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 4bc855159c52..134e569cd75a 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -26,13 +26,11 @@
 
 #include <netdb.h>
 #include <netinet/in.h>
-#include <netinet/tcp.h>
 
+#include <linux/tcp.h>
 #include <linux/time_types.h>
 #include <linux/sockios.h>
 
-
-
 extern int optind;
 
 #ifndef IPPROTO_MPTCP
@@ -93,6 +91,12 @@ struct tcp_inq_state {
 	bool expect_eof;
 };
 
+struct wstate {
+	char buf[8192];
+	unsigned int len;
+	unsigned int off;
+};
+
 static struct tcp_inq_state tcp_inq;
 
 static struct cfg_cmsg_types cfg_cmsg_types;
@@ -239,7 +243,7 @@ static void set_mptfo(int fd, int pf)
 {
 	int qlen = 25;
 
-	if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
+	if (setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
 		perror("TCP_FASTOPEN");
 }
 
@@ -345,17 +349,14 @@ static int sock_listen_mptcp(const char * const listenaddr,
 static int sock_connect_mptcp(const char * const remoteaddr,
 			      const char * const port, int proto,
 			      struct addrinfo **peer,
-			      int infd,
-			      unsigned int *woff)
+			      int infd, struct wstate *winfo)
 {
 	struct addrinfo hints = {
 		.ai_protocol = IPPROTO_TCP,
 		.ai_socktype = SOCK_STREAM,
 	};
 	struct addrinfo *a, *addr;
-	unsigned int wlen = 0;
 	int syn_copied = 0;
-	char wbuf[8192];
 	int sock = -1;
 
 	hints.ai_family = pf;
@@ -374,13 +375,15 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 			set_mark(sock, cfg_mark);
 
 		if (cfg_sockopt_types.mptfo) {
-			if (wlen == 0)
-				wlen = read(infd, wbuf, sizeof(wbuf));
+			winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
 
-			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
+			syn_copied = sendto(sock, winfo->buf, winfo->len, MSG_FASTOPEN,
 					    a->ai_addr, a->ai_addrlen);
+			if (syn_copied < 0)
+				perror("sendto");
 			if (syn_copied) {
-				*woff = wlen;
+				winfo->off = syn_copied;
+				winfo->len -= syn_copied;
 				*peer = a;
 				break; /* success */
 			}
@@ -610,14 +613,13 @@ static void shut_wr(int fd)
 }
 
 static int copyfd_io_poll(int infd, int peerfd, int outfd,
-			  bool *in_closed_after_out, unsigned int woff)
+			  bool *in_closed_after_out, struct wstate *winfo)
 {
 	struct pollfd fds = {
 		.fd = peerfd,
 		.events = POLLIN | POLLOUT,
 	};
-	unsigned int wlen = 0, total_wlen = 0, total_rlen = 0;
-	char wbuf[8192];
+	unsigned int total_wlen = 0, total_rlen = 0;
 
 	set_nonblock(peerfd, true);
 
@@ -677,19 +679,19 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 		}
 
 		if (fds.revents & POLLOUT) {
-			if (wlen == 0) {
-				woff = 0;
-				wlen = read(infd, wbuf, sizeof(wbuf));
+			if (winfo->len == 0) {
+				winfo->off = 0;
+				winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
 			}
 
-			if (wlen > 0) {
+			if (winfo->len > 0) {
 				ssize_t bw;
 
 				/* limit the total amount of written data to the trunc value */
-				if (cfg_truncate > 0 && wlen + total_wlen > cfg_truncate)
-					wlen = cfg_truncate - total_wlen;
+				if (cfg_truncate > 0 && winfo->len + total_wlen > cfg_truncate)
+					winfo->len = cfg_truncate - total_wlen;
 
-				bw = do_rnd_write(peerfd, wbuf + woff, wlen);
+				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
 				if (bw < 0) {
 					if (cfg_rcv_trunc)
 						return 0;
@@ -697,10 +699,10 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
 					return 111;
 				}
 
-				woff += bw;
-				wlen -= bw;
+				winfo->off += bw;
+				winfo->len -= bw;
 				total_wlen += bw;
-			} else if (wlen == 0) {
+			} else if (winfo->len == 0) {
 				/* We have no more data to send. */
 				fds.events &= ~POLLOUT;
 
@@ -878,7 +880,7 @@ static int copyfd_io_sendfile(int infd, int peerfd, int outfd,
 	return err;
 }
 
-static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigned int woff)
+static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, struct wstate *winfo)
 {
 	bool in_closed_after_out = false;
 	struct timespec start, end;
@@ -890,7 +892,7 @@ static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigne
 
 	switch (cfg_mode) {
 	case CFG_MODE_POLL:
-		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, woff);
+		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, winfo);
 		break;
 
 	case CFG_MODE_MMAP:
@@ -1038,6 +1040,7 @@ static void maybe_close(int fd)
 int main_loop_s(int listensock)
 {
 	struct sockaddr_storage ss;
+	struct wstate winfo;
 	struct pollfd polls;
 	socklen_t salen;
 	int remotesock;
@@ -1072,7 +1075,8 @@ int main_loop_s(int listensock)
 
 		SOCK_TEST_TCPULP(remotesock, 0);
 
-		copyfd_io(fd, remotesock, 1, true, 0);
+		memset(&winfo, 0, sizeof(winfo));
+		copyfd_io(fd, remotesock, 1, true, &winfo);
 	} else {
 		perror("accept");
 		return 1;
@@ -1210,53 +1214,40 @@ void xdisconnect(int fd, int addrlen)
 
 int main_loop(void)
 {
-	int fd, ret, fd_in = 0;
+	int fd = 0, ret, fd_in = 0;
 	struct addrinfo *peer;
-	unsigned int woff = 0;
-
-	if (!cfg_sockopt_types.mptfo) {
-		/* listener is ready. */
-		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, 0, 0);
-		if (fd < 0)
-			return 2;
-again:
-		check_getpeername_connect(fd);
+	struct wstate winfo;
 
-		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
-
-		if (cfg_rcvbuf)
-			set_rcvbuf(fd, cfg_rcvbuf);
-		if (cfg_sndbuf)
-			set_sndbuf(fd, cfg_sndbuf);
-		if (cfg_cmsg_types.cmsg_enabled)
-			apply_cmsg_types(fd, &cfg_cmsg_types);
-	}
-
-	if (cfg_input) {
+	if (cfg_input && cfg_sockopt_types.mptfo) {
 		fd_in = open(cfg_input, O_RDONLY);
 		if (fd < 0)
 			xerror("can't open %s:%d", cfg_input, errno);
 	}
 
-	if (cfg_sockopt_types.mptfo) {
-		/* sendto() instead of connect */
-		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &woff);
-		if (fd < 0)
-			return 2;
+	memset(&winfo, 0, sizeof(winfo));
+	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &winfo);
+	if (fd < 0)
+		return 2;
 
-		check_getpeername_connect(fd);
+again:
+	check_getpeername_connect(fd);
 
-		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
+	SOCK_TEST_TCPULP(fd, cfg_sock_proto);
 
-		if (cfg_rcvbuf)
-			set_rcvbuf(fd, cfg_rcvbuf);
-		if (cfg_sndbuf)
-			set_sndbuf(fd, cfg_sndbuf);
-		if (cfg_cmsg_types.cmsg_enabled)
-			apply_cmsg_types(fd, &cfg_cmsg_types);
+	if (cfg_rcvbuf)
+		set_rcvbuf(fd, cfg_rcvbuf);
+	if (cfg_sndbuf)
+		set_sndbuf(fd, cfg_sndbuf);
+	if (cfg_cmsg_types.cmsg_enabled)
+		apply_cmsg_types(fd, &cfg_cmsg_types);
+
+	if (cfg_input && !cfg_sockopt_types.mptfo) {
+		fd_in = open(cfg_input, O_RDONLY);
+		if (fd < 0)
+			xerror("can't open %s:%d", cfg_input, errno);
 	}
 
-	ret = copyfd_io(fd_in, fd, 1, 0, 0);
+	ret = copyfd_io(fd_in, fd, 1, 0, &winfo);
 	if (ret)
 		return ret;
 
@@ -1273,6 +1264,7 @@ int main_loop(void)
 			xerror("can't reconnect: %d", errno);
 		if (cfg_input)
 			close(fd_in);
+		memset(&winfo, 0, sizeof(winfo));
 		goto again;
 	} else {
 		close(fd);
-- 
2.38.1
Re: [PATCH net-next] Squash-to: "selftests: mptcp: mptfo Initiator/Listener"
Posted by Dmytro Shytyi 4 weeks, 1 day ago
Hello Paolo,

Thank you very much for investing a time to improve the patch (including 
the selftests)!

I'm very happy to see these commits. I'm glad to see that you pursued 
the method of adding the sendto in the selftest.

Comments are in-line.

  Best,

Dmytro.

On 11/9/2022 3:52 PM, Paolo Abeni wrote:
> The TFO is allowed to send less data than the provided one,
> we need to track more status info to properly allow the next sendmsg()
> starting from the next to send byte.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>   .../selftests/net/mptcp/mptcp_connect.c       | 118 ++++++++----------
>   1 file changed, 55 insertions(+), 63 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 4bc855159c52..134e569cd75a 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -26,13 +26,11 @@
>   
>   #include <netdb.h>
>   #include <netinet/in.h>
> -#include <netinet/tcp.h>
>   
> +#include <linux/tcp.h>
>   #include <linux/time_types.h>
>   #include <linux/sockios.h>
>   
> -
> -
>   extern int optind;
>   
>   #ifndef IPPROTO_MPTCP
> @@ -93,6 +91,12 @@ struct tcp_inq_state {
>   	bool expect_eof;
>   };
>   
> +struct wstate {
> +	char buf[8192];
> +	unsigned int len;
> +	unsigned int off;
> +};
> +
>   static struct tcp_inq_state tcp_inq;
>   
>   static struct cfg_cmsg_types cfg_cmsg_types;
> @@ -239,7 +243,7 @@ static void set_mptfo(int fd, int pf)
>   {
>   	int qlen = 25;
>   
This is a nice replacement. No need modify the "include" anymore :)
> -	if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
> +	if (setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
>   		perror("TCP_FASTOPEN");
>   }
>   
> @@ -345,17 +349,14 @@ static int sock_listen_mptcp(const char * const listenaddr,
>   static int sock_connect_mptcp(const char * const remoteaddr,
>   			      const char * const port, int proto,
>   			      struct addrinfo **peer,
> -			      int infd,
> -			      unsigned int *woff)
> +			      int infd, struct wstate *winfo)
>   {
>   	struct addrinfo hints = {
>   		.ai_protocol = IPPROTO_TCP,
>   		.ai_socktype = SOCK_STREAM,
>   	};
>   	struct addrinfo *a, *addr;
> -	unsigned int wlen = 0;
>   	int syn_copied = 0;
> -	char wbuf[8192];
>   	int sock = -1;
>   
>   	hints.ai_family = pf;
> @@ -374,13 +375,15 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>   			set_mark(sock, cfg_mark);
>   
>   		if (cfg_sockopt_types.mptfo) {

Below I used  (wlen == 0) check to avoid possible multiple times 
execution in the loop of double/triple/..-time offset of read(infd,).

Example: 1st enterance of the loop will read() from infd and set offset 
0+X in infd, thus 2nd enterance of the loop will start reading from X, so

we could loose the previous data  (data between 0 and X) by attempting 
the sendto in a loop.

Should we drop this check or this assumption is not relevant in the 
current context?

> -			if (wlen == 0)
> -				wlen = read(infd, wbuf, sizeof(wbuf));
> +			winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
>   
> -			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
> +			syn_copied = sendto(sock, winfo->buf, winfo->len, MSG_FASTOPEN,
>   					    a->ai_addr, a->ai_addrlen);
> +			if (syn_copied < 0)
> +				perror("sendto");
>   			if (syn_copied) {
> -				*woff = wlen;
> +				winfo->off = syn_copied;
> +				winfo->len -= syn_copied;
>   				*peer = a;
>   				break; /* success */
>   			}
> @@ -610,14 +613,13 @@ static void shut_wr(int fd)
>   }
>   
>   static int copyfd_io_poll(int infd, int peerfd, int outfd,
> -			  bool *in_closed_after_out, unsigned int woff)
> +			  bool *in_closed_after_out, struct wstate *winfo)
>   {
>   	struct pollfd fds = {
>   		.fd = peerfd,
>   		.events = POLLIN | POLLOUT,
>   	};
> -	unsigned int wlen = 0, total_wlen = 0, total_rlen = 0;
> -	char wbuf[8192];
> +	unsigned int total_wlen = 0, total_rlen = 0;
>   
>   	set_nonblock(peerfd, true);
>   
> @@ -677,19 +679,19 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>   		}
>   
>   		if (fds.revents & POLLOUT) {
> -			if (wlen == 0) {
> -				woff = 0;
> -				wlen = read(infd, wbuf, sizeof(wbuf));
> +			if (winfo->len == 0) {
> +				winfo->off = 0;
> +				winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
>   			}
>   
> -			if (wlen > 0) {
> +			if (winfo->len > 0) {
>   				ssize_t bw;
>   
>   				/* limit the total amount of written data to the trunc value */
> -				if (cfg_truncate > 0 && wlen + total_wlen > cfg_truncate)
> -					wlen = cfg_truncate - total_wlen;
> +				if (cfg_truncate > 0 && winfo->len + total_wlen > cfg_truncate)
> +					winfo->len = cfg_truncate - total_wlen;
>   
> -				bw = do_rnd_write(peerfd, wbuf + woff, wlen);
I also attempted to modify like is presented here (below). But in my 
case I observed negative impacting on other tests, so in conclusion, It 
is nice to see that you did it like this :)
> +				bw = do_rnd_write(peerfd, winfo->buf + winfo->off, winfo->len);
>   				if (bw < 0) {
>   					if (cfg_rcv_trunc)
>   						return 0;
> @@ -697,10 +699,10 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd,
>   					return 111;
>   				}
>   
> -				woff += bw;
> -				wlen -= bw;
> +				winfo->off += bw;
> +				winfo->len -= bw;
>   				total_wlen += bw;
> -			} else if (wlen == 0) {
> +			} else if (winfo->len == 0) {
>   				/* We have no more data to send. */
>   				fds.events &= ~POLLOUT;
>   
> @@ -878,7 +880,7 @@ static int copyfd_io_sendfile(int infd, int peerfd, int outfd,
>   	return err;
>   }
>   
> -static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigned int woff)
> +static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, struct wstate *winfo)
>   {
>   	bool in_closed_after_out = false;
>   	struct timespec start, end;
> @@ -890,7 +892,7 @@ static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigne
>   
>   	switch (cfg_mode) {
>   	case CFG_MODE_POLL:
> -		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, woff);
> +		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, winfo);
>   		break;
>   
>   	case CFG_MODE_MMAP:
> @@ -1038,6 +1040,7 @@ static void maybe_close(int fd)
>   int main_loop_s(int listensock)
>   {
>   	struct sockaddr_storage ss;
> +	struct wstate winfo;
>   	struct pollfd polls;
>   	socklen_t salen;
>   	int remotesock;
> @@ -1072,7 +1075,8 @@ int main_loop_s(int listensock)
>   
>   		SOCK_TEST_TCPULP(remotesock, 0);
>   
> -		copyfd_io(fd, remotesock, 1, true, 0);
> +		memset(&winfo, 0, sizeof(winfo));
> +		copyfd_io(fd, remotesock, 1, true, &winfo);
>   	} else {
>   		perror("accept");
>   		return 1;
> @@ -1210,53 +1214,40 @@ void xdisconnect(int fd, int addrlen)
>   
>   int main_loop(void)
>   {
> -	int fd, ret, fd_in = 0;
> +	int fd = 0, ret, fd_in = 0;
>   	struct addrinfo *peer;
> -	unsigned int woff = 0;
> -
> -	if (!cfg_sockopt_types.mptfo) {
> -		/* listener is ready. */
> -		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, 0, 0);
> -		if (fd < 0)
> -			return 2;
> -again:
> -		check_getpeername_connect(fd);
> +	struct wstate winfo;
>   
> -		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> -
> -		if (cfg_rcvbuf)
> -			set_rcvbuf(fd, cfg_rcvbuf);
> -		if (cfg_sndbuf)
> -			set_sndbuf(fd, cfg_sndbuf);
> -		if (cfg_cmsg_types.cmsg_enabled)
> -			apply_cmsg_types(fd, &cfg_cmsg_types);
> -	}
> -
> -	if (cfg_input) {
> +	if (cfg_input && cfg_sockopt_types.mptfo) {
>   		fd_in = open(cfg_input, O_RDONLY);
>   		if (fd < 0)
>   			xerror("can't open %s:%d", cfg_input, errno);
>   	}
>   
> -	if (cfg_sockopt_types.mptfo) {
> -		/* sendto() instead of connect */
> -		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &woff);
> -		if (fd < 0)
> -			return 2;
> +	memset(&winfo, 0, sizeof(winfo));
> +	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &winfo);
> +	if (fd < 0)
> +		return 2;
>   
> -		check_getpeername_connect(fd);
> +again:
> +	check_getpeername_connect(fd);
>   
> -		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> +	SOCK_TEST_TCPULP(fd, cfg_sock_proto);
>   
> -		if (cfg_rcvbuf)
> -			set_rcvbuf(fd, cfg_rcvbuf);
> -		if (cfg_sndbuf)
> -			set_sndbuf(fd, cfg_sndbuf);
> -		if (cfg_cmsg_types.cmsg_enabled)
> -			apply_cmsg_types(fd, &cfg_cmsg_types);
> +	if (cfg_rcvbuf)
> +		set_rcvbuf(fd, cfg_rcvbuf);
> +	if (cfg_sndbuf)
> +		set_sndbuf(fd, cfg_sndbuf);
> +	if (cfg_cmsg_types.cmsg_enabled)
> +		apply_cmsg_types(fd, &cfg_cmsg_types);
> +
> +	if (cfg_input && !cfg_sockopt_types.mptfo) {
> +		fd_in = open(cfg_input, O_RDONLY);
> +		if (fd < 0)
> +			xerror("can't open %s:%d", cfg_input, errno);
>   	}
>   
> -	ret = copyfd_io(fd_in, fd, 1, 0, 0);
> +	ret = copyfd_io(fd_in, fd, 1, 0, &winfo);
>   	if (ret)
>   		return ret;
>   
> @@ -1273,6 +1264,7 @@ int main_loop(void)
>   			xerror("can't reconnect: %d", errno);
>   		if (cfg_input)
>   			close(fd_in);
> +		memset(&winfo, 0, sizeof(winfo));
>   		goto again;
>   	} else {
>   		close(fd);
Re: [PATCH net-next] Squash-to: "selftests: mptcp: mptfo Initiator/Listener"
Posted by Paolo Abeni 4 weeks ago
On Wed, 2022-11-09 at 19:15 +0100, Dmytro Shytyi wrote:
> > @@ -374,13 +375,15 @@ static int sock_connect_mptcp(const char * const remoteaddr,
> >   			set_mark(sock, cfg_mark);
> >   
> >   		if (cfg_sockopt_types.mptfo) {
> 
> Below I used  (wlen == 0) check to avoid possible multiple times 
> execution in the loop of double/triple/..-time offset of read(infd,).
> 
> Example: 1st enterance of the loop will read() from infd and set offset 
> 0+X in infd, thus 2nd enterance of the loop will start reading from X, so
> 
> we could loose the previous data  (data between 0 and X) by attempting 
> the sendto in a loop.
> 
> Should we drop this check or this assumption is not relevant in the 
> current context?

Yup, you are right, we must preserve the len check here and do the read
only when !wbuf->len ...
> 
> > -			if (wlen == 0)
> > -				wlen = read(infd, wbuf, sizeof(wbuf));
> > +			winfo->len = read(infd, winfo->buf, sizeof(winfo->buf));
> >   
> > -			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
> > +			syn_copied = sendto(sock, winfo->buf, winfo->len, MSG_FASTOPEN,
> >   					    a->ai_addr, a->ai_addrlen);
> > +			if (syn_copied < 0)
> > +				perror("sendto");
> >   			if (syn_copied) {

... additionally drop the perror() here (the relevant one is below) and
explicitly check that syn_copied is not an error code:

			if (syn_copied >= 0) {

I would opt for more squash-to patches...

Thanks,

Paolo