[LTP] [PATCH 0/3] Improve test_1_to_1_initmsg_connect SCTP test

[LTP] [PATCH 0/3] Improve test_1_to_1_initmsg_connect SCTP test

[Martin Doucha]

One of the SCTP tests often fails on older kernels on some platforms
due to memory allocation issues. This patchset ports the test to the new LTP
library and fixes the failures by splitting the test into two test cases:
1. Functional test with reduced number of connection streams
2. Stress test with original (maximum) number of connection streams

The stress test accepts ENOMEM error in connect() as success.

This patchset also adds SAFE_ACCEPT() function to the LTP library to simplify
network tests.

Martin Doucha (3):
  Add SAFE_ACCEPT() to LTP safe net library
  Port test_1_to_1_initmsg_connect (SCTP) to new API
  Split SCTP initmsg test into two test cases

 include/safe_net_fn.h                         |   3 +
 include/tst_safe_net.h                        |   3 +
 lib/safe_net.c                                |  16 ++
 .../func_tests/test_1_to_1_initmsg_connect.c  | 161 +++++++++---------
 4 files changed, 98 insertions(+), 85 deletions(-)

-- 
2.24.0

[LTP] [PATCH 1/3] Add SAFE_ACCEPT() to LTP safe net library

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/safe_net_fn.h  |  3 +++
 include/tst_safe_net.h |  3 +++
 lib/safe_net.c         | 16 ++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/include/safe_net_fn.h b/include/safe_net_fn.h
index fdbb3791c..2fda11fab 100644
--- a/include/safe_net_fn.h
+++ b/include/safe_net_fn.h
@@ -57,6 +57,9 @@ int safe_bind(const char *file, const int lineno, void (cleanup_fn)(void),
 int safe_listen(const char *file, const int lineno, void (cleanup_fn)(void),
 		int socket, int backlog);
 
+int safe_accept(const char *file, const int lineno, void (cleanup_fn)(void),
+		int sockfd, struct sockaddr *addr, socklen_t *addrlen);
+
 int safe_connect(const char *file, const int lineno, void (cleanup_fn)(void),
 		 int sockfd, const struct sockaddr *addr, socklen_t addrlen);
 
diff --git a/include/tst_safe_net.h b/include/tst_safe_net.h
index d49a36073..9bc257ce8 100644
--- a/include/tst_safe_net.h
+++ b/include/tst_safe_net.h
@@ -64,6 +64,9 @@
 #define SAFE_LISTEN(socket, backlog) \
 	safe_listen(__FILE__, __LINE__, NULL, socket, backlog)
 
+#define SAFE_ACCEPT(sockfd, addr, addrlen) \
+	safe_accept(__FILE__, __LINE__, NULL, sockfd, addr, addrlen)
+
 #define SAFE_CONNECT(sockfd, addr, addrlen) \
 	safe_connect(__FILE__, __LINE__, NULL, sockfd, addr, addrlen)
 
diff --git a/lib/safe_net.c b/lib/safe_net.c
index abebd1899..499368007 100644
--- a/lib/safe_net.c
+++ b/lib/safe_net.c
@@ -322,6 +322,22 @@ int safe_listen(const char *file, const int lineno, void (cleanup_fn)(void),
 	return rval;
 }
 
+int safe_accept(const char *file, const int lineno, void (cleanup_fn)(void),
+		int sockfd, struct sockaddr *addr, socklen_t *addrlen)
+{
+	int rval;
+
+	rval = accept(sockfd, addr, addrlen);
+
+	if (rval < 0) {
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			"%s:%d: accept(%d, %p, %d) failed", file, lineno,
+			sockfd, addr, *addrlen);
+	}
+
+	return rval;
+}
+
 int safe_connect(const char *file, const int lineno, void (cleanup_fn)(void),
 		 int sockfd, const struct sockaddr *addr, socklen_t addrlen)
 {
-- 
2.24.0

[LTP] [PATCH 1/3] Add SAFE_ACCEPT() to LTP safe net library

[Petr Vorel]

Hi Martin,

LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH 2/3] Port test_1_to_1_initmsg_connect (SCTP) to new API

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../func_tests/test_1_to_1_initmsg_connect.c  | 109 +++++++-----------
 1 file changed, 42 insertions(+), 67 deletions(-)

diff --git a/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
index 8efb4f59c..d85474ad6 100644
--- a/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
+++ b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
@@ -1,38 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /* SCTP kernel Implementation
  * Copyright (c) 2003 Hewlett-Packard Development Company, L.P
  * (C) Copyright IBM Corp. 2004
  *
  * When init timeout is set to zero, a connect () crashed the system. This case
  * tests the fix for the same.
- *
- * The SCTP implementation is free software;
- * you can redistribute it and/or modify it under the terms of
- * the GNU General Public License as published by
- * the Free Software Foundation; either version 2, or (at your option)
- * any later version.
- *
- * The SCTP implementation is distributed in the hope that it
- * will be useful, but WITHOUT ANY WARRANTY; without even the implied
- *                 ************************
- * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- * See the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GNU CC; see the file COPYING.  If not, write to
- * the Free Software Foundation, 59 Temple Place - Suite 330,
- * Boston, MA 02111-1307, USA.
- *
- * Please send any bug reports or fixes you make to the
- * email address(es):
- *    lksctp developers <lksctp-developers@lists.sourceforge.net>
- *
- * Or submit a bug report through the following website:
- *    http://www.sf.net/projects/lksctp
- *
- *
- * Any bugs reported given to us we will try to fix... any fixes shared will
- * be incorporated into the next SCTP release.
- *
  */
 #include <stdio.h>
 #include <unistd.h>
@@ -43,38 +15,32 @@
 #include <sys/socket.h>
 #include <netinet/in.h>         /* for sockaddr_in */
 #include <arpa/inet.h>
-#include <errno.h>
 #include <netinet/sctp.h>
-#include <sys/uio.h>
-#include <sctputil.h> 
+#include "tst_test.h"
+#include "tst_net.h"
+
+#ifdef PROT_SOCK
+#define SCTP_TESTPORT_1 PROT_SOCK
+#else
+#define SCTP_TESTPORT_1 1024
+#endif
 
-char *TCID = __FILE__;
-int TST_TOTAL = 1;
-int TST_CNT = 0;
+#define SCTP_IP_LOOPBACK  htonl(0x7f000001)
 
-int 
-main (int argc, char **argv)
+static void test_sctp(void)
 {
-	int sk1, sk2, sk3, pf_class;
+	int sk1, sk2, sk3, pf_class, msglen;
 	socklen_t len;
 	struct sockaddr_in lstn_addr, acpt_addr;
 	struct sockaddr_in conn_addr;
-	char * buffer_rcv;
+	char *buffer_rcv;
 	struct sctp_initmsg sinmsg;
-	char *message = "Hello World!\n";
+	const char *message = "Hello World!\n";
 
-	/* Rather than fflush() throughout the code, set stdout to
-	 * be unbuffered.
-	 */
-	setvbuf(stdout, NULL, _IONBF, 0);
-	setvbuf(stderr, NULL, _IONBF, 0);
-
-	/* Opening the socket*/
-	
 	pf_class = PF_INET;
 
-	sk1 = test_socket(pf_class, SOCK_STREAM, IPPROTO_SCTP);
-	sk3 = test_socket(pf_class, SOCK_STREAM, IPPROTO_SCTP);
+	sk1 = SAFE_SOCKET(pf_class, SOCK_STREAM, IPPROTO_SCTP);
+	sk3 = SAFE_SOCKET(pf_class, SOCK_STREAM, IPPROTO_SCTP);
 
         conn_addr.sin_family = AF_INET;
         conn_addr.sin_addr.s_addr = SCTP_IP_LOOPBACK;
@@ -84,37 +50,46 @@ main (int argc, char **argv)
         lstn_addr.sin_addr.s_addr = SCTP_IP_LOOPBACK;
         lstn_addr.sin_port = htons(SCTP_TESTPORT_1);
 
-	test_bind(sk3, (struct sockaddr *) &lstn_addr, sizeof(lstn_addr));
+	SAFE_BIND(sk3, (struct sockaddr *) &lstn_addr, sizeof(lstn_addr));
 
 	len = sizeof(struct sctp_initmsg);
 	sinmsg.sinit_num_ostreams = 65535;
 	sinmsg.sinit_max_instreams = 10;
 	sinmsg.sinit_max_attempts = 1;
 	sinmsg.sinit_max_init_timeo = 0;
-	test_setsockopt(sk1, SCTP_INITMSG, &sinmsg, len);
+	SAFE_SETSOCKOPT(sk1, SOL_SCTP, SCTP_INITMSG, &sinmsg, len);
 	sinmsg.sinit_num_ostreams = 10;
 	sinmsg.sinit_max_instreams = 65535;
-	test_setsockopt(sk3, SCTP_INITMSG, &sinmsg, len);
+	SAFE_SETSOCKOPT(sk3, SOL_SCTP, SCTP_INITMSG, &sinmsg, len);
 
-	test_listen(sk3, 1);
+	SAFE_LISTEN(sk3, 1);
 
 	len = sizeof(struct sockaddr_in);
-	test_connect(sk1, (struct sockaddr *) &conn_addr, len);
+	SAFE_CONNECT(sk1, (struct sockaddr *) &conn_addr, len);
+	sk2 = SAFE_ACCEPT(sk3, (struct sockaddr *) &acpt_addr, &len);
 
-	sk2 = test_accept(sk3, (struct sockaddr *) &acpt_addr, &len);
+	msglen = strlen(message) + 1;
+	TEST(sctp_sendmsg(sk1, message, msglen, (struct sockaddr *)&conn_addr,
+		len, 0, 0, 65534, 0, 0));
 
-	test_sctp_sendmsg(sk1, message, strlen(message) + 1,
-			  (struct sockaddr *)&conn_addr, len,
-			  0, 0, 65534, 0, 0);
+	if (TST_RET != msglen) {
+		tst_brk(TBROK | TTERRNO, "sctp_sendmsg() failed");
+	}
 
-	buffer_rcv = malloc(100);
-	test_recv(sk2, buffer_rcv, (strlen(message) + 1), MSG_NOSIGNAL);
+	buffer_rcv = malloc(msglen);
+	TEST(recv(sk2, buffer_rcv, msglen, MSG_NOSIGNAL));
 
-	tst_resm(TPASS, "connect() with init timeout set to 0 - SUCCESS");
+	if (TST_RET != msglen) {
+		tst_res(TFAIL | TTERRNO, "recv() failed");
+	} else {
+		tst_res(TPASS, "connect() with init timeout set to 0 - SUCCESS");
+	}
 
-	close (sk1);
-	close (sk2);
-	close (sk3);
-	
-        return 0;
+	SAFE_CLOSE(sk1);
+	SAFE_CLOSE(sk2);
+	SAFE_CLOSE(sk3);
 }
+
+static struct tst_test test = {
+	.test_all = test_sctp,
+};
-- 
2.24.0

[LTP] [PATCH 2/3] Port test_1_to_1_initmsg_connect (SCTP) to new API

[Petr Vorel]

Hi Martin,

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thanks for the rewrite!

...
> +++ b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
...
> -	tst_resm(TPASS, "connect() with init timeout set to 0 - SUCCESS");
> +	if (TST_RET != msglen) {
> +		tst_res(TFAIL | TTERRNO, "recv() failed");
> +	} else {
> +		tst_res(TPASS, "connect() with init timeout set to 0 - SUCCESS");
If you don't mind, I'd remove " - SUCCESS" from the message before merge.
> +	}

Kind regards,
Petr

[LTP] [PATCH 2/3] Port test_1_to_1_initmsg_connect (SCTP) to new API

[Martin Doucha]

On 12/13/19 4:39 PM, Petr Vorel wrote:
>> +++ b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
> ...
>> -	tst_resm(TPASS, "connect() with init timeout set to 0 - SUCCESS");
>> +	if (TST_RET != msglen) {
>> +		tst_res(TFAIL | TTERRNO, "recv() failed");
>> +	} else {
>> +		tst_res(TPASS, "connect() with init timeout set to 0 - SUCCESS");
> If you don't mind, I'd remove " - SUCCESS" from the message before merge.

Please do.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

[LTP] [PATCH 2/3] Port test_1_to_1_initmsg_connect (SCTP) to new API

[Petr Vorel]

Hi Martin,

> >> -	tst_resm(TPASS, "connect() with init timeout set to 0 - SUCCESS");
> >> +	if (TST_RET != msglen) {
> >> +		tst_res(TFAIL | TTERRNO, "recv() failed");
> >> +	} else {
> >> +		tst_res(TPASS, "connect() with init timeout set to 0 - SUCCESS");
> > If you don't mind, I'd remove " - SUCCESS" from the message before merge.

> Please do.

Thanks your patch!
Merged with this change and copyright in utils/sctp/func_tests/test_1_to_1_initmsg_connect.c

Kind regards,
Petr

[LTP] [PATCH 2/3] Port test_1_to_1_initmsg_connect (SCTP) to new API

[Petr Vorel]

Hi Martin,

this also needs inline in tst_net.h
https://patchwork.ozlabs.org/patch/1209197/

which is not a surprise for you I guess.

Kind regards,
Petr

[LTP] [PATCH 3/3] Split SCTP initmsg test into two test cases

[Martin Doucha]

The original test allocates 65535 output streams which may fail on older
kernels due to lack of memory. Split the test into two test cases:
- allocate 10 output streams and accept no errors (functional test)
- allocate 65535 output streams and accept ENOMEM (stress test)

Also clean up some unnecessary code and check that the test message is
transferred correctly.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../func_tests/test_1_to_1_initmsg_connect.c  | 56 ++++++++++++-------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
index d85474ad6..a98e0c003 100644
--- a/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
+++ b/utils/sctp/func_tests/test_1_to_1_initmsg_connect.c
@@ -27,50 +27,64 @@
 
 #define SCTP_IP_LOOPBACK  htonl(0x7f000001)
 
-static void test_sctp(void)
+static const struct test_case {
+	__u16 streams;
+	int accept_err;
+} testcase_list[] = {
+	{10, 0},
+	{65535, ENOMEM}
+};
+
+static void test_sctp(unsigned int n)
 {
-	int sk1, sk2, sk3, pf_class, msglen;
+	int sk1, sk2, sk3, msglen;
 	socklen_t len;
 	struct sockaddr_in lstn_addr, acpt_addr;
-	struct sockaddr_in conn_addr;
 	char *buffer_rcv;
 	struct sctp_initmsg sinmsg;
 	const char *message = "Hello World!\n";
+	const struct test_case *tc = testcase_list + n;
 
-	pf_class = PF_INET;
-
-	sk1 = SAFE_SOCKET(pf_class, SOCK_STREAM, IPPROTO_SCTP);
-	sk3 = SAFE_SOCKET(pf_class, SOCK_STREAM, IPPROTO_SCTP);
+	tst_res(TINFO, "Running test with %u streams", tc->streams);
 
-        conn_addr.sin_family = AF_INET;
-        conn_addr.sin_addr.s_addr = SCTP_IP_LOOPBACK;
-        conn_addr.sin_port = htons(SCTP_TESTPORT_1);
+	sk1 = SAFE_SOCKET(PF_INET, SOCK_STREAM, IPPROTO_SCTP);
+	sk3 = SAFE_SOCKET(PF_INET, SOCK_STREAM, IPPROTO_SCTP);
 
-        lstn_addr.sin_family = AF_INET;
-        lstn_addr.sin_addr.s_addr = SCTP_IP_LOOPBACK;
-        lstn_addr.sin_port = htons(SCTP_TESTPORT_1);
+	lstn_addr.sin_family = AF_INET;
+	lstn_addr.sin_addr.s_addr = SCTP_IP_LOOPBACK;
+	lstn_addr.sin_port = htons(SCTP_TESTPORT_1);
 
 	SAFE_BIND(sk3, (struct sockaddr *) &lstn_addr, sizeof(lstn_addr));
 
 	len = sizeof(struct sctp_initmsg);
-	sinmsg.sinit_num_ostreams = 65535;
+	sinmsg.sinit_num_ostreams = tc->streams;
 	sinmsg.sinit_max_instreams = 10;
 	sinmsg.sinit_max_attempts = 1;
 	sinmsg.sinit_max_init_timeo = 0;
 	SAFE_SETSOCKOPT(sk1, SOL_SCTP, SCTP_INITMSG, &sinmsg, len);
 	sinmsg.sinit_num_ostreams = 10;
-	sinmsg.sinit_max_instreams = 65535;
+	sinmsg.sinit_max_instreams = tc->streams;
 	SAFE_SETSOCKOPT(sk3, SOL_SCTP, SCTP_INITMSG, &sinmsg, len);
 
 	SAFE_LISTEN(sk3, 1);
 
 	len = sizeof(struct sockaddr_in);
-	SAFE_CONNECT(sk1, (struct sockaddr *) &conn_addr, len);
+	TEST(connect(sk1, (struct sockaddr *) &lstn_addr, len));
+
+	if (TST_RET == -1 && tc->accept_err && TST_ERR == tc->accept_err) {
+		tst_res(TPASS, "connect() failed in an acceptable way");
+		SAFE_CLOSE(sk1);
+		SAFE_CLOSE(sk3);
+		return;
+	} else if (TST_RET < 0) {
+		tst_brk(TBROK | TTERRNO, "connect() failed");
+	}
+
 	sk2 = SAFE_ACCEPT(sk3, (struct sockaddr *) &acpt_addr, &len);
 
 	msglen = strlen(message) + 1;
-	TEST(sctp_sendmsg(sk1, message, msglen, (struct sockaddr *)&conn_addr,
-		len, 0, 0, 65534, 0, 0));
+	TEST(sctp_sendmsg(sk1, message, msglen, (struct sockaddr *)&lstn_addr,
+		len, 0, 0, tc->streams - 1, 0, 0));
 
 	if (TST_RET != msglen) {
 		tst_brk(TBROK | TTERRNO, "sctp_sendmsg() failed");
@@ -79,17 +93,19 @@ static void test_sctp(void)
 	buffer_rcv = malloc(msglen);
 	TEST(recv(sk2, buffer_rcv, msglen, MSG_NOSIGNAL));
 
-	if (TST_RET != msglen) {
+	if (TST_RET != msglen || strncmp(buffer_rcv, message, msglen)) {
 		tst_res(TFAIL | TTERRNO, "recv() failed");
 	} else {
 		tst_res(TPASS, "connect() with init timeout set to 0 - SUCCESS");
 	}
 
+	free(buffer_rcv);
 	SAFE_CLOSE(sk1);
 	SAFE_CLOSE(sk2);
 	SAFE_CLOSE(sk3);
 }
 
 static struct tst_test test = {
-	.test_all = test_sctp,
+	.test = test_sctp,
+	.tcnt = ARRAY_SIZE(testcase_list),
 };
-- 
2.24.0

[LTP] [PATCH 3/3] Split SCTP initmsg test into two test cases

[Petr Vorel]

Hi Martin,

> The original test allocates 65535 output streams which may fail on older
> kernels due to lack of memory. Split the test into two test cases:
> - allocate 10 output streams and accept no errors (functional test)
> - allocate 65535 output streams and accept ENOMEM (stress test)

> Also clean up some unnecessary code and check that the test message is
> transferred correctly.

Great idea, thanks for implementing it.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr