Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catching missing Flow Control Frame during data transmission #42

Closed
Sauci opened this issue Mar 30, 2021 · 1 comment
Closed

Catching missing Flow Control Frame during data transmission #42

Sauci opened this issue Mar 30, 2021 · 1 comment

Comments

@Sauci
Copy link

Sauci commented Mar 30, 2021

This issue was opened after an exchange of emails with @hartkopp

Extract of the e-mail sent to @hartkopp

I had a question concerning the behavior when sending a multi-frame packet. When I (client side) send a large chunk of data to the server (ECU), and the server does not send a Flow Control frame back to the client, the write(2) operation does not return any clue about this misbehaving transaction (no error returned, and the number of bytes sent corresponds to the size of the packet).
I wanted to know if there is a way to know when the client enters in such a state.

I digged into the examples but did not found such a use-case.

extract of the reply from @hartkopp

When there's no FC we run into a timeout here:

https://elixir.bootlin.com/linux/v5.11.10/source/net/can/isotp.c#L751

I wanted to know if there is a way to know when the client enters in
such a state.

Good point. Usually you catch these errors by monitoring for a timeout on application(!) level as ISO-TP is an unreliable datagramm protocol anyway (like UDP).

Btw. there is a 'blocking write' feature with the CAN_ISOTP_WAIT_TX_DONE flag. This only returns from the write call when either the transmission is completed OR there has been a problem (e.g. timeout).

In the 'problem' case the sk_error is set.

But looking at the code here ...

https://elixir.bootlin.com/linux/v5.11.10/source/net/can/isotp.c#L955

... it always seems to return the original length value. And I did not test how the sk_error is propagated to the user.

Hm - maybe we can both take a look at this.

Did you try CAN_ISOTP_WAIT_TX_DONE?

Without CAN_ISOTP_WAIT_TX_DONE the errors can not be assigned to a specific PDU.

@Sauci
Copy link
Author

Sauci commented Mar 31, 2021

Setting the CAN_ISOTP_WAIT_TX_DONE doesn't indeed affect the return value of write(2). Bellow a sample code illustrating this behavior:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include <net/if.h>
#include <linux/can.h>
#include <linux/can/isotp.h>

static const u_int8_t tx_buffer[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

int main() {
    int status = 0;

    int fd;
    struct sockaddr_can address;
    static struct can_isotp_options isotp_options;
    static struct can_isotp_ll_options isotp_ll_loptions;

    address.can_family = AF_CAN;
    address.can_addr.tp.rx_id = 0x98DAF900u;
    address.can_addr.tp.tx_id = 0x98DA00F9u;
    address.can_ifindex = (signed) if_nametoindex("can1");

    isotp_options.flags = CAN_ISOTP_DEFAULT_FLAGS | CAN_ISOTP_WAIT_TX_DONE;

    isotp_ll_loptions.mtu = 16u;
    isotp_ll_loptions.tx_dl = 8u;
    isotp_ll_loptions.tx_flags = 0u;

    if ((fd = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP)) >= 0) {
        if ((setsockopt(fd, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &isotp_options, sizeof(isotp_options))) >= 0) {
            if ((setsockopt(fd, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &isotp_ll_loptions, sizeof(isotp_ll_loptions))) >= 0) {
                if ((bind(fd, (struct sockaddr *)&address, sizeof(address))) >= 0) {
                    ssize_t result = write(fd, &tx_buffer, sizeof(tx_buffer) / sizeof(tx_buffer[0]));
                    printf("result = %d", result);
                } else {
                    status = 4;
                }
            } else {
                status = 3;
            }
        } else {
            status = 2;
        }
    } else {
        status = 1;
    }

    return status;
}

This executable outputs:

/home/pi/isotp/cmake-build-debug-rpi/isotp
result = 10
Process finished with exit code 0

While the can dump utility outputs:

$ candump can1
  can1  18DA00F9   [8]  10 08 01 02 03 04 05 06

Should the call to write(2) return a negative value? Or should it return the number of byte effectively written (in this case 6) if the writing operations are configured in blocking mode trough CAN_ISOTP_WAIT_TX_DONE option?

Sauci pushed a commit to Sauci/can-isotp that referenced this issue Apr 1, 2021
fengguang pushed a commit to 0day-ci/linux that referenced this issue Oct 19, 2021
When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
2020147570 pushed a commit to 2020147570/linux that referenced this issue Oct 24, 2021
When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Whissi pushed a commit to Whissi/linux-stable that referenced this issue Oct 27, 2021
commit d674a8f upstream.

When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
woodsts pushed a commit to woodsts/linux-stable that referenced this issue Oct 27, 2021
commit d674a8f upstream.

When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
it-is-a-robot pushed a commit to openeuler-mirror/kernel that referenced this issue Nov 16, 2021
stable inclusion
from stable-5.10.76
commit 0917fb04069a51c5458a5ca95b7859de88cb61b8
bugzilla: 182988 https://gitee.com/openeuler/kernel/issues/I4IAHF

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=0917fb04069a51c5458a5ca95b7859de88cb61b8

--------------------------------

commit d674a8f upstream.

When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
Acked-by: Weilong Chen <chenweilong@huawei.com>

Signed-off-by: Chen Jun <chenjun102@huawei.com>
Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com>
vinzv pushed a commit to tuxedocomputers/linux that referenced this issue Dec 1, 2021
BugLink: https://bugs.launchpad.net/bugs/1952136

commit d674a8f upstream.

When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
hartkopp added a commit that referenced this issue Dec 1, 2021
When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Upstream commit d674a8f123b4

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Link: #42
Link: #43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
@hartkopp hartkopp closed this as completed Dec 1, 2021
Rohail33 pushed a commit to Rohail33/Realking_xiaomi_xaga that referenced this issue Dec 31, 2022
commit d674a8f123b4096d85955c7eaabec688f29724c9 upstream.

When the a large chunk of data send and the receiver does not send a
Flow Control frame back in time, the sendmsg() does not return a error
code, but the number of bytes sent corresponding to the size of the
packet.

If a timeout occurs the isotp_tx_timer_handler() is fired, sets
sk->sk_err and calls the sk->sk_error_report() function. It was
wrongly expected that the error would be propagated to user space in
every case. For isotp_sendmsg() blocking on wait_event_interruptible()
this is not the case.

This patch fixes the problem by checking if sk->sk_err is set and
returning the error to user space.

Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol")
Link: hartkopp/can-isotp#42
Link: hartkopp/can-isotp#43
Link: https://lore.kernel.org/all/20210507091839.1366379-1-mkl@pengutronix.de
Cc: stable@vger.kernel.org
Reported-by: Sottas Guillaume (LMB) <Guillaume.Sottas@liebherr.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants