can: isotp: add module parameter for maximum pdu size
Subsystems (2)
Oliver Hartkopp
maintainerRFC PATCH 1/1 can: isotp: add module parameter for maximum pdu size
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.
--- net/can/isotp.c | 59 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 8 deletions(-)
net/can/isotp.c +51 -8
@@ -83,14 +83,22 @@ MODULE_ALIAS("can-proto-6"); (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \ (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))/* ISO 15765-2:2016 supports more than 4095 byte per ISO PDU as the FF_DL can * take full 32 bit values (4 Gbyte). We would need some good concept to handle -* this between user space and kernel space. For now increase the static buffer -* to something about 64 kbyte to be able to test this new functionality. +* this between user space and kernel space. For now set the static buffer to +* something about 8 kbyte to be able to test this new functionality. */ -#define MAX_MSG_LENGTH 66000 +#define DEFAULT_MAX_PDU_SIZE 8300 + +/* limit the isotp pdu size from the optional module parameter to 1MByte */ +#define MAX_PDU_SIZE (1025 * 1024U) + +static unsigned int max_pdu_size__read_mostly =DEFAULT_MAX_PDU_SIZE ; +module_param (max_pdu_size, uint,0444 ); +MODULE_PARM_DESC(max_pdu_size, "maximum isotp pdu size (default " + __stringify(DEFAULT_MAX_PDU_SIZE) ")");/* N_PCI type values in bits 7-4 of N_PCI bytes */ #define N_PCI_SF 0x00/* single frame */ #define N_PCI_FF 0x10/* first frame */ #define N_PCI_CF 0x20/* consecutive frame */
@@ -121,17 +129,19 @@ enum { ISOTP_WAIT_DATA,ISOTP_SENDING };struct tpcon { -unsigned int idx; +u8 *buf; +unsigned int buflen;unsigned int len; +unsigned int idx;u32 state;u8 bs;u8 sn;u8 ll_dl; -u8 buf[MAX_MSG_LENGTH +1 ]; +u8 sbuf[DEFAULT_MAX_PDU_SIZE ]; };struct isotp_sock {struct sock sk;int bound;
@@ -501,11 +511,21 @@ static int isotp_rcv_ff(struct sock *sk, struct canfd_frame *cf, int ae) off = (so->rx.ll_dl >CAN_MAX_DLEN ) ?1 :0 ;if (so->rx.len + ae + off + ff_pci_sz < so->rx.ll_dl)return 1 ; -if (so->rx.len >MAX_MSG_LENGTH ) { +/* PDU size > default => try max_pdu_size */ +if (so->rx.len > so->rx.buflen && so->rx.buflen < max_pdu_size) { +u8 *newbuf =kmalloc (max_pdu_size,GFP_ATOMIC ); + +if (newbuf) { + so->rx.buf = newbuf; + so->rx.buflen = max_pdu_size; + } + } + +if (so->rx.len > so->rx.buflen) {/* send FC frame with overflow status */ isotp_send_fc (sk, ae,ISOTP_FC_OVFLW );return 1 ; }
@@ -945,11 +965,21 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) goto err_out; so->tx.state =ISOTP_SENDING ; } -if (!size || size >MAX_MSG_LENGTH ) { +/* PDU size > default => try max_pdu_size */ +if (size > so->tx.buflen && so->tx.buflen < max_pdu_size) { +u8 *newbuf =kmalloc (max_pdu_size,GFP_KERNEL ); + +if (newbuf) { + so->tx.buf = newbuf; + so->tx.buflen = max_pdu_size; + } + } + +if (!size || size > so->tx.buflen) { err = -EINVAL ; goto err_out_drop; }/* take care of a potential SF_DL ESC offset for TX_DL > 8 */
@@ -1193,10 +1223,16 @@ static int isotp_release(struct socket *sock)hrtimer_cancel (&so->rxtimer); so->ifindex =0 ; so->bound =0 ; +if (so->rx.buf != so->rx.sbuf) +kfree (so->rx.buf); + +if (so->tx.buf != so->tx.sbuf) +kfree (so->tx.buf); +sock_orphan (sk); sock->sk =NULL ;release_sock (sk);sock_put (sk);
@@ -1589,10 +1625,15 @@ static int isotp_init(struct sock *sk) so->tx.ll_dl = so->ll.tx_dl; so->rx.state =ISOTP_IDLE ; so->tx.state =ISOTP_IDLE ; + so->rx.buf = so->rx.sbuf; + so->tx.buf = so->tx.sbuf; + so->rx.buflen =DEFAULT_MAX_PDU_SIZE ; + so->tx.buflen =DEFAULT_MAX_PDU_SIZE ; +hrtimer_init (&so->rxtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_REL_SOFT ); so->rxtimer.function = isotp_rx_timer_handler;hrtimer_init (&so->txtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_REL_SOFT ); so->txtimer.function = isotp_tx_timer_handler;hrtimer_init (&so->txfrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_REL_SOFT );
@@ -1656,11 +1697,13 @@ static struct notifier_block canisotp_notifier = {static __init int isotp_module_init (void ) {int err; -pr_info ("can: isotp protocol\n" ); + max_pdu_size =min (max_pdu_size, (unsigned int )MAX_PDU_SIZE ); + +pr_info ("can: isotp protocol (max_pdu_size %d)\n" , max_pdu_size); err =can_proto_register (&isotp_can_proto);if (err <0 )pr_err ("can: registration of isotp protocol failed %pe\n" ,ERR_PTR (err));else
Signature · 1 line
2.30.2
Hello,
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.
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
BR,
Patrick
--
Am 11.03.23 um 15:34 schrieb Oliver Hartkopp:
Quoted 13 lines
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.
Oliver Hartkopp
maintainerHi Patrick,
Quoted 7 lines · Patrick Menschel
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/
Quoted 3 lines
Apart from that the PDU size does not matter for UDS, because UDS splits the data into chunks that are sent via
transfer_dataservice 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
Quoted 17 lines
Signature · 1 line
Am 11.03.23 um 15:34 schrieb Oliver Hartkopp:Quoted 13 lines
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.