tests/gnrc_ndp: enhance coverage #10987

miri64 · 2019-02-11T13:38:28Z

Contribution description

To ensure that the bug I was trying to fix in #10985 wasn't actually caused by the gnrc_ndp sending functions I enhanced the module's test coverage.

Testing procedure

To make sure the coverage is actually enhanced I applied the following patch

diff --git a/sys/net/gnrc/network_layer/ndp/gnrc_ndp.c b/sys/net/gnrc/network_layer/ndp/gnrc_ndp.c
index c42477cfee..cc82c4c35b 100644
--- a/sys/net/gnrc/network_layer/ndp/gnrc_ndp.c
+++ b/sys/net/gnrc/network_layer/ndp/gnrc_ndp.c
@@ -287,6 +287,7 @@ void gnrc_ndp_nbr_sol_send(const ipv6_addr_t *tgt, gnrc_netif_t *netif,
                 hdr = gnrc_ndp_opt_sl2a_build(l2src, l2src_len, pkt);
 
                 if (hdr == NULL) {
+                    puts("option failed");
                     DEBUG("ndp: error allocating SL2AO.\n");
                     break;
                 }
@@ -296,6 +297,7 @@ void gnrc_ndp_nbr_sol_send(const ipv6_addr_t *tgt, gnrc_netif_t *netif,
         /* add neighbor solicitation header */
         hdr = gnrc_ndp_nbr_sol_build(tgt, pkt);
         if (hdr == NULL) {
+            puts("message failed");
             DEBUG("ndp: error allocating neighbor solicitation.\n");
             break;
         }
@@ -369,6 +371,7 @@ void gnrc_ndp_nbr_adv_send(const ipv6_addr_t *tgt, gnrc_netif_t *netif,
                 hdr = gnrc_ndp_opt_tl2a_build(l2tgt, l2tgt_len, pkt);
 
                 if (hdr == NULL) {
+                    puts("option failed");
                     DEBUG("ndp: error allocating TL2AO.\n");
                     break;
                 }
@@ -385,6 +388,7 @@ void gnrc_ndp_nbr_adv_send(const ipv6_addr_t *tgt, gnrc_netif_t *netif,
         /* add neighbor advertisement header */
         hdr = gnrc_ndp_nbr_adv_build(tgt, adv_flags, pkt);
         if (hdr == NULL) {
+            puts("message failed");
             DEBUG("ndp: error allocating neighbor advertisement.\n");
             break;
         }
@@ -434,6 +438,7 @@ void gnrc_ndp_rtr_sol_send(gnrc_netif_t *netif, const ipv6_addr_t *dst)
                 /* add source address link-layer address option */
                 pkt = gnrc_ndp_opt_sl2a_build(l2src, l2src_len, NULL);
                 if (pkt == NULL) {
+                    puts("option failed");
                     DEBUG("ndp: error allocating SL2AO.\n");
                     break;
                 }
@@ -442,6 +447,7 @@ void gnrc_ndp_rtr_sol_send(gnrc_netif_t *netif, const ipv6_addr_t *dst)
         /* add router solicitation header */
         hdr = gnrc_ndp_rtr_sol_build(pkt);
         if (hdr == NULL) {
+            puts("message failed");
             DEBUG("ndp: error allocating router solicitation.\n");
             break;
         }
@@ -518,6 +524,7 @@ void gnrc_ndp_rtr_adv_send(gnrc_netif_t *netif, const ipv6_addr_t *src,
                 hdr = gnrc_ndp_opt_sl2a_build(l2src, l2src_len, pkt);
 
                 if (hdr == NULL) {
+                    puts("option failed");
                     DEBUG("ndp: error allocating Source Link-layer address "
                           "option.\n");
                     break;
@@ -556,6 +563,7 @@ void gnrc_ndp_rtr_adv_send(gnrc_netif_t *netif, const ipv6_addr_t *src,
         hdr = gnrc_ndp_rtr_adv_build(cur_hl, flags, adv_ltime, reach_time,
                                      retrans_timer, pkt);
         if (hdr == NULL) {
+            puts("message failed");
             DEBUG("ndp: error allocating router advertisement.\n");
             break;
         }
@@ -598,6 +606,7 @@ static gnrc_pktsnip_t *_build_headers(gnrc_netif_t *netif,
     gnrc_pktsnip_t *iphdr = gnrc_ipv6_hdr_build(payload, src, dst);
 
     if (iphdr == NULL) {
+        puts("IPv6 failed");
         DEBUG("ndp: error allocating IPv6 header.\n");
         return NULL;
     }
@@ -605,6 +614,7 @@ static gnrc_pktsnip_t *_build_headers(gnrc_netif_t *netif,
     /* add netif header for send interface specification */
     l2hdr = gnrc_netif_hdr_build(NULL, 0, NULL, 0);
     if (l2hdr == NULL) {
+        puts("netif failed");
         DEBUG("ndp: error allocating netif header.\n");
         gnrc_pktbuf_remove_snip(iphdr, iphdr);
         return NULL;

The test should then show something like

option failed
message failed
IPv6 failed
netif failed

4 times with varying numbers periods in-between with this PR, without it it probably won't show up at all (I did not test this case). I tested the first case on native, samr21-xpro, and iotlab-m3

Issues/PRs references

Ensurance that #10985 was the right hunch for the leak we've seen.

cladmi · 2019-02-26T21:50:43Z

The test results with frdm-kw41z I have nearby:

2019-02-26 22:44:38,227 - INFO # ����������즮sjw���ʺ�main(): This is RIOT! (Version: 2019.04-devel-159-g9fa22-pr/riot/10987/enhance_coverage)
2019-02-26 22:44:38,232 - INFO # .........................option failed
2019-02-26 22:44:38,233 - INFO # .message failed
2019-02-26 22:44:38,234 - INFO # .IPv6 failed
2019-02-26 22:44:38,234 - INFO # .netif failed
2019-02-26 22:44:38,238 - INFO # .................option failed
2019-02-26 22:44:38,242 - INFO # .message failed
2019-02-26 22:44:38,247 - INFO # .IPv6 failed
2019-02-26 22:44:38,248 - INFO # .netif failed
2019-02-26 22:44:38,249 - INFO # ....option failed
2019-02-26 22:44:38,250 - INFO # .message failed
2019-02-26 22:44:38,251 - INFO # .IPv6 failed
2019-02-26 22:44:38,252 - INFO # .netif failed
2019-02-26 22:44:38,253 - INFO # .................option failed
2019-02-26 22:44:38,257 - INFO # .message failed
2019-02-26 22:44:38,258 - INFO # .IPv6 failed
2019-02-26 22:44:38,259 - INFO # .netif failed
2019-02-26 22:44:38,259 - INFO #
2019-02-26 22:44:38,260 - INFO # OK (75 tests)

And the test output for iotlab-m3 BOARD=iotlab-m3 make -C tests/gnrc_ndp/ flash test

2019-02-26 22:50:23,223 - INFO # �main(): This is RIOT! (Version: 2019.04-devel-159-g9fa22-pr/riot/10987/enhance_coverage)
2019-02-26 22:50:23,225 - INFO # .........................option failed
2019-02-26 22:50:23,226 - INFO # .message failed
2019-02-26 22:50:23,226 - INFO # .IPv6 failed
2019-02-26 22:50:23,227 - INFO # .netif failed
2019-02-26 22:50:23,229 - INFO # .................option failed
2019-02-26 22:50:23,229 - INFO # .message failed
2019-02-26 22:50:23,236 - INFO # .IPv6 failed
2019-02-26 22:50:23,237 - INFO # .netif failed
2019-02-26 22:50:23,238 - INFO # ....option failed
2019-02-26 22:50:23,239 - INFO # .message failed
2019-02-26 22:50:23,240 - INFO # .IPv6 failed
2019-02-26 22:50:23,241 - INFO # .netif failed
2019-02-26 22:50:23,242 - INFO # .................option failed
2019-02-26 22:50:23,243 - INFO # .message failed
2019-02-26 22:50:23,244 - INFO # .IPv6 failed
2019-02-26 22:50:23,245 - INFO # .netif failed
2019-02-26 22:50:23,245 - INFO #
2019-02-26 22:50:23,246 - INFO # OK (75 tests)

miri64 · 2019-02-26T21:52:31Z

For both it is the output expected as per the testing procedures. So do you ACK?

fjmolinas

Code looks good, I followed the test procedure and got the output expected. ACK!

make -C tests/gnrc_ndp/ BOARD=samr21-xpro test
make: Entering directory '/home/francisco/workspace/RIOT/tests/gnrc_ndp'
/home/francisco/workspace/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-08-09 11:08:26,937 - INFO # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2019-08-09 11:08:30,028 - INFO # main(): This is RIOT! (Version: 2019.04-devel-159-g9fa22-pr-10987)
2019-08-09 11:08:30,034 - INFO # .........................option failed
2019-08-09 11:08:30,035 - INFO # .message failed
2019-08-09 11:08:30,036 - INFO # .IPv6 failed
2019-08-09 11:08:30,038 - INFO # .netif failed
2019-08-09 11:08:30,043 - INFO # .................option failed
2019-08-09 11:08:30,045 - INFO # .message failed
2019-08-09 11:08:30,046 - INFO # .IPv6 failed
2019-08-09 11:08:30,048 - INFO # .netif failed
2019-08-09 11:08:30,050 - INFO # ....option failed
2019-08-09 11:08:30,051 - INFO # .message failed
2019-08-09 11:08:30,054 - INFO # .IPv6 failed
2019-08-09 11:08:30,055 - INFO # .netif failed
2019-08-09 11:08:30,060 - INFO # .................option failed
2019-08-09 11:08:30,061 - INFO # .message failed
2019-08-09 11:08:30,062 - INFO # .IPv6 failed
2019-08-09 11:08:30,063 - INFO # .netif failed
2019-08-09 11:08:30,064 - INFO # 
2019-08-09 11:08:30,065 - INFO # OK (75 tests)

fjmolinas · 2019-08-09T09:24:28Z

@miri64 apparently with the changes the test doesn't fit in ROM for msb430, can you add it to BOARDS_INSUFFICIENT_MEMMORY

miri64 · 2019-08-09T09:25:13Z

@miri64 apparently with the changes the test doesn't fit in ROM for msb430, can you add it to BOARDS_INSUFFICIENT_MEMMORY

I'll wait until Murdock passed, in case more boards require to be added.

miri64 · 2019-08-09T09:25:42Z

Sorry, GitHub did not update, so I only saw now, that it is already done ^^"

miri64 · 2019-08-09T09:33:55Z

Updated BOARDS_INSUFFICIENT_MEMORY and squashed immediately

fjmolinas · 2019-08-09T09:40:36Z

@miri64 needs rebasing, the BOARDS_INSUFFICIENT_MEMORY list has been extended quite a bit in master already.

miri64 · 2019-08-09T09:42:15Z

What's up with GH today, not updating the page??!?

miri64 · 2019-08-09T09:57:44Z

Not only that: The test was touched in #10532 and gnrc_ndp was also touched since this PR was opened. Please re-test (I've updated the patch in OP for the new version of gnrc_ndp).

fjmolinas

I'm still getting the same output, re-ACK!

make -C tests/gnrc_ndp/ BOARD=samr21-xpro test
make: Entering directory '/home/francisco/workspace/RIOT/tests/gnrc_ndp'
/home/francisco/workspace/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-08-09 12:02:39,709 - INFO # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
2019-08-09 12:02:42,781 - INFO # main(): This is RIOT! (Version: 2019.10-devel-352-g0bfef-pr-10987)
2019-08-09 12:02:42,787 - INFO # .........................option failed
2019-08-09 12:02:42,788 - INFO # .message failed
2019-08-09 12:02:42,789 - INFO # .IPv6 failed
2019-08-09 12:02:42,791 - INFO # .netif failed
2019-08-09 12:02:42,797 - INFO # .................option failed
2019-08-09 12:02:42,798 - INFO # .message failed
2019-08-09 12:02:42,799 - INFO # .IPv6 failed
2019-08-09 12:02:42,801 - INFO # .netif failed
2019-08-09 12:02:42,803 - INFO # ....option failed
2019-08-09 12:02:42,804 - INFO # .message failed
2019-08-09 12:02:42,806 - INFO # .IPv6 failed
2019-08-09 12:02:42,807 - INFO # .netif failed
2019-08-09 12:02:42,813 - INFO # .................option failed
2019-08-09 12:02:42,814 - INFO # .message failed
2019-08-09 12:02:42,815 - INFO # .IPv6 failed
2019-08-09 12:02:42,817 - INFO # .netif failed
2019-08-09 12:02:42,818 - INFO # 
2019-08-09 12:02:42,819 - INFO # OK (75 tests)

fjmolinas · 2019-08-09T10:21:25Z

All green go!

miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking labels Feb 11, 2019

miri64 requested review from cladmi and bergzand February 11, 2019 13:38

miri64 added this to the Release 2019.04 milestone Feb 11, 2019

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 1, 2019

miri64 modified the milestones: Release 2019.04, Release 2019.07 May 7, 2019

MrKevinWeiss modified the milestones: Release 2019.07, Release 2019.10 Jul 16, 2019

fjmolinas approved these changes Aug 9, 2019

View reviewed changes

miri64 force-pushed the tests/enh/gnrc_ndp-coverage branch from 9fa22de to c7e8153 Compare August 9, 2019 09:33

tests/gnrc_ndp: enhance coverage

0bfef4c

miri64 removed the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Aug 9, 2019

miri64 force-pushed the tests/enh/gnrc_ndp-coverage branch from c7e8153 to 0bfef4c Compare August 9, 2019 09:57

miri64 requested review from fjmolinas and removed request for cladmi August 9, 2019 09:57

fjmolinas approved these changes Aug 9, 2019

View reviewed changes

fjmolinas merged commit 8b396b4 into RIOT-OS:master Aug 9, 2019

miri64 deleted the tests/enh/gnrc_ndp-coverage branch August 9, 2019 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/gnrc_ndp: enhance coverage #10987

tests/gnrc_ndp: enhance coverage #10987

miri64 commented Feb 11, 2019 •

edited

Loading

cladmi commented Feb 26, 2019 •

edited

Loading

miri64 commented Feb 26, 2019

fjmolinas left a comment

fjmolinas commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

fjmolinas commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

fjmolinas left a comment

fjmolinas commented Aug 9, 2019

tests/gnrc_ndp: enhance coverage #10987

tests/gnrc_ndp: enhance coverage #10987

Conversation

miri64 commented Feb 11, 2019 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

cladmi commented Feb 26, 2019 • edited Loading

miri64 commented Feb 26, 2019

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

fjmolinas commented Aug 9, 2019

miri64 commented Aug 9, 2019

miri64 commented Aug 9, 2019

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas commented Aug 9, 2019

miri64 commented Feb 11, 2019 •

edited

Loading

cladmi commented Feb 26, 2019 •

edited

Loading