netdev - Re: [PATCH net-next] net: core: change bool members of struct net

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]

Message-ID: <f3d3a0c9-58e5-3d4e-a0bf-a712da9adcb2@gmail.com>
Date:   Mon, 8 Oct 2018 22:14:58 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Randy Dunlap <rdunlap@...radead.org>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: core: change bool members of struct
 net_device to bitfield members

On 08.10.2018 22:07, Randy Dunlap wrote:
> On 10/8/18 1:00 PM, Heiner Kallweit wrote:
>> bool is good as parameter type or function return type, but if used
>> for struct members it consumes more memory than needed.
>> Changing the bool members of struct net_device to bitfield members
>> allows to decrease the memory footprint of this struct.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>> ---
>>  include/linux/netdevice.h | 24 +++++++++++++-----------
>>  1 file changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 76603ee13..3d7b8df2e 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1651,10 +1651,6 @@ enum netdev_priv_flags {
>>   * 	@dev_port:		Used to differentiate devices that share
>>   * 				the same function
>>   *	@addr_list_lock:	XXX: need comments on this one
>> - *	@uc_promisc:		Counter that indicates promiscuous mode
>> - *				has been enabled due to the need to listen to
>> - *				additional unicast addresses in a device that
>> - *				does not implement ndo_set_rx_mode()
>>   *	@uc:			unicast mac addresses
>>   *	@mc:			multicast mac addresses
>>   *	@dev_addrs:		list of device hw addresses
>> @@ -1714,11 +1710,9 @@ enum netdev_priv_flags {
>>   *	@link_watch_list:	XXX: need comments on this one
>>   *
>>   *	@reg_state:		Register/unregister state machine
>> - *	@dismantle:		Device is going to be freed
>>   *	@rtnl_link_state:	This enum represents the phases of creating
>>   *				a new link
>>   *
>> - *	@needs_free_netdev:	Should unregister perform free_netdev?
>>   *	@priv_destructor:	Called from unregister
>>   *	@npinfo:		XXX: need comments on this one
>>   * 	@nd_net:		Network namespace this network device is inside
>> @@ -1758,6 +1752,15 @@ enum netdev_priv_flags {
>>   *	@qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
>>   *	@qdisc_running_key: lockdep class annotating Qdisc->running seqcount
>>   *
>> + *	@uc_promisc:	Counter that indicates promiscuous mode
>> + *			has been enabled due to the need to listen to
>> + *			additional unicast addresses in a device that
>> + *			does not implement ndo_set_rx_mode()
> 
> Hi,
> 
> I see that all you did is copy/paste that text (above), but I wouldn't call
> a single bit a [1-bit] Counter.
> 
I stumbled across this comment too. Neither a bool member nor a 1-bit
bitfield member should be called a counter. I kept the original comment, 
but I'm totally fine with changing Counter -> Flag and will provide a v2.

> thanks,
>