diff mbox series

[net-next,01/10] can: isotp: add module parameter for maximum pdu size

Message ID 20230404145908.1714400-2-mkl@pengutronix.de (mailing list archive)
State Accepted
Commit 96d1c81e6a0478535342dff6c730adb076cd84e8
Delegated to: Netdev Maintainers
Headers show
Series [net-next,01/10] can: isotp: add module parameter for maximum pdu size | expand

Checks

Context Check Description
netdev/series_format success Pull request is its own cover letter
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 18
netdev/cc_maintainers warning 2 maintainers not CCed: edumazet@google.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api fail Found: 'module_param' was: 0 now: 1
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 18 this patch: 18
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 119 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Marc Kleine-Budde April 4, 2023, 2:58 p.m. UTC
From: Oliver Hartkopp <socketcan@hartkopp.net>

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.

changes since v2: https://lore.kernel.org/all/20230313172510.3851-1-socketcan@hartkopp.net
- use ARRAY_SIZE() to reference DEFAULT_MAX_PDU_SIZE only at one place

changes since v1: https://lore.kernel.org/all/20230311143446.3183-1-socketcan@hartkopp.net
- limit the minimum 'max_pdu_size' to 4095 to maintain the classic
  behavior before ISO 15765-2:2016

Link: https://github.com/raspberrypi/linux/issues/5371
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230326115911.15094-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 65 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 9 deletions(-)


base-commit: 4cee0fb9cc4b5477637fdeb2e965f5fc7d624622

Comments

patchwork-bot+netdevbpf@kernel.org April 6, 2023, 12:30 a.m. UTC | #1
Hello:

This series was applied to netdev/net-next.git (main)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Tue,  4 Apr 2023 16:58:59 +0200 you wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,01/10] can: isotp: add module parameter for maximum pdu size
    https://git.kernel.org/netdev/net-next/c/96d1c81e6a04
  - [net-next,02/10] dt-bindings: arm: stm32: add compatible for syscon gcan node
    https://git.kernel.org/netdev/net-next/c/b341be6de98c
  - [net-next,03/10] dt-bindings: net: can: add STM32 bxcan DT bindings
    https://git.kernel.org/netdev/net-next/c/e43250c0ac81
  - [net-next,04/10] ARM: dts: stm32: add CAN support on stm32f429
    https://git.kernel.org/netdev/net-next/c/7355ad1950f4
  - [net-next,05/10] ARM: dts: stm32: add pin map for CAN controller on stm32f4
    https://git.kernel.org/netdev/net-next/c/559a6e75b4bc
  - [net-next,06/10] can: bxcan: add support for ST bxCAN controller
    https://git.kernel.org/netdev/net-next/c/f00647d8127b
  - [net-next,07/10] can: rcar_canfd: rcar_canfd_probe(): fix plain integer in transceivers[] init
    https://git.kernel.org/netdev/net-next/c/8e85d550c127
  - [net-next,08/10] dt-bindings: can: fsl,flexcan: add optional power-domains property
    https://git.kernel.org/netdev/net-next/c/066b41a599d6
  - [net-next,09/10] can: esd_usb: Add support for CAN_CTRLMODE_BERR_REPORTING
    https://git.kernel.org/netdev/net-next/c/c42fc3694923
  - [net-next,10/10] kvaser_usb: convert USB IDs to hexadecimal values
    https://git.kernel.org/netdev/net-next/c/1afae605e0b2

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 9bc344851704..f1ea91772ae7 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -85,10 +85,21 @@  MODULE_ALIAS("can-proto-6");
 
 /* 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
+
+/* maximum PDU size before ISO 15765-2:2016 extension was 4095 */
+#define MAX_12BIT_PDU_SIZE 4095
+
+/* 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 */
@@ -123,13 +134,15 @@  enum {
 };
 
 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 {
@@ -503,7 +516,17 @@  static int isotp_rcv_ff(struct sock *sk, struct canfd_frame *cf, int ae)
 	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;
@@ -807,7 +830,7 @@  static void isotp_create_fframe(struct canfd_frame *cf, struct isotp_sock *so,
 		cf->data[0] = so->opt.ext_address;
 
 	/* create N_PCI bytes with 12/32 bit FF_DL data length */
-	if (so->tx.len > 4095) {
+	if (so->tx.len > MAX_12BIT_PDU_SIZE) {
 		/* use 32 bit FF_DL notation */
 		cf->data[ae] = N_PCI_FF;
 		cf->data[ae + 1] = 0;
@@ -947,7 +970,17 @@  static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		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;
 	}
@@ -1195,6 +1228,12 @@  static int isotp_release(struct socket *sock)
 	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;
 
@@ -1591,6 +1630,11 @@  static int isotp_init(struct sock *sk)
 	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 = ARRAY_SIZE(so->rx.sbuf);
+	so->tx.buflen = ARRAY_SIZE(so->tx.sbuf);
+
 	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);
@@ -1658,7 +1702,10 @@  static __init int isotp_module_init(void)
 {
 	int err;
 
-	pr_info("can: isotp protocol\n");
+	max_pdu_size = max_t(unsigned int, max_pdu_size, MAX_12BIT_PDU_SIZE);
+	max_pdu_size = min_t(unsigned int, max_pdu_size, 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)