Re: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path
From: David Miller <davem@davemloft.net>
Date: 2012-09-28 02:29:02
From: Sathya Perla <redacted> Date: Thu, 27 Sep 2012 12:02:47 +0530
The AMAP macros are used in be2net for setting and parsing bits in HW descriptors. The macros do this by calculating the mask & offset of each field from the AMAP structure definition. In the TX patch, replace the usage of these macros with code to explicitly shift & mask each field. Doing this reduces instructions and improves readability. Signed-off-by: Sathya Perla <redacted>
Now you have endianness bugs. The previous code worked with 8-bit struct members and as such was endian neutral. Now you're working with words, so you thus have to take endianness into consideration. The readability aspect is also extremely questionable, here's why. The old code accessed struct members with _NAMES_ which described what the values are and what they do. This new code puts values into opaque "word" array members. That's about as crappy as it comes wrt. readability. What in the world does word[0] do? I can't tell from it's name. Yet with the existing "struct amap_eth_hdr_wrb" there is none of that kind of confusion. So don't pretend this new code even looks better, it looks like opaque garbage to me. Just admit this is an optimization that broke things on the endianness you did not test. be2net patches have been of a very low quality lately. So I suggest you improve things or else your submissions will be processed with an extremely low priority. Thanks.