netdev - Re: Link modes representation in phylib

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, 29 Jun 2018 15:26:13 +0200
From:   Maxime Chevallier <maxime.chevallier@...tlin.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev <netdev@...r.kernel.org>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>,
        Gregory CLEMENT <gregory.clement@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Subject: Re: Link modes representation in phylib

Hello Andrew,

On Tue, 19 Jun 2018 17:21:55 +0200
Andrew Lunn <andrew@...n.ch> wrote:

>> What I propose is that we add 3 link_mode fields in phy_device, and keep
>> the legacy fields for now. It would be up to the driver to fill the new
>> "supported" field in config_init, kind of like what's done in the
>> marvell10g driver.  
>
>Hi Maxime
>
>You can do this conversion in the core. If features == 0, and some
>bits are set in the features link_mode, do the conversion at probe
>time. The same can be done for lp_advertising, when the call into the
>drivers read_status() has completed.

Thanks for the suggestion. I see how this can be done with
phydrv->supported and phydev->lp_advertising, however I'm not sure how
we should deal with the fact that ethernet drivers directly access
fields such as "advertising" or "supported".

Should we update the new fields in phy_device when functions such as
"phy_start" or "phy_start_aneg" are called, just in case the ethernet
driver modified the phydev->supported / phydev->advertising fields
in the meantime ?

My concern is that phylink will rely on the new link_mode
representation to be up-to-date, and ethernet drivers that aren't
converted to phylink, but link to a new PHY will rely on the legacy
representation.

That might be just fine if we take good care making sure both the
legacy and the new representation are well sync'd, but since I don't
have a good big-picture view of all this, I prefer having your opinion
first :)

>> Would that be acceptable ?  
>
>It sounds reasonable. Lets see what the code looks like.

Ok I'll be glad to send a series for that, I just want to make sure I
go in the right direction

Thanks,

Maxime