search

hartkopp / can-isotp

Linux Kernel Module for ISO 15765-2:2016 CAN transport protocol PLEASE NOTE: This module is part of the mainline Linux kernel since version 5.10
Other
239 stars 69 forks source link

ISOTP module does not ignore the priority bits (high 3 bits of extended ID) #63

Closed ndushman closed 6 months ago

ndushman commented 6 months ago

Per ISO15765-2 "The priority field should be masked off by the receiver (ignored)." The ISOTP module doesn't do this, so if a node responds with a different priority than the one expected, communication fails. Looks like the macro at https://github.com/hartkopp/can-isotp/blob/master/net/can/isotp.c#L107 needs to be adjusted. I can put up a PR for this, but I'm not sure what the best way to do it is, while maintaining consistency with the other preprocessor definitions - thoughts?

hartkopp commented 6 months ago

This reference from Annex A is a J1939 addressing topic - you can check for the CAN_J1939 protocol inside the kernel, see priority handling here:

https://github.com/torvalds/linux/blob/master/net/can/j1939/main.c#L330

The native ISO15765-2 protocol is a point-to-point transport protocol on two single CAN Identifiers, not to a bunch of CAN IDs.

ndushman commented 6 months ago

Thanks for the quick reply! I did not know there was J1939 support within the kernel. I'm not sure we can use it on our old kernel (3.18) but I'll look into it.

hartkopp commented 6 months ago

You should really try to upgrade your 3.18 (Android??) kernel! The J1939 implementation had several implementation and integration steps until it was upstreamed for the mainline kernel. So it could be that you find an older version, which probably fits into the 3.18, but which might have an outdated user-space socket API.

ndushman commented 6 months ago

Looks like as a quick fix, patching the ISOTP module locally to mask off the J1939 priority bits works, and we'll look into a kernel upgrade. Thanks again for your help!