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: <02874ECE860811409154E81DA85FBB58C8B20741@fmsmsx101.amr.corp.intel.com>
Date:   Mon, 27 Jul 2020 22:32:03 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     Michal Kubecek <mkubecek@...e.cz>, Andrew Lunn <andrew@...n.ch>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jamie Gloudon <jamie.gloudon@....fr>
Subject: RE: [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK



> -----Original Message-----
> From: Michal Kubecek <mkubecek@...e.cz>
> Sent: Monday, July 27, 2020 3:27 PM
> To: Andrew Lunn <andrew@...n.ch>
> Cc: Keller, Jacob E <jacob.e.keller@...el.com>; netdev@...r.kernel.org; Jamie
> Gloudon <jamie.gloudon@....fr>
> Subject: Re: [ethtool] ethtool: fix netlink bitmasks when sent as NOMASK
> 
> On Tue, Jul 28, 2020 at 12:11:04AM +0200, 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.
> 
> This is a different issue, the last call to dump_link_modes() in
> dump_peer_modes() should be
> 
> 	ret = dump_link_modes(nlctx, attr, false, LM_CLASS_FEC,
> 
> (third parameter needs to be false, not true).
> 
> Michal
> 

That's part of it, yea, but also it should use the string "Not Reported" instead of "No" I think?

> >
> > 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