JulianSchmid/etherparse

Limit the size of `Ipv4Extensions`/`Ipv6Extensions`

Opened this issue · 1 comments

These structs have large buffers as it contains arrays that are allocated on the stack, this pushes the size of the IpHeader struct at over 9kB (and therefore PacketImpl and PacketBuilderStep along with it).

dbg!(std::mem::size_of::<Option<IpHeader>>());
// std::mem::size_of::<Option<IpHeader>>() = 9280

This is not a big issue in itself, but it ended up causing a stack overflow in one of our test files where we build a bunch of packets to check different scenarios in a single chunk of code. This is not helped by the fact that rustc/LLVM don't seem to be able to reuse stack space after dropping any reference to PacketBuilderStep as soon as possible, at last using rustc 1.65 without optimizations.

Given that extensions are rarely used, maybe they should be stored as Vec instead of statically sized arrays, or IpHeader enum could be defined as:

pub enum IpHeader {
    Version4(Ipv4Header, Box<Ipv4Extensions>),
    Version6(Ipv6Header, Box<Ipv6Extensions>)
}

I'd be happy to try making a PR if this is considered to be an issue.

Hi @jdesgats ,

Yeah that was always in the back of my mind as an issue, especially for embedded targets. It is a valid point and an issue & weakness of the crate.

The problem is that I tried to stay clear of any kind of syscall causing allocation in the crate. Boxing the extension headers is a guaranteed allocation, which I tried to avoid as much as possible. Especially to support embedded targets (oh the irony, I know).

But I think there is a lot of use cases where allocations are a good idea (e.g. #40 ) and I am thinking about adding a feature flag to the crate to allow users to opt-in to "allocations". But I have no clear idea yet how this should look like.

So sorry, I have no clear idea how to proceed here (yet).

Greets
Julian