add memory alignment for ARM32 #4208
Conversation
| @@ -76,7 +76,7 @@ typedef struct DATAPATH_RX_IO_BLOCK { | |||
|
|
|||
| } DATAPATH_RX_IO_BLOCK; | |||
|
|
|||
| typedef struct DATAPATH_RX_PACKET { | |||
| typedef struct __attribute__((aligned(16))) DATAPATH_RX_PACKET { | |||
There was a problem hiding this comment.
Can you add more info in your PR description as to why this helps and is necessary?
There was a problem hiding this comment.
Not 100% sure yet, so trying to run automation first
There was a problem hiding this comment.
I did play with this little bit more and it is also sufficient (as far as I can tell) to increase RecvBlockStride
msquic/src/platform/datapath_epoll.c
Lines 333 to 334 in fbef931
I originally added +4 but it also works for me with +1 or +2. I sort of expected corrupted pointer if something overrides the location.
It seems like we are casting bunch of packets to single big buffer, right @nibanks? Are we positive the structures always land on aligned boundary? It seems like the issue is triggered only when we pack multiple packets into single buffer. In the test we look at with @ami-GS, there is single packet and then we have few together. As @ami-GS noticed it was always crashing on processing third one, e.g. first that is packed second in the buffer.
There was a problem hiding this comment.
@ami-GS can we shoot to get this merged this week? Please address my comment above, and associate this PR with the bug that's tracking the issue.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4208 +/- ##
==========================================
+ Coverage 84.22% 85.27% +1.04%
==========================================
Files 56 56
Lines 15358 15358
==========================================
+ Hits 12935 13096 +161
+ Misses 2423 2262 -161 ☔ View full report in Codecov by Sentry. |
Description
Fixes #3958
Support arm32 memory alignment
The fix was for an alignment problem -> arm is sensitive in places where x64 is not.
Testing
Tested and passed on raspberry pi.
It had been failed at processing 3rd or 4th packets consistently (during handshake). now it has gone with the fix.
Documentation
Is there any documentation impact for this change?