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

From: Daniel Hartmeier (daniel_at_benzedrine.cx)
Date: 06/16/05

  • Next message: Marcel Moolenaar: "Re: ia64/81284: Unaligned Reference with pf on 5.4/IA64" 
    Date: Thu, 16 Jun 2005 00:34:50 +0200
To: Marcel Moolenaar <marcel@xcllnt.net>

    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
    _______________________________________________
    freebsd-net@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-net
    To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"

  • Next message: Marcel Moolenaar: "Re: ia64/81284: Unaligned Reference with pf on 5.4/IA64"

    Relevant Pages

    • Re: struct alignment
      ... necessary so that all the members can be accesssed correctly. ... Does any compiler let you do this? ... Permit reordering of struct members, ... but that surely is rare. ...
      (comp.lang.c)
    • Re: procedure parameter struct order
      ... the members of this struct would always have the same order on ... >> members in the order they are declared. ... Or if there even is a stack as such, ... > compiler would be quite correct if, behind the scenes, all it pushed on ...
      (comp.lang.c)
    • Re: struct alignment
      ... necessary so that all the members can be accesssed correctly. ... Does any compiler let you do this? ... Permit reordering of struct members, ... that is enough reason for not allowing reordering. ...
      (comp.lang.c)
    • Re: komplexes Problem mit Funktionszeigern
      ... Die Funktion gibt einen struct "by value" zurueck und bei der ... am Borland Compiler, da der Microsoft Compiler ... Man kann Argumente fuer eine Funktion z.B auf dem Stack uebergeben. ... Das funktioniert natuerlich nur fuer "Basis Typen" die in ein Register passen. ...
      (de.comp.lang.c)
    • Re: Replace bcopy() to update ether_addr
      ... Good for source code ugliness and bloat (not just in the macro). ... reduces to a struct copy if the object being copy is a struct, ... It has the side effect of allowing the compiler to not ... have more than byte alignment. ...
      (freebsd-hackers)