Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core (TCP Layer): Added bytes-alignment field in TCP Header to 8-byte-align the TCP user payload. #1050

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

FlorianReimold
Copy link
Member

This solves an issue with Capnproto. Capnproto requires the serialized data to be 8-bytes-aligned in memory. By already sending the payload 8-byte-aligned, this solves that issue.

Fixes #1017

…-align the TCP user payload.

This solves an issue with Capnproto. Capnproto requires the serialized data to be 8-bytes-aligned in memory. By already sending the payload 8-byte-aligned, this solves that issue.
Copy link
Contributor

@KerstinKeller KerstinKeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what I don't get here. It's 100% compatible with older senders / receivers, yet it doesn't work if you fill the memory with "/0".
So what does it actually append? If you know, you can manually fill the memory accordingly 😉
But also do not know if this is better or not.
Still hate this solution. And (latest for eCAL6) I vote for version numbers in the protocol, so ecal senders / receivers can negotiate a protocol version on all ecal layers (shm, udp, tcp, ...)

@rex-schilasky rex-schilasky merged commit 2955ab0 into master Apr 6, 2023
@FlorianReimold
Copy link
Member Author

This PR was untested and should not have been merged. I wanted to merge it after @chengguizi tested it, as mentioned in #1017.
I will re-open the issue that was closed by this PR.

@FlorianReimold FlorianReimold deleted the feature/tcp_payload_alignment branch July 18, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buffer memory not aligned in TCP mode (-> Capnproto issue)
3 participants