From 0cd24285d4f7bbb3cde20ce3ee268336b285dcf9 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 13 Oct 2024 10:07:14 -0700 Subject: [PATCH 1/4] UDP: burst testing to improve coverage We are finding that on darwin its very easy for us to lose UDP messages as the socket buffer appears to be depressingly small. --- src/sp/transport/udp/udp.c | 34 ++++++-- src/sp/transport/udp/udp_tran_test.c | 112 ++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 10 deletions(-) diff --git a/src/sp/transport/udp/udp.c b/src/sp/transport/udp/udp.c index b55fcdb16..df5845fe8 100644 --- a/src/sp/transport/udp/udp.c +++ b/src/sp/transport/udp/udp.c @@ -1289,7 +1289,8 @@ udp_timer_cb(void *arg) } static int -udp_ep_init(udp_ep **epp, nng_url *url, nni_sock *sock) +udp_ep_init(udp_ep **epp, nng_url *url, nni_sock *sock, nni_dialer *dialer, + nni_listener *listener) { udp_ep *ep; int rv; @@ -1432,7 +1433,29 @@ udp_ep_init(udp_ep **epp, nng_url *url, nni_sock *sock) nni_stat_init(&ep->st_snd_nobuf, &snd_nobuf_info); nni_stat_init(&ep->st_peer_inactive, &peer_inactive_info); - nni_stat_set_value(&ep->st_rcv_max, ep->rcvmax); + if (listener) { + nni_listener_add_stat(listener, &ep->st_rcv_max); + nni_listener_add_stat(listener, &ep->st_copy_max); + nni_listener_add_stat(listener, &ep->st_rcv_copy); + nni_listener_add_stat(listener, &ep->st_rcv_nocopy); + nni_listener_add_stat(listener, &ep->st_rcv_reorder); + nni_listener_add_stat(listener, &ep->st_rcv_toobig); + nni_listener_add_stat(listener, &ep->st_rcv_nomatch); + nni_listener_add_stat(listener, &ep->st_rcv_nobuf); + nni_listener_add_stat(listener, &ep->st_snd_toobig); + nni_listener_add_stat(listener, &ep->st_snd_nobuf); + } else { + nni_dialer_add_stat(dialer, &ep->st_rcv_max); + nni_dialer_add_stat(dialer, &ep->st_copy_max); + nni_dialer_add_stat(dialer, &ep->st_rcv_copy); + nni_dialer_add_stat(dialer, &ep->st_rcv_nocopy); + nni_dialer_add_stat(dialer, &ep->st_rcv_reorder); + nni_dialer_add_stat(dialer, &ep->st_rcv_toobig); + nni_dialer_add_stat(dialer, &ep->st_rcv_nomatch); + nni_dialer_add_stat(dialer, &ep->st_rcv_nobuf); + nni_dialer_add_stat(dialer, &ep->st_snd_toobig); + nni_dialer_add_stat(dialer, &ep->st_snd_nobuf); + } // schedule our timer callback - forever for now // adjusted automatically as we add pipes or other @@ -1474,11 +1497,10 @@ udp_dialer_init(void **dp, nng_url *url, nni_dialer *ndialer) return (rv); } - if ((rv = udp_ep_init(&ep, url, sock)) != 0) { + if ((rv = udp_ep_init(&ep, url, sock, ndialer, NULL)) != 0) { return (rv); } - nni_dialer_add_stat(ndialer, &ep->st_rcv_max); *dp = ep; return (0); } @@ -1497,13 +1519,11 @@ udp_listener_init(void **lp, nng_url *url, nni_listener *nlistener) return (rv); } - if ((rv = udp_ep_init(&ep, url, sock)) != 0) { + if ((rv = udp_ep_init(&ep, url, sock, NULL, nlistener)) != 0) { return (rv); } ep->self_sa = sa; - nni_listener_add_stat(nlistener, &ep->st_rcv_max); - *lp = ep; return (0); } diff --git a/src/sp/transport/udp/udp_tran_test.c b/src/sp/transport/udp/udp_tran_test.c index c2b515e8d..304df0976 100644 --- a/src/sp/transport/udp/udp_tran_test.c +++ b/src/sp/transport/udp/udp_tran_test.c @@ -1,8 +1,5 @@ // // Copyright 2024 Staysail Systems, Inc. -// Copyright 2018 Capitar IT Group BV -// Copyright 2018 Devolutions -// Copyright 2018 Cody Piersall // // This software is supplied under the terms of the MIT License, a // copy of which should be located in the distribution where this @@ -195,6 +192,113 @@ test_udp_recv_copy(void) NUTS_CLOSE(s1); } +void +test_udp_multi_send_recv(void) +{ + char msg[256]; + char buf[256]; + nng_socket s0; + nng_socket s1; + nng_listener l; + nng_dialer d; + size_t sz; + char *addr; + + NUTS_ADDR(addr, "udp"); + + NUTS_OPEN(s0); + NUTS_PASS(nng_socket_set_ms(s0, NNG_OPT_RECVTIMEO, 100)); + NUTS_PASS(nng_socket_set_ms(s0, NNG_OPT_SENDTIMEO, 100)); + NUTS_PASS(nng_listener_create(&l, s0, addr)); + NUTS_PASS(nng_listener_set_size(l, NNG_OPT_UDP_COPY_MAX, 100)); + NUTS_PASS(nng_listener_get_size(l, NNG_OPT_UDP_COPY_MAX, &sz)); + NUTS_TRUE(sz == 100); + NUTS_PASS(nng_listener_start(l, 0)); + + NUTS_OPEN(s1); + NUTS_PASS(nng_socket_set_ms(s1, NNG_OPT_RECVTIMEO, 100)); + NUTS_PASS(nng_socket_set_ms(s1, NNG_OPT_SENDTIMEO, 100)); + NUTS_PASS(nng_dialer_create(&d, s1, addr)); + NUTS_PASS(nng_dialer_set_size(d, NNG_OPT_UDP_COPY_MAX, 100)); + NUTS_PASS(nng_dialer_get_size(d, NNG_OPT_UDP_COPY_MAX, &sz)); + NUTS_PASS(nng_dialer_start(d, 0)); + nng_msleep(100); + + for (int i = 0; i < 1000; i++) { + NUTS_PASS(nng_send(s1, msg, 95, 0)); + NUTS_PASS(nng_recv(s0, buf, &sz, 0)); + NUTS_TRUE(sz == 95); + NUTS_PASS(nng_send(s0, msg, 95, 0)); + NUTS_PASS(nng_recv(s1, buf, &sz, 0)); + NUTS_TRUE(sz == 95); + } + NUTS_CLOSE(s0); + NUTS_CLOSE(s1); +} + +void +test_udp_multi_small_burst(void) +{ + char msg[256]; + char buf[256]; + nng_socket s0; + nng_socket s1; + nng_listener l; + nng_dialer d; + size_t sz; + char *addr; + + NUTS_ADDR(addr, "udp"); + + NUTS_OPEN(s0); + NUTS_PASS(nng_socket_set_ms(s0, NNG_OPT_RECVTIMEO, 10)); + NUTS_PASS(nng_socket_set_ms(s0, NNG_OPT_SENDTIMEO, 1000)); + NUTS_PASS(nng_listener_create(&l, s0, addr)); + NUTS_PASS(nng_listener_set_size(l, NNG_OPT_UDP_COPY_MAX, 100)); + NUTS_PASS(nng_listener_get_size(l, NNG_OPT_UDP_COPY_MAX, &sz)); + NUTS_TRUE(sz == 100); + NUTS_PASS(nng_listener_start(l, 0)); + + NUTS_OPEN(s1); + NUTS_PASS(nng_socket_set_ms(s1, NNG_OPT_RECVTIMEO, 10)); + NUTS_PASS(nng_socket_set_ms(s1, NNG_OPT_SENDTIMEO, 1000)); + NUTS_PASS(nng_dialer_create(&d, s1, addr)); + NUTS_PASS(nng_dialer_set_size(d, NNG_OPT_UDP_COPY_MAX, 100)); + NUTS_PASS(nng_dialer_get_size(d, NNG_OPT_UDP_COPY_MAX, &sz)); + NUTS_PASS(nng_dialer_start(d, 0)); + nng_msleep(100); + + float actual = 0; + float expect = 0; + int burst = 4; + int count = 20; + + // Experimentally at least on Darwin, we see some packet losses + // even for loopback. Loss rates appear depressingly high. + for (int i = 0; i < count; i++) { + for (int j = 0; j < burst; j++) { + NUTS_PASS(nng_send(s1, msg, 95, 0)); + expect++; + } + for (int j = 0; j < burst; j++) { + if (nng_recv(s0, buf, &sz, 0) == 0) { + NUTS_TRUE(sz == 95); + actual++; + } + } + NUTS_PASS(nng_send(s0, msg, 95, 0)); + NUTS_PASS(nng_recv(s1, buf, &sz, 0)); + NUTS_TRUE(sz == 95); + } + NUTS_TRUE(actual <= expect); + NUTS_TRUE( + actual / expect > 0.80); // maximum reasonable packet loss of 20% + NUTS_MSG("Packet loss: %.02f (got %.f of %.f)", 1.0 - actual / expect, + actual, expect); + NUTS_CLOSE(s0); + NUTS_CLOSE(s1); +} + NUTS_TESTS = { { "udp wild card connect fail", test_udp_wild_card_connect_fail }, @@ -205,5 +309,7 @@ NUTS_TESTS = { { "udp malformed address", test_udp_malformed_address }, { "udp recv max", test_udp_recv_max }, { "udp recv copy", test_udp_recv_copy }, + { "udp multi send recv", test_udp_multi_send_recv }, + { "udp multi small burst", test_udp_multi_small_burst }, { NULL, NULL }, }; From 175a134b94dd3a3d77e6c3237408f102f2659288 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 13 Oct 2024 10:15:59 -0700 Subject: [PATCH 2/4] UDP/sanitizer: Don't be strict about message loss in sanitizer or coverage runs. --- CMakeLists.txt | 2 ++ src/sp/transport/udp/udp_tran_test.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c67b49da5..7b753d388 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -180,6 +180,7 @@ include(CheckSanitizer) CheckSanitizer() if (NOT NNG_SANITIZER STREQUAL "none") set(NNG_SANITIZER_FLAGS "-fsanitize=${NNG_SANITIZER}") + add_definitions(-DNNG_SANITIZER) endif () if (NNG_ENABLE_COVERAGE) @@ -195,6 +196,7 @@ if (NNG_ENABLE_COVERAGE) else () message(FATAL_ERROR "Unable to enable coverage for your compiler.") endif () + add_definitions(-DNNG_CONVERAGE) endif () set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${NNG_WARN_FLAGS} ${NNG_COVERAGE_C_FLAGS} ${NNG_SANITIZER_FLAGS}") diff --git a/src/sp/transport/udp/udp_tran_test.c b/src/sp/transport/udp/udp_tran_test.c index 304df0976..3b46c618d 100644 --- a/src/sp/transport/udp/udp_tran_test.c +++ b/src/sp/transport/udp/udp_tran_test.c @@ -291,10 +291,13 @@ test_udp_multi_small_burst(void) NUTS_TRUE(sz == 95); } NUTS_TRUE(actual <= expect); +#if !defined(NNG_SANITIZER) && !defined(NNG_COVERAGE) + // Under sanitizer runs we lose a lot, maybe even majority, of packets NUTS_TRUE( - actual / expect > 0.80); // maximum reasonable packet loss of 20% + actual / expect > 0.50); // maximum reasonable packet loss of 20% NUTS_MSG("Packet loss: %.02f (got %.f of %.f)", 1.0 - actual / expect, actual, expect); +#endif NUTS_CLOSE(s0); NUTS_CLOSE(s1); } From b61285dc16974efe31a3b8d8923f17c8ce3cceb0 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 13 Oct 2024 12:05:12 -0700 Subject: [PATCH 3/4] udp: fix race, fix tests --- src/sp/transport/udp/udp.c | 12 ++++++++---- src/sp/transport/udp/udp_tran_test.c | 6 ++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/sp/transport/udp/udp.c b/src/sp/transport/udp/udp.c index df5845fe8..0e3b4ae80 100644 --- a/src/sp/transport/udp/udp.c +++ b/src/sp/transport/udp/udp.c @@ -969,6 +969,7 @@ udp_pipe_send(void *arg, nni_aio *aio) udp_ep *ep; udp_sp_data dreq; nng_msg *msg; + size_t count = 0; if (nni_aio_begin(aio) != 0) { // No way to give the message back to the protocol, @@ -981,6 +982,10 @@ udp_pipe_send(void *arg, nni_aio *aio) msg = nni_aio_get_msg(aio); ep = p->ep; + if (msg != NULL) { + count = nni_msg_len(msg) + nni_msg_header_len(msg); + } + nni_mtx_lock(&ep->mtx); if ((nni_msg_len(msg) + nni_msg_header_len(msg)) > p->sndmax) { nni_mtx_unlock(&ep->mtx); @@ -999,14 +1004,13 @@ udp_pipe_send(void *arg, nni_aio *aio) dreq.us_sender_id = p->self_id; dreq.us_peer_id = p->peer_id; dreq.us_sequence = p->self_seq++; - dreq.us_length = - msg != NULL ? nni_msg_len(msg) + nni_msg_header_len(msg) : 0; + dreq.us_length = (uint16_t) count; // Just queue it, or fail it. udp_queue_tx(ep, &p->peer_addr, (void *) &dreq, msg); nni_mtx_unlock(&ep->mtx); - nni_aio_finish( - aio, 0, msg ? nni_msg_len(msg) + nni_msg_header_len(msg) : 0); + + nni_aio_finish(aio, 0, count); } static void diff --git a/src/sp/transport/udp/udp_tran_test.c b/src/sp/transport/udp/udp_tran_test.c index 3b46c618d..d68f60f9f 100644 --- a/src/sp/transport/udp/udp_tran_test.c +++ b/src/sp/transport/udp/udp_tran_test.c @@ -273,6 +273,12 @@ test_udp_multi_small_burst(void) int burst = 4; int count = 20; +#ifdef NNG_PLATFORM_WINDOWS + // Windows seems to drop a lot - maybe because of virtualization + burst = 2; + count = 10; +#endif + // Experimentally at least on Darwin, we see some packet losses // even for loopback. Loss rates appear depressingly high. for (int i = 0; i < count; i++) { From f9591be4916fb2467ce5a45ad74a12242946083c Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 13 Oct 2024 13:02:56 -0700 Subject: [PATCH 4/4] UDP: More test tuning for lossy environments. --- src/sp/transport/udp/udp_tran_test.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/sp/transport/udp/udp_tran_test.c b/src/sp/transport/udp/udp_tran_test.c index d68f60f9f..fe567e7bf 100644 --- a/src/sp/transport/udp/udp_tran_test.c +++ b/src/sp/transport/udp/udp_tran_test.c @@ -268,15 +268,20 @@ test_udp_multi_small_burst(void) NUTS_PASS(nng_dialer_start(d, 0)); nng_msleep(100); - float actual = 0; - float expect = 0; - int burst = 4; - int count = 20; - -#ifdef NNG_PLATFORM_WINDOWS + float actual = 0; + float expect = 0; + float require = 0.50; + int burst = 4; + int count = 20; + +#if defined(NNG_SANITIZER) || defined(NNG_COVERAGE) + // sanitizers may drop a lot, so can coverage + require = 0.0; +#elif defined(NNG_PLATFORM_WINDOWS) // Windows seems to drop a lot - maybe because of virtualization - burst = 2; - count = 10; + burst = 2; + count = 10; + require = 0.10; #endif // Experimentally at least on Darwin, we see some packet losses @@ -297,13 +302,9 @@ test_udp_multi_small_burst(void) NUTS_TRUE(sz == 95); } NUTS_TRUE(actual <= expect); -#if !defined(NNG_SANITIZER) && !defined(NNG_COVERAGE) - // Under sanitizer runs we lose a lot, maybe even majority, of packets - NUTS_TRUE( - actual / expect > 0.50); // maximum reasonable packet loss of 20% + NUTS_TRUE(actual / expect > require); NUTS_MSG("Packet loss: %.02f (got %.f of %.f)", 1.0 - actual / expect, actual, expect); -#endif NUTS_CLOSE(s0); NUTS_CLOSE(s1); }