From: Al Viro <v...@zeniv.linux.org.uk> Date: Tue, 3 Apr 2018 17:29:33 +0100
> On Mon, Apr 02, 2018 at 03:58:55PM -0700, Florian Fainelli wrote: >> skb->protocol is a __be16 which we would be calling htons() against, >> while this is not wrong per-se as it correctly results in swapping the >> value on LE hosts, this still upsets sparse. Adopt a similar pattern to >> what other drivers do and just assign ip_ver to skb->protocol, and then >> use htons() against the different constants such that the compiler can >> resolve the values at build time. > > Fuck that and fuck other drivers sharing that "pattern". It's not hard > to remember: > host-endian to net-endian short is htons > host-endian to net-endian long is htonl > net-endian to host-endian short is ntohs > net-endian to host-endian long is ntohl > > That's *it*. No convoluted changes needed, just use the right primitive. > You are turning trivial one-liners ("conversions between host- and net-endian > are the same in both directions, so the current code works even though we > are using the wrong primitive, confusing the readers. Use the right one") > which are absolutely self-contained and safe into much more massive changes > that are harder to follow and which are *not* self-contained at all. > > Don't do it. Yes Al, however the pattern choosen here is probably cheaper on little endian: __beXX val = x; switch (val) { case htons(ETH_P_FOO): ... } This way only the compiler byte swaps the constants at compile time, no code is actually generated to do it.