netdev - Re: [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK

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: <fb270523-f4b0-a824-7c98-e69e255061d6@intel.com>
Date:   Mon, 27 Jul 2020 15:22:27 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Michal Kubecek <mkubecek@...e.cz>, netdev@...r.kernel.org,
        Jamie Gloudon <jamie.gloudon@....fr>
Subject: Re: [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK



On 7/27/2020 3:11 PM, Andrew Lunn wrote:
> On Mon, Jul 27, 2020 at 02:47:00PM -0700, Jacob Keller wrote:
>> The ethtool netlink API can send bitsets without an associated bitmask.
>> These do not get displayed properly, because the dump_link_modes, and
>> bitset_get_bit to not check whether the provided bitset is a NOMASK
>> bitset. This results in the inability to display peer advertised link
>> modes.
>>
>> The dump_link_modes and bitset_get_bit functions are designed so they
>> can print either the values or the mask. For a nomask bitmap, this
>> doesn't make sense. There is no mask.
>>
>> Modify dump_link_modes to check ETHTOOL_A_BITSET_NOMASK. For compact
>> bitmaps, always check and print the ETHTOOL_A_BITSET_VALUE bits,
>> regardless of the request to display the mask or the value. For full
>> size bitmaps, the set of provided bits indicates the valid values,
>> without using ETHTOOL_A_BITSET_VALUE fields. Thus, do not skip printing
>> bits without this attribute if nomask is set. This essentially means
>> that dump_link_modes will treat a NOMASK bitset as having a mask
>> equivalent to all of its set bits.
>>
>> For bitset_get_bit, also check for ETHTOOL_A_BITSET_NOMASK. For compact
>> bitmaps, always use ETHTOOL_A_BITSET_BIT_VALUE as in dump_link_modes.
>> For full bitmaps, if nomask is set, then always return true of the bit
>> is in the set, rather than only if it provides an
>> ETHTOOL_A_BITSET_BIT_VALUE. This will then correctly report the set
>> bits.
>>
>> This fixes display of link partner advertised fields when using the
>> netlink API.
> 
> Hi Jacob
> 
> This is close
> 
> Netlink
> 	Link partner advertised link modes:  10baseT/Half 10baseT/Full
> 	                                     100baseT/Half 100baseT/Full
> 	                                     1000baseT/Full
> 	Link partner advertised pause frame use: No
> 	Link partner advertised auto-negotiation: Yes
> 	Link partner advertised FEC modes: No
> 
> IOCTL
> 	Link partner advertised link modes:  10baseT/Half 10baseT/Full 
> 	                                     100baseT/Half 100baseT/Full 
> 	                                     1000baseT/Full 
> 	Link partner advertised pause frame use: No
> 	Link partner advertised auto-negotiation: Yes
> 	Link partner advertised FEC modes: Not reported
> 
> So just the FEC modes differ.
> 
> However, i don't think this was part of the original issue, so:
> 
> Tested-by: Andrew Lunn <andrew@...n.ch>
> 
> It would be nice to get the FEC modes fixed.
> 
>     Andrew
> 

Yea, it looks like FEC modes is because FEC actually send a "None" flag,
as opposed to using an empty set as "none". I can follow-up this fix
with a change I think will resolve this as well.

Thanks,
Jake