[LTP] [RESEND 1/2] network/lib6/asapi_02: Fix T_WILLBLOCK/T_WILLPASS no FAIL/PASS bug

[LTP] [RESEND 1/2] network/lib6/asapi_02: Fix T_WILLBLOCK/T_WILLPASS no FAIL/PASS bug

[Yang Xu]

Currently, icmp6_ft will skip check between rv and expetec result when testing T_WILLBLOCK/T_WILLPASS case.

the result log as below:
asapi_02    1  TPASS  :  ICMP6_FILTER_SETPASS s 20 f 20
asapi_02    2  TPASS  :  ICMP6_FILTER_SETPASS s 20 f 21
asapi_02    3  TPASS  :  ICMP6_FILTER_SETBLOCK s 20 f 20
asapi_02    4  TPASS  :  ICMP6_FILTER_SETBLOCK s 20 f 21
asapi_02    5  TPASS  :  ICMP6_FILTER_PASSALL s 20
asapi_02    6  TPASS  :  ICMP6_FILTER_PASSALL s 20
asapi_02    7  TPASS  :  ICMP6_FILTER_BLOCKALL s 20
asapi_02    8  TPASS  :  ICMP6_FILTER_BLOCKALL s 20

after removing the wrong else judgment, the result log as below:
asapi_02    1  TPASS  :  ICMP6_FILTER_SETPASS s 20 f 20
asapi_02    2  TPASS  :  ICMP6_FILTER_SETPASS s 20 f 21
asapi_02    3  TPASS  :  ICMP6_FILTER_SETBLOCK s 20 f 20
asapi_02    4  TPASS  :  ICMP6_FILTER_SETBLOCK s 20 f 21
asapi_02    5  TPASS  :  ICMP6_FILTER_PASSALL s 20
asapi_02    6  TPASS  :  ICMP6_FILTER_PASSALL s 20
asapi_02    7  TPASS  :  ICMP6_FILTER_BLOCKALL s 20
asapi_02    8  TPASS  :  ICMP6_FILTER_BLOCKALL s 20
asapi_02    9  TPASS  :  ICMP6_FILTER_WILLBLOCK s 20 f 21
asapi_02   10  TPASS  :  ICMP6_FILTER_WILLBLOCK s 20 f 20
asapi_02   11  TPASS  :  ICMP6_FILTER_WILLPASS s 20 f 21
asapi_02   12  TPASS  :  ICMP6_FILTER_WILLPASS s 22 f 22

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/network/lib6/asapi_02.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/testcases/network/lib6/asapi_02.c b/testcases/network/lib6/asapi_02.c
index f9843346c..13b3b2b70 100644
--- a/testcases/network/lib6/asapi_02.c
+++ b/testcases/network/lib6/asapi_02.c
@@ -246,8 +246,6 @@ static void icmp6_ft(void)
 			if (ic6_send1(ftab[i].ft_tname, ftab[i].ft_sndtype))
 				continue;
 			rv = ic6_recv1(ftab[i].ft_tname, sall, sf);
-		} else {
-			rv = -1;
 		}
 
 		if (rv < 0)
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [RESEND 2/2] network/lib6/asapi_02: Convert into new api

[Yang Xu]

1.make use of SAFE_RECV/SAFE_SENDTO macro
2.add detail description for this case
3.use more meaningful variable name or struct name

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/network/lib6/asapi_02.c | 328 +++++++++++++++---------------
 1 file changed, 159 insertions(+), 169 deletions(-)

diff --git a/testcases/network/lib6/asapi_02.c b/testcases/network/lib6/asapi_02.c
index 13b3b2b70..bbc8b9a73 100644
--- a/testcases/network/lib6/asapi_02.c
+++ b/testcases/network/lib6/asapi_02.c
@@ -1,102 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2015 Fujitsu Ltd.
+ * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
  * Copyright (c) International Business Machines  Corp., 2001
  *
- * This program 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 3 of the License, or
- * (at your option) any later version.
+ * Author: David L Stevens
+ */
+
+/*\
+ * [Description]
  *
- * This program 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.
+ * It is a basic test for ICMP6_FILTER.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ * For ICMP6_FILTER usage, refer to the following url
+ * https://man.openbsd.org/icmp6
  *
- * Author: David L Stevens
+ * Because of the extra functionality of ICMPv6 in comparison to ICMPv4, a
+ * larger number of messages may be potentially received on an ICMPv6 socket.
+ * Input filters may therefore be used to restrict input to a subset of the
+ * incoming ICMPv6 messages so only interesting messages are returned by the
+ * recv(2) family of calls to an application.
+
+ * The icmp6_filter structure may be used to refine the input message set
+ * according to the ICMPv6 type. By default, all messages types are allowed
+ * on newly created raw ICMPv6 sockets. The following macros may be used to
+ * refine the input set:
+ *
+ * Macros used to manipulate filter value
+ *
+ * void ICMP6_FILTER_SETPASSALL(struct icmp6_filter *filterp);
+ * Allow all incoming messages. filterp is modified to allow all message types.
+ *
+ * void ICMP6_FILTER_SETBLOCKALL(struct icmp6_filter *filterp);
+ * Ignore all incoming messages. filterp is modified to ignore all message types.
+ *
+ * void ICMP6_FILTER_SETPASS(int, struct icmp6_filter *filterp);
+ * Allow ICMPv6 messages with the given type. filterp is modified to allow such
+ * messages.
+ *
+ * void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp);
+ * Ignore ICMPv6 messages with the given type. filterp is modified to ignore
+ * such messages.
+ *
+ * int ICMP6_FILTER_WILLPASS(int, const struct icmp6_filter *filterp);
+ * Determine if the given filter will allow an ICMPv6 message of the given type.
+ *
+ * int ICMP6_FILTER_WILLBLOCK(int, const struct icmp6_filter *);
+ * Determine if the given filter will ignore an ICMPv6 message of the given type.
+ *
+ * The getsockopt(2) and setsockopt(2) calls may be used to obtain and install
+ * the filter on ICMPv6 sockets at option level IPPROTO_ICMPV6 and name ICMP6_FILTER
+ * with a pointer to the icmp6_filter structure as the option value.
  */
 
 #include <stdio.h>
 #include <unistd.h>
-#include <errno.h>
-
 #include <sys/wait.h>
 #include <sys/socket.h>
-
 #include <netinet/in.h>
 #include <netinet/ip6.h>
 #include <netinet/icmp6.h>
+#include "tst_test.h"
 
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "asapi_02";
-
-static void setup(void);
+static int sall = -1, sf = -1;
 
-static void icmp6_ft(void);
-
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); ++lc)
-		icmp6_ft();
-
-	tst_exit();
-}
-
-static void setup(void)
-{
-	TEST_PAUSE;
-	tst_require_root();
-}
-
-enum tt {
-	T_WILLPASS,
-	T_WILLBLOCK,
+enum filter_macro {
 	T_SETPASS,
 	T_SETBLOCK,
 	T_SETPASSALL,
-	T_SETBLOCKALL
+	T_SETBLOCKALL,
+	T_WILLBLOCK,
+	T_WILLPASS
 };
 
-static struct ftent {
-	char *ft_tname;			/* test name, for logging */
-	unsigned char ft_sndtype;	/* send type field */
-	unsigned char ft_flttype;	/* filter type field */
-	enum tt ft_test;		/* what macro to test */
-	int ft_expected;		/* packet should pass? */
-} ftab[] = {
-	{"ICMP6_FILTER_SETPASS s 20 f 20", 20, 20, T_SETPASS, 1},
-	{"ICMP6_FILTER_SETPASS s 20 f 21", 20, 21, T_SETPASS, 0},
-	{"ICMP6_FILTER_SETBLOCK s 20 f 20", 20, 20, T_SETBLOCK, 0},
-	{"ICMP6_FILTER_SETBLOCK s 20 f 21", 20, 21, T_SETBLOCK, 1},
-	{"ICMP6_FILTER_PASSALL s 20", 20, 0, T_SETPASSALL, 1},
-	{"ICMP6_FILTER_PASSALL s 20", 21, 0, T_SETPASSALL, 1},
-	{"ICMP6_FILTER_BLOCKALL s 20", 20, 0, T_SETBLOCKALL, 0},
-	{"ICMP6_FILTER_BLOCKALL s 20", 21, 0, T_SETBLOCKALL, 0},
-	{"ICMP6_FILTER_WILLBLOCK s 20 f 21", 20, 21, T_WILLBLOCK, 0},
-	{"ICMP6_FILTER_WILLBLOCK s 20 f 20", 20, 20, T_WILLBLOCK, 1},
-	{"ICMP6_FILTER_WILLPASS s 20 f 21", 20, 21, T_WILLPASS, 0},
-	{"ICMP6_FILTER_WILLPASS s 22 f 22", 22, 22, T_WILLPASS, 1},
+static struct tcase {
+	char *tname;
+	unsigned char send_type;
+	unsigned char filter_type;
+	enum filter_macro test_macro;
+	int pass_packet;
+} tcases[] = {
+	{"ICMP6_FILTER_SETPASS send type 20 filter type 20", 20, 20, T_SETPASS, 1},
+	{"ICMP6_FILTER_SETPASS send type 20 filter type 21", 20, 21, T_SETPASS, 0},
+	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 20", 20, 20, T_SETBLOCK, 0},
+	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 21", 20, 21, T_SETBLOCK, 1},
+	{"ICMP6_FILTER_PASSALL send type 20", 20, 0, T_SETPASSALL, 1},
+	{"ICMP6_FILTER_PASSALL send type 20", 21, 0, T_SETPASSALL, 1},
+	{"ICMP6_FILTER_BLOCKALL send type 20", 20, 0, T_SETBLOCKALL, 0},
+	{"ICMP6_FILTER_BLOCKALL send type 20", 21, 0, T_SETBLOCKALL, 0},
+	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 21", 20, 21, T_WILLBLOCK, 0},
+	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 20", 20, 20, T_WILLBLOCK, 1},
+	{"ICMP6_FILTER_WILLPASS send type 20 filter type 21", 20, 21, T_WILLPASS, 0},
+	{"ICMP6_FILTER_WILLPASS send type 22 filter type 22", 22, 22, T_WILLPASS, 1},
 };
 
-#define FTCOUNT	ARRAY_SIZE(ftab)
-
-static int ic6_send1(char *tname, unsigned char type)
+static void ic6_send(unsigned char type)
 {
 	struct sockaddr_in6 sin6;
 	struct icmp6_hdr ic6;
 	int s;
 
-	s = SAFE_SOCKET(NULL, AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
+	s = SAFE_SOCKET(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
 
 	memset(&ic6, 0, sizeof(ic6));
 	ic6.icmp6_type = type;
@@ -105,22 +108,15 @@ static int ic6_send1(char *tname, unsigned char type)
 	memset(&sin6, 0, sizeof(sin6));
 	sin6.sin6_family = AF_INET6;
 	sin6.sin6_addr = in6addr_loopback;
-	if (sendto(s, &ic6, sizeof(ic6), 0, (struct sockaddr *)&sin6,
-		   sizeof(sin6)) == -1) {
-		tst_resm(TBROK | TERRNO, "%s: sendto failed", tname);
-		return 1;
-	}
-	return 0;
+	SAFE_SENDTO(0, s, &ic6, sizeof(ic6), 0, (struct sockaddr *)&sin6, sizeof(sin6));
 }
 
-static int ic6_recv1(char *tname, int sall, int sf)
+static int ic6_recv(void)
 {
 	fd_set readfds, readfds_saved;
 	struct timeval tv;
-	int maxfd, nfds;
-	int gotall, gotone;
-	int cc;
-	static unsigned char rbuf[2048];
+	int maxfd, nfds, gotall, gotone;
+	unsigned char rbuf[2048];
 
 	tv.tv_sec = 0;
 	tv.tv_usec = 250000;
@@ -133,127 +129,121 @@ static int ic6_recv1(char *tname, int sall, int sf)
 	memcpy(&readfds, &readfds_saved, sizeof(readfds));
 
 	gotall = gotone = 0;
-	/*
-	 * Note: this relies on linux-specific behavior (select
-	 * updating tv with time elapsed)
-	 */
+
 	while (!gotall || !gotone) {
 		struct icmp6_hdr *pic6 = (struct icmp6_hdr *)rbuf;
 
 		nfds = select(maxfd + 1, &readfds, 0, 0, &tv);
 		if (nfds == 0)
-			break;	/* timed out */
+			break;
 		if (nfds < 0) {
 			if (errno == EINTR)
 				continue;
-			tst_resm(TBROK | TERRNO, "%s: select failed", tname);
+			tst_brk(TBROK | TERRNO, "select failed");
 		}
 		if (FD_ISSET(sall, &readfds)) {
-			cc = recv(sall, rbuf, sizeof(rbuf), 0);
-			if (cc < 0) {
-				tst_resm(TBROK | TERRNO,
-					 "%s: recv(sall, ..) failed", tname);
-				return -1;
-			}
-			/* if packet check succeeds... */
+			SAFE_RECV(0, sall, rbuf, sizeof(rbuf), 0);
 			if (htonl(pic6->icmp6_data32[0]) == (uint32_t)getpid())
 				gotall = 1;
 		}
 		if (FD_ISSET(sf, &readfds)) {
-			cc = recv(sf, rbuf, sizeof(rbuf), 0);
-			if (cc < 0) {
-				tst_resm(TBROK | TERRNO,
-					 "%s: recv(sf, ..) failed", tname);
-				return -1;
-			}
-			/* if packet check succeeds... */
+			SAFE_RECV(0, sf, rbuf, sizeof(rbuf), 0);
 			if (htonl(pic6->icmp6_data32[0]) == (uint32_t)getpid())
 				gotone = 1;
 		}
 		memcpy(&readfds, &readfds_saved, sizeof(readfds));
 	}
 	if (!gotall) {
-		tst_resm(TBROK, "%s: recv all timed out", tname);
+		tst_res(TFAIL, "recv all time out");
 		return -1;
 	}
+
 	if (gotone)
 		return 1;
+
 	return 0;
 }
 
-/* functional tests */
-static void icmp6_ft(void)
+static void verify_icmp6_filter(unsigned int n)
 {
+	struct tcase *tc = &tcases[n];
 	struct icmp6_filter i6f;
-	int sall, sf;
-	unsigned int i;
-
-	sall = SAFE_SOCKET(NULL, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
-
-	ICMP6_FILTER_SETPASSALL(&i6f);
-	if (setsockopt(sall, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f,
-		       sizeof(i6f)) < 0) {
-		tst_resm(TBROK | TERRNO,
-			 "setsockopt pass all ICMP6_FILTER failed");
+	int rc;
+
+	tst_res(TINFO, "Testing %s", tc->tname);
+	switch (tc->test_macro) {
+	case T_SETPASS:
+		ICMP6_FILTER_SETBLOCKALL(&i6f);
+		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
+		break;
+	case T_SETPASSALL:
+		ICMP6_FILTER_SETPASSALL(&i6f);
+		break;
+	case T_SETBLOCK:
+		ICMP6_FILTER_SETPASSALL(&i6f);
+		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
+		break;
+	case T_SETBLOCKALL:
+		ICMP6_FILTER_SETBLOCKALL(&i6f);
+		break;
+	case T_WILLBLOCK:
+		ICMP6_FILTER_SETPASSALL(&i6f);
+		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
+		rc = ICMP6_FILTER_WILLBLOCK(tc->send_type, &i6f);
+		goto check_will_rc;
+	case T_WILLPASS:
+		ICMP6_FILTER_SETBLOCKALL(&i6f);
+		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
+		rc = ICMP6_FILTER_WILLPASS(tc->send_type, &i6f);
+		goto check_will_rc;
+	default:
+		tst_brk(TBROK, "unknown test type %d", tc->filter_type);
+		break;
 	}
+	SAFE_SETSOCKOPT(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
+	ic6_send(tc->send_type);
+	rc = ic6_recv();
+	if (rc < 0)
+		return;
+	if (rc != tc->pass_packet) {
+		tst_res(TFAIL, "%s packet type %d unexpectedly",
+				rc ? "pass" : "block", tc->send_type);
+	} else {
+		tst_res(TPASS, "%s packet type %d as expected",
+				tc->pass_packet ? "pass" : "block", tc->send_type);
+	}
+	return;
 
-	sf = SAFE_SOCKET(NULL, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
-
-	int rv;
+check_will_rc:
+	if (rc != tc->pass_packet)
+		tst_res(TFAIL, "rc %d != expected %d", rc, tc->pass_packet);
+	else
+		tst_res(TPASS, "expected rc %d", rc);
+}
 
-	for (i = 0; i < FTCOUNT; ++i) {
+static void setup(void)
+{
+	struct icmp6_filter i6f;
 
-		rv = -1;
+	sall = SAFE_SOCKET(PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
+	ICMP6_FILTER_SETPASSALL(&i6f);
+	SAFE_SETSOCKOPT(sall, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
 
-		switch (ftab[i].ft_test) {
-		case T_SETPASS:
-			ICMP6_FILTER_SETBLOCKALL(&i6f);
-			ICMP6_FILTER_SETPASS(ftab[i].ft_flttype, &i6f);
-			break;
-		case T_SETPASSALL:
-			ICMP6_FILTER_SETPASSALL(&i6f);
-			break;
-		case T_SETBLOCK:
-			ICMP6_FILTER_SETPASSALL(&i6f);
-			ICMP6_FILTER_SETBLOCK(ftab[i].ft_flttype, &i6f);
-			break;
-		case T_SETBLOCKALL:
-			ICMP6_FILTER_SETBLOCKALL(&i6f);
-			break;
-		case T_WILLBLOCK:
-			ICMP6_FILTER_SETPASSALL(&i6f);
-			ICMP6_FILTER_SETBLOCK(ftab[i].ft_flttype, &i6f);
-			rv = ICMP6_FILTER_WILLBLOCK(ftab[i].ft_sndtype, &i6f);
-			break;
-		case T_WILLPASS:
-			ICMP6_FILTER_SETBLOCKALL(&i6f);
-			ICMP6_FILTER_SETPASS(ftab[i].ft_flttype, &i6f);
-			rv = ICMP6_FILTER_WILLPASS(ftab[i].ft_sndtype, &i6f);
-			break;
-		default:
-			tst_resm(TBROK, "%s: unknown test type %d",
-				 ftab[i].ft_tname, ftab[i].ft_test);
-			continue;
-		}
-		if (ftab[i].ft_test != T_WILLBLOCK &&
-		    ftab[i].ft_test != T_WILLPASS) {
-			if (setsockopt(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f,
-				       sizeof(i6f)) < 0) {
-				tst_resm(TFAIL | TERRNO,
-					 "setsockopt ICMP6_FILTER");
-				continue;
-			}
-			if (ic6_send1(ftab[i].ft_tname, ftab[i].ft_sndtype))
-				continue;
-			rv = ic6_recv1(ftab[i].ft_tname, sall, sf);
-		}
+	sf = SAFE_SOCKET(PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
+}
 
-		if (rv < 0)
-			continue;
-		if (rv != ftab[i].ft_expected)
-			tst_resm(TFAIL, "%s: rv %d != expected %d",
-				 ftab[i].ft_tname, rv, ftab[i].ft_expected);
-		else
-			tst_resm(TPASS, "%s", ftab[i].ft_tname);
-	}
+static void cleanup(void)
+{
+	if (sall > -1)
+		SAFE_CLOSE(sall);
+	if (sf > -1)
+		SAFE_CLOSE(sf);
 }
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_icmp6_filter,
+	.tcnt = ARRAY_SIZE(tcases)
+};
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [RESEND 2/2] network/lib6/asapi_02: Convert into new api

[Petr Vorel]

Hi Xu,

> 1.make use of SAFE_RECV/SAFE_SENDTO macro
> 2.add detail description for this case
> 3.use more meaningful variable name or struct name

...
> diff --git a/testcases/network/lib6/asapi_02.c b/testcases/network/lib6/asapi_02.c
...
> +/*\
> + * [Description]
>   *
> - * This program 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.
> + * It is a basic test for ICMP6_FILTER.
nit: I'd prefer just: Basic test for ICMP6_FILTER.
>   *
> - * You should have received a copy of the GNU General Public License
> - * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + * For ICMP6_FILTER usage, refer to the following url
> + * https://man.openbsd.org/icmp6

nit: how about just:
For ICMP6_FILTER usage, refer to: https://man.openbsd.org/icmp6.

(Besides URL is an abbreviation, thus should be upper case, it will be in our
docs on single line, thus looks bad without dot in the end.)

>   *
> - * Author: David L Stevens
> + * Because of the extra functionality of ICMPv6 in comparison to ICMPv4, a
> + * larger number of messages may be potentially received on an ICMPv6 socket.
> + * Input filters may therefore be used to restrict input to a subset of the
> + * incoming ICMPv6 messages so only interesting messages are returned by the
> + * recv(2) family of calls to an application.
> +
> + * The icmp6_filter structure may be used to refine the input message set
> + * according to the ICMPv6 type. By default, all messages types are allowed
> + * on newly created raw ICMPv6 sockets. The following macros may be used to
> + * refine the input set:
> + *
> + * Macros used to manipulate filter value
nit: this and previous sentence are strange to be put together.

> + *
> + * void ICMP6_FILTER_SETPASSALL(struct icmp6_filter *filterp);
> + * Allow all incoming messages. filterp is modified to allow all message types.
We copy paste functions with their descriptions from https://man.openbsd.org/icmp6,
I was thinking if it's really useful, but let's keep it.

This is inline in our docs, thus looks bad:

Therefore I'd remove ';' and add &ndash;
* void ICMP6_FILTER_SETPASSALL(struct icmp6_filter *filterp)
* &ndash; Allow all incoming messages. filterp is modified to allow all message types.

That will result in docs:
void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp) – Ignore ICMPv6 messages with the given type. filterp is modified to ignore such messages.
instead of
void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp); Ignore ICMPv6 messages with the given type. filterp is modified to ignore such messages.
(both on single line)

> + *
> + * void ICMP6_FILTER_SETBLOCKALL(struct icmp6_filter *filterp);
> + * Ignore all incoming messages. filterp is modified to ignore all message types.
> + *
> + * void ICMP6_FILTER_SETPASS(int, struct icmp6_filter *filterp);
> + * Allow ICMPv6 messages with the given type. filterp is modified to allow such
> + * messages.
> + *
> + * void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp);
> + * Ignore ICMPv6 messages with the given type. filterp is modified to ignore
> + * such messages.
> + *
> + * int ICMP6_FILTER_WILLPASS(int, const struct icmp6_filter *filterp);
> + * Determine if the given filter will allow an ICMPv6 message of the given type.
> + *
> + * int ICMP6_FILTER_WILLBLOCK(int, const struct icmp6_filter *);
> + * Determine if the given filter will ignore an ICMPv6 message of the given type.
> + *
> + * The getsockopt(2) and setsockopt(2) calls may be used to obtain and install
> + * the filter on ICMPv6 sockets at option level IPPROTO_ICMPV6 and name ICMP6_FILTER
> + * with a pointer to the icmp6_filter structure as the option value.
>   */

NOTE: from system headers only <netinet/icmp6.h> is needed for compilation.
While some of headers are needed but included by library headers, at least
<sys/wait.h> is not needed.

> +#include "tst_test.h"
...
> -enum tt {
> -	T_WILLPASS,
> -	T_WILLBLOCK,
> +enum filter_macro {
>  	T_SETPASS,
>  	T_SETBLOCK,
>  	T_SETPASSALL,
> -	T_SETBLOCKALL
> +	T_SETBLOCKALL,
> +	T_WILLBLOCK,
> +	T_WILLPASS
>  };
...
> +static struct tcase {
> +	char *tname;
> +	unsigned char send_type;
> +	unsigned char filter_type;
> +	enum filter_macro test_macro;
> +	int pass_packet;
> +} tcases[] = {
> +	{"ICMP6_FILTER_SETPASS send type 20 filter type 20", 20, 20, T_SETPASS, 1},

First, I wonder what are these cryptic numbers 20, 21, 22.
It'd be great to replace numbers with constants.

For maybe one of these:

$ grep -E -e define.*[[:space:]]20$ -e define.*0x14 include/net*
include/netinet/igmp.h:#define IGMP_PIM                 0x14
include/netinet/in.h:#define IP_ORIGDSTADDR     20
include/netinet/in.h:#define IP_MAX_MEMBERSHIPS              20
include/netinet/in.h:#define IPV6_JOIN_GROUP         20
...

$ grep -E -e define.*[[:space:]]21$ -e define.*0x15 include/net*
include/netinet/igmp.h:#define IGMP_TRACE                       0x15
include/netinet/in.h:#define IP_MINTTL          21
include/netinet/in.h:#define IPV6_LEAVE_GROUP        21
...

$ grep -E -e define.*[[:space:]]22$ -e define.*0x16 include/net*
include/netinet/igmp.h:#define IGMP_V2_MEMBERSHIP_REPORT        0x16
include/netinet/in.h:#define IPPROTO_IDP      22
include/netinet/in.h:#define IP_NODEFRAG        22
include/netinet/in.h:#define IPV6_ROUTER_ALERT       22
...


> +	{"ICMP6_FILTER_SETPASS send type 20 filter type 21", 20, 21, T_SETPASS, 0},
> +	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 20", 20, 20, T_SETBLOCK, 0},
> +	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 21", 20, 21, T_SETBLOCK, 1},
> +	{"ICMP6_FILTER_PASSALL send type 20", 20, 0, T_SETPASSALL, 1},
> +	{"ICMP6_FILTER_PASSALL send type 20", 21, 0, T_SETPASSALL, 1},
Here is mistake: you claim "send type 20", but it's actually 21. That's actually
error from original code (copy pasted by you).

> +	{"ICMP6_FILTER_BLOCKALL send type 20", 20, 0, T_SETBLOCKALL, 0},
> +	{"ICMP6_FILTER_BLOCKALL send type 20", 21, 0, T_SETBLOCKALL, 0},
And here as well.

That could be avoided, if values are assigned with help of macro
(code below would require to rename enum values).

enum filter_macro {
	FILTER_SETPASS,
	FILTER_SETBLOCK,
	FILTER_PASSALL,
	FILTER_BLOCKALL,
	FILTER_WILLBLOCK,
	FILTER_WILLPASS
};

#define DESC(x, y, z) .tname = "ICMP6_" #x ", send type: " #y ", filter type: " \
	#z, .send_type = y, .filter_type = z, .test_macro = x

static struct tcase {
	char *tname;
	unsigned char send_type;
	unsigned char filter_type;
	enum filter_macro test_macro;
	int pass_packet;
} tcases[] = {
	{DESC(FILTER_SETPASS, 20, 20), .pass_packet = 1},
	{DESC(FILTER_SETPASS, 20, 21)},
	{DESC(FILTER_SETBLOCK, 20, 20)},
	{DESC(FILTER_SETBLOCK, 20, 21), .pass_packet = 1},
	{DESC(FILTER_PASSALL, 20, 20), .pass_packet = 1},
	{DESC(FILTER_PASSALL, 21, 0), .pass_packet = 1},
	{DESC(FILTER_BLOCKALL, 20, 0)},
	{DESC(FILTER_BLOCKALL, 21, 0)},
	{DESC(FILTER_WILLBLOCK, 20, 21)},
	{DESC(FILTER_WILLBLOCK, 20, 20), .pass_packet = 1},
	{DESC(FILTER_WILLPASS, 20, 21)},
	{DESC(FILTER_WILLPASS, 22, 22), .pass_packet = 1},
};
> +	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 21", 20, 21, T_WILLBLOCK, 0},
> +	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 20", 20, 20, T_WILLBLOCK, 1},
> +	{"ICMP6_FILTER_WILLPASS send type 20 filter type 21", 20, 21, T_WILLPASS, 0},
> +	{"ICMP6_FILTER_WILLPASS send type 22 filter type 22", 22, 22, T_WILLPASS, 1},
>  };

...
> +static void verify_icmp6_filter(unsigned int n)
>  {
> +	struct tcase *tc = &tcases[n];
>  	struct icmp6_filter i6f;
...
> +	int rc;
> +
> +	tst_res(TINFO, "Testing %s", tc->tname);
Please add space here (readability)
> +	switch (tc->test_macro) {
> +	case T_SETPASS:
> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
> +		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
> +		break;
> +	case T_SETPASSALL:
> +		ICMP6_FILTER_SETPASSALL(&i6f);
> +		break;
> +	case T_SETBLOCK:
> +		ICMP6_FILTER_SETPASSALL(&i6f);
> +		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
> +		break;
> +	case T_SETBLOCKALL:
> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
> +		break;
> +	case T_WILLBLOCK:
> +		ICMP6_FILTER_SETPASSALL(&i6f);
> +		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
> +		rc = ICMP6_FILTER_WILLBLOCK(tc->send_type, &i6f);
> +		goto check_will_rc;
> +	case T_WILLPASS:
> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
> +		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
> +		rc = ICMP6_FILTER_WILLPASS(tc->send_type, &i6f);
> +		goto check_will_rc;
I'm not really a big fun of big switch, when some parts need goto.
TST_EXP_EXPR() and return can help to avoid it (see below).


> +	default:
> +		tst_brk(TBROK, "unknown test type %d", tc->filter_type);
> +		break;
>  	}
> +	SAFE_SETSOCKOPT(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
> +	ic6_send(tc->send_type);
> +	rc = ic6_recv();
> +	if (rc < 0)
> +		return;
And please space here.
> +	if (rc != tc->pass_packet) {
> +		tst_res(TFAIL, "%s packet type %d unexpectedly",
> +				rc ? "pass" : "block", tc->send_type);
> +	} else {
> +		tst_res(TPASS, "%s packet type %d as expected",
> +				tc->pass_packet ? "pass" : "block", tc->send_type);
> +	}
This can be replaced by TST_EXP_EXPR().
> +	return;

> -	sf = SAFE_SOCKET(NULL, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
> -
> -	int rv;
> +check_will_rc:
> +	if (rc != tc->pass_packet)
> +		tst_res(TFAIL, "rc %d != expected %d", rc, tc->pass_packet);
> +	else
> +		tst_res(TPASS, "expected rc %d", rc);
> +}

How about this? (goto is gone, TST_EXP_EXPR() simplifies the code).

	switch (tc->test_macro) {
	case FILTER_SETPASS:
		ICMP6_FILTER_SETBLOCKALL(&i6f);
		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
		break;
	case FILTER_PASSALL:
		ICMP6_FILTER_SETPASSALL(&i6f);
		break;
	case FILTER_SETBLOCK:
		ICMP6_FILTER_SETPASSALL(&i6f);
		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
		break;
	case FILTER_BLOCKALL:
		ICMP6_FILTER_SETBLOCKALL(&i6f);
		break;
	case FILTER_WILLBLOCK:
		ICMP6_FILTER_SETPASSALL(&i6f);
		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
		rc = ICMP6_FILTER_WILLBLOCK(tc->send_type, &i6f);
		TST_EXP_EXPR(rc == tc->pass_packet, "%d (%d)", tc->pass_packet, rc);
		return;
	case FILTER_WILLPASS:
		ICMP6_FILTER_SETBLOCKALL(&i6f);
		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
		rc = ICMP6_FILTER_WILLPASS(tc->send_type, &i6f);
		TST_EXP_EXPR(rc == tc->pass_packet, "%d (%d)", tc->pass_packet, rc);
		return;
	default:
		tst_brk(TBROK, "unknown test type %d", tc->filter_type);
		break;
	}

	SAFE_SETSOCKOPT(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
	ic6_send(tc->send_type);
	rc = ic6_recv();
	if (rc < 0)
		return;

	TST_EXP_EXPR(rc == tc->pass_packet, "%s packet type %d",
				 rc ? "pass" : "block", tc->send_type);
}
...
> +static void cleanup(void)
> +{
> +	if (sall > -1)
> +		SAFE_CLOSE(sall);
Please add blank line here (readability).
> +	if (sf > -1)
> +		SAFE_CLOSE(sf);
>  }
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test = verify_icmp6_filter,
> +	.tcnt = ARRAY_SIZE(tcases)
> +};

NOTE: most of my comments are implemented in my fork, branch
yang_xu/asapi_02.fixes. Feel free to reuse it.

https://raw.githubusercontent.com/pevik/ltp/yang_xu/asapi_02.fixes/testcases/network/lib6/asapi_02.c

If you like the changes and use them, you can merge with

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

or even with :)
Co-developed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [RESEND 2/2] network/lib6/asapi_02: Convert into new api

[Petr Vorel]

...
> > +static struct tcase {
> > +	char *tname;
> > +	unsigned char send_type;
> > +	unsigned char filter_type;
> > +	enum filter_macro test_macro;
> > +	int pass_packet;
> > +} tcases[] = {
> > +	{"ICMP6_FILTER_SETPASS send type 20 filter type 20", 20, 20, T_SETPASS, 1},

> First, I wonder what are these cryptic numbers 20, 21, 22.
> It'd be great to replace numbers with constants.

https://man.openbsd.org/icmp6 claims: "Suitable values are defined in
<netinet/icmp6.h>.", but I see none in their header:
http://fxr.watson.org/fxr/source/netinet/icmp6.h?v=OPENBSD;im=3

Kind regards,
Petr

> For maybe one of these:

> $ grep -E -e define.*[[:space:]]20$ -e define.*0x14 include/net*
> include/netinet/igmp.h:#define IGMP_PIM                 0x14
> include/netinet/in.h:#define IP_ORIGDSTADDR     20
> include/netinet/in.h:#define IP_MAX_MEMBERSHIPS              20
> include/netinet/in.h:#define IPV6_JOIN_GROUP         20
> ...

> $ grep -E -e define.*[[:space:]]21$ -e define.*0x15 include/net*
> include/netinet/igmp.h:#define IGMP_TRACE                       0x15
> include/netinet/in.h:#define IP_MINTTL          21
> include/netinet/in.h:#define IPV6_LEAVE_GROUP        21
> ...

> $ grep -E -e define.*[[:space:]]22$ -e define.*0x16 include/net*
> include/netinet/igmp.h:#define IGMP_V2_MEMBERSHIP_REPORT        0x16
> include/netinet/in.h:#define IPPROTO_IDP      22
> include/netinet/in.h:#define IP_NODEFRAG        22
> include/netinet/in.h:#define IPV6_ROUTER_ALERT       22
> ...
...

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [RESEND 2/2] network/lib6/asapi_02: Convert into new api

[Yang Xu (Fujitsu)]

Hi Petr

> Hi Xu,
> 
>> 1.make use of SAFE_RECV/SAFE_SENDTO macro
>> 2.add detail description for this case
>> 3.use more meaningful variable name or struct name
> 
> ...
>> diff --git a/testcases/network/lib6/asapi_02.c b/testcases/network/lib6/asapi_02.c
> ...
>> +/*\
>> + * [Description]
>>    *
>> - * This program 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.
>> + * It is a basic test for ICMP6_FILTER.
> nit: I'd prefer just: Basic test for ICMP6_FILTER.

Look simple and clear.
>>    *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + * For ICMP6_FILTER usage, refer to the following url
>> + * https://man.openbsd.org/icmp6
> 
> nit: how about just:
> For ICMP6_FILTER usage, refer to: https://man.openbsd.org/icmp6.
Of course.
> 
> (Besides URL is an abbreviation, thus should be upper case, it will be in our
> docs on single line, thus looks bad without dot in the end.)

Agree.
> 
>>    *
>> - * Author: David L Stevens
>> + * Because of the extra functionality of ICMPv6 in comparison to ICMPv4, a
>> + * larger number of messages may be potentially received on an ICMPv6 socket.
>> + * Input filters may therefore be used to restrict input to a subset of the
>> + * incoming ICMPv6 messages so only interesting messages are returned by the
>> + * recv(2) family of calls to an application.
>> +
>> + * The icmp6_filter structure may be used to refine the input message set
>> + * according to the ICMPv6 type. By default, all messages types are allowed
>> + * on newly created raw ICMPv6 sockets. The following macros may be used to
>> + * refine the input set:
>> + *
>> + * Macros used to manipulate filter value
> nit: this and previous sentence are strange to be put together.

Yes, will remove the previous sentence.

> 
>> + *
>> + * void ICMP6_FILTER_SETPASSALL(struct icmp6_filter *filterp);
>> + * Allow all incoming messages. filterp is modified to allow all message types.
> We copy paste functions with their descriptions from https://man.openbsd.org/icmp6,
> I was thinking if it's really useful, but let's keep it.
> 
> This is inline in our docs, thus looks bad:
> 
> Therefore I'd remove ';' and add &ndash;
> * void ICMP6_FILTER_SETPASSALL(struct icmp6_filter *filterp)
> * &ndash; Allow all incoming messages. filterp is modified to allow all message types.
> 
> That will result in docs:
> void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp) – Ignore ICMPv6 messages with the given type. filterp is modified to ignore such messages.
> instead of
> void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp); Ignore ICMPv6 messages with the given type. filterp is modified to ignore such messages.
> (both on single line)

OK.
> 
>> + *
>> + * void ICMP6_FILTER_SETBLOCKALL(struct icmp6_filter *filterp);
>> + * Ignore all incoming messages. filterp is modified to ignore all message types.
>> + *
>> + * void ICMP6_FILTER_SETPASS(int, struct icmp6_filter *filterp);
>> + * Allow ICMPv6 messages with the given type. filterp is modified to allow such
>> + * messages.
>> + *
>> + * void ICMP6_FILTER_SETBLOCK(int, struct icmp6_filter *filterp);
>> + * Ignore ICMPv6 messages with the given type. filterp is modified to ignore
>> + * such messages.
>> + *
>> + * int ICMP6_FILTER_WILLPASS(int, const struct icmp6_filter *filterp);
>> + * Determine if the given filter will allow an ICMPv6 message of the given type.
>> + *
>> + * int ICMP6_FILTER_WILLBLOCK(int, const struct icmp6_filter *);
>> + * Determine if the given filter will ignore an ICMPv6 message of the given type.
>> + *
>> + * The getsockopt(2) and setsockopt(2) calls may be used to obtain and install
>> + * the filter on ICMPv6 sockets at option level IPPROTO_ICMPV6 and name ICMP6_FILTER
>> + * with a pointer to the icmp6_filter structure as the option value.
>>    */
> 
> NOTE: from system headers only <netinet/icmp6.h> is needed for compilation.
> While some of headers are needed but included by library headers, at least
> <sys/wait.h> is not needed.

OK.
> 
>> +#include "tst_test.h"
> ...
>> -enum tt {
>> -	T_WILLPASS,
>> -	T_WILLBLOCK,
>> +enum filter_macro {
>>   	T_SETPASS,
>>   	T_SETBLOCK,
>>   	T_SETPASSALL,
>> -	T_SETBLOCKALL
>> +	T_SETBLOCKALL,
>> +	T_WILLBLOCK,
>> +	T_WILLPASS
>>   };
> ...
>> +static struct tcase {
>> +	char *tname;
>> +	unsigned char send_type;
>> +	unsigned char filter_type;
>> +	enum filter_macro test_macro;
>> +	int pass_packet;
>> +} tcases[] = {
>> +	{"ICMP6_FILTER_SETPASS send type 20 filter type 20", 20, 20, T_SETPASS, 1},
> 
> First, I wonder what are these cryptic numbers 20, 21, 22.
> It'd be great to replace numbers with constants.
> 
> For maybe one of these:
> 
> $ grep -E -e define.*[[:space:]]20$ -e define.*0x14 include/net*
> include/netinet/igmp.h:#define IGMP_PIM                 0x14
> include/netinet/in.h:#define IP_ORIGDSTADDR     20
> include/netinet/in.h:#define IP_MAX_MEMBERSHIPS              20
> include/netinet/in.h:#define IPV6_JOIN_GROUP         20
> ...
> 
> $ grep -E -e define.*[[:space:]]21$ -e define.*0x15 include/net*
> include/netinet/igmp.h:#define IGMP_TRACE                       0x15
> include/netinet/in.h:#define IP_MINTTL          21
> include/netinet/in.h:#define IPV6_LEAVE_GROUP        21
> ...
> 
> $ grep -E -e define.*[[:space:]]22$ -e define.*0x16 include/net*
> include/netinet/igmp.h:#define IGMP_V2_MEMBERSHIP_REPORT        0x16
> include/netinet/in.h:#define IPPROTO_IDP      22
> include/netinet/in.h:#define IP_NODEFRAG        22
> include/netinet/in.h:#define IPV6_ROUTER_ALERT       22
> ...

Indeed, I don't know which macro was used by this case.
> 
> 
>> +	{"ICMP6_FILTER_SETPASS send type 20 filter type 21", 20, 21, T_SETPASS, 0},
>> +	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 20", 20, 20, T_SETBLOCK, 0},
>> +	{"ICMP6_FILTER_SETBLOCK send type 20 filter type 21", 20, 21, T_SETBLOCK, 1},
>> +	{"ICMP6_FILTER_PASSALL send type 20", 20, 0, T_SETPASSALL, 1},
>> +	{"ICMP6_FILTER_PASSALL send type 20", 21, 0, T_SETPASSALL, 1},
> Here is mistake: you claim "send type 20", but it's actually 21. That's actually
> error from original code (copy pasted by you).

Good catch.
> 
>> +	{"ICMP6_FILTER_BLOCKALL send type 20", 20, 0, T_SETBLOCKALL, 0},
>> +	{"ICMP6_FILTER_BLOCKALL send type 20", 21, 0, T_SETBLOCKALL, 0},
> And here as well.

Yes.
> 
> That could be avoided, if values are assigned with help of macro
> (code below would require to rename enum values).

Yes.
> 
> enum filter_macro {
> 	FILTER_SETPASS,
> 	FILTER_SETBLOCK,
> 	FILTER_PASSALL,
> 	FILTER_BLOCKALL,
> 	FILTER_WILLBLOCK,
> 	FILTER_WILLPASS
> };
> 
> #define DESC(x, y, z) .tname = "ICMP6_" #x ", send type: " #y ", filter type: " \
> 	#z, .send_type = y, .filter_type = z, .test_macro = x
> 
> static struct tcase {
> 	char *tname;
> 	unsigned char send_type;
> 	unsigned char filter_type;
> 	enum filter_macro test_macro;
> 	int pass_packet;
> } tcases[] = {
> 	{DESC(FILTER_SETPASS, 20, 20), .pass_packet = 1},
> 	{DESC(FILTER_SETPASS, 20, 21)},
> 	{DESC(FILTER_SETBLOCK, 20, 20)},
> 	{DESC(FILTER_SETBLOCK, 20, 21), .pass_packet = 1},
> 	{DESC(FILTER_PASSALL, 20, 20), .pass_packet = 1},
> 	{DESC(FILTER_PASSALL, 21, 0), .pass_packet = 1},
> 	{DESC(FILTER_BLOCKALL, 20, 0)},
> 	{DESC(FILTER_BLOCKALL, 21, 0)},
> 	{DESC(FILTER_WILLBLOCK, 20, 21)},
> 	{DESC(FILTER_WILLBLOCK, 20, 20), .pass_packet = 1},
> 	{DESC(FILTER_WILLPASS, 20, 21)},
> 	{DESC(FILTER_WILLPASS, 22, 22), .pass_packet = 1},
> };
>> +	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 21", 20, 21, T_WILLBLOCK, 0},
>> +	{"ICMP6_FILTER_WILLBLOCK send type 20 filter type 20", 20, 20, T_WILLBLOCK, 1},
>> +	{"ICMP6_FILTER_WILLPASS send type 20 filter type 21", 20, 21, T_WILLPASS, 0},
>> +	{"ICMP6_FILTER_WILLPASS send type 22 filter type 22", 22, 22, T_WILLPASS, 1},
>>   };
> 
> ...
>> +static void verify_icmp6_filter(unsigned int n)
>>   {
>> +	struct tcase *tc = &tcases[n];
>>   	struct icmp6_filter i6f;
> ...
>> +	int rc;
>> +
>> +	tst_res(TINFO, "Testing %s", tc->tname);
> Please add space here (readability)
>> +	switch (tc->test_macro) {
>> +	case T_SETPASS:
>> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
>> +		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
>> +		break;
>> +	case T_SETPASSALL:
>> +		ICMP6_FILTER_SETPASSALL(&i6f);
>> +		break;
>> +	case T_SETBLOCK:
>> +		ICMP6_FILTER_SETPASSALL(&i6f);
>> +		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
>> +		break;
>> +	case T_SETBLOCKALL:
>> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
>> +		break;
>> +	case T_WILLBLOCK:
>> +		ICMP6_FILTER_SETPASSALL(&i6f);
>> +		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
>> +		rc = ICMP6_FILTER_WILLBLOCK(tc->send_type, &i6f);
>> +		goto check_will_rc;
>> +	case T_WILLPASS:
>> +		ICMP6_FILTER_SETBLOCKALL(&i6f);
>> +		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
>> +		rc = ICMP6_FILTER_WILLPASS(tc->send_type, &i6f);
>> +		goto check_will_rc;
> I'm not really a big fun of big switch, when some parts need goto.
> TST_EXP_EXPR() and return can help to avoid it (see below).
> 
> 
>> +	default:
>> +		tst_brk(TBROK, "unknown test type %d", tc->filter_type);
>> +		break;
>>   	}
>> +	SAFE_SETSOCKOPT(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
>> +	ic6_send(tc->send_type);
>> +	rc = ic6_recv();
>> +	if (rc < 0)
>> +		return;
> And please space here.
>> +	if (rc != tc->pass_packet) {
>> +		tst_res(TFAIL, "%s packet type %d unexpectedly",
>> +				rc ? "pass" : "block", tc->send_type);
>> +	} else {
>> +		tst_res(TPASS, "%s packet type %d as expected",
>> +				tc->pass_packet ? "pass" : "block", tc->send_type);
>> +	}
> This can be replaced by TST_EXP_EXPR().
>> +	return;
> 
>> -	sf = SAFE_SOCKET(NULL, PF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
>> -
>> -	int rv;
>> +check_will_rc:
>> +	if (rc != tc->pass_packet)
>> +		tst_res(TFAIL, "rc %d != expected %d", rc, tc->pass_packet);
>> +	else
>> +		tst_res(TPASS, "expected rc %d", rc);
>> +}
> 
> How about this? (goto is gone, TST_EXP_EXPR() simplifies the code).
> 
> 	switch (tc->test_macro) {
> 	case FILTER_SETPASS:
> 		ICMP6_FILTER_SETBLOCKALL(&i6f);
> 		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
> 		break;
> 	case FILTER_PASSALL:
> 		ICMP6_FILTER_SETPASSALL(&i6f);
> 		break;
> 	case FILTER_SETBLOCK:
> 		ICMP6_FILTER_SETPASSALL(&i6f);
> 		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
> 		break;
> 	case FILTER_BLOCKALL:
> 		ICMP6_FILTER_SETBLOCKALL(&i6f);
> 		break;
> 	case FILTER_WILLBLOCK:
> 		ICMP6_FILTER_SETPASSALL(&i6f);
> 		ICMP6_FILTER_SETBLOCK(tc->filter_type, &i6f);
> 		rc = ICMP6_FILTER_WILLBLOCK(tc->send_type, &i6f);
> 		TST_EXP_EXPR(rc == tc->pass_packet, "%d (%d)", tc->pass_packet, rc);
> 		return;
> 	case FILTER_WILLPASS:
> 		ICMP6_FILTER_SETBLOCKALL(&i6f);
> 		ICMP6_FILTER_SETPASS(tc->filter_type, &i6f);
> 		rc = ICMP6_FILTER_WILLPASS(tc->send_type, &i6f);
> 		TST_EXP_EXPR(rc == tc->pass_packet, "%d (%d)", tc->pass_packet, rc);
> 		return;
> 	default:
> 		tst_brk(TBROK, "unknown test type %d", tc->filter_type);
> 		break;
> 	}
> 
> 	SAFE_SETSOCKOPT(sf, IPPROTO_ICMPV6, ICMP6_FILTER, &i6f, sizeof(i6f));
> 	ic6_send(tc->send_type);
> 	rc = ic6_recv();
> 	if (rc < 0)
> 		return;
> 
> 	TST_EXP_EXPR(rc == tc->pass_packet, "%s packet type %d",
> 				 rc ? "pass" : "block", tc->send_type);
> }

Of course, OK.
> ...
>> +static void cleanup(void)
>> +{
>> +	if (sall > -1)
>> +		SAFE_CLOSE(sall);
> Please add blank line here (readability).
>> +	if (sf > -1)
>> +		SAFE_CLOSE(sf);
>>   }
>> +
>> +static struct tst_test test = {
>> +	.needs_root = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test = verify_icmp6_filter,
>> +	.tcnt = ARRAY_SIZE(tcases)
>> +};
> 
> NOTE: most of my comments are implemented in my fork, branch
> yang_xu/asapi_02.fixes. Feel free to reuse it.
> 
> https://raw.githubusercontent.com/pevik/ltp/yang_xu/asapi_02.fixes/testcases/network/lib6/asapi_02.c
> 
> If you like the changes and use them, you can merge with
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> or even with :)
> Co-developed-by: Petr Vorel <pvorel@suse.cz>

Thanks for your patient review, I will add these two flags and then merged.

Best Regards
Yang Xu
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp