[PATCH can-next v1 0/8] selftests: can: add J1939 socket and transport tests

[PATCH can-next v1 0/8] selftests: can: add J1939 socket and transport tests

[Oleksij Rempel]

net/can/j1939 has had no in-tree selftests. This series adds a kselftest
suite for the J1939 socket layer and the TP/ETP transport, run on vcan with
a CAN_RAW socket as an independent on-the-wire peer.

The whole suite runs on vcan under QEMU and passes. Two limits span the
series and are left for follow-up: the transport timers are checked only
with wide windows, not tight bounds, and the ETP abort/timeout edge cases
are not covered yet (the abort/timeout tests are TP only).

Oleksij Rempel (8):
  selftests: can: add J1939 test helper header and library
  selftests: can: add J1939 base functional tests
  selftests: can: add J1939 socket option and metadata tests
  selftests: can: add J1939 addressing and concurrency tests
  selftests: can: add J1939 wire-format tests against CAN_RAW
  selftests: can: add J1939 TP/ETP multi-frame tests against CAN_RAW
  selftests: can: cover J1939 transport abort and timeout paths
  selftests: can: check SO_J1939_PROMISC receive effect

 tools/testing/selftests/net/can/.gitignore    |   5 +
 tools/testing/selftests/net/can/Makefile      |  20 +-
 tools/testing/selftests/net/can/config        |   2 +
 tools/testing/selftests/net/can/j1939_lib.sh  |  74 ++
 tools/testing/selftests/net/can/j1939_tests.h | 460 +++++++++++++
 .../selftests/net/can/test_j1939_addr.c       | 336 +++++++++
 .../selftests/net/can/test_j1939_addr.sh      |   5 +
 .../selftests/net/can/test_j1939_base.c       | 160 +++++
 .../selftests/net/can/test_j1939_base.sh      |   5 +
 .../selftests/net/can/test_j1939_multi.c      | 638 ++++++++++++++++++
 .../selftests/net/can/test_j1939_multi.sh     |   5 +
 .../selftests/net/can/test_j1939_opts.c       | 347 ++++++++++
 .../selftests/net/can/test_j1939_opts.sh      |   5 +
 .../selftests/net/can/test_j1939_wire.c       | 170 +++++
 .../selftests/net/can/test_j1939_wire.sh      |   5 +
 15 files changed, 2235 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/can/j1939_lib.sh
 create mode 100644 tools/testing/selftests/net/can/j1939_tests.h
 create mode 100644 tools/testing/selftests/net/can/test_j1939_addr.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_addr.sh
 create mode 100644 tools/testing/selftests/net/can/test_j1939_base.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_base.sh
 create mode 100644 tools/testing/selftests/net/can/test_j1939_multi.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_multi.sh
 create mode 100644 tools/testing/selftests/net/can/test_j1939_opts.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_opts.sh
 create mode 100644 tools/testing/selftests/net/can/test_j1939_wire.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_wire.sh

--
2.47.3

[PATCH can-next v1 1/8] selftests: can: add J1939 test helper header and library

[Oleksij Rempel]

Add two shared pieces used by all of the J1939 selftests that follow:

 - j1939_tests.h: the common C helpers, as static inline functions and
   macros grouped and documented by purpose, so each test .c keeps to the
   behaviour it checks rather than the socket and wire plumbing.
 - j1939_lib.sh: the shared vcan setup/cleanup and test runner; a wrapper
   just sets J1939_TEST_PROG and sources it.

They remove boilerplate the tests would otherwise repeat: resolving the
test interface, opening and connecting CAN_J1939 sockets, opening a
CAN_RAW socket, poll()+recv() and send/verify helpers, CAN-ID and
PGN/NAME encoding, the raw TP/ETP frame building blocks, and vcan
bring-up in every shell wrapper.

No tests use them yet; the following patches add the tests on top.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/Makefile      |   2 +
 tools/testing/selftests/net/can/j1939_lib.sh  |  74 +++
 tools/testing/selftests/net/can/j1939_tests.h | 460 ++++++++++++++++++
 3 files changed, 536 insertions(+)
 create mode 100644 tools/testing/selftests/net/can/j1939_lib.sh
 create mode 100644 tools/testing/selftests/net/can/j1939_tests.h

diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index 5b82e60a03e7..e2092c289109 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -6,6 +6,8 @@ CFLAGS += -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INC
 
 TEST_PROGS := test_raw_filter.sh
 
+TEST_FILES := j1939_lib.sh
+
 TEST_GEN_FILES := test_raw_filter
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/j1939_lib.sh b/tools/testing/selftests/net/can/j1939_lib.sh
new file mode 100644
index 000000000000..7b7fff228b16
--- /dev/null
+++ b/tools/testing/selftests/net/can/j1939_lib.sh
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Shared bring-up for the J1939 selftest wrappers. A wrapper sets
+# J1939_TEST_PROG to its test binary and sources this file:
+#
+#	J1939_TEST_PROG=test_j1939_base
+#	source "$(dirname "$0")/j1939_lib.sh"
+#
+# It sets up (and tears down) the CAN interface named by CANIF (a vcan by
+# default), runs the test binary through the kselftest net lib and reports the
+# result.
+
+ALL_TESTS="j1939_run"
+
+net_dir=$(dirname "$0")/..
+source "$net_dir/lib.sh"
+
+export CANIF=${CANIF:-"vcan0"}
+# 250 kbit/s: ISO 11783-2:2002 §3.6 (nominal 4 us bit time); same value in SAE
+# J1939-11:2006 §3.12. J1939-15:1999 §3.14 Table 1 defers to J1939-11.
+# Only the real-CAN path uses this; the default vcan ignores bitrate.
+BITRATE=${BITRATE:-250000}
+
+# Only undo what this script actually did: delete a vcan we created, and bring
+# down an interface only if we brought it up. A failed setup() (e.g. CANIF
+# already exists) must not disturb a pre-existing interface.
+created=0
+brought_up=0
+
+setup()
+{
+	if [[ $CANIF == vcan* ]]; then
+		ip link add name "$CANIF" type vcan || exit $ksft_skip
+		created=1
+	else
+		ip link set dev "$CANIF" type can bitrate "$BITRATE" || exit $ksft_skip
+	fi
+	ip link set dev "$CANIF" up || exit $ksft_skip
+	brought_up=1
+}
+
+cleanup()
+{
+	if [ "$brought_up" = 1 ]; then
+		ip link set dev "$CANIF" down 2>/dev/null
+	fi
+	if [ "$created" = 1 ]; then
+		ip link delete "$CANIF" 2>/dev/null
+	fi
+}
+
+j1939_run()
+{
+	local rc
+
+	./"$J1939_TEST_PROG"
+	rc=$?
+	# The test binary exits ksft_skip when CANIF is missing or the kernel
+	# lacks CAN_J1939; keep that a skip instead of letting check_err turn
+	# any non-zero into a failure.
+	if ((rc == ksft_skip)); then
+		ret_set_ksft_status $ksft_skip "$J1939_TEST_PROG"
+	else
+		check_err $rc
+	fi
+	log_test "$J1939_TEST_PROG"
+}
+
+trap cleanup EXIT INT TERM
+setup
+
+tests_run
+
+exit $EXIT_STATUS
diff --git a/tools/testing/selftests/net/can/j1939_tests.h b/tools/testing/selftests/net/can/j1939_tests.h
new file mode 100644
index 000000000000..dd70a7d60ec7
--- /dev/null
+++ b/tools/testing/selftests/net/can/j1939_tests.h
@@ -0,0 +1,460 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Shared helpers for the J1939 selftests.
+ *
+ * The individual tests are standalone kselftest binaries, so the common
+ * boilerplate (interface lookup, socket open/connect, timed receive, CAN-ID
+ * encoding and the CANIF/main() plumbing) lives here as static inline helpers.
+ *
+ * The including .c file must include "kselftest_harness.h" before this header
+ * (the J1939_TEST_MAIN() macro uses KSFT_SKIP and test_harness_run()).
+ */
+#ifndef __SELFTESTS_NET_CAN_J1939_TESTS_H__
+#define __SELFTESTS_NET_CAN_J1939_TESTS_H__
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <poll.h>
+#include <stdint.h>
+#include <errno.h>
+#include <time.h>
+
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <net/if.h>
+#include <linux/if.h>
+#include <linux/can.h>
+#include <linux/can/raw.h>
+#include <linux/can/j1939.h>
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(a)		(sizeof(a) / sizeof((a)[0]))
+#endif
+
+/* Harness patience for an expected frame (not a protocol timer). */
+#define J1939_RECV_TIMEOUT_MS	2000
+
+/*
+ * Timeout for a check that expects no frame. Long enough that a real
+ * local frame would have arrived, so a timeout means "nothing came".
+ * Much shorter than J1939_RECV_TIMEOUT_MS, to keep the test fast.
+ */
+#define J1939_NEG_RECV_TIMEOUT_MS	300
+
+/* kernel's default J1939 TX priority (net/can/j1939 seeds 6). */
+#define J1939_PRIO_DEFAULT	6
+
+/* J1939 PGN helpers: PF >= 0xf0 selects PDU2 (broadcast, no destination). */
+#define J1939_PF(pgn)		(((pgn) >> 8) & 0xff)
+#define J1939_PS(pgn)		((pgn) & 0xff)
+#define J1939_IS_PDU1(pgn)	(J1939_PF(pgn) < 0xf0)
+
+/* Decoders for the fields of a received 29-bit CAN id (mask with CAN_EFF_MASK). */
+#define J1939_ID_PRIO(id)	(((id) >> 26) & 0x7)
+#define J1939_ID_PF(id)		(((id) >> 16) & 0xff)
+#define J1939_ID_PS(id)		(((id) >> 8) & 0xff)
+#define J1939_ID_SA(id)		((id) & 0xff)
+
+/* Number of 7-byte TP/ETP DT packets needed for len bytes, rounded up. */
+#define J1939_NUM_PKTS(len)	(((len) + 6) / 7)
+
+/*
+ * Size of a CAN_RAW receive buffer large enough for a full ETP burst.
+ * A generous round-number headroom, not a computed minimum; applied
+ * best-effort via SO_RCVBUFFORCE.
+ */
+#define J1939_RAW_BIG_RCVBUF	(8 * 1024 * 1024)
+
+/* Flags for j1939_open_flags(). */
+#define J1939_OPEN_BCAST	(1 << 0)
+
+/* Set by J1939_TEST_MAIN() from the CANIF environment variable. */
+static char CANIF[IFNAMSIZ];
+
+/* Resolve the ifindex of the test interface named by CANIF. */
+static inline int j1939_if_index(void)
+{
+	struct ifreq ifr = {};
+	int s, idx = -1;
+
+	s = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
+	if (s < 0)
+		return -1;
+	strncpy(ifr.ifr_name, CANIF, sizeof(ifr.ifr_name) - 1);
+	if (ioctl(s, SIOCGIFINDEX, &ifr) >= 0)
+		idx = ifr.ifr_ifindex;
+	close(s);
+	return idx;
+}
+
+/*
+ * FIXTURE_SETUP helper: resolve CANIF's ifindex into self->ifindex and abort the
+ * test if it cannot be found. Used as a statement: J1939_SETUP_IFINDEX(self);
+ */
+#define J1939_SETUP_IFINDEX(self)					       \
+	do {								       \
+		(self)->ifindex = j1939_if_index();			       \
+		ASSERT_GE((self)->ifindex, 0)				       \
+			TH_LOG("failed to resolve ifindex for '%s': %d",       \
+			       CANIF, errno);				       \
+	} while (0)
+
+/* Open a J1939 socket bound to (name, sa, pgn); flags may set J1939_OPEN_BCAST. */
+static inline int j1939_open_flags(int ifindex, name_t name, uint8_t sa,
+				   pgn_t pgn, int flags)
+{
+	struct sockaddr_can addr = {
+		.can_family = AF_CAN,
+		.can_ifindex = ifindex,
+	};
+	int s, one = 1;
+
+	s = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
+	if (s < 0)
+		return -1;
+	if ((flags & J1939_OPEN_BCAST) &&
+	    setsockopt(s, SOL_SOCKET, SO_BROADCAST, &one, sizeof(one))) {
+		close(s);
+		return -1;
+	}
+	addr.can_addr.j1939.name = name;
+	addr.can_addr.j1939.addr = sa;
+	addr.can_addr.j1939.pgn = pgn;
+	if (bind(s, (struct sockaddr *)&addr, sizeof(addr))) {
+		close(s);
+		return -1;
+	}
+	return s;
+}
+
+/* Open a J1939 socket bound to (sa, pgn) with no NAME and no broadcast. */
+static inline int j1939_open(int ifindex, uint8_t sa, pgn_t pgn)
+{
+	return j1939_open_flags(ifindex, J1939_NO_NAME, sa, pgn, 0);
+}
+
+/* Connect a J1939 socket to the peer (name, da, pgn). */
+static inline int j1939_connect_name(int s, int ifindex, name_t name,
+				     uint8_t da, pgn_t pgn)
+{
+	struct sockaddr_can addr = {
+		.can_family = AF_CAN,
+		.can_ifindex = ifindex,
+	};
+
+	addr.can_addr.j1939.name = name;
+	addr.can_addr.j1939.addr = da;
+	addr.can_addr.j1939.pgn = pgn;
+	return connect(s, (struct sockaddr *)&addr, sizeof(addr));
+}
+
+static inline int j1939_connect(int s, int ifindex, uint8_t da, pgn_t pgn)
+{
+	return j1939_connect_name(s, ifindex, J1939_NO_NAME, da, pgn);
+}
+
+/* Fill a sockaddr_can describing a J1939 (name, da, pgn) destination. */
+static inline void j1939_fill_sockaddr(struct sockaddr_can *addr, int ifindex,
+				       name_t name, uint8_t da, pgn_t pgn)
+{
+	memset(addr, 0, sizeof(*addr));
+	addr->can_family = AF_CAN;
+	addr->can_ifindex = ifindex;
+	addr->can_addr.j1939.name = name;
+	addr->can_addr.j1939.addr = da;
+	addr->can_addr.j1939.pgn = pgn;
+}
+
+/* sendto() len bytes from buf to the J1939 (name, da, pgn) destination. */
+static inline int j1939_sendto(int s, const void *buf, size_t len, int ifindex,
+			       name_t name, uint8_t da, pgn_t pgn)
+{
+	struct sockaddr_can dst;
+
+	j1939_fill_sockaddr(&dst, ifindex, name, da, pgn);
+	return sendto(s, buf, len, 0, (struct sockaddr *)&dst, sizeof(dst));
+}
+
+/*
+ * Open a CAN_RAW socket on ifindex. rcvbuf > 0 requests that SO_RCVBUFFORCE
+ * size (best-effort: a failure is logged, not fatal).
+ */
+static inline int open_raw_rcvbuf(int ifindex, int rcvbuf)
+{
+	struct sockaddr_can addr = {
+		.can_family = AF_CAN,
+		.can_ifindex = ifindex,
+	};
+	int s;
+
+	s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
+	if (s < 0)
+		return -1;
+	if (rcvbuf > 0 &&
+	    setsockopt(s, SOL_SOCKET, SO_RCVBUFFORCE, &rcvbuf, sizeof(rcvbuf)))
+		fprintf(stderr, "%s: SO_RCVBUFFORCE(%d) failed: %d\n",
+			__func__, rcvbuf, errno);
+	if (bind(s, (struct sockaddr *)&addr, sizeof(addr))) {
+		close(s);
+		return -1;
+	}
+	return s;
+}
+
+static inline int open_raw(int ifindex)
+{
+	return open_raw_rcvbuf(ifindex, 0);
+}
+
+/*
+ * FIXTURE_SETUP helper: open a CAN_RAW socket on self->ifindex into self->raw
+ * with the given SO_RCVBUFFORCE size (0 for the default) and abort the test if
+ * it cannot be opened. Used as a statement: J1939_SETUP_RAW(self, rcvbuf);
+ */
+#define J1939_SETUP_RAW(self, rcvbuf)					       \
+	do {								       \
+		(self)->raw = open_raw_rcvbuf((self)->ifindex, (rcvbuf));      \
+		ASSERT_GE((self)->raw, 0)				       \
+			TH_LOG("failed to open raw socket: %d", errno);	       \
+	} while (0)
+
+/* poll() + recv(): returns the recv() result, or -1 on timeout/error. */
+static inline int recv_timeout(int s, void *buf, size_t len, int timeout_ms)
+{
+	struct pollfd pfd = { .fd = s, .events = POLLIN };
+
+	if (poll(&pfd, 1, timeout_ms) <= 0)
+		return -1;
+	return recv(s, buf, len, 0);
+}
+
+/* Send len bytes on tx and verify they arrive byte-for-byte on rx. 0 / -1. */
+static inline int send_recv_verify(int tx, int rx, const void *buf, size_t len,
+				   int timeout_ms)
+{
+	uint8_t *rx_buf = malloc(len);
+	int rc = -1;
+
+	if (!rx_buf)
+		return -1;
+	if (send(tx, buf, len, 0) == (ssize_t)len &&
+	    recv_timeout(rx, rx_buf, len, timeout_ms) == (int)len &&
+	    memcmp(buf, rx_buf, len) == 0)
+		rc = 0;
+	free(rx_buf);
+	return rc;
+}
+
+/* Encode a J1939 single frame the way the kernel does on transmit. */
+static inline canid_t j1939_canid(uint8_t prio, pgn_t pgn, uint8_t da,
+				  uint8_t sa)
+{
+	canid_t id = CAN_EFF_FLAG | ((canid_t)prio << 26) | (pgn << 8) | sa;
+
+	if (J1939_IS_PDU1(pgn))
+		id |= (canid_t)da << 8;
+	return id;
+}
+
+/* Start an 8-byte TP/ETP control frame: unused CM bytes are 0xff, byte 0 = cmd. */
+static inline void ctl_dat_init(uint8_t *dat, uint8_t cmd)
+{
+	memset(dat, 0xff, 8);
+	dat[0] = cmd;
+}
+
+/* Embed a 24-bit PGN into a TP/ETP control frame (bytes 5..7, little endian). */
+static inline void put_pgn(uint8_t *dat, pgn_t pgn)
+{
+	dat[5] = pgn;
+	dat[6] = pgn >> 8;
+	dat[7] = pgn >> 16;
+}
+
+/* Serialize a 64-bit NAME little-endian into an 8-byte buffer (host-agnostic). */
+static inline void put_name(uint8_t *dat, name_t name)
+{
+	int i;
+
+	for (i = 0; i < 8; i++)
+		dat[i] = (uint8_t)(name >> (8 * i));
+}
+
+/* Little-endian codecs for the TP/ETP CM multi-byte fields (host-agnostic). */
+static inline void put_le16(uint8_t *p, uint16_t v)
+{
+	p[0] = v;
+	p[1] = v >> 8;
+}
+
+static inline void put_le24(uint8_t *p, uint32_t v)
+{
+	p[0] = v;
+	p[1] = v >> 8;
+	p[2] = v >> 16;
+}
+
+static inline void put_le32(uint8_t *p, uint32_t v)
+{
+	p[0] = v;
+	p[1] = v >> 8;
+	p[2] = v >> 16;
+	p[3] = v >> 24;
+}
+
+static inline uint16_t get_le16(const uint8_t *p)
+{
+	return (uint16_t)(p[0] | (p[1] << 8));
+}
+
+static inline uint32_t get_le24(const uint8_t *p)
+{
+	return (uint32_t)p[0] | ((uint32_t)p[1] << 8) | ((uint32_t)p[2] << 16);
+}
+
+static inline uint32_t get_le32(const uint8_t *p)
+{
+	return (uint32_t)p[0] | ((uint32_t)p[1] << 8) |
+	       ((uint32_t)p[2] << 16) | ((uint32_t)p[3] << 24);
+}
+
+/* Send one 8-byte J1939-encoded frame (any PGN) with the given priority. */
+static inline int send_raw8(int raw, uint8_t prio, pgn_t pgn, uint8_t da,
+			    uint8_t sa, const uint8_t dat[8])
+{
+	struct can_frame f = {};
+
+	f.can_id = j1939_canid(prio, pgn, da, sa);
+	f.can_dlc = 8;
+	memcpy(f.data, dat, 8);
+	return write(raw, &f, sizeof(f)) == (int)sizeof(f) ? 0 : -1;
+}
+
+/* Send a TP/ETP data (DT) frame carrying seq + up to 7 payload bytes. */
+static inline int send_dt(int raw, uint8_t prio, pgn_t dat_pgn, uint8_t da,
+			  uint8_t sa, uint8_t seq, const uint8_t *payload,
+			  size_t off, size_t total)
+{
+	struct can_frame f = {};
+	size_t n;
+
+	/* off >= total would make the size_t subtraction total - off underflow. */
+	if (off >= total)
+		return -1;
+	n = total - off;
+	if (n > 7)
+		n = 7;
+	f.can_id = j1939_canid(prio, dat_pgn, da, sa);
+	f.can_dlc = 8;
+	memset(f.data, 0xff, 8);
+	f.data[0] = seq;
+	memcpy(&f.data[1], payload + off, n);
+	return write(raw, &f, sizeof(f)) == (int)sizeof(f) ? 0 : -1;
+}
+
+/* Elapsed milliseconds between two CLOCK_MONOTONIC samples. */
+static inline long timespec_diff_ms(const struct timespec *a, const struct timespec *b)
+{
+	return (b->tv_sec - a->tv_sec) * 1000 + (b->tv_nsec - a->tv_nsec) / 1000000;
+}
+
+/* Poll+recv on raw until match() accepts a frame, else timeout. 0/-1. */
+static inline int recv_match(int raw, int timeout_ms, struct can_frame *frame,
+			     bool (*match)(canid_t id, const void *ctx),
+			     const void *ctx)
+{
+	struct pollfd pfd = { .fd = raw, .events = POLLIN };
+	struct timespec now, deadline;
+	int remaining = timeout_ms;
+
+	/*
+	 * Non-matching frames (other traffic on the bus) must not refresh the
+	 * timeout, or a busy bus would keep the loop alive forever. Poll against
+	 * a fixed deadline and shrink the budget after every frame.
+	 */
+	clock_gettime(CLOCK_MONOTONIC, &deadline);
+	deadline.tv_sec += timeout_ms / 1000;
+	deadline.tv_nsec += (long)(timeout_ms % 1000) * 1000000;
+	if (deadline.tv_nsec >= 1000000000) {
+		deadline.tv_sec++;
+		deadline.tv_nsec -= 1000000000;
+	}
+	while (remaining > 0 && poll(&pfd, 1, remaining) > 0) {
+		if (recv(raw, frame, sizeof(*frame), 0) < 0)
+			return -1;
+		if (match(frame->can_id & CAN_EFF_MASK, ctx))
+			return 0;
+		clock_gettime(CLOCK_MONOTONIC, &now);
+		remaining = timespec_diff_ms(&now, &deadline);
+	}
+	return -1;
+}
+
+struct j1939_peer_match { uint8_t want_pf, want_da; };
+static inline bool match_peer_dest(canid_t id, const void *ctx)
+{
+	const struct j1939_peer_match *m = ctx;
+
+	return J1939_ID_PF(id) == m->want_pf &&
+	       J1939_ID_PS(id) == m->want_da;		/* destination */
+}
+
+/* Receive the next frame whose CAN-id PF == want_pf addressed to want_da (PS). */
+static inline int recv_for_peer(int raw, uint8_t want_pf, uint8_t want_da,
+				struct can_frame *frame, int timeout_ms)
+{
+	struct j1939_peer_match m = { .want_pf = want_pf, .want_da = want_da };
+
+	return recv_match(raw, timeout_ms, frame, match_peer_dest, &m);
+}
+
+/* recv_for_peer() + check the control command byte; 0 on match, -1 otherwise. */
+static inline int recv_ctl_cmd(int raw, uint8_t want_pf, uint8_t want_da,
+			       struct can_frame *frame, int timeout_ms,
+			       uint8_t want_cmd)
+{
+	if (recv_for_peer(raw, want_pf, want_da, frame, timeout_ms))
+		return -1;
+	return frame->data[0] == want_cmd ? 0 : -1;
+}
+
+/* Fill buf with a deterministic, position-dependent byte pattern. */
+static inline void fill_pattern(uint8_t *buf, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		buf[i] = (uint8_t)(i ^ (i >> 8) ^ 0x3c);
+}
+
+/*
+ * Define main(): require CANIF in the environment and a kernel that supports
+ * CAN_J1939, then run the harness. Skip (rather than fail) when either is
+ * missing, per the kselftest skip contract.
+ */
+#define J1939_TEST_MAIN()						       \
+int main(int argc, char **argv)						       \
+{									       \
+	char *ifname = getenv("CANIF");					       \
+	int probe;							       \
+									       \
+	if (!ifname) {							       \
+		fprintf(stderr,						       \
+			"CANIF environment variable must contain the test interface\n"); \
+		return KSFT_SKIP;					       \
+	}								       \
+	strncpy(CANIF, ifname, sizeof(CANIF) - 1);			       \
+									       \
+	probe = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);			       \
+	if (probe < 0) {						       \
+		if (errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT)	       \
+			return KSFT_SKIP;				       \
+	} else {							       \
+		close(probe);						       \
+	}								       \
+	return test_harness_run(argc, argv);				       \
+}
+
+#endif /* __SELFTESTS_NET_CAN_J1939_TESTS_H__ */
-- 
2.47.3

[PATCH can-next v1 2/8] selftests: can: add J1939 base functional tests

[Oleksij Rempel]

Add a functional selftest for the SAE J1939 socket interface. It runs two
CAN_J1939 sockets (a sender and a receiver) over a vcan interface and
checks payload integrity across the transport-size transitions: single
frame (PDU1), Transport Protocol, Extended Transport Protocol, the exact
size boundaries around those transitions, and single-frame and BAM
broadcast (PDU2).

Declare j1939_tests.h as a local header dependency and enable
CONFIG_CAN_J1939 in the test config.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/.gitignore    |   1 +
 tools/testing/selftests/net/can/Makefile      |  10 +-
 tools/testing/selftests/net/can/config        |   1 +
 .../selftests/net/can/test_j1939_base.c       | 160 ++++++++++++++++++
 .../selftests/net/can/test_j1939_base.sh      |   5 +
 5 files changed, 175 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/can/test_j1939_base.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_base.sh

diff --git a/tools/testing/selftests/net/can/.gitignore b/tools/testing/selftests/net/can/.gitignore
index 764a53fc837f..0ab563fe9ddf 100644
--- a/tools/testing/selftests/net/can/.gitignore
+++ b/tools/testing/selftests/net/can/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 test_raw_filter
+test_j1939_base
diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index e2092c289109..cf4bd2461a6c 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -4,10 +4,16 @@ top_srcdir = ../../../../..
 
 CFLAGS += -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INCLUDES)
 
-TEST_PROGS := test_raw_filter.sh
+LOCAL_HDRS := j1939_tests.h
+
+TEST_PROGS := \
+	test_raw_filter.sh \
+	test_j1939_base.sh \
 
 TEST_FILES := j1939_lib.sh
 
-TEST_GEN_FILES := test_raw_filter
+TEST_GEN_FILES := \
+	test_raw_filter \
+	test_j1939_base \
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/config b/tools/testing/selftests/net/can/config
index 188f79796670..52a186bd1055 100644
--- a/tools/testing/selftests/net/can/config
+++ b/tools/testing/selftests/net/can/config
@@ -1,3 +1,4 @@
 CONFIG_CAN=m
 CONFIG_CAN_DEV=m
 CONFIG_CAN_VCAN=m
+CONFIG_CAN_J1939=m
diff --git a/tools/testing/selftests/net/can/test_j1939_base.c b/tools/testing/selftests/net/can/test_j1939_base.c
new file mode 100644
index 000000000000..59f1448b7fb2
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_base.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * J1939 base functional selftests.
+ *
+ * Run two SOCK_DGRAM/CAN_J1939 sockets (a sender and a receiver) over a vcan
+ * interface and verify payload integrity across the transport-size transitions:
+ * a single frame (PDU1), Transport Protocol, Extended Transport Protocol, the
+ * exact boundaries between them, and single-frame and BAM broadcast (PDU2).
+ *
+ * The vcan interface is set up by the caller (see test_j1939_base.sh); its name
+ * is passed via the CANIF environment variable.
+ */
+#include <errno.h>
+
+#include "kselftest_harness.h"
+#include "j1939_tests.h"
+
+#define SA_TX			0x80
+#define SA_RX			0x90
+#define PGN_PDU1		0x0ab00	/* PF 0xab (< 240): destination specific */
+#define PGN_PDU2		0x0feee	/* PF 0xfe (>= 240): broadcast */
+
+FIXTURE(j1939_base)
+{
+	int ifindex;
+	int tx;
+	int rx;
+};
+
+FIXTURE_SETUP(j1939_base)
+{
+	self->tx = -1;
+	self->rx = -1;
+	J1939_SETUP_IFINDEX(self);
+}
+
+FIXTURE_TEARDOWN(j1939_base)
+{
+	if (self->tx >= 0)
+		close(self->tx);
+	if (self->rx >= 0)
+		close(self->rx);
+}
+
+/*
+ * Set up a receiver bound to (rx_addr, pgn) and a sender bound to SA_TX and
+ * connected to (tx_dst, pgn). Any pair opened by a previous call is closed
+ * first. Returns 0 on success, -1 otherwise.
+ */
+static int pair_up(FIXTURE_DATA(j1939_base) *self, uint8_t rx_addr,
+		   uint8_t tx_dst, pgn_t pgn, int bcast)
+{
+	int flags = bcast ? J1939_OPEN_BCAST : 0;
+
+	if (self->tx >= 0) {
+		close(self->tx);
+		self->tx = -1;
+	}
+	if (self->rx >= 0) {
+		close(self->rx);
+		self->rx = -1;
+	}
+
+	self->rx = j1939_open_flags(self->ifindex, J1939_NO_NAME, rx_addr, pgn,
+				   flags);
+	if (self->rx < 0) {
+		fprintf(stderr, "# open rx failed: %d\n", errno);
+		return -1;
+	}
+
+	self->tx = j1939_open_flags(self->ifindex, J1939_NO_NAME, SA_TX,
+				   J1939_NO_PGN, flags);
+	if (self->tx < 0) {
+		fprintf(stderr, "# open tx failed: %d\n", errno);
+		return -1;
+	}
+
+	if (j1939_connect(self->tx, self->ifindex, tx_dst, pgn)) {
+		fprintf(stderr, "# connect tx failed: %d\n", errno);
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Send a deterministic payload of the given size on tx and verify it is
+ * received byte for byte on rx. Returns 0 on success, -1 otherwise.
+ */
+static int roundtrip(FIXTURE_DATA(j1939_base) *self, size_t size)
+{
+	uint8_t *tx_buf;
+	int rc;
+
+	tx_buf = malloc(size);
+	if (!tx_buf) {
+		fprintf(stderr, "# alloc failed\n");
+		return -1;
+	}
+	fill_pattern(tx_buf, size);
+
+	rc = send_recv_verify(self->tx, self->rx, tx_buf, size,
+			      J1939_RECV_TIMEOUT_MS);
+	if (rc)
+		fprintf(stderr, "# round-trip failed at size %zu, errno %d\n",
+			size, errno);
+
+	free(tx_buf);
+	return rc;
+}
+
+/* single frame (<= 8 bytes), unicast PDU1. */
+TEST_F(j1939_base, single_frame_pdu1)
+{
+	ASSERT_EQ(pair_up(self, SA_RX, SA_RX, PGN_PDU1, 0), 0);
+	EXPECT_EQ(roundtrip(self, 8), 0);
+}
+
+/* Transport Protocol (9 .. 1785 bytes), unicast. */
+TEST_F(j1939_base, tp_roundtrip)
+{
+	ASSERT_EQ(pair_up(self, SA_RX, SA_RX, PGN_PDU1, 0), 0);
+	EXPECT_EQ(roundtrip(self, 128), 0);
+}
+
+/* Extended Transport Protocol (> 1785 bytes), unicast. */
+TEST_F(j1939_base, etp_roundtrip)
+{
+	ASSERT_EQ(pair_up(self, SA_RX, SA_RX, PGN_PDU1, 0), 0);
+	EXPECT_EQ(roundtrip(self, 2000), 0);
+}
+
+/* exact size boundaries around single / TP / ETP transitions. */
+TEST_F(j1939_base, size_boundaries)
+{
+	static const size_t sizes[] = { 8, 9, 1785, 1786 };
+	size_t k;
+
+	for (k = 0; k < ARRAY_SIZE(sizes); k++) {
+		ASSERT_EQ(pair_up(self, SA_RX, SA_RX, PGN_PDU1, 0), 0);
+		EXPECT_EQ(roundtrip(self, sizes[k]), 0)
+			TH_LOG("failed at boundary size %zu", sizes[k]);
+	}
+}
+
+/* single-frame broadcast, PDU2. */
+TEST_F(j1939_base, single_frame_broadcast_pdu2)
+{
+	ASSERT_EQ(pair_up(self, SA_RX, J1939_NO_ADDR, PGN_PDU2, 1), 0);
+	EXPECT_EQ(roundtrip(self, 8), 0);
+}
+
+/* BAM (broadcast transport). */
+TEST_F(j1939_base, bam_broadcast)
+{
+	ASSERT_EQ(pair_up(self, SA_RX, J1939_NO_ADDR, PGN_PDU2, 1), 0);
+	EXPECT_EQ(roundtrip(self, 128), 0);
+}
+
+J1939_TEST_MAIN();
diff --git a/tools/testing/selftests/net/can/test_j1939_base.sh b/tools/testing/selftests/net/can/test_j1939_base.sh
new file mode 100755
index 000000000000..3cd2b14a74bb
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_base.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+J1939_TEST_PROG=test_j1939_base
+source "$(dirname "$0")/j1939_lib.sh"
-- 
2.47.3

[PATCH can-next v1 3/8] selftests: can: add J1939 socket option and metadata tests

[Oleksij Rempel]

Add a selftest covering the J1939 socket options and send metadata: the
PGN filter (SO_J1939_FILTER), the send priority (SO_J1939_SEND_PRIO,
observed on the wire via a CAN_RAW peer), the promisc flag, destination
addressing via sendto(), a wrong-family bind, and the TX-completion
error-queue notification (SO_J1939_ERRQUEUE / SO_TIMESTAMPING), including
that it is labelled SCM_TSTAMP_ACK. setsockopt values are also read back
with getsockopt.

This is the first J1939 test to open a CAN_RAW socket, so enable
CONFIG_CAN_RAW in the test config.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/.gitignore    |   1 +
 tools/testing/selftests/net/can/Makefile      |   2 +
 tools/testing/selftests/net/can/config        |   1 +
 .../selftests/net/can/test_j1939_opts.c       | 305 ++++++++++++++++++
 .../selftests/net/can/test_j1939_opts.sh      |   5 +
 5 files changed, 314 insertions(+)
 create mode 100644 tools/testing/selftests/net/can/test_j1939_opts.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_opts.sh

diff --git a/tools/testing/selftests/net/can/.gitignore b/tools/testing/selftests/net/can/.gitignore
index 0ab563fe9ddf..427eed83e49e 100644
--- a/tools/testing/selftests/net/can/.gitignore
+++ b/tools/testing/selftests/net/can/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 test_raw_filter
 test_j1939_base
+test_j1939_opts
diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index cf4bd2461a6c..24a57edb912c 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -9,11 +9,13 @@ LOCAL_HDRS := j1939_tests.h
 TEST_PROGS := \
 	test_raw_filter.sh \
 	test_j1939_base.sh \
+	test_j1939_opts.sh \
 
 TEST_FILES := j1939_lib.sh
 
 TEST_GEN_FILES := \
 	test_raw_filter \
 	test_j1939_base \
+	test_j1939_opts \
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/config b/tools/testing/selftests/net/can/config
index 52a186bd1055..908342600101 100644
--- a/tools/testing/selftests/net/can/config
+++ b/tools/testing/selftests/net/can/config
@@ -2,3 +2,4 @@ CONFIG_CAN=m
 CONFIG_CAN_DEV=m
 CONFIG_CAN_VCAN=m
 CONFIG_CAN_J1939=m
+CONFIG_CAN_RAW=m
diff --git a/tools/testing/selftests/net/can/test_j1939_opts.c b/tools/testing/selftests/net/can/test_j1939_opts.c
new file mode 100644
index 000000000000..f761a42db213
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_opts.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * J1939 socket option and metadata selftests.
+ *
+ * On top of the basic transport, cover the user-visible J1939 socket options and
+ * addressing: the SO_J1939_FILTER PGN whitelist, the SO_J1939_SEND_PRIO send
+ * priority seen in the CAN id (via a CAN_RAW peer, including the default of 6),
+ * unconnected sendto(2), a wrong-family bind rejection, and SO_J1939_ERRQUEUE
+ * TX-completion notifications.
+ *
+ * The vcan interface is set up by the caller (see test_j1939_opts.sh); its name
+ * is passed via the CANIF environment variable.
+ */
+#include <errno.h>
+#include <time.h>
+
+#include "kselftest_harness.h"
+#include "j1939_tests.h"
+
+#include <linux/net_tstamp.h>
+#include <linux/errqueue.h>
+
+#define SA_TX			0x80
+#define SA_RX			0x90
+#define PGN_A			0x0ab00
+#define PGN_B			0x0cd00
+#define PRIO_SET		3
+/*
+ * Worst-case error-queue drain budget: each iteration can block up to
+ * J1939_RECV_TIMEOUT_MS, so this bounds the drain time. A generous cap; only one
+ * ACK is expected.
+ */
+#define ERRQUEUE_POLL_RETRIES	8
+
+/* Read the next CAN frame from a raw socket and return its J1939 priority. */
+static int raw_prio(int raw, int timeout_ms)
+{
+	struct can_frame frame;
+
+	if (recv_timeout(raw, &frame, sizeof(frame), timeout_ms) < 0)
+		return -1;
+	return (int)J1939_ID_PRIO(frame.can_id & CAN_EFF_MASK);
+}
+
+/* Set an int J1939 sockopt, read it back, and return the value (or -1). */
+static int sockopt_roundtrip(int s, int optname, int val)
+{
+	int got = -1;
+	socklen_t len = sizeof(got);
+
+	if (setsockopt(s, SOL_CAN_J1939, optname, &val, sizeof(val)) < 0)
+		return -1;
+	if (getsockopt(s, SOL_CAN_J1939, optname, &got, &len) < 0)
+		return -1;
+	return got;
+}
+
+FIXTURE(j1939_opts)
+{
+	int ifindex;
+	int tx;
+	int rx;
+	int raw;
+};
+
+FIXTURE_SETUP(j1939_opts)
+{
+	self->tx = -1;
+	self->rx = -1;
+	self->raw = -1;
+
+	J1939_SETUP_IFINDEX(self);
+
+	J1939_SETUP_RAW(self, 0);
+}
+
+FIXTURE_TEARDOWN(j1939_opts)
+{
+	if (self->tx >= 0)
+		close(self->tx);
+	if (self->rx >= 0)
+		close(self->rx);
+	if (self->raw >= 0)
+		close(self->raw);
+}
+
+/* SO_J1939_FILTER whitelists PGN_A; PGN_A is received, PGN_B is dropped. */
+TEST_F(j1939_opts, filter_pgn)
+{
+	struct j1939_filter filt = {
+		.pgn = PGN_A,
+		.pgn_mask = J1939_PGN_MAX,
+	};
+	uint8_t tx_buf[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+	uint8_t rx_buf[8];
+	int ret;
+
+	self->rx = j1939_open(self->ifindex, SA_RX, J1939_NO_PGN);
+	ASSERT_GE(self->rx, 0);
+	ASSERT_EQ(setsockopt(self->rx, SOL_CAN_J1939, SO_J1939_FILTER,
+			     &filt, sizeof(filt)), 0)
+		TH_LOG("SO_J1939_FILTER failed: %d", errno);
+
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+
+	/* PGN_A must pass the filter */
+	ret = j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			   J1939_NO_NAME, SA_RX, PGN_A);
+	ASSERT_EQ(ret, (int)sizeof(tx_buf))
+		TH_LOG("sendto PGN_A failed: %d", errno);
+	EXPECT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_RECV_TIMEOUT_MS), (int)sizeof(tx_buf))
+		TH_LOG("PGN_A was not received despite matching filter");
+
+	/* PGN_B must be dropped by the filter */
+	ret = j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			   J1939_NO_NAME, SA_RX, PGN_B);
+	ASSERT_EQ(ret, (int)sizeof(tx_buf))
+		TH_LOG("sendto PGN_B failed: %d", errno);
+	EXPECT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_NEG_RECV_TIMEOUT_MS), -1)
+		TH_LOG("PGN_B was received but should have been filtered out");
+}
+
+/* without SO_J1939_SEND_PRIO the J1939 default priority (6) is used. */
+TEST_F(j1939_opts, send_prio_default)
+{
+	uint8_t buf[8] = {};
+
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+
+	ASSERT_EQ(j1939_sendto(self->tx, buf, sizeof(buf), self->ifindex,
+			       J1939_NO_NAME, SA_RX, PGN_A),
+		  (int)sizeof(buf));
+
+	EXPECT_EQ(raw_prio(self->raw, J1939_RECV_TIMEOUT_MS), J1939_PRIO_DEFAULT);
+}
+
+/* SO_J1939_SEND_PRIO changes the priority carried in the CAN id. */
+TEST_F(j1939_opts, send_prio_set)
+{
+	int prio = PRIO_SET;
+	uint8_t buf[8] = {};
+
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	ASSERT_EQ(setsockopt(self->tx, SOL_CAN_J1939, SO_J1939_SEND_PRIO,
+			     &prio, sizeof(prio)), 0)
+		TH_LOG("SO_J1939_SEND_PRIO failed: %d", errno);
+
+	ASSERT_EQ(j1939_sendto(self->tx, buf, sizeof(buf), self->ifindex,
+			       J1939_NO_NAME, SA_RX, PGN_A),
+		  (int)sizeof(buf));
+
+	EXPECT_EQ(raw_prio(self->raw, J1939_RECV_TIMEOUT_MS), PRIO_SET);
+}
+
+/* an unconnected socket can address a peer via sendto(2). */
+TEST_F(j1939_opts, sendto_unconnected)
+{
+	uint8_t tx_buf[8] = { 9, 8, 7, 6, 5, 4, 3, 2 };
+	uint8_t rx_buf[8];
+
+	self->rx = j1939_open(self->ifindex, SA_RX, PGN_A);
+	ASSERT_GE(self->rx, 0);
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+
+	ASSERT_EQ(j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			       J1939_NO_NAME, SA_RX, PGN_A),
+		  (int)sizeof(tx_buf));
+
+	ASSERT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_RECV_TIMEOUT_MS), (int)sizeof(tx_buf));
+	EXPECT_EQ(memcmp(tx_buf, rx_buf, sizeof(tx_buf)), 0);
+}
+
+/* bind(2) with a wrong address family is rejected. */
+TEST_F(j1939_opts, bind_wrong_family)
+{
+	struct sockaddr_can addr = {
+		.can_family = AF_INET,
+		.can_ifindex = self->ifindex,
+	};
+	int s;
+
+	s = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
+	ASSERT_GE(s, 0);
+	EXPECT_LT(bind(s, (struct sockaddr *)&addr, sizeof(addr)), 0)
+		TH_LOG("bind with AF_INET unexpectedly succeeded");
+	close(s);
+}
+
+/* SO_J1939_ERRQUEUE delivers TX completion notifications for a TP send. */
+TEST_F(j1939_opts, errqueue_tx_completion)
+{
+	int enable = 1;
+	int ts = SOF_TIMESTAMPING_TX_ACK | SOF_TIMESTAMPING_TX_SCHED |
+		 SOF_TIMESTAMPING_OPT_ID;
+	uint8_t tx_buf[128], rx_buf[128];
+	int got_ack = 0, i;
+
+	memset(tx_buf, 0x5a, sizeof(tx_buf));
+
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	/*
+	 * setsockopt(SO_J1939_ERRQUEUE) returns the flag value (1) on success
+	 * rather than 0, so only check for a non-negative result.
+	 */
+	ASSERT_GE(setsockopt(self->tx, SOL_CAN_J1939, SO_J1939_ERRQUEUE,
+			     &enable, sizeof(enable)), 0)
+		TH_LOG("SO_J1939_ERRQUEUE failed: %d", errno);
+	ASSERT_EQ(setsockopt(self->tx, SOL_SOCKET, SO_TIMESTAMPING,
+			     &ts, sizeof(ts)), 0)
+		TH_LOG("SO_TIMESTAMPING failed: %d", errno);
+
+	/* A receiver lets the TP session complete and be acknowledged. */
+	self->rx = j1939_open(self->ifindex, SA_RX, PGN_A);
+	ASSERT_GE(self->rx, 0);
+
+	ASSERT_EQ(j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			       J1939_NO_NAME, SA_RX, PGN_A),
+		  (int)sizeof(tx_buf));
+
+	ASSERT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_RECV_TIMEOUT_MS), (int)sizeof(tx_buf));
+
+	/*
+	 * Collect the TX-completion (ACK) from the error queue. Allow a few extra
+	 * iterations because other control messages may arrive before the ACK.
+	 */
+	for (i = 0; i < ERRQUEUE_POLL_RETRIES && !got_ack; i++) {
+		struct pollfd pfd = { .fd = self->tx, .events = POLLERR };
+		union {
+			char buf[CMSG_SPACE(sizeof(struct sock_extended_err)) +
+				 CMSG_SPACE(sizeof(struct scm_timestamping))];
+			struct cmsghdr align;	/* portable cmsg alignment */
+		} ctrl;
+		char data[128];
+		struct iovec iov = { .iov_base = data, .iov_len = sizeof(data) };
+		struct msghdr msg = {
+			.msg_iov = &iov,
+			.msg_iovlen = 1,
+			.msg_control = ctrl.buf,
+			.msg_controllen = sizeof(ctrl.buf),
+		};
+		struct cmsghdr *cmsg;
+
+		if (poll(&pfd, 1, J1939_RECV_TIMEOUT_MS) <= 0)
+			break;
+		if (recvmsg(self->tx, &msg, MSG_ERRQUEUE) < 0)
+			break;
+
+		for (cmsg = CMSG_FIRSTHDR(&msg); cmsg;
+		     cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+			struct sock_extended_err *serr;
+
+			if (cmsg->cmsg_level != SOL_CAN_J1939 ||
+			    cmsg->cmsg_type != SCM_J1939_ERRQUEUE)
+				continue;
+			serr = (struct sock_extended_err *)CMSG_DATA(cmsg);
+			if (serr->ee_origin != SO_EE_ORIGIN_TIMESTAMPING)
+				continue;
+			/* The TX-completion notification must be SCM_TSTAMP_ACK. */
+			if (serr->ee_info == SCM_TSTAMP_ACK)
+				got_ack = 1;
+		}
+	}
+
+	EXPECT_EQ(got_ack, 1)
+		TH_LOG("no SCM_TSTAMP_ACK TX completion on the error queue");
+}
+
+/* getsockopt reads back SO_J1939_SEND_PRIO. */
+TEST_F(j1939_opts, getsockopt_send_prio)
+{
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	EXPECT_EQ(sockopt_roundtrip(self->tx, SO_J1939_SEND_PRIO, PRIO_SET),
+		  PRIO_SET)
+		TH_LOG("SO_J1939_SEND_PRIO round-trip failed: %d", errno);
+}
+
+/* getsockopt reads back the SO_J1939_PROMISC flag. */
+TEST_F(j1939_opts, getsockopt_promisc)
+{
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	EXPECT_EQ(sockopt_roundtrip(self->tx, SO_J1939_PROMISC, 1), 1)
+		TH_LOG("SO_J1939_PROMISC round-trip failed: %d", errno);
+}
+
+/* getsockopt reads back the SO_J1939_ERRQUEUE flag. */
+TEST_F(j1939_opts, getsockopt_errqueue)
+{
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	EXPECT_EQ(sockopt_roundtrip(self->tx, SO_J1939_ERRQUEUE, 1), 1)
+		TH_LOG("SO_J1939_ERRQUEUE round-trip failed: %d", errno);
+}
+
+J1939_TEST_MAIN();
diff --git a/tools/testing/selftests/net/can/test_j1939_opts.sh b/tools/testing/selftests/net/can/test_j1939_opts.sh
new file mode 100755
index 000000000000..5d76c56f24a4
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_opts.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+J1939_TEST_PROG=test_j1939_opts
+source "$(dirname "$0")/j1939_lib.sh"
-- 
2.47.3

[PATCH can-next v1 4/8] selftests: can: add J1939 addressing and concurrency tests

[Oleksij Rempel]

Add a selftest for J1939 NAME-based addressing and address claiming:
binding and reading back a NAME, the source address seen on the wire,
NAME-addressed transfers (with a forged Address Claimed), the Address
Claimed a socket emits, and address-claim conflicts where the contender
with the higher NAME loses and the one with the lower NAME keeps the
address. It also runs several concurrent transport sessions as a stress
check.

The NAME is serialized onto the wire byte-wise (put_name) so the tests
behave the same on big- and little-endian hosts.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/.gitignore    |   1 +
 tools/testing/selftests/net/can/Makefile      |   2 +
 .../selftests/net/can/test_j1939_addr.c       | 336 ++++++++++++++++++
 .../selftests/net/can/test_j1939_addr.sh      |   5 +
 4 files changed, 344 insertions(+)
 create mode 100644 tools/testing/selftests/net/can/test_j1939_addr.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_addr.sh

diff --git a/tools/testing/selftests/net/can/.gitignore b/tools/testing/selftests/net/can/.gitignore
index 427eed83e49e..7ca63a19472c 100644
--- a/tools/testing/selftests/net/can/.gitignore
+++ b/tools/testing/selftests/net/can/.gitignore
@@ -2,3 +2,4 @@
 test_raw_filter
 test_j1939_base
 test_j1939_opts
+test_j1939_addr
diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index 24a57edb912c..6e4176a1b985 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -10,6 +10,7 @@ TEST_PROGS := \
 	test_raw_filter.sh \
 	test_j1939_base.sh \
 	test_j1939_opts.sh \
+	test_j1939_addr.sh \
 
 TEST_FILES := j1939_lib.sh
 
@@ -17,5 +18,6 @@ TEST_GEN_FILES := \
 	test_raw_filter \
 	test_j1939_base \
 	test_j1939_opts \
+	test_j1939_addr \
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/test_j1939_addr.c b/tools/testing/selftests/net/can/test_j1939_addr.c
new file mode 100644
index 000000000000..8b1b093f0839
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_addr.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * J1939 addressing and concurrency selftests.
+ *
+ * Cover NAME-based addressing and address claiming: binding a NAME and reading
+ * it back, the bound source address appearing in the CAN id, a sender reaching a
+ * peer by NAME, address-claim contention where the lower NAME wins, and a stress
+ * run of many concurrent transport sessions.
+ *
+ * The vcan interface is set up by the caller (see test_j1939_addr.sh); its name
+ * is passed via the CANIF environment variable.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/wait.h>
+
+#include "kselftest_harness.h"
+#include "j1939_tests.h"
+
+#define SA_TX			0x80
+#define SA_RX			0x90
+#define NAME_TX			0x00000000c0ffee01ULL
+#define NAME_RX			0x00000000c0ffee02ULL
+#define PGN_TEST		0x0ab00
+#define STRESS_N		8	/* arbitrary test tuning */
+#define STRESS_SIZE		128	/* arbitrary test tuning */
+#define ACLAIM_SETTLE_US	300000	/* settle: empirical, not spec */
+
+/* Open a J1939 socket bound to (name, sa, pgn). */
+static int j1939_open_name(int ifindex, name_t name, uint8_t sa, pgn_t pgn)
+{
+	return j1939_open_flags(ifindex, name, sa, pgn, 0);
+}
+
+/*
+ * Inject an Address Claimed message (PGN 0xee00, global destination) for the
+ * given source address and NAME via a raw socket, so the kernel learns the
+ * NAME <-> address mapping. The kernel does not claim addresses itself.
+ */
+static int forge_address_claim(int raw, uint8_t sa, name_t name)
+{
+	uint8_t nb[8];
+
+	put_name(nb, name);
+	if (send_raw8(raw, J1939_PRIO_DEFAULT, J1939_PGN_ADDRESS_CLAIMED,
+		      J1939_NO_ADDR, sa, nb))
+		return -1;
+	/* give the kernel RX path a moment to process the claim */
+	usleep(ACLAIM_SETTLE_US);
+	return 0;
+}
+
+/* One independent transport-protocol round-trip; returns 0 on success. */
+static int stress_child(int ifindex, uint8_t sa_tx, uint8_t sa_rx)
+{
+	uint8_t tx_buf[STRESS_SIZE];
+	int tx, rx, rc = 1;
+
+	fill_pattern(tx_buf, sizeof(tx_buf));
+
+	rx = j1939_open_name(ifindex, J1939_NO_NAME, sa_rx, PGN_TEST);
+	tx = j1939_open_name(ifindex, J1939_NO_NAME, sa_tx, J1939_NO_PGN);
+	if (rx < 0 || tx < 0)
+		goto out;
+	if (j1939_connect_name(tx, ifindex, J1939_NO_NAME, sa_rx, PGN_TEST))
+		goto out;
+
+	if (send_recv_verify(tx, rx, tx_buf, sizeof(tx_buf),
+			     J1939_RECV_TIMEOUT_MS) == 0)
+		rc = 0;
+out:
+	if (rx >= 0)
+		close(rx);
+	if (tx >= 0)
+		close(tx);
+	return rc;
+}
+
+FIXTURE(j1939_addr)
+{
+	int ifindex;
+	int tx;
+	int rx;
+	int raw;
+};
+
+FIXTURE_SETUP(j1939_addr)
+{
+	self->tx = -1;
+	self->rx = -1;
+	self->raw = -1;
+
+	J1939_SETUP_IFINDEX(self);
+}
+
+FIXTURE_TEARDOWN(j1939_addr)
+{
+	if (self->tx >= 0)
+		close(self->tx);
+	if (self->rx >= 0)
+		close(self->rx);
+	if (self->raw >= 0)
+		close(self->raw);
+}
+
+/* bind with a NAME and read it back with getsockname(2). */
+TEST_F(j1939_addr, name_bind_getsockname)
+{
+	struct sockaddr_can addr = {};
+	socklen_t len = sizeof(addr);
+
+	self->tx = j1939_open_name(self->ifindex, NAME_TX, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0)
+		TH_LOG("bind with NAME failed: %d", errno);
+
+	ASSERT_EQ(getsockname(self->tx, (struct sockaddr *)&addr, &len), 0);
+	EXPECT_EQ(addr.can_addr.j1939.name, NAME_TX);
+	EXPECT_EQ(addr.can_addr.j1939.addr, SA_TX);
+}
+
+/* the bound source address is used as source address on the wire. */
+TEST_F(j1939_addr, source_sa_on_wire)
+{
+	uint8_t buf[8] = {};
+	struct can_frame frame;
+	int ret;
+
+	self->raw = open_raw(self->ifindex);
+	ASSERT_GE(self->raw, 0);
+
+	/* static source address: no address claim needed to transmit */
+	self->tx = j1939_open_name(self->ifindex, J1939_NO_NAME, SA_TX,
+			      J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	ASSERT_EQ(j1939_connect_name(self->tx, self->ifindex, J1939_NO_NAME,
+				SA_RX, PGN_TEST), 0);
+
+	ASSERT_EQ(send(self->tx, buf, sizeof(buf), 0), (int)sizeof(buf));
+
+	ret = recv_timeout(self->raw, &frame, sizeof(frame), J1939_RECV_TIMEOUT_MS);
+	ASSERT_GT(ret, 0);
+	EXPECT_EQ((int)J1939_ID_SA(frame.can_id), SA_TX)
+		TH_LOG("source address on wire is 0x%02x, expected 0x%02x",
+		       (unsigned int)J1939_ID_SA(frame.can_id), SA_TX);
+}
+
+/*
+ * NAME based addressing reaches the socket bound to that NAME.
+ *
+ * The peer must have claimed its address before it is reachable by NAME, so
+ * inject an Address Claimed message for (SA_RX, NAME_RX) first. The sender
+ * uses a static source address and resolves the destination by NAME.
+ */
+TEST_F(j1939_addr, name_addressing)
+{
+	uint8_t tx_buf[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+	uint8_t rx_buf[8];
+
+	/* static receiver on SA_RX; the forged claim maps NAME_RX -> SA_RX */
+	self->rx = j1939_open_name(self->ifindex, J1939_NO_NAME, SA_RX, PGN_TEST);
+	ASSERT_GE(self->rx, 0);
+
+	self->raw = open_raw(self->ifindex);
+	ASSERT_GE(self->raw, 0);
+	ASSERT_EQ(forge_address_claim(self->raw, SA_RX, NAME_RX), 0)
+		TH_LOG("failed to inject address claim: %d", errno);
+
+	self->tx = j1939_open_name(self->ifindex, J1939_NO_NAME, SA_TX,
+			      J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+	ASSERT_EQ(j1939_connect_name(self->tx, self->ifindex, NAME_RX,
+				J1939_NO_ADDR, PGN_TEST), 0)
+		TH_LOG("connect to peer NAME failed: %d", errno);
+
+	ASSERT_EQ(send(self->tx, tx_buf, sizeof(tx_buf), 0),
+		  (int)sizeof(tx_buf))
+		TH_LOG("send to peer NAME failed: %d", errno);
+	ASSERT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_RECV_TIMEOUT_MS), (int)sizeof(tx_buf));
+	EXPECT_EQ(memcmp(tx_buf, rx_buf, sizeof(tx_buf)), 0);
+}
+
+/* many concurrent transport-protocol sessions run without corruption. */
+TEST_F(j1939_addr, stress_concurrent_sessions)
+{
+	pid_t pids[STRESS_N];
+	int i, failed = 0;
+
+	for (i = 0; i < STRESS_N; i++) {
+		pids[i] = fork();
+		ASSERT_GE(pids[i], 0);
+		if (pids[i] == 0)
+			_exit(stress_child(self->ifindex, 0x40 + i, 0x20 + i));
+	}
+
+	for (i = 0; i < STRESS_N; i++) {
+		int status;
+
+		if (waitpid(pids[i], &status, 0) < 0 ||
+		    !WIFEXITED(status) || WEXITSTATUS(status) != 0)
+			failed++;
+	}
+
+	EXPECT_EQ(failed, 0)
+		TH_LOG("%d of %d concurrent sessions failed", failed, STRESS_N);
+}
+
+#define SA_CLAIM		0x50
+#define SA_DST			0x60
+#define NAME_CLAIM		0x00000000c0ffee10ULL
+#define NAME_LOWER		0x00000000c0ffee00ULL	/* higher priority */
+#define NAME_HIGHER		0x00000000c0ffee20ULL	/* lower priority */
+
+/*
+ * Claim an address the way j1939acd would: bind a NAME to an address and send
+ * an Address Claimed message, then wait for the claim to settle. Returns the
+ * (still open) socket on success.
+ */
+static int claim_addr(int ifindex, name_t name, uint8_t sa)
+{
+	uint8_t nb[8];
+	int s;
+
+	s = j1939_open_flags(ifindex, name, sa, J1939_NO_PGN, J1939_OPEN_BCAST);
+	if (s < 0)
+		return -1;
+
+	put_name(nb, name);
+	if (j1939_sendto(s, nb, 8, ifindex, J1939_NO_NAME, J1939_NO_ADDR,
+			 J1939_PGN_ADDRESS_CLAIMED) != 8) {
+		close(s);
+		return -1;
+	}
+
+	usleep(ACLAIM_SETTLE_US);	/* settling time */
+	return s;
+}
+
+/* Send a single data frame from a socket to (SA_DST, PGN_TEST). */
+static int send_data(int s, int ifindex)
+{
+	uint8_t data[8] = {};
+
+	return j1939_sendto(s, data, sizeof(data), ifindex, J1939_NO_NAME,
+			    SA_DST, PGN_TEST);
+}
+
+static bool match_addr_claimed(canid_t id, const void *ctx)
+{
+	(void)ctx;
+	return J1939_ID_PF(id) == J1939_PF(J1939_PGN_ADDRESS_CLAIMED) &&
+	       J1939_ID_SA(id) == SA_CLAIM;
+}
+
+/* sending an Address Claimed message puts it on the wire and claims. */
+TEST_F(j1939_addr, address_claim_send)
+{
+	struct can_frame frame;
+
+	self->raw = open_raw(self->ifindex);
+	ASSERT_GE(self->raw, 0);
+
+	self->tx = claim_addr(self->ifindex, NAME_CLAIM, SA_CLAIM);
+	ASSERT_GE(self->tx, 0)
+		TH_LOG("address claim failed: %d", errno);
+
+	/* the Address Claimed frame must have appeared on the wire */
+	EXPECT_EQ(recv_match(self->raw, ACLAIM_SETTLE_US / 1000, &frame,
+			     match_addr_claimed, NULL), 0)
+		TH_LOG("no Address Claimed frame seen on the wire");
+
+	/* with the address claimed, the socket can transmit */
+	EXPECT_EQ(send_data(self->tx, self->ifindex), 8)
+		TH_LOG("transmit from claimed address failed: %d", errno);
+}
+
+/*
+ * Shared address-claim arbitration setup for the win/lose conflict tests.
+ * Claim SA_CLAIM with NAME_CLAIM, confirm we own it, then let a contender with
+ * @contender claim the same SA. On success stores the result of transmitting
+ * after the contention in *tx_ret (errno is left as that send set it) and
+ * returns 0; returns -1 if any setup step failed. Each caller asserts its own
+ * win/lose outcome on *tx_ret.
+ */
+static int ac_conflict_probe(FIXTURE_DATA(j1939_addr) *self, name_t contender,
+			     int *tx_ret)
+{
+	self->raw = open_raw(self->ifindex);
+	if (self->raw < 0)
+		return -1;
+	self->tx = claim_addr(self->ifindex, NAME_CLAIM, SA_CLAIM);
+	if (self->tx < 0)
+		return -1;
+
+	/* baseline: we own the address and can transmit */
+	if (send_data(self->tx, self->ifindex) != 8)
+		return -1;
+
+	/* let the contender claim our SA */
+	if (forge_address_claim(self->raw, SA_CLAIM, contender))
+		return -1;
+
+	*tx_ret = send_data(self->tx, self->ifindex);
+	return 0;
+}
+
+/* a higher-priority competing claim takes the address away from us. */
+TEST_F(j1939_addr, ac_conflict_lose)
+{
+	int tx_ret;
+
+	/* a contender with a lower NAME (higher priority) claims our SA */
+	ASSERT_EQ(ac_conflict_probe(self, NAME_LOWER, &tx_ret), 0);
+
+	/* we must have lost the address */
+	EXPECT_LT(tx_ret, 0)
+		TH_LOG("transmit still succeeded after losing the address");
+	EXPECT_EQ(errno, EADDRNOTAVAIL);
+}
+
+/* a lower-priority competing claim does not take our address. */
+TEST_F(j1939_addr, ac_conflict_defend)
+{
+	int tx_ret;
+
+	/* a contender with a higher NAME (lower priority) must not win */
+	ASSERT_EQ(ac_conflict_probe(self, NAME_HIGHER, &tx_ret), 0);
+
+	EXPECT_EQ(tx_ret, 8)
+		TH_LOG("lost the address to a lower-priority claim");
+}
+
+J1939_TEST_MAIN();
diff --git a/tools/testing/selftests/net/can/test_j1939_addr.sh b/tools/testing/selftests/net/can/test_j1939_addr.sh
new file mode 100755
index 000000000000..e611fe211c6d
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_addr.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+J1939_TEST_PROG=test_j1939_addr
+source "$(dirname "$0")/j1939_lib.sh"
-- 
2.47.3

[PATCH can-next v1 5/8] selftests: can: add J1939 wire-format tests against CAN_RAW

[Oleksij Rempel]

Add a test that pairs a J1939 socket with a CAN_RAW socket so the actual
CAN frames on the wire are checked in both directions: a J1939 send
produces the expected CAN id (priority, PGN, source and destination) and
payload, and an injected CAN frame is parsed and delivered to the J1939
socket. The base test only exercises J1939-socket-to-J1939-socket
transfers, which stay on the local delivery path inside one netdevice, so
the on-wire format is otherwise unchecked. Both PDU1 (destination
specific) and PDU2 (broadcast) single frames are covered.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/.gitignore    |   1 +
 tools/testing/selftests/net/can/Makefile      |   2 +
 .../selftests/net/can/test_j1939_wire.c       | 170 ++++++++++++++++++
 .../selftests/net/can/test_j1939_wire.sh      |   5 +
 4 files changed, 178 insertions(+)
 create mode 100644 tools/testing/selftests/net/can/test_j1939_wire.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_wire.sh

diff --git a/tools/testing/selftests/net/can/.gitignore b/tools/testing/selftests/net/can/.gitignore
index 7ca63a19472c..3f31fa5b7438 100644
--- a/tools/testing/selftests/net/can/.gitignore
+++ b/tools/testing/selftests/net/can/.gitignore
@@ -3,3 +3,4 @@ test_raw_filter
 test_j1939_base
 test_j1939_opts
 test_j1939_addr
+test_j1939_wire
diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index 6e4176a1b985..d80fa114a0cd 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -11,6 +11,7 @@ TEST_PROGS := \
 	test_j1939_base.sh \
 	test_j1939_opts.sh \
 	test_j1939_addr.sh \
+	test_j1939_wire.sh \
 
 TEST_FILES := j1939_lib.sh
 
@@ -19,5 +20,6 @@ TEST_GEN_FILES := \
 	test_j1939_base \
 	test_j1939_opts \
 	test_j1939_addr \
+	test_j1939_wire \
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/test_j1939_wire.c b/tools/testing/selftests/net/can/test_j1939_wire.c
new file mode 100644
index 000000000000..8f5113394f99
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_wire.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * J1939 wire-format selftests: a J1939 socket against CAN_RAW.
+ *
+ * Where test_j1939_base keeps transfers on the local path between two J1939
+ * sockets, these pair a J1939 socket with a CAN_RAW socket and check the actual
+ * CAN frames on the wire both ways: a J1939 send produces the expected CAN id
+ * (priority, PGN, source and destination) and payload, and an injected CAN frame
+ * is parsed and delivered to the J1939 socket. Both destination-specific (PDU1)
+ * and broadcast (PDU2) single frames are covered.
+ *
+ * The vcan interface is set up by the caller (see test_j1939_wire.sh); its name
+ * is passed via the CANIF environment variable.
+ */
+#include <errno.h>
+
+#include "kselftest_harness.h"
+#include "j1939_tests.h"
+
+#define SA_LOCAL		0x90	/* the J1939 socket under test */
+#define SA_PEER			0x20	/* the CAN_RAW peer */
+#define PGN_PDU1		0x0ab00	/* PF 0xab (< 240): destination specific */
+#define PGN_PDU2		0x0feee	/* PF 0xfe (>= 240): broadcast */
+#define PRIO			J1939_PRIO_DEFAULT
+
+FIXTURE(j1939_wire)
+{
+	int ifindex;
+	int j1939;
+	int raw;
+};
+
+FIXTURE_SETUP(j1939_wire)
+{
+	self->j1939 = -1;
+	self->raw = -1;
+
+	J1939_SETUP_IFINDEX(self);
+
+	J1939_SETUP_RAW(self, 0);
+}
+
+FIXTURE_TEARDOWN(j1939_wire)
+{
+	if (self->j1939 >= 0)
+		close(self->j1939);
+	if (self->raw >= 0)
+		close(self->raw);
+}
+
+/*
+ * Inject one CAN frame from the raw peer and confirm the J1939 socket receives
+ * the identical 8-byte payload. Opens the J1939 RX socket into self->j1939 (the
+ * fixture teardown closes it). @bcast selects a broadcast (PDU2) socket and
+ * frame; @pgn and @da address the injected frame. Returns 0 on success, -1
+ * otherwise; the caller ASSERTs the result.
+ */
+static int rx_single_frame_check(FIXTURE_DATA(j1939_wire) *self, bool bcast,
+				 pgn_t pgn, uint8_t da, const uint8_t payload[8])
+{
+	uint8_t rx_buf[8];
+
+	if (bcast)
+		self->j1939 = j1939_open_flags(self->ifindex, J1939_NO_NAME,
+					       SA_LOCAL, pgn, J1939_OPEN_BCAST);
+	else
+		self->j1939 = j1939_open(self->ifindex, SA_LOCAL, pgn);
+	if (self->j1939 < 0)
+		return -1;
+
+	if (send_raw8(self->raw, PRIO, pgn, da, SA_PEER, payload))
+		return -1;
+
+	if (recv_timeout(self->j1939, rx_buf, sizeof(rx_buf),
+			 J1939_RECV_TIMEOUT_MS) != (int)sizeof(rx_buf))
+		return -1;
+
+	return memcmp(rx_buf, payload, sizeof(rx_buf)) == 0 ? 0 : -1;
+}
+
+/*
+ * Send one 8-byte payload from the J1939 socket to the raw peer and return the
+ * CAN frame seen on the wire via *frame, so each caller can assert the id fields
+ * it cares about. Opens the J1939 TX socket into self->j1939 (the fixture
+ * teardown closes it) and connects it to (@da, @pgn); @bcast opens a broadcast
+ * (PDU2) socket. Returns 0 on success, -1 otherwise; the caller ASSERTs the
+ * result.
+ */
+static int tx_to_raw_send_recv(FIXTURE_DATA(j1939_wire) *self, bool bcast,
+			       pgn_t pgn, uint8_t da, const uint8_t payload[8],
+			       struct can_frame *frame)
+{
+	if (bcast)
+		self->j1939 = j1939_open_flags(self->ifindex, J1939_NO_NAME,
+					       SA_LOCAL, J1939_NO_PGN,
+					       J1939_OPEN_BCAST);
+	else
+		self->j1939 = j1939_open(self->ifindex, SA_LOCAL, J1939_NO_PGN);
+	if (self->j1939 < 0)
+		return -1;
+
+	if (j1939_connect(self->j1939, self->ifindex, da, pgn))
+		return -1;
+
+	if (send(self->j1939, payload, 8, 0) != 8)
+		return -1;
+
+	if (recv_timeout(self->raw, frame, sizeof(*frame),
+			 J1939_RECV_TIMEOUT_MS) <= 0)
+		return -1;
+
+	return 0;
+}
+
+/* J1939 TX -> CAN_RAW: destination-specific (PDU1) single frame. */
+TEST_F(j1939_wire, tx_pdu1_to_raw)
+{
+	uint8_t payload[8] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88 };
+	struct can_frame frame;
+	canid_t id;
+
+	ASSERT_EQ(tx_to_raw_send_recv(self, false, PGN_PDU1, SA_PEER, payload,
+				      &frame), 0);
+	id = frame.can_id & CAN_EFF_MASK;
+
+	EXPECT_EQ(J1939_ID_PRIO(id), PRIO);
+	EXPECT_EQ(J1939_ID_SA(id), SA_LOCAL);			/* source address */
+	EXPECT_EQ(J1939_ID_PS(id), SA_PEER);		/* destination (PS) */
+	EXPECT_EQ(J1939_ID_PF(id), J1939_PF(PGN_PDU1));
+	ASSERT_EQ(frame.can_dlc, sizeof(payload));
+	EXPECT_EQ(memcmp(frame.data, payload, sizeof(payload)), 0);
+}
+
+/* CAN_RAW -> J1939 RX: destination-specific (PDU1) single frame. */
+TEST_F(j1939_wire, rx_pdu1_from_raw)
+{
+	uint8_t payload[8] = { 0xde, 0xad, 0xbe, 0xef, 0x01, 0x02, 0x03, 0x04 };
+
+	ASSERT_EQ(rx_single_frame_check(self, false, PGN_PDU1, SA_LOCAL,
+					payload), 0);
+}
+
+/* J1939 TX -> CAN_RAW: broadcast (PDU2) single frame. */
+TEST_F(j1939_wire, tx_pdu2_to_raw)
+{
+	uint8_t payload[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+	struct can_frame frame;
+	canid_t id;
+
+	ASSERT_EQ(tx_to_raw_send_recv(self, true, PGN_PDU2, J1939_NO_ADDR,
+				      payload, &frame), 0);
+	id = frame.can_id & CAN_EFF_MASK;
+
+	EXPECT_EQ(J1939_ID_PRIO(id), PRIO);
+	EXPECT_EQ(J1939_ID_SA(id), SA_LOCAL);			/* source address */
+	EXPECT_EQ((id >> 8) & J1939_PGN_MAX, PGN_PDU2);	/* full broadcast PGN */
+	ASSERT_EQ(frame.can_dlc, sizeof(payload));
+	EXPECT_EQ(memcmp(frame.data, payload, sizeof(payload)), 0);
+}
+
+/* CAN_RAW -> J1939 RX: broadcast (PDU2) single frame. */
+TEST_F(j1939_wire, rx_pdu2_from_raw)
+{
+	uint8_t payload[8] = { 0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7 };
+
+	ASSERT_EQ(rx_single_frame_check(self, true, PGN_PDU2, J1939_NO_ADDR,
+					payload), 0);
+}
+
+J1939_TEST_MAIN();
diff --git a/tools/testing/selftests/net/can/test_j1939_wire.sh b/tools/testing/selftests/net/can/test_j1939_wire.sh
new file mode 100755
index 000000000000..77a1bddb5854
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_wire.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+J1939_TEST_PROG=test_j1939_wire
+source "$(dirname "$0")/j1939_lib.sh"
-- 
2.47.3

[PATCH can-next v1 6/8] selftests: can: add J1939 TP/ETP multi-frame tests against CAN_RAW

[Oleksij Rempel]

Add multi-frame transport tests that pair a J1939 socket with a CAN_RAW
peer. They drive the Transport Protocol (TP) and Extended Transport
Protocol (ETP) round-trips in both directions. The raw peer answers the
kernel's RTS/CTS/DPO sequencing, and the reassembled payload is checked
byte for byte.

The ETP payload spans several 255-packet blocks. In the receive
direction, the test checks the kernel-emitted EOMA on the wire (command
and size, plus the packet count for TP). In the transmit direction the
kernel sends the RTS, so the test also checks its ETP RTS message-size
field.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 tools/testing/selftests/net/can/.gitignore    |   1 +
 tools/testing/selftests/net/can/Makefile      |   2 +
 .../selftests/net/can/test_j1939_multi.c      | 453 ++++++++++++++++++
 .../selftests/net/can/test_j1939_multi.sh     |   5 +
 4 files changed, 461 insertions(+)
 create mode 100644 tools/testing/selftests/net/can/test_j1939_multi.c
 create mode 100755 tools/testing/selftests/net/can/test_j1939_multi.sh

diff --git a/tools/testing/selftests/net/can/.gitignore b/tools/testing/selftests/net/can/.gitignore
index 3f31fa5b7438..b6bf4c613ba3 100644
--- a/tools/testing/selftests/net/can/.gitignore
+++ b/tools/testing/selftests/net/can/.gitignore
@@ -4,3 +4,4 @@ test_j1939_base
 test_j1939_opts
 test_j1939_addr
 test_j1939_wire
+test_j1939_multi
diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
index d80fa114a0cd..559cca255875 100644
--- a/tools/testing/selftests/net/can/Makefile
+++ b/tools/testing/selftests/net/can/Makefile
@@ -12,6 +12,7 @@ TEST_PROGS := \
 	test_j1939_opts.sh \
 	test_j1939_addr.sh \
 	test_j1939_wire.sh \
+	test_j1939_multi.sh \
 
 TEST_FILES := j1939_lib.sh
 
@@ -21,5 +22,6 @@ TEST_GEN_FILES := \
 	test_j1939_opts \
 	test_j1939_addr \
 	test_j1939_wire \
+	test_j1939_multi \
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/can/test_j1939_multi.c b/tools/testing/selftests/net/can/test_j1939_multi.c
new file mode 100644
index 000000000000..1f27ee58f48c
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_multi.c
@@ -0,0 +1,453 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * J1939 multi-frame wire selftests: TP and ETP against CAN_RAW.
+ *
+ * A CAN_RAW socket plays the transport peer and drives the full handshake
+ * against the kernel J1939 stack over the wire, in both directions: for TP the
+ * raw side answers RTS/CTS and exchanges DT frames with EOMA; ETP does the same
+ * with a payload spanning several CTS blocks (DPO windows).
+ *
+ * The vcan interface is set up by the caller (see test_j1939_multi.sh); its name
+ * is passed via the CANIF environment variable.
+ */
+#include <errno.h>
+
+#include "kselftest_harness.h"
+#include "j1939_tests.h"
+
+#define SA_LOCAL		0x90	/* the J1939 socket under test */
+#define SA_PEER			0x20	/* the CAN_RAW transport peer */
+#define PGN_DATA		0x0feef	/* transported message PGN */
+/*
+ * Priority for frames this test injects as the CAN_RAW peer; the stack
+ * does not check it. SAE J1939-21:2001 lists 7 as the TP.CM/TP.DT
+ * default; the kernel defaults its own J1939 TX to 6 (J1939_PRIO_DEFAULT).
+ */
+#define PRIO			7
+
+#define TP_CTL			0x0ec00
+#define TP_DAT			0x0eb00
+#define ETP_CTL			0x0c800
+#define ETP_DAT			0x0c700
+
+#define TP_CMD_RTS		0x10
+#define TP_CMD_CTS		0x11
+#define TP_CMD_EOMA		0x13
+#define TP_CMD_ABORT		0xff
+#define ETP_CMD_RTS		0x14
+#define ETP_CMD_CTS		0x15
+#define ETP_CMD_DPO		0x16
+#define ETP_CMD_EOMA		0x17
+
+#define TP_SIZE			200
+#define ETP_SIZE		6000		/* > TP max 1785 (255*7); several blocks */
+
+FIXTURE(j1939_multi)
+{
+	int ifindex;
+	int j1939;
+	int raw;
+};
+
+FIXTURE_SETUP(j1939_multi)
+{
+	self->j1939 = -1;
+	self->raw = -1;
+
+	J1939_SETUP_IFINDEX(self);
+
+	J1939_SETUP_RAW(self, J1939_RAW_BIG_RCVBUF);
+}
+
+FIXTURE_TEARDOWN(j1939_multi)
+{
+	if (self->j1939 >= 0)
+		close(self->j1939);
+	if (self->raw >= 0)
+		close(self->raw);
+}
+
+/*
+ * Finish a TP/ETP control frame by embedding its data PGN and sending it on the
+ * control PGN. dat must already hold the command and its command-specific bytes.
+ */
+static int send_ctl_frame(int raw, uint8_t prio, pgn_t ctl_pgn, uint8_t da,
+			  uint8_t sa, uint8_t *dat, pgn_t data_pgn)
+{
+	put_pgn(dat, data_pgn);
+	return send_raw8(raw, prio, ctl_pgn, da, sa, dat);
+}
+
+/* Build and send a TP.CM_RTS: message size, total packets and max packets/CTS. */
+static int send_tp_rts(int raw, uint8_t prio, pgn_t pgn, uint8_t da, uint8_t sa,
+		       uint16_t size, uint8_t total_pkts, uint8_t max_cts_pkts)
+{
+	uint8_t dat[8];
+
+	ctl_dat_init(dat, TP_CMD_RTS);
+	put_le16(&dat[1], size);
+	dat[3] = total_pkts;
+	dat[4] = max_cts_pkts;
+	return send_ctl_frame(raw, prio, TP_CTL, da, sa, dat, pgn);
+}
+
+/*
+ * Send a burst of @count DT frames for the block starting at the 1-based packet
+ * @next, each carrying 7 payload bytes from the matching absolute offset. The DT
+ * sequence byte differs by protocol: TP numbers packets absolutely across the
+ * whole message, ETP numbers them 1-based within each DPO block (@seq_relative
+ * selects the latter). Returns 0 on success, -1 on error.
+ */
+static int send_dt_burst(int raw, uint8_t prio, pgn_t dat_pgn, uint8_t da,
+			 uint8_t sa, unsigned int next, unsigned int count,
+			 const uint8_t *payload, size_t size, bool seq_relative)
+{
+	unsigned int i;
+
+	for (i = 0; i < count; i++) {
+		unsigned int abs_pkt = (next - 1) + i;
+		uint8_t seq = seq_relative ? (i + 1) : (abs_pkt + 1);
+
+		if (send_dt(raw, prio, dat_pgn, da, sa, seq, payload,
+			    (size_t)abs_pkt * 7, size))
+			return -1;
+	}
+	return 0;
+}
+
+/*
+ * Answer each kernel CTS with its DT burst until num_pkts are delivered.
+ * Returns the largest CTS packet count the kernel requested, or -1 on error.
+ */
+static int tp_serve_cts(int raw, uint8_t prio, uint8_t da, uint8_t sa,
+			const uint8_t *payload, size_t size, unsigned int num_pkts)
+{
+	unsigned int sent = 0, max_want = 0;
+	struct can_frame frame;
+
+	while (sent < num_pkts) {
+		unsigned int want, next;
+
+		if (recv_ctl_cmd(raw, J1939_PF(TP_CTL), sa, &frame,
+				 J1939_RECV_TIMEOUT_MS, TP_CMD_CTS))
+			return -1;
+		want = frame.data[1];
+		next = frame.data[2];		/* 1-based packet number */
+		if (want > max_want)
+			max_want = want;
+		if (send_dt_burst(raw, prio, TP_DAT, da, sa, next, want,
+				  payload, size, false))
+			return -1;
+		sent = next - 1 + want;
+	}
+	return (int)max_want;
+}
+
+/*
+ * Scatter a received DT frame's payload into buf at the offset implied by its
+ * sequence number. base_pkt is the 0-based absolute packet the 1-based seq is
+ * relative to (0 for TP, the DPO offset for ETP).
+ */
+static void unpack_dt_into(uint8_t *buf, size_t total, unsigned int base_pkt,
+			   const struct can_frame *frame)
+{
+	unsigned int abs_pkt = base_pkt + (frame->data[0] - 1);
+	size_t off = (size_t)abs_pkt * 7;
+	size_t n;
+
+	/*
+	 * A bogus sequence number (e.g. data[0] == 0 wrapping abs_pkt) can put
+	 * off past the buffer; total - off would then underflow size_t into a
+	 * huge length. Drop such frames instead of overrunning buf.
+	 */
+	if (off >= total)
+		return;
+	n = total - off;
+	if (n > 7)
+		n = 7;
+	memcpy(buf + off, &frame->data[1], n);
+}
+
+/* Open a TP RX j1939 socket and send the RTS that starts a receive session. */
+static int tp_rx_open_and_send_rts(int *j1939, int raw, int ifindex,
+				   unsigned int num_pkts,
+				   unsigned int max_cts_pkts)
+{
+	*j1939 = j1939_open(ifindex, SA_LOCAL, PGN_DATA);
+	if (*j1939 < 0)
+		return -1;
+	return send_tp_rts(raw, PRIO, PGN_DATA, SA_LOCAL, SA_PEER, TP_SIZE,
+			   num_pkts, max_cts_pkts);
+}
+
+/*
+ * Shared TP-RX round-trip: open the j1939 socket, drive a full TP receive from
+ * the raw peer advertising max_cts_pkts per CTS, and verify the reassembled
+ * payload against the sent pattern. The kernel's largest single-CTS packet
+ * request is returned via *max_want_out and its EOMA frame via *eoma_out, so
+ * each caller can assert the specific property it cares about. ASSERT_* cannot
+ * be used here (the harness jump target lives in the TEST body), so failures
+ * are reported through the return value. Returns 0 on success, -1 otherwise.
+ */
+static int tp_rx_roundtrip(FIXTURE_DATA(j1939_multi) *self,
+			   unsigned int max_cts_pkts, int *max_want_out,
+			   struct can_frame *eoma_out)
+{
+	uint8_t payload[TP_SIZE], rx_buf[TP_SIZE];
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	int max_want;
+
+	fill_pattern(payload, sizeof(payload));
+
+	if (tp_rx_open_and_send_rts(&self->j1939, self->raw, self->ifindex,
+				    num_pkts, max_cts_pkts))
+		return -1;
+
+	max_want = tp_serve_cts(self->raw, PRIO, SA_LOCAL, SA_PEER,
+				payload, TP_SIZE, num_pkts);
+	if (max_want < 0)
+		return -1;
+	*max_want_out = max_want;
+
+	if (recv_ctl_cmd(self->raw, J1939_PF(TP_CTL), SA_PEER, eoma_out,
+			 J1939_RECV_TIMEOUT_MS, TP_CMD_EOMA))
+		return -1;
+
+	if (recv_timeout(self->j1939, rx_buf, sizeof(rx_buf),
+			 J1939_RECV_TIMEOUT_MS) != (int)sizeof(rx_buf))
+		return -1;
+	if (memcmp(payload, rx_buf, sizeof(payload)) != 0)
+		return -1;
+
+	return 0;
+}
+
+/*
+ * TP RX: the CAN_RAW peer transmits a TP message to the J1939 socket.
+ * raw sends RTS, then answers each CTS from the kernel with DT frames.
+ */
+TEST_F(j1939_multi, tp_rx_from_raw)
+{
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	struct can_frame frame;
+	int max_want;
+
+	ASSERT_EQ(tp_rx_roundtrip(self, num_pkts, &max_want, &frame), 0)
+		TH_LOG("TP RX round-trip against the raw peer failed");
+
+	/*
+	 * The kernel (TP receiver) must acknowledge with a well-formed EOMA on
+	 * the wire: command, the 16-bit message size and the total packet count.
+	 */
+	EXPECT_EQ(get_le16(&frame.data[1]), TP_SIZE);
+	EXPECT_EQ(frame.data[3], num_pkts);
+}
+
+/*
+ * Start a TP/ETP transmit session from the J1939 socket: open an unbound
+ * socket, connect to the peer, send the payload and collect the kernel's RTS
+ * from the raw peer (ctl_pf/rts_cmd select TP or ETP). *j1939 is set even on
+ * open failure so the fixture teardown can close it. Returns 0 on success, -1
+ * otherwise; the caller ASSERTs the result.
+ */
+static int tx_start_session(int *j1939, int ifindex, const void *payload,
+			    size_t len, int raw, uint8_t ctl_pf,
+			    uint8_t rts_cmd, struct can_frame *rts)
+{
+	*j1939 = j1939_open(ifindex, SA_LOCAL, J1939_NO_PGN);
+	if (*j1939 < 0)
+		return -1;
+	if (j1939_connect(*j1939, ifindex, SA_PEER, PGN_DATA))
+		return -1;
+	if (send(*j1939, payload, len, 0) != (ssize_t)len)
+		return -1;
+	return recv_ctl_cmd(raw, ctl_pf, SA_PEER, rts,
+			    J1939_RECV_TIMEOUT_MS, rts_cmd);
+}
+
+/*
+ * TP TX: the J1939 socket transmits a TP message to the CAN_RAW peer.
+ * raw answers the kernel's RTS with a CTS, collects the DT frames and
+ * acknowledges with EOMA.
+ */
+TEST_F(j1939_multi, tp_tx_to_raw)
+{
+	uint8_t payload[TP_SIZE], rx_buf[TP_SIZE];
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	struct can_frame frame;
+	uint8_t dat[8];
+	unsigned int got = 0;
+
+	fill_pattern(payload, sizeof(payload));
+	memset(rx_buf, 0, sizeof(rx_buf));
+
+	ASSERT_EQ(tx_start_session(&self->j1939, self->ifindex, payload,
+				   sizeof(payload), self->raw, J1939_PF(TP_CTL),
+				   TP_CMD_RTS, &frame), 0);
+	/* The kernel's TP RTS must carry the message size and packet count. */
+	EXPECT_EQ(get_le16(&frame.data[1]), TP_SIZE);
+	EXPECT_EQ(frame.data[3], num_pkts);
+
+	/* request all packets in one CTS */
+	ctl_dat_init(dat, TP_CMD_CTS);
+	dat[1] = num_pkts;
+	dat[2] = 1;
+	ASSERT_EQ(send_ctl_frame(self->raw, PRIO, TP_CTL, SA_LOCAL, SA_PEER,
+				 dat, PGN_DATA), 0);
+
+	while (got < num_pkts) {
+		ASSERT_EQ(recv_for_peer(self->raw, J1939_PF(TP_DAT), SA_PEER, &frame,
+					J1939_RECV_TIMEOUT_MS), 0)
+			TH_LOG("missing DT after %u/%u packets", got, num_pkts);
+		unpack_dt_into(rx_buf, TP_SIZE, 0, &frame);
+		got++;
+	}
+
+	/* acknowledge with EOMA */
+	ctl_dat_init(dat, TP_CMD_EOMA);
+	put_le16(&dat[1], TP_SIZE);
+	dat[3] = num_pkts;
+	ASSERT_EQ(send_ctl_frame(self->raw, PRIO, TP_CTL, SA_LOCAL, SA_PEER,
+				 dat, PGN_DATA), 0);
+
+	EXPECT_EQ(memcmp(payload, rx_buf, sizeof(payload)), 0);
+}
+
+/*
+ * ETP RX: the CAN_RAW peer transmits an ETP message to the J1939 socket.
+ * The kernel requests blocks with ETP.CTS; raw answers each with a DPO and
+ * the block's DT frames. The payload spans several 255-packet blocks.
+ */
+TEST_F(j1939_multi, etp_rx_from_raw)
+{
+	unsigned int num_pkts = J1939_NUM_PKTS(ETP_SIZE);
+	uint8_t *payload, *rx_buf;
+	struct can_frame frame;
+	unsigned int sent = 0;
+	uint8_t dat[8];
+
+	payload = malloc(ETP_SIZE);
+	rx_buf = malloc(ETP_SIZE);
+	ASSERT_NE(payload, NULL);
+	ASSERT_NE(rx_buf, NULL);
+	fill_pattern(payload, ETP_SIZE);
+
+	self->j1939 = j1939_open(self->ifindex, SA_LOCAL, PGN_DATA);
+	ASSERT_GE(self->j1939, 0);
+
+	ctl_dat_init(dat, ETP_CMD_RTS);
+	put_le32(&dat[1], ETP_SIZE);
+	ASSERT_EQ(send_ctl_frame(self->raw, PRIO, ETP_CTL, SA_LOCAL, SA_PEER,
+				 dat, PGN_DATA), 0);
+
+	while (sent < num_pkts) {
+		unsigned int num, next;
+
+		ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(ETP_CTL), SA_PEER,
+				       &frame, J1939_RECV_TIMEOUT_MS,
+				       ETP_CMD_CTS), 0)
+			TH_LOG("no ETP CTS after %u/%u packets", sent,
+			       num_pkts);
+		num = frame.data[1];
+		next = get_le24(&frame.data[2]);	/* 1-based */
+
+		ctl_dat_init(dat, ETP_CMD_DPO);
+		dat[1] = num;
+		put_le24(&dat[2], next - 1);
+		ASSERT_EQ(send_ctl_frame(self->raw, PRIO, ETP_CTL, SA_LOCAL,
+					 SA_PEER, dat, PGN_DATA), 0);
+
+		/* ETP numbers DT packets 1-based within each DPO block */
+		ASSERT_EQ(send_dt_burst(self->raw, PRIO, ETP_DAT, SA_LOCAL,
+					SA_PEER, next, num, payload, ETP_SIZE,
+					true), 0);
+		sent = (next - 1) + num;
+	}
+
+	/*
+	 * The kernel (ETP receiver) must acknowledge with a well-formed ETP EOMA
+	 * carrying the 32-bit total message size in bytes 1..4.
+	 */
+	ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(ETP_CTL), SA_PEER, &frame,
+			       J1939_RECV_TIMEOUT_MS, ETP_CMD_EOMA), 0)
+		TH_LOG("no well-formed ETP EOMA from kernel after last DT");
+	EXPECT_EQ(get_le32(&frame.data[1]), (uint32_t)ETP_SIZE);
+
+	ASSERT_EQ(recv_timeout(self->j1939, rx_buf, ETP_SIZE,
+			       J1939_RECV_TIMEOUT_MS), (int)ETP_SIZE);
+	EXPECT_EQ(memcmp(payload, rx_buf, ETP_SIZE), 0);
+
+	free(payload);
+	free(rx_buf);
+}
+
+/*
+ * ETP TX: the J1939 socket transmits an ETP message to the CAN_RAW peer.
+ * raw requests the data in 255-packet blocks (ETP.CTS); for each block the
+ * kernel sends a DPO followed by the DT frames, acknowledged by EOMA.
+ */
+TEST_F(j1939_multi, etp_tx_to_raw)
+{
+	unsigned int num_pkts = J1939_NUM_PKTS(ETP_SIZE);
+	const unsigned int block = 255;
+	uint8_t *payload, *rx_buf;
+	struct can_frame frame;
+	unsigned int got = 0;
+	uint8_t dat[8];
+
+	payload = malloc(ETP_SIZE);
+	rx_buf = malloc(ETP_SIZE);
+	ASSERT_NE(payload, NULL);
+	ASSERT_NE(rx_buf, NULL);
+	fill_pattern(payload, ETP_SIZE);
+	memset(rx_buf, 0, ETP_SIZE);
+
+	ASSERT_EQ(tx_start_session(&self->j1939, self->ifindex, payload,
+				   ETP_SIZE, self->raw, J1939_PF(ETP_CTL),
+				   ETP_CMD_RTS, &frame), 0);
+	/* The kernel's ETP RTS must carry the correct 32-bit message size. */
+	EXPECT_EQ(get_le32(&frame.data[1]), (uint32_t)ETP_SIZE);
+
+	while (got < num_pkts) {
+		unsigned int req = num_pkts - got;
+		unsigned int next = got + 1;		/* 1-based */
+		unsigned int i, dpo_num, dpo_off;
+
+		if (req > block)
+			req = block;
+
+		ctl_dat_init(dat, ETP_CMD_CTS);
+		dat[1] = req;
+		put_le24(&dat[2], next);
+		ASSERT_EQ(send_ctl_frame(self->raw, PRIO, ETP_CTL, SA_LOCAL,
+					 SA_PEER, dat, PGN_DATA), 0);
+
+		ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(ETP_CTL), SA_PEER,
+				       &frame, J1939_RECV_TIMEOUT_MS,
+				       ETP_CMD_DPO), 0)
+			TH_LOG("no DPO after %u/%u packets", got, num_pkts);
+		dpo_num = frame.data[1];
+		dpo_off = get_le24(&frame.data[2]);
+
+		for (i = 0; i < dpo_num; i++) {
+			ASSERT_EQ(recv_for_peer(self->raw, J1939_PF(ETP_DAT),
+						SA_PEER, &frame,
+						J1939_RECV_TIMEOUT_MS), 0)
+				TH_LOG("missing ETP DT %u in block", i);
+			unpack_dt_into(rx_buf, ETP_SIZE, dpo_off, &frame);
+		}
+		got += dpo_num;
+	}
+
+	ctl_dat_init(dat, ETP_CMD_EOMA);
+	put_le32(&dat[1], ETP_SIZE);
+	ASSERT_EQ(send_ctl_frame(self->raw, PRIO, ETP_CTL, SA_LOCAL, SA_PEER,
+				 dat, PGN_DATA), 0);
+
+	EXPECT_EQ(memcmp(payload, rx_buf, ETP_SIZE), 0);
+
+	free(payload);
+	free(rx_buf);
+}
+
+J1939_TEST_MAIN();
diff --git a/tools/testing/selftests/net/can/test_j1939_multi.sh b/tools/testing/selftests/net/can/test_j1939_multi.sh
new file mode 100755
index 000000000000..53f473588154
--- /dev/null
+++ b/tools/testing/selftests/net/can/test_j1939_multi.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+J1939_TEST_PROG=test_j1939_multi
+source "$(dirname "$0")/j1939_lib.sh"
-- 
2.47.3

[PATCH can-next v1 7/8] selftests: can: cover J1939 transport abort and timeout paths

[Oleksij Rempel]

Extend the multi-frame tests with the transport edge cases the round-trip
tests do not reach: a per-CTS block limit smaller than the message, the
RTS/CTS and rx timeouts that must end a stalled session with a Connection
Abort, and a received Connection Abort that cancels an incoming session.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../selftests/net/can/test_j1939_multi.c      | 187 +++++++++++++++++-
 1 file changed, 186 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/can/test_j1939_multi.c b/tools/testing/selftests/net/can/test_j1939_multi.c
index 1f27ee58f48c..069a9c8690b5 100644
--- a/tools/testing/selftests/net/can/test_j1939_multi.c
+++ b/tools/testing/selftests/net/can/test_j1939_multi.c
@@ -5,7 +5,18 @@
  * A CAN_RAW socket plays the transport peer and drives the full handshake
  * against the kernel J1939 stack over the wire, in both directions: for TP the
  * raw side answers RTS/CTS and exchanges DT frames with EOMA; ETP does the same
- * with a payload spanning several CTS blocks (DPO windows).
+ * with a payload spanning several CTS blocks (DPO windows). The tests also
+ * cover the CTS block limit and the abort and timeout paths. Those edge cases
+ * are TP only; ETP has no equivalents here yet.
+ *
+ * Timing limitation: the abort/timeout tests wait with wide receive windows
+ * rather than tight bounds, because timing over vcan under QEMU is not tightly
+ * bounded. They catch a gross timer regression (e.g. a doubled timeout) but not
+ * a subtle shift; see the per-test comments.
+ *
+ * Coverage limitation: a received Connection Abort is exercised RX-side only;
+ * there is no kernel-as-transmitter received-abort test, matching the known
+ * net-next limitation that a TX session ignores an incoming Connection Abort.
  *
  * The vcan interface is set up by the caller (see test_j1939_multi.sh); its name
  * is passed via the CANIF environment variable.
@@ -39,6 +50,14 @@
 #define ETP_CMD_DPO		0x16
 #define ETP_CMD_EOMA		0x17
 
+/* TP.Abort reason codes (not durations), from the kernel's enum j1939_xtp_abort */
+#define ABORT_REASON_TIMEOUT	3	/* J1939_XTP_ABORT_TIMEOUT */
+#define ABORT_REASON_BUSY	1	/* J1939_XTP_ABORT_BUSY */
+
+/* TP responder T2: wait for the next DT after CTS (ISO 11783-3:1998 §3.10.2.4). */
+#define TP_T2_MS		1250
+#define TP_RX_ABORT_WAIT_MS	(TP_T2_MS * 2)	/* generous; floor check is the real test */
+
 #define TP_SIZE			200
 #define ETP_SIZE		6000		/* > TP max 1785 (255*7); several blocks */
 
@@ -313,6 +332,69 @@ TEST_F(j1939_multi, tp_tx_to_raw)
 	EXPECT_EQ(memcmp(payload, rx_buf, sizeof(payload)), 0);
 }
 
+/*
+ * TP RX, per-CTS block limit: the RTS advertises a "max packets per CTS"
+ * (RTS data byte 4, 0-based; the spec's byte 5) smaller than the total packet
+ * count. The kernel must never request more than that many packets in a
+ * single CTS.
+ */
+TEST_F(j1939_multi, tp_rx_cts_block_limit)
+{
+	const unsigned int limit = 3;
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	struct can_frame frame;
+	int max_want;
+
+	ASSERT_GT(num_pkts, limit)
+		TH_LOG("test needs more packets than the CTS limit");
+
+	/* advertise a max packets/CTS (limit) smaller than the total */
+	ASSERT_EQ(tp_rx_roundtrip(self, limit, &max_want, &frame), 0)
+		TH_LOG("TP RX round-trip against the raw peer failed");
+
+	EXPECT_LE(max_want, (int)limit)
+		TH_LOG("kernel CTS requested %d packets, sender limit was %u",
+		       max_want, limit);
+}
+
+/*
+ * TP TX, RTS->CTS timeout: the J1939 socket starts a TP transmit; the raw peer
+ * receives the RTS but never answers with a CTS. The kernel must give up (spec
+ * T3 = 1250 ms, SAE J1939-21:2001 §5.12.3) and send a Connection Abort.
+ *
+ * The abort must arrive within 2000 ms (a doubled T3 would miss this window) but
+ * no sooner than 800 ms, safely below the 1250 ms timer, so an instant-fire
+ * regression (an abort emitted immediately on a zeroed timer) is caught too. The
+ * bounds are wide, not tight, to stay reliable over vcan, so a subtle shift
+ * (e.g. 1250->1800 ms) still escapes.
+ */
+TEST_F(j1939_multi, tp_tx_rts_cts_timeout)
+{
+	uint8_t payload[TP_SIZE];
+	struct timespec t0, t1;
+	struct can_frame frame;
+	long elapsed_ms;
+
+	fill_pattern(payload, sizeof(payload));
+
+	ASSERT_EQ(tx_start_session(&self->j1939, self->ifindex, payload,
+				   sizeof(payload), self->raw, J1939_PF(TP_CTL),
+				   TP_CMD_RTS, &frame), 0);
+
+	/* Deliberately send no CTS; expect the kernel to abort within 2000 ms. */
+	clock_gettime(CLOCK_MONOTONIC, &t0);
+	ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(TP_CTL), SA_PEER, &frame,
+			       2000, TP_CMD_ABORT), 0)
+		TH_LOG("kernel did not send a Connection Abort within 2000 ms");
+	clock_gettime(CLOCK_MONOTONIC, &t1);
+	elapsed_ms = timespec_diff_ms(&t0, &t1);
+	/* must not fire instantly: the T3 timer has to run before the abort */
+	ASSERT_GE(elapsed_ms, 800)
+		TH_LOG("abort fired after only %ld ms, timer did not run", elapsed_ms);
+	/* the T3 expiry must be reported as a timeout abort */
+	EXPECT_EQ(frame.data[1], ABORT_REASON_TIMEOUT);
+}
+
 /*
  * ETP RX: the CAN_RAW peer transmits an ETP message to the J1939 socket.
  * The kernel requests blocks with ETP.CTS; raw answers each with a DPO and
@@ -450,4 +532,107 @@ TEST_F(j1939_multi, etp_tx_to_raw)
 	free(rx_buf);
 }
 
+/* Send a TP.CM_Abort control frame. */
+static int send_abort(int raw, uint8_t da, uint8_t sa, uint8_t reason)
+{
+	uint8_t dat[8];
+
+	ctl_dat_init(dat, TP_CMD_ABORT);
+	dat[1] = reason;
+	return send_ctl_frame(raw, PRIO, TP_CTL, da, sa, dat, PGN_DATA);
+}
+
+/* Open a TP RX j1939 socket, send a full RTS, consume the first CTS. */
+static int tp_rx_start_session(int *j1939, int raw, int ifindex,
+			       unsigned int num_pkts, struct can_frame *cts)
+{
+	if (tp_rx_open_and_send_rts(j1939, raw, ifindex, num_pkts,
+				   num_pkts /* no per-CTS cap */))
+		return -1;
+	return recv_ctl_cmd(raw, J1939_PF(TP_CTL), SA_PEER, cts,
+			    J1939_RECV_TIMEOUT_MS, TP_CMD_CTS);
+}
+
+/*
+ * TP RX timeout: the CAN_RAW peer opens a session with RTS but never sends the
+ * data; the kernel's rx timer must abort the session and emit a TP.CM_Abort.
+ * The wait uses wide bounds (not a tight window) to stay reliable over vcan: the
+ * lower bound catches an instant-fire regression; the upper bound is a generous
+ * T2 multiple, so timer shifts within it escape.
+ */
+TEST_F(j1939_multi, tp_rx_timeout)
+{
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	struct timespec t0, t1;
+	struct can_frame frame;
+	long elapsed_ms;
+
+	ASSERT_EQ(tp_rx_start_session(&self->j1939, self->raw, self->ifindex,
+				      num_pkts, &frame), 0)
+		TH_LOG("failed to open and start the TP rx session");
+
+	/*
+	 * No data follows the CTS; the kernel's rx timer (T2, see TP_T2_MS) must
+	 * fire and abort; the wait is a generous TP_T2_MS * 2, the floor below is
+	 * the real check.
+	 */
+	clock_gettime(CLOCK_MONOTONIC, &t0);
+	ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(TP_CTL), SA_PEER, &frame,
+			       TP_RX_ABORT_WAIT_MS, TP_CMD_ABORT), 0)
+		TH_LOG("kernel did not abort the stalled rx session");
+	clock_gettime(CLOCK_MONOTONIC, &t1);
+	elapsed_ms = timespec_diff_ms(&t0, &t1);
+	/*
+	 * Must not fire instantly: the T2 timer (1250 ms) has to run before the
+	 * abort, so 800 ms is a floor safely below it.
+	 */
+	ASSERT_GE(elapsed_ms, 800)
+		TH_LOG("abort fired after only %ld ms, timer did not run", elapsed_ms);
+	EXPECT_EQ(frame.data[1], ABORT_REASON_TIMEOUT);
+}
+
+/*
+ * TP RX abort: the CAN_RAW peer aborts an open session; the kernel must cancel
+ * it and deliver nothing to the J1939 socket.
+ */
+TEST_F(j1939_multi, tp_rx_abort_from_raw)
+{
+	uint8_t payload[TP_SIZE];
+	unsigned int num_pkts = J1939_NUM_PKTS(TP_SIZE);
+	struct can_frame frame;
+	struct pollfd pfd;
+	unsigned int i;
+
+	fill_pattern(payload, sizeof(payload));
+
+	ASSERT_EQ(tp_rx_start_session(&self->j1939, self->raw, self->ifindex,
+				      num_pkts, &frame), 0)
+		TH_LOG("failed to open and start the TP rx session");
+
+	ASSERT_EQ(send_abort(self->raw, SA_LOCAL, SA_PEER, ABORT_REASON_BUSY), 0);
+	/* let the kernel process the abort first; 100 ms is a generous
+	 * empirical margin, not a spec value.
+	 */
+	usleep(100000);
+
+	/*
+	 * Send the full DT burst anyway. A kernel that honoured the abort has
+	 * torn the session down and drops these; one that ignored it would
+	 * reassemble them and deliver, so POLLIN below means the abort was not
+	 * honoured.
+	 */
+	for (i = 1; i <= num_pkts; i++)
+		ASSERT_EQ(send_dt(self->raw, PRIO, TP_DAT, SA_LOCAL, SA_PEER,
+				  i, payload, (i - 1) * 7, TP_SIZE), 0);
+
+	pfd.fd = self->j1939;
+	pfd.events = POLLIN;
+	/*
+	 * Short negative-check window: nothing should arrive after the abort,
+	 * so this is deliberately shorter than J1939_RECV_TIMEOUT_MS.
+	 */
+	EXPECT_EQ(poll(&pfd, 1, J1939_NEG_RECV_TIMEOUT_MS), 0)
+		TH_LOG("socket delivered data despite the abort");
+}
+
 J1939_TEST_MAIN();
-- 
2.47.3

[PATCH can-next v1 8/8] selftests: can: check SO_J1939_PROMISC receive effect

[Oleksij Rempel]

Add a test that exercises the SO_J1939_PROMISC receive effect. The
existing coverage only round-trips the flag through set/getsockopt, so a
kernel that stores it but never acts on it in the RX path stays green.

The test binds an rx socket to a specific (address, PGN), checks that a
frame to a different destination address and PGN is not delivered, then
enables SO_J1939_PROMISC and checks the same otherwise-unmatched frame is
now delivered unchanged.

Assisted-by: LLM
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../selftests/net/can/test_j1939_opts.c       | 46 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/can/test_j1939_opts.c b/tools/testing/selftests/net/can/test_j1939_opts.c
index f761a42db213..3afda9b4e4a8 100644
--- a/tools/testing/selftests/net/can/test_j1939_opts.c
+++ b/tools/testing/selftests/net/can/test_j1939_opts.c
@@ -5,8 +5,8 @@
  * On top of the basic transport, cover the user-visible J1939 socket options and
  * addressing: the SO_J1939_FILTER PGN whitelist, the SO_J1939_SEND_PRIO send
  * priority seen in the CAN id (via a CAN_RAW peer, including the default of 6),
- * unconnected sendto(2), a wrong-family bind rejection, and SO_J1939_ERRQUEUE
- * TX-completion notifications.
+ * unconnected sendto(2), a wrong-family bind rejection, SO_J1939_ERRQUEUE
+ * TX-completion notifications, and the SO_J1939_PROMISC receive effect.
  *
  * The vcan interface is set up by the caller (see test_j1939_opts.sh); its name
  * is passed via the CANIF environment variable.
@@ -22,6 +22,7 @@
 
 #define SA_TX			0x80
 #define SA_RX			0x90
+#define SA_OTHER		0x50
 #define PGN_A			0x0ab00
 #define PGN_B			0x0cd00
 #define PRIO_SET		3
@@ -123,6 +124,47 @@ TEST_F(j1939_opts, filter_pgn)
 		TH_LOG("PGN_B was received but should have been filtered out");
 }
 
+/* SO_J1939_PROMISC delivers a frame that does not match the bound (addr,pgn). */
+TEST_F(j1939_opts, promisc_receive)
+{
+	uint8_t tx_buf[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+	uint8_t rx_buf[8];
+	int enable = 1;
+	int ret;
+
+	self->rx = j1939_open(self->ifindex, SA_RX, PGN_A);
+	ASSERT_GE(self->rx, 0);
+	self->tx = j1939_open(self->ifindex, SA_TX, J1939_NO_PGN);
+	ASSERT_GE(self->tx, 0);
+
+	/*
+	 * This frame uses a different address and PGN than rx is bound to.
+	 * Without promisc, rx must not receive it.
+	 */
+	ret = j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			   J1939_NO_NAME, SA_OTHER, PGN_B);
+	ASSERT_EQ(ret, (int)sizeof(tx_buf))
+		TH_LOG("sendto unmatched frame failed: %d", errno);
+	EXPECT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_NEG_RECV_TIMEOUT_MS), -1)
+		TH_LOG("unmatched frame was received without promisc");
+
+	/* SO_J1939_PROMISC returns the flag value (1), not 0, on success. */
+	ASSERT_GE(setsockopt(self->rx, SOL_CAN_J1939, SO_J1939_PROMISC,
+			     &enable, sizeof(enable)), 0)
+		TH_LOG("SO_J1939_PROMISC failed: %d", errno);
+
+	/* The same otherwise-unmatched frame must now be delivered. */
+	ret = j1939_sendto(self->tx, tx_buf, sizeof(tx_buf), self->ifindex,
+			   J1939_NO_NAME, SA_OTHER, PGN_B);
+	ASSERT_EQ(ret, (int)sizeof(tx_buf))
+		TH_LOG("sendto unmatched frame failed: %d", errno);
+	ASSERT_EQ(recv_timeout(self->rx, rx_buf, sizeof(rx_buf),
+			       J1939_RECV_TIMEOUT_MS), (int)sizeof(tx_buf))
+		TH_LOG("unmatched frame not received despite promisc");
+	EXPECT_EQ(memcmp(tx_buf, rx_buf, sizeof(tx_buf)), 0);
+}
+
 /* without SO_J1939_SEND_PRIO the J1939 default priority (6) is used. */
 TEST_F(j1939_opts, send_prio_default)
 {
-- 
2.47.3

Re: [PATCH can-next v1 0/8] selftests: can: add J1939 socket and transport tests

[Marc Kleine-Budde]

> net/can/j1939 has had no in-tree selftests. This series adds a kselftest
> suite for the J1939 socket layer and the TP/ETP transport, run on vcan with
> a CAN_RAW socket as an independent on-the-wire peer.
>
> The whole suite runs on vcan under QEMU and passes. Two limits span the
> series and are left for follow-up: the transport timers are checked only
> with wide windows, not tight bounds, and the ETP abort/timeout edge cases
> are not covered yet (the abort/timeout tests are TP only).

please run checkpatch on the series before sending it.

regards,
Marc

-- 
Marc Kleine-Budde <mkl@pengutronix.de>

Re: [PATCH can-next v1 1/8] selftests: can: add J1939 test helper header and library

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] j1939_lib.sh is new and is installed through TEST_FILES in
  tools/testing/selftests/net/can/Makefile.
- [Medium] send_recv_verify() in j1939_tests.h says it verifies that len
  bytes 'arrive byte-for-byte on rx', but it cannot catch a delivery…
- [Low] j1939_lib.sh installs `trap cleanup EXIT INT TERM`, but cleanup()
  never calls exit.
- [Low] On the real-CAN path (CANIF not vcan*), setup() reprograms the
  interface with `ip link set dev "$CANIF" type can bitrate "$BITRATE"`…

--

selftests: can: add J1939 test helper header and library

This adds j1939_tests.h, which holds the shared C helpers for the J1939
selftests, and j1939_lib.sh, which holds the shared vcan setup, teardown
and test runner. No tests use them yet; the following patches in the
series add the tests.

> diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
> index 5b82e60a03e7..e2092c289109 100644
> --- a/tools/testing/selftests/net/can/Makefile
> +++ b/tools/testing/selftests/net/can/Makefile
> @@ -6,6 +6,8 @@ CFLAGS += -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INC
>  
>  TEST_PROGS := test_raw_filter.sh
>  
> +TEST_FILES := j1939_lib.sh
> +

[Severity: Medium]
Does net/can also need a TEST_INCLUDES line here?

j1939_lib.sh sources ../lib.sh, and net/lib.sh in turn sources
lib/sh/defer.sh. net/can/Makefile does not declare either file for
installation.

net/lib.sh is only installed through TEST_FILES in net/Makefile. The top
level selftests Makefile only pulls in the net lib dependency when TARGETS
contains net, drivers/net or drivers/net/hw:

tools/testing/selftests/Makefile:
    ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),)

With make TARGETS=net/can install, or when net/can is packaged on its own,
would every J1939 wrapper added later in the series (test_j1939_base.sh,
_opts, _addr, _wire, _multi) fail at the source line? They would then run
with ksft_skip, check_err, log_test, tests_run and EXIT_STATUS undefined.

net/mptcp/Makefile handles the same dependency with:

    TEST_INCLUDES := ../lib.sh $(wildcard ../lib/sh/*.sh)

test_raw_filter.sh already has a similar gap. This patch, though, adds a
new shared library that the following tests depend on, and the gap is
still there at the end of the series.

>  TEST_GEN_FILES := test_raw_filter
>  
>  include ../../lib.mk
> diff --git a/tools/testing/selftests/net/can/j1939_lib.sh b/tools/testing/selftests/net/can/j1939_lib.sh
> new file mode 100644
> index 000000000000..7b7fff228b16
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/j1939_lib.sh

[ ... ]

> +# Only undo what this script actually did: delete a vcan we created, and bring
> +# down an interface only if we brought it up. A failed setup() (e.g. CANIF
> +# already exists) must not disturb a pre-existing interface.
> +created=0
> +brought_up=0
> +
> +setup()
> +{
> +	if [[ $CANIF == vcan* ]]; then
> +		ip link add name "$CANIF" type vcan || exit $ksft_skip
> +		created=1
> +	else
> +		ip link set dev "$CANIF" type can bitrate "$BITRATE" || exit $ksft_skip
> +	fi
> +	ip link set dev "$CANIF" up || exit $ksft_skip
> +	brought_up=1
> +}
> +
> +cleanup()
> +{
> +	if [ "$brought_up" = 1 ]; then
> +		ip link set dev "$CANIF" down 2>/dev/null
> +	fi
> +	if [ "$created" = 1 ]; then
> +		ip link delete "$CANIF" 2>/dev/null
> +	fi
> +}

[Severity: Low]
On the real CAN path (CANIF not vcan*), setup() reprograms an existing
interface to $BITRATE. It does not save the previous bittiming. cleanup()
only undoes brought_up and created.

Does this leave the interface at 250 kbit/s after the test? The same
question applies when the following ip link set ... up fails and setup()
exits with brought_up=0.

Bittiming survives down/up, so the next user of the interface would bring
it up at the new rate. That seems to contradict the comment above about
not disturbing a pre-existing interface. test_raw_filter.sh behaves the
same way.

[ ... ]

> +trap cleanup EXIT INT TERM
> +setup
> +
> +tests_run
> +
> +exit $EXIT_STATUS

[Severity: Low]
cleanup() never calls exit. In bash, when the INT or TERM trap returns,
the script carries on with the next command.

If the signal arrives between setup and tests_run, could the test binary
still run against the vcan that cleanup() just deleted? J1939_SETUP_IFINDEX
would fail and log_test would report a FAIL.

If the signal arrives while the binary is running, log_test and
exit $EXIT_STATUS still run after the trap returns, and the EXIT trap then
calls cleanup() a second time.

bash also runs the EXIT trap on these signals. Would trap cleanup EXIT be
enough on its own, as in test_raw_filter.sh?

> diff --git a/tools/testing/selftests/net/can/j1939_tests.h b/tools/testing/selftests/net/can/j1939_tests.h
> new file mode 100644
> index 000000000000..dd70a7d60ec7
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/j1939_tests.h

[ ... ]

> +/* poll() + recv(): returns the recv() result, or -1 on timeout/error. */
> +static inline int recv_timeout(int s, void *buf, size_t len, int timeout_ms)
> +{
> +	struct pollfd pfd = { .fd = s, .events = POLLIN };
> +
> +	if (poll(&pfd, 1, timeout_ms) <= 0)
> +		return -1;
> +	return recv(s, buf, len, 0);
> +}
> +
> +/* Send len bytes on tx and verify they arrive byte-for-byte on rx. 0 / -1. */
> +static inline int send_recv_verify(int tx, int rx, const void *buf, size_t len,
> +				   int timeout_ms)
> +{
> +	uint8_t *rx_buf = malloc(len);
> +	int rc = -1;
> +
> +	if (!rx_buf)
> +		return -1;
> +	if (send(tx, buf, len, 0) == (ssize_t)len &&
> +	    recv_timeout(rx, rx_buf, len, timeout_ms) == (int)len &&
> +	    memcmp(buf, rx_buf, len) == 0)

[Severity: Medium]
Can send_recv_verify() catch a delivery that is longer than len?

rx_buf is exactly len bytes, and recv_timeout() calls recv(s, buf, len, 0).
For an oversized datagram, j1939_sk_recvmsg() does:

net/can/j1939/socket.c:j1939_sk_recvmsg() {
    ...
	if (size < skb->len)
		msg->msg_flags |= MSG_TRUNC;
	else
		size = skb->len;
    ...
	return size;
}

So recv() returns len and the MSG_TRUNC indication is lost. An over-long
delivery, for example from a TP/ETP reassembly regression that adds
padding bytes, would pass both the == (int)len check and the memcmp() over
the prefix.

Later in the series, test_j1939_base.c and test_j1939_addr.c use this
helper to check payload integrity at the transport size boundaries.

Passing MSG_TRUNC as an input flag won't work, because j1939_sk_recvmsg()
returns -EINVAL for any flag other than MSG_DONTWAIT, MSG_ERRQUEUE and
MSG_CMSG_COMPAT. Would a len + 1 receive buffer, or recvmsg() with a check
of msg_flags, work instead?

> +		rc = 0;
> +	free(rx_buf);
> +	return rc;
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 2/8] selftests: can: add J1939 base functional tests

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] test_j1939_base.sh breaks when net/can is installed on its
  own.
- [Low] roundtrip() in test_j1939_base.c is documented as checking that
  the payload is "received byte for byte", and size_boundaries claims to…
- [Low] Baseline decision: false positive for the concrete scenarios
  given.
- [Low] roundtrip() in tools/testing/selftests/net/can/test_j1939_base.c
  (lines 104-106) prints `errno %d` whenever send_recv_verify() fails.

--

selftests: can: add J1939 base functional tests

This adds a kselftest for the SAE J1939 socket interface. It opens a
sender and a receiver CAN_J1939 socket on a vcan interface and checks
payload integrity for single frame, TP, ETP, the size boundaries between
them, and single-frame and BAM broadcast. The test runs through a new
test_j1939_base.sh wrapper that sources j1939_lib.sh.

> diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
> index e2092c289109..cf4bd2461a6c 100644
> --- a/tools/testing/selftests/net/can/Makefile
> +++ b/tools/testing/selftests/net/can/Makefile
> @@ -4,10 +4,16 @@ top_srcdir = ../../../../..
>  
>  CFLAGS += -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INCLUDES)
>  
> -TEST_PROGS := test_raw_filter.sh
> +LOCAL_HDRS := j1939_tests.h
> +
> +TEST_PROGS := \
> +	test_raw_filter.sh \
> +	test_j1939_base.sh \
>  
>  TEST_FILES := j1939_lib.sh

[Severity: Medium]
Will test_j1939_base.sh still work when only net/can is installed?

The wrapper sources j1939_lib.sh, which is in TEST_FILES. But j1939_lib.sh
then sources a file from the parent directory:

tools/testing/selftests/net/can/j1939_lib.sh:
    net_dir=$(dirname "$0")/..
    source "$net_dir/lib.sh"

net/lib.sh then sources $net_dir/lib/sh/defer.sh.

This Makefile has no TEST_INCLUDES, so lib.mk doesn't install ../lib.sh
or ../lib/sh/*.sh for this target. The top-level selftests Makefile only
adds net/lib for these targets:

    ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),)

After "make -C tools/testing/selftests TARGETS=net/can install", wouldn't
the new test fail at the source step with "No such file or directory"
instead of running or skipping?

Other net subdirectories with the same dependency declare it. For example,
net/mptcp/Makefile has:

    TEST_INCLUDES := ../lib.sh $(wildcard ../lib/sh/*.sh)

test_raw_filter.sh already sources ../lib.sh the same way. Since the new
test depends on it too, could a similar TEST_INCLUDES line go in here?
The Makefile still has no TEST_INCLUDES at the end of the series.

[ ... ]

> diff --git a/tools/testing/selftests/net/can/test_j1939_base.c b/tools/testing/selftests/net/can/test_j1939_base.c
> new file mode 100644
> index 000000000000..59f1448b7fb2
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_j1939_base.c

[ ... ]

> +/*
> + * Send a deterministic payload of the given size on tx and verify it is
> + * received byte for byte on rx. Returns 0 on success, -1 otherwise.
> + */
> +static int roundtrip(FIXTURE_DATA(j1939_base) *self, size_t size)
> +{
> +	uint8_t *tx_buf;
> +	int rc;
> +
> +	tx_buf = malloc(size);
> +	if (!tx_buf) {
> +		fprintf(stderr, "# alloc failed\n");
> +		return -1;
> +	}
> +	fill_pattern(tx_buf, size);
> +
> +	rc = send_recv_verify(self->tx, self->rx, tx_buf, size,
> +			      J1939_RECV_TIMEOUT_MS);

[Severity: Low]
Can this check catch a message that arrives longer than it was sent?

send_recv_verify() in j1939_tests.h receives into a buffer of exactly len
bytes and compares only those len bytes:

    uint8_t *rx_buf = malloc(len);
    ...
    recv_timeout(rx, rx_buf, len, timeout_ms) == (int)len &&
    memcmp(buf, rx_buf, len) == 0)

On the kernel side, j1939_sk_recvmsg() truncates without an error and
returns the user buffer size:

    if (size < skb->len)
        msg->msg_flags |= MSG_TRUNC;
    else
        size = skb->len;
    ...
    return size;

Plain recv() can't see the MSG_TRUNC output flag. So if TP/ETP reassembly
ever delivered extra bytes, every subtest here would still pass. That
includes the 9/1785/1786 cases in size_boundaries.

Passing MSG_TRUNC as an input flag won't work either. j1939_sk_recvmsg()
returns -EINVAL for any flag other than MSG_DONTWAIT, MSG_ERRQUEUE and
MSG_CMSG_COMPAT.

Two options that might work: receive into a buffer of at least len + 1
bytes and require recv() to return exactly len, or use recvmsg() and
check msg_flags. The helper is unchanged at the end of the series, and
test_j1939_addr.c uses it too.

> +	if (rc)
> +		fprintf(stderr, "# round-trip failed at size %zu, errno %d\n",
> +			size, errno);

[Severity: Low]
Is errno meaningful on every path that reaches this message?

When poll() times out, recv_timeout() returns -1 without setting errno:

    if (poll(&pfd, 1, timeout_ms) <= 0)
        return -1;

A memcmp() mismatch or a short send()/recv() count also leaves errno
unchanged. A timeout or a data mismatch would then print whatever errno
an earlier call left behind, often 0.

Could the timeout and mismatch cases print their own messages?

[ ... ]

> +/* exact size boundaries around single / TP / ETP transitions. */
> +TEST_F(j1939_base, size_boundaries)
> +{
> +	static const size_t sizes[] = { 8, 9, 1785, 1786 };
> +	size_t k;
> +
> +	for (k = 0; k < ARRAY_SIZE(sizes); k++) {
> +		ASSERT_EQ(pair_up(self, SA_RX, SA_RX, PGN_PDU1, 0), 0);
> +		EXPECT_EQ(roundtrip(self, sizes[k]), 0)
> +			TH_LOG("failed at boundary size %zu", sizes[k]);
> +	}
> +}

[Severity: Low]
This isn't a bug, but does this test, or tp_roundtrip and etp_roundtrip,
check which transport was actually used?

Only payload delivery is checked. A one-sided transmit selection bug in
j1939_tp_send() would still be caught. The receive side checks the size
on its own in j1939_xtp_rx_rts_session_new() and aborts the session:

    else if (len <= J1939_MAX_TP_PACKET_SIZE)
        abort = J1939_XTP_ABORT_FAULT;
    ...
    else if (len < J1939_MIN_TP_PACKET_SIZE)
        abort = J1939_XTP_ABORT_FAULT;

However, the TX and RX paths share J1939_MAX_TP_PACKET_SIZE. A symmetric
change to that constant would not be noticed by this loopback-only test.
Nothing here checks whether TP.CM_RTS (0x10) or ETP.CM_RTS (0x14) went
out on the wire.

The later "selftests: can: add J1939 TP/ETP multi-frame tests against
CAN_RAW" patch does check the RTS command on the wire, but only at sizes
200 and 6000.

Would it make sense to add the 8/9/1785/1786 sizes there, or to reword
the "exact size boundaries" comment here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 3/8] selftests: can: add J1939 socket option and metadata tests

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] FIXTURE_SETUP(j1939_opts) always calls J1939_SETUP_RAW(self,
  0).
- [Medium] raw_prio() takes the first frame received on the unfiltered
  CAN_RAW socket and returns J1939_ID_PRIO(frame.can_id & CAN_EFF_MASK).
- [Low] test_j1939_opts.sh cannot run from a selective install of the CAN
  selftests (e.g. `make -C tools/testing/selftests TARGETS=net/can…

--

selftests: can: add J1939 socket option and metadata tests

This adds a new J1939 selftest, test_j1939_opts, that covers:

  - SO_J1939_FILTER
  - SO_J1939_SEND_PRIO, observed on the wire through a CAN_RAW socket
  - SO_J1939_PROMISC
  - unconnected sendto()
  - a bind() with the wrong address family
  - SO_J1939_ERRQUEUE TX-completion notifications

It also enables CONFIG_CAN_RAW in the test config fragment.

> diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
> index cf4bd2461a6c..24a57edb912c 100644
> --- a/tools/testing/selftests/net/can/Makefile
> +++ b/tools/testing/selftests/net/can/Makefile
> @@ -9,11 +9,13 @@ LOCAL_HDRS := j1939_tests.h
>  TEST_PROGS := \
>  	test_raw_filter.sh \
>  	test_j1939_base.sh \
> +	test_j1939_opts.sh \
>  
>  TEST_FILES := j1939_lib.sh

[Severity: Low]
Can test_j1939_opts.sh run from a CAN-only install, such as
make -C tools/testing/selftests TARGETS=net/can install?

The wrapper sources j1939_lib.sh, which does:

tools/testing/selftests/net/can/j1939_lib.sh:
    net_dir=$(dirname "$0")/..
    source "$net_dir/lib.sh"

net/lib.sh then sources lib/sh/defer.sh. This Makefile lists only
j1939_lib.sh in TEST_FILES. Nothing installs ../lib.sh or ../lib/sh/*.sh.

Other net subdirectories declare these files. For example,
net/mptcp/Makefile has:

    TEST_INCLUDES := ../lib.sh $(wildcard ../lib/sh/*.sh)

The top-level selftests Makefile adds net/lib as an install dependency
only for the net, drivers/net and drivers/net/hw targets, not for net/can.
Even when net/lib is added, it does not install net/lib.sh.

In the installed tree, sourcing ../lib.sh would fail. That leaves
ksft_skip, tests_run, check_err and log_test undefined, so the test can't
run.

test_raw_filter.sh has the same gap. The first patch of this series
carried it into j1939_lib.sh, this patch adds another test that depends on
it, and nothing later in the series fixes it.

Would a TEST_INCLUDES line like the mptcp one fix this?

[ ... ]

> diff --git a/tools/testing/selftests/net/can/test_j1939_opts.c b/tools/testing/selftests/net/can/test_j1939_opts.c
> new file mode 100644
> index 000000000000..f761a42db213
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_j1939_opts.c
> @@ -0,0 +1,305 @@

[ ... ]

> +/* Read the next CAN frame from a raw socket and return its J1939 priority. */
> +static int raw_prio(int raw, int timeout_ms)
> +{
> +	struct can_frame frame;
> +
> +	if (recv_timeout(raw, &frame, sizeof(frame), timeout_ms) < 0)
> +		return -1;
> +	return (int)J1939_ID_PRIO(frame.can_id & CAN_EFF_MASK);
> +}

[Severity: Medium]
Can raw_prio() decode a frame other than the one the test just sent?

self->raw comes from open_raw_rcvbuf(). That binds without a
CAN_RAW_FILTER, so CAN_RAW's default receive-all filter applies.

raw_prio() returns the priority of the first frame that arrives. It does
not check that J1939_ID_SA() is SA_TX, J1939_ID_PF() is J1939_PF(PGN_A) or
J1939_ID_PS() is SA_RX.

j1939_lib.sh supports real CAN interfaces as well as vcan:

    else
        ip link set dev "$CANIF" type can bitrate "$BITRATE" || exit $ksft_skip
    fi

On a bus with other nodes, a frame from another node could arrive between
FIXTURE_SETUP and the J1939 send. raw_prio() would decode that frame
instead. send_prio_default and send_prio_set would then compare its
priority with 6 or 3, and could fail or pass wrongly.

j1939_tests.h already has recv_match() and recv_for_peer() for this case.
The comment in recv_match() covers it: "Non-matching frames (other
traffic on the bus) ...".

Could raw_prio() use one of these, matching on source address, PF and PS?

On the default private vcan only the test's own frame is on the bus, so
the default run gives a reliable result.

[ ... ]

> +FIXTURE_SETUP(j1939_opts)
> +{
> +	self->tx = -1;
> +	self->rx = -1;
> +	self->raw = -1;
> +
> +	J1939_SETUP_IFINDEX(self);
> +
> +	J1939_SETUP_RAW(self, 0);
> +}

[Severity: Medium]
If a kernel has CONFIG_CAN_J1939 enabled but no CAN_RAW support, should
these tests be reported as skipped instead of failed?

J1939_SETUP_RAW() opens socket(PF_CAN, SOCK_RAW, CAN_RAW) through
open_raw_rcvbuf() and then calls ASSERT_GE() on the result. A missing
CAN_RAW protocol therefore fails the test instead of skipping it.

J1939_TEST_MAIN() says it will "Skip (rather than fail) when either is
missing", but it only probes CAN_J1939:

    probe = socket(PF_CAN, SOCK_DGRAM, CAN_J1939);
    if (probe < 0) {
        if (errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT)
            return KSFT_SKIP;

CONFIG_CAN_RAW is a separate tristate, which is why this patch adds it to
the config fragment. Without it, all nine tests in the fixture fail in
setup.

Seven of the nine never use self->raw:

  filter_pgn
  sendto_unconnected
  bind_wrong_family
  errqueue_tx_completion
  getsockopt_send_prio
  getsockopt_promisc
  getsockopt_errqueue

Only send_prio_default and send_prio_set use it, through raw_prio().
Nothing later in the series changes this.

Could the raw socket be opened only in the two send_prio tests? Or could
the test skip when CAN_RAW is not available?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 4/8] selftests: can: add J1939 addressing and concurrency tests

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 4

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Several tests sync with the kernel's address-claim commit
  timer only through a fixed `usleep(ACLAIM_SETTLE_US)` of 300 ms.
- [Low] The comment above the name_addressing test says "NAME based
  addressing reaches the socket bound to that NAME", but no receiver is…
- [Low] The shared runner j1939_lib.sh setup() accepts a real CAN
  interface (`else ip link set dev "$CANIF" type can bitrate…
- [Low] stress_concurrent_sessions claims to show that "many concurrent
  transport-protocol sessions run without corruption", but all eight…
- [Low] In stress_concurrent_sessions, if fork() fails at iteration k >
  0, `ASSERT_GE(pids[i], 0)` goes through __bail(), which runs the…

--

selftests: can: add J1939 addressing and concurrency tests

This adds a kselftest for J1939 NAME-based addressing and address
claiming. It covers NAME bind and getsockname readback, the source
address on the wire, NAME-addressed transfers using a forged Address
Claimed, and address-claim conflicts. It also runs several concurrent
transport sessions as a stress check.

> diff --git a/tools/testing/selftests/net/can/test_j1939_addr.c b/tools/testing/selftests/net/can/test_j1939_addr.c
> new file mode 100644
> index 000000000000..8b1b093f0839
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_j1939_addr.c
> @@ -0,0 +1,336 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * J1939 addressing and concurrency selftests.

[ ... ]

> + * The vcan interface is set up by the caller (see test_j1939_addr.sh); its name
> + * is passed via the CANIF environment variable.
> + */

[Severity: Low]
setup() in the shared runner j1939_lib.sh also accepts a real CAN
device:

	else
		ip link set dev "$CANIF" type can bitrate "$BITRATE" || exit $ksft_skip
	fi

If CANIF points at a real bus with other J1939 nodes, what happens to an
ECU that already holds 0x50 or 0x90?

forge_address_claim() sends an Address Claimed from a CAN_RAW socket for
SA_RX (0x90) in name_addressing. It also sends one for SA_CLAIM (0x50)
with NAME_LOWER (0x00000000c0ffee00) in ac_conflict_lose, and
claim_addr() claims 0x50 too. A compliant ECU that sees a competing claim
with a lower NAME has to give up its address. The kernel follows the same
rule in j1939_ac_process().

Should these tests skip when CANIF is not a vcan? If not, could the
header comment and commit message at least mention this side effect?

[ ... ]

> +#define STRESS_SIZE		128	/* arbitrary test tuning */
> +#define ACLAIM_SETTLE_US	300000	/* settle: empirical, not spec */

[ ... ]

> +static int forge_address_claim(int raw, uint8_t sa, name_t name)
> +{
> +	uint8_t nb[8];
> +
> +	put_name(nb, name);
> +	if (send_raw8(raw, J1939_PRIO_DEFAULT, J1939_PGN_ADDRESS_CLAIMED,
> +		      J1939_NO_ADDR, sa, nb))
> +		return -1;
> +	/* give the kernel RX path a moment to process the claim */
> +	usleep(ACLAIM_SETTLE_US);
> +	return 0;
> +}

[Severity: Medium]
Could this fixed 300 ms sleep make the tests flaky?

The kernel maps a newly claimed address only when the ECU's ac_timer
fires:

net/can/j1939/bus.c:j1939_ecu_timer_start() {
	...
	/* Schedule timer in 250 msec to commit address change. */
	hrtimer_start(&ecu->ac_timer, ms_to_ktime(250),
		      HRTIMER_MODE_REL_SOFT);
}

Until j1939_ecu_timer_handler() runs, j1939_name_to_addr() returns
J1939_IDLE_ADDR. j1939_ac_fixup() then fails with -EADDRNOTAVAIL on both
the src_name path and the dst_name path.

That leaves about 50 ms of slack. The soft hrtimer can be delayed, for
example when softirq work is pushed to ksoftirqd on a loaded VM, or on
debug or RT kernels. In that case the send after the sleep fails.

This affects the send in name_addressing and the final send in
address_claim_send. It also affects the baseline send_data() in
ac_conflict_probe(), so both ac_conflict_lose and ac_conflict_defend
would fail.

Would polling or retrying until a deadline be more robust here?

The comments also look inaccurate. ACLAIM_SETTLE_US is labelled
"empirical, not spec", but it has to be longer than the kernel's fixed
250 ms commit timer. The forge_address_claim() comment says it gives "the
kernel RX path a moment", but the sleep is really waiting for that timer.

> +
> +/* One independent transport-protocol round-trip; returns 0 on success. */
> +static int stress_child(int ifindex, uint8_t sa_tx, uint8_t sa_rx)
> +{
> +	uint8_t tx_buf[STRESS_SIZE];
> +	int tx, rx, rc = 1;
> +
> +	fill_pattern(tx_buf, sizeof(tx_buf));

[Severity: Low]
Every child sends the same payload here, because fill_pattern() uses only
the byte position:

	buf[i] = (uint8_t)(i ^ (i >> 8) ^ 0x3c);

send_recv_verify() only checks the length and runs memcmp() against that
same buffer.

Suppose a regression mixed up sessions. A wrong session lookup could
consume another session's TP.DT packets, or two receivers could get each
other's data. Would stress_concurrent_sessions still pass?

Adding a per-session byte to each payload (for example, one derived from
sa_tx) would make this kind of mixing visible.

[ ... ]

> +/*
> + * NAME based addressing reaches the socket bound to that NAME.
> + *
> + * The peer must have claimed its address before it is reachable by NAME, so
> + * inject an Address Claimed message for (SA_RX, NAME_RX) first. The sender
> + * uses a static source address and resolves the destination by NAME.
> + */
> +TEST_F(j1939_addr, name_addressing)
> +{
> +	uint8_t tx_buf[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
> +	uint8_t rx_buf[8];
> +
> +	/* static receiver on SA_RX; the forged claim maps NAME_RX -> SA_RX */
> +	self->rx = j1939_open_name(self->ifindex, J1939_NO_NAME, SA_RX, PGN_TEST);

[Severity: Low]
Is the header comment accurate? It says the transfer "reaches the socket
bound to that NAME". But the receiver is bound with J1939_NO_NAME, so
jsk->addr.src_name is 0.

As a result, j1939_sk_match_dst() skips the NAME branch:

net/can/j1939/socket.c:j1939_sk_match_dst() {
	...
	if (jsk->addr.src_name && skcb->addr.dst_name) {
		if (jsk->addr.src_name != skcb->addr.dst_name)
			return false;
	} else {
		...
		if (j1939_address_is_unicast(skcb->addr.da)) {
			if (jsk->addr.sa != skcb->addr.da)
				return false;
	...
}

The frame is delivered through the plain SA check instead.

The test does cover the TX side, where j1939_ac_fixup() resolves NAME_RX
to SA_RX. It never runs the RX-side NAME filter, though, so it would
still pass if that filter were broken.

Could the comment be reworded to match what is tested? Another option is
to bind the receiver to NAME_RX so the NAME match path runs as well.

[ ... ]

> +/* many concurrent transport-protocol sessions run without corruption. */
> +TEST_F(j1939_addr, stress_concurrent_sessions)
> +{
> +	pid_t pids[STRESS_N];
> +	int i, failed = 0;
> +
> +	for (i = 0; i < STRESS_N; i++) {
> +		pids[i] = fork();
> +		ASSERT_GE(pids[i], 0);

[Severity: Low]
If fork() fails partway through this loop, what happens to the children
that were already forked?

ASSERT_GE() runs the fixture teardown and aborts the test, so the
waitpid() loop below is skipped. The earlier children are neither killed
nor reaped. They keep running TP sessions with rx and tx sockets bound to
0x20 + i and 0x40 + i. The harness kills the process group only on
timeout, in __wait_for_test().

They do exit on their own once J1939_RECV_TIMEOUT_MS and the TP timeouts
expire, so this only matters when the test has already failed. They
could still overlap with the next test on the same interface.

Should the forked children be killed and reaped before the test fails?

> +		if (pids[i] == 0)
> +			_exit(stress_child(self->ifindex, 0x40 + i, 0x20 + i));
> +	}
> +
> +	for (i = 0; i < STRESS_N; i++) {
> +		int status;
> +
> +		if (waitpid(pids[i], &status, 0) < 0 ||

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 5/8] selftests: can: add J1939 wire-format tests against CAN_RAW

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] On a real CAN interface, tx_to_raw_send_recv() in
  tools/testing/selftests/net/can/test_j1939_wire.c can check the wrong…
- [Low] The TX tests check only part of the CAN id, although the file
  header (lines 6-8) says they verify 'the expected CAN id (priority,…
- [Low] The RX tests do not check how the kernel parses the source
  address or priority, although the file header says frames are checked…
- [Low] The commit message overstates the gap the test fills.

--

selftests: can: add J1939 wire-format tests against CAN_RAW

This adds test_j1939_wire, which pairs a J1939 socket with a CAN_RAW
socket so the CAN frames on the wire are checked in both directions. It
covers PDU1 (destination specific) and PDU2 (broadcast) single frames,
both for J1939 TX seen on CAN_RAW and for raw-injected frames received
on a J1939 socket.

> The base test only exercises J1939-socket-to-J1939-socket
> transfers, which stay on the local delivery path inside one netdevice, so
> the on-wire format is otherwise unchecked.

[Severity: Low]
Is this description accurate?

Earlier patches in this series already look at on-wire CAN ids through
a CAN_RAW peer. raw_prio() in test_j1939_opts.c (from "selftests: can:
add J1939 socket option and metadata tests") decodes the priority:

    return (int)J1939_ID_PRIO(frame.can_id & CAN_EFF_MASK);

source_sa_on_wire() in test_j1939_addr.c (from "selftests: can: add
J1939 addressing and concurrency tests") checks the source address:

    EXPECT_EQ((int)J1939_ID_SA(frame.can_id), SA_TX)

J1939-to-J1939 traffic also does not take a shortcut inside the stack.
j1939_send_one() builds the full CAN id and loops the frame back through
can_send():

net/can/j1939/main.c:j1939_send_one() {
    ...
	cf->can_id = canid;
	cf->len = dlc;

	return can_send(skb, 1);
}

j1939_can_recv() then decodes the looped-back frame, so the base test
already exercises encode and decode. It only misses errors that are
symmetric between the two.

Could the commit message describe what this patch actually adds? That
is the PF/PS/full-PGN and payload checks for PDU1/PDU2, plus delivery
of injected raw frames to J1939 data sockets.

> diff --git a/tools/testing/selftests/net/can/test_j1939_wire.c b/tools/testing/selftests/net/can/test_j1939_wire.c
> new file mode 100644
> index 000000000000..8f5113394f99
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_j1939_wire.c
> @@ -0,0 +1,170 @@

[ ... ]

> +static int rx_single_frame_check(FIXTURE_DATA(j1939_wire) *self, bool bcast,
> +				 pgn_t pgn, uint8_t da, const uint8_t payload[8])
> +{
> +	uint8_t rx_buf[8];
> +
> +	if (bcast)
> +		self->j1939 = j1939_open_flags(self->ifindex, J1939_NO_NAME,
> +					       SA_LOCAL, pgn, J1939_OPEN_BCAST);
> +	else
> +		self->j1939 = j1939_open(self->ifindex, SA_LOCAL, pgn);
> +	if (self->j1939 < 0)
> +		return -1;
> +
> +	if (send_raw8(self->raw, PRIO, pgn, da, SA_PEER, payload))
> +		return -1;
> +
> +	if (recv_timeout(self->j1939, rx_buf, sizeof(rx_buf),
> +			 J1939_RECV_TIMEOUT_MS) != (int)sizeof(rx_buf))
> +		return -1;
> +
> +	return memcmp(rx_buf, payload, sizeof(rx_buf)) == 0 ? 0 : -1;
> +}

[Severity: Low]
Does this check how the kernel parses the source address or the
priority? The file header says an injected frame "is parsed and
delivered to the J1939 socket". However, recv_timeout() is a plain
recv(), so the sockaddr_can holding the parsed SA/PGN is thrown away.
Only the payload gets compared.

The RX socket is bound but never connected. j1939_sk_match_dst() only
applies the source address filter to connected sockets:

net/can/j1939/socket.c:j1939_sk_match_dst() {
    ...
	/* Source address filter */
	if (jsk->state & J1939_SOCK_CONNECTED) {
    ...
}

Suppose j1939_can_recv() decoded skcb->addr.sa wrongly (to any value
other than 0xff), or decoded skcb->priority wrongly. The payload would
still be delivered, and rx_pdu1_from_raw and rx_pdu2_from_raw would
pass. Wrong PGN or DA parsing would stop delivery, so those are only
covered indirectly.

Would it work to use recvfrom() here and check that
addr.can_addr.j1939.addr == SA_PEER and addr.can_addr.j1939.pgn == pgn?
The SCM_J1939_PRIO cmsg could also be checked.

> +static int tx_to_raw_send_recv(FIXTURE_DATA(j1939_wire) *self, bool bcast,
> +			       pgn_t pgn, uint8_t da, const uint8_t payload[8],
> +			       struct can_frame *frame)
> +{

[ ... ]

> +	if (send(self->j1939, payload, 8, 0) != 8)
> +		return -1;
> +
> +	if (recv_timeout(self->raw, frame, sizeof(*frame),
> +			 J1939_RECV_TIMEOUT_MS) <= 0)
> +		return -1;
> +
> +	return 0;
> +}

[Severity: Medium]
Can this return an unrelated frame when CANIF is a real CAN interface?

FIXTURE_SETUP opens self->raw with J1939_SETUP_RAW(self, 0), which keeps
the default accept-all filter, before the J1939 socket exists. The first
frame it returns is assumed to be the one the J1939 socket just sent.
Nothing checks SA, PGN or destination before tx_pdu1_to_raw() and
tx_pdu2_to_raw() assert on its fields.

j1939_lib.sh supports non-vcan interfaces explicitly:

tools/testing/selftests/net/can/j1939_lib.sh:setup() {
	if [[ $CANIF == vcan* ]]; then
		ip link add name "$CANIF" type vcan || exit $ksft_skip
		created=1
	else
		ip link set dev "$CANIF" type can bitrate "$BITRATE" || exit $ksft_skip
	fi
    ...
}

If another node sends a frame on that bus, the PRIO/SA/PS/PF/payload
checks run against that frame, and the TX tests fail on a correct
kernel. A freshly created vcan has no other traffic, so the default run
is not affected.

j1939_tests.h already has a helper for this case. The comment in
recv_match() says:

	/*
	 * Non-matching frames (other traffic on the bus) must not refresh the
	 * timeout, or a busy bus would keep the loop alive forever. ...

Could this use recv_for_peer(self->raw, J1939_PF(pgn), da, frame, ...)
for PDU1, and a recv_match() predicate on the full PGN plus SA_LOCAL
for PDU2?

The same unfiltered pattern is already in source_sa_on_wire() and
raw_prio() from earlier patches in this series. This patch adds new
instances of it.

Also, the file header says "The vcan interface is set up by the caller
(see test_j1939_wire.sh)". The wrapper accepts non-vcan interfaces too,
so the header does not match.

> +/* J1939 TX -> CAN_RAW: destination-specific (PDU1) single frame. */
> +TEST_F(j1939_wire, tx_pdu1_to_raw)
> +{
> +	uint8_t payload[8] = { 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88 };
> +	struct can_frame frame;
> +	canid_t id;
> +
> +	ASSERT_EQ(tx_to_raw_send_recv(self, false, PGN_PDU1, SA_PEER, payload,
> +				      &frame), 0);
> +	id = frame.can_id & CAN_EFF_MASK;
> +
> +	EXPECT_EQ(J1939_ID_PRIO(id), PRIO);
> +	EXPECT_EQ(J1939_ID_SA(id), SA_LOCAL);			/* source address */
> +	EXPECT_EQ(J1939_ID_PS(id), SA_PEER);		/* destination (PS) */
> +	EXPECT_EQ(J1939_ID_PF(id), J1939_PF(PGN_PDU1));

[Severity: Low]
Does this cover the whole CAN id that the file header promises ("the
expected CAN id (priority, PGN, source and destination)")?

Both tx_pdu1_to_raw() and tx_pdu2_to_raw() mask with CAN_EFF_MASK
before any assertion, so CAN_EFF_FLAG (and RTR/ERR) are never checked.
j1939_send_one() builds the id with the flag set:

	canid = CAN_EFF_FLAG |
		(skcb->priority << 26) |
		(skcb->addr.pgn << 8) |
		skcb->addr.sa;

A regression that dropped CAN_EFF_FLAG would still pass.

For PDU1, the PGN check here only covers PF (bits 16-23). The DP and
EDP bits (24-25) are never checked. tx_pdu2_to_raw() compares the full
PGN with (id >> 8) & J1939_PGN_MAX, so the two TX tests check different
things.

j1939_tests.h already has j1939_canid(), documented as "Encode a J1939
single frame the way the kernel does on transmit", and send_raw8() uses
it on the RX side. Would a direct comparison like this cover every
field in both tests?

    EXPECT_EQ(frame.can_id, j1939_canid(PRIO, pgn, da, SA_LOCAL));

> +	ASSERT_EQ(frame.can_dlc, sizeof(payload));
> +	EXPECT_EQ(memcmp(frame.data, payload, sizeof(payload)), 0);
> +}

[ ... ]

> +/* J1939 TX -> CAN_RAW: broadcast (PDU2) single frame. */
> +TEST_F(j1939_wire, tx_pdu2_to_raw)
> +{
> +	uint8_t payload[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
> +	struct can_frame frame;
> +	canid_t id;
> +
> +	ASSERT_EQ(tx_to_raw_send_recv(self, true, PGN_PDU2, J1939_NO_ADDR,
> +				      payload, &frame), 0);
> +	id = frame.can_id & CAN_EFF_MASK;
> +
> +	EXPECT_EQ(J1939_ID_PRIO(id), PRIO);
> +	EXPECT_EQ(J1939_ID_SA(id), SA_LOCAL);			/* source address */
> +	EXPECT_EQ((id >> 8) & J1939_PGN_MAX, PGN_PDU2);	/* full broadcast PGN */

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 6/8] selftests: can: add J1939 TP/ETP multi-frame tests against CAN_RAW

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Neither transmit test checks that the kernel completed the TX
  session after the peer's EOMA.
- [Low] The TP/ETP transmit tests accept DT frames shorter than 8 bytes,
  which the kernel's own transport receiver rejects.
- [Low] Both receive tests would accept an oversized reassembled datagram
  as a correct round-trip.
- [Low] The transmit tests would pass an out-of-order TP/ETP data stream,
  because unpack_dt_into() reorders the packets before the payload is…

--

selftests: can: add J1939 TP/ETP multi-frame tests against CAN_RAW

This commit adds a kselftest that pairs a J1939 socket with a CAN_RAW
peer and runs TP and ETP multi-frame transfers in both directions. The
raw peer answers the kernel's RTS/CTS/DPO sequencing, and the reassembled
payload is compared against the sent pattern. The test also checks the
size fields of the EOMA the kernel sends on receive and of the RTS it
sends on transmit.

> diff --git a/tools/testing/selftests/net/can/test_j1939_multi.c b/tools/testing/selftests/net/can/test_j1939_multi.c
> new file mode 100644
> index 000000000000..1f27ee58f48c
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/test_j1939_multi.c
> @@ -0,0 +1,453 @@

[ ... ]

> +/*
> + * Scatter a received DT frame's payload into buf at the offset implied by its
> + * sequence number. base_pkt is the 0-based absolute packet the 1-based seq is
> + * relative to (0 for TP, the DPO offset for ETP).
> + */
> +static void unpack_dt_into(uint8_t *buf, size_t total, unsigned int base_pkt,
> +			   const struct can_frame *frame)
> +{
> +	unsigned int abs_pkt = base_pkt + (frame->data[0] - 1);
> +	size_t off = (size_t)abs_pkt * 7;

[Severity: Low]
unpack_dt_into() places each payload at the offset given by the frame's
own sequence byte. Would the TX tests still pass if the kernel sent the
DT frames out of order?

In tp_tx_to_raw(), the loop increments got for every TP_DAT frame. It
never checks that frame.data[0] == got + 1. The inner loop in
etp_tx_to_raw() also never checks that the block-relative sequence
equals i + 1.

A permuted stream such as 2, 1, 3, ... with matching payloads would
still rebuild the expected rx_buf, and the memcmp() would pass.

The kernel's own receiver only advances on in-order packets, in
j1939_xtp_rx_dat_one():

    if (packet == session->pkt.rx)
        session->pkt.rx++;

So a conforming receiver would stall or abort on such a stream. Could
both TX loops assert the expected sequence number?

[ ... ]

> +static int tp_rx_roundtrip(FIXTURE_DATA(j1939_multi) *self,
> +			   unsigned int max_cts_pkts, int *max_want_out,
> +			   struct can_frame *eoma_out)
> +{
> +	uint8_t payload[TP_SIZE], rx_buf[TP_SIZE];

[ ... ]

> +	if (recv_timeout(self->j1939, rx_buf, sizeof(rx_buf),
> +			 J1939_RECV_TIMEOUT_MS) != (int)sizeof(rx_buf))
> +		return -1;
> +	if (memcmp(payload, rx_buf, sizeof(payload)) != 0)
> +		return -1;

[Severity: Low]
Can this length check catch a delivered datagram that is larger than
TP_SIZE?

recv_timeout() calls plain recv() into a buffer of exactly
sizeof(rx_buf). j1939_sk_recvmsg() reports an oversized datagram only
through msg_flags, and it returns the number of bytes copied:

net/can/j1939/socket.c:j1939_sk_recvmsg() {
    ...
	if (size < skb->len)
		msg->msg_flags |= MSG_TRUNC;
	else
		size = skb->len;
    ...
}

Suppose a reassembly regression delivered 203 bytes instead of 200
(pkt.total * 7 rounding plus trailing 0xff padding). recv() would still
return 200, and the memcmp() would pass.

The same gap exists in etp_rx_from_raw(), where the recv_timeout() of
ETP_SIZE bytes would not notice 6006 bytes delivered instead of 6000.

The commit message says "the reassembled payload is checked byte for
byte", but the length of the datagram that reaches the socket is never
checked. Would it help to receive into a larger buffer, or to use
recvmsg() and check for MSG_TRUNC?

[ ... ]

> +TEST_F(j1939_multi, tp_tx_to_raw)
> +{

[ ... ]

> +	while (got < num_pkts) {
> +		ASSERT_EQ(recv_for_peer(self->raw, J1939_PF(TP_DAT), SA_PEER, &frame,
> +					J1939_RECV_TIMEOUT_MS), 0)
> +			TH_LOG("missing DT after %u/%u packets", got, num_pkts);
> +		unpack_dt_into(rx_buf, TP_SIZE, 0, &frame);
> +		got++;
> +	}

[Severity: Low]
Should this loop also check that frame.can_dlc == 8 for each DT frame?

match_peer_dest() in j1939_tests.h, used by recv_for_peer(), only
compares the PF and PS fields of the CAN ID. unpack_dt_into() then
copies at most the bytes still needed.

The kernel's own receiver rejects short DT frames in
j1939_xtp_rx_dat_one():

    if (skb->len != 8) {
        /* makes no sense */
        abort = J1939_XTP_ABORT_UNEXPECTED_DATA;

On transmit, full-length DT frames depend on the padding in
j1939_tp_tx_dat():

    if (j1939_tp_padding && len < 8)
        memset(skb_put(skb, 8 - len), 0xff, 8 - len);

If that padding regressed, the last DT frame would go out short. That is
5 bytes for packet 29 with TP_SIZE, and 2 bytes for packet 858 with
ETP_SIZE. rx_buf would still match, so both tp_tx_to_raw() and
etp_tx_to_raw() would pass.

> +
> +	/* acknowledge with EOMA */
> +	ctl_dat_init(dat, TP_CMD_EOMA);
> +	put_le16(&dat[1], TP_SIZE);
> +	dat[3] = num_pkts;
> +	ASSERT_EQ(send_ctl_frame(self->raw, PRIO, TP_CTL, SA_LOCAL, SA_PEER,
> +				 dat, PGN_DATA), 0);
> +
> +	EXPECT_EQ(memcmp(payload, rx_buf, sizeof(payload)), 0);
> +}

[Severity: Low]
After the EOMA is sent, the test only compares rx_buf, and rx_buf was
filled from DT frames received before the EOMA went out. Does anything
confirm that the kernel actually accepted the acknowledgement?

The TX session is only acked and completed in j1939_xtp_rx_eoma_one():

    session->pkt.tx_acked = session->pkt.total;
    j1939_session_timers_cancel(session);
    /* transmitted without problems */
    j1939_session_completed(session);

Several failures would still let tp_tx_to_raw() pass:

  - a bad-PGN match in j1939_xtp_rx_cmd_bad_pgn()
  - a failed session lookup
  - a T3 timeout abort after the last DT

etp_tx_to_raw() has the same gap: it ends right after sending the ETP
EOMA. In both cases, teardown closes the sockets before any abort or
completion can be observed.

The commit message says the tests drive the round-trips "in both
directions", but the last step on the TX side is never verified. Could
the tests wait for SCM_TSTAMP_ACK on the j1939 socket, or check that no
TP.Abort follows on the raw socket?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de

Re: [PATCH can-next v1 7/8] selftests: can: cover J1939 transport abort and timeout paths

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The file header comment in
  tools/testing/selftests/net/can/test_j1939_multi.c (line 14) says the…
- [Low] tp_tx_rts_cts_timeout and tp_rx_timeout in
  tools/testing/selftests/net/can/test_j1939_multi.c time the kernel's…

--

selftests: can: cover J1939 transport abort and timeout paths

This commit adds TP-only edge-case tests to the J1939 multi-frame
selftests. One test covers a per-CTS block limit, two cover the RTS/CTS
and rx timeouts that should end a stalled session with a Connection
Abort, and one covers a received Connection Abort cancelling an incoming
session.

> diff --git a/tools/testing/selftests/net/can/test_j1939_multi.c b/tools/testing/selftests/net/can/test_j1939_multi.c
> index 1f27ee58f48c..069a9c8690b5 100644
> --- a/tools/testing/selftests/net/can/test_j1939_multi.c
> +++ b/tools/testing/selftests/net/can/test_j1939_multi.c
> @@ -5,7 +5,18 @@

[ ... ]

> + * Timing limitation: the abort/timeout tests wait with wide receive windows
> + * rather than tight bounds, because timing over vcan under QEMU is not tightly
> + * bounded. They catch a gross timer regression (e.g. a doubled timeout) but not
> + * a subtle shift; see the per-test comments.

[Severity: Low]
Does tp_rx_timeout() really catch a doubled timeout, as this comment
says? Its receive window is:

#define TP_RX_ABORT_WAIT_MS	(TP_T2_MS * 2)	/* generous; floor check is the real test */

That is 2500 ms, which is exactly a doubled T2.

The window also starts late. tp_rx_timeout() takes t0 only after
tp_rx_start_session() has dequeued the CTS. The kernel arms the rx timer
earlier, when it sends the CTS:

net/can/j1939/transport.c:j1939_session_tx_cts() {
    ...
	ret = j1939_tp_tx_ctl(session, true, dat);
	if (ret < 0)
		return ret;
    ...
	j1939_tp_set_rxtimeout(session, 1250);
    ...
}

Suppose T2 regressed from 1250 to 2500 ms. The abort would go out at
about T_cts + 2500 ms. The recv_ctl_cmd() deadline is t0 + 2500 ms, and
t0 is later than T_cts. So the abort would arrive inside the window or
right at its edge, and the test would usually pass. Any regression below
2x (for example 1250 -> 2400 ms) would always pass.

The comment on tp_rx_timeout() itself ("the upper bound is a generous T2
multiple, so timer shifts within it escape") also contradicts the header.
Only tp_tx_rts_cts_timeout(), with its 2000 ms window, matches the
header's claim. The last patch in this series doesn't change this file.

Could the header claim be limited to the TX test? Alternatively, the RX
window could be set below 2 * T2, for example 2000 ms as in the TX test.

> + *
> + * Coverage limitation: a received Connection Abort is exercised RX-side only;

[ ... ]

> @@ -313,6 +332,69 @@ TEST_F(j1939_multi, tp_tx_to_raw)

[ ... ]

> +TEST_F(j1939_multi, tp_tx_rts_cts_timeout)
> +{
> +	uint8_t payload[TP_SIZE];
> +	struct timespec t0, t1;
> +	struct can_frame frame;
> +	long elapsed_ms;
> +
> +	fill_pattern(payload, sizeof(payload));
> +
> +	ASSERT_EQ(tx_start_session(&self->j1939, self->ifindex, payload,
> +				   sizeof(payload), self->raw, J1939_PF(TP_CTL),
> +				   TP_CMD_RTS, &frame), 0);
> +
> +	/* Deliberately send no CTS; expect the kernel to abort within 2000 ms. */
> +	clock_gettime(CLOCK_MONOTONIC, &t0);
> +	ASSERT_EQ(recv_ctl_cmd(self->raw, J1939_PF(TP_CTL), SA_PEER, &frame,
> +			       2000, TP_CMD_ABORT), 0)
> +		TH_LOG("kernel did not send a Connection Abort within 2000 ms");
> +	clock_gettime(CLOCK_MONOTONIC, &t1);
> +	elapsed_ms = timespec_diff_ms(&t0, &t1);
> +	/* must not fire instantly: the T3 timer has to run before the abort */
> +	ASSERT_GE(elapsed_ms, 800)
> +		TH_LOG("abort fired after only %ld ms, timer did not run", elapsed_ms);

[Severity: Low]
Is t0 measured from the right reference point here? The kernel arms T3
when it transmits the RTS, not when userspace reads it:

net/can/j1939/transport.c:j1939_session_tx_rts() {
    ...
	ret = j1939_tp_tx_ctl(session, false, dat);
	if (ret < 0)
		return ret;
    ...
	} else {
		j1939_tp_set_rxtimeout(session, 1250);
	}
    ...
}

tx_start_session() has already dequeued the RTS before
clock_gettime(&t0) runs. So elapsed_ms is 1250 ms minus the delay
between the kernel transmit and t0.

If the test task stalls for about 450 ms or more before t0, the abort
from a correct kernel arrives in under 800 ms. ASSERT_GE(elapsed_ms, 800)
then fails even though the kernel is fine. If the stall is over 500 ms, a
doubled T3 (2500 ms) would also fit inside the 2000 ms window. The
comment above this test says that case should be caught.

tp_rx_timeout() has the same pattern. j1939_session_tx_cts() arms the
1250 ms rx timer right after it transmits the CTS. t0 is taken only after
tp_rx_start_session() has consumed that CTS, and the same 800 ms floor
applies.

The header comment says vcan timing under QEMU is not tightly bounded,
and that is where stalls like this are most likely. Would kernel socket
timestamps on the RTS/CTS frame and on the abort give a more reliable
measure of the timer interval? The last patch in the series doesn't
change this.

> +	/* the T3 expiry must be reported as a timeout abort */
> +	EXPECT_EQ(frame.data[1], ABORT_REASON_TIMEOUT);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014154435.3577983-1-o.rempel%40pengutronix.de