| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BANLkTinTT5SMutDq+7GCDhgRYsUWLj-5mA@mail.gmail.com>
Date: Wed, 6 Apr 2011 10:34:14 -0700
From: Mahesh Bandewar <maheshb@...gle.com>
To: Michał Mirosław <mirqus@...il.com>
Cc: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 01/20] net-core: extending (hw_/wanted_/vlan_)features
fields to a bitmap.
On Wed, Apr 6, 2011 at 3:29 AM, Michał Mirosław <mirqus@...il.com> wrote:
> 2011/4/6 Mahesh Bandewar <maheshb@...gle.com>:
>> Converting current use of (hw_/wanted_/vlan_)features to
>> legacy_(hw_/wanted_/vlan_)features to differntiate from the proposed usage.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>> ---
>> include/linux/netdevice.h | 110 +++++++++++++++++++++++++++++++-------------
>> net/core/dev.c | 51 +++++++++++----------
>> net/core/ethtool.c | 97 ++++++++++++++++++++-------------------
>> net/core/net-sysfs.c | 4 +-
>> net/core/sock.c | 2 +-
>> 5 files changed, 155 insertions(+), 109 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 09d2624..637bf2a 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -980,6 +980,42 @@ struct net_device_ops {
>> u32 features);
>> };
>>
>> +enum netdev_features {
>> + NETIF_F_SG_BIT, /* Scatter/gather IO. */
>> + NETIF_F_IP_CSUM_BIT, /* Can checksum TCP/UDP over IPv4. */
>> + NETIF_F_NO_CSUM_BIT, /* Does not require checksum. F.e. loopack. */
>> + NETIF_F_HW_CSUM_BIT, /* Can checksum all the packets. */
>> + NETIF_F_IPV6_CSUM_BIT, /* Can checksum TCP/UDP over IPV6 */
>> + NETIF_F_HIGHDMA_BIT, /* Can DMA to high memory. */
>> + NETIF_F_FRAGLIST_BIT, /* Scatter/gather IO. */
>> + NETIF_F_HW_VLAN_TX_BIT, /* Transmit VLAN hw acceleration */
>> + NETIF_F_HW_VLAN_RX_BIT, /* Receive VLAN hw acceleration */
>> + NETIF_F_HW_VLAN_FILTER_BIT, /* Receive filtering on VLAN */
>> + NETIF_F_VLAN_CHALLENGED_BIT, /* Device cannot handle VLAN packets */
>> + NETIF_F_GSO_BIT, /* Enable software GSO. */
>> + NETIF_F_LLTX_BIT, /* LockLess TX - deprecated. Please */
>> + /* do not use LLTX in new drivers */
>> + NETIF_F_NETNS_LOCAL_BIT, /* Does not change network namespaces */
>> + NETIF_F_GRO_BIT, /* Generic receive offload */
>> + NETIF_F_LRO_BIT, /* large receive offload */
>> + /* the GSO_MASK reserves bits 16 through 23 */
>> + RESERVED1_BIT,
>> + RESERVED2_BIT,
>> + RESERVED3_BIT,
>> + RESERVED4_BIT,
>> + RESERVED5_BIT,
>> + RESERVED6_BIT,
>> + RESERVED7_BIT,
>> + RESERVED8_BIT,
>> + NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
>> + NETIF_F_SCTP_CSUM_BIT, /* SCTP checksum offload */
>> + NETIF_F_FCOE_MTU_BIT, /* Supports max FCoE MTU, 2158 bytes*/
>> + NETIF_F_NTUPLE_BIT, /* N-tuple filters supported */
>> + NETIF_F_RXHASH_BIT, /* Receive hashing offload */
>> + NETIF_F_RXCSUM_BIT, /* Receive checksumming offload */
>> + NETIF_F_NOCACHE_COPY_BIT, /* Use no-cache copyfromuser */
>> +};
>> +
>
> This should be a separate cleanup patch. And after that, for the
> conversion you would add as a last entry:
> NETIF_F_NUM_BITS and use it later (see below).
>
I like the idea of NETIF_F_NUM_BITS and I'll change the #defines
accordingly. Couple of bitmaps are defined in this patch and NUM_BITS
will be required to define those bitmaps. This is reason why I have
defined above enum now rather than waiting for the cleanup phase. Also
it should not change radically in that time span.
>> /*
>> * The DEVICE structure.
>> * Actually, this whole structure is a big mistake. It mixes I/O
>> @@ -1029,44 +1065,51 @@ struct net_device {
>> struct list_head napi_list;
>> struct list_head unreg_list;
>>
>> +#define DEV_FEATURE_WORDS 2
>> +#define DEV_FEATURE_BITS (DEV_FEATURE_WORDS*sizeof(long)*BITS_PER_BYTE)
>> +#define LEGACY_FEATURE_WORD 0
>> +
>
> #define DEV_FEATURE_WORDS BITS_TO_LONGS(NETIF_F_NUM_BITS)
> #define DEV_FEATURE_BITS (DEV_FEATURE_WORDS*BITS_PER_LONG)
>
Yes, this is good!
> Though using bitmaps will make a mess for 32 versus 64 bit archs. It
> would be better to stick to u32 as the base type instead of long.
>
Once it's a bitmap; type shouldn't matter and each arch and it's
specific macros/inlines should handle them properly, no?
(I changed the base type since DECLARE_BITMAP() declares 'unsigned
long' and we can make use of readily avaialble set/test/clear_bit
macros)
> [...]
>> @@ -2376,13 +2419,13 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
>> }
>>
>> #define HARD_TX_LOCK(dev, txq, cpu) { \
>> - if ((dev->features & NETIF_F_LLTX) == 0) { \
>> + if ((dev->legacy_features & NETIF_F_LLTX) == 0) { \
> [...]
>
> For those type of conversion there is really no point in using the
> macro. Changing it to
> dev->features[0] instead of dev->legacy_features needs the same effort
> but avoids the
> cleanup later. Flags in other feature words could have names line
> NETIF_F2_xxx so that
> it would be clear in which word they belong.
>
I know! But changing all at once in zillion places is hard. This will
let us make changes progressively. I'm expecting the following
progress path -
(1) (Current patch)
+ #define legacy_feactures features
+ #define legacy_vlan_features vlan_features
Slowly make all the changes (relatively easy but a lengthy process!).
So the places where vlan_features, features fields are used will
compile and at the same time updates, where legacy_vlan_features,
legacy_features fields are used, will compile too. Once all is changes
are in place -
(2) Relatively small change
- unsigned long features;
+ DECLARE_BITMAP(feature, DEV_FEATURE_BITS);
- unsigned long vlan_features;
+ DECLARE_BITMAP(vlan_feature, DEV_FEATURE_BITS);
- #define legacy_features features
+ #define legacy_features feature[0]
- #define legacy_vlan_features vlan_features
+ #define legacy_vlan_features vlan_feature[0]
At this moment old fields are gone and are replaced by the bitmaps and
the legacy usage is indicated by the use "legacy_*" fields. This
should eventually be changed to use the (set/test/clear)_bit()
macros/inlines. So the above place should look like -
#define HARD_TX_LOCK(dev, txq, cpu) { \
- if ((dev->legacy_features & NETIF_F_LLTX) == 0) { \
+ if (!test_bit(dev->feature, NETIF_F_LLTX_BIT) { \
Thanks,
--mahesh..
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists