[PATCH v3 0/7] Convert CAN tests to new LTP API

[PATCH v3 0/7] Convert CAN tests to new LTP API

[Richard Palethorpe]

Hello,

This is an attempt to convert the CAN tests to the (modern) Linux Test
Project API and removes the wrapper script. To be clear, this is a
patch-set for the LTP *not* the kernel tree or can-utils.

I have tried to keep the core test behaviour the same, but (for
example) moving to the SAFE_ functions will naturally introduce some
changes in error checking. Deliberate behavioral changes have been
noted in the commit messages.

FYI, we appear to have 4 CAN tests in LTP including two tests for
SLCAN (pty03, pty04).

V2: Update e-mail addresses and resend
V3:
* Add COPYING and use dual license SPDX identifier
* Fix compilation issues with can_common.h

Richard Palethorpe (7):
  API: Add FILE_SCANF to new lib
  can: Add can_common.h for vcan device setup
  can: Add COPYING with dual license text
  can_filter: Convert to new library
  can_recv_own_msgs: Convert to new library
  can: Remove obsolete test wrapper script
  can: Update contact details

 include/tst_safe_file_ops.h                   |   3 +
 testcases/network/can/Makefile                |   2 -
 .../can/filter-tests/00_Descriptions.txt      |   6 +-
 testcases/network/can/filter-tests/COPYING    |  35 ++
 testcases/network/can/filter-tests/INSTALL    |   3 +-
 testcases/network/can/filter-tests/Makefile   |   4 -
 .../network/can/filter-tests/can_common.h     |  75 +++++
 .../network/can/filter-tests/can_filter.c     | 317 +++++++-----------
 .../can/filter-tests/can_rcv_own_msgs.c       | 273 +++++----------
 .../network/can/filter-tests/can_run_tests.sh | 106 ------
 10 files changed, 331 insertions(+), 493 deletions(-)
 create mode 100644 testcases/network/can/filter-tests/COPYING
 create mode 100644 testcases/network/can/filter-tests/can_common.h
 delete mode 100755 testcases/network/can/filter-tests/can_run_tests.sh

-- 
2.30.0

[PATCH v3 1/7] API: Add FILE_SCANF to new lib

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_safe_file_ops.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/tst_safe_file_ops.h b/include/tst_safe_file_ops.h
index 894c16123..ea8f89263 100644
--- a/include/tst_safe_file_ops.h
+++ b/include/tst_safe_file_ops.h
@@ -7,6 +7,9 @@
 
 #include "safe_file_ops_fn.h"
 
+#define FILE_SCANF(path, fmt, ...) \
+	file_scanf(__FILE__, __LINE__, (path), (fmt), ## __VA_ARGS__)
+
 #define SAFE_FILE_SCANF(path, fmt, ...) \
 	safe_file_scanf(__FILE__, __LINE__, NULL, \
 	                (path), (fmt), ## __VA_ARGS__)
-- 
2.30.0

[PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Richard Palethorpe]

Note that we call modprobe to set echo=1. However this does seem to be
necessary for the current tests on 5.10. It has been kept to avoid
changing the test behavior unnecessarily, but can most likely be
safely removed if it causes problems.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 .../network/can/filter-tests/can_common.h     | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 testcases/network/can/filter-tests/can_common.h

diff --git a/testcases/network/can/filter-tests/can_common.h b/testcases/network/can/filter-tests/can_common.h
new file mode 100644
index 000000000..f15145f30
--- /dev/null
+++ b/testcases/network/can/filter-tests/can_common.h
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 SUSE LLC
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+
+#include "tst_cmd.h"
+#include "tst_safe_stdio.h"
+#include "tst_safe_file_ops.h"
+
+#include <linux/if.h>
+#include <linux/can.h>
+#include <linux/can/raw.h>
+
+#ifndef IFF_ECHO
+# define IFF_ECHO (1<<18)
+#endif
+
+static char *can_dev_name;
+static int can_created_dev;
+
+static void can_cmd(const char *const argv[])
+{
+	tst_cmd(argv, NULL, NULL, TST_CMD_TCONF_ON_MISSING);
+}
+
+#define CAN_CMD(...) can_cmd((const char *const[]){ __VA_ARGS__, NULL })
+
+static void can_setup_vcan(void)
+{
+	unsigned int flags;
+	char *path;
+
+	if (can_dev_name)
+		goto check_echo;
+
+	can_dev_name = "vcan0";
+
+	tst_res(TINFO, "Creating vcan0 device; use -D option to avoid this");
+
+	CAN_CMD("modprobe", "-r", "vcan");
+	CAN_CMD("modprobe", "vcan", "echo=1");
+
+	can_created_dev = 1;
+
+	CAN_CMD("ip", "link", "add", "dev", "vcan0", "type", "vcan");
+	CAN_CMD("ip", "link", "set", "dev", "vcan0", "up");
+
+check_echo:
+	/* Precondition for the frame flow test? */
+	SAFE_ASPRINTF(&path, "/sys/class/net/%s/flags", can_dev_name);
+	if (FILE_SCANF(path, "%x", &flags) || !(flags & IFF_ECHO)) {
+		tst_res(TWARN, "Could not determine if ECHO is set on %s",
+			can_dev_name);
+	}
+}
+
+static void can_cleanup_vcan(void)
+{
+	if (!can_created_dev)
+		return;
+
+	CAN_CMD("ip", "link", "set", "dev", "vcan0", "down");
+	CAN_CMD("ip", "link", "del", "dev", "vcan0");
+	CAN_CMD("modprobe", "-r", "vcan");
+}
-- 
2.30.0

[PATCH v3 3/7] can: Add COPYING with dual license text

[Richard Palethorpe]

This may be referred to with

SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)

The kernel version includes the syscall note, but the contents of the
syscall note have never been included within the LTP CAN tests.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/network/can/filter-tests/COPYING | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 testcases/network/can/filter-tests/COPYING

diff --git a/testcases/network/can/filter-tests/COPYING b/testcases/network/can/filter-tests/COPYING
new file mode 100644
index 000000000..2fa5eb911
--- /dev/null
+++ b/testcases/network/can/filter-tests/COPYING
@@ -0,0 +1,35 @@
+Copyright (c) 2010-2011 Volkswagen Group Electronic Research
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+1. Redistributions of source code must retain the above copyright
+   notice, this list of conditions and the following disclaimer.
+2. Redistributions in binary form must reproduce the above copyright
+   notice, this list of conditions and the following disclaimer in the
+   documentation and/or other materials provided with the distribution.
+3. Neither the name of Volkswagen nor the names of its contributors
+   may be used to endorse or promote products derived from this software
+   without specific prior written permission.
+
+Alternatively, provided that this notice is retained in full, this
+software may be distributed under the terms of the GNU General
+Public License ("GPL") version 2, in which case the provisions of the
+GPL apply INSTEAD OF those given above.
+
+The provided data structures and external interfaces from this code
+are not restricted to be used by modules with a GPL compatible license.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+DAMAGE.
-- 
2.30.0

[PATCH v3 4/7] can_filter: Convert to new library

[Richard Palethorpe]

Behavior is mostly the same, but on some errors we print fail and
continue instead of exiting immediately. It appears this would result
in more information being printed if there is a failure.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 .../network/can/filter-tests/can_filter.c     | 317 +++++++-----------
 1 file changed, 124 insertions(+), 193 deletions(-)

diff --git a/testcases/network/can/filter-tests/can_filter.c b/testcases/network/can/filter-tests/can_filter.c
index db017451a..7377556cb 100644
--- a/testcases/network/can/filter-tests/can_filter.c
+++ b/testcases/network/can/filter-tests/can_filter.c
@@ -1,63 +1,15 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
 /*
- * tst-filter.c
- *
  * Copyright (c) 2011 Volkswagen Group Electronic Research
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- * 3. Neither the name of Volkswagen nor the names of its contributors
- *    may be used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * Alternatively, provided that this notice is retained in full, this
- * software may be distributed under the terms of the GNU General
- * Public License ("GPL") version 2, in which case the provisions of the
- * GPL apply INSTEAD OF those given above.
- *
- * The provided data structures and external interfaces from this code
- * are not restricted to be used by modules with a GPL compatible license.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
- * DAMAGE.
- *
- * Send feedback to <socketcan-users@lists.berlios.de>
- *
+ * Copyright (c) 2021 SUSE LLC
  */
 
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <sys/time.h>
-#include <net/if.h>
 #include "config.h"
-#include "tst_res_flags.h"
+#include "tst_test.h"
 
 #ifdef HAVE_LINUX_CAN_H
 
-#include <linux/can.h>
-#include <linux/can/raw.h>
+#include "can_common.h"
 
 #define ID 0x123
 #define TC 18			/* # of testcases */
@@ -66,7 +18,9 @@ const int rx_res[TC] = { 4, 4, 4, 4, 2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1 };
 const int rxbits_res[TC] = { 4369, 4369, 4369, 4369, 17, 4352, 17, 4352, 257,
 			     257, 4112, 4112, 1, 256, 16, 4096, 1, 256 };
 
-canid_t calc_id(int testcase)
+static int s;
+
+static canid_t calc_id(int testcase)
 {
 	canid_t id = ID;
 
@@ -78,7 +32,7 @@ canid_t calc_id(int testcase)
 	return id;
 }
 
-canid_t calc_mask(int testcase)
+static canid_t calc_mask(int testcase)
 {
 	canid_t mask = CAN_SFF_MASK;
 
@@ -93,174 +47,151 @@ canid_t calc_mask(int testcase)
 	return mask;
 }
 
-int main(int argc, char **argv)
+static void setup(void)
 {
-	fd_set rdfs;
-	struct timeval tv;
-	int s;
 	struct sockaddr_can addr;
-	struct can_filter rfilter;
-	struct can_frame frame;
-	int testcase;
-	int have_rx;
-	int rx;
-	int rxbits, rxbitval;
-	int ret;
-	int recv_own_msgs = 1;
 	struct ifreq ifr;
+	int recv_own_msgs = 1;
 
-	/* check command line options */
-	if (argc != 2) {
-		fprintf(stderr, "Usage: %s <device>\n", argv[0]);
-		return TFAIL;
-	}
+	can_setup_vcan();
 
-	s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
-	if (s < 0) {
-		perror("socket");
-		return TFAIL;
-	}
+	s = SAFE_SOCKET(PF_CAN, SOCK_RAW, CAN_RAW);
+
+	strcpy(ifr.ifr_name, can_dev_name);
+	SAFE_IOCTL(s, SIOCGIFINDEX, &ifr);
 
-	strcpy(ifr.ifr_name, argv[1]);
-	if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
-		perror("SIOCGIFINDEX");
-		return TFAIL;
-	}
 	addr.can_family = AF_CAN;
 	addr.can_ifindex = ifr.ifr_ifindex;
 
-	setsockopt(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS,
-		   &recv_own_msgs, sizeof(recv_own_msgs));
+	SAFE_SETSOCKOPT(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS,
+			&recv_own_msgs, sizeof(recv_own_msgs));
 
-	if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
-		perror("bind");
-		return TFAIL;
-	}
+	SAFE_BIND(s, (struct sockaddr *)&addr, sizeof(addr));
+}
 
-	printf("---\n");
+static void cleanup(void)
+{
+	if (s)
+		SAFE_CLOSE(s);
 
-	for (testcase = 0; testcase < TC; testcase++) {
+	can_cleanup_vcan();
+}
 
-		rfilter.can_id = calc_id(testcase);
-		rfilter.can_mask = calc_mask(testcase);
-		setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER,
-			   &rfilter, sizeof(rfilter));
+static void run(unsigned int n)
+{
+	fd_set rdfs;
+	struct timeval tv;
+	struct can_frame frame;
+	static struct can_filter rfilter;
+	int testcase = n;
+	int have_rx = 1;
+	int rx = 0;
+	int rxbits = 0, rxbitval;
 
-		printf("testcase %2d filters : can_id = 0x%08X can_mask = "
-		       "0x%08X\n", testcase, rfilter.can_id, rfilter.can_mask);
+	rfilter.can_id = calc_id(testcase);
+	rfilter.can_mask = calc_mask(testcase);
+	SAFE_SETSOCKOPT(s, SOL_CAN_RAW, CAN_RAW_FILTER,
+			&rfilter, sizeof(rfilter));
 
-		printf("testcase %2d sending patterns ... ", testcase);
+	tst_res(TINFO, "testcase %2d filters : can_id = 0x%08X can_mask = "
+	       "0x%08X", testcase, rfilter.can_id, rfilter.can_mask);
 
-		frame.can_dlc = 1;
-		frame.data[0] = testcase;
+	tst_res(TINFO, "testcase %2d sending patterns ... ", testcase);
 
-		frame.can_id = ID;
-		if (write(s, &frame, sizeof(frame)) < 0) {
-			perror("write");
-			exit(1);
-		}
-		frame.can_id = (ID | CAN_RTR_FLAG);
-		if (write(s, &frame, sizeof(frame)) < 0) {
-			perror("write");
-			exit(1);
-		}
-		frame.can_id = (ID | CAN_EFF_FLAG);
-		if (write(s, &frame, sizeof(frame)) < 0) {
-			perror("write");
-			exit(1);
-		}
-		frame.can_id = (ID | CAN_EFF_FLAG | CAN_RTR_FLAG);
-		if (write(s, &frame, sizeof(frame)) < 0) {
-			perror("write");
-			exit(1);
-		}
+	frame.can_dlc = 1;
+	frame.data[0] = testcase;
 
-		printf("ok\n");
+	frame.can_id = ID;
+	SAFE_WRITE(1, s, &frame, sizeof(frame));
 
-		have_rx = 1;
-		rx = 0;
-		rxbits = 0;
+	frame.can_id = (ID | CAN_RTR_FLAG);
+	SAFE_WRITE(1, s, &frame, sizeof(frame));
 
-		while (have_rx) {
+	frame.can_id = (ID | CAN_EFF_FLAG);
+	SAFE_WRITE(1, s, &frame, sizeof(frame));
 
-			have_rx = 0;
-			FD_ZERO(&rdfs);
-			FD_SET(s, &rdfs);
-			tv.tv_sec = 0;
-			tv.tv_usec = 50000;	/* 50ms timeout */
+	frame.can_id = (ID | CAN_EFF_FLAG | CAN_RTR_FLAG);
+	SAFE_WRITE(1, s, &frame, sizeof(frame));
 
-			ret = select(s + 1, &rdfs, NULL, NULL, &tv);
-			if (ret < 0) {
-				perror("select");
-				exit(1);
-			}
+	tst_res(TPASS, "testcase %2d Sent patterns", testcase);
+
+	while (have_rx) {
+
+		have_rx = 0;
+		FD_ZERO(&rdfs);
+		FD_SET(s, &rdfs);
+		tv.tv_sec = 0;
+		tv.tv_usec = 50000;	/* 50ms timeout */
+
+		if (select(s + 1, &rdfs, NULL, NULL, &tv) < 0)
+			tst_brk(TBROK | TERRNO, "select");
 
-			if (FD_ISSET(s, &rdfs)) {
-				have_rx = 1;
-				ret = read(s, &frame, sizeof(struct can_frame));
-				if (ret < 0) {
-					perror("read");
-					exit(1);
-				}
-				if ((frame.can_id & CAN_SFF_MASK) != ID) {
-					fprintf(stderr,
-						"received wrong can_id!\n");
-					exit(1);
-				}
-				if (frame.data[0] != testcase) {
-					fprintf(stderr,
-						"received wrong testcase!\n");
-					exit(1);
-				}
-
-				/* test & calc rxbits */
-				rxbitval = 1 << ((frame.can_id &
-						 (CAN_EFF_FLAG | CAN_RTR_FLAG |
-						  CAN_ERR_FLAG)) >> 28);
-
-				/* only receive a rxbitval once */
-				if ((rxbits & rxbitval) == rxbitval) {
-					fprintf(stderr,
-						"received rxbitval %d twice!\n",
-						rxbitval);
-					exit(1);
-				}
-				rxbits |= rxbitval;
-				rx++;
-
-				printf("testcase %2d rx : can_id = 0x%08X rx = "
-				       "%d rxbits = %d\n", testcase,
-				       frame.can_id, rx, rxbits);
+		if (FD_ISSET(s, &rdfs)) {
+			have_rx = 1;
+			SAFE_READ(1, s, &frame, sizeof(struct can_frame));
+
+			if ((frame.can_id & CAN_SFF_MASK) != ID)
+				tst_res(TFAIL, "received wrong can_id!");
+
+			if (frame.data[0] != testcase)
+				tst_res(TFAIL, "received wrong testcase!");
+
+			/* test & calc rxbits */
+			rxbitval = 1 << ((frame.can_id &
+					  (CAN_EFF_FLAG | CAN_RTR_FLAG |
+					   CAN_ERR_FLAG)) >> 28);
+
+			/* only receive a rxbitval once */
+			if ((rxbits & rxbitval) == rxbitval) {
+				tst_res(TFAIL, "received rxbitval %d twice!",
+					rxbitval);
 			}
+			rxbits |= rxbitval;
+			rx++;
+
+			tst_res(TINFO, "testcase %2d rx : can_id = 0x%08X rx = "
+			       "%d rxbits = %d", testcase,
+			       frame.can_id, rx, rxbits);
 		}
-		/* rx timed out -> check the received results */
-		if (rx_res[testcase] != rx) {
-			fprintf(stderr,
-				"wrong rx value in testcase %d : %d (expected "
-				"%d)\n", testcase, rx, rx_res[testcase]);
-			exit(1);
-		}
-		if (rxbits_res[testcase] != rxbits) {
-			fprintf(stderr,
-				"wrong rxbits value in testcase %d : %d "
-				"(expected %d)\n", testcase, rxbits,
-				rxbits_res[testcase]);
-			exit(1);
-		}
-		printf("testcase %2d ok\n---\n", testcase);
 	}
-
-	close(s);
-
-	return TPASS;
+	/* rx timed out -> check the received results */
+	if (rx_res[testcase] != rx) {
+		tst_brk(TBROK,
+			"wrong rx value in testcase %d : %d (expected "
+			"%d)", testcase, rx, rx_res[testcase]);
+	}
+	if (rxbits_res[testcase] != rxbits) {
+		tst_brk(TBROK,
+			"wrong rxbits value in testcase %d : %d "
+			"(expected %d)", testcase, rxbits,
+			rxbits_res[testcase]);
+	}
+	tst_res(TPASS, "testcase %2d ok", testcase);
 }
 
+static struct tst_test test = {
+	.tcnt = TC,
+	.options = (struct tst_option[]) {
+		{"D:", &can_dev_name, "-D <device>	CAN device name"},
+		{}
+	},
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = run,
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_NET_RAW),
+		TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
+		{}
+	},
+	.needs_drivers = (const char *const[]) {
+		"vcan",
+		"can-raw",
+		NULL
+	}
+};
+
 #else
 
-int main(void)
-{
-	printf("The linux/can.h was missing upon compilation.\n");
-	return TCONF;
-}
+TST_TEST_TCONF("The linux/can.h was missing upon compilation");
 
 #endif /* HAVE_LINUX_CAN_H */
-- 
2.30.0

[PATCH v3 5/7] can_recv_own_msgs: Convert to new library

[Richard Palethorpe]

Behavior is approximately the same except for the following:

* Test will continue after non-fatal errors.
* We now check the return value of `setsocketopt` and that I/O
  completed the specified number of bytes.
* The command line arg, device name, must be prepended with `-D`

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 .../can/filter-tests/can_rcv_own_msgs.c       | 273 ++++++------------
 1 file changed, 88 insertions(+), 185 deletions(-)

diff --git a/testcases/network/can/filter-tests/can_rcv_own_msgs.c b/testcases/network/can/filter-tests/can_rcv_own_msgs.c
index 8ad51d298..517daae51 100644
--- a/testcases/network/can/filter-tests/can_rcv_own_msgs.c
+++ b/testcases/network/can/filter-tests/can_rcv_own_msgs.c
@@ -1,88 +1,31 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
 /*
- * tst-rcv-own-msgs.c
- *
- * Copyright (c) 2010 Volkswagen Group Electronic Research
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- * 3. Neither the name of Volkswagen nor the names of its contributors
- *    may be used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * Alternatively, provided that this notice is retained in full, this
- * software may be distributed under the terms of the GNU General
- * Public License ("GPL") version 2, in which case the provisions of the
- * GPL apply INSTEAD OF those given above.
- *
- * The provided data structures and external interfaces from this code
- * are not restricted to be used by modules with a GPL compatible license.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
- * DAMAGE.
- *
- * Send feedback to <socketcan-users@lists.berlios.de>
- *
+ * Copyright (c) 2011 Volkswagen Group Electronic Research
+ * Copyright (c) 2021 SUSE LLC
  */
 
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <sys/time.h>
-#include <net/if.h>
 #include "config.h"
-#include "tst_res_flags.h"
-#include "tst_minmax.h"
+#include "tst_test.h"
 
 #ifdef HAVE_LINUX_CAN_H
 
-#include <linux/can.h>
-#include <linux/can/raw.h>
+#include "can_common.h"
+#include "tst_minmax.h"
 
-struct rxs {
-	int s;
-	int t;
-};
+static int s, t;
 
-struct rxs test_sockets(int s, int t, canid_t can_id)
+void test_sockets(canid_t can_id, int expect_rxs, int expect_rxt)
 {
 	fd_set rdfs;
 	struct timeval tv;
 	int m = MAX(s, t) + 1;
 	int have_rx = 1;
 	struct can_frame frame;
-	struct rxs rx;
-	int ret;
+	int rxs = 0, rxt = 0;
 
 	frame.can_id = can_id;
 	frame.can_dlc = 0;
-	if (write(s, &frame, sizeof(frame)) < 0) {
-		perror("write");
-		exit(1);
-	}
-
-	rx.s = rx.t = 0;
+	SAFE_WRITE(1, s, &frame, sizeof(frame));
 
 	while (have_rx) {
 
@@ -93,164 +36,124 @@ struct rxs test_sockets(int s, int t, canid_t can_id)
 		tv.tv_usec = 50000;	/* 50ms timeout */
 		have_rx = 0;
 
-		ret = select(m, &rdfs, NULL, NULL, &tv);
-		if (ret < 0) {
-			perror("select");
-			exit(1);
-		}
+		if (select(m, &rdfs, NULL, NULL, &tv) < 0)
+			tst_brk(TBROK | TERRNO, "select");
 
 		if (FD_ISSET(s, &rdfs)) {
 
 			have_rx = 1;
-			ret = read(s, &frame, sizeof(struct can_frame));
-			if (ret < 0) {
-				perror("read");
-				exit(1);
-			}
-			if (frame.can_id != can_id) {
-				fprintf(stderr, "received wrong can_id!\n");
-				exit(1);
-			}
-			rx.s++;
+			SAFE_READ(1, s, &frame, sizeof(struct can_frame));
+
+			if (frame.can_id != can_id)
+				tst_res(TFAIL, "received wrong can_id!");
+
+			rxs++;
 		}
 
 		if (FD_ISSET(t, &rdfs)) {
 
 			have_rx = 1;
-			ret = read(t, &frame, sizeof(struct can_frame));
-			if (ret < 0) {
-				perror("read");
-				exit(1);
-			}
-			if (frame.can_id != can_id) {
-				fprintf(stderr, "received wrong can_id!\n");
-				exit(1);
-			}
-			rx.t++;
+			SAFE_READ(1, t, &frame, sizeof(struct can_frame));
+
+			if (frame.can_id != can_id)
+				tst_res(TFAIL, "received wrong can_id!");
+
+			rxt++;
 		}
 	}
 
 	/* timeout */
 
-	return rx;
+	tst_res(rxs == expect_rxs && rxt == expect_rxt ? TPASS : TFAIL,
+		"s received %d of %d, t received %d of %d",
+		rxs, expect_rxs, rxt, expect_rxt);
 }
 
-void setopts(int s, int loopback, int recv_own_msgs)
+static void setopts(int loopback, int recv_own_msgs)
 {
-	setsockopt(s, SOL_CAN_RAW, CAN_RAW_LOOPBACK, &loopback,
-		   sizeof(loopback));
-	setsockopt(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS, &recv_own_msgs,
-		   sizeof(recv_own_msgs));
+	SAFE_SETSOCKOPT(s, SOL_CAN_RAW, CAN_RAW_LOOPBACK, &loopback,
+			sizeof(loopback));
+	SAFE_SETSOCKOPT(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS, &recv_own_msgs,
+			sizeof(recv_own_msgs));
 
-	printf("check loopback %d recv_own_msgs %d ... ", loopback,
-	       recv_own_msgs);
+	tst_res(TINFO, "set loopback = %d, recv_own_msgs = %d",
+		loopback, recv_own_msgs);
 }
 
-int main(int argc, char **argv)
+static void setup(void)
 {
-	int s, t;
 	struct sockaddr_can addr;
 	struct ifreq ifr;
-	struct rxs rx;
 
-	/* check command line options */
-	if (argc != 2) {
-		fprintf(stderr, "Usage: %s <device>\n", argv[0]);
-		return TFAIL;
-	}
+	can_setup_vcan();
 
-	s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
-	if (s < 0) {
-		perror("socket");
-		return TFAIL;
-	}
-	t = socket(PF_CAN, SOCK_RAW, CAN_RAW);
-	if (t < 0) {
-		perror("socket");
-		return TFAIL;
-	}
+	s = SAFE_SOCKET(PF_CAN, SOCK_RAW, CAN_RAW);
+	t = SAFE_SOCKET(PF_CAN, SOCK_RAW, CAN_RAW);
+
+	strcpy(ifr.ifr_name, can_dev_name);
+	SAFE_IOCTL(s, SIOCGIFINDEX, &ifr);
 
-	strcpy(ifr.ifr_name, argv[1]);
-	if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
-		perror("SIOCGIFINDEX");
-		return TFAIL;
-	}
 	addr.can_ifindex = ifr.ifr_ifindex;
 	addr.can_family = AF_CAN;
 
-	if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
-		perror("bind");
-		return TFAIL;
-	}
-	if (bind(t, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
-		perror("bind");
-		return TFAIL;
-	}
+	SAFE_BIND(s, (struct sockaddr *)&addr, sizeof(addr));
+	SAFE_BIND(t, (struct sockaddr *)&addr, sizeof(addr));
+}
 
-	printf("Starting PF_CAN frame flow test.\n");
-	printf("checking socket default settings ... ");
-	rx = test_sockets(s, t, 0x340);
-	if (rx.s == 0 && rx.t == 1)
-		printf("ok.\n");
-	else {
-		printf("failure!\n");
-		return TFAIL;
-	}
+static void cleanup(void)
+{
+	if (s)
+		SAFE_CLOSE(s);
+	if (t)
+		SAFE_CLOSE(t);
 
-	/* check loopback 0 recv_own_msgs 0 */
-	setopts(s, 0, 0);
-	rx = test_sockets(s, t, 0x341);
-	if (rx.s == 0 && rx.t == 0)
-		printf("ok.\n");
-	else {
-		printf("failure!\n");
-		return TFAIL;
-	}
+	can_cleanup_vcan();
+}
 
-	/* check loopback 0 recv_own_msgs 1 */
-	setopts(s, 0, 1);
-	rx = test_sockets(s, t, 0x342);
-	if (rx.s == 0 && rx.t == 0)
-		printf("ok.\n");
-	else {
-		printf("failure!\n");
-		return TFAIL;
-	}
+static void run(void)
+{
+	tst_res(TINFO, "Starting PF_CAN frame flow test.");
+	tst_res(TINFO, "checking socket default settings");
+	test_sockets(0x340, 0, 1);
 
-	/* check loopback 1 recv_own_msgs 0 */
-	setopts(s, 1, 0);
-	rx = test_sockets(s, t, 0x343);
-	if (rx.s == 0 && rx.t == 1)
-		printf("ok.\n");
-	else {
-		printf("failure!\n");
-		return TFAIL;
-	}
+	setopts(0, 0);
+	test_sockets(0x341, 0, 0);
 
-	/* check loopback 1 recv_own_msgs 1 */
-	setopts(s, 1, 1);
-	rx = test_sockets(s, t, 0x344);
-	if (rx.s == 1 && rx.t == 1)
-		printf("ok.\n");
-	else {
-		printf("failure!\n");
-		return TFAIL;
-	}
+	setopts(0, 1);
+	test_sockets(0x342, 0, 0);
 
-	printf("PF_CAN frame flow test was successful.\n");
+	setopts(1, 0);
+	test_sockets(0x343, 0, 1);
 
-	close(s);
-	close(t);
+	setopts(1, 1);
+	test_sockets(0x344, 1, 1);
 
-	return TPASS;
+	/* Return to defaults for when -i is used */
+	setopts(1, 0);
 }
 
+static struct tst_test test = {
+	.options = (struct tst_option[]) {
+		{"D:", &can_dev_name, "-D <device>	CAN device name"},
+		{}
+	},
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_NET_RAW),
+		TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
+		{}
+	},
+	.needs_drivers = (const char *const[]) {
+		"vcan",
+		"can-raw",
+		NULL
+	}
+};
+
 #else
 
-int main(void)
-{
-	printf("The linux/can.h was missing upon compilation.\n");
-	return TCONF;
-}
+TST_TEST_TCONF("The linux/can.h was missing upon compilation");
 
 #endif /* HAVE_LINUX_CAN_H */
-- 
2.30.0

[PATCH v3 6/7] can: Remove obsolete test wrapper script

[Richard Palethorpe]

Functionality moved into can_common.h and the test executables.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/network/can/Makefile                |   2 -
 .../can/filter-tests/00_Descriptions.txt      |   1 +
 testcases/network/can/filter-tests/INSTALL    |   3 +-
 testcases/network/can/filter-tests/Makefile   |   2 -
 .../network/can/filter-tests/can_run_tests.sh | 106 ------------------
 5 files changed, 3 insertions(+), 111 deletions(-)
 delete mode 100755 testcases/network/can/filter-tests/can_run_tests.sh

diff --git a/testcases/network/can/Makefile b/testcases/network/can/Makefile
index 31744534a..a69f6be87 100644
--- a/testcases/network/can/Makefile
+++ b/testcases/network/can/Makefile
@@ -22,6 +22,4 @@ top_srcdir		?= ../../..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-INSTALL_TARGETS		:= *.sh
-
 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/network/can/filter-tests/00_Descriptions.txt b/testcases/network/can/filter-tests/00_Descriptions.txt
index 89610982c..55d213456 100644
--- a/testcases/network/can/filter-tests/00_Descriptions.txt
+++ b/testcases/network/can/filter-tests/00_Descriptions.txt
@@ -5,5 +5,6 @@ information on the CAN network protocol family PF_CAN is contained in
 <Kernel/Documentation/networking/can.txt>.
 
 For any issue(s) with CAN tests please write to:
+<ltp@lists.linux.it>
 <socketcan-users@lists.berlios.de>,
 Oliver Hartkopp <oliver.hartkopp@volkswagen.de>,
diff --git a/testcases/network/can/filter-tests/INSTALL b/testcases/network/can/filter-tests/INSTALL
index e549aed0f..7d62d65fe 100644
--- a/testcases/network/can/filter-tests/INSTALL
+++ b/testcases/network/can/filter-tests/INSTALL
@@ -12,7 +12,8 @@ enable CAN protocol, please see ltp/README file.
 $ make
 
 2) To Run the tests from this directory, do:
-$ ./run_ltp-can_tests.sh
+$ ./can_filter
+$ ./can_rcv_own_msgs
 
 3) To let LTP run the tests from $LTPROOT, execute:
 $ ./runltp -f can
diff --git a/testcases/network/can/filter-tests/Makefile b/testcases/network/can/filter-tests/Makefile
index 6a9474164..29aaa3240 100644
--- a/testcases/network/can/filter-tests/Makefile
+++ b/testcases/network/can/filter-tests/Makefile
@@ -23,6 +23,4 @@ include $(top_srcdir)/include/mk/testcases.mk
 
 CPPFLAGS		+= -DETH_P_CAN=0x000C -DPF_CAN=29 -DAF_CAN=PF_CAN
 
-INSTALL_TARGETS		:= *.sh
-
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/network/can/filter-tests/can_run_tests.sh b/testcases/network/can/filter-tests/can_run_tests.sh
deleted file mode 100755
index 762d90eca..000000000
--- a/testcases/network/can/filter-tests/can_run_tests.sh
+++ /dev/null
@@ -1,106 +0,0 @@
-#!/bin/sh
-################################################################################
-## Copyright (c) Oliver Hartkopp <oliver.hartkopp@volkswagen.de>, 2011        ##
-## Copyright (c) International Business Machines  Corp., 2009                 ##
-##                                                                            ##
-## 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 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## 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.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
-
-TCID="$1"
-TST_TOTAL=1
-export TST_COUNT
-
-. test.sh
-
-setup()
-{
-	tst_require_root
-
-	# load needed CAN networklayer modules
-	modprobe can
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TCONF "modprobe can failed: ret - $ret"
-	fi
-
-	modprobe can_raw
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TCONF "modprobe can_raw failed: ret - $ret"
-	fi
-
-	# ensure the vcan driver to perform the ECHO on driver level
-	modprobe -r vcan
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TCONF "modprobe -r vcan failed: ret - $ret"
-	fi
-
-	modprobe vcan echo=1
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TCONF "modprobe vcan echo=1 failed: ret - $ret"
-	fi
-
-	VCAN=vcan0
-
-	# create virtual CAN device
-	ip link add dev $VCAN type vcan
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TBROK \
-			 "ip link add dev $VCAN type vcan failed: ret - $ret"
-	fi
-
-	ip link set dev $VCAN up
-	ret=$?
-	if [ $ret -ne 0 ]; then
-		tst_brkm TBROK "ip link set dev $VCAN up failed: ret - $ret"
-	fi
-
-	# check precondition for CAN frame flow test
-	HAS_ECHO=`ip link show $VCAN | grep -c ECHO`
-	if [ $HAS_ECHO -ne 1 ]; then
-		tst_brkm TBROK "ECHO is not 1"
-	fi
-}
-
-cleanup()
-{
-	ip link set dev $VCAN down
-	ip link del dev $VCAN
-	modprobe -r vcan
-	modprobe -r can_raw
-	modprobe -r can
-}
-
-if [ $# -ne 1 ]; then
-	tst_brkm TBROK "Usage: $0 [can_filter | can_rcv_own_msgs]"
-fi
-
-setup
-TST_CLEANUP=cleanup
-
-"$1" "$VCAN"
-ret=$?
-case "$ret" in
-0)	tst_resm TPASS "Test $1 PASS";;
-1)	tst_resm TFAIL "Test $1 FAIL";;
-32)	tst_resm TCONF "$1 is not appropriate for configuration flag";;
-*)	tst_resm TBROK "Invalid resm type $ret";;
-esac
-
-tst_exit
-- 
2.30.0

[PATCH v3 7/7] can: Update contact details

[Richard Palethorpe]

socketcan-users appears to be no more.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/network/can/filter-tests/00_Descriptions.txt | 5 +++--
 testcases/network/can/filter-tests/Makefile            | 2 --
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/testcases/network/can/filter-tests/00_Descriptions.txt b/testcases/network/can/filter-tests/00_Descriptions.txt
index 55d213456..9623d29a0 100644
--- a/testcases/network/can/filter-tests/00_Descriptions.txt
+++ b/testcases/network/can/filter-tests/00_Descriptions.txt
@@ -6,5 +6,6 @@ information on the CAN network protocol family PF_CAN is contained in
 
 For any issue(s) with CAN tests please write to:
 <ltp@lists.linux.it>
-<socketcan-users@lists.berlios.de>,
-Oliver Hartkopp <oliver.hartkopp@volkswagen.de>,
+Oliver Hartkopp <socketcan@hartkopp.net>
+Linux-CAN ML <linux-can@vger.kernel.org>
+
diff --git a/testcases/network/can/filter-tests/Makefile b/testcases/network/can/filter-tests/Makefile
index 29aaa3240..bd57c7fff 100644
--- a/testcases/network/can/filter-tests/Makefile
+++ b/testcases/network/can/filter-tests/Makefile
@@ -14,8 +14,6 @@
 #    You should have received a copy of the GNU General Public License along
 #    with this program; if not, write to the Free Software Foundation, Inc.,
 #    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-#  Send feedback to <socketcan-users@lists.berlios.de>
 
 top_srcdir		?= ../../../..
 
-- 
2.30.0

Re: [LTP] [PATCH v3 6/7] can: Remove obsolete test wrapper script

[Petr Vorel]

Hi Richie,

>  2) To Run the tests from this directory, do:
> -$ ./run_ltp-can_tests.sh
> +$ ./can_filter
> +$ ./can_rcv_own_msgs
>  3) To let LTP run the tests from $LTPROOT, execute:
>  $ ./runltp -f can

You also need to update runtest/can (which can be done during merge).

diff --git runtest/can runtest/can
index 6aa2ae9f5..b637183c6 100644
--- runtest/can
+++ runtest/can
@@ -1,2 +1,2 @@
-can_filter can_run_tests.sh can_filter
-can_rcv_own_msgs can_run_tests.sh can_rcv_own_msgs
+can_filter can_filter
+can_rcv_own_msgs can_rcv_own_msgs

Whole patchset LGTM, thanks!

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

Kind regards,
Petr

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Oliver Hartkopp]

Hello Richard,

On 20.01.21 15:37, Richard Palethorpe wrote:
> Note that we call modprobe to set echo=1. However this does seem to be
> necessary for the current tests on 5.10. It has been kept to avoid
> changing the test behavior unnecessarily, but can most likely be
> safely removed if it causes problems.

Without echo=1 a shortcut in af_can.c is used.

I just checked that it works too - but with echo=1 we increase the test 
coverage to a test of the message flow down to the virtual CAN driver vcan.

> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>   .../network/can/filter-tests/can_common.h     | 75 +++++++++++++++++++
>   1 file changed, 75 insertions(+)
>   create mode 100644 testcases/network/can/filter-tests/can_common.h
> 
> diff --git a/testcases/network/can/filter-tests/can_common.h b/testcases/network/can/filter-tests/can_common.h
> new file mode 100644
> index 000000000..f15145f30
> --- /dev/null
> +++ b/testcases/network/can/filter-tests/can_common.h
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/ioctl.h>
> +#include <sys/time.h>
> +
> +#include "tst_cmd.h"
> +#include "tst_safe_stdio.h"
> +#include "tst_safe_file_ops.h"
> +
> +#include <linux/if.h>
> +#include <linux/can.h>
> +#include <linux/can/raw.h>
> +
> +#ifndef IFF_ECHO
> +# define IFF_ECHO (1<<18)
> +#endif

IFF_ECHO was included into Linux 2.6.25 together with the CAN subsystem 
itself.

So when you run the tests on Kernels < 2.6.25 you don't have CAN support 
and don't need IFF_ECHO too.

Regards,
Oliver

> +
> +static char *can_dev_name;
> +static int can_created_dev;
> +
> +static void can_cmd(const char *const argv[])
> +{
> +	tst_cmd(argv, NULL, NULL, TST_CMD_TCONF_ON_MISSING);
> +}
> +
> +#define CAN_CMD(...) can_cmd((const char *const[]){ __VA_ARGS__, NULL })
> +
> +static void can_setup_vcan(void)
> +{
> +	unsigned int flags;
> +	char *path;
> +
> +	if (can_dev_name)
> +		goto check_echo;
> +
> +	can_dev_name = "vcan0";
> +
> +	tst_res(TINFO, "Creating vcan0 device; use -D option to avoid this");
> +
> +	CAN_CMD("modprobe", "-r", "vcan");
> +	CAN_CMD("modprobe", "vcan", "echo=1");
> +
> +	can_created_dev = 1;
> +
> +	CAN_CMD("ip", "link", "add", "dev", "vcan0", "type", "vcan");
> +	CAN_CMD("ip", "link", "set", "dev", "vcan0", "up");
> +
> +check_echo:
> +	/* Precondition for the frame flow test? */
> +	SAFE_ASPRINTF(&path, "/sys/class/net/%s/flags", can_dev_name);
> +	if (FILE_SCANF(path, "%x", &flags) || !(flags & IFF_ECHO)) {
> +		tst_res(TWARN, "Could not determine if ECHO is set on %s",
> +			can_dev_name);
> +	}
> +}
> +
> +static void can_cleanup_vcan(void)
> +{
> +	if (!can_created_dev)
> +		return;
> +
> +	CAN_CMD("ip", "link", "set", "dev", "vcan0", "down");
> +	CAN_CMD("ip", "link", "del", "dev", "vcan0");
> +	CAN_CMD("modprobe", "-r", "vcan");
> +}
> 

Re: [PATCH v3 7/7] can: Update contact details

[Oliver Hartkopp]

On 20.01.21 15:37, Richard Palethorpe wrote:
> socketcan-users appears to be no more.
> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>

Thanks for the cleanup and the rework, Richard!

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
(for the entire series)

Best regards,
Oliver

> ---
>   testcases/network/can/filter-tests/00_Descriptions.txt | 5 +++--
>   testcases/network/can/filter-tests/Makefile            | 2 --
>   2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/network/can/filter-tests/00_Descriptions.txt b/testcases/network/can/filter-tests/00_Descriptions.txt
> index 55d213456..9623d29a0 100644
> --- a/testcases/network/can/filter-tests/00_Descriptions.txt
> +++ b/testcases/network/can/filter-tests/00_Descriptions.txt
> @@ -6,5 +6,6 @@ information on the CAN network protocol family PF_CAN is contained in
>   
>   For any issue(s) with CAN tests please write to:
>   <ltp@lists.linux.it>
> -<socketcan-users@lists.berlios.de>,
> -Oliver Hartkopp <oliver.hartkopp@volkswagen.de>,
> +Oliver Hartkopp <socketcan@hartkopp.net>
> +Linux-CAN ML <linux-can@vger.kernel.org>
> +
> diff --git a/testcases/network/can/filter-tests/Makefile b/testcases/network/can/filter-tests/Makefile
> index 29aaa3240..bd57c7fff 100644
> --- a/testcases/network/can/filter-tests/Makefile
> +++ b/testcases/network/can/filter-tests/Makefile
> @@ -14,8 +14,6 @@
>   #    You should have received a copy of the GNU General Public License along
>   #    with this program; if not, write to the Free Software Foundation, Inc.,
>   #    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> -#
> -#  Send feedback to <socketcan-users@lists.berlios.de>
>   
>   top_srcdir		?= ../../../..
>   
> 

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Richard Palethorpe]

Hello Oliver and Petr,

Oliver Hartkopp <socketcan@hartkopp.net> writes:

> Hello Richard,
>
> On 20.01.21 15:37, Richard Palethorpe wrote:
>> Note that we call modprobe to set echo=1. However this does seem to be
>> necessary for the current tests on 5.10. It has been kept to avoid
>> changing the test behavior unnecessarily, but can most likely be
>> safely removed if it causes problems.
>
> Without echo=1 a shortcut in af_can.c is used.
>
> I just checked that it works too - but with echo=1 we increase the
> test coverage to a test of the message flow down to the virtual CAN
> driver vcan.

Ah, thanks, I will amend the comments.

>
>> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
>> ---
>>   .../network/can/filter-tests/can_common.h     | 75 +++++++++++++++++++
>>   1 file changed, 75 insertions(+)
>>   create mode 100644 testcases/network/can/filter-tests/can_common.h
>> diff --git a/testcases/network/can/filter-tests/can_common.h
>> b/testcases/network/can/filter-tests/can_common.h
>> new file mode 100644
>> index 000000000..f15145f30
>> --- /dev/null
>> +++ b/testcases/network/can/filter-tests/can_common.h
>> @@ -0,0 +1,75 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2021 SUSE LLC
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <string.h>
>> +
>> +#include <sys/types.h>
>> +#include <sys/socket.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/time.h>
>> +
>> +#include "tst_cmd.h"
>> +#include "tst_safe_stdio.h"
>> +#include "tst_safe_file_ops.h"
>> +
>> +#include <linux/if.h>
>> +#include <linux/can.h>
>> +#include <linux/can/raw.h>
>> +
>> +#ifndef IFF_ECHO
>> +# define IFF_ECHO (1<<18)
>> +#endif
>
> IFF_ECHO was included into Linux 2.6.25 together with the CAN
> subsystem itself.
>
> So when you run the tests on Kernels < 2.6.25 you don't have CAN
> support and don't need IFF_ECHO too.

Petr, what kernel version and/or distro version did compilation fail on?

There is a small chance someone might be compiling with old kernel
headers relative to their kernel. However it is a challenge to compile
LTP with such an old user land.

>
> Regards,
> Oliver
>
>> +
>> +static char *can_dev_name;
>> +static int can_created_dev;
>> +
>> +static void can_cmd(const char *const argv[])
>> +{
>> +	tst_cmd(argv, NULL, NULL, TST_CMD_TCONF_ON_MISSING);
>> +}
>> +
>> +#define CAN_CMD(...) can_cmd((const char *const[]){ __VA_ARGS__, NULL })
>> +
>> +static void can_setup_vcan(void)
>> +{
>> +	unsigned int flags;
>> +	char *path;
>> +
>> +	if (can_dev_name)
>> +		goto check_echo;
>> +
>> +	can_dev_name = "vcan0";
>> +
>> +	tst_res(TINFO, "Creating vcan0 device; use -D option to avoid this");
>> +
>> +	CAN_CMD("modprobe", "-r", "vcan");
>> +	CAN_CMD("modprobe", "vcan", "echo=1");
>> +
>> +	can_created_dev = 1;
>> +
>> +	CAN_CMD("ip", "link", "add", "dev", "vcan0", "type", "vcan");
>> +	CAN_CMD("ip", "link", "set", "dev", "vcan0", "up");
>> +
>> +check_echo:
>> +	/* Precondition for the frame flow test? */
>> +	SAFE_ASPRINTF(&path, "/sys/class/net/%s/flags", can_dev_name);
>> +	if (FILE_SCANF(path, "%x", &flags) || !(flags & IFF_ECHO)) {
>> +		tst_res(TWARN, "Could not determine if ECHO is set on %s",
>> +			can_dev_name);
>> +	}
>> +}
>> +
>> +static void can_cleanup_vcan(void)
>> +{
>> +	if (!can_created_dev)
>> +		return;
>> +
>> +	CAN_CMD("ip", "link", "set", "dev", "vcan0", "down");
>> +	CAN_CMD("ip", "link", "del", "dev", "vcan0");
>> +	CAN_CMD("modprobe", "-r", "vcan");
>> +}
>> 


-- 
Thank you,
Richard.

Re: [LTP] [PATCH v3 5/7] can_recv_own_msgs: Convert to new library

[Cyril Hrubis]

Hi!
> -struct rxs test_sockets(int s, int t, canid_t can_id)
> +void test_sockets(canid_t can_id, int expect_rxs, int expect_rxt)

static?

However this is very minor.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [LTP] [PATCH v3 7/7] can: Update contact details

[Cyril Hrubis]

Hi!
Minus the runtest files that has to be updated the rest looks ok.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Also I do wonder how much of can functionality these tests does not cover.

I guess that we can as well write a test that would add two or more
virtual devices setup routing and send messages between these.

Also there seems to be several different protocols and we seem to be
using only CAN_RAW.

There seems to be also flag to inverse the filter mask with
CAN_INV_FILTER, filter only for error messages and so on.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Petr Vorel]

Hi Oliver, Richie,

> >> --- /dev/null
> >> +++ b/testcases/network/can/filter-tests/can_common.h
> >> @@ -0,0 +1,75 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Copyright (c) 2021 SUSE LLC
> >> + */
> >> +
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <unistd.h>
> >> +#include <string.h>
> >> +
> >> +#include <sys/types.h>
> >> +#include <sys/socket.h>
> >> +#include <sys/ioctl.h>
> >> +#include <sys/time.h>
> >> +
> >> +#include "tst_cmd.h"
> >> +#include "tst_safe_stdio.h"
> >> +#include "tst_safe_file_ops.h"
> >> +
> >> +#include <linux/if.h>
> >> +#include <linux/can.h>
> >> +#include <linux/can/raw.h>
> >> +
> >> +#ifndef IFF_ECHO
> >> +# define IFF_ECHO (1<<18)
> >> +#endif
Thanks a lot, Richie!

> > IFF_ECHO was included into Linux 2.6.25 together with the CAN
> > subsystem itself.

> > So when you run the tests on Kernels < 2.6.25 you don't have CAN
> > support and don't need IFF_ECHO too.

> Petr, what kernel version and/or distro version did compilation fail on?

> There is a small chance someone might be compiling with old kernel
> headers relative to their kernel. However it is a challenge to compile
> LTP with such an old user land.

No, we don't support 2.6.25 :). I was playing with Buildroot distro in my spare time.
These embedded toolchains suffer compatibility problems (usually uclibc-ng and
sometimes musl lack the support). This problem was when using sourcery-arm-*.

But this is definitely not a blocker for this patchset. That lapi is not a must,
I can fix it some time later. I usually fix few of these problems before each
LTP release.

Kind regards,
Petr

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Oliver Hartkopp]

Hi Petr,

On 26.01.21 22:28, Petr Vorel wrote:
> Hi Oliver, Richie,
> 
>>>> --- /dev/null
>>>> +++ b/testcases/network/can/filter-tests/can_common.h
>>>> @@ -0,0 +1,75 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>> +/*
>>>> + * Copyright (c) 2021 SUSE LLC
>>>> + */
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <stdlib.h>
>>>> +#include <unistd.h>
>>>> +#include <string.h>
>>>> +
>>>> +#include <sys/types.h>
>>>> +#include <sys/socket.h>
>>>> +#include <sys/ioctl.h>
>>>> +#include <sys/time.h>
>>>> +
>>>> +#include "tst_cmd.h"
>>>> +#include "tst_safe_stdio.h"
>>>> +#include "tst_safe_file_ops.h"
>>>> +
>>>> +#include <linux/if.h>
>>>> +#include <linux/can.h>
>>>> +#include <linux/can/raw.h>
>>>> +
>>>> +#ifndef IFF_ECHO
>>>> +# define IFF_ECHO (1<<18)
>>>> +#endif
> Thanks a lot, Richie!
> 
>>> IFF_ECHO was included into Linux 2.6.25 together with the CAN
>>> subsystem itself.
> 
>>> So when you run the tests on Kernels < 2.6.25 you don't have CAN
>>> support and don't need IFF_ECHO too.
> 
>> Petr, what kernel version and/or distro version did compilation fail on?
> 
>> There is a small chance someone might be compiling with old kernel
>> headers relative to their kernel. However it is a challenge to compile
>> LTP with such an old user land.
> 
> No, we don't support 2.6.25 :). I was playing with Buildroot distro in my spare time.
> These embedded toolchains suffer compatibility problems (usually uclibc-ng and
> sometimes musl lack the support). This problem was when using sourcery-arm-*.

:-/

> But this is definitely not a blocker for this patchset. That lapi is not a must,
> I can fix it some time later. I usually fix few of these problems before each
> LTP release.

Ok. No problem. I wasn't aware that e.g. musl or other toolchains select 
such strange starting points for their include files.

Thanks,
Oliver

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Richard Palethorpe]

Hello Petr & Oliver,

Oliver Hartkopp <socketcan@hartkopp.net> writes:

> Hi Petr,
>
> On 26.01.21 22:28, Petr Vorel wrote:
>> Hi Oliver, Richie,
>> 
>>>>> --- /dev/null
>>>>> +++ b/testcases/network/can/filter-tests/can_common.h
>>>>> @@ -0,0 +1,75 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>>> +/*
>>>>> + * Copyright (c) 2021 SUSE LLC
>>>>> + */
>>>>> +
>>>>> +#include <stdio.h>
>>>>> +#include <stdlib.h>
>>>>> +#include <unistd.h>
>>>>> +#include <string.h>
>>>>> +
>>>>> +#include <sys/types.h>
>>>>> +#include <sys/socket.h>
>>>>> +#include <sys/ioctl.h>
>>>>> +#include <sys/time.h>
>>>>> +
>>>>> +#include "tst_cmd.h"
>>>>> +#include "tst_safe_stdio.h"
>>>>> +#include "tst_safe_file_ops.h"
>>>>> +
>>>>> +#include <linux/if.h>
>>>>> +#include <linux/can.h>
>>>>> +#include <linux/can/raw.h>
>>>>> +
>>>>> +#ifndef IFF_ECHO
>>>>> +# define IFF_ECHO (1<<18)
>>>>> +#endif
>> Thanks a lot, Richie!
>> 
>>>> IFF_ECHO was included into Linux 2.6.25 together with the CAN
>>>> subsystem itself.
>> 
>>>> So when you run the tests on Kernels < 2.6.25 you don't have CAN
>>>> support and don't need IFF_ECHO too.
>> 
>>> Petr, what kernel version and/or distro version did compilation fail on?
>> 
>>> There is a small chance someone might be compiling with old kernel
>>> headers relative to their kernel. However it is a challenge to compile
>>> LTP with such an old user land.
>> No, we don't support 2.6.25 :). I was playing with Buildroot distro
>> in my spare time.
>> These embedded toolchains suffer compatibility problems (usually uclibc-ng and
>> sometimes musl lack the support). This problem was when using sourcery-arm-*.
>
> :-/
>
>> But this is definitely not a blocker for this patchset. That lapi is not a must,
>> I can fix it some time later. I usually fix few of these problems before each
>> LTP release.
>
> Ok. No problem. I wasn't aware that e.g. musl or other toolchains
> select such strange starting points for their include files.

I wonder Petr, is it still necessary to define IFF_ECHO now only
<linux/if.h> is included? Or do they somehow symlink linux/if.h ->
net/if.h?

Indeed it seems the current version of uclibc-ng doesn't include
IFF_ECHO in <net/if.h>. OTOH musl does define it.

-- 
Thank you,
Richard.

Re: [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Petr Vorel]

Hi Richie,

<snip>
> >>>> IFF_ECHO was included into Linux 2.6.25 together with the CAN
> >>>> subsystem itself.

> >>>> So when you run the tests on Kernels < 2.6.25 you don't have CAN
> >>>> support and don't need IFF_ECHO too.

> >>> Petr, what kernel version and/or distro version did compilation fail on?

> >>> There is a small chance someone might be compiling with old kernel
> >>> headers relative to their kernel. However it is a challenge to compile
> >>> LTP with such an old user land.
> >> No, we don't support 2.6.25 :). I was playing with Buildroot distro
> >> in my spare time.
> >> These embedded toolchains suffer compatibility problems (usually uclibc-ng and
> >> sometimes musl lack the support). This problem was when using sourcery-arm-*.

> > :-/

> >> But this is definitely not a blocker for this patchset. That lapi is not a must,
> >> I can fix it some time later. I usually fix few of these problems before each
> >> LTP release.

> > Ok. No problem. I wasn't aware that e.g. musl or other toolchains
> > select such strange starting points for their include files.

> I wonder Petr, is it still necessary to define IFF_ECHO now only
> <linux/if.h> is included? Or do they somehow symlink linux/if.h ->
> net/if.h?
No.

> Indeed it seems the current version of uclibc-ng doesn't include
> IFF_ECHO in <net/if.h>. OTOH musl does define it.
Yes => please keep that definition in can_common.h (enough here, we don't have
to bother with if.h when we don't have it yet).
And I'll send tonight patch to uclibc-ng.
The more mature musl is the less relevant uclibc-ng is.

Kind regards,
Petr

Re: [LTP] [PATCH v3 2/7] can: Add can_common.h for vcan device setup

[Petr Vorel]

Hi Richie,

...
> > I wonder Petr, is it still necessary to define IFF_ECHO now only
> > <linux/if.h> is included? Or do they somehow symlink linux/if.h ->
> > net/if.h?
> No.

> > Indeed it seems the current version of uclibc-ng doesn't include
> > IFF_ECHO in <net/if.h>. OTOH musl does define it.
> Yes => please keep that definition in can_common.h (enough here, we don't have
> to bother with if.h when we don't have it yet).
> And I'll send tonight patch to uclibc-ng.
> The more mature musl is the less relevant uclibc-ng is.

Just for a record, I was wrong. Using <linux/if.h> is enough, no need to
keep definition on can_common.h. Sorry for wasting your time.

I mixed two problems: <linux/if.h> and <net/if.h> conflict (there are more headers
which conflict) [1] and sometimes missing definition on uclibc-ng.

*But* musl defines IFF_ECHO in <net/if.h> as they try to allow people not having
to depend on <linux/*.h>, which is IMHO better than blindly relying on
<linux/*.h> which glibc and uclibc{,-ng} (which follows glibc) does much more
than musl:

$ git grep '^#include <linux/.*\.h>' |wc -l
43 # glibc
37 # uclibc-ng
3 # musl

Kind regards,
Petr

[1] https://sourceware.org/glibc/wiki/Synchronizing_Headers#Known_Pairs_of_Headers_that_Conflict