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
248 stars 71 forks source link

Address extension in one direction but not the other #62

Closed pylessard closed 10 months ago

pylessard commented 10 months ago

I was implementing a feature requested by a user on the python-can-isotp module where one can send using a 11 bits ID and receive with a 29 bits ID. I generalized the idea and let the user sends with any kind of ISOTP address and receive with any kind of ISOTP address.

By doing so, I noticed that the isotp socket cannot use an address extension in a direction without using one in the other direction. I do not have any use case for that scenario, but I wonder if it should be allowed since it would be the most flexible solution.

Have a look at what I was hoping to achieve

But I had to add this check

Any opinion on this?

hartkopp commented 10 months ago

I was implementing a feature requested by a user on the python-can-isotp module where one can send using a 11 bits ID and receive with a 29 bits ID.

I had to double check this myself - but the kernel implementation already supports this 11/29 bit mix.

Interesting that there are really people working with such weird setups.

I generalized the idea and let the user sends with any kind of ISOTP address and receive with any kind of ISOTP address.

By doing so, I noticed that the isotp socket cannot use an address extension in a direction without using one in the other direction. I do not have any use case for that scenario, but I wonder if it should be allowed since it would be the most flexible solution.

So far I've seen only two use cases that use the address extension. And in these use cases it is just clear that AE is enabled or not (on the connection level).

Have a look at what I was hoping to achieve

But I had to add this check

Any opinion on this?

Of course it would be no big deal to adapt the kernel code in that way - but I checked the options and have the feeling that the sockopt API with additional flags would become even more confusing for the user then.

As I also do not see a real use case for this feature right now I would tend to raise the discussion again, when someone really shows up with this error message in his hands: "The IsoTP socket module does not support asymmetric addresses with inconsistent address_extension byte" ;-)

My assumption is that no one will ever show up with it ¯\_(ツ)_/¯

pylessard commented 10 months ago

that's what I believe too. I guess we should close this issue. I will add a comment that points to it in my code base.

Thanks for your input