Re: [RFC PATCH] can: isotp: add module parameter for maximum pdu size

From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Patrick Menschel <menschel.p@posteo.de>, linux-can@vger.kernel.org
Subject: Re: [RFC PATCH] can: isotp: add module parameter for maximum pdu size
Date: Sat, 11 Mar 2023 20:17:00 +0100	[thread overview]
Message-ID: <405b08b7-df80-dc09-3a94-70bff3bb6e0b@hartkopp.net> (raw)
In-Reply-To: <fd198411-dc23-1c57-7599-d8b5214d0ce7@posteo.de>

Hi Patrick,

On 11.03.23 18:23, Patrick Menschel wrote:

> IMO it is the right direction but it is just half the story.
> 
> I remember having problems with the txqueuelen even with 4KB PDUs 
> originally and I'm not sure if that has been improved already.
> 
> https://gitlab.com/Menschel/socketcan#can-isotp-overflows-when-the-tx-queue-is-too-small
> 
> It would be great to have all of those size options in one place and 
> some intelligent IO buffer.

I solved that problem since Linux 6.0 \o/

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/net/can/isotp.c?id=4b7fe92c06901f4563af0e36d25223a5ab343782

> Apart from that the PDU size does not matter for UDS, because UDS splits 
> the data into chunks that are sent via transfer_data service and 
> concatenated on the receiver end again, sort of the protocol's built-in 
> IO buffer.
> 
> https://gitlab.com/Menschel/socketcan-uds/-/blob/master/uds/programmer.py#L495

Yes. It is not needed for UDS as you stated correctly.

The reason for the increased PDU length was discussed when we created 
the CAN FD support for ISO TP in 2014. Some guy talked about the 
flashing of bootloader firmware "in one piece". The firmware was about 
64kbyte but then we decided to go for a 32 bit value. IMHO 24 bits would 
have made it too :-D

Best regards,
Oliver

> -- 
> 
> Am 11.03.23 um 15:34 schrieb Oliver Hartkopp:
>> With ISO 15765-2:2016 the PDU size is not limited to 2^12 - 1 (4095) 
>> bytes
>> but can be represented as a 32 bit unsigned integer value which allows
>> 2^32 - 1 bytes (~4GB). The use-cases like automotive unified diagnostic
>> services (UDS) and flashing of ECUs still use the small static buffers
>> which are provided at socket creation time.
>>
>> When a use-case requires to transfer PDUs up to 1025 kByte the maximum
>> PDU size can now be extended by setting the module parameter 
>> max_pdu_size.
>> The extended size buffers are only allocated on a per-socket/connection
>> base when needed at run-time.
>>
>> Link: https://github.com/raspberrypi/linux/issues/5371
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 