On 29/09/2021 16:50, Jian Shen wrote:
This patchset try to solve it by change the prototype of netdev_features_t from u64 to bitmap. With this change, it's necessary to introduce a set of bitmap operation helpers for netdev features. Meanwhile, the functions which use netdev_features_t as return value are also need to be changed, return the result as an output parameter.
This might be a terrible idea, but could you not do something like typedef struct { DECLARE_BITMAP(bits, NETDEV_FEATURE_COUNT); } netdev_features_t; thereby allowing functions to carry on returning it directly? The compiler would still likely turn it into an output parameter at an ABI level (at least once NETDEV_FEATURE_COUNT goes above 64), but the amount of code churn might be significantly reduced. Another advantage is that, whereas bitwise ops (&, |, ^) on a pointer (such as unsigned long *) are legal (meaning something like "if (features & NETIF_F_GSO_MASK)" may still compile, at best with a warning, despite having nonsensical semantics), they aren't possible on a struct; so there's less risk of unpatched code (perhaps merged in from another subsystem, or in out-of-tree modules) silently breaking — instead, any mix of new and old code will be caught at build time.
WDYT? -ed