Increase DHCP timeout, and rework networking tests logic #4369

bridadan · 2017-05-23T00:29:03Z

Description

This PR attempts to fix a number of issues that have cropped up with the networking tests lately. It alters the behavior of the test to be more forgiving of certain types of network hiccups.

This is a continuation of #4240.

I have removed the random assignment of mac addresses from this PR since the issue is more complex and requires greater consensus.

Status

READY

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Todos

Get the nightly passing

bridadan · 2017-05-23T00:31:54Z

@adbridge I tried to incorporate all of your feedback as best I could, if you see anything woefully amiss, please be sure to let me know!

bridadan · 2017-05-23T00:32:01Z

/morph test-nightly

mbed-bot · 2017-05-23T14:13:51Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 315

Test failed!

adbridge · 2017-05-23T15:04:27Z

@studavekar Lots of timeouts for NUCLEO_F429ZI ??

studavekar · 2017-05-23T15:06:22Z

Have replaced the board NUCLEO_F429ZI.

adbridge · 2017-05-23T15:08:36Z

/morph test-nightly

AnotherButler · 2017-05-23T20:26:59Z

@bridadan Because we use PR subject lines in our release notes, could you please add a comma after "timeout"? [Two independent clauses separated by a coordinating conjunction require a comma before the conjunction.]

mbed-bot · 2017-05-24T00:49:20Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 322

Test failed!

adbridge · 2017-05-24T09:39:55Z

NCS36510 failed again with the same error as some other PRs:

18:50:51 [1495583536.10][CONN][RXD] delay 40ms => error 65059us
18:50:51 [1495583536.17][CONN][RXD] :96::FAIL: Values Not Within Delta 5000 Expected 105059 Was 40000
18:50:52 [1495583536.23][CONN][INF] found KV pair in stream: {{__testcase_finish;Testing accuracy of equeue semaphore;0;1}}, queued...
18:50:52 [1495583536.34][CONN][RXD] >>> 'Testing accuracy of equeue semaphore': 0 passed, 1 failed with reason 'Assertion

@studavekar do we have another problem board here? Either that or we have a genuine failure case

geky · 2017-05-25T17:36:43Z

Huh, it looks like it may be more issues with the NCS36510's 16-bit timer arithmetic, although I thought we clamped down the implementation over in #3779.

@pan- has a pr coming in that lets us move the 16-bit timer arithmetic out of target code: #4094
I have a reimplementation of the NCS36510's timer ready to go: geky@492b558

Feel free to raise in issue to track this failure, but we should go ahead and adopt @pan-'s hal level implementation rather than continuing to debug target code.

0xc0170 · 2017-05-26T11:59:07Z

OK so this is the second failure with NCS36510. I left the comment in the issue we closed previously just like 2 hours ago. I however did not realize that there is that 64bit pull request. It is ready for the very last review, so hopefully will make it to 5.5.

sg- · 2017-05-29T13:10:11Z

/morph test-nightly

mbed-bot · 2017-05-29T19:29:28Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 365

Test failed!

sg- · 2017-05-30T21:09:23Z

/morph test-nightly

mbed-bot · 2017-05-31T00:59:37Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 372

Test failed!

0xc0170 · 2017-05-31T09:58:11Z

@geky @bridadan Can you help to resolve the remaining issues?

adbridge

Some minor changes

adbridge · 2017-05-31T10:23:18Z

features/FEATURE_LWIP/TESTS/mbedmicro-net/udp_echo/main.cpp

-    eth.connect();
+
+    int err = eth.connect();
+    TEST_ASSERT_EQUAL(0, err);


Why do we have TEST_ASSERT_EQUAL(0, err) both before and after the if statement ?

adbridge · 2017-05-31T10:29:04Z

features/FEATURE_LWIP/TESTS/mbedmicro-net/udp_echo_parallel/main.cpp

        err = sock.close();
        TEST_ASSERT_EQUAL(0, err);
+        if (err) {


Should this assert be after the setting of result ?

adbridge · 2017-05-31T10:32:40Z

features/frameworks/greentea-client/source/greentea_test_env.cpp

+/** \brief Handshake with host and send setup data (timeout and host test name)
+ *  \details This function will send preamble to master.
+ *           After host test name is received master will invoke host test script
+ *           and add hos test's callback handlers to main event loop


adbridge · 2017-05-31T10:33:03Z

features/frameworks/greentea-client/source/greentea_test_env.cpp

+/** \brief Handshake with host and send setup data (timeout and host test name). Allows you to preserve sync UUID.
+ *  \details This function will send preamble to master.
+ *           After host test name is received master will invoke host test script
+ *           and add hos test's callback handlers to main event loop


sg- · 2017-05-31T16:04:53Z

@bridadan seems like the same failures 2 times. Mind having a look?

sg- · 2017-06-10T14:05:35Z

Compile error for mbed-os-example-cellular broken on master due to ee30df9 introduced by #4513

The dtls test already has the ability to retry upon a UDP failure. However the sockets are currently configured to be blocking and to wait forever. I added a timeout of 1.5 seconds in order the test to correctly timeout.

This adds some extra logs to the test for debugging purposes. Also allows the test to fail immediately if it fails to obtain an IP address.

The Greentea UUID can be used as a source of entropy/randomness during the test. This is useful for uniquely identifying a test run.

Before, the UDP test was very strict on the number of packets it would try to match before failing. Now it will keep trying for the whole test to get enough passing packets. It also includes the test's UUID so you can validate which packets are being received.

Making the test more forgiven for minor networking issues. Also adding more debug prints to make it easier to see which packets are coming from where.

This matches the timeout used in linux: https://linux.die.net/man/5/dhclient.conf This resolves several issues noticed during testing when we have a very large number of devices that try to get an IP address around the same time.

Avoids getting stuck in loop where the device always picks up the previous test's packets

@bridadan

per @bridadan

This is a workaround for IAR's lack of flexibility with memory regions. Otherwise these tests would use very little heap and be mostly global allocations.

This creates a macro for the UUID length used by Greentea. This cuts down on the use of "magic numbers" in test cases that use the GREENTEA_SETUP_UUID function.

This file is moved to targets that support eth. It can't be common for any F4, as not all targets from this family support eth.

0xc0170 · 2017-06-10T14:24:08Z

@bridadan I rebased ,and fix a regression from the previous CI (ethernet for F4), should be green !

sg- · 2017-06-10T14:29:08Z

/morph test-nightly

mbed-bot · 2017-06-10T19:57:14Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 525

All builds and test passed!

bridadan requested review from adbridge and geky May 23, 2017 00:29

bridadan mentioned this pull request May 23, 2017

Networking test fixes #4240

Closed

3 tasks

bridadan requested a review from kegilbert May 23, 2017 00:31

bridadan changed the title ~~Increase DHCP timeout and rework networking tests logic~~ Increase DHCP timeout, and rework networking tests logic May 24, 2017

geky mentioned this pull request May 25, 2017

FAT: Add support for block sizes of 512-4096 bytes #3972

Merged

geky mentioned this pull request May 26, 2017

bd: Add MBR block device for standard storage partitioning #3936

Merged

adbridge added release-version: 5.5.0-rc1 needs: work labels May 30, 2017

sg- added needs: CI and removed needs: work labels May 30, 2017

adbridge requested changes May 31, 2017

View reviewed changes

sg- added the needs: work label May 31, 2017

bridadan and others added 21 commits June 10, 2017 15:14

Allowing dtls test to retry correctly.

26eafd9

The dtls test already has the ability to retry upon a UDP failure. However the sockets are currently configured to be blocking and to wait forever. I added a timeout of 1.5 seconds in order the test to correctly timeout.

Adding failure logs to TCP echo test.

98a5262

This adds some extra logs to the test for debugging purposes. Also allows the test to fail immediately if it fails to obtain an IP address.

Adding ability to preserve Greentea UUID in test.

551ff03

The Greentea UUID can be used as a source of entropy/randomness during the test. This is useful for uniquely identifying a test run.

Making udp_echo_parallel test more robust against network issues.

dc87aaa

Making the test more forgiven for minor networking issues. Also adding more debug prints to make it easier to see which packets are coming from where.

Arranging logic in test to prevent left-over behavior

f313bc2

Fixing some issues in the tcp/udp tests

70dd7cc

lwip: Increased DHCP timeout to 60 seconds

f11f2b3

This matches the timeout used in linux: https://linux.die.net/man/5/dhclient.conf This resolves several issues noticed during testing when we have a very large number of devices that try to get an IP address around the same time.

lwip: Added udp buffer flushing after dropped packets

036f1cd

Avoids getting stuck in loop where the device always picks up the previous test's packets

lwip: Fixed count of udp iterations to reflect attempts

0887683

lwip: Migrated to utest framework

467eb34

per @bridadan

Moved network test buffers into heap

d451d17

This is a workaround for IAR's lack of flexibility with memory regions. Otherwise these tests would use very little heap and be mostly global allocations.

Specifying default length of UUID for Greentea

6e76fce

This creates a macro for the UUID length used by Greentea. This cuts down on the use of "magic numbers" in test cases that use the GREENTEA_SETUP_UUID function.

Simplifying assert statements

388968f

Adding comments describing buffer prep functions

ccad027

Cleaning up udp tests

c38b709

Adding comments for added Greentea function

f616bf2

Fixing bad asserts

94b4160

Fixing typos in docs

00a41ad

Fix bad print message in test

4f5a199

lwip: fix stm32f4 eth conf file

d0d7c6f

This file is moved to targets that support eth. It can't be common for any F4, as not all targets from this family support eth.

0xc0170 force-pushed the networking_test_update branch from 2523fc8 to d0d7c6f Compare June 10, 2017 14:21

sg- added ready for merge and removed needs: CI labels Jun 10, 2017

sg- merged commit f56d64f into ARMmbed:master Jun 10, 2017

sg- removed the ready for merge label Jun 10, 2017

Increase DHCP timeout, and rework networking tests logic #4369

Increase DHCP timeout, and rework networking tests logic #4369

Uh oh!

Conversation

bridadan commented May 23, 2017

Description

Status

Migrations

Todos

Uh oh!

bridadan commented May 23, 2017

Uh oh!

bridadan commented May 23, 2017

Uh oh!

mbed-bot commented May 23, 2017

Output

Uh oh!

adbridge commented May 23, 2017

Uh oh!

studavekar commented May 23, 2017

Uh oh!

adbridge commented May 23, 2017

Uh oh!

AnotherButler commented May 23, 2017

Uh oh!

mbed-bot commented May 24, 2017

Output

Uh oh!

adbridge commented May 24, 2017

Uh oh!

geky commented May 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented May 26, 2017

Uh oh!

sg- commented May 29, 2017

Uh oh!

mbed-bot commented May 29, 2017

Output

Uh oh!

sg- commented May 30, 2017

Uh oh!

mbed-bot commented May 31, 2017

Output

Uh oh!

0xc0170 commented May 31, 2017

Uh oh!

adbridge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sg- commented May 31, 2017

Uh oh!

sg- commented Jun 10, 2017

Uh oh!

0xc0170 commented Jun 10, 2017

Uh oh!

sg- commented Jun 10, 2017

Uh oh!

mbed-bot commented Jun 10, 2017

Output

Uh oh!

Uh oh!

geky commented May 25, 2017 •

edited

Loading