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

path towards mainline? #28

Closed pdp7 closed 3 years ago

pdp7 commented 4 years ago

Hi @hartkopp, I'm not sure if this is the proper venue to ask, but are there any plans for mainline?

thanks!

hartkopp commented 4 years ago

Hi Drew! Yes, in fact I still have your request on the mailing list on my TODO list. I did some updates on a branch (4.17+) https://github.com/hartkopp/can-isotp/commits/4.17%2B that removes the Linux version dependent code and makes use of the more recent soft hrtimers. My problem is to get some interested people to test this latest version.

Additionally I was thinking about some 'blocking write' semantic: By now the PDU is pushed into kernel space and the write() syscall returns immediately. Only the second write() call is blocked when the first PDU is still transmitted.

I wonder if it makes sense to change the semantic to have a blocking write that returns, when the PDU has been sent?!?

Do you know someone who can test the 4.17+ branch with the latest kernel?

Best, Oliver

pdp7 commented 4 years ago

Do you know someone who can test the 4.17+ branch with the latest kernel?

Best, Oliver

Hi Oliver!

Do you mean compile these kernel modules against mainline?

I could try that on the PocketBeagle (AM3358). I'm already running mainline on that. I believe I can setup two PocketBeagle's with the Macchina P1 adapters and do a test over the ODB cable.

hartkopp commented 4 years ago

Hi Drew, the 4.17+ branch compiles well with the latest 5.8-rc3. I can test the changes (hrtimer/returnval) on my own - but it would be cool if you would have some real world application (e.g. performing UDS diagnosis) to check if there is any difference in behaviour.

pdp7 commented 4 years ago

@hartkopp good to hear, I can create a build for the PocketBeagle (AM3358) and send it to the Macchina people so that they could test with vehicles.

hartkopp commented 4 years ago

Excellent! Many people have been using the implementation (in its older version) but I really did not get that much feedback. This can be good - as it works - or bad in the way that people see issues but (accidentally) don't report them back. Thanks for testing!

pschichtel commented 4 years ago

I just built the 4.17+ on my system and successfully ran a few basic tests against it. I could set it up on my Pi (PiCAN 2 / MCP2515) and test it against a few cars.

hartkopp commented 4 years ago

Thanks Phillip! I did some more cleanups and added some socket error reporting to the 4.17+ branch - which now needs the next Linux 5.9 as base revision. I plan to post the patch when the 5.9 merge window closes and net-next opens (next week).

pschichtel commented 4 years ago

Just yesterday I got my stack running again. I still have to fix up some infrastructure, but a first run in my car went well. Once I have the infrastructure sorted, I'll get some more cars tested. I also haven't exactly looked into the stuff I'm reading (I'm sampling OBD parameters), but parsing went fine so far, so I'm fairly confident this is not getting any garbled data and I'm not seeing any timeouts or bad performance either.

Kernel 5.9 would generally be fine for me, but I currently don't have any cross-compilation setup yet. So I'll have to look into that, because it'll be a while before Arch ARM has a prebuilt rpi3 kernel 5.9.

hartkopp commented 4 years ago

Great! Thanks for testing! I only updated the code for 5.9 for the mainline process. You can complie it for older kernels when you just revert this patch: https://github.com/hartkopp/can-isotp/commit/98986fc2f07cb5c7fa73a86d9b4b3ae417d35d79

I did not find any issues either and added some more error notifications in case of timeouts or bad message reception.

pschichtel commented 3 years ago

So given that this will now eventually land in mainline, but with default n, the next item on the list would probably be to get in touch with distribution maintainers to get this enabled.

hartkopp commented 3 years ago

Yes. As you might have read from the discussion here https://lore.kernel.org/linux-can/20201010084421.308645a2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/ I really tried my very best ;-)

Do you have any pointers whom to contact at the different distributions, e.g. for Debian/RH?

pschichtel commented 3 years ago

No, not yet. I'll personally be going after the Arch (ARM) maintainers. I think the Raspberry PI guys over at https://github.com/raspberrypi/linux are fairly open to PRs for this kind of stuff, which might be a good start, given that many other arm distros pull this kernel for the raspberry pi 3.

hartkopp commented 3 years ago

There is a Debian Kernel Mailing List which is probably a better place as Raspian usually adopts the Debian defaults, right? I'll try to show up there when 5.10-rc1 has been tagged.

hartkopp commented 3 years ago

Yeah! @pdp7

pdp7-isotp

pdp7 commented 3 years ago

@hartkopp thanks for all work!

hartkopp commented 3 years ago

Thanks to @ukleinek Linux 5.10+ Debian kernels have enabled CAN_ISOTP by default! https://deb.li/3xVH7 I assume this will have an impact on Raspian too. \o/

pschichtel commented 3 years ago

Arch is also building 5.10 with CAN_ISOTP by default: https://github.com/archlinux/svntogit-packages/commit/282c90d1e14ef7dd0f56f8c8192ba1c830e906ad#diff-6503b30d816e211d1909eed3e1eff5f9105fdc48f5735921a59f4906f99415beR1864