netdev - Re: [PATCHv2] net: Define enum for the bits used in features.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 25 May 2011 11:05:30 -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>,
	Tom Herbert <therbert@...gle.com>,
	Stephen Hemminger <shemminger@...tta.com>
Subject: Re: [PATCHv2] net: Define enum for the bits used in features.

On Wed, May 25, 2011 at 2:43 AM, Michał Mirosław <mirqus@...il.com> wrote:
> 2011/5/25 Mahesh Bandewar <maheshb@...gle.com>:
>> Little bit cleanup by defining enum for all bits used. Also use those enum
>> values to redefine flags.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>> ---
>> Changes since v1:
>>  Split the patch into two pieces.
>>
>>  include/linux/netdevice.h |  100 +++++++++++++++++++++++++++++++-------------
>>  1 files changed, 70 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index ca333e7..b4520b2 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -51,6 +51,7 @@
>>  #ifdef CONFIG_DCB
>>  #include <net/dcbnl.h>
>>  #endif
>> +#include <linux/netdev_features.h>
>
> This should go to other part.
>
Ahh! split mishap!

>>  struct vlan_group;
>>  struct netpoll_info;
>> @@ -981,6 +982,49 @@ struct net_device_ops {
>>  };
>>
>>  /*
>> + * Net device feature bits; if you change something,
>> + * also update netdev_features_strings[] in ethtool.c
>> + */
>> +enum netdev_features {
> [...]
>
>> +       RESERVED16_BIT,         /* the GSO_MASK reserved bit 16 */
>> +       RESERVED17_BIT,         /* the GSO_MASK reserved bit 17 */
>> +       RESERVED18_BIT,         /* the GSO_MASK reserved bit 18 */
>> +       RESERVED19_BIT,         /* the GSO_MASK reserved bit 19 */
>> +       RESERVED20_BIT,         /* the GSO_MASK reserved bit 20 */
>> +       RESERVED21_BIT,         /* the GSO_MASK reserved bit 21 */
>> +       RESERVED22_BIT,         /* the GSO_MASK reserved bit 22 */
>> +       RESERVED23_BIT,         /* the GSO_MASK reserved bit 23 */
>
> This could also define NETIF_F_GSO_SHIFT and name TSO and others.
> Maybe like this:
>
>
> NETIF_F_GSO_SHIFT, /* must == 16, for now */
> GSO_RESERVED0_BIT = NETIF_F_GSO_SHIFT,
> GSO_RESERVED1_BIT,
> ...
> GSO_RESERVED7_BIT,
> ... (other bits)
>
Yes, I was thinking about that but then thought that itself will
deserve a separate patch.

>> +       /* Add you bit above this */
>
> your.
>
>> +       ND_FEATURE_NUM_BITS,     /* (LAST VALUE) Total bits in use */
>
> And here GSO aliases:
>
> TSO_BIT = NETIF_F_GSO_SHIFT + SKB_GSO_TCPV4,
> UFO_BIT = NETIF_F_GSO_SHIFT + SKB_GSO_UDP,
> ...
>
> Afther this is done, I can convert the feature-names table in
> ethtool.c to use C99 array assignments, so that no one else will trip
> on a missing comma or wrong entries order there.
>
> About the bit names: NETIF_F_xxx_BIT?
>
I thought it was getting longer so tried to trim it, but don't have
reservations against it.

--mahesh..

> Best Regards,
> Michał Mirosław
>
--
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