Re: [PATCH] ICMP extensions for MPLS support for traceroute(8)

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:> +#if BYTE_ORDER == BIG_ENDIAN
:> +	unsigned char   version:4;
:> +	unsigned char   reserved1:4;
:> +#else
:> +	unsigned char   reserved1:4;
:> +	unsigned char   version:4;
:> +#endif
:> +	unsigned char   reserved2;
:> +	unsigned short  checksum;
:> +};
:
:Please don't add new code that depends on the order of bitfields.
:(Or bitfields in general for wire protocols).
:
:Joerg

    What Joerg is saying is that the patch looks great!  But we're
    trying to get away from using bitfields so if it would not be too
    much trouble, could you just make that a u_char and extract the fields
    manually with a macro?

    Note that when used as a u_char, I'm pretty sure that no endian
    conversions are needed when doing a manual extraction.  Endian
    conversions on bitfield ordering within single bytes are an artifact
    of the way the compiler assigns the bitfield indices.  The actual
    storage (within a char) is the same.

    Take for example the IP header (now there's something I would like to
    clean up and just make _IP_VHL the default).   Note that the IP_VHL_*()
    macros do not require any endian conditionalization.

struct ip {
#ifdef _IP_VHL
        u_char  ip_vhl;                 /* version << 4 | header length >> 2 */
#elif _BYTE_ORDER == _LITTLE_ENDIAN
        u_int   ip_hl:4,                /* header length */
                ip_v:4;                 /* version */
#elif _BYTE_ORDER == _BIG_ENDIAN
        u_int   ip_v:4,                 /* version */
                ip_hl:4;                /* header length */
#else
. ..
#endif

#define  IP_VHL_HL(vhl)          ((vhl) & 0x0f)
#define  IP_VHL_V(vhl)           ((vhl) >> 4)
#define  IP_VHL_BORING           0x45

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>