[PATCH v2 0/8] net/tcp: Fix several bugs and allow rttcp to be built

[PATCH v2 0/8] net/tcp: Fix several bugs and allow rttcp to be built

[Sebastian Smolorz]

This patch series makes RTnet's tcp module usable again. It was not
working due to several bugs which showed up in combination with reworked
RTDM of Xenomai 3.

Tests on real hardware (little endian) were successful. A test on big
endian is still pending.

Sebastian Smolorz (8):
  net/tcp: return ufd in rt_tcp_accept()
  net/tcp: fix listen() and shutdown() IOCTL calls
  net/tcp: Fix bug when obtaining RST socket's private structure from
    its rtdm_fd pointer
  net/tcp: Copy data back to user buffer in rt_tcp_recvmsg
  net/tcp: fix TCP connection termination
  net/tcp: Set valid flags in struct tcphdr on little and big endian
    machines
  net/tcp: return already copied number of bytes to recv() callers when
    peer socket terminates
  net/tcp: Allow choice of rttcp protocol in Kconfig

 kernel/drivers/net/stack/ipv4/Kconfig   |  2 +-
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 54 ++++++++++++---------------------
 2 files changed, 21 insertions(+), 35 deletions(-)

-- 
2.7.4

[PATCH v2 1/8] net/tcp: return ufd in rt_tcp_accept()

[Sebastian Smolorz]

Fix compilation error in rt_tcp_accept() when returning its socket's own file
descriptor.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index c5e42bc..c713635 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -1668,7 +1668,7 @@ static int rt_tcp_accept(struct tcp_socket *ts, struct sockaddr *addr,
     ts->is_accepted = 1;
     rtdm_lock_put_irqrestore(&ts->socket_lock, context);
 
-    ret = rt_socket_fd(&ts->sock)->fd;
+    ret = rtdm_fd_ufd(rt_socket_fd(&ts->sock));
 
  err:
     /* it is not critical to leave this unlocked
-- 
2.7.4

[PATCH v2 2/8] net/tcp: fix listen() and shutdown() IOCTL calls

[Sebastian Smolorz]

listen() and shutdown() pass their simple int argument directly casted to
void *arg, not as a pointer to their value.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index c713635..8605a8c 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -1790,8 +1790,6 @@ static int rt_tcp_ioctl(struct rtdm_fd *fd,
     struct _rtdm_getsockopt_args _getopt;
     const struct _rtdm_setsockopt_args *setopt;
     struct _rtdm_setsockopt_args _setopt;
-    const long *val;
-    long _val;
     int in_rt;
 
     /* fast path for common socket IOCTLs */
@@ -1815,10 +1813,7 @@ static int rt_tcp_ioctl(struct rtdm_fd *fd,
 		return rt_tcp_connect(ts, setaddr->addr, setaddr->addrlen);
 
 	case _RTIOC_LISTEN:
-		val = rtnet_get_arg(fd, &_val, arg, sizeof(long));
-		if (IS_ERR(val))
-			return PTR_ERR(val);
-		return rt_tcp_listen(ts, *val);
+		return rt_tcp_listen(ts, (unsigned long)arg);
 
 	case _RTIOC_ACCEPT:
 		if (!in_rt)
@@ -1829,10 +1824,7 @@ static int rt_tcp_ioctl(struct rtdm_fd *fd,
 		return rt_tcp_accept(ts, getaddr->addr, getaddr->addrlen);
 
 	case _RTIOC_SHUTDOWN:
-		val = rtnet_get_arg(fd, &_val, arg, sizeof(long));
-		if (IS_ERR(val))
-			return PTR_ERR(val);
-		return rt_tcp_shutdown(ts, *val);
+		return rt_tcp_shutdown(ts, (unsigned long)arg);
 
 	case _RTIOC_SETSOCKOPT:
 		setopt = rtnet_get_arg(fd, &_setopt, arg, sizeof(_setopt));
-- 
2.7.4

[PATCH v2 3/8] net/tcp: Fix bug when obtaining RST socket's private structure from its rtdm_fd pointer

[Sebastian Smolorz]

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index 8605a8c..2678e6a 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -182,7 +182,7 @@ static struct {
 } rst_socket_container;
 
 #define rst_fd		(&rst_socket_container.dummy.fd)
-#define rst_socket	(*(struct tcp_socket *)rtdm_private_to_fd(rst_fd))
+#define rst_socket	(*(struct tcp_socket *)rtdm_fd_to_private(rst_fd))
 
 static u32 tcp_auto_port_start = 1024;
 static u32 tcp_auto_port_mask  = ~(RT_TCP_SOCKETS-1);
-- 
2.7.4

[PATCH v2 4/8] net/tcp: Copy data back to user buffer in rt_tcp_recvmsg

[Sebastian Smolorz]

A bug in rt_tcp_recvmsg() prevented an application to receive data
over an RTTCP socket. Data was not copied back to the application's
buffer but rather into a temporary kernel buffer.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index 2678e6a..3242a08 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -2084,17 +2084,10 @@ static ssize_t rt_tcp_recvmsg(struct rtdm_fd *fd, struct user_msghdr *msg, int m
 
 	len = iov[0].iov_len;
 	if (len > 0) {
-		buf = xnmalloc(len);
-		if (buf == NULL) {
-			ret = -ENOMEM;
-			goto out;
-		}
-		ret = rtdm_copy_from_user(fd, buf, iov[0].iov_base, len);
-		if (!ret)
-			ret = rt_tcp_read(fd, buf, len);
-		xnfree(buf);
+		buf = iov[0].iov_base;
+		ret = rt_tcp_read(fd, buf, len);
 	}
-out:
+
 	rtdm_drop_iovec(iov, iov_fast);
 
 	return ret;
-- 
2.7.4

Re: [PATCH v2 4/8] net/tcp: Copy data back to user buffer in rt_tcp_recvmsg

[Jan Kiszka]

On 06.11.18 11:00, Sebastian Smolorz via Xenomai wrote:
> A bug in rt_tcp_recvmsg() prevented an application to receive data
> over an RTTCP socket. Data was not copied back to the application's
> buffer but rather into a temporary kernel buffer.
> 
> Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
> ---
>  kernel/drivers/net/stack/ipv4/tcp/tcp.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> index 2678e6a..3242a08 100644
> --- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> +++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> @@ -2084,17 +2084,10 @@ static ssize_t rt_tcp_recvmsg(struct rtdm_fd *fd, struct user_msghdr *msg, int m
>  
>  	len = iov[0].iov_len;
>  	if (len > 0) {
> -		buf = xnmalloc(len);
> -		if (buf == NULL) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> -		ret = rtdm_copy_from_user(fd, buf, iov[0].iov_base, len);
> -		if (!ret)
> -			ret = rt_tcp_read(fd, buf, len);
> -		xnfree(buf);
> +		buf = iov[0].iov_base;
> +		ret = rt_tcp_read(fd, buf, len);

Does this go well with smap? IOW: Was everything testing on a system with smap
support available and enabled?

>  	}
> -out:
> +
>  	rtdm_drop_iovec(iov, iov_fast);
>  
>  	return ret;
> 

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2 4/8] net/tcp: Copy data back to user buffer in rt_tcp_recvmsg

[Sebastian Smolorz]

On 06.11.18, 19:52, Jan Kiszka wrote:
> On 06.11.18 11:00, Sebastian Smolorz via Xenomai wrote:
> > A bug in rt_tcp_recvmsg() prevented an application to receive data
> > over an RTTCP socket. Data was not copied back to the application's
> > buffer but rather into a temporary kernel buffer.
> > 
> > Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
> > ---
> > 
> >  kernel/drivers/net/stack/ipv4/tcp/tcp.c | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> > 
> > diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> > b/kernel/drivers/net/stack/ipv4/tcp/tcp.c index 2678e6a..3242a08
> > 100644
> > --- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> > +++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
> > @@ -2084,17 +2084,10 @@ static ssize_t rt_tcp_recvmsg(struct rtdm_fd
> > *fd, struct user_msghdr *msg, int m> 
> >  	len = iov[0].iov_len;
> >  	if (len > 0) {
> > 
> > -		buf = xnmalloc(len);
> > -		if (buf == NULL) {
> > -			ret = -ENOMEM;
> > -			goto out;
> > -		}
> > -		ret = rtdm_copy_from_user(fd, buf, iov[0].iov_base, len);
> > -		if (!ret)
> > -			ret = rt_tcp_read(fd, buf, len);
> > -		xnfree(buf);
> > +		buf = iov[0].iov_base;
> > +		ret = rt_tcp_read(fd, buf, len);
> 
> Does this go well with smap? IOW: Was everything testing on a system
> with smap support available and enabled?

smap was enabled. Tests were executed on an Atom E3845. cat /proc/
cpuinfo showed no smap flag.

> >  	}
> > 
> > -out:
> > +
> > 
> >  	rtdm_drop_iovec(iov, iov_fast);
> >  	
> >  	return ret;

-- 
Sebastian

[PATCH v2 5/8] net/tcp: fix TCP connection termination

[Sebastian Smolorz]

Closing a connected TCP socket initiates a handshake procedure. The
closing socket is supposed to receive two more packets and send one
ACK at the end. RTnet's TCP stack tries to get a reference to a
socket before writing data to its buffer. Taking a reference to an
RTnet socket in Xenomai 3 is realized by calling rtdm_fd_lock() which
fails when a socket is about to be closed.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index 3242a08..c066e06 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -249,15 +249,18 @@ static struct rtsocket *rt_tcp_v4_lookup(u32 daddr, u16 dport)
 {
     rtdm_lockctx_t  context;
     struct tcp_socket *ts;
+    int ret;
 
     rtdm_lock_get_irqsave(&tcp_socket_base_lock, context);
     ts = port_hash_search(daddr, dport);
 
-    if (ts && rt_socket_reference(&ts->sock) == 0) {
-
-	rtdm_lock_put_irqrestore(&tcp_socket_base_lock, context);
+    if (ts != NULL) {
+	ret = rt_socket_reference(&ts->sock);
+	if (ret == 0 || (ret == -EIDRM && ts->is_closed)) {
+		rtdm_lock_put_irqrestore(&tcp_socket_base_lock, context);
 
-	return &ts->sock;
+		return &ts->sock;
+	}
     }
 
     rtdm_lock_put_irqrestore(&tcp_socket_base_lock, context);
-- 
2.7.4

[PATCH v2 6/8] net/tcp: Set valid flags in struct tcphdr on little and big endian machines

[Sebastian Smolorz]

Setting TCP header flags like ACK was fragile and not working when
RTnet was running on big endian processors. Use the macro provided
by the linux kernel for setting the TCP flags instead.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index c066e06..134469c 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -280,12 +280,6 @@ static inline int rt_tcp_after(__u32 seq1, __u32 seq2)
     return (__s32)(seq2-seq1) <= 0;
 }
 
-static inline void rt_tcp_set_flags(struct tcphdr* th, __be32 flags)
-{
-    __be16 *tf = ((__be16*)th) + 6;
-    *tf = flags;
-}
-
 static inline u32 rt_tcp_compute_ack_seq(struct tcphdr* th, u32 len)
 {
     u32 ack_seq = ntohl(th->seq) + len;
@@ -625,11 +619,10 @@ static void rt_tcp_build_header(struct tcp_socket *ts, struct rtskb *skb,
     if (unlikely(is_keepalive))
 	th->seq--;
 
+    tcp_flag_word(th) = flags;
     th->ack_seq = htonl(ts->sync.ack_seq);
     th->window  = htons(ts->sync.window);
 
-    rt_tcp_set_flags(th, flags);
-
     th->doff = tcphdrlen >> 2; /* No options for now */
     th->res1 = 0;
     th->check   = 0;
-- 
2.7.4

[PATCH v2 7/8] net/tcp: return already copied number of bytes to recv() callers when peer socket terminates

[Sebastian Smolorz]

In the following situation a recv() call to a RT-TCP socket returned 0
although there was data already copied to the caller's buffer:

- S1 connects to S2
- S1 calls recv() with buffer size b1
- S2 sends data of size b2 < b1
- S1 receives the data, copies it to its buffer and waits for more data
- S2 terminates the connection
- S1 receives the FIN packet and also starts its shutdown procedure
     which includes returning from the recv() call. But instead of
     returning the number of bytes which were already copied
     it returned 0.

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/drivers/net/stack/ipv4/tcp/tcp.c b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
index 134469c..828f7d1 100644
--- a/kernel/drivers/net/stack/ipv4/tcp/tcp.c
+++ b/kernel/drivers/net/stack/ipv4/tcp/tcp.c
@@ -1909,6 +1909,11 @@ static ssize_t rt_tcp_read(struct rtdm_fd *fd, void *buf, size_t nbyte)
 		return (copied ? copied : ret);
 
 	    case -EIDRM: /* event is destroyed */
+		if (ts->is_closed)
+			return -EBADF;
+
+		return copied;
+
 	    default:
 		if (ts->is_closed) {
 		    return -EBADF;
-- 
2.7.4

[PATCH v2 8/8] net/tcp: Allow choice of rttcp protocol in Kconfig

[Sebastian Smolorz]

Signed-off-by: Sebastian Smolorz <sebastian.smolorz@gmx.de>
---
 kernel/drivers/net/stack/ipv4/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/drivers/net/stack/ipv4/Kconfig b/kernel/drivers/net/stack/ipv4/Kconfig
index fef7e19..8fb3c1a 100644
--- a/kernel/drivers/net/stack/ipv4/Kconfig
+++ b/kernel/drivers/net/stack/ipv4/Kconfig
@@ -72,4 +72,4 @@ config XENO_DRIVERS_NET_RTIPV4_DEBUG
     may want to turn this on for tracing issues in packet delivery.
 
 source "drivers/xenomai/net/stack/ipv4/udp/Kconfig"
-# source "drivers/xenomai/net/stack/ipv4/tcp/Kconfig"
+source "drivers/xenomai/net/stack/ipv4/tcp/Kconfig"
-- 
2.7.4

Re: [PATCH v2 0/8] net/tcp: Fix several bugs and allow rttcp to be built

[Jan Kiszka]

On 06.11.18 11:00, Sebastian Smolorz via Xenomai wrote:
> This patch series makes RTnet's tcp module usable again. It was not
> working due to several bugs which showed up in combination with reworked
> RTDM of Xenomai 3.
> 
> Tests on real hardware (little endian) were successful. A test on big
> endian is still pending.
> 
> Sebastian Smolorz (8):
>    net/tcp: return ufd in rt_tcp_accept()
>    net/tcp: fix listen() and shutdown() IOCTL calls
>    net/tcp: Fix bug when obtaining RST socket's private structure from
>      its rtdm_fd pointer
>    net/tcp: Copy data back to user buffer in rt_tcp_recvmsg
>    net/tcp: fix TCP connection termination
>    net/tcp: Set valid flags in struct tcphdr on little and big endian
>      machines
>    net/tcp: return already copied number of bytes to recv() callers when
>      peer socket terminates
>    net/tcp: Allow choice of rttcp protocol in Kconfig
> 
>   kernel/drivers/net/stack/ipv4/Kconfig   |  2 +-
>   kernel/drivers/net/stack/ipv4/tcp/tcp.c | 54 ++++++++++++---------------------
>   2 files changed, 21 insertions(+), 35 deletions(-)
> 

Thanks, applied to next.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux