netdev - Re: [PATCH] net: phy: sfp: correct store of detected link modes

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87o9a86p6q.fsf@tkos.co.il>
Date:   Thu, 29 Nov 2018 14:30:53 +0200
From:   Baruch Siach <baruch@...s.co.il>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: phy: sfp: correct store of detected link modes

Hi Russell,

Russell King - ARM Linux writes:
> On Thu, Nov 29, 2018 at 12:40:11PM +0200, Baruch Siach wrote:
>> The link modes that sfp_parse_support() detects are stored in the
>> 'modes' bitmap. There is no reason to make an exception for 1000Base-PX
>> or 1000Base-BX10.
>
> I think you may be carrying some local patch, have an incorrect merge,
> or maybe there's a patch in -next which changed this.
>
> Mainline has:
>
>         if (bitmap_empty(modes, __ETHTOOL_LINK_MODE_MASK_NBITS)) {
>                 /* If the encoding and bit rate allows 1000baseX */
>                 if (id->base.encoding == SFP_ENCODING_8B10B && br_nom &&
>                     br_min <= 1300 && br_max >= 1200)
>                         phylink_set(modes, 1000baseX_Full);
>         }
>
> but your patch changes that phylink_set() from:
>
>                         phylink_set(support, 1000baseX_Full);
>
> to:
>
>                         phylink_set(modes, 1000baseX_Full);
>
> which in the context of what's in mainline doesn't make sense.

The code that this patch touches is at line 165 in current mainline as
of commit 60b548237fe:

    162         /* 1000Base-PX or 1000Base-BX10 */
    163         if ((id->base.e_base_px || id->base.e_base_bx10) &&
    164             br_min <= 1300 && br_max >= 1200)
    165                 phylink_set(support, 1000baseX_Full);

net-next as of e561bb29b6 carries no change in this file, as far as I
can see.

The first condition in the code snippet you cited above might
incorrectly evaluate as true because the PX and BX10 modes are currently
not reflected in the 'modes' bitmap. This patch should fix that. The
final set of modes would be the same regardless of this patch, but it's
still worth fixing for consistency, I think.

baruch

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -