On 28.04.21 11:26, Oliver Hartkopp wrote:
On 28.04.21 11:12, Marc Kleine-Budde wrote:
On 28.04.2021 11:09:14, Marc Kleine-Budde wrote:
@@ -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.
I added some debug print in isotpsend:
diff --git a/isotpsend.c b/isotpsend.c
index 3ea574c..c2937fa 100644
--- a/isotpsend.c
+++ b/isotpsend.c
@@ -45,10 +45,11 @@
#include <libgen.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
#include <net/if.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/types.h>
@@ -252,10 +253,11 @@ int main(int argc, char **argv)
buf[buflen] = ((buflen % 0xFF) + 1) & 0xFF;
}
retval = write(s, buf, buflen);
+ printf("retval %d errno %d\n", retval, errno);
if (retval < 0) {
perror("write");
return retval;
}
$ date +%S.%N && ./isotpsend vcan0 -s 123 -d 321 -D 44 -b && date +%S.%N
43.269173590
retval 44 errno 0
44.271162277
So it waits for the timeout as required by the '-b' option - but the
errno is not set :-/
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
