On Thu, 13 Feb 2025 14:07:22 -0500
Jon Maloy <jmaloy(a)redhat.com> wrote:
On 2025-02-12 16:39, Jon Maloy wrote:
On 2025-02-10 19:55, David Gibson wrote:
> On Sun, Feb 09, 2025 at 12:00:56PM -0500, Jon Maloy wrote:
>> + const struct flowside *toside = flowside_at_sidx(tosidx);
>> + struct in_addr oaddr = toside->oaddr.v4mapped.a4;
>> + struct in_addr eaddr = toside->eaddr.v4mapped.a4;
>> + struct iov_tail data = IOV_TAIL(iov, 1, 0);
>> + struct {
>> + struct icmphdr icmp4h;
>> + struct iphdr ip4h;
>> + struct udphdr uh;
>> + char data[8];
>> + } msg;
>
> Needs a ((packed)) attribute to ensure we don't get compiler padding.
I get
udp.c:437:23: warning: taking address of packed member of ‘struct
<anonymous>’ may result in an unaligned pointer value
[-Waddress-of-packed-member]
437 | tap_push_ip4h(&msg.ip4h, eaddr, oaddr, udplen,
IPPROTO_UDP);
Also, the sizes of these members are 8+20+8+8, so I cannot see how
this struct can possibly be packed or how there can be an unaligned
pointer.
Unaligned pointer because it's on the stack, and it can happily be at a
semi-random point of it. Packed needs to be packed because you could
have 8 + 20 + ("oops we are at 28 let's make it 32") 4 + 8 + 8.
In short: ((packed)) seems unnecessary, and only
causes problems.
...well, but it still should be ((packed)) because it goes on the wire,
and ((aligned)) because you need to access the single fields:
__attribute__((packed, aligned(__alignof__(max_align_t))));
meaning: align it to whatever any kind of machine might possibly need.