netdev - Re: [PATCH net-next 1/3] net: mdio: add support for passing a PTP system timestamp to the mii

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: <20190819131736.GD8981@lunn.ch>
Date:   Mon, 19 Aug 2019 15:17:36 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Hubert Feurstein <h.feurstein@...il.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Richard Cochran <richardcochran@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next 1/3] net: mdio: add support for passing a PTP
 system timestamp to the mii_bus driver

On Fri, Aug 16, 2019 at 06:31:55PM +0200, Hubert Feurstein wrote:
> In order to improve the synchronisation precision of phc2sys (from
> the linuxptp project) for devices like switches which are attached
> to the MDIO bus, it is necessary the get the system timestamps as
> close as possible to the access which causes the PTP timestamp
> register to be snapshotted in the switch hardware. Usually this is
> triggered by an MDIO write access, the snapshotted timestamp is then
> transferred by several MDIO reads.
> 
> This patch adds the required infrastructure to solve the problem described
> above.
> 
> Signed-off-by: Hubert Feurstein <h.feurstein@...il.com>
> ---
>  drivers/net/phy/mdio_bus.c | 105 +++++++++++++++++++++++++++++++++++++
>  include/linux/mdio.h       |   7 +++
>  include/linux/phy.h        |  25 +++++++++
>  3 files changed, 137 insertions(+)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index bd04fe762056..167a21f267fa 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -34,6 +34,7 @@
>  #include <linux/phy.h>
>  #include <linux/io.h>
>  #include <linux/uaccess.h>
> +#include <linux/ptp_clock_kernel.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/mdio.h>
> @@ -697,6 +698,110 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
>  }
>  EXPORT_SYMBOL(mdiobus_write);
>  
> +/**
> + * __mdiobus_write_sts - Unlocked version of the mdiobus_write_sts function
> + * @bus: the mii_bus struct
> + * @addr: the phy address
> + * @regnum: register number to write
> + * @val: value to write to @regnum
> + * @sts: the ptp system timestamp
> + *
> + * Write a MDIO bus register and request the MDIO bus driver to take the
> + * system timestamps when sts-pointer is valid. When the bus driver doesn't
> + * support this, the timestamps are taken in this function instead.
> + *
> + * In order to improve the synchronisation precision of phc2sys (from
> + * the linuxptp project) for devices like switches which are attached
> + * to the MDIO bus, it is necessary the get the system timestamps as
> + * close as possible to the access which causes the PTP timestamp
> + * register to be snapshotted in the switch hardware. Usually this is
> + * triggered by an MDIO write access, the snapshotted timestamp is then
> + * transferred by several MDIO reads.
> + *
> + * Caller must hold the mdio bus lock.
> + *
> + * NOTE: MUST NOT be called from interrupt context.
> + */
> +int __mdiobus_write_sts(struct mii_bus *bus, int addr, u32 regnum, u16 val,
> +			struct ptp_system_timestamp *sts)
> +{
> +	int retval;
> +
> +	WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
> +
> +	if (!bus->ptp_sts_supported)
> +		ptp_read_system_prets(sts);

How expensive is ptp_read_system_prets()? My original suggestion was
to unconditionally call it here, and then let the driver overwrite it
if it supports finer grained time stamping. MDIO is slow, so as long
as ptp_read_system_prets() is not too expensive, i prefer KISS.

   Andrew