On 01.11.2022 22:29:02, Oliver Hartkopp wrote:
In commit 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive frames") the data flow for consecutive frames (CF) has been reworked to improve the reliability of long data transfers.
This rework did not touch the transmission and the tx state changes of single frame (SF) transfers which likely led to the WARN in the isotp_tx_timer_handler() catching a wrong tx state. This patch makes use of the improved frame processing for SF frames and sets the ISOTP_SENDING state in isotp_sendmsg() within the cmpxchg() condition handling.
A review of the state machine and the timer handling additionally revealed a missing echo timeout handling in the case of the burst mode in isotp_rcv_echo() and removes a potential timer configuration uncertainty in isotp_rcv_fc() when the receiver requests consecutive frames.
Fixes: 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive frames") Link: https://lore.kernel.org/linux-can/CAO4mrfe3dG7cMP1V5FLUkw7s+50c9vichigUMQwsx... Reported-by: Wei Chen harperchen1110@gmail.com Cc: stable@vger.kernel.org # v6.0 Signed-off-by: Oliver Hartkopp socketcan@hartkopp.net
[...]
@@ -905,10 +915,11 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer) so->tx.state = ISOTP_IDLE; wake_up_interruptible(&so->wait); break; default:
pr_notice_once("can-isotp: tx timer state %X\n", so->tx.state);
Can you use WARN_ONCE() instead of pr_notice_once() + WARN_ON_ONCE() here?
Marc