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: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.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.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. -- Stefano