All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Iso-tp status
       [not found]         ` <CAEQ58sjBCytryhrCQ1DYRcYpg=3-6DGvDtDzJVHV2mBcDa5NiA@mail.gmail.com>
@ 2017-03-05 19:55           ` Oliver Hartkopp
       [not found]             ` <CAEQ58sjaHg4-+E-RaFtwXdeZ71YCE_1LQDvGj6z2ssU_FERaXw@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2017-03-05 19:55 UTC (permalink / raw)
  To: Francesco Giancane; +Cc: linux-can

Hi Francesco,

On 03/03/2017 09:49 AM, Francesco Giancane wrote:
> I currently work in the automotive field and in particular in the
> diagnostics field. I am a Linux and oss enthusiasts and got by chance
> interest in can protocols.

Fine :-)

> I see that till now the Isotp L4 is still not
> supported mainline. I want to contribute so my question is can you give
> me some reference to the current status of the project? Are you planning
> to carry it on ?

Yes. Of course!

I updated the ISO-TP to the latest ISO 15765-2:2016 version for CAN FD - 
as I was part of the ISO standardization process.

> I read the docs and I saw that there are some open points.

Due to the update of ISO 15765-2:2016 the PDU length increased from 12 
bits (0 - 4095 byte) to 32(!) bit -> 4GB(!). See:

https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L102

That's why I was thinking about if/how to rearrange the code.
Probably the sendfile() syscall could be a way to go for PDU sizes > 
16kByte?!?

Some other open point is
https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L178

By now no one expects error values - and many people are using the 
implementation :-)

So maybe we need to introduce a new isotp option flag to enable error 
reporting.

Finally this

https://github.com/hartkopp/can-isotp-modules/blob/master/README.isotp#L234

is not a really good 'hack'. I wonder if it makes sense to throttle the 
transmission of outgoing CAN frames based on the ability of the CAN 
interface to cope with the traffic.

Besides these nitpicks I'm done for mainline :-)

Best regards,
Oliver

ps. putting linux-can ML on CC to be able to catch other peoples opinion 
too.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Iso-tp status
       [not found]               ` <CAEQ58sjOO=o-0s2y_BQAobNhEMo9X+=VDGfJcb10PcWmRs3q7Q@mail.gmail.com>
@ 2017-03-11 22:42                 ` Oliver Hartkopp
  2017-03-16 18:52                   ` Francesco Giancane
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2017-03-11 22:42 UTC (permalink / raw)
  To: Francesco Giancane; +Cc: linux-can, mauro.cerrato

Hi Francesco,

On 03/10/2017 05:10 PM, Francesco Giancane wrote:

> #1) What if we use conventional read/writes sys calls for supporting
> "old" ISO-TP specification up to 2^12 bytes, and using sendfile() +
> mmap() for source file descriptor greater than that? I think it would be
> unfeasible to statically allocate MAX_MSG_LENGTH to 4GB directly inside
> kernel space; maybe using zero copy would help us.

Yes. Something like this would be my intention too.
I would suggest to stay on the established read/write API e.g. up to 
8192 byte to be able to play with the new length format.

But for 'longer' PDUs sendfile & mmap would be cool.

What about pushing the current approach to mainline first and add the 
'long PDU' API in a second step?

> #2) Errors are good thing but as always developers prefer the Happy
> Path. I will double check standard socket() errors and see if there are
> already suitable matching codes.

As referenced in 
https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L178 
there is a possibility to detect some kind of timeout on protocol level.

But in fact ISO15765-2 is an unreliable protocol anyway (like UDP) and 
you never know whether the PDU was properly received at the data sink.

For that reason you need a timeout handling a layer above anyway - e.g. 
provided by an UDS application. So it's worth to discuss whether we 
really need to provide a TIMEOUT error by the socket API.

> #3) I wonder if setting an appropriate STmin parameter would fix this
> issue. (Mauro suggests STMin strictly greater than 0, to avoid CAN
> network flooding). Maybe also opening the socket in ISOTP mode should
> increase the txqueuelen.

Yeah - at least this reduces the problems. Adding an appropriate minimum 
N_As value depending on the minimum CAN frame transmission time for CAN 
or CAN FD.
https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L374

But still this reasonable default value does not solve the general 
problem of potential tx queue overflows. I wonder wether/how other 
network protocols handle this - but I did not take a look on this so far %-)

Thanks for your feedback & the discussion,
Oliver


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Iso-tp status
  2017-03-11 22:42                 ` Oliver Hartkopp
@ 2017-03-16 18:52                   ` Francesco Giancane
  0 siblings, 0 replies; 3+ messages in thread
From: Francesco Giancane @ 2017-03-16 18:52 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can, mauro.cerrato

Hi Oliver,
sorry for being late; thanks again for your feedback. Totally agree
for the first point, maybe let us just stay focused on being mainline
first - then we may think on how to send up to 4Gigs of data ;)

Being the ISO-TP unreliable, i think that the only meaningful
information may be whether the FF->FC exchange happened or not at
all... Maybe nobody is answering on the other side, so it is worth
informing the application at layer 7.

I have some good reference documentation by my side, it is the next
schedule to be read; in time I will have more accurate answers in the
form of which standard E* enumeration for errors maps good with which
case.

As always, I am available for helping & contributing to this project.
Let me know whether you need help :)

Bye,
Francesco Giancane

Via Paolo Braccini 46, 10141 Torino (TO)
+393933920466
https://it.linkedin.com/in/francesco-giancane-aa7213117



2017-03-11 23:42 GMT+01:00 Oliver Hartkopp <socketcan@hartkopp.net>:
> Hi Francesco,
>
> On 03/10/2017 05:10 PM, Francesco Giancane wrote:
>
>> #1) What if we use conventional read/writes sys calls for supporting
>> "old" ISO-TP specification up to 2^12 bytes, and using sendfile() +
>> mmap() for source file descriptor greater than that? I think it would be
>> unfeasible to statically allocate MAX_MSG_LENGTH to 4GB directly inside
>> kernel space; maybe using zero copy would help us.
>
>
> Yes. Something like this would be my intention too.
> I would suggest to stay on the established read/write API e.g. up to 8192
> byte to be able to play with the new length format.
>
> But for 'longer' PDUs sendfile & mmap would be cool.
>
> What about pushing the current approach to mainline first and add the 'long
> PDU' API in a second step?
>
>> #2) Errors are good thing but as always developers prefer the Happy
>> Path. I will double check standard socket() errors and see if there are
>> already suitable matching codes.
>
>
> As referenced in
> https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L178
> there is a possibility to detect some kind of timeout on protocol level.
>
> But in fact ISO15765-2 is an unreliable protocol anyway (like UDP) and you
> never know whether the PDU was properly received at the data sink.
>
> For that reason you need a timeout handling a layer above anyway - e.g.
> provided by an UDS application. So it's worth to discuss whether we really
> need to provide a TIMEOUT error by the socket API.
>
>> #3) I wonder if setting an appropriate STmin parameter would fix this
>> issue. (Mauro suggests STMin strictly greater than 0, to avoid CAN
>> network flooding). Maybe also opening the socket in ISOTP mode should
>> increase the txqueuelen.
>
>
> Yeah - at least this reduces the problems. Adding an appropriate minimum
> N_As value depending on the minimum CAN frame transmission time for CAN or
> CAN FD.
> https://github.com/hartkopp/can-isotp-modules/blob/master/net/can/isotp.c#L374
>
> But still this reasonable default value does not solve the general problem
> of potential tx queue overflows. I wonder wether/how other network protocols
> handle this - but I did not take a look on this so far %-)
>
> Thanks for your feedback & the discussion,
> Oliver
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-16 18:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAEQ58siD2MfV0=VFoGC4B67KcbJLtTFQJzAUFN19XF=AnVhhCQ@mail.gmail.com>
     [not found] ` <CAEQ58shwC09Q+TTAhG=dH7VjZ9RSQ5xsW8Z9tBhhbyN+3JKvAQ@mail.gmail.com>
     [not found]   ` <CAEQ58sj93WMMgyQVdBqUkaaH8fcwOwNZvwbwsdJ0zOR0dgjpKA@mail.gmail.com>
     [not found]     ` <CAEQ58sgZh==ZTB6HSzOWJzXv56pQZ+VEKPsWThYFHCUtzUHTdA@mail.gmail.com>
     [not found]       ` <CAEQ58sgmm5=UzxU_DcGj-qy+B+qpb1d9ko-GPxx75+D=8Mq4VA@mail.gmail.com>
     [not found]         ` <CAEQ58sjBCytryhrCQ1DYRcYpg=3-6DGvDtDzJVHV2mBcDa5NiA@mail.gmail.com>
2017-03-05 19:55           ` Iso-tp status Oliver Hartkopp
     [not found]             ` <CAEQ58sjaHg4-+E-RaFtwXdeZ71YCE_1LQDvGj6z2ssU_FERaXw@mail.gmail.com>
     [not found]               ` <CAEQ58sjOO=o-0s2y_BQAobNhEMo9X+=VDGfJcb10PcWmRs3q7Q@mail.gmail.com>
2017-03-11 22:42                 ` Oliver Hartkopp
2017-03-16 18:52                   ` Francesco Giancane

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.