net: Change the API of PHY default timestamp to MAC
authorKory Maincent <kory.maincent@bootlin.com>
Tue, 14 Nov 2023 11:28:41 +0000 (12:28 +0100)
committerDavid S. Miller <davem@davemloft.net>
Sat, 18 Nov 2023 14:52:57 +0000 (14:52 +0000)
Change the API to select MAC default time stamping instead of the PHY.
Indeed the PHY is closer to the wire therefore theoretically it has less
delay than the MAC timestamping but the reality is different. Due to lower
time stamping clock frequency, latency in the MDIO bus and no PHC hardware
synchronization between different PHY, the PHY PTP is often less precise
than the MAC. The exception is for PHY designed specially for PTP case but
these devices are not very widespread. For not breaking the compatibility I
introduce a default_timestamp flag in phy_device that is set by the phy
driver to know we are using the old API behavior.

The phy_set_timestamp function is called at each call of phy_attach_direct.
In case of MAC driver using phylink this function is called when the
interface is turned up. Then if the interface goes down and up again the
last choice of timestamp will be overwritten by the default choice.
A solution could be to cache the timestamp status but it can bring other
issues. In case of SFP, if we change the module, it doesn't make sense to
blindly re-set the timestamp back to PHY, if the new module has a PHY with
mediocre timestamping capabilities.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
12 files changed:
drivers/net/phy/bcm-phy-ptp.c
drivers/net/phy/dp83640.c
drivers/net/phy/micrel.c
drivers/net/phy/mscc/mscc_ptp.c
drivers/net/phy/nxp-c45-tja11xx.c
drivers/net/phy/phy_device.c
include/linux/netdevice.h
include/linux/phy.h
net/core/dev.c
net/core/dev_ioctl.c
net/core/timestamping.c
net/ethtool/common.c

index 617d384d4551ac31dd8a7752b24c9087883e70d2..d3e825c951eef1b74068cb90c9970d379698b559 100644 (file)
@@ -931,6 +931,9 @@ struct bcm_ptp_private *bcm_ptp_probe(struct phy_device *phydev)
                return ERR_CAST(clock);
        priv->ptp_clock = clock;
 
+       /* Timestamp selected by default to keep legacy API */
+       phydev->default_timestamp = true;
+
        priv->phydev = phydev;
        bcm_ptp_init(priv);
 
index 5c42c47dc56469349a0162e35e928812d339746e..64fd1a109c0f6b46ee3891edd095c58fdbb2d873 100644 (file)
@@ -1450,6 +1450,9 @@ static int dp83640_probe(struct phy_device *phydev)
        phydev->mii_ts = &dp83640->mii_ts;
        phydev->priv = dp83640;
 
+       /* Timestamp selected by default to keep legacy API */
+       phydev->default_timestamp = true;
+
        spin_lock_init(&dp83640->rx_lock);
        skb_queue_head_init(&dp83640->rx_queue);
        skb_queue_head_init(&dp83640->tx_queue);
index bd4cd082662f419c6c6f15b6c9122aca6f2fea32..2b8dd0131926a0e913af0022a958e24141cd5746 100644 (file)
@@ -3158,6 +3158,9 @@ static void lan8814_ptp_init(struct phy_device *phydev)
        ptp_priv->mii_ts.ts_info  = lan8814_ts_info;
 
        phydev->mii_ts = &ptp_priv->mii_ts;
+
+       /* Timestamp selected by default to keep legacy API */
+       phydev->default_timestamp = true;
 }
 
 static int lan8814_ptp_probe_once(struct phy_device *phydev)
@@ -4586,6 +4589,9 @@ static int lan8841_probe(struct phy_device *phydev)
 
        phydev->mii_ts = &ptp_priv->mii_ts;
 
+       /* Timestamp selected by default to keep legacy API */
+       phydev->default_timestamp = true;
+
        return 0;
 }
 
index eb0b032cb613d1c91c798323664918707a3dd0a3..fd174eb06d4a62183703b153a40e674b31bc852e 100644 (file)
@@ -1570,6 +1570,8 @@ int vsc8584_ptp_probe(struct phy_device *phydev)
                return PTR_ERR(vsc8531->load_save);
        }
 
+       /* Timestamp selected by default to keep legacy API */
+       phydev->default_timestamp = true;
        vsc8531->ptp->phydev = phydev;
 
        return 0;
index 780ad353cf55774e8ea2aa5becb9d335d6592d3e..0515c7b979db04e9db9866477713010d83e837dd 100644 (file)
@@ -1658,6 +1658,9 @@ static int nxp_c45_probe(struct phy_device *phydev)
                priv->mii_ts.ts_info = nxp_c45_ts_info;
                phydev->mii_ts = &priv->mii_ts;
                ret = nxp_c45_init_ptp_clock(priv);
+
+               /* Timestamp selected by default to keep legacy API */
+               phydev->default_timestamp = true;
        } else {
                phydev_dbg(phydev, "PTP support not enabled even if the phy supports it");
        }
index 2ce74593d6e4a1d92c54de76e311eafedb78a222..8c4794631daa7f79566876517ce2c270dbca7960 100644 (file)
@@ -1411,6 +1411,26 @@ int phy_sfp_probe(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_sfp_probe);
 
+/**
+ * phy_set_timestamp - set the default selected timestamping device
+ * @dev: Pointer to net_device
+ * @phydev: Pointer to phy_device
+ *
+ * This is used to set default timestamping device taking into account
+ * the new API choice, which is selecting the timestamping from MAC by
+ * default if the phydev does not have default_timestamp flag enabled.
+ */
+static void phy_set_timestamp(struct net_device *dev, struct phy_device *phydev)
+{
+       const struct ethtool_ops *ops = dev->ethtool_ops;
+
+       if (!phy_has_tsinfo(phydev))
+               return;
+
+       if (!ops->get_ts_info || phydev->default_timestamp)
+               dev->ts_layer = PHY_TIMESTAMPING;
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1484,6 +1504,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
        phydev->phy_link_change = phy_link_change;
        if (dev) {
+               phy_set_timestamp(dev, phydev);
                phydev->attached_dev = dev;
                dev->phydev = phydev;
 
@@ -1812,6 +1833,22 @@ void phy_detach(struct phy_device *phydev)
 
        phy_suspend(phydev);
        if (dev) {
+               const struct ethtool_ops *ops = dev->ethtool_ops;
+               struct ethtool_ts_info ts_info = {0};
+
+               if (ops->get_ts_info) {
+                       ops->get_ts_info(dev, &ts_info);
+                       if ((ts_info.so_timestamping &
+                           SOF_TIMESTAMPING_HARDWARE_MASK) ==
+                           SOF_TIMESTAMPING_HARDWARE_MASK)
+                               dev->ts_layer = MAC_TIMESTAMPING;
+                       else if ((ts_info.so_timestamping &
+                                SOF_TIMESTAMPING_SOFTWARE_MASK) ==
+                                SOF_TIMESTAMPING_SOFTWARE_MASK)
+                               dev->ts_layer = SOFTWARE_TIMESTAMPING;
+               } else {
+                       dev->ts_layer = NO_TIMESTAMPING;
+               }
                phydev->attached_dev->phydev = NULL;
                phydev->attached_dev = NULL;
        }
index 2d840d7056f20ac54692aca1eb4bdf8fa6e82c9b..f020d2790c12fccff6a335bff4fa45ca4b2bd11b 100644 (file)
@@ -47,6 +47,7 @@
 #include <uapi/linux/if_bonding.h>
 #include <uapi/linux/pkt_cls.h>
 #include <uapi/linux/netdev.h>
+#include <uapi/linux/net_tstamp.h>
 #include <linux/hashtable.h>
 #include <linux/rbtree.h>
 #include <net/net_trackers.h>
@@ -2074,6 +2075,8 @@ enum netdev_ml_priv_type {
  *
  *     @dpll_pin: Pointer to the SyncE source pin of a DPLL subsystem,
  *                where the clock is recovered.
+ *     @ts_layer:      Tracks which network device
+ *                     performs packet time stamping.
  *
  *     FIXME: cleanup struct net_device such that network protocol info
  *     moves out.
@@ -2435,6 +2438,8 @@ struct net_device {
 #if IS_ENABLED(CONFIG_DPLL)
        struct dpll_pin         *dpll_pin;
 #endif
+
+       enum timestamping_layer ts_layer;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
index e5f1f41e399c70aef54ff78608f839399d177312..317def2a7843eb19cd01418f3616b374d7d0cacf 100644 (file)
@@ -604,6 +604,8 @@ struct macsec_ops;
  *                 handling shall be postponed until PHY has resumed
  * @irq_rerun: Flag indicating interrupts occurred while PHY was suspended,
  *             requiring a rerun of the interrupt handler after resume
+ * @default_timestamp: Flag indicating whether we are using the phy
+ *                    timestamp as the default one
  * @interface: enum phy_interface_t value
  * @skb: Netlink message for cable diagnostics
  * @nest: Netlink nest used for cable diagnostics
@@ -667,6 +669,8 @@ struct phy_device {
        unsigned irq_suspended:1;
        unsigned irq_rerun:1;
 
+       unsigned default_timestamp:1;
+
        int rate_matching;
 
        enum phy_state state;
index af53f6d838ce9e5ec73570d31ac39b9f36b1c5c5..05ce00632892654bba120de3d87843a0df40fa99 100644 (file)
@@ -10212,6 +10212,9 @@ int register_netdevice(struct net_device *dev)
            dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
                rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, 0, NULL);
 
+       if (dev->ethtool_ops->get_ts_info)
+               dev->ts_layer = MAC_TIMESTAMPING;
+
 out:
        return ret;
 
index 267cd00269d01ef620ac7b50027a0da0fb660985..bc8be9749376e3f7207cb2095257d3ae3289733c 100644 (file)
@@ -259,9 +259,7 @@ static int dev_eth_ioctl(struct net_device *dev,
  * @dev: Network device
  * @cfg: Timestamping configuration structure
  *
- * Helper for enforcing a common policy that phylib timestamping, if available,
- * should take precedence in front of hardware timestamping provided by the
- * netdev.
+ * Helper for calling the selected hardware provider timestamping.
  *
  * Note: phy_mii_ioctl() only handles SIOCSHWTSTAMP (not SIOCGHWTSTAMP), and
  * there only exists a phydev->mii_ts->hwtstamp() method. So this will return
@@ -271,10 +269,14 @@ static int dev_eth_ioctl(struct net_device *dev,
 static int dev_get_hwtstamp_phylib(struct net_device *dev,
                                   struct kernel_hwtstamp_config *cfg)
 {
-       if (phy_has_hwtstamp(dev->phydev))
+       enum timestamping_layer ts_layer = dev->ts_layer;
+
+       if (ts_layer == PHY_TIMESTAMPING)
                return phy_hwtstamp_get(dev->phydev, cfg);
+       else if (ts_layer == MAC_TIMESTAMPING)
+               return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
 
-       return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
+       return -EOPNOTSUPP;
 }
 
 static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
@@ -315,9 +317,8 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
  * @cfg: Timestamping configuration structure
  * @extack: Netlink extended ack message structure, for error reporting
  *
- * Helper for enforcing a common policy that phylib timestamping, if available,
- * should take precedence in front of hardware timestamping provided by the
- * netdev. If the netdev driver needs to perform specific actions even for PHY
+ * Helper for calling the selected hardware provider timestamping.
+ * If the netdev driver needs to perform specific actions even for PHY
  * timestamping to work properly (a switch port must trap the timestamped
  * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
  * dev->priv_flags.
@@ -327,20 +328,26 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
                            struct netlink_ext_ack *extack)
 {
        const struct net_device_ops *ops = dev->netdev_ops;
-       bool phy_ts = phy_has_hwtstamp(dev->phydev);
+       enum timestamping_layer ts_layer = dev->ts_layer;
        struct kernel_hwtstamp_config old_cfg = {};
        bool changed = false;
        int err;
 
-       cfg->source = phy_ts ? PHY_TIMESTAMPING : MAC_TIMESTAMPING;
+       cfg->source = ts_layer;
+
+       if (ts_layer != PHY_TIMESTAMPING &&
+           ts_layer != MAC_TIMESTAMPING)
+               return -EOPNOTSUPP;
 
-       if (phy_ts && (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
+       if (ts_layer == PHY_TIMESTAMPING &&
+           dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS) {
                err = ops->ndo_hwtstamp_get(dev, &old_cfg);
                if (err)
                        return err;
        }
 
-       if (!phy_ts || (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
+       if (ts_layer == MAC_TIMESTAMPING ||
+           dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS) {
                err = ops->ndo_hwtstamp_set(dev, cfg, extack);
                if (err) {
                        if (extack->_msg)
@@ -349,10 +356,11 @@ int dev_set_hwtstamp_phylib(struct net_device *dev,
                }
        }
 
-       if (phy_ts && (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS))
+       if (ts_layer == PHY_TIMESTAMPING &&
+           dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)
                changed = kernel_hwtstamp_config_changed(&old_cfg, cfg);
 
-       if (phy_ts) {
+       if (ts_layer == PHY_TIMESTAMPING) {
                err = phy_hwtstamp_set(dev->phydev, cfg, extack);
                if (err) {
                        if (changed)
index 04840697fe79fa5e1f4e2908cac8cb8ad13ffc40..5cf51a523fb34b0951ad15818b1b060a4b7ec76b 100644 (file)
@@ -21,6 +21,7 @@ static unsigned int classify(const struct sk_buff *skb)
 
 void skb_clone_tx_timestamp(struct sk_buff *skb)
 {
+       enum timestamping_layer ts_layer;
        struct mii_timestamper *mii_ts;
        struct sk_buff *clone;
        unsigned int type;
@@ -28,6 +29,10 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
        if (!skb->sk)
                return;
 
+       ts_layer = skb->dev->ts_layer;
+       if (ts_layer != PHY_TIMESTAMPING)
+               return;
+
        type = classify(skb);
        if (type == PTP_CLASS_NONE)
                return;
@@ -44,12 +49,17 @@ EXPORT_SYMBOL_GPL(skb_clone_tx_timestamp);
 
 bool skb_defer_rx_timestamp(struct sk_buff *skb)
 {
+       enum timestamping_layer ts_layer;
        struct mii_timestamper *mii_ts;
        unsigned int type;
 
        if (!skb->dev || !skb->dev->phydev || !skb->dev->phydev->mii_ts)
                return false;
 
+       ts_layer = skb->dev->ts_layer;
+       if (ts_layer != PHY_TIMESTAMPING)
+               return false;
+
        if (skb_headroom(skb) < ETH_HLEN)
                return false;
 
index 11d8797f63f666faac7c5033e446415faab46f5d..9f6e3b2c74e22c3a6b1b35adcfe9a05a071a48ac 100644 (file)
@@ -633,13 +633,28 @@ int __ethtool_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
 {
        const struct ethtool_ops *ops = dev->ethtool_ops;
        struct phy_device *phydev = dev->phydev;
+       enum timestamping_layer ts_layer;
+       int ret;
 
        memset(info, 0, sizeof(*info));
        info->cmd = ETHTOOL_GET_TS_INFO;
 
-       if (phy_has_tsinfo(phydev))
+       ts_layer = dev->ts_layer;
+       if (ts_layer == SOFTWARE_TIMESTAMPING) {
+               ret = ops->get_ts_info(dev, info);
+               if (ret)
+                       return ret;
+               info->so_timestamping &= ~SOF_TIMESTAMPING_HARDWARE_MASK;
+               info->phc_index = -1;
+               info->rx_filters = 0;
+               info->tx_types = 0;
+               return 0;
+       }
+
+       if (ts_layer == PHY_TIMESTAMPING)
                return phy_ts_info(phydev, info);
-       if (ops->get_ts_info)
+
+       if (ts_layer == MAC_TIMESTAMPING)
                return ops->get_ts_info(dev, info);
 
        info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |