ia64/81284: Unaligned Reference with pf on 5.4/IA64

[Bruce Evans]

On Thu, 16 Jun 2005, Daniel Hartmeier wrote:

> 'packed', as I understand it, prohibits the compiler from inserting any
> padding anywhere in the struct. That is, it guarantees that the total
> size of a struct object equals the sum of the sizes of its members.

It also requires the complier to generate specially pessimized code to
access the struct.  Otherwise even direct references like ip->ip_src
might trap.

> As a consequence, individual members can't be aligned properly if that
> would require padding in front of them. The compiler can (and must?)
> still align the first member, i.e. the beginning of the struct object,
> though, no?

No.  This wouldn't be very useful, and wouldn't work right for things
like arrays of packed structs -- the individual structs would have to
be padded at the end since there can be no space between array elements,
but packed structs are supposed to be unpadded at the end too.  You can
see that there is no alignment requirement for packed structs as a whole
by looking at assembler output -- for a packed struct ip there is a
.comm..,1 where the 1 says 1-byte alignment.

> The IP header and the struct that represents it are defined very
                                                   ^^^ were ...
> carefully. It's no coincidence that ip_src/dst are placed where they
> are:
>
> struct ip {
>        u_int   ip_hl:4,                /* header length */
>                ip_v:4;                 /* version */
>        u_char  ip_tos;                 /* type of service */
>        u_short ip_len;                 /* total length */
>        u_short ip_id;                  /* identification */
>        u_short ip_off;                 /* fragment offset field */
>        u_char  ip_ttl;                 /* time to live */
>        u_char  ip_p;                   /* protocol */
>        u_short ip_sum;                 /* checksum */
>        struct  in_addr ip_src,ip_dst;  /* source and dest address */
> } __packed;
    ^^^^^^^^^^ ... before this bug
>
> This guarantees that
>
>  struct ip h;
>  &h.ip_src == (char *)&h + 12
>  &h.ip_dst == (char *)&h + 16
>
> i.e. they are both on 32-bit aligned if h itself is 32-bit aligned.
>
> If you look at any example in sys/netinet where struct ip members are
> accessed, you see direct access to both u_short and uint32_t members.
> Nowhere does anyone memcpy, bcopy or char-wise-copy ip_src/dst, for
> instance.

Except the compiler does this.

> The issue also involves how the IP header is aligned within mbufs.
> Functions usually get passed an mbuf pointer, and do
>
>  struct mbuf *m;
>  struct ip *ip = mtod(m, struct ip *);
>
> and then happily access ip_src/dst without further alignment checks.

mtod() just does a dubious cast.  It depends on the data already being
sufficiently aligned.  For non-bogusly-packed IP headers this means 
32-bit alignment (for the in_addr's).

It's not clear where the misaligned headers come from.  At least with
gcc-3.3.3, I couldn't get a non-32-bit-aligned struct ip as a local
variable by putting a char variable or char array before or after it.
gcc has obscure rules for reordering local variables to pack them
better, and this helps here.  Padded structs withing (even unpadded)
structs can easily be misaligned depending on what is before them, but
struct ip's within structs seem to be rare.  The one in ip_icmp.h is OK.

Bruce