hartkopp/can-isotp

blocking read() function using ISO-TP driver !

Closed this issue · 7 comments

Hello !

I used to make an application that sends a lot of CAN frames (encapsulating UDS requests) with the write() function using the ISO-TP driver through a peak-can device to an ECU that responds with the appropriate CAN frame/frames and i should get those responses each time i send a request using the read() function...the sending/receiving of requests/responses is done inside a while() loop, i extract the requests from a text file which has a structure like:
SID1 DID11 DID21
SID1 DID12 DID22
SID1 DID13 DID23
.....
SID2 DID11 DID21
SID2 DID12 DID22
SID2 DID13 DID23
....
then when getting the responses i write them in an xml file, the problem when i launch the app, it starts correctly and makes the necessary send/receive until it blocks after a while, (it can blocks in any part of the whole number of requests, it can be just in the beginning (after sending/receiving 6to9 requests/responses) , in the middle or just before finishing, there is no particular position), this picture show one of those blocking scenarios :
image

the red lines corresponds to the requests and the blue ones are the responses,

i fetched the code and i found that the read() function is causing the block situation, normally if the ECU didn't response in a certain timeout the read() must not stay waiting and blocking the whole program no ?

i have the an Ubuntu 14.04, i used the isotp module to handle the socketCAN just the way like the isotp utilies (isotpdump, isotprecv, ..)

does the ISO-TP driver handle any sort of timeout when getting stacked in a such situation, knowing that, when i open a terminal in parallel when the app get blocked and send a random request using isotpsend, the app gets out of the blocking situation and continue to send but then it blocks again !

Thank you very much !

Hi,
in general the blocking read behaviour is intended to wait for incoming PDUs.
Are you only using isotp[send|recv] with some scripting or did you write your own C-code for your program?
Did you increase the tx queue length of can0?

Hi @hartkopp ,

i made an app that contains the same configuration of the ISO-TP socket, the same like in the isotp[recv|send] :

  int s;
    struct sockaddr_can addr;
    static struct can_isotp_options opts;
    static struct can_isotp_ll_options llopts;
     if ((s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP)) < 0) {
	                perror("socket");
	                   exit(1);
           }
           static struct can_isotp_fc_options fcopts;
           setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fcopts, sizeof(fcopts));
           setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts));
           if (llopts.tx_dl) {
	   if (setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &llopts, sizeof(llopts)) < 0) {
	    perror("link layer sockopt");
	    exit(1);
	    }
        }
           addr.can_family = AF_CAN;
            addr.can_ifindex = if_nametoindex(can_interface);
            if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
	    perror("bind");
	    close(s);
	    exit(1);
            }

then i used the write() and the read() as mentioned before...simultaneously:

retval = write(s, buf, buflen);
                               if (retval < 0) {
                               perror("write");
                               return retval;
                               }

                               if (retval != buflen){
                                fprintf(stderr, "wrote only %d from %d byte\n", retval, buflen);
                                    

                               }
                         
                               printf("start reading ....\n");
                           nbytes = read(s, msg, BUFSIZE);
                               printf("End reading....\n");

Concerning the tx queue length, i didn't change it at first:
image

until you mentioned this, i just set the tx queue length with the ip link set txqueuelen 1000 dev can0 :
image

but i got the same behavior ...the read() continue to block ...

does setting up the isotp socket like that way (as mentioned above) enable automatically any timeout features .... i was thinking about multi-threading the program and whenever the read thread is blocked i send a signal to interrupt it ...but i'm afraid that it could not be a safe interruption ...

static struct can_isotp_options opts;
static struct can_isotp_ll_options llopts;
if ((s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP)) < 0) {
              perror("socket");
                  exit(1);
        }
         static struct can_isotp_fc_options fcopts;
          setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fcopts, sizeof(fcopts));
          setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts));

Why do you set the sockopts ???
What values did you configure?

@hartkopp i didn't configure any values ..i just thought that declaring the struct can_isotp_options & static struct can_isotp_ll_options & static struct can_isotp_fc_options and set them (even with its default values) is essential to make the ISO-TP socket work correctly....but i removed them and i tried again launching the app and got the same behaviour ...the read() still blocking ... any suggestions ?

When you perform the two setsockopt()'s you are OVERWRITING the reasonable default values with zeros. This very probable not what you want!!

The read() syscall is intended to block. The caller usually (blocking-)waits for content and then prepares the answer.
You can search for select() or epoll if you want to handle different file descriptors in your program (recommended).
Or you can switch the socket to a non-blocking socket with fcntl(socket_fd, F_SETFL, flags | O_NONBLOCK), see:
https://jameshfisher.com/2017/04/05/set_socket_nonblocking/
But this finally results in a polling loop - therefore better check out for select() examples.

@hartkopp thank you very much !

i'll check all these propositions ....BTW i tried to set time-out for read() using this :

struct timeval tv;
    tv.tv_sec = 0;
    tv.tv_usec = 10000; 
  if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, (const char*)&tv,sizeof(tv)) < 0) {
        	perror("Setting timeout failed");

            }

and yes it helps to stop the blocking, if the timeout (in my case 10ms) is reached and the read() is still blocking then it will return -1 otherwise if there are some data...it will be managed (read() returns >0) ...i'll try to solve the problem by resending the CAN frames whenever i got -1 from read() ..could it be safe that way ?

I would still suggest select() or epoll. The SO_RCVTIMEO seems to alleviate your pain but does not really fit to your use-case ...