Things to bear in mind while upstreaming struct packet changes

[kbara]

• The 'headroom' abstraction or the size of the underlying data buffer has to change. (Either the data buffer becomes 10k+max_headroom, or we export the size as 10k-max_headroom), to avoid introducing buffer overflows.
  -- Rationale: code which does bounds-checking on the constant representing the 10000 byte payload size otherwise needs to be headroom-aware. Merely using functions like packet.append is not a solution; there is no standardized way of returning errors, just asserts on overflows. Appending 9999 bytes to an empty packet after checking that 9999 is less than the exported data buffer size should be an operation that works.
• The 'data' pointer abstraction is dangerous. See IPv6 reassembly + new packet definitions failed (solved: added a new packet clone method) #393 . There is no facility to clone packets to new memory (for good reasons - see Merge max-next into next snabbco/snabb#998 (comment) ). An idiom that works on 2-element packets is to simply ffi.copy the whole struct - but this in turn means that the data pointer in the copy is pointing to memory in the original! (See IPv6 reassembly + new packet definitions failed (solved: added a new packet clone method) #393 ).
  -- We either need to:
  --- get rid of the 'data' pointer abstraction
  --- have a facility to clone packets to memory (with all the complication this entails - unlikely to be the right approach)
  --- accept that copying the struct as a whole is going to lead to incorrect code (likely including code that has already been written) unless the caller is aware of the current definition of struct packet and its implications
  --- as a consequence of the third option, require that code either be aware of struct packet details, or make heavier use of clone+free in contexts where conceptually, clone_to_memory is needed. Perhaps this is a rare enough case to be workable... :/

[lukego]

Just want to promote my pet idea that might support headroom without modifying the packet struct:

Could allocate memory for packets on a 512-byte aligned address and then put the struct packet at address+256. Then you would have 256 bytes of both headroom and tailroom. You could relocate the packet left or right by simply copying the non-data portion of the struct packet in place and returning a new pointer. You could also check how much headroom/tailroom you have left by looking at the alignment of the current struct packet address.

This way the packet struct would look the same and we would only be adding some incremental complexity to the "how we allocate packet memory" invariants.

[kbara]

In practice, I think that's more or less the form we all want; the above issue is largely about documenting features to avoid in our current implementation as we move towards upstreaming something that fills this conceptual space. The point about exported buffer capacity needing to be smaller than the symbolically exported one to avoid buffer overflows regardless of the current amount of headroom/tailroom without careful fiddly work by all application developers still applies, though!

[kbara]

I'm closing this, as it's resolved with @wingo 's implementation of @lukego 's idea.