[PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Marc Kleine-Budde]

Link: https://github.com/hartkopp/can-isotp/pull/43
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hey Sottas,

can you check if this fixes your problem and that normal operation
still works.

regards,
Marc

 net/can/isotp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 9f94ad3caee9..823f047a28ad 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -954,6 +954,9 @@ 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);
+
+		if (sk->sk_err)
+			return sk->sk_err;
 	}
 
 	return size;
-- 
2.30.2

Re: [PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Oliver Hartkopp]

On 06.05.21 15:36, Marc Kleine-Budde wrote:
> Link: https://github.com/hartkopp/can-isotp/pull/43
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hey Sottas,
> 
> can you check if this fixes your problem and that normal operation
> still works.
> 
> regards,
> Marc
> 
>   net/can/isotp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 9f94ad3caee9..823f047a28ad 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -954,6 +954,9 @@ 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);
> +
> +		if (sk->sk_err)
> +			return sk->sk_err;

That's a nice simple improvement.

But it has to be a negative value:

	return -sk->sk_err;

Your patch produces this:

$ date +%S.%N && ./isotpsend vcan0 -s 123 -d 321 -D 44 -b && date +%S.%N
55.587968123
retval 70 errno 0
wrote only 70 from 44 byte
56.594017244

With the negative value you get:

$ date +%S.%N && ./isotpsend vcan0 -s 123 -d 321 -D 44 -b && date +%S.%N
22.989331212
retval -1 errno 70
write: Communication error on send

Which we would need.

Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>

Many thanks,
Oliver

Re: [PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 552 bytes --]

On 5/6/21 7:36 PM, Oliver Hartkopp wrote:
>> can you check if this fixes your problem and that normal operation
>> still works.

[...]

> That's a nice simple improvement.
> 
> But it has to be a negative value:

I've send v2 that fixes the problem.

regards,
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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Marc Kleine-Budde]

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

On 19.10.2021 04:51:18, Sottas Guillaume (LMB) wrote:
> Hope you are doing fine?

Thanks, yes :)

> I was wondering if this patch were integrated in the mainline? I
> searched around but I've not be able to find it.

Oh, that somehow slipped my fingers. I'm about to send a pull request,
hopefully this patch gets into v5.15.

> Thank you in advance for your feedback,

regards,
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 RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Marc Kleine-Budde]

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

On 07.05.2021 16:12:49, Oliver Hartkopp wrote:
> Hi Marc,
> 
> On 07.05.21 14:50, Marc Kleine-Budde wrote:
> > On 07.05.2021 12:29:33, Sottas Guillaume (LMB) wrote:
> > > Indeed the first version (with positive value) does not work, but I
> > > just tested the V2 of the patch, and work like a charm. Let me know if
> > > I can help to get it integrated
> > 
> > You can send you Tested-by to that v2 patch, like Oliver did to the v1.
> 
> Of course I did the test to my adapted version which now became the v2 ;-)

I interpreted your post that way :D

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 RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Oliver Hartkopp]

Hi Marc,

On 07.05.21 14:50, Marc Kleine-Budde wrote:
> On 07.05.2021 12:29:33, Sottas Guillaume (LMB) wrote:
>> Indeed the first version (with positive value) does not work, but I
>> just tested the V2 of the patch, and work like a charm. Let me know if
>> I can help to get it integrated
> 
> You can send you Tested-by to that v2 patch, like Oliver did to the v1.

Of course I did the test to my adapted version which now became the v2 ;-)

Best,
Oliver

> The patch will be part of my next pull request to net/master.
> 
> regards,
> Marc
> 

Re: [PATCH RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Marc Kleine-Budde]

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

On 07.05.2021 12:29:33, Sottas Guillaume (LMB) wrote:
> Indeed the first version (with positive value) does not work, but I
> just tested the V2 of the patch, and work like a charm. Let me know if
> I can help to get it integrated

You can send you Tested-by to that v2 patch, like Oliver did to the v1.
The patch will be part of my next pull request to net/master.

regards,
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 RFC] can: isotp: isotp_sendmsg(): return error on FC timeout on TX path

[Sottas Guillaume (LMB)]

Hello Marc,

Indeed the first version (with positive value) does not work, but I just tested the V2 of the patch, and work like a charm.
Let me know if I can help to get it integrated

Best regards,

Guillaume