netdev - RE: [PATCH v3] net: macb: add support for high speed interface

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]

Date:   Fri, 23 Oct 2020 10:59:42 +0000
From:   Parshuram Raju Thombare <pthombar@...ence.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
CC:     "andrew@...n.ch" <andrew@...n.ch>,
        "nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Milind Parab <mparab@...ence.com>
Subject: RE: [PATCH v3] net: macb: add support for high speed interface

Hi,

I was trying to find out any ethernet driver where this issue of selecting appropriate
pcs_ops due to phylink changing interface mode dynamically is handled. 
But, apparently, so far only mvpp2 has adapted pcs_ops. And even in mvpp2, it is
not obvious how this case is handled. 

Also, apart from interface mode changed due to SFPs with different types of PHY
being used, it is not clear when phylink selects interface mode different than it
initially requested to the ethernet driver.

>pcs_config and pcs_link_up passes "interface" as an argument, and in
>pcs_get_state call "state->interface" appeared to be populated just before
>calling it and hence should be valid.

It seems state->interface in pcs_get_state is not always valid when SFPs with
different types of PHY are used.  
There is a chance of SFP with different type of PHY is inserted, eventually invoking
phylink_resolve for interface mode different than phylink initially requested,
and causing major reconfiguration.

However, pcs_get_state is called before major reconfiguration, where selecting
which pcs_ops and PCS to be used is difficult without correct interface mode.  

As struct phylink and hence phy_state is private to phylink layer, IMO this need to be
handled at phylink level by passing appropriate interface mode to all necessary
methods registered by drivers.

Something like

523 static void phylink_mac_pcs_get_state(struct phylink *pl,
 524                                       struct phylink_link_state *state)
 525 {
 526         linkmode_copy(state->advertising, pl->link_config.advertising);
 527         linkmode_zero(state->lp_advertising);
 528         if (pl->phydev)
 529                 state->interface = pl->phy_state.interface;
 530         else
 531                 state->interface = pl->link_config.interface;
 532         state->an_enabled = pl->link_config.an_enabled;
 533         state->speed = SPEED_UNKNOWN;
 534         state->duplex = DUPLEX_UNKNOWN;
 535         state->pause = MLO_PAUSE_NONE;
 536         state->an_complete = 0;
 537         state->link = 1;
 538
 539         if (pl->pcs_ops)
 540                 pl->pcs_ops->pcs_get_state(pl->pcs, state);
 541         else
 542                 pl->mac_ops->mac_pcs_get_state(pl->config, state);

Regards,
Parshuram Thombare