Skip to content

gnrc_tcp: rewrite passive open #16459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

brummer-simon
Copy link
Member

@brummer-simon brummer-simon commented May 8, 2021

Contribution description

This PR rewrites the passive connection establishment mechanism of GNRC_TCP.
The new version aligns the GNRC_TCP interface with the SOCK_TCP interface with the goal to make a upcoming SOCK integration straight forward and trivial.

Since this is a large change, I've split the PR into three commits (api, implementation and tests) to help reviewing this.
The new tests are written in different style than existing tests, please ignore the difference in style for now, I plan to align the existing test structure in a dedicated PR on after merging this.

Testing procedure

This PR is covered by the GNRC_TCP test suite.
To test this PR follow the instructions under tests/gnrc_tcp/Readme.md

Issues/PRs references

Fixes #10664 - TCP Sockets can not be used / built

@benpicco benpicco added Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels May 8, 2021
@jeandudey jeandudey added this to the Release 2021.07 milestone May 12, 2021
@miri64 miri64 added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label May 17, 2021
@MrKevinWeiss
Copy link
Contributor

Should we move this to next release as it is soft feature freeze and this contains an API change?

@brummer-simon
Copy link
Member Author

This PR and the followup PR contain large API changes. Although the changes are tested in depth, it might be much effort to review all of this. I personally don't care if this is part of this release or the next. From my side, do whatever relaxes your release management. ;)

@MrKevinWeiss MrKevinWeiss added the Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. label Jun 24, 2021
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good at a cursory glance

@brummer-simon brummer-simon force-pushed the gnrc_tcp-rewrite_passive_open branch from 7dc3013 to e74e8b7 Compare July 2, 2021 08:00
@github-actions github-actions bot added Area: doc Area: Documentation Area: sys Area: System Area: tests Area: tests and testing framework labels Jul 2, 2021
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Process: blocked by feature freeze Integration Process: The impact of this PR is too high for merging it during soft feature freeze. labels Jul 4, 2021
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, Murdock should ensure that the tests are still passing on native.

@miri64
Copy link
Member

miri64 commented Jul 4, 2021

Looks good to me, Murdock should ensure that the tests are still passing on native.

Given that the tests require root it probably doesn't ;-).

@benpicco
Copy link
Contributor

benpicco commented Jul 4, 2021

True - but Murdock found that compile/fuzzing/gnrc_tcp does not compile anymore now.

@brummer-simon
Copy link
Member Author

True - but Murdock found that compile/fuzzing/gnrc_tcp does not compile anymore now.

Should be fixed by now

@miri64
Copy link
Member

miri64 commented Jul 5, 2021

Nevertheless the test-as-root target should be run by a maintainer on a handful of platforms locally before merging ;-). If I find the time I will do :-).

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I looked at the code and found a problem 😅 .

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, why my review comment did not get posted though.

@miri64
Copy link
Member

miri64 commented Jul 6, 2021

Mh test works on native, but not on samr21-xpro, which isn't the case on master :-/. I'll post the relevant output with echo activated. Maybe you have an idea.

@miri64
Copy link
Member

miri64 commented Jul 6, 2021

Here it is
01-conn_lifecycle_as_client.py: success

02-conn_lifecycle_as_server.py: success

03-send_data.py: success

04-receive_data.py: success



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:36511
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:36511
- test_short_payload gnrc_tcp_open_passive: returns 0
gnrc_tcp_recv 1000000 1
> gnrc_tcp_recv: argc=3, argv[0] = gnrc_tcp_recv, argv[1] = 1000000, argv[2] = 1
gnrc_tcp_recv: received 1
pktbuf
> packet buffer: first byte: 0x20002690, last byte: 0x20003e90 (size: 6144)
  position of last byte used: 392
~ unused: 0x20002690 (next: (nil), size: 6144) ~

gnrc_tcp_close



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:44364
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:44364
- test_short_header gnrc_tcp_open_passive: returns 0
pktbuf
> packet buffer: first byte: 0x20002690, last byte: 0x20003e90 (size: 6144)
  position of last byte used: 392
~ unused: 0x20002690 (next: (nil), size: 6144) ~

gnrc_tcp_close



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:38905
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:38905
- test_send_ack_instead_of_syn gnrc_tcp_close

Traceback (most recent call last):
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 154, in <module>
    res = run(test, timeout=10, echo=True)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/dist/pythonlibs/testrunner/__init__.py", line 30, in run
    testfunc(child)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 45, in runner
    func(child, tap, host_ll, dst_if, dst_l2, dst_ll, port)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 124, in test_send_ack_instead_of_syn
    sock.connect(addr_info[0][-1])
socket.timeout: timed out
make: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/makefiles/tests/tests.inc.mk:35: test-as-root] Error 1

@brummer-simon
Copy link
Member Author

Here it is

01-conn_lifecycle_as_client.py: success

02-conn_lifecycle_as_server.py: success

03-send_data.py: success

04-receive_data.py: success



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:36511
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:36511
- test_short_payload gnrc_tcp_open_passive: returns 0
gnrc_tcp_recv 1000000 1
> gnrc_tcp_recv: argc=3, argv[0] = gnrc_tcp_recv, argv[1] = 1000000, argv[2] = 1
gnrc_tcp_recv: received 1
pktbuf
> packet buffer: first byte: 0x20002690, last byte: 0x20003e90 (size: 6144)
  position of last byte used: 392
~ unused: 0x20002690 (next: (nil), size: 6144) ~

gnrc_tcp_close



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:44364
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:44364
- test_short_header gnrc_tcp_open_passive: returns 0
pktbuf
> packet buffer: first byte: 0x20002690, last byte: 0x20003e90 (size: 6144)
  position of last byte used: 392
~ unused: 0x20002690 (next: (nil), size: 6144) ~

gnrc_tcp_close



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_open_passive [::]:38905
> gnrc_tcp_open_passive: argc=2, argv[0] = gnrc_tcp_open_passive, argv[1] = [::]:38905
- test_send_ack_instead_of_syn gnrc_tcp_close

Traceback (most recent call last):
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 154, in <module>
    res = run(test, timeout=10, echo=True)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/dist/pythonlibs/testrunner/__init__.py", line 30, in run
    testfunc(child)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 45, in runner
    func(child, tap, host_ll, dst_if, dst_l2, dst_ll, port)
  File "/home/mlenders/Repositories/RIOT-OS/RIOT/tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py", line 124, in test_send_ack_instead_of_syn
    sock.connect(addr_info[0][-1])
socket.timeout: timed out
make: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/makefiles/tests/tests.inc.mk:35: test-as-root] Error 1

The log seems strange to me since it sends gnrc_tcp_open_passive to the target, a function that should not exist anymore. Could you perform a clean build and test again?

@miri64
Copy link
Member

miri64 commented Jul 6, 2021

I don't see how a clean build has any effect on the test script ;-).

@miri64
Copy link
Member

miri64 commented Jul 6, 2021

But did just that and same result.

@miri64
Copy link
Member

miri64 commented Jul 6, 2021

Ah wait, the output indeed differs

01-conn_lifecycle_as_client.py: success


- Test "test_lifecycle_as_server": SUCCESS
02-conn_lifecycle_as_server.py: success

03-send_data.py: success

04-receive_data.py: success



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_listen [::]:20132
gnrc_tcp_tcb_init: returns 0
> gnrc_tcp_listen: argc=2, argv[0] = gnrc_tcp_listen, argv[1] = [::]:20132
gnrc_tcp_accept 2000
- test_short_payload gnrc_tcp_listen: returns 0
> gnrc_tcp_accept: argc=2, argv[0] = gnrc_tcp_accept, argv[1] = 2000
gnrc_tcp_accept: returns 0
gnrc_tcp_recv 1000000 1
> gnrc_tcp_recv: argc=3, argv[0] = gnrc_tcp_recv, argv[1] = 1000000, argv[2] = 1
gnrc_tcp_recv: received 1
pktbuf
> packet buffer: first byte: 0x200026b4, last byte: 0x20003eb4 (size: 6144)
  position of last byte used: 392
~ unused: 0x200026b4 (next: (nil), size: 6144) ~

gnrc_tcp_close
gnrc_tcp_stop_listen



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_listen [::]:41242
gnrc_tcp_tcb_init: returns 0
> gnrc_tcp_listen: argc=2, argv[0] = gnrc_tcp_listen, argv[1] = [::]:41242
gnrc_tcp_accept 2000
- test_short_header gnrc_tcp_listen: returns 0
> gnrc_tcp_accept: argc=2, argv[0] = gnrc_tcp_accept, argv[1] = 2000
gnrc_tcp_accept: returns 0
pktbuf
> packet buffer: first byte: 0x200026b4, last byte: 0x20003eb4 (size: 6144)
  position of last byte used: 392
~ unused: 0x200026b4 (next: (nil), size: 6144) ~

gnrc_tcp_close
gnrc_tcp_stop_listen



make[2]: Nothing to be done for 'all'.
sudo /home/mlenders/Repositories/RIOT-OS/RIOT/dist/tools/ethos/ethos tap0 /dev/ttyACM0 115200 
----> ethos: sending hello.
----> ethos: activating serial pass through.
----> ethos: hello reply received
ifconfig
> > Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
ifconfig
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> ifconfig
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init
Iface  4  HWaddr: 66:DA:1D:63:7D:CF 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::64da:1dff:fe63:7dcf  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff63:7dcf
          
> gnrc_tcp_tcb_init: argc=1, argv[0] = gnrc_tcp_tcb_init
gnrc_tcp_listen [::]:51203
gnrc_tcp_tcb_init: returns 0
> gnrc_tcp_listen: argc=2, argv[0] = gnrc_tcp_listen, argv[1] = [::]:51203
gnrc_tcp_accept 2000
- test_send_ack_instead_of_syn gnrc_tcp_listen: returns 0
> gnrc_tcp_accept: argc=2, argv[0] = gnrc_tcp_accept, argv[1] = 2000
gnrc_tcp_accept: returns -ETIMEDOUT
> gnrc_tcp_close
gnrc_tcp_stop_listen
Timeout in expect script at "child.expect_exact('gnrc_tcp_accept: returns 0')" (tests/gnrc_tcp/tests-as-root/05-garbage-pkts.py:128)

@brummer-simon
Copy link
Member Author

Seems like a timeout thats large enough on native. I'll see if I can find a board here and try to determine a suitable value

@brummer-simon
Copy link
Member Author

@miri64 - I tested the PR on the nucleo-f401re and it worked. I added a commit increasing the accept timeout in the failing test script from 2000ms to 4000ms, I hope this is long enough for all boards with less processing power.

The failing test is the one where 1000 malformed packets are send and discarded, afterwards a valid packet is sent and used to accept the connection. In that test processing power might be the limiting factor. Could you test again?

@miri64 miri64 added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Discussion: needs consensus Marks a discussion that needs a (not necessarily moderated) consensus and removed Discussion: needs consensus Marks a discussion that needs a (not necessarily moderated) consensus labels Jul 7, 2021
@miri64
Copy link
Member

miri64 commented Jul 7, 2021

Nope. But 10000 works. How about, just to be on the save side to set it to 15000? Also if that is done, please squash the commits reflecting the review history.

@brummer-simon brummer-simon force-pushed the gnrc_tcp-rewrite_passive_open branch from 5e426bf to 0d5db28 Compare July 7, 2021 17:44
@brummer-simon
Copy link
Member Author

Nope. But 10000 works. How about, just to be on the save side to set it to 15000? Also if that is done, please squash the commits reflecting the review history.

Done

@miri64
Copy link
Member

miri64 commented Jul 7, 2021

0d5db28 still needs to be squashed in the original commits.

@brummer-simon brummer-simon force-pushed the gnrc_tcp-rewrite_passive_open branch from 0d5db28 to 1fd9af1 Compare July 8, 2021 04:16
@brummer-simon
Copy link
Member Author

0d5db28 still needs to be squashed in the original commits.

Forgot this one. I squashed it.

@miri64 miri64 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 8, 2021
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@miri64
Copy link
Member

miri64 commented Jul 8, 2021

Not sure why murdock is marked as failed or still doing label checks...

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 8, 2021
@brummer-simon
Copy link
Member Author

brummer-simon commented Jul 8, 2021

@miri64: Should I squash it in a single commit? I splitted the PR only to help with the review.

EDIT: Since all is reviewed and green, I just squash the entire thing and merge it.

@brummer-simon brummer-simon force-pushed the gnrc_tcp-rewrite_passive_open branch from 1fd9af1 to 88a0273 Compare July 8, 2021 08:04
@brummer-simon
Copy link
Member Author

@miri64 , @benpicco, since everything is green, could you press the merge button? For an unknown reason I am not able to do that anymore :(

@fjmolinas fjmolinas merged commit 79ee4fd into RIOT-OS:master Jul 8, 2021
@brummer-simon brummer-simon deleted the gnrc_tcp-rewrite_passive_open branch July 8, 2021 12:08
@miri64
Copy link
Member

miri64 commented Jul 8, 2021

@miri64 , @benpicco, since everything is green, could you press the merge button? For an unknown reason I am not able to do that anymore :(

Where you ever? You need write permissions to hit the green button (which only maintainers have).

@brummer-simon
Copy link
Member Author

Maybe my mind is playing a trick on me but I could swere that there was I time were I was able to press the merge button for my own PR after a successfull review. But I had never maintainer status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TCP Sockets can not be used / built
6 participants