netdev - Re: Question about settings array in phy.c

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] [day] [month] [year] [list]

Message-ID: <CAGVrzcbrasYjNMqVVjVMdeM+Lv4w+s-=NM4XM_oB-mYw_a+7bw@mail.gmail.com>
Date:	Fri, 11 Jul 2014 10:21:09 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Tom Lendacky <thomas.lendacky@....com>
Cc:	netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: Question about settings array in phy.c

Hi Tom,

2014-07-11 8:50 GMT-07:00 Tom Lendacky <thomas.lendacky@....com>:
> In drivers/net/phy/phy.c there is an array of struct phy_setting named
> settings that is used when auto negotiation is disabled to sanitize
> the phy settings.  This array uses only *baseT* features for validating
> the various speed/duplex combinations.  This may result in not finding
> the correct array entry in regards to speed and duplex if the *baseT*
> feature is not supported.

Part of the reason is that libphy was designed around 2006-2007 and
initially only supported 10/100/1000 PHY devices, and until recently
it did not support 10GbE PHYs at all.

>
> For example, a 10GbE phy that supports KR mode would end up not matching
> an entry and defaulting to the last entry of the array (SPEED_10 and
> DUPLEX_HALF) which is not what is desired.
>
> Is it appropriate to be able to extend this array?  Either by adding
> additional array entries or extending the "setting" to include other
> features?  Or is there a specific reason that only the *baseT* values
> are used?

It seems to me like it will be easier to refine/extend the matching
logic by adding a new entries which contain a single 'setting' to be
matched against, such as the one you show below:

>
> Examples of changing the array:
>
>   Add a new entry
>         {
>                 .speed = SPEED_10000,
>                 .duplex = DUPLEX_FULL,
>                 .setting = SUPPORTED_10000baseKR_Full,
>         },
>
>   or extend current entry
>         {
>                 .speed = SPEED_10000,
>                 .duplex = DUPLEX_FULL,
>                 .setting = SUPPORTED_10000baseT_Full |
>                            SUPPORTED_10000baseKR_Full,
>         },
>
> Thanks,
> Tom



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