Page MenuHomeFreeBSD

netinet: Decorate IPv4 structures used for byte buffer overlays as packed.
Needs ReviewPublic

Authored by mmel on Thu, May 1, 9:08 AM.

Details

Reviewers
kib
markj
kp
glebius
Group Reviewers
transport
Summary

The C language only allows pointer casting to another type if both sides have
compatible alignments, unaligned casts causes undefined behavior.
Since we do not have declared (and therefore not checked) mbuf alignments for
the various input functions in the IP stack, the worst case (alignment to char*)
should be expected.

A lot of work still needs to be done on IPv6, especially on the terrible accesses
to in6_addr members.

It should have no performance impact on all unaligned architectures.

MFC after: 1 month

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mmel requested review of this revision.Thu, May 1, 9:08 AM

I do not have objections against adding the __packed annotations. But the connection between the annotations and the (true) claim in the first paragraph of the summary is not clear to me. __packed only eliminates padding, but I believe that the structures already have no padding on any supported arch.

I think that __packed attribute also forces the object to be aligned to 1, so it begins conform with the C language rules. I think this is the only way to reduce the alignment of an object -> __attribute__((packed, aligned(x)))

I think that __packed attribute also forces the object to be aligned to 1, so it begins conform with the C language rules. I think this is the only way to reduce the alignment of an object -> __attribute__((packed, aligned(x)))

Yes, packed + aligned(1). __packed alone is not enough.

I disagree, __packed resets the alignment to 1, so __attribute__((aligned(x))) can increase it. Imho, internally, __packed does nothing but reset the alignment of all members to 1 . Everything else (padding, size) is derived from that.
see: https://developer.arm.com/documentation/dui0472/m/Compiler-specific-Features/--attribute----packed---variable-attribute

printf("raw: %lu, packed: %lu\n", __alignof(struct a {int b;}), __alignof(struct b {int b;}__packed));

gives me "raw: 4, packed: 1 (on armv7)"