[PATCH net 0/5] pull-request: can 2023-02-02

[PATCH net 0/5] pull-request: can 2023-02-02

[Marc Kleine-Budde]

Hello Jakub, hello David,

this is a pull request of 5 patches for net/master.

The first patch is by Ziyang Xuan and removes a errant WARN_ON_ONCE()
in the CAN J1939 protocol.

The next 3 patches are by Oliver Hartkopp. The first 2 target the CAN
ISO-TP protocol and fix the state machine with respect to signals and
a regression found by the syzbot.

The last patch is by me an missing assignment during the ethtool ring
configuration callback.

regards,
Marc

---

The following changes since commit 917d5e04d4dd2bbbf36fc6976ba442e284ccc42d:

  octeontx2-af: Fix devlink unregister (2023-02-01 21:22:41 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-6.2-20230202

for you to fetch changes up to 1613fff7a32e1d9e2ac09db73feba0e71a188445:

  can: mcp251xfd: mcp251xfd_ring_set_ringparam(): assign missing tx_obj_num_coalesce_irq (2023-02-02 10:33:27 +0100)

----------------------------------------------------------------
linux-can-fixes-for-6.2-20230202

----------------------------------------------------------------
Marc Kleine-Budde (1):
      can: mcp251xfd: mcp251xfd_ring_set_ringparam(): assign missing tx_obj_num_coalesce_irq

Oliver Hartkopp (3):
      can: raw: fix CAN FD frame transmissions over CAN XL devices
      can: isotp: handle wait_event_interruptible() return values
      can: isotp: split tx timer into transmission and timeout

Ziyang Xuan (1):
      can: j1939: fix errant WARN_ON_ONCE in j1939_session_deactivate

 drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c |  1 +
 net/can/isotp.c                                   | 69 +++++++++++------------
 net/can/j1939/transport.c                         |  4 --
 net/can/raw.c                                     | 47 +++++++++------
 4 files changed, 65 insertions(+), 56 deletions(-)

[PATCH net 1/5] can: j1939: fix errant WARN_ON_ONCE in j1939_session_deactivate

[Marc Kleine-Budde]

From: Ziyang Xuan <william.xuanziyang@huawei.com>

The conclusion "j1939_session_deactivate() should be called with a
session ref-count of at least 2" is incorrect. In some concurrent
scenarios, j1939_session_deactivate can be called with the session
ref-count less than 2. But there is not any problem because it
will check the session active state before session putting in
j1939_session_deactivate_locked().

Here is the concurrent scenario of the problem reported by syzbot
and my reproduction log.

        cpu0                            cpu1
                                j1939_xtp_rx_eoma
j1939_xtp_rx_abort_one
                                j1939_session_get_by_addr [kref == 2]
j1939_session_get_by_addr [kref == 3]
j1939_session_deactivate [kref == 2]
j1939_session_put [kref == 1]
				j1939_session_completed
				j1939_session_deactivate
				WARN_ON_ONCE(kref < 2)

=====================================================
WARNING: CPU: 1 PID: 21 at net/can/j1939/transport.c:1088 j1939_session_deactivate+0x5f/0x70
CPU: 1 PID: 21 Comm: ksoftirqd/1 Not tainted 5.14.0-rc7+ #32
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/01/2014
RIP: 0010:j1939_session_deactivate+0x5f/0x70
Call Trace:
 j1939_session_deactivate_activate_next+0x11/0x28
 j1939_xtp_rx_eoma+0x12a/0x180
 j1939_tp_recv+0x4a2/0x510
 j1939_can_recv+0x226/0x380
 can_rcv_filter+0xf8/0x220
 can_receive+0x102/0x220
 ? process_backlog+0xf0/0x2c0
 can_rcv+0x53/0xf0
 __netif_receive_skb_one_core+0x67/0x90
 ? process_backlog+0x97/0x2c0
 __netif_receive_skb+0x22/0x80

Fixes: 0c71437dd50d ("can: j1939: j1939_session_deactivate(): clarify lifetime of session object")
Reported-by: syzbot+9981a614060dcee6eeca@syzkaller.appspotmail.com
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/all/20210906094200.95868-1-william.xuanziyang@huawei.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/j1939/transport.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 5c722b55fe23..fce9b9ebf13f 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1092,10 +1092,6 @@ static bool j1939_session_deactivate(struct j1939_session *session)
 	bool active;
 
 	j1939_session_list_lock(priv);
-	/* This function should be called with a session ref-count of at
-	 * least 2.
-	 */
-	WARN_ON_ONCE(kref_read(&session->kref) < 2);
 	active = j1939_session_deactivate_locked(session);
 	j1939_session_list_unlock(priv);
 

base-commit: 917d5e04d4dd2bbbf36fc6976ba442e284ccc42d
-- 
2.39.1

Re: [PATCH net 1/5] can: j1939: fix errant WARN_ON_ONCE in j1939_session_deactivate

[patchwork-bot+netdevbpf]

Hello:

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

On Thu,  2 Feb 2023 10:41:31 +0100 you wrote:
> From: Ziyang Xuan <william.xuanziyang@huawei.com>
> 
> The conclusion "j1939_session_deactivate() should be called with a
> session ref-count of at least 2" is incorrect. In some concurrent
> scenarios, j1939_session_deactivate can be called with the session
> ref-count less than 2. But there is not any problem because it
> will check the session active state before session putting in
> j1939_session_deactivate_locked().
> 
> [...]

Here is the summary with links:
  - [net,1/5] can: j1939: fix errant WARN_ON_ONCE in j1939_session_deactivate
    https://git.kernel.org/netdev/net/c/d0553680f94c
  - [net,2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices
    https://git.kernel.org/netdev/net/c/3793301cbaa4
  - [net,3/5] can: isotp: handle wait_event_interruptible() return values
    https://git.kernel.org/netdev/net/c/823b2e42720f
  - [net,4/5] can: isotp: split tx timer into transmission and timeout
    https://git.kernel.org/netdev/net/c/4f027cba8216
  - [net,5/5] can: mcp251xfd: mcp251xfd_ring_set_ringparam(): assign missing tx_obj_num_coalesce_irq
    https://git.kernel.org/netdev/net/c/1613fff7a32e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net 2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices

[Marc Kleine-Budde]

From: Oliver Hartkopp <socketcan@hartkopp.net>

A CAN XL device is always capable to process CAN FD frames. The former
check when sending CAN FD frames relied on the existence of a CAN FD
device and did not check for a CAN XL device that would be correct
too.

With this patch the CAN FD feature is enabled automatically when CAN
XL is switched on - and CAN FD cannot be switch off while CAN XL is
enabled.

This precondition also leads to a clean up and reduction of checks in
the hot path in raw_rcv() and raw_sendmsg(). Some conditions are
reordered to handle simple checks first.

changes since v1: https://lore.kernel.org/all/20230131091012.50553-1-socketcan@hartkopp.net
- fixed typo: devive -> device
changes since v2: https://lore.kernel.org/all/20230131091824.51026-1-socketcan@hartkopp.net/
- reorder checks in if statements to handle simple checks first

Fixes: 626332696d75 ("can: raw: add CAN XL support")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230131105613.55228-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/raw.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/net/can/raw.c b/net/can/raw.c
index 81071cdb0301..ba86782ba8bb 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -132,8 +132,8 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
 		return;
 
 	/* make sure to not pass oversized frames to the socket */
-	if ((can_is_canfd_skb(oskb) && !ro->fd_frames && !ro->xl_frames) ||
-	    (can_is_canxl_skb(oskb) && !ro->xl_frames))
+	if ((!ro->fd_frames && can_is_canfd_skb(oskb)) ||
+	    (!ro->xl_frames && can_is_canxl_skb(oskb)))
 		return;
 
 	/* eliminate multiple filter matches for the same skb */
@@ -670,6 +670,11 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
 		if (copy_from_sockptr(&ro->fd_frames, optval, optlen))
 			return -EFAULT;
 
+		/* Enabling CAN XL includes CAN FD */
+		if (ro->xl_frames && !ro->fd_frames) {
+			ro->fd_frames = ro->xl_frames;
+			return -EINVAL;
+		}
 		break;
 
 	case CAN_RAW_XL_FRAMES:
@@ -679,6 +684,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
 		if (copy_from_sockptr(&ro->xl_frames, optval, optlen))
 			return -EFAULT;
 
+		/* Enabling CAN XL includes CAN FD */
+		if (ro->xl_frames)
+			ro->fd_frames = ro->xl_frames;
 		break;
 
 	case CAN_RAW_JOIN_FILTERS:
@@ -786,6 +794,25 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
 	return 0;
 }
 
+static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
+{
+	/* Classical CAN -> no checks for flags and device capabilities */
+	if (can_is_can_skb(skb))
+		return false;
+
+	/* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
+	if (ro->fd_frames && can_is_canfd_skb(skb) &&
+	    (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
+		return false;
+
+	/* CAN XL -> needs to be enabled and a CAN XL device */
+	if (ro->xl_frames && can_is_canxl_skb(skb) &&
+	    can_is_canxl_dev_mtu(mtu))
+		return false;
+
+	return true;
+}
+
 static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 {
 	struct sock *sk = sock->sk;
@@ -833,20 +860,8 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		goto free_skb;
 
 	err = -EINVAL;
-	if (ro->xl_frames && can_is_canxl_dev_mtu(dev->mtu)) {
-		/* CAN XL, CAN FD and Classical CAN */
-		if (!can_is_canxl_skb(skb) && !can_is_canfd_skb(skb) &&
-		    !can_is_can_skb(skb))
-			goto free_skb;
-	} else if (ro->fd_frames && dev->mtu == CANFD_MTU) {
-		/* CAN FD and Classical CAN */
-		if (!can_is_canfd_skb(skb) && !can_is_can_skb(skb))
-			goto free_skb;
-	} else {
-		/* Classical CAN */
-		if (!can_is_can_skb(skb))
-			goto free_skb;
-	}
+	if (raw_bad_txframe(ro, skb, dev->mtu))
+		goto free_skb;
 
 	sockcm_init(&sockc, sk);
 	if (msg->msg_controllen) {
-- 
2.39.1

Re: [PATCH net 2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices

[Oliver Hartkopp]

Hi Marc,

you asked me to introduce a new variable instead of rolling back the 
ro->fd_frames setting in setsockopt.

I just wondered why you didn't pick the V4 patch then:

https://lore.kernel.org/linux-can/20230131112657.59247-1-socketcan@hartkopp.net/T/#u

It has no functional drawback but I was able to add an error return code 
in V4. I just wanted to know.

Best regards,
Oliver


On 02.02.23 10:41, Marc Kleine-Budde wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> A CAN XL device is always capable to process CAN FD frames. The former
> check when sending CAN FD frames relied on the existence of a CAN FD
> device and did not check for a CAN XL device that would be correct
> too.
> 
> With this patch the CAN FD feature is enabled automatically when CAN
> XL is switched on - and CAN FD cannot be switch off while CAN XL is
> enabled.
> 
> This precondition also leads to a clean up and reduction of checks in
> the hot path in raw_rcv() and raw_sendmsg(). Some conditions are
> reordered to handle simple checks first.
> 
> changes since v1: https://lore.kernel.org/all/20230131091012.50553-1-socketcan@hartkopp.net
> - fixed typo: devive -> device
> changes since v2: https://lore.kernel.org/all/20230131091824.51026-1-socketcan@hartkopp.net/
> - reorder checks in if statements to handle simple checks first
> 
> Fixes: 626332696d75 ("can: raw: add CAN XL support")
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Link: https://lore.kernel.org/all/20230131105613.55228-1-socketcan@hartkopp.net
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>   net/can/raw.c | 47 +++++++++++++++++++++++++++++++----------------
>   1 file changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 81071cdb0301..ba86782ba8bb 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -132,8 +132,8 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
>   		return;
>   
>   	/* make sure to not pass oversized frames to the socket */
> -	if ((can_is_canfd_skb(oskb) && !ro->fd_frames && !ro->xl_frames) ||
> -	    (can_is_canxl_skb(oskb) && !ro->xl_frames))
> +	if ((!ro->fd_frames && can_is_canfd_skb(oskb)) ||
> +	    (!ro->xl_frames && can_is_canxl_skb(oskb)))
>   		return;
>   
>   	/* eliminate multiple filter matches for the same skb */
> @@ -670,6 +670,11 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>   		if (copy_from_sockptr(&ro->fd_frames, optval, optlen))
>   			return -EFAULT;
>   
> +		/* Enabling CAN XL includes CAN FD */
> +		if (ro->xl_frames && !ro->fd_frames) {
> +			ro->fd_frames = ro->xl_frames;
> +			return -EINVAL;
> +		}
>   		break;
>   
>   	case CAN_RAW_XL_FRAMES:
> @@ -679,6 +684,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>   		if (copy_from_sockptr(&ro->xl_frames, optval, optlen))
>   			return -EFAULT;
>   
> +		/* Enabling CAN XL includes CAN FD */
> +		if (ro->xl_frames)
> +			ro->fd_frames = ro->xl_frames;
>   		break;
>   
>   	case CAN_RAW_JOIN_FILTERS:
> @@ -786,6 +794,25 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
>   	return 0;
>   }
>   
> +static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
> +{
> +	/* Classical CAN -> no checks for flags and device capabilities */
> +	if (can_is_can_skb(skb))
> +		return false;
> +
> +	/* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
> +	if (ro->fd_frames && can_is_canfd_skb(skb) &&
> +	    (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
> +		return false;
> +
> +	/* CAN XL -> needs to be enabled and a CAN XL device */
> +	if (ro->xl_frames && can_is_canxl_skb(skb) &&
> +	    can_is_canxl_dev_mtu(mtu))
> +		return false;
> +
> +	return true;
> +}
> +
>   static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
>   {
>   	struct sock *sk = sock->sk;
> @@ -833,20 +860,8 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
>   		goto free_skb;
>   
>   	err = -EINVAL;
> -	if (ro->xl_frames && can_is_canxl_dev_mtu(dev->mtu)) {
> -		/* CAN XL, CAN FD and Classical CAN */
> -		if (!can_is_canxl_skb(skb) && !can_is_canfd_skb(skb) &&
> -		    !can_is_can_skb(skb))
> -			goto free_skb;
> -	} else if (ro->fd_frames && dev->mtu == CANFD_MTU) {
> -		/* CAN FD and Classical CAN */
> -		if (!can_is_canfd_skb(skb) && !can_is_can_skb(skb))
> -			goto free_skb;
> -	} else {
> -		/* Classical CAN */
> -		if (!can_is_can_skb(skb))
> -			goto free_skb;
> -	}
> +	if (raw_bad_txframe(ro, skb, dev->mtu))
> +		goto free_skb;
>   
>   	sockcm_init(&sockc, sk);
>   	if (msg->msg_controllen) {

Re: [PATCH net 2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 847 bytes --]

On 02.02.2023 13:43:07, Oliver Hartkopp wrote:
> you asked me to introduce a new variable instead of rolling back the
> ro->fd_frames setting in setsockopt.
> 
> I just wondered why you didn't pick the V4 patch then:

Doh - that was not intentional!

> https://lore.kernel.org/linux-can/20230131112657.59247-1-socketcan@hartkopp.net/T/#u
> 
> It has no functional drawback but I was able to add an error return code in
> V4. I just wanted to know.

Please send a follow up patch against net-next/main once the net/main
has been merged in net-next/main.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net 2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices

[Oliver Hartkopp]

On 02.02.23 14:10, Marc Kleine-Budde wrote:
> On 02.02.2023 13:43:07, Oliver Hartkopp wrote:
>> you asked me to introduce a new variable instead of rolling back the
>> ro->fd_frames setting in setsockopt.
>>
>> I just wondered why you didn't pick the V4 patch then:
> 
> Doh - that was not intentional!
> 

No problem ;-)

>> https://lore.kernel.org/linux-can/20230131112657.59247-1-socketcan@hartkopp.net/T/#u
>>
>> It has no functional drawback but I was able to add an error return code in
>> V4. I just wanted to know.
> 
> Please send a follow up patch against net-next/main once the net/main
> has been merged in net-next/main.
> 

Ok, will do!

Thanks!

Oliver

Re: [PATCH net 2/5] can: raw: fix CAN FD frame transmissions over CAN XL devices

[Oliver Hartkopp]

> On 02.02.23 14:10, Marc Kleine-Budde wrote:

>>
>> Please send a follow up patch against net-next/main once the net/main
>> has been merged in net-next/main.
>>
Done!

https://lore.kernel.org/linux-can/20230203090807.97100-1-socketcan@hartkopp.net/T/#u

Thanks,
Oliver

[PATCH net 3/5] can: isotp: handle wait_event_interruptible() return values

[Marc Kleine-Budde]

From: Oliver Hartkopp <socketcan@hartkopp.net>

When wait_event_interruptible() has been interrupted by a signal the
tx.state value might not be ISOTP_IDLE. Force the state machines
into idle state to inhibit the timer handlers to continue working.

Fixes: 866337865f37 ("can: isotp: fix tx state handling for echo tx processing")
Cc: stable@vger.kernel.org
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230112192347.1944-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 608f8c24ae46..dae421f6c901 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1162,6 +1162,10 @@ static int isotp_release(struct socket *sock)
 	/* wait for complete transmission of current pdu */
 	wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
 
+	/* force state machines to be idle also when a signal occurred */
+	so->tx.state = ISOTP_IDLE;
+	so->rx.state = ISOTP_IDLE;
+
 	spin_lock(&isotp_notifier_lock);
 	while (isotp_busy_notifier == so) {
 		spin_unlock(&isotp_notifier_lock);
-- 
2.39.1

[PATCH net 4/5] can: isotp: split tx timer into transmission and timeout

[Marc Kleine-Budde]

From: Oliver Hartkopp <socketcan@hartkopp.net>

The timer for the transmission of isotp PDUs formerly had two functions:
1. send two consecutive frames with a given time gap
2. monitor the timeouts for flow control frames and the echo frames

This led to larger txstate checks and potentially to a problem discovered
by syzbot which enabled the panic_on_warn feature while testing.

The former 'txtimer' function is split into 'txfrtimer' and 'txtimer'
to handle the two above functionalities with separate timer callbacks.

The two simplified timers now run in one-shot mode and make the state
transitions (especially with isotp_rcv_echo) better understandable.

Fixes: 866337865f37 ("can: isotp: fix tx state handling for echo tx processing")
Reported-by: syzbot+5aed6c3aaba661f5b917@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org # >= v6.0
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230104145701.2422-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 65 ++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index dae421f6c901..fc81d77724a1 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -140,7 +140,7 @@ struct isotp_sock {
 	canid_t rxid;
 	ktime_t tx_gap;
 	ktime_t lastrxcf_tstamp;
-	struct hrtimer rxtimer, txtimer;
+	struct hrtimer rxtimer, txtimer, txfrtimer;
 	struct can_isotp_options opt;
 	struct can_isotp_fc_options rxfc, txfc;
 	struct can_isotp_ll_options ll;
@@ -871,7 +871,7 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
 	}
 
 	/* start timer to send next consecutive frame with correct delay */
-	hrtimer_start(&so->txtimer, so->tx_gap, HRTIMER_MODE_REL_SOFT);
+	hrtimer_start(&so->txfrtimer, so->tx_gap, HRTIMER_MODE_REL_SOFT);
 }
 
 static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
@@ -879,49 +879,39 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
 	struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
 					     txtimer);
 	struct sock *sk = &so->sk;
-	enum hrtimer_restart restart = HRTIMER_NORESTART;
 
-	switch (so->tx.state) {
-	case ISOTP_SENDING:
+	/* don't handle timeouts in IDLE state */
+	if (so->tx.state == ISOTP_IDLE)
+		return HRTIMER_NORESTART;
 
-		/* cfecho should be consumed by isotp_rcv_echo() here */
-		if (!so->cfecho) {
-			/* start timeout for unlikely lost echo skb */
-			hrtimer_set_expires(&so->txtimer,
-					    ktime_add(ktime_get(),
-						      ktime_set(ISOTP_ECHO_TIMEOUT, 0)));
-			restart = HRTIMER_RESTART;
+	/* we did not get any flow control or echo frame in time */
 
-			/* push out the next consecutive frame */
-			isotp_send_cframe(so);
-			break;
-		}
-
-		/* cfecho has not been cleared in isotp_rcv_echo() */
-		pr_notice_once("can-isotp: cfecho %08X timeout\n", so->cfecho);
-		fallthrough;
+	/* report 'communication error on send' */
+	sk->sk_err = ECOMM;
+	if (!sock_flag(sk, SOCK_DEAD))
+		sk_error_report(sk);
 
-	case ISOTP_WAIT_FC:
-	case ISOTP_WAIT_FIRST_FC:
+	/* reset tx state */
+	so->tx.state = ISOTP_IDLE;
+	wake_up_interruptible(&so->wait);
 
-		/* we did not get any flow control frame in time */
+	return HRTIMER_NORESTART;
+}
 
-		/* report 'communication error on send' */
-		sk->sk_err = ECOMM;
-		if (!sock_flag(sk, SOCK_DEAD))
-			sk_error_report(sk);
+static enum hrtimer_restart isotp_txfr_timer_handler(struct hrtimer *hrtimer)
+{
+	struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
+					     txfrtimer);
 
-		/* reset tx state */
-		so->tx.state = ISOTP_IDLE;
-		wake_up_interruptible(&so->wait);
-		break;
+	/* start echo timeout handling and cover below protocol error */
+	hrtimer_start(&so->txtimer, ktime_set(ISOTP_ECHO_TIMEOUT, 0),
+		      HRTIMER_MODE_REL_SOFT);
 
-	default:
-		WARN_ONCE(1, "can-isotp: tx timer state %08X cfecho %08X\n",
-			  so->tx.state, so->cfecho);
-	}
+	/* cfecho should be consumed by isotp_rcv_echo() here */
+	if (so->tx.state == ISOTP_SENDING && !so->cfecho)
+		isotp_send_cframe(so);
 
-	return restart;
+	return HRTIMER_NORESTART;
 }
 
 static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
@@ -1198,6 +1188,7 @@ static int isotp_release(struct socket *sock)
 		}
 	}
 
+	hrtimer_cancel(&so->txfrtimer);
 	hrtimer_cancel(&so->txtimer);
 	hrtimer_cancel(&so->rxtimer);
 
@@ -1601,6 +1592,8 @@ static int isotp_init(struct sock *sk)
 	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);
+	so->txfrtimer.function = isotp_txfr_timer_handler;
 
 	init_waitqueue_head(&so->wait);
 	spin_lock_init(&so->rx_lock);
-- 
2.39.1

[PATCH net 5/5] can: mcp251xfd: mcp251xfd_ring_set_ringparam(): assign missing tx_obj_num_coalesce_irq

[Marc Kleine-Budde]

If the a new ring layout is set, the max coalesced frames for RX and
TX are re-calculated, too. Add the missing assignment of the newly
calculated TX max coalesced frames.

Fixes: 656fc12ddaf8 ("can: mcp251xfd: add TX IRQ coalescing ethtool support")
Link: https://lore.kernel.org/all/20230130154334.1578518-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c
index 3585f02575df..57eeb066a945 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ethtool.c
@@ -48,6 +48,7 @@ mcp251xfd_ring_set_ringparam(struct net_device *ndev,
 	priv->rx_obj_num = layout.cur_rx;
 	priv->rx_obj_num_coalesce_irq = layout.rx_coalesce;
 	priv->tx->obj_num = layout.cur_tx;
+	priv->tx_obj_num_coalesce_irq = layout.tx_coalesce;
 
 	return 0;
 }
-- 
2.39.1