Hello Marc and Guillaume,
thanks for picking this up here. I was busy the last days.
On 28.04.21 11:12, Marc Kleine-Budde wrote:
On 28.04.2021 11:09:14, Marc Kleine-Budde wrote:
From: "Sottas Guillaume (LMB)" <Guillaume.Sottas@xxxxxxxxxxxx>
Link: https://github.com/hartkopp/can-isotp/pull/43
Cc: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
Not-Signed-off-by: Sottas Guillaume (LMB) <Guillaume.Sottas@xxxxxxxxxxxx>
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---
Hello,
for better reviewing I'm posting Sottas's patch here. While porting to
current net/master I've fixed up the indention.
regards,
Marc
net/can/isotp.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 9f94ad3caee9..d02e8f1f240f 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -110,6 +110,11 @@ MODULE_ALIAS("can-proto-6");
#define ISOTP_FC_WT 1 /* wait */
#define ISOTP_FC_OVFLW 2 /* overflow */
+enum {
+ ISOTP_ERR_NO_ERROR = 0,
+ ISOTP_ERR_FC_TIMEOUT,
+};
+
enum {
ISOTP_IDLE = 0,
ISOTP_WAIT_FIRST_FC,
@@ -122,6 +127,7 @@ struct tpcon {
int idx;
int len;
u8 state;
+ u8 error;
u8 bs;
u8 sn;
u8 ll_dl;
@@ -756,6 +762,10 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
sk->sk_err = ECOMM;
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_error_report(sk);
I think the idea is that sk_error_report takes care of propagation of
the error to the user space. I don't know why it's not working as
expected.
Yes, I fact I was thinking about this question too.
+ /* set error flag in order to consume it later in the
+ * isotp_sendmsg function */
+ so->tx.error = ISOTP_ERR_FC_TIMEOUT;
+
/* reset tx state */
so->tx.state = ISOTP_IDLE;
wake_up_interruptible(&so->wait);
@@ -954,6 +964,13 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (wait_tx_done) {
/* wait for complete transmission of current pdu */
wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+
+ /* check if an error has been raised in the timer
+ * function handler */
+ if (so->tx.error == ISOTP_ERR_FC_TIMEOUT) {
+ so->tx.error = ISOTP_ERR_NO_ERROR;
+ return -ECOMM;
Maybe the way to trigger the sk_error_report(sk) we might return '-1'
while the error is then propagated inside errno.
But I haven't checked that myself so far.
The reason why this return value is not that important is that ISO-TP is
an (unreliable!) UDP like protocol where the application checks for an
application response timeout.
Having the information of a FF timeout only makes sense when
CAN_ISOTP_WAIT_TX_DONE is enabled to assign the timeout to a that
specific PDU.
Best,
Oliver
+ }
}
return size;
@@ -1371,6 +1388,9 @@ static int isotp_init(struct sock *sk)
so->tx.state = ISOTP_IDLE;
hrtimer_init(&so->rxtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
+
+ so->tx.error = ISOTP_ERR_NO_ERROR;
+ so->rx.error = ISOTP_ERR_NO_ERROR;
so->rxtimer.function = isotp_rx_timer_handler;
hrtimer_init(&so->txtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
so->txtimer.function = isotp_tx_timer_handler;
Marc
