在 2022/3/25 9:09, Jakub Kicinski 写道:
On Thu, 24 Mar 2022 23:49:14 +0800 Jian Shen wrote:
Introduce a set of bitmap operation helpers for netdev features, then we can use them to replace the logical operation with them. As the nic driversare not supposed to modify netdev_features directly, it also introduces wrappers helpers to this.
The implementation of these helpers are based on the old prototype of netdev_features_t is still u64. I will rewrite them on the last patch, when the prototype changes.
Signed-off-by: Jian Shen shenjian15@huawei.com
include/linux/netdevice.h | 597 ++++++++++++++++++++++++++++++++++++++
Please move these helpers to a new header file which won't be included by netdevice.h and include it at users appropriately.
I introduced a new header file "netdev_features_helper", and moved thses helpers to it. Some helpers need to include struct net_device which defined in netdevice.h, but there are also some inline functions in netdevice.h need to use these netdev_features helpers. It's conflicted.
So far I thought 3 ways to solved it, but all of them are not satisfactory. 1) Split netdevice.h, move the definition of struct net_device and its relative definitions to a new header file A( haven't got a reasonable name). Both the netdev_features_helper.h and the netdevice include A.
2) Split netdevice.h, move the inline functions to a new header file B. The netdev_features_helper.h inlucde netdevice.h， and B include netdev_features_helper.h and netdevice.h. All the source files which using these ininline functions should include B.
3) Split netdevice.h, move the inline functions to to netdev_featurer_helper.h. The netdev_features_helper.h inlucde netdevice.h, All the source files which using these ininline functions should include netde_features_helper.h.
I'd like to get more advice to this.
Thanks!
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 7307b9553bcf..0af4b26896d6 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2295,6 +2295,603 @@ struct net_device { }; #define to_net_dev(d) container_of(d, struct net_device, dev)
+static inline void netdev_features_zero(netdev_features_t *dst) +{
- *dst = 0;
+}
+static inline void netdev_features_fill(netdev_features_t *dst) +{
- *dst = ~0ULL;
+}
+static inline bool netdev_features_empty(const netdev_features_t src)
Don't pass by value something larger than 64 bit.
+{
- return src == 0;
+}
+/* helpers for netdev features '==' operation */ +static inline bool netdev_features_equal(const netdev_features_t src1,
const netdev_features_t src2)+{
- return src1 == src2;
+} +/* helpers for netdev features '&=' operation */ +static inline void +netdev_features_direct_and(netdev_features_t *dst,
const netdev_features_t features)+{
- *dst = netdev_features_and(*dst, features);
+}
+static inline void +netdev_active_features_direct_and(struct net_device *ndev,
s/direct_and/mask/ ?
const netdev_features_t features)+{
- ndev->active_features = netdev_active_features_and(ndev, features);
+}
+/* helpers for netdev features '|' operation */ +static inline netdev_features_t +netdev_features_or(const netdev_features_t a, const netdev_features_t b) +{
- return a | b;
+} +/* helpers for netdev features '|=' operation */ +static inline void +netdev_features_direct_or(netdev_features_t *dst,
s/direct_or/set/ ?
const netdev_features_t features)+{
- *dst = netdev_features_or(*dst, features);
+} +/* helpers for netdev features '^' operation */ +static inline netdev_features_t +netdev_features_xor(const netdev_features_t a, const netdev_features_t b) +{
- return a ^ b;
+} +/* helpers for netdev features '^=' operation */ +static inline void +netdev_active_features_direct_xor(struct net_device *ndev,
s/direct_xor/toggle/ ?
+/* helpers for netdev features '& ~' operation */ +static inline netdev_features_t +netdev_features_andnot(const netdev_features_t a, const netdev_features_t b) +{
- return a & ~b;
+} +static inline void +netdev_features_direct_andnot(netdev_features_t *dst,
s/andnot/clear/ ?
const netdev_features_t features)+{
- *dst = netdev_features_andnot(*dst, features);
+} +/* helpers for netdev features 'set bit' operation */ +static inline void netdev_features_set_bit(int nr, netdev_features_t *src)
s/features_set_bit/feature_add/ ?
+{
- *src |= __NETIF_F_BIT(nr);
+} +/* helpers for netdev features 'set bit array' operation */ +static inline void netdev_features_set_array(const int *array, int array_size,
netdev_features_t *dst)+{
- int i;
- for (i = 0; i < array_size; i++)
netdev_features_set_bit(array[i], dst);+}
+#define netdev_active_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->active_features)+#define netdev_hw_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->hw_features)+#define netdev_wanted_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->wanted_features)+#define netdev_vlan_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->vlan_features)+#define netdev_hw_enc_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->hw_enc_features)+#define netdev_mpls_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->mpls_features)+#define netdev_gso_partial_features_set_array(ndev, array, array_size) \
netdev_features_set_array(array, array_size, &ndev->gso_partial_features)+/* helpers for netdev features 'clear bit' operation */ +static inline void netdev_features_clear_bit(int nr, netdev_features_t *src)
All the mentions of '_bit' are unnecessary IMHO. .