Internal loopback feature (Linux IFF_ECHO) should be made configurable (related to #56) about dev-can-linux

[Deniz-Eren]

@phicore can you please post code for functions fctCmdRxD and fctSendCmdTxD also?

[Deniz-Eren]

@phicore one initial comment; when you send message on /dev/can3/tx0 that same message is echoed to /dev/can3/rx0.

Could this be causing the issue reported?

[phicore]

@phicore can you please post code for functions fctCmdRxD and fctSendCmdTxD also?

Sure, here they are.

CAN_DEF_ERREUR fctCmdRxD (int fd, trameCan_t * Trame, unsigned int TimeOut)
{
	// Check whether fd is in bounds and device entry is marked open?
	if (fd >= 0 && fd < MAX_CAN_DEVICES && canDevices[fd].open)
	{
		CAN_DEF_ERREUR     status = CAN_NODEFINE;
		uint64_t           nTime;
		struct sigevent    event;
		struct can_msg     canmsg;
		int                error;

		nTime = (uint64_t)TimeOut * (uint64_t)1000000;
		event.sigev_notify = SIGEV_UNBLOCK ;
		TimerTimeout(CLOCK_MONOTONIC, _NTO_TIMEOUT_SEND | _NTO_TIMEOUT_REPLY, &event, &nTime, NULL) ;

		memset(&canmsg, 0, sizeof(struct can_msg));
		error = read_frame_raw_block(canDevices[fd].rx, &canmsg);

		// Convert to trame_can_t structure
		// if data in low 12 bits copy as is
		if ( (canmsg.mid & 0x00000FFF) !=0)
			Trame->idCan = (canmsg.mid & 0x000007FF);

		// if in upper area then copy to low word
		if ( (canmsg.mid & 0x0FFF0000) !=0)
			Trame->idCan = ((canmsg.mid >>18) & 0x7FF);

		Trame->len = canmsg.len;
		memcpy(Trame->tabCan,canmsg.dat,8); 

		if( ( error != EOK ) && ( error != EINTR ) )
		{ 
			fprintf(stderr, "Err fctCmdRxD, (read_frame_raw_block) code: %d %s\n",error, strerror(error));
		} 
		// Test timeout and `error` value on timeout.
		else if ( ( error == EINTR ) ||  ( error == ETIME ))
		{
			status = CAN_TIMEOUT;
		}
		else
		{
			status = CAN_NOERR;
		}
		return(status);
	}
	else
		return (-1);
}

int fctSendCmdTxD(int fd, trameCan_t * Trame)
{
	// Check whether fd is in bounds and device entry is marked open?
	if (fd >= 0 && fd < MAX_CAN_DEVICES && canDevices[fd].open)
		{
			struct can_msg canmsg;
			memset(&canmsg, 0, sizeof(struct can_msg));

			canmsg.len = Trame->len;

			//printf("fd: %d deviceindex: %d", fd,canDevices[fd].dev_id);
			
#if 0    // Needed for 1.0.26 dev-can-linux driver			
			if(canDevices[fd].dev_id == PEAK_CANINDEX)
			{
				// PEAK driver does not respect QNX mapping
				canmsg.mid = Trame->idCan;
			}
			else
#endif			
			{
				// QNX needs standard MID in bits 18 to 29
				canmsg.mid = (Trame->idCan) << 18;
			}			
			canmsg.ext.is_extended_mid = 0;
			memcpy(&canmsg.dat, Trame->tabCan, 8);
			return write_frame_raw(canDevices[fd].tx, &canmsg);
		}
	else
		return (-1);
}

[phicore]

Sorry didn't meant to close

[phicore]

@phicore one initial comment; when you send message on /dev/can3/tx0 that same message is echoed to /dev/can3/rx0.

Could this be causing the issue reported?

The goal of the can-loop program is to answer back the same message with an incremented MID. Is this what you meant ?

The issue here is that for one received message we send out several answers answer instead of just one. The same code on the 3 other integrated CAN interfaces behaves correctly. To solve the issue we found an ugly fix consisting to close and reopen the device for each message. This possibly leads to the other issue we list in #56 in which we observe that by doing so we are limited to sending around 65K messages and then the driver does not seem to handle new messages.

[Deniz-Eren]

The goal of the can-loop program is to answer back the same message with an incremented MID. Is this what you meant ?

I was just letting you know that the current implementation is done such that any messages you send on /dev/can3/tx0 will be received on /dev/can3/rx0, it's an auto echo back feature that exists with Linux implementations and I've repeated it in this QNX implementation. I was wondering if this in-built feature is the cause of your issue report here? If that's the case, then I can make the echo back feature an optional feature somehow.

[phicore]

Interesting, if you can point me to the source file location were I could test to remove it it will be a good test. Thanks a lot

[Deniz-Eren]

That's a good idea. If you do this experiment and see that it fixes the issue, then I can implement more options for users to turn off the echo back feature.

To do the experiment you need to add a small bit of code to function netif_rx; inside the netif_rx function at line161 of netif.c just add the following:

    if (skb->is_echo) {
        return NET_RX_SUCCESS;
    }

Let me know if this fixes your issues so that I can make a permanent feature.

[phicore]

We tried your solution, it works perfectly. Thanks a lot.

[Deniz-Eren]

Excellent to hear.

Let's leave this ticket open until I implement this feature as a configurable option.

[Deniz-Eren]

@phicore Actually a question for you; would the default behaviour for QNX CAN-bus drivers be the non-echo behaviour?

It seems in Linux the default is to echo. When I implement this configuration, I'm thinking of making the default non-echo based on your experiences with other QNX CAN-bus drivers; let me know what you think.

[phicore]

@phicore Actually a question for you; would the default behaviour for QNX CAN-bus drivers be the non-echo behaviour?

I only ever used QNX on the iMX8QM platform, and the CAN interface is used in RAW mode only, but yes, I suppose it is non-echo by default on QNX.

That being said, internal loop-back exists in the devctl_info structure:
https://www.qnx.com/developers/docs/7.1/#com.qnx.doc.neutrino.devctl/topic/can/can_devctl_get_info.html)

[Deniz-Eren]

Yes I will update those loop back info messages accordingly also.