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

[Xenomai] [PATCH 0/4] 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
selectable in Kconfig for a long time. Now it is supposed to work in
the way it worked when RTnet was a separate project for Xenomai 2.

Sebastian Smolorz (4):
  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: Allow choice of rttcp protocol in Kconfig

 kernel/drivers/net/stack/ipv4/Kconfig   |  2 +-
 kernel/drivers/net/stack/ipv4/tcp/tcp.c | 16 ++++------------
 2 files changed, 5 insertions(+), 13 deletions(-)

-- 
2.7.4

[Xenomai] [PATCH 1/4] 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

[Xenomai] [PATCH 2/4] 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

[Xenomai] [PATCH 3/4] 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

[Xenomai] [PATCH 4/4] 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: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

[Jan Kiszka]

On 17.10.18 13:31, Sebastian Smolorz wrote:
> This patch series makes RTnet's tcp module usable again. It was not
> selectable in Kconfig for a long time. Now it is supposed to work in
> the way it worked when RTnet was a separate project for Xenomai 2.

Despite the "supposed", can I assume you tested it already successfully with 
some real workload?

> 
> Sebastian Smolorz (4):
>    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: Allow choice of rttcp protocol in Kconfig
> 
>   kernel/drivers/net/stack/ipv4/Kconfig   |  2 +-
>   kernel/drivers/net/stack/ipv4/tcp/tcp.c | 16 ++++------------
>   2 files changed, 5 insertions(+), 13 deletions(-)
> 

Thanks for reanimating this!

Jan

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

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

[Sebastian Smolorz]

On 17.10.18 13:44, Jan Kiszka wrote:
> On 17.10.18 13:31, Sebastian Smolorz wrote:
> > This patch series makes RTnet's tcp module usable again. It was not
> > selectable in Kconfig for a long time. Now it is supposed to work in
> > the way it worked when RTnet was a separate project for Xenomai 2.
> 
> Despite the "supposed", can I assume you tested it already
> successfully with some real workload?

Tested it under QEMU with the rttcp example (rttcp-server and rttcp-
client) over rtlo. Tests on real hardware are pending and planned in the 
next few days. No problem if you want to postpone the patches until I 
report the successful outcome of those tests.

-- 
Sebastian

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

[Pham, Phong]

Hi Sebastian,

I verified your changes on a real hardware, Xenomai stack + network drivers.  rt_dev_connect  returns 0 when attempting to connect to my FTP server 204.75.16.51; however, there is no packet sent on the network.  Below is my routing table.

# ./rtroute
Host Routing Table
Hash    Destination     HW Address              Device
01      192.168.7.1     A8:9D:21:34:8C:65       rteth0
3F      192.168.7.255   FF:FF:FF:FF:FF:FF       rteth0

Network Routing Table
Hash    Destination     Mask                    Gateway
10      204.75.16.51    255.255.255.255         192.168.7.1

Another observation is if I setup my local FTP server (192.168.7.152) on the same LAN (my system has IP 192.168.7.79)
# ./rtroute
Host Routing Table
Hash    Destination     HW Address              Device
18      192.168.7.152   50:7B:9D:2D:95:01       rteth0
3F      192.168.7.255   FF:FF:FF:FF:FF:FF       rteth0

Network Routing Table
Hash    Destination     Mask                    Gateway
10      204.75.16.51    255.255.255.255         192.168.7.1

I do see a TCP packet sent out on the LAN when rt_dev_connect() is invoked; however, the packet itself is not valid.  The SYN flag in the TCP protocol is not set when the first TCP packet was sent.

Could you look into these issues where at least one box talking to another over a LAN?  One best ex. is a socket programming exercise downloading a simple file using FTP protocol?  (no need for bells & whistle of FTP completeness; just simple download).  Also let me know if I can help you in anyway since I have the hw and my Ethernet driver is functional (able to ping).

Thanks,
Phong.

-----Original Message-----
From: Xenomai [mailto:xenomai-bounces@xenomai.org] On Behalf Of Sebastian Smolorz
Sent: Wednesday, October 17, 2018 9:10 AM
To: Jan Kiszka
Cc: xenomai@xenomai.org
Subject: Re: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

On 17.10.18 13:44, Jan Kiszka wrote:
> On 17.10.18 13:31, Sebastian Smolorz wrote:
> > This patch series makes RTnet's tcp module usable again. It was not
> > selectable in Kconfig for a long time. Now it is supposed to work in
> > the way it worked when RTnet was a separate project for Xenomai 2.
>
> Despite the "supposed", can I assume you tested it already
> successfully with some real workload?

Tested it under QEMU with the rttcp example (rttcp-server and rttcp-
client) over rtlo. Tests on real hardware are pending and planned in the
next few days. No problem if you want to postpone the patches until I
report the successful outcome of those tests.

--
Sebastian



_______________________________________________
Xenomai mailing list
Xenomai@xenomai.org
https://xenomai.org/mailman/listinfo/xenomai
Notice: This e-mail and any files transmitted with it may contain Data Device Corporation's and its subsidiaries privileged and proprietary information. It is intended solely for the use of the individual or entity to whom it is addressed. If you are not the named recipient of this transmission, any disclosure, copying, distribution or reliance on the contents of this message is prohibited. If you received this e-mail in error, please destroy it and any attached files and notify me immediately.

Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view our privacy statement.

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

[Sebastian Smolorz]

Hi,

on 10/19/18 04:00 AM, Pham, Phong wrote:
> I do see a TCP packet sent out on the LAN when rt_dev_connect() is
> invoked; however, the packet itself is not valid.  The SYN flag in
> the TCP protocol is not set when the first TCP packet was sent.

Strange. Wireshark shows me that a connection initiated by a RTnet box 
can be successfully established and that the SYN flag indeed is set.

-- 
Sebastian

RE: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

[Pham, Phong]

Hi Sebastian,

This is what I see when my RTnet box, my laptop, and my FTP server (local on laptop) is on the same subnet 192.168.7.0:

192.168.7.79 is my RTnet box.  I used my laptop (192.168.7.106) to ping the RTnet box which is what you see with ICMP protocol.  The first TCP packet sent by my RTnet box (packet # 11) has the SYN bit not set.  (see attached image.  I pulled in your patch along with the patch named "rtdm: add integer file descriptor to struct rtdm_fd" to get TCP to compile.  Any advice you may have?  Do you know which relevant routine goes and set the TCP SYN bit that I can start looking?

Phong.


-----Original Message-----
From: Sebastian Smolorz [mailto:Sebastian.Smolorz@gmx.de]
Sent: Tuesday, October 30, 2018 1:17 PM
To: Pham, Phong
Cc: Jan Kiszka; xenomai@xenomai.org; Hillman, Robert
Subject: Re: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

Hi,

on 10/19/18 04:00 AM, Pham, Phong wrote:
> I do see a TCP packet sent out on the LAN when rt_dev_connect() is
> invoked; however, the packet itself is not valid.  The SYN flag in
> the TCP protocol is not set when the first TCP packet was sent.

Strange. Wireshark shows me that a connection initiated by a RTnet box
can be successfully established and that the SYN flag indeed is set.

--
Sebastian



Notice: This e-mail and any files transmitted with it may contain Data Device Corporation's and its subsidiaries privileged and proprietary information. It is intended solely for the use of the individual or entity to whom it is addressed. If you are not the named recipient of this transmission, any disclosure, copying, distribution or reliance on the contents of this message is prohibited. If you received this e-mail in error, please destroy it and any attached files and notify me immediately.

Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view our privacy statement.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wireshark_capture.png
Type: image/png
Size: 135068 bytes
Desc: wireshark_capture.png
URL: <http://xenomai.org/pipermail/xenomai/attachments/20181030/3faf5119/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wireshark_capture.png
Type: image/png
Size: 135068 bytes
Desc: wireshark_capture.png
URL: <http://xenomai.org/pipermail/xenomai/attachments/20181030/3faf5119/attachment-0001.png>

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

[Sebastian Smolorz]

Pham, Phong wrote:
> This is what I see when my RTnet box, my laptop, and my FTP server
> (local on laptop) is on the same subnet 192.168.7.0:
> 
> 192.168.7.79 is my RTnet box.

Which processor architecture is it?

> I used my laptop (192.168.7.106) to
> ping the RTnet box which is what you see with ICMP protocol.  The
> first TCP packet sent by my RTnet box (packet # 11) has the SYN bit
> not set.  (see attached image.  I pulled in your patch along with the
> patch named "rtdm: add integer file descriptor to struct rtdm_fd" to
> get TCP to compile.

So you don't use latest next? Do you use stable? Did you made changes to 
the source code we are not aware of?

> Any advice you may have?  Do you know which
> relevant routine goes and set the TCP SYN bit that I can start
> looking?

It is set in rt_tcp_connect() line 1536:

ret = rt_tcp_send(ts, TCP_FLAG_SYN);

in kernel/drivers/net/stack/ipv4/tcp/tcp.c

-- 
Sebastian

RE: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

[Pham, Phong]

Hi Sebastian,

The issue has to do with endianness.  We are running PowerPC (big endian).  So in rt_tcp_set_flags(), flags is 32-bit wide and *tf is 16-bit wide.  I modified the line
from:

*tf = flags;

to

*tf = *(__be16 *)&flags;

and that does the job.

Now I encounter a different problem,

recv() returns 0 bytes but I know my FTP server sends a packet (seen by WireShark).  Can you point me where is the recv() implemented in tcp.c?  Is it rt_tcp_rcv or rt_tcp_recvmsg?

Phong.


-----Original Message-----
From: Sebastian Smolorz [mailto:Sebastian.Smolorz@gmx.de]
Sent: Wednesday, October 31, 2018 7:08 AM
To: Pham, Phong
Cc: Jan Kiszka; xenomai@xenomai.org; Hillman, Robert
Subject: Re: [Xenomai] [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

Pham, Phong wrote:
> This is what I see when my RTnet box, my laptop, and my FTP server
> (local on laptop) is on the same subnet 192.168.7.0:
>
> 192.168.7.79 is my RTnet box.

Which processor architecture is it?

> I used my laptop (192.168.7.106) to
> ping the RTnet box which is what you see with ICMP protocol.  The
> first TCP packet sent by my RTnet box (packet # 11) has the SYN bit
> not set.  (see attached image.  I pulled in your patch along with the
> patch named "rtdm: add integer file descriptor to struct rtdm_fd" to
> get TCP to compile.

So you don't use latest next? Do you use stable? Did you made changes to
the source code we are not aware of?

> Any advice you may have?  Do you know which
> relevant routine goes and set the TCP SYN bit that I can start
> looking?

It is set in rt_tcp_connect() line 1536:

ret = rt_tcp_send(ts, TCP_FLAG_SYN);

in kernel/drivers/net/stack/ipv4/tcp/tcp.c

--
Sebastian



Notice: This e-mail and any files transmitted with it may contain Data Device Corporation's and its subsidiaries privileged and proprietary information. It is intended solely for the use of the individual or entity to whom it is addressed. If you are not the named recipient of this transmission, any disclosure, copying, distribution or reliance on the contents of this message is prohibited. If you received this e-mail in error, please destroy it and any attached files and notify me immediately.

Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view our privacy statement.

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

[Jan Kiszka]

On 31.10.18 17:55, Pham, Phong wrote:
> 
> Hi Sebastian,
> 
> The issue has to do with endianness.  We are running PowerPC (big endian).  So in rt_tcp_set_flags(), flags is 32-bit wide and *tf is 16-bit wide.  I modified the line
> from:
> 
> *tf = flags;
> 
> to
> 
> *tf = *(__be16 *)&flags;

Something is fishy with rt_tcp_set_flags. Shouldn't we rather ensure that only a 
__be16 flag value is passed to it?

> 
> and that does the job.
> 
> Now I encounter a different problem,
> 
> recv() returns 0 bytes but I know my FTP server sends a packet (seen by WireShark).  Can you point me where is the recv() implemented in tcp.c?  Is it rt_tcp_rcv or rt_tcp_recvmsg?

The incoming packet is forwarded to rt_tcp_rcv. rt_tcp_recvmsg is called when an 
application does some recvmsg-like call.


You are probably the first one to try the tcp implementation on big endian. IOW: 
There might be more endianness issues ahead.

Jan

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

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

[Sebastian Smolorz]

Pham, Phong wrote:
> Now I encounter a different problem,
> 
> recv() returns 0 bytes but I know my FTP server sends a packet (seen
> by WireShark).  Can you point me where is the recv() implemented in
> tcp.c?  Is it rt_tcp_rcv or rt_tcp_recvmsg?

There are indeed two more issues with the rttcp driver: one in the
receive path, one in the close path. I have prepared patches for both.
Could you give them a try? I will include them in the v2 patch series
for the rttcp driver.

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;

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);

-- 
Sebastian

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

[Sebastian Smolorz]

On 31.10.18 19:18, Jan Kiszka wrote:
> On 31.10.18 17:55, Pham, Phong wrote:
> > Hi Sebastian,
> > 
> > The issue has to do with endianness.  We are running PowerPC (big
> > endian).  So in rt_tcp_set_flags(), flags is 32-bit wide and *tf is
> > 16-bit wide.  I modified the line from:
> > 
> > *tf = flags;
> > 
> > to
> > 
> > *tf = *(__be16 *)&flags;
> 
> Something is fishy with rt_tcp_set_flags. Shouldn't we rather ensure
> that only a __be16 flag value is passed to it?

I think the reason why flags are passed as __be32 is that they are defined 
with the help of __constant_cpu_to_be32().

After reading some TCP related code in the kernel I think that this 
issue is properly fixed by

tcp_flag_word(th) = flags;

I will test this for little endian. Phong, can you test this on your 
machine?

-- 
Sebastian

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

[Sebastian Smolorz]

Sebastian Smolorz wrote:
> On 31.10.18 19:18, Jan Kiszka wrote:
> > On 31.10.18 17:55, Pham, Phong wrote:
> > > Hi Sebastian,
> > > 
> > > The issue has to do with endianness.  We are running PowerPC (big
> > > endian).  So in rt_tcp_set_flags(), flags is 32-bit wide and *tf
> > > is
> > > 16-bit wide.  I modified the line from:
> > > 
> > > *tf = flags;
> > > 
> > > to
> > > 
> > > *tf = *(__be16 *)&flags;
> > 
> > Something is fishy with rt_tcp_set_flags. Shouldn't we rather ensure
> > that only a __be16 flag value is passed to it?
> 
> I think the reason why flags are passed as __be32 is that they are
> defined with the help of __constant_cpu_to_be32().
> 
> After reading some TCP related code in the kernel I think that this
> issue is properly fixed by
> 
> tcp_flag_word(th) = flags;
> 
> I will test this for little endian. Phong, can you test this on your
> machine?

The following patch works for me on little endian:

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;

-- 
Sebastian

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

[Pham, Phong]

Hi Sebastian,

" Phong, can you test this on your machine?"
No the fix does not work.  It overwrites the window field.

Phong.


-----Original Message-----
From: Sebastian Smolorz [mailto:Sebastian.Smolorz@gmx.de]
Sent: Friday, November 02, 2018 7:42 AM
To: Jan Kiszka
Cc: Pham, Phong; Hillman, Robert; xenomai@xenomai.org
Subject: Re: [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

On 31.10.18 19:18, Jan Kiszka wrote:
> On 31.10.18 17:55, Pham, Phong wrote:
> > Hi Sebastian,
> >
> > The issue has to do with endianness.  We are running PowerPC (big
> > endian).  So in rt_tcp_set_flags(), flags is 32-bit wide and *tf is
> > 16-bit wide.  I modified the line from:
> >
> > *tf = flags;
> >
> > to
> >
> > *tf = *(__be16 *)&flags;
>
> Something is fishy with rt_tcp_set_flags. Shouldn't we rather ensure
> that only a __be16 flag value is passed to it?

I think the reason why flags are passed as __be32 is that they are defined
with the help of __constant_cpu_to_be32().

After reading some TCP related code in the kernel I think that this
issue is properly fixed by

tcp_flag_word(th) = flags;

I will test this for little endian. Phong, can you test this on your
machine?

--
Sebastian



Notice: This e-mail and any files transmitted with it may contain Data Device Corporation's and its subsidiaries privileged and proprietary information. It is intended solely for the use of the individual or entity to whom it is addressed. If you are not the named recipient of this transmission, any disclosure, copying, distribution or reliance on the contents of this message is prohibited. If you received this e-mail in error, please destroy it and any attached files and notify me immediately.

Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view our privacy statement.

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

[Sebastian Smolorz]

Hi,

Pham, Phong wrote:
> Hi Sebastian,
> 
> " Phong, can you test this on your machine?"
> No the fix does not work.  It overwrites the window field.

Did you try the patch I sent here?

https://www.xenomai.org/pipermail/xenomai/2018-November/039831.html

-- 
Sebastian

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

[Sebastian Smolorz]

Hi Phong,

Sebastian Smolorz wrote:
> Pham, Phong wrote:
> > Hi Sebastian,
> >
> > " Phong, can you test this on your machine?"
> > No the fix does not work.  It overwrites the window field.
>
> Did you try the patch I sent here?
>
> https://www.xenomai.org/pipermail/xenomai/2018-November/039831.html

I have sent out a new patch series. You can fetch it and apply
it on top of master. This should also fix your problem of
receiving zero bytes although there was data sent.

--
Sebastian

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

[Pham, Phong]

Hi Sebastian,

Your patch (fixing TCP flags endianness issue) does work.  (I was under the impression to use tcp_flag_word and fix it in rt_tcp_set_flags() which I reported doesn't work earlier.)

Phong.

-----Original Message-----
From: Sebastian Smolorz [mailto:Sebastian.Smolorz@gmx.de]
Sent: Monday, November 05, 2018 10:38 AM
To: Pham, Phong
Cc: Jan Kiszka; Hillman, Robert; xenomai@xenomai.org
Subject: Re: [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to be built

Hi,

Pham, Phong wrote:
> Hi Sebastian,
>
> " Phong, can you test this on your machine?"
> No the fix does not work.  It overwrites the window field.

Did you try the patch I sent here?

https://www.xenomai.org/pipermail/xenomai/2018-November/039831.html

--
Sebastian
Notice: This e-mail and any files transmitted with it may contain Data Device Corporation's and its subsidiaries privileged and proprietary information. It is intended solely for the use of the individual or entity to whom it is addressed. If you are not the named recipient of this transmission, any disclosure, copying, distribution or reliance on the contents of this message is prohibited. If you received this e-mail in error, please destroy it and any attached files and notify me immediately.

Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view our privacy statement.

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

[Sebastian Smolorz]

Hi Phong,

Pham, Phong wrote:
> Hi Sebastian,
>
> Your patch (fixing TCP flags endianness issue) does work.

Great, thanks for testing.

--
Sebastian

> (I was
> under the impression to use tcp_flag_word and fix it in
> rt_tcp_set_flags() which I reported doesn't work earlier.)

> Phong.
>
> -----Original Message-----
> From: Sebastian Smolorz [mailto:Sebastian.Smolorz@gmx.de]
> Sent: Monday, November 05, 2018 10:38 AM
> To: Pham, Phong
> Cc: Jan Kiszka; Hillman, Robert; xenomai@xenomai.org
> Subject: Re: [PATCH 0/4] net/tcp: Fix several bugs and allow rttcp to
> be built

> Hi,
>
> Pham, Phong wrote:
>
> > Hi Sebastian,
> >
> >
> >
> > " Phong, can you test this on your machine?"
> > No the fix does not work.  It overwrites the window field.
>
>
> Did you try the patch I sent here?
>
> https://www.xenomai.org/pipermail/xenomai/2018-November/039831.html
>
> --
> Sebastian
> Notice: This e-mail and any files transmitted with it may contain Data
> Device Corporation's and its subsidiaries privileged and proprietary
> information. It is intended solely for the use of the individual or
> entity to whom it is addressed. If you are not the named recipient of
> this transmission, any disclosure, copying, distribution or reliance
> on the contents of this message is prohibited. If you received this
> e-mail in error, please destroy it and any attached files and notify
> me immediately.

> Click Here<http://www.ddc-web.com/Profile/EmailPrivacy.aspx> to view
> our privacy statement.