hartkopp/can-isotp

use read() function and add notify function.

Closed this issue · 18 comments

hi,
first, I have a question, what if I use read() and Incoming len parameter more than upcoming cantp message length, weather sure to be able to guarantee read() function recevie the whole cantp message?

then, when develop uds with can-isotp base linux , P2client's definition is shown in the figure below. But when I read can-isotp source code, i cannot find can-isotp support user get this event. This is caused by my ignorance or not implement it. Is it possible to add this if not implemented? Because I don't understand linux kernel development, even if I have ideas(register callback and i can implement it), I don't know how to do it. Similarly, can some cantp error(e.g. SF_DL error handling, FF_DL error handling and so on) be notified to the upper layer.

image
image

first, I have a question, what if I use read() and Incoming len parameter more than upcoming cantp message length, weather sure to be able to guarantee read() function recevie the whole cantp message?

The PDU message length is properly defined on the communication layer and you only get the PDU from the Linux ISOTP socket if it was received completely. Of course you need to provide a buffer in userspace that is able to store the entire PDU.

See https://github.com/linux-can/can-utils/blob/master/isotprecv.c#L245

then, when develop uds with can-isotp base linux , P2client's definition is shown in the figure below. But when I read can-isotp source code, i cannot find can-isotp support user get this event. This is caused by my ignorance or not implement it. Is it possible to add this if not implemented? Because I don't understand linux kernel development, even if I have ideas(register callback and i can implement it), I don't know how to do it. Similarly, can some cantp error(e.g. SF_DL error handling, FF_DL error handling and so on) be notified to the upper layer.

As written above you get the complete isotp PDU when completely received by the system.

In the case the PDU reception runs into a timeout (e.g. no CFs after a FF within a specific time) you get a timeout notification on read() in the errno number.

There is no extra notification for the FF reception to the userspace. The timeout is monitored inside the Linux kernel.

You should play with the isotp[send|recv|sniffer|dump] tools of the can-utils to get an impression how the Linux kernel implementation works.

hahah~~tThank you for replying so quickly.My English is so poor, I am really ashamed after seeing your reply.

The PDU message length is properly defined on the communication layer and you only get the PDU from the Linux ISOTP socket if it was received completely. Of course you need to provide a buffer in userspace that is able to store the entire PDU.
See https://github.com/linux-can/can-utils/blob/master/isotprecv.c#L245

The link provided is particularly helpful for me, as you said below, I got a lot of other uses from can-utils.After receiving your reply, I went to read the source code of can-isotp again. Due to my carelessness, I didn't see a datagram before,ahaha~. As shown below.
image

In the case the PDU reception runs into a timeout (e.g. no CFs after a FF within a specific time) you get a timeout notification on read() in the errno number.
There is no extra notification for the FF reception to the userspace. The timeout is monitored inside the Linux kernel.

Well, since you haven't implemented these notification mechanisms, I plan to use the kernel space and user space communication methods to achieve the notification effect. I need to evaluate the specific method, for example. Do you think this is a good way?I need to evaluate the specific method, e.g. netlink or use file with select. Do you think this is a good way?
image

hahah~~tThank you for replying so quickly.My English is so poor, I am really ashamed after seeing your reply.

Oh, not really. I recently bought something on Ebay from an US guy and the discussion with him completely got out of hand :-D

Well, since you haven't implemented these notification mechanisms, I plan to use the kernel space and user space communication methods to achieve the notification effect. I need to evaluate the specific method, for example. Do you think this is a good way?I need to evaluate the specific method, e.g. netlink or use file with select. Do you think this is a good way?

IMO this notification stuff is pointless and should not be implemented at all.
What is the use-case behind it? Why do you think you need that notification??

Oh, not really. I recently bought something on Ebay from an US guy and the discussion with him completely got out of hand :-D

This is so intesting.

Well, since you haven't implemented these notification mechanisms, I plan to use the kernel space and user space communication methods to achieve the notification effect. I need to evaluate the specific method, e.g. netlink or use file with select. Do you think this is a good way?

IMO this notification stuff is pointless and should not be implemented at all.
What is the use-case behind it? Why do you think you need that notification??

As show there figure, the time starting point of the P2client of uds(reference 14229-2)is the FF for cantp, Therefore, there must be a corresponding mechanism to notify the user of the event that generated the FF. Or you have other opinions? :-D:
image
image

If I can add, this notification can be usefull to manage timeout properly. With it, it is easier to deal with payloads that take long time to transmit.
If we get only a notification at the end of transmission, receiving a payload that takes 10 seconds to transmit will require a timeout bigger than 10 sec.

It's the difference between p2 and p6 timer as per iso-14229

As show there figure, the time starting point of the P2client of uds(reference 14229-2)is the FF for cantp, Therefore, there must be a corresponding mechanism to notify the user of the event that generated the FF. Or you have other opinions? :-D:

I must first correct my mistakes, the end of P2client is when the ff cantp message is received. No starting.

If I can add, this notification can be usefull to manage timeout properly. With it, it is easier to deal with payloads that take long time to transmit.
If we get only a notification at the end of transmission, receiving a payload that takes 10 seconds to transmit will require a timeout bigger than 10 sec.

Yes , I agree with you. If the message is too long, user or user app don’t know when the message will be sent.

It's the difference between p2 and p6 timer as per iso-14229.

yeah,yeah.
So do you think you need to add an event notification mechanism?

@jinbaotang , i am not @hartkopp ;)

@jinbaotang , i am not @hartkopp ;)

yeah, i see.ahaha~I want to find someone who can support me and make me more courageous.:laughing:

Hm, didn't now that this kind of timeout is really relevant in real world use-cases. ¯_(ツ)_/¯

I would take a look into three approaches:

  1. There is a mechanism for so-called out-of-band data (aka OOB) which can be provided by TCP/IP sockets (AFAIK) - maybe the notification can be send via OOB. You could take a look into that.
  2. You could use the CAN-BCM socket and set a filter on the 4-bit PCI value in can_frame.data[0] on the CAN-ID where you receive the iso-tp stream for this connection: This would lead to the reception of SF/FF/CF frames ONLY the first time you receive them on CAN. E.g. if you only receive long (segmented) PDUs you would just get single(!) FF and CF frames (e.g. FF, CF, FF, CF, ...) and you could use the FF reception time for your timeout
  3. We could think about a new flag for the isotp-flags that e.g. causes the implementation to provide an empty (len = 0) PDU at FF reception time.

Hm, didn't now that this kind of timeout is really relevant in real world use-cases. ¯_(ツ)_/¯

I would take a look into three approaches:

  1. There is a mechanism for so-called out-of-band data (aka OOB) which can be provided by TCP/IP sockets (AFAIK) - maybe the notification can be send via OOB. You could take a look into that.
  2. You could use the CAN-BCM socket and set a filter on the 4-bit PCI value in can_frame.data[0] on the CAN-ID where you receive the iso-tp stream for this connection: This would lead to the reception of SF/FF/CF frames ONLY the first time you receive them on CAN. E.g. if you only receive long (segmented) PDUs you would just get single(!) FF and CF frames (e.g. FF, CF, FF, CF, ...) and you could use the FF reception time for your timeout
  3. We could think about a new flag for the isotp-flags that e.g. causes the implementation to provide an empty (len = 0) PDU at FF reception time.

Well. From your description, it seems that the third type is more suitable, and the changes are minor. But have you ever considered that if read() returns 0, it will conflict with other situations?

  1. We could think about a new flag for the isotp-flags that e.g. causes the implementation to provide an empty (len = 0) PDU at FF reception time.

Well. From your description, it seems that the third type is more suitable, and the changes are minor. But have you ever considered that if read() returns 0, it will conflict with other situations?

AFAIK a PDU length of zero is not defined in ISO-TP. And when an error is signaled the return value is negative (and the error code can be retrieved from errno). (I hope I got this right - maybe I should doublecheck that again)

So if you enable this FF signaling with e.g. CAN_ISOTP_FF_SIGNALING and you get a length of zero on the read() call then you can be sure that this is the FF signal.

  1. We could think about a new flag for the isotp-flags that e.g. causes the implementation to provide an empty (len = 0) PDU at FF reception time.

Well. From your description, it seems that the third type is more suitable, and the changes are minor. But have you ever considered that if read() returns 0, it will conflict with other situations?

AFAIK a PDU length of zero is not defined in ISO-TP. And when an error is signaled the return value is negative (and the error code can be retrieved from errno). (I hope I got this right - maybe I should doublecheck that again)

So if you enable this FF signaling with e.g. CAN_ISOTP_FF_SIGNALING and you get a length of zero on the read() call then you can be sure that this is the FF signal.

yeah, you are right.Maybe I will implement this feature in the near future. And test. Can I apply for a pull request at that time?;)

In addition, I think some errors also need to be notified to users, e.g. SF_DL error handling, FF_DL error handling and so on. I think this is easier to implement, for example, read() returns -1, and then the user gets errno. But I don't know that **errno **of linux kernel can be defined at will.

yeah, you are right.Maybe I will implement this feature in the near future. And test. Can I apply for a pull request at that time?;)

Not really. I only maintain this repository for fixes that show up in the Linux mainline development - to support users with Linux kernel versions pre Linux 5.10

The discussions for new features and patches need to be discussed on the Linux CAN mailing list.

In addition, I think some errors also need to be notified to users, e.g. SF_DL error handling, FF_DL error handling and so on. I think this is easier to implement, for example, read() returns -1, and then the user gets errno. But I don't know that **errno **of linux kernel can be defined at will.

There are already different error notifications that use the socket error queue. But regarding a "SF_DL" error handling I'm not sure if you are asking for things where the ISO specification already decided to drop/ignore malformed PDUs.
If you want to create a 'conformance test' this can be done as a separate CAN tool, e.g. using the CAN_RAW socket, like isotpdump does it.

Not really. I only maintain this repository for fixes that show up in the Linux mainline development - to support users with Linux kernel versions pre Linux 5.10

The discussions for new features and patches need to be discussed on the Linux CAN mailing list.

Okay, I don’t know that it’s necessary to do this normally, because I don’t understand linux kernel meetings, haha~

Then I can only add a patch to the current version. If can-isotp is updated but this function is not implemented, then update the patch to a new version of can-isotp

In addition, do you have any plans to discuss this feature on the Linux CAN mailing list?

There are already different error notifications that use the socket error queue. But regarding a "SF_DL" error handling I'm not sure if you are asking for things where the ISO specification already decided to drop/ignore malformed PDUs.
If you want to create a 'conformance test' this can be done as a separate CAN tool, e.g. using the CAN_RAW socket, like isotpdump does it.

These error events do not seem to be so important to me at the moment. But I consider from the specification (15765-2), I think it is necessary to notify these error events to the upper layer.Because just like tcp/ip, some errors will be notified to the upper layer to better analyze the problem. Since I mainly develop vehicle-mounted Ethernet-related development, I have less can-based development, so I am also a guess based on experience. What do you think for this?

In addition, do you have any plans to discuss this feature on the Linux CAN mailing list?

Of course I want to maintain ONE implementation. And the lead development is now in the Linux kernel and therefore the Linux CAN mailing list is the usual discussion forum.

But I consider from the specification (15765-2), I think it is necessary to notify these error events to the upper layer.Because just like tcp/ip, some errors will be notified to the upper layer to better analyze the problem.

Please take a look into the ISO spec. IIRC malformed PDUs are just ignored. And this is turned out to be a pretty good development approach.

Of course I want to maintain ONE implementation. And the lead development is now in the Linux kernel and therefore the Linux CAN mailing list is the usual discussion forum.

Ahaha, thank you very much.

Please take a look into the ISO spec. IIRC malformed PDUs are just ignored. And this is turned out to be a pretty good development approach.

Thank you for your reminder, I will take a closer look at the ISO specification.

Of course I want to maintain ONE implementation. And the lead development is now in the Linux kernel and therefore the Linux CAN mailing list is the usual discussion forum.

image
When I further read the ISO 14229-2 specification, I found that the start time of P2client also needs to be notified by isotp.As shown in figure. So do we need to notify the event after the last frame of a cantp message is transmitted?

Not everything which is written in the specification makes sense or needs to be implemented.
I have understood that it can be relevant to get a notification for the FF as @pylessard pointed out.
But I really do not want do support every crappy notification just because somebody thought this would be nice to have it in the UDS spec. I know different UDS implementations working on the can-isotp sockets that work without this notification.
So there must be a really good reason to add more complexity.