[lwip-devel] [bug #27352] Change ip_addr from struct to typedef (u32_t)

[Stephane Lesage]

Follow-up Comment #34, bug #27352 (project lwip):

ntohX as macros is nice for constants, but it's not for run-time code
especially if you have assembly instruction or intrinsics in my case.

mix that with misaligned access because of packing and you get really UGLY
code, which is both neither fast nor small.

In my case, when I changed to pack(2) and intrinsics, I gained 8 KiB of
code...

Most of the time, just reading one u32_t value is very inefficient with
pack(1) and the standard ntohl() function for little endian.

This was my case, so imagine the code I get with
#define IPH_TTL(hdr) (ntohs((hdr)->_ttl_proto) >> 8)
#define IPH_PROTO_SET(hdr, proto) (hdr)->_ttl_proto = (htons((proto) |
(IPH_TTL(hdr) << 8))) !!

misaligned read, call inefficient ntohs, shifts R+L (optimized as and ?), or,
call inefficient htons, misaligned write !!!

why isn't _ttl_proto separated as 2 u8_t ??
and this would result in a simple byte write, as it should be.

or #define IPH_V(hdr)  (ntohs((hdr)->_v_hl_tos) >> 12)

why read 16 bits, convert endianness, to finally get only 4 bits that could
be read directly from the containing byte ?

Maybe ntohX as macros can help.

But I'd better optimize these types of macros and use my optimized
byteswap4() intrinsic for the cases where it's really needed.

I'd also better use macros to read and convert.
#define read_u32(p) for example could be defined as:

1. big endian, no packing
#define read_u32(p) (*(p))

2. little endian, no packing
#define read_u32(p) ntohl(*(p))

3. big endian, pack(2)
#define read_u32(p) ( (((u16_t*)(p)[0])<<16) | ((u16_t*)(p)[1]) )

4. little endian pack(2) or pack(1)
#define read_u32(p) ( (((u8_t*)(p)[0])<<24) | (((u8_t*)(p)[1])<<16) |
(((u8_t*)(p)[2])<<8) | ((u8_t*)(p)[3]) )

We could offer the choice to the user, explaining the details, packing needed
with(out) ETH_PAD_SIZE, and providing advice for the different architectures
the developpers use here.



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27352>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/