linux-can / can-utils

Linux-CAN / SocketCAN user space applications
2.44k stars 714 forks source link

Support for FIONREAD ioctl #347

Closed derek-will closed 2 years ago

derek-will commented 2 years ago

When I try to inquire about the number of bytes available to read on a Raw CAN or ISO-TP socket via the FIONREAD ioctl I get back a -1 return code and when I query errno I get back ENOTTY.

int bytesAvailable = 0;
int returnCode = ioctl(socket, FIONREAD, &bytesAvailable);

I suspect that FIONREAD is simply just not supported on the CAN sockets. Is this the case? If so, are there any plan to support this ioctl?

Thanks for your time!

marckleinebudde commented 2 years ago

Can you explain your use-case.

You can only read whole CAN frames from the socket. With read(), recv(), recvmsg() it's only 1 struct can_frame at a time, or 0 if no data is available. If you want to read more than 1 CAN frame, you can use recvmmsg().

derek-will commented 2 years ago

For ISO-TP and J1939 sockets it would be useful to be able to see how large the next message within the buffer is so that the byte array passed into read, recv, etc. is optimally and correctly sized. It could also potentially be useful to see how many CAN Frames are in the queue for Raw CAN sockets.

To elaborate further, for ISO-TP sockets we currently have to "guess" at what the appropriate size for the next read message will be. Prior to ISO 15765-2:2016 we would have been able to confidently just pass a byte array of length 4095 into the read / recv call everytime. However, with escape sequence support we could be dealing with much larger messages of size 8KB, 16KB, 32KB, ... or if you encounter a madman: 4GB.

I am currently developing a series of classes that abstract / "object-orient" the socket functionality similar to what the .NET BCL offers with it's own Socket class and other related classes (TcpClient, UdpClient, etc.). This is for the Managed C# Wrapper for SocketCAN which I have authored.

While honoring the Socket class design which should be familiar (and therefore more accessible) to anyone who has spent time working in .NET, I came across the Available property. It was the only functionality that could be applicable to these sockets that I was unable to provide.

marckleinebudde commented 2 years ago

Can you use recvmsg() + MSG_PEEK?

The recvmsg() function returns the total length of the message. For message-based sockets such as SOCK_DGRAM and SOCK_SEQPACKET, the entire message must be read in a single operation. If a message is too long to fit in the supplied buffers, and MSG_PEEK is not set in the flags argument, the excess bytes are discarded, and MSG_TRUNC is set in the msg_flags member of the msghdr structure. For stream-based sockets such as SOCK_STREAM, message boundaries are ignored. In this case, data is returned to the user as soon as it becomes available, and no data is discarded.

See also this discussion: https://stackoverflow.com/questions/48861927/how-do-i-get-the-size-of-the-msg-control-buffer-for-recvmsg

hartkopp commented 2 years ago

To elaborate further, for ISO-TP sockets we currently have to "guess" at what the appropriate size for the next read message will be. Prior to ISO 15765-2:2016 we would have been able to confidently just pass a byte array of length 4095 into the read / recv call everytime. However, with escape sequence support we could be dealing with much larger messages of size 8KB, 16KB, 32KB, ... or if you encounter a madman: 4GB.

Yes, i've been thinking about the 4GB ISO-TP PDU length too. As I was involved in the specification process, I remember that the justification for the PDU length extension was "to flash the 64k bootloader in one block". Well ... don't know if anyone really uses that feature. At least it is implemented in the Linux kernel implementation which uses a fixed size buffer of currently 8200 byte to be able to test the length extension feature https://github.com/torvalds/linux/blob/master/net/can/isotp.c#L87

This buffer size can be changed on compile time. Would you think the API using read and write on a socket would still fit for a 1GB PDU?

derek-will commented 2 years ago

Can you use recvmsg() + MSG_PEEK?

So I tried using recv which is equivalent to read with the addition of the flags argument. The recv call on the CAN_ISOTP socket seems to respect MSG_PEEK flag as the following read call retrieves the full data from the receive buffer. However, MSG_TRUNC does not seem to be working. The recv call always returns the number of bytes actually read and not the number of bytes associated with the next message.

For example, if I pass a buffer of length 1 to the recv call then it returns 1 and not the real length of the message.

derek-will commented 2 years ago

Yes, i've been thinking about the 4GB ISO-TP PDU length too. As I was involved in the specification process, I remember that the justification for the PDU length extension was "to flash the 64k bootloader in one block". Well ... don't know if anyone really uses that feature. At least it is implemented in the Linux kernel implementation which uses a fixed size buffer of currently 8200 byte to be able to test the length extension feature https://github.com/torvalds/linux/blob/master/net/can/isotp.c#L87

Ah. I see why my attempts to send 16K messages were not working before, I had to drop back to 8K messages to obtain successful transmits during my tests.

So there are two main use cases that I am aware of:

  1. Speed of flash reprogramming can be improved by using larger (> 4K) TransferData (0x36) transmits from the tester side.
  2. Some of the responses from ECUs can exceed 4K. I know this was a problem with Classic OBD / OBDonEDS where ECUs could run into situations where they had so many DTCs set on them that it would exceed the maximum message length allowed by ISO-TP prior to the 2016 edition. This of course gets worse when you look at OBDonUDS as DTCs are represented by more bytes (2-byte --> 3-byte). In addition for OBDonUDS, a tester could send a Service 0x19 Subfunction 0x42 request using a Status Mask that could pull up a lot of the DTCs supported by an ECU. In light of all of this, OBDonUDS mandates support for the escape sequence from ISO-TP which allows for > 4K messages. This is all just in the relatively limited scope of legislated diagnostic communications. :)

This buffer size can be changed on compile time. Would you think the API using read and write on a socket would still fit for a 1GB PDU?

To be honest, I don't think it would anymore - At least not without special provisions. I know this was a topic that the J2534 API committee was discussing back when I was a member. My two cents is that the PDU size should be capped at 64K in the Linux kernel as that seems like a good practical maximum number.

hartkopp commented 2 years ago

This buffer size can be changed on compile time. Would you think the API using read and write on a socket would still fit for a 1GB PDU?

To be honest, I don't think it would anymore - At least not without special provisions. I know this was a topic that the J2534 API committee was discussing back when I was a member. My two cents is that the PDU size should be capped at 64K in the Linux kernel as that seems like a good practical maximum number.

That sounds like a reasonable approach! In fact I did not get much feedback on the isotp implementation I've started in 2009 ;-) Would you suggest to go for some value > 65.536 on PDU level, e.g. 66.000 to be able to provide some additional stuff (counters, checksums, signatures) to the 64k block?

@marckleinebudde Should we think of a memory budget for isotp sockets as every socket would request ~128k at creation time then? E.g. with a kernel module parameter for the buffer size that only root can set? Or am I just thinking too far and an unprivileged user can consume all the memory with opening TCP/IP sockets today anyway?

derek-will commented 2 years ago

That sounds like a reasonable approach! In fact I did not get much feedback on the isotp implementation I've started in 2009 ;-) Would you suggest to go for some value > 65.536 on PDU level, e.g. 66.000 to be able to provide some additional stuff (counters, checksums, signatures) to the 64k block?

The payload provided to ISO-TP is usually a diagnostic protocol such as UDS, KWP2000, GMLAN, OBDonEDS (0x01-0x0A), etc. Data like Counters, Checksums, and Signatures are considered part of the payload. For example, such data could be part of the DataRecord for a given DID or part of the ControlOptionRecord / StatusRecord for a given RID.

Thinking more along the lines of providing the ability for a single TransferData (0x36) request to include say the entire image for a 64K bootloader, then the payload would be 0x36 0x01 { up to 64K of flash data }. The image is probably not going to use the entire flash memory, but let us assume the worse case. In addition, you may have additional metadata like a digital signature, CRC, etc. that could be part of the download as an authenticity or integrity check and won't necessarily be written to flash. At that point, you are looking at a payload of 0x36 0x01 { 64K of flash data } { Metadata }.

This is all to say that I think adding a little extra pad space is a solid idea!

Plus, as ISO 15765-2 states: "Although primarily intended for diagnostic systems, it also meets requirements from other CAN‑based systems needing a network layer protocol." So I think a bit of wiggle room here is more than merited. :)

hartkopp commented 2 years ago

Hi Derek,

the change for 64kByte frames is now on its way to mainline Linux:

https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?h=testing&id=3b42241f90bf2b587524ab86772dbed652393aff

along with some other improvements, especially this one:

https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?h=testing&id=fb440a021a2e99ece37e6528391751dc51ae1f6e

Will update the isotp specific can-utils soon.

derek-will commented 2 years ago

So I tried using recv which is equivalent to read with the addition of the flags argument. The recv call on the CAN_ISOTP socket seems to respect MSG_PEEK flag as the following read call retrieves the full data from the receive buffer. However, MSG_TRUNC does not seem to be working. The recv call always returns the number of bytes actually read and not the number of bytes associated with the next message.

For example, if I pass a buffer of length 1 to the recv call then it returns 1 and not the real length of the message.

@marckleinebudde @hartkopp : Is there any comment on why I am seeing the above behavior when calling recv while using the MSG_TRUNC flag? Is there any chance on getting support for the FIONREAD ioctl for the PF_CAN sockets?

hartkopp commented 2 years ago

Hi Derek,

So I tried using recv which is equivalent to read with the addition of the flags argument. The recv call on the CAN_ISOTP socket seems to respect MSG_PEEK flag as the following read call retrieves the full data from the receive buffer. However, MSG_TRUNC does not seem to be working. The recv call always returns the number of bytes actually read and not the number of bytes associated with the next message. For example, if I pass a buffer of length 1 to the recv call then it returns 1 and not the real length of the message.

@marckleinebudde @hartkopp : Is there any comment on why I am seeing the above behavior when calling recv while using the MSG_TRUNC flag? Is there any chance on getting support for the FIONREAD ioctl for the PF_CAN sockets?

We currently do not support ioctls in the CAN networklayer. And from what I've seen providing the MSG_TRUNC functionality is more common.

If I got it right the big difference is, that the read() or recv() call returns the real length of the packet or datagram, even when it was longer than the passed buffer. E.g. when the given buffer is 10 bytes and the PDU is 20, the read() call fills the buffer with 10 bytes and returns "20" when MSG_TRUNC was set, right?

Edit: Like here https://elixir.bootlin.com/linux/v5.16.14/source/net/packet/af_packet.c#L3527 right? https://elixir.bootlin.com/linux/v5.16.14/source/net/phonet/datagram.c#L144

derek-will commented 2 years ago

We currently do not support ioctls in the CAN networklayer. And from what I've seen providing the MSG_TRUNC functionality is more common.

If I got it right the big difference is, that the read() or recv() call returns the real length of the packet or datagram, even when it was longer than the passed buffer. E.g. when the given buffer is 10 bytes and the PDU is 20, the read() call fills the buffer with 10 bytes and returns "20" when MSG_TRUNC was set, right?

Edit: Like here https://elixir.bootlin.com/linux/v5.16.14/source/net/packet/af_packet.c#L3527 right? https://elixir.bootlin.com/linux/v5.16.14/source/net/phonet/datagram.c#L144

That is correct. From the recv manpage the MSG_TRUNC flag represents the following:

For raw (AF_PACKET), Internet datagram (since Linux 2.4.27/2.6.8), netlink (since Linux 2.6.22), and UNIX datagram (since Linux 3.4) sockets: return the real length of the packet or datagram, even when it was longer than the passed buffer.

hartkopp commented 2 years ago

Hi Derek,

your suggestion is now in the upstream process and will show up in Linux 5.18 (and the stable kernels): https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=42bf50a1795a1854d4

Many thanks, Oliver

derek-will commented 2 years ago

Hi Oliver,

I was reviewing the new code and I have a concern with the following part of isotp_recvmsg:

if (flags & ~(MSG_DONTWAIT | MSG_TRUNC))
        return -EINVAL;

To be able to get the size of the buffer to allocate for the next message without removing the message from the receive queue, I need to be able to set the flags to MSG_PEEK | MSG_TRUNC. However, the new code will result in EINVAL being returned when MSG_PEEK is included in the flags.

I believe the code should be adjusted to:

if (flags & ~(MSG_DONTWAIT | MSG_TRUNC | MSG_PEEK))
        return -EINVAL;

Do you agree? Or am I missing something here?

Thanks, Derek

hartkopp commented 2 years ago

Oh, I just checked for the flags I'm processing and therefore supporting in isotp.c myself. Now that you have pointed me to MSG_PEEK it looks like that this flag is being processed later in the call chain in __skb_try_recv_from_queue() .

As you have already confirmed MSG_PEEK to work here https://github.com/linux-can/can-utils/issues/347#issuecomment-1053705397 it really seems that I accidentally removed that feature when adding the new flags check :-(

I will send a follow-up patch when the merge window is closed and the stable kernel updates are settled. So the Linux 5.18 kernel will support it, when released. Do you have the possibility to add and test this flag on your machine?

Thanks for the review!

derek-will commented 2 years ago

Hi Oliver,

I added this change, recompiled the can-isotp module, and tested it on my machine. It works! I also tested #349 which is working as well. :-)

I submitted a pull request to the can-isotp repo with the change. Please let me know of any other way that I can be of help.

Thanks, Derek

hartkopp commented 2 years ago

Hi Derek,

many thanks for the PR and the testing!

I already pulled your fix. Will send a fix to the CAN mailing list in some days with some slightly changed commit message content.

But you'll see anyway as git send-email will put you into CC ;-)

Here we are: https://lore.kernel.org/linux-can/20220328113611.3691-1-socketcan@hartkopp.net/T/#u

marckleinebudde commented 2 years ago

Hello @derek-will,

can you please reply to the patch that Oliver sent and write your S-o-b line:

Signed-off-by Derek Will <derekrobertwill@gmail.com>