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

[Daniel Hartmeier]

On Wed, Jun 15, 2005 at 02:23:24PM -0700, Marcel Moolenaar wrote:

> That entirely depends. If a struct ip pointer is constructed without
> any form of casting, then one can assume that alignment is guaranteed.
> The compiler guarantees to do so, except of course in this case:
> the structure is defined as a packed structure. We, as the developers,
> have told the compiler to *NOT* guarantee alignment of fields. We're
> on our own and we miserably fail being on our own.

'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.

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?

The IP header and the struct that represents it are defined very
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;

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.

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.

So, are you really sure we should do differently in pf, instead of
looking for a bridge problem, where bridge constructs an mbuf with the
IP header not properly aligned?

I.e. if the IP header is properly aligned within the mbuf (on 32-bit
boundaries, I presume), wouldn't ip_src/dst have to be properly aligned
as well, even though __packed is used, because the layout of struct ip
is chosen like that?

Daniel