| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMZ6RqLLchj4oJUJEzObBpK4_dDHooo0ZHNz26tH33DQh7Ugag@mail.gmail.com>
Date: Thu, 24 Apr 2025 16:42:59 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Felix Maurer <fmaurer@...hat.com>
Cc: socketcan@...tkopp.net, mkl@...gutronix.de, shuah@...nel.org,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
horms@...nel.org, linux-can@...r.kernel.org, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, dcaratti@...hat.com, fstornio@...hat.com
Subject: Re: [PATCH 2/4] selftests: can: use kselftest harness in test_raw_filter
On Tue. 22 Apr. 2025 at 21:04, Felix Maurer <fmaurer@...hat.com> wrote:
> Update test_raw_filter to use the kselftest harness to make the test output
> conform with TAP. Use the logging and assertations from the harness.
^^^^^^^^^^^^
assertions?
>
> Signed-off-by: Felix Maurer <fmaurer@...hat.com>
> ---
> tools/testing/selftests/net/can/Makefile | 2 +
> .../selftests/net/can/test_raw_filter.c | 152 ++++++++----------
> 2 files changed, 73 insertions(+), 81 deletions(-)
>
> diff --git a/tools/testing/selftests/net/can/Makefile b/tools/testing/selftests/net/can/Makefile
> index 44ef37f064ad..5b82e60a03e7 100644
> --- a/tools/testing/selftests/net/can/Makefile
> +++ b/tools/testing/selftests/net/can/Makefile
> @@ -2,6 +2,8 @@
>
> top_srcdir = ../../../../..
>
> +CFLAGS += -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INCLUDES)
> +
> TEST_PROGS := test_raw_filter.sh
>
> TEST_GEN_FILES := test_raw_filter
> diff --git a/tools/testing/selftests/net/can/test_raw_filter.c b/tools/testing/selftests/net/can/test_raw_filter.c
> index c84260f36565..7414b9aef823 100644
> --- a/tools/testing/selftests/net/can/test_raw_filter.c
> +++ b/tools/testing/selftests/net/can/test_raw_filter.c
> @@ -18,6 +18,8 @@
> #include <linux/can.h>
> #include <linux/can/raw.h>
>
> +#include "../../kselftest_harness.h"
> +
> #define ID 0x123
> #define TC 18 /* # of testcases */
>
> @@ -53,7 +55,38 @@ canid_t calc_mask(int testcase)
> return mask;
> }
>
> -int main(int argc, char **argv)
> +int send_can_frames(int sock, int testcase)
> +{
> + struct can_frame frame;
> +
> + frame.can_dlc = 1;
> + frame.data[0] = testcase;
> +
> + frame.can_id = ID;
> + if (write(sock, &frame, sizeof(frame)) < 0) {
> + perror("write");
> + return 1;
> + }
> + frame.can_id = (ID | CAN_RTR_FLAG);
> + if (write(sock, &frame, sizeof(frame)) < 0) {
> + perror("write");
> + return 1;
> + }
> + frame.can_id = (ID | CAN_EFF_FLAG);
> + if (write(sock, &frame, sizeof(frame)) < 0) {
> + perror("write");
> + return 1;
> + }
> + frame.can_id = (ID | CAN_EFF_FLAG | CAN_RTR_FLAG);
> + if (write(sock, &frame, sizeof(frame)) < 0) {
> + perror("write");
> + return 1;
> + }
> +
> + return 0;
Nitpick: can you centralize the error handling under a goto label?
write_err:
perror("write");
return 1;
> +}
> +
> +TEST(can_filter)
> {
> fd_set rdfs;
> struct timeval tv;
> @@ -67,34 +100,26 @@ int main(int argc, char **argv)
> int rxbits, rxbitval;
> int ret;
> int recv_own_msgs = 1;
> - int err = 0;
> struct ifreq ifr;
>
> - if ((s = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) {
> - perror("socket");
> - err = 1;
> - goto out;
> - }
> + s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
> + ASSERT_LT(0, s)
> + TH_LOG("failed to create CAN_RAW socket (%d)", errno);
>
> strcpy(ifr.ifr_name, VCANIF);
> - if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
> - perror("SIOCGIFINDEX");
> - err = 1;
> - goto out_socket;
> - }
> + ret = ioctl(s, SIOCGIFINDEX, &ifr);
> + ASSERT_LE(0, ret)
> + TH_LOG("failed SIOCGIFINDEX (%d)", errno);
> +
> addr.can_family = AF_CAN;
> addr.can_ifindex = ifr.ifr_ifindex;
>
> setsockopt(s, SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS,
> &recv_own_msgs, sizeof(recv_own_msgs));
>
> - if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
> - perror("bind");
> - err = 1;
> - goto out_socket;
> - }
> -
> - printf("---\n");
> + ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
> + ASSERT_EQ(0, ret)
> + TH_LOG("failed bind socket (%d)", errno);
>
> for (testcase = 0; testcase < TC; testcase++) {
>
> @@ -103,36 +128,14 @@ int main(int argc, char **argv)
> setsockopt(s, SOL_CAN_RAW, CAN_RAW_FILTER,
> &rfilter, sizeof(rfilter));
>
> - printf("testcase %2d filters : can_id = 0x%08X can_mask = 0x%08X\n",
> + TH_LOG("testcase %2d filters : can_id = 0x%08X can_mask = 0x%08X",
> testcase, rfilter.can_id, rfilter.can_mask);
>
> - printf("testcase %2d sending patterns ... ", testcase);
> -
> - frame.can_dlc = 1;
> - frame.data[0] = testcase;
> -
> - frame.can_id = ID;
> - if (write(s, &frame, sizeof(frame)) < 0) {
> - perror("write");
> - exit(1);
> - }
> - frame.can_id = (ID | CAN_RTR_FLAG);
> - if (write(s, &frame, sizeof(frame)) < 0) {
> - perror("write");
> - exit(1);
> - }
> - frame.can_id = (ID | CAN_EFF_FLAG);
> - if (write(s, &frame, sizeof(frame)) < 0) {
> - perror("write");
> - exit(1);
> - }
> - frame.can_id = (ID | CAN_EFF_FLAG | CAN_RTR_FLAG);
> - if (write(s, &frame, sizeof(frame)) < 0) {
> - perror("write");
> - exit(1);
> - }
> + TH_LOG("testcase %2d sending patterns...", testcase);
>
> - printf("ok\n");
> + ret = send_can_frames(s, testcase);
> + ASSERT_EQ(0, ret)
> + TH_LOG("failed to send CAN frames");
>
> have_rx = 1;
> rx = 0;
> @@ -147,58 +150,45 @@ int main(int argc, char **argv)
> tv.tv_usec = 50000; /* 50ms timeout */
>
> ret = select(s+1, &rdfs, NULL, NULL, &tv);
> - if (ret < 0) {
> - perror("select");
> - exit(1);
> - }
> + ASSERT_LE(0, ret)
> + TH_LOG("failed select for frame %d (%d)", rx, errno);
^^^^
Nitpick: the Linux coding style suggests not printing numbers in
parentheses (%d).
https://www.kernel.org/doc/html/latest/process/coding-style.html#printing-kernel-messages
Suggestion:
TH_LOG("failed select for frame %d, err: %d", rx, errno);
> if (FD_ISSET(s, &rdfs)) {
> have_rx = 1;
> ret = read(s, &frame, sizeof(struct can_frame));
> - if (ret < 0) {
> - perror("read");
> - exit(1);
> - }
> - if ((frame.can_id & CAN_SFF_MASK) != ID) {
> - fprintf(stderr, "received wrong can_id!\n");
> - exit(1);
> - }
> - if (frame.data[0] != testcase) {
> - fprintf(stderr, "received wrong testcase!\n");
> - exit(1);
> - }
> + ASSERT_LE(0, ret)
> + TH_LOG("failed to read frame %d (%d)", rx, errno);
> +
> + ASSERT_EQ(ID, frame.can_id & CAN_SFF_MASK)
> + TH_LOG("received wrong can_id");
> + ASSERT_EQ(testcase, frame.data[0])
> + TH_LOG("received wrong test case");
>
> /* test & calc rxbits */
> rxbitval = 1 << ((frame.can_id & (CAN_EFF_FLAG|CAN_RTR_FLAG|CAN_ERR_FLAG)) >> 28);
>
> /* only receive a rxbitval once */
> - if ((rxbits & rxbitval) == rxbitval) {
> - fprintf(stderr, "received rxbitval %d twice!\n", rxbitval);
> - exit(1);
> - }
> + ASSERT_NE(rxbitval, rxbits & rxbitval)
> + TH_LOG("received rxbitval %d twice", rxbitval);
> rxbits |= rxbitval;
> rx++;
>
> - printf("testcase %2d rx : can_id = 0x%08X rx = %d rxbits = %d\n",
> + TH_LOG("testcase %2d rx : can_id = 0x%08X rx = %d rxbits = %d",
> testcase, frame.can_id, rx, rxbits);
> }
> }
> /* rx timed out -> check the received results */
> - if (rx_res[testcase] != rx) {
> - fprintf(stderr, "wrong rx value in testcase %d : %d (expected %d)\n",
> - testcase, rx, rx_res[testcase]);
> - exit(1);
> - }
> - if (rxbits_res[testcase] != rxbits) {
> - fprintf(stderr, "wrong rxbits value in testcase %d : %d (expected %d)\n",
> - testcase, rxbits, rxbits_res[testcase]);
> - exit(1);
> - }
> - printf("testcase %2d ok\n---\n", testcase);
> + ASSERT_EQ(rx_res[testcase], rx)
> + TH_LOG("wrong number of received frames %d", testcase);
> + ASSERT_EQ(rxbits_res[testcase], rxbits)
> + TH_LOG("wrong rxbits value in testcase %d", testcase);
> +
> + TH_LOG("testcase %2d ok", testcase);
> + TH_LOG("---");
> }
>
> -out_socket:
> close(s);
> -out:
> - return err;
> + return;
> }
> +
> +TEST_HARNESS_MAIN
Yours sincerely,
Vincent Mailhol
Powered by blists - more mailing lists