Re: [PATCH 2/4] selftests: can: use kselftest harness in test_raw_filter

On Tue. 22 Apr. 2025 at 21:04, Felix Maurer fmaurer@redhat.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@redhat.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-ke...

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