netdev - RE: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query support to PHYlib API

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]

Message-ID: <BL0PR11MB2913839BD93ACB61B06C30F4E70F2@BL0PR11MB2913.namprd11.prod.outlook.com>
Date: Wed, 17 Apr 2024 18:48:37 +0000
From: <Woojung.Huh@...rochip.com>
To: <o.rempel@...gutronix.de>, <alexandre.torgue@...s.st.com>,
	<joabreu@...opsys.com>, <davem@...emloft.net>, <andrew@...n.ch>,
	<hkallweit1@...il.com>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <mcoquelin.stm32@...il.com>,
	<Arun.Ramadoss@...rochip.com>, <richardcochran@...il.com>,
	<linux@...linux.org.uk>
CC: <kernel@...gutronix.de>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <UNGLinuxDriver@...rochip.com>,
	<linux-stm32@...md-mailman.stormreply.com>
Subject: RE: [PATCH net-next v1 1/4] net: phy: Add TimeSync delay query
 support to PHYlib API

Hi

> +/**
> + * phy_get_timesync_data_path_delays - get the TimeSync data path
> ingress/egress
> + *                                     delays
> + * @phydev: phy_device struct
> + * @tx_delay_ns: pointer to the transmit delay in nanoseconds
> + * @rx_delay_ns: pointer to the receive delay in nanoseconds
> + *
> + * This function is used to get the TimeSync data path ingress/egress
> delays
> + * as described in IEEE 802.3-2022 sections:
> + * 30.13.1.3 aTimeSyncDelayTXmax, 30.13.1.4 aTimeSyncDelayTXmin,
> + * 30.13.1.5 aTimeSyncDelayRXmax and 30.13.1.6 aTimeSyncDelayRXmin.
> + *
> + * The delays are returned in nanoseconds and can be used to compensate
> time
> + * added by the PHY to the PTP packets.
> + *
> + * Returns 0 on success, negative value on failure.
> + */
> +int phy_get_timesync_data_path_delays(struct phy_device *phydev,
> +                                     u64 *tx_delay_ns, u64 *rx_delay_ns)
> +{
> +       struct phy_timesync_delay tsd = { 0 };
> +       int err;
> +
> +       if (!phydev->drv->get_timesync_data_path_delays)
> +               return -EOPNOTSUPP;
> +
> +       if (!tx_delay_ns || !rx_delay_ns)
> +               return -EINVAL;
> +
> +       err = phydev->drv->get_timesync_data_path_delays(phydev, &tsd);
> +       if (err)
> +               return err;
> +
> +       if ((!tsd.tx_max_delay_ns && !tsd.tx_min_delay_ns) ||
> +           (!tsd.rx_max_delay_ns && !tsd.rx_min_delay_ns)) {
> +               phydev_err(phydev, "Invalid TimeSync data path delays\n");
> +               return -EINVAL;
> +       }

Because all 4 variables are u64, it can be zero technically.
I think the condition of "max >= min" could be better option for validation
because actual *_delay_ns is average of min and max value.

> +
> +       if (tsd.tx_max_delay_ns && tsd.tx_min_delay_ns)
> +               *tx_delay_ns = (tsd.tx_max_delay_ns + tsd.tx_min_delay_ns) /
> 2;
> +       else if (tsd.tx_max_delay_ns)
> +               *tx_delay_ns = tsd.tx_max_delay_ns;
> +       else
> +               *tx_delay_ns = tsd.tx_min_delay_ns;
> +
> +       if (tsd.rx_max_delay_ns && tsd.rx_min_delay_ns)
> +               *rx_delay_ns = (tsd.rx_max_delay_ns + tsd.rx_min_delay_ns) /
> 2;
> +       else if (tsd.rx_max_delay_ns)
> +               *rx_delay_ns = tsd.rx_max_delay_ns;
> +       else
> +               *rx_delay_ns = tsd.rx_min_delay_ns;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(phy_get_timesync_data_path_delays);
> +

Thanks.
Woojung