[PATCH] qcan: isotp: implement N_Ar timeout handling for FC transmission

[yewentian395]

Add N_Ar (ISO 15765-2) timeout logic to detect FC frame transmission
failures on the receiver side. Previously, isotp_send_fc() fired the
FC and immediately started the N_Cr timer (rxtimer) without confirming
that the FC was actually transmitted onto the CAN bus.

Introduce ISOTP_WAIT_FC_TX_CONFIRM state and fc_artimer to implement
a two-phase approach:

  1. N_Ar phase: after can_send(FC), wait for local echo confirmation
  2. N_Cr phase: after echo arrives, start rxtimer to wait for next CF

Key design points:

- fc_artimer and rxtimer are mutually exclusive (never both active)
- Timer handler uses guard condition (no spinlock) to avoid deadlock
  with hrtimer_cancel() callers, following isotp_rx_timer_handler pattern
- CF arrival during WAIT_FC_TX_CONFIRM acts as implicit FC confirmation
  (CF from remote proves FC was transmitted successfully)
- New FF/SF arrival during WAIT_FC_TX_CONFIRM cancels fc_artimer
- ISOTP_FC_OVFLW does not enter N_Ar phase (fire-and-forget)
- can_send() failure triggers immediate rollback

State machine extension (RX path only):

  WAIT_DATA -> isotp_send_fc(CTS) -> WAIT_FC_TX_CONFIRM
  WAIT_FC_TX_CONFIRM -> FC echo / CF arrival -> WAIT_DATA
  WAIT_FC_TX_CONFIRM -> fc_artimer timeout -> IDLE (ECOMM)
  WAIT_FC_TX_CONFIRM -> new FF/SF -> IDLE (reset)

Signed-off-by: yewentian395 <yewentian395@xxxxxxxxx>
---
 net/can/isotp.c | 93 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 13 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 1efa377f002e..2bdedfc1d1f8 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -125,12 +125,14 @@ MODULE_PARM_DESC(max_pdu_size, "maximum isotp pdu size (default "
 
 #define ISOTP_FC_TIMEOUT 1	/* 1 sec */
 #define ISOTP_ECHO_TIMEOUT 2	/* 2 secs */
+#define ISOTP_FC_AR_TIMEOUT 1	/* 1 sec - N_Ar timeout for FC tx confirm */
 
 enum {
 	ISOTP_IDLE = 0,
 	ISOTP_WAIT_FIRST_FC,
 	ISOTP_WAIT_FC,
 	ISOTP_WAIT_DATA,
+	ISOTP_WAIT_FC_TX_CONFIRM,	/* RX: waiting for local FC echo */
 	ISOTP_SENDING,
 	ISOTP_SHUTDOWN,
 };
@@ -155,7 +157,7 @@ struct isotp_sock {
 	canid_t rxid;
 	ktime_t tx_gap;
 	ktime_t lastrxcf_tstamp;
-	struct hrtimer rxtimer, txtimer, txfrtimer;
+	struct hrtimer rxtimer, txtimer, txfrtimer, fc_artimer;
 	struct can_isotp_options opt;
 	struct can_isotp_fc_options rxfc, txfc;
 	struct can_isotp_ll_options ll;
@@ -210,6 +212,25 @@ static enum hrtimer_restart isotp_rx_timer_handler(struct hrtimer *hrtimer)
 	return HRTIMER_NORESTART;
 }
 
+static enum hrtimer_restart isotp_fc_ar_timer_handler(struct hrtimer *hrtimer)
+{
+	struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
+					     fc_artimer);
+	struct sock *sk = &so->sk;
+
+	if (so->rx.state != ISOTP_WAIT_FC_TX_CONFIRM)
+		return HRTIMER_NORESTART;
+
+	so->rx.state = ISOTP_IDLE;
+	so->rx.len = 0;
+
+	sk->sk_err = ECOMM;
+	if (!sock_flag(sk, SOCK_DEAD))
+		sk_error_report(sk);
+
+	return HRTIMER_NORESTART;
+}
+
 static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 {
 	struct net_device *dev;
@@ -256,22 +277,37 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 
 	ncf->flags = so->ll.tx_flags;
 
-	can_send_ret = can_send(nskb, 1);
-	if (can_send_ret)
-		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
-			       __func__, ERR_PTR(can_send_ret));
-
-	dev_put(dev);
-
 	/* reset blocksize counter */
 	so->rx.bs = 0;
 
 	/* reset last CF frame rx timestamp for rx stmin enforcement */
 	so->lastrxcf_tstamp = ktime_set(0, 0);
 
-	/* start rx timeout watchdog */
-	hrtimer_start(&so->rxtimer, ktime_set(ISOTP_FC_TIMEOUT, 0),
-		      HRTIMER_MODE_REL_SOFT);
+	if (flowstatus == ISOTP_FC_CTS) {
+		/* cancel rxtimer before entering N_Ar phase */
+		hrtimer_cancel(&so->rxtimer);
+
+		/* enter N_Ar confirmation phase */
+		so->rx.state = ISOTP_WAIT_FC_TX_CONFIRM;
+		hrtimer_start(&so->fc_artimer,
+			      ktime_set(ISOTP_FC_AR_TIMEOUT, 0),
+			      HRTIMER_MODE_REL_SOFT);
+	}
+
+	can_send_ret = can_send(nskb, 1);
+	if (can_send_ret) {
+		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+			       __func__, ERR_PTR(can_send_ret));
+		if (flowstatus == ISOTP_FC_CTS) {
+			hrtimer_cancel(&so->fc_artimer);
+			so->rx.state = ISOTP_IDLE;
+			so->rx.len = 0;
+		}
+		dev_put(dev);
+		return 1;
+	}
+
+	dev_put(dev);
 	return 0;
 }
 
@@ -447,6 +483,9 @@ static int isotp_rcv_sf(struct sock *sk, struct canfd_frame *cf, int pcilen,
 	struct isotp_sock *so = isotp_sk(sk);
 	struct sk_buff *nskb;
 
+	if (so->rx.state == ISOTP_WAIT_FC_TX_CONFIRM)
+		hrtimer_cancel(&so->fc_artimer);
+
 	hrtimer_cancel(&so->rxtimer);
 	so->rx.state = ISOTP_IDLE;
 
@@ -481,6 +520,9 @@ static int isotp_rcv_ff(struct sock *sk, struct canfd_frame *cf, int ae)
 	int off;
 	int ff_pci_sz;
 
+	if (so->rx.state == ISOTP_WAIT_FC_TX_CONFIRM)
+		hrtimer_cancel(&so->fc_artimer);
+
 	hrtimer_cancel(&so->rxtimer);
 	so->rx.state = ISOTP_IDLE;
 
@@ -554,6 +596,13 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 	struct sk_buff *nskb;
 	int i;
 
+	if (so->rx.state == ISOTP_WAIT_FC_TX_CONFIRM) {
+		hrtimer_cancel(&so->fc_artimer);
+		so->rx.state = ISOTP_WAIT_DATA;
+		hrtimer_start(&so->rxtimer, ktime_set(ISOTP_FC_TIMEOUT, 0),
+			      HRTIMER_MODE_REL_SOFT);
+	}
+
 	if (so->rx.state != ISOTP_WAIT_DATA)
 		return 0;
 
@@ -855,9 +904,22 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
 	struct sock *sk = (struct sock *)data;
 	struct isotp_sock *so = isotp_sk(sk);
 	struct canfd_frame *cf = (struct canfd_frame *)skb->data;
+	int ae = (so->opt.flags & CAN_ISOTP_EXTEND_ADDR) ? 1 : 0;
 
-	/* only handle my own local echo CF/SF skb's (no FF!) */
-	if (skb->sk != sk || so->cfecho != *(u32 *)cf->data)
+	if (skb->sk != sk)
+		return;
+
+	/* FC echo handling: confirm FC was transmitted (N_Ar) */
+	if (so->rx.state == ISOTP_WAIT_FC_TX_CONFIRM &&
+	    (cf->data[ae] & 0xF0) == N_PCI_FC) {
+		hrtimer_cancel(&so->fc_artimer);
+		so->rx.state = ISOTP_WAIT_DATA;
+		hrtimer_start(&so->rxtimer, ktime_set(ISOTP_FC_TIMEOUT, 0),
+			      HRTIMER_MODE_REL_SOFT);
+		return;
+	}
+
+	if (so->cfecho != *(u32 *)cf->data)
 		return;
 
 	/* cancel local echo timeout */
@@ -1225,6 +1287,7 @@ static int isotp_release(struct socket *sock)
 	hrtimer_cancel(&so->txfrtimer);
 	hrtimer_cancel(&so->txtimer);
 	hrtimer_cancel(&so->rxtimer);
+	hrtimer_cancel(&so->fc_artimer);
 
 	so->ifindex = 0;
 	so->bound = 0;
@@ -1563,6 +1626,8 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg,
 					  isotp_rcv_echo, sk);
 		}
 
+		hrtimer_cancel(&so->fc_artimer);
+		so->rx.state = ISOTP_IDLE;
 		so->ifindex = 0;
 		so->bound  = 0;
 		release_sock(sk);
@@ -1639,6 +1704,8 @@ static int isotp_init(struct sock *sk)
 	hrtimer_setup(&so->txtimer, isotp_tx_timer_handler, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
 	hrtimer_setup(&so->txfrtimer, isotp_txfr_timer_handler, CLOCK_MONOTONIC,
 		      HRTIMER_MODE_REL_SOFT);
+	hrtimer_setup(&so->fc_artimer, isotp_fc_ar_timer_handler, CLOCK_MONOTONIC,
+		      HRTIMER_MODE_REL_SOFT);
 
 	init_waitqueue_head(&so->wait);
 	spin_lock_init(&so->rx_lock);
-- 
2.43.0