[Openvpn-devel,v4,3/3] Add unit test for reliable_get_num_output_sequenced_available

Message ID 20220921104930.3452270-3-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v4,1/3] Ensure that control channel packet are respecting maximum packet size | expand

Commit Message

Arne Schwabe Sept. 21, 2022, 12:49 a.m. UTC
Patch v4: rebase

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 tests/unit_tests/openvpn/Makefile.am       |  5 +-
 tests/unit_tests/openvpn/mock_get_random.c | 10 ++++
 tests/unit_tests/openvpn/test_packet_id.c  | 55 ++++++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)

Comments

Frank Lichtenheld Oct. 18, 2022, 4:16 p.m. UTC | #1
Acked-By: Frank Lichtenheld <frank@lichtenheld>

Already acked in 1957647252.642516.1652264253337@office.mailbox.org

On Wed, Sep 21, 2022 at 12:49:30PM +0200, Arne Schwabe wrote:
> Patch v4: rebase
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  tests/unit_tests/openvpn/Makefile.am       |  5 +-
>  tests/unit_tests/openvpn/mock_get_random.c | 10 ++++
>  tests/unit_tests/openvpn/test_packet_id.c  | 55 ++++++++++++++++++++++
>  3 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
> index 65cf9549c..c7846f3e6 100644
> --- a/tests/unit_tests/openvpn/Makefile.am
> +++ b/tests/unit_tests/openvpn/Makefile.am
> @@ -61,7 +61,10 @@ packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c mock_msg.h \
>  	$(openvpn_srcdir)/buffer.c \
>  	$(openvpn_srcdir)/otime.c \
>  	$(openvpn_srcdir)/packet_id.c \
> -	$(openvpn_srcdir)/platform.c
> +	$(openvpn_srcdir)/platform.c \
> +	$(openvpn_srcdir)/reliable.c \
> +    $(openvpn_srcdir)/session_id.c
> +
>  
>  pkt_testdriver_CFLAGS  = @TEST_CFLAGS@ \
>  	-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir)
> diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
> index d0d2574ae..787b5e33e 100644
> --- a/tests/unit_tests/openvpn/mock_get_random.c
> +++ b/tests/unit_tests/openvpn/mock_get_random.c
> @@ -26,6 +26,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <setjmp.h>
> +#include <stdint.h>
>  #include <cmocka.h>
>  
>  unsigned long
> @@ -34,3 +35,12 @@ get_random(void)
>      /* rand() is not very random, but it's C99 and this is just for testing */
>      return rand();
>  }
> +
> +void
> +prng_bytes(uint8_t *output, int len)
> +{
> +    for (int i = 0; i < len; i++)
> +    {
> +        output[i] = rand();
> +    }
> +}
> diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
> index a3d4db285..c610d9727 100644
> --- a/tests/unit_tests/openvpn/test_packet_id.c
> +++ b/tests/unit_tests/openvpn/test_packet_id.c
> @@ -36,6 +36,7 @@
>  #include <cmocka.h>
>  
>  #include "packet_id.h"
> +#include "reliable.h"
>  
>  #include "mock_msg.h"
>  
> @@ -156,6 +157,59 @@ test_packet_id_write_long_wrap(void **state)
>      assert_true(data->test_buf_data.buf_time == htonl(now));
>  }
>  
> +static void
> +test_get_num_output_sequenced_available(void **state)
> +{
> +
> +    struct reliable *rel = malloc(sizeof(struct reliable));
> +    reliable_init(rel, 100, 50, 8, false);
> +
> +    rel->array[5].active = true;
> +    rel->array[5].packet_id = 100;
> +
> +    rel->packet_id = 103;
> +
> +    assert_int_equal(5, reliable_get_num_output_sequenced_available(rel));
> +
> +    rel->array[6].active = true;
> +    rel->array[6].packet_id = 97;
> +    assert_int_equal(2, reliable_get_num_output_sequenced_available(rel));
> +
> +    /* test ids close to int/unsigned int barrier */
> +
> +    rel->array[5].active = true;
> +    rel->array[5].packet_id = (0x80000000u -3);
> +    rel->array[6].active = false;
> +    rel->packet_id = (0x80000000u -1);
> +
> +    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
> +
> +    rel->array[5].active = true;
> +    rel->array[5].packet_id = (0x80000000u -3);
> +    rel->packet_id = 0x80000001u;
> +
> +    assert_int_equal(4, reliable_get_num_output_sequenced_available(rel));
> +
> +
> +    /* test wrapping */
> +    rel->array[5].active = true;
> +    rel->array[5].packet_id = (0xffffffffu -3);
> +    rel->array[6].active = false;
> +    rel->packet_id = (0xffffffffu - 1);
> +
> +    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
> +
> +    rel->array[2].packet_id = 0;
> +    rel->array[2].active = true;
> +
> +    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
> +
> +    rel->packet_id = 3;
> +    assert_int_equal(1, reliable_get_num_output_sequenced_available(rel));
> +
> +    reliable_free(rel);
> +}
> +
>  int
>  main(void)
>  {
> @@ -178,6 +232,7 @@ main(void)
>          cmocka_unit_test_setup_teardown(test_packet_id_write_long_wrap,
>                                          test_packet_id_write_setup,
>                                          test_packet_id_write_teardown),
> +        cmocka_unit_test(test_get_num_output_sequenced_available)
>      };
>  
>      return cmocka_run_group_tests_name("packet_id tests", tests, NULL, NULL);
> -- 
> 2.37.0 (Apple Git-136)
> 
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Frank Lichtenheld Oct. 19, 2022, 9:29 a.m. UTC | #2
On Tue, Oct 18, 2022 at 06:16:54PM +0200, Frank Lichtenheld wrote:
> Acked-By: Frank Lichtenheld <frank@lichtenheld>
> 
> Already acked in 1957647252.642516.1652264253337@office.mailbox.org
> 

Note: this patch does not depend on 2/3, only on 1/3.

Regards,
Gert Doering Nov. 5, 2022, 7:02 a.m. UTC | #3
This adds the unit test for the previously introduced new function - 
and applies nicely on "v6 1/2".  Tested on FreeBSD and Linux, and
"it passes".

..
[ RUN      ] test_get_num_output_sequenced_available
[       OK ] test_get_num_output_sequenced_available
[==========] 7 test(s) run.
[  PASSED  ] 7 test(s).
PASS: packet_id_testdriver

Since this got an ACK from Frank, I did not spend much stare-at-code
to verify that it actually tests something useful - I *have* found a
tab error in tests/unit_tests/openvpn/Makefile.am, though, and fixed
on the fly :-)


Your patch has been applied to the master branch.

commit a5a30ec311ce9d0dbcd6162daab97a225189d570
Author: Arne Schwabe
Date:   Wed Sep 21 12:49:30 2022 +0200

     Add unit test for reliable_get_num_output_sequenced_available

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20220921104930.3452270-3-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25292.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index 65cf9549c..c7846f3e6 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -61,7 +61,10 @@  packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c mock_msg.h \
 	$(openvpn_srcdir)/buffer.c \
 	$(openvpn_srcdir)/otime.c \
 	$(openvpn_srcdir)/packet_id.c \
-	$(openvpn_srcdir)/platform.c
+	$(openvpn_srcdir)/platform.c \
+	$(openvpn_srcdir)/reliable.c \
+    $(openvpn_srcdir)/session_id.c
+
 
 pkt_testdriver_CFLAGS  = @TEST_CFLAGS@ \
 	-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir)
diff --git a/tests/unit_tests/openvpn/mock_get_random.c b/tests/unit_tests/openvpn/mock_get_random.c
index d0d2574ae..787b5e33e 100644
--- a/tests/unit_tests/openvpn/mock_get_random.c
+++ b/tests/unit_tests/openvpn/mock_get_random.c
@@ -26,6 +26,7 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <setjmp.h>
+#include <stdint.h>
 #include <cmocka.h>
 
 unsigned long
@@ -34,3 +35,12 @@  get_random(void)
     /* rand() is not very random, but it's C99 and this is just for testing */
     return rand();
 }
+
+void
+prng_bytes(uint8_t *output, int len)
+{
+    for (int i = 0; i < len; i++)
+    {
+        output[i] = rand();
+    }
+}
diff --git a/tests/unit_tests/openvpn/test_packet_id.c b/tests/unit_tests/openvpn/test_packet_id.c
index a3d4db285..c610d9727 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -36,6 +36,7 @@ 
 #include <cmocka.h>
 
 #include "packet_id.h"
+#include "reliable.h"
 
 #include "mock_msg.h"
 
@@ -156,6 +157,59 @@  test_packet_id_write_long_wrap(void **state)
     assert_true(data->test_buf_data.buf_time == htonl(now));
 }
 
+static void
+test_get_num_output_sequenced_available(void **state)
+{
+
+    struct reliable *rel = malloc(sizeof(struct reliable));
+    reliable_init(rel, 100, 50, 8, false);
+
+    rel->array[5].active = true;
+    rel->array[5].packet_id = 100;
+
+    rel->packet_id = 103;
+
+    assert_int_equal(5, reliable_get_num_output_sequenced_available(rel));
+
+    rel->array[6].active = true;
+    rel->array[6].packet_id = 97;
+    assert_int_equal(2, reliable_get_num_output_sequenced_available(rel));
+
+    /* test ids close to int/unsigned int barrier */
+
+    rel->array[5].active = true;
+    rel->array[5].packet_id = (0x80000000u -3);
+    rel->array[6].active = false;
+    rel->packet_id = (0x80000000u -1);
+
+    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
+
+    rel->array[5].active = true;
+    rel->array[5].packet_id = (0x80000000u -3);
+    rel->packet_id = 0x80000001u;
+
+    assert_int_equal(4, reliable_get_num_output_sequenced_available(rel));
+
+
+    /* test wrapping */
+    rel->array[5].active = true;
+    rel->array[5].packet_id = (0xffffffffu -3);
+    rel->array[6].active = false;
+    rel->packet_id = (0xffffffffu - 1);
+
+    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
+
+    rel->array[2].packet_id = 0;
+    rel->array[2].active = true;
+
+    assert_int_equal(6, reliable_get_num_output_sequenced_available(rel));
+
+    rel->packet_id = 3;
+    assert_int_equal(1, reliable_get_num_output_sequenced_available(rel));
+
+    reliable_free(rel);
+}
+
 int
 main(void)
 {
@@ -178,6 +232,7 @@  main(void)
         cmocka_unit_test_setup_teardown(test_packet_id_write_long_wrap,
                                         test_packet_id_write_setup,
                                         test_packet_id_write_teardown),
+        cmocka_unit_test(test_get_num_output_sequenced_available)
     };
 
     return cmocka_run_group_tests_name("packet_id tests", tests, NULL, NULL);