Skip to content

perf: Don't call read_unaligned in getter_be#84

Draft
vadorovsky wants to merge 3 commits into
mainfrom
faster-getters
Draft

perf: Don't call read_unaligned in getter_be#84
vadorovsky wants to merge 3 commits into
mainfrom
faster-getters

Conversation

@vadorovsky

@vadorovsky vadorovsky commented Feb 6, 2026

Copy link
Copy Markdown
Owner

It unnecessarily adds a redundant copy, especially on little-endian targets, where call to swap_bytes (that calls the bswap intrinsic) already makes an another copy.


This change is Reviewable

vadorovsky and others added 3 commits February 6, 2026 14:18
All the variants carried by them have the same size (4 bytes). Given
than using C-style unions in Rust makes it difficult to use
serialization libraries, remove them and instead define the `data`
fields as `[u8; 4]`, then implement our own (safe) abstraction that
allow users to check and access the appropriate variants (depending
on the ICMP type). Keep the unsafe variants public as well.
It unnecessarily adds a redundant copy, especially on little-endian
targets, where call to `swap_bytes` (that calls the `bswap` intrinsic)
already makes an another copy.
@pythops

pythops commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator

@aureliar8 this is somehow similar to the PR you have opened and closed about the alignment of 4

@aureliar8

Copy link
Copy Markdown
Contributor

Yeah it's similar to what I tried to do in #73

But nowadays I don't think #align(4) is a good idea. Because we can't assume that all of those header will be 4-aligned in an ebpf context.

Yes the kernel will make sure that when creating a packet buffer, the first IP header is 4-aligned.
But in case where there's packet encapsulation, the user can try to inspect a inner header that is not 4-aligned.

In theory all sane encapsulation protocol make that the inner packets are all 2-aligned, so we could simply update this PR to use #align(2)

But the more I think about it the more I consider that for the ebpf usecase, the alignment can not be a property of the type itself: it's a property of the instance of this type. I think the most flexible approach would be that it's up to the caller to decide if he wants to do an aligned-read or an unaligned read. Because he's the only one to know where in the packet he is looking.

This means increasing the API surface of those type quite significantly and we'd need to duplicate each get_xxx method with its counterpart get_xxx_unaligned. But maybe if we let the default to aligned operation 99% of user will not care and never read for the unaligned version ?

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.

3 participants