[openib-general] ipoib_ib.c - alignment questions

[Roland Dreier]

 > ipoib_ib.c has this:
 >         /*
 >          * IB will leave a 40 byte gap for a GRH and IPoIB adds a 4 byte
 >          * header.  So we need 4 more bytes to get to 48 and align the
 >          * IP header to a multiple of 16.
 >          */
 >         skb_reserve(skb, 4);
 > 
 > Some questions on this:
 > - Why do we try to align the IP header to a multiple of 16?

As the comment in linux/skbuff.h near the definition of NET_IP_ALIGN says:

 * CPUs often take a performance hit when accessing unaligned memory
 * locations. The actual performance hit varies, it can be small if the
 * hardware handles it or large if we have to take an exception and fix it
 * in software.

 > - This works if skb start is 16 byte aligned.
 >   What guarantees that skb data is 16 byte aligned?

The implementation of the slab allocator I guess.

 > - Would the following code be better than the comment:
 > 	skb_reserve(skb, ALIGN(IB_GRH_BYTES + IPOIB_ENCAP_LEN, 16) -
 > 		    IB_GRH_BYTES - IPOIB_ENCAP_LEN);
 > 
 >   comments have a bigger tenency to bitrot ...

I think that's a lot harder to understand when you first read it.  I
guess we would still need the comment there.

BTW there is still a possibility of improvement here, because for
example on ppc64, the cost of unaligned DMA is much higher than the
cost of an unaligned IP header -- as linux/skbuff.h goes on to say:

 * The downside to this alignment of the IP header is that the DMA is now
 * unaligned. On some architectures the cost of an unaligned DMA is high
 * and this cost outweighs the gains made by aligning the IP header.

So really we should have something else there if NET_IP_ALIGN is 0 --
the goal would be to DMA received packets starting at a 64-byte
aligned address.  But I've never bothered to work out the best way to
do that.

 - R.