erpc icon indicating copy to clipboard operation
erpc copied to clipboard

FramedTransport::Header not serialized?!

Open infn-ke opened this issue 4 years ago • 8 comments

Why is not the erpc header (FramedTransport::Header) serialized? E.g. do htons on the 16 bit members m_messageSize and m_crc.

infn-ke avatar Feb 12 '21 11:02 infn-ke

I experienced problems due to lack of serialized header, and added some code to framed transport class to serialize header. I can share the code on request.

ierturk avatar Feb 17 '21 12:02 ierturk

Maybe you can raise a PR so that it can be included in master branch of erpc? I don't want to maintain a fix on my own.

infn-ke avatar Feb 17 '21 12:02 infn-ke

Hi, Could you explain why you need serialize framedTransport header? Currently it is send to other side before serialized message as it contains extra data in compare with other possible transports.

Hadatko avatar Feb 17 '21 12:02 Hadatko

Hi @Hadatko , using the term serialize may be wrong here. Sometimes the transferred header may be corrupted due to lack of frame and CRC. So I have to add an extra frame and CRC to check header integrity into header.

ierturk avatar Feb 17 '21 12:02 ierturk

So the issue is related to sending receiving message size. As second receive call is using size from first call. Sounds reasonable to me to have crc of header.

Hadatko avatar Feb 17 '21 12:02 Hadatko

Yes, it is related to message size. The second side wait for message with incorrect size. So the transfer is aborted if there is a timeout for the transfer. In our particular situation, the content of message were coded as ASCII and added framing characters. May be the measures other than CRC are not needed.

ierturk avatar Feb 17 '21 13:02 ierturk

@Hadatko Why is it not needed to serialize and deserialize header? Representing header in network byte order would be the only way to let little-endian and big-endian exchange header before receiving the payload.

infn-ke avatar Feb 17 '21 13:02 infn-ke

So we have two issues here. Serializing header and validate header. If you can create PR for both of them it should be great ;) Thank you guys.

Hadatko avatar Feb 17 '21 14:02 Hadatko

I will take a look on theese.

Hadatko avatar Aug 28 '23 06:08 Hadatko

Didn't it solved enough at #276 ? If someone want he can use htons as implementation of the endiannes agnostic defines. Maybe we can consider add such implementation.

amgross avatar Aug 28 '23 08:08 amgross

How is that PR solving CRC of header data?

Hadatko avatar Aug 28 '23 08:08 Hadatko

https://github.com/EmbeddedRPC/erpc/blob/b3951d834d446b87e6e9a3b47dca71c36f5b37cf/erpc_c/infra/erpc_framed_transport.cpp#L128

amgross avatar Aug 28 '23 08:08 amgross

@amgross :D That is crc of next message (serialized data). But there is no check if sended header data are correct. From what i understand here we need add crcHeader into Header struct. This one will be computed from header crc and length. Other side then need firstly check header crc and then receive body and check body crc. This way you know that length and crc in header struct was received correctly

Hadatko avatar Aug 28 '23 09:08 Hadatko

The first message here talked about 'E.g. do htons on the 16 bit members m_messageSize and m_crc', so I thought we are talking on endianness issue

amgross avatar Aug 28 '23 10:08 amgross

I think @infn-ke (which opened the issue) talked here on endianness and @ierturk talked about length issue (which I also faced once)

amgross avatar Aug 28 '23 11:08 amgross