Based on previous RFCs from Maxim Georgiev:
https://lore.kernel.org/netdev/[email protected]/
this series attempts to introduce new API for the hardware timestamping
control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
I don't have any board with phylib hardware timestamping, so I would
appreciate testing (especially on lan966x, the most intricate
conversion). I was, however, able to test netdev level timestamping,
because I also have some more unsubmitted conversions in progress:
https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
I hope that the concerns expressed in the comments of previous series
were addressed, and that Köry Maincent's series:
https://lore.kernel.org/netdev/[email protected]/
can make progress in parallel with the conversion of the rest of drivers.
Maxim Georgiev (5):
net: add NDOs for configuring hardware timestamping
net: add hwtstamping helpers for stackable net devices
net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
Vladimir Oltean (5):
net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: fec: delete fec_ptp_disable_hwts()
net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
converted drivers
drivers/net/bonding/bond_main.c | 105 ++++++----
drivers/net/ethernet/freescale/fec.h | 6 +-
drivers/net/ethernet/freescale/fec_main.c | 41 ++--
drivers/net/ethernet/freescale/fec_ptp.c | 43 ++--
.../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++--
.../ethernet/microchip/lan966x/lan966x_main.h | 12 +-
.../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++--
.../ethernet/microchip/sparx5/sparx5_main.h | 9 +-
.../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++-
.../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++-
drivers/net/macvlan.c | 34 ++--
include/linux/net_tstamp.h | 28 +++
include/linux/netdevice.h | 25 +++
net/8021q/vlan_dev.c | 27 ++-
net/core/dev_ioctl.c | 183 +++++++++++++++++-
15 files changed, 480 insertions(+), 187 deletions(-)
--
2.34.1
From: Maxim Georgiev <[email protected]>
The stackable net devices with hwtstamping support (vlan, macvlan,
bonding) only pass the hwtstamping ops to the lower (real) device.
These drivers are the first that need to be converted to the new
timestamping API, because if they aren't prepared to handle that,
then no real device driver cannot be converted to the new API either.
After studying what vlan_dev_ioctl(), macvlan_eth_ioctl() and
bond_eth_ioctl() have in common, here we propose two generic
implementations of ndo_hwtstamp_get() and ndo_hwtstamp_set() which
can be called by those 3 drivers, with "dev" being their lower device.
These helpers cover both cases, when the lower driver is converted to
the new API or unconverted.
We need some hacks in case of an unconverted driver, namely to stuff
some pointers in struct kernel_hwtstamp_config which shouldn't have
been there (since the new API isn't supposed to need it). These will
be removed when all drivers will have been converted to the new API.
Signed-off-by: Maxim Georgiev <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Reword commit message
- Convert "int kernel_flags" to "bool copied_to_user"
- Minor style fixups, and a refactor of duplicated code into a common
generic_hwtstamp_ioctl_lower()
Changes in v6:
- Patch title was updated. No code changes.
Changes in v5:
- kernel_hwtstamp_config kdoc is updated with the new field
descriptions.
Changes in V4:
- Introducing KERNEL_HWTSTAMP_FLAG_IFR_RESULT flag indicating that
the operation results are returned in the ifr referred by
struct kernel_hwtstamp_config instead of kernel_hwtstamp_config
glags/tx_type/rx_filter fields.
- Implementing generic_hwtstamp_set/set_lower() functions
which will be used by vlan, maxvlan, bond and potentially
other drivers translating ndo_hwtstamp_set/set calls to
lower level drivers.
include/linux/net_tstamp.h | 6 +++
include/linux/netdevice.h | 5 +++
net/core/dev_ioctl.c | 75 ++++++++++++++++++++++++++++++++++----
3 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
index 7c59824f43f5..03e922814851 100644
--- a/include/linux/net_tstamp.h
+++ b/include/linux/net_tstamp.h
@@ -11,6 +11,10 @@
* @flags: see struct hwtstamp_config
* @tx_type: see struct hwtstamp_config
* @rx_filter: see struct hwtstamp_config
+ * @ifr: pointer to ifreq structure from the original ioctl request, to pass to
+ * a legacy implementation of a lower driver
+ * @copied_to_user: request was passed to a legacy implementation which already
+ * copied the ioctl request back to user space
*
* Prefer using this structure for in-kernel processing of hardware
* timestamping configuration, over the inextensible struct hwtstamp_config
@@ -20,6 +24,8 @@ struct kernel_hwtstamp_config {
int flags;
int tx_type;
int rx_filter;
+ struct ifreq *ifr;
+ bool copied_to_user;
};
static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg,
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 17a442ed683b..ca3bcf2257c0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3949,6 +3949,11 @@ int put_user_ifreq(struct ifreq *ifr, void __user *arg);
int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
void __user *data, bool *need_copyout);
int dev_ifconf(struct net *net, struct ifconf __user *ifc);
+int generic_hwtstamp_get_lower(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_cfg);
+int generic_hwtstamp_set_lower(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_cfg,
+ struct netlink_ext_ack *extack);
int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata);
unsigned int dev_get_flags(const struct net_device *);
int __dev_change_flags(struct net_device *dev, unsigned int flags,
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 10c0e173b38b..d0223ecd6f6f 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -265,14 +265,20 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
if (!netif_device_present(dev))
return -ENODEV;
+ kernel_cfg.ifr = ifr;
err = ops->ndo_hwtstamp_get(dev, &kernel_cfg);
if (err)
return err;
- hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
+ /* If the request was resolved through an unconverted driver, omit
+ * the copy_to_user(), since the implementation has already done that
+ */
+ if (!kernel_cfg.copied_to_user) {
+ hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
- if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
- return -EFAULT;
+ if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+ return -EFAULT;
+ }
return 0;
}
@@ -280,7 +286,7 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
{
const struct net_device_ops *ops = dev->netdev_ops;
- struct kernel_hwtstamp_config kernel_cfg;
+ struct kernel_hwtstamp_config kernel_cfg = {};
struct netlink_ext_ack extack = {};
struct hwtstamp_config cfg;
int err;
@@ -289,6 +295,7 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
return -EFAULT;
hwtstamp_config_to_kernel(&kernel_cfg, &cfg);
+ kernel_cfg.ifr = ifr;
err = net_hwtstamp_validate(&kernel_cfg);
if (err)
@@ -317,14 +324,68 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
/* The driver may have modified the configuration, so copy the
* updated version of it back to user space
*/
- hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
+ if (!kernel_cfg.copied_to_user) {
+ hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
- if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
- return -EFAULT;
+ if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static int generic_hwtstamp_ioctl_lower(struct net_device *dev, int cmd,
+ struct kernel_hwtstamp_config *kernel_cfg)
+{
+ struct ifreq ifrr;
+ int err;
+
+ strscpy_pad(ifrr.ifr_name, dev->name, IFNAMSIZ);
+ ifrr.ifr_ifru = kernel_cfg->ifr->ifr_ifru;
+
+ err = dev_eth_ioctl(dev, &ifrr, cmd);
+ if (err)
+ return err;
+
+ kernel_cfg->ifr->ifr_ifru = ifrr.ifr_ifru;
+ kernel_cfg->copied_to_user = true;
return 0;
}
+int generic_hwtstamp_get_lower(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_cfg)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+
+ if (!netif_device_present(dev))
+ return -ENODEV;
+
+ if (ops->ndo_hwtstamp_get)
+ return ops->ndo_hwtstamp_get(dev, kernel_cfg);
+
+ /* Legacy path: unconverted lower driver */
+ return generic_hwtstamp_ioctl_lower(dev, SIOCGHWTSTAMP, kernel_cfg);
+}
+EXPORT_SYMBOL(generic_hwtstamp_get_lower);
+
+int generic_hwtstamp_set_lower(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_cfg,
+ struct netlink_ext_ack *extack)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+
+ if (!netif_device_present(dev))
+ return -ENODEV;
+
+ if (ops->ndo_hwtstamp_set)
+ return ops->ndo_hwtstamp_set(dev, kernel_cfg, extack);
+
+ /* Legacy path: unconverted lower driver */
+ return generic_hwtstamp_ioctl_lower(dev, SIOCSHWTSTAMP, kernel_cfg);
+}
+EXPORT_SYMBOL(generic_hwtstamp_set_lower);
+
static int dev_siocbond(struct net_device *dev,
struct ifreq *ifr, unsigned int cmd)
{
--
2.34.1
From: Maxim Georgiev <[email protected]>
macvlan is one of the stackable net devices which pass the hardware
timestamping ops to the real device through ndo_eth_ioctl(). This
prevents converting any device driver to the new hwtimestamping API
without regressions.
Remove that limitation in macvlan by using the newly introduced helpers
for timestamping through lower devices, that handle both the new and the
old driver API.
macvlan only implements ndo_eth_ioctl() for these 2 operations, so
delete that method.
Signed-off-by: Maxim Georgiev <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Split vlan and macvlan to separate patches
- Reword commit message
Changes in v6:
- Patch title was updated. No code changes.
Changes in v5:
- Re-introduced the net namespace check which
was dropped in v4.
Changes in v4:
- Moved hw timestamp get/set request processing logic
from vlan_dev_ioctl() to .ndo_hwtstamp_get/set callbacks.
- Use the shared generic_hwtstamp_get/set_lower() functions
to handle ndo_hwtstamp_get/set requests.
- Apply the same changes to macvlan driver.
drivers/net/macvlan.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 4a53debf9d7c..01acb57aa40c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -868,31 +868,24 @@ static int macvlan_change_mtu(struct net_device *dev, int new_mtu)
return 0;
}
-static int macvlan_eth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
+static int macvlan_hwtstamp_get(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg)
{
struct net_device *real_dev = macvlan_dev_real_dev(dev);
- const struct net_device_ops *ops = real_dev->netdev_ops;
- struct ifreq ifrr;
- int err = -EOPNOTSUPP;
- strscpy(ifrr.ifr_name, real_dev->name, IFNAMSIZ);
- ifrr.ifr_ifru = ifr->ifr_ifru;
+ return generic_hwtstamp_get_lower(real_dev, cfg);
+}
- switch (cmd) {
- case SIOCSHWTSTAMP:
- if (!net_eq(dev_net(dev), &init_net))
- break;
- fallthrough;
- case SIOCGHWTSTAMP:
- if (netif_device_present(real_dev) && ops->ndo_eth_ioctl)
- err = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
- break;
- }
+static int macvlan_hwtstamp_set(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *real_dev = macvlan_dev_real_dev(dev);
- if (!err)
- ifr->ifr_ifru = ifrr.ifr_ifru;
+ if (!net_eq(dev_net(dev), &init_net))
+ return -EOPNOTSUPP;
- return err;
+ return generic_hwtstamp_set_lower(real_dev, cfg, extack);
}
/*
@@ -1193,7 +1186,6 @@ static const struct net_device_ops macvlan_netdev_ops = {
.ndo_stop = macvlan_stop,
.ndo_start_xmit = macvlan_start_xmit,
.ndo_change_mtu = macvlan_change_mtu,
- .ndo_eth_ioctl = macvlan_eth_ioctl,
.ndo_fix_features = macvlan_fix_features,
.ndo_change_rx_flags = macvlan_change_rx_flags,
.ndo_set_mac_address = macvlan_set_mac_address,
@@ -1212,6 +1204,8 @@ static const struct net_device_ops macvlan_netdev_ops = {
#endif
.ndo_get_iflink = macvlan_dev_get_iflink,
.ndo_features_check = passthru_features_check,
+ .ndo_hwtstamp_get = macvlan_hwtstamp_get,
+ .ndo_hwtstamp_set = macvlan_hwtstamp_set,
};
static void macvlan_dev_free(struct net_device *dev)
--
2.34.1
Commit 340746398b67 ("net: fec: fix hardware time stamping by external
devices") was overly cautious with calling fec_ptp_disable_hwts() when
cmd == SIOCSHWTSTAMP and use_fec_hwts == false, because use_fec_hwts is
based on a runtime invariant (phy_has_hwtstamp()). Thus, if use_fec_hwts
is false, then fep->hwts_tx_en and fep->hwts_rx_en cannot be changed at
runtime; their values depend on the initial memory allocation, which
already sets them to zeroes.
If the core will ever gain support for switching timestamping layers,
it will arrange for a more organized calling convention and disable
timestamping in the previous layer as a first step. This means that the
code in the FEC driver is not necessary in any case.
The purpose of this change is to arrange the phy_has_hwtstamp() code in
a way in which it can be refactored away into generic logic.
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Patch is new
drivers/net/ethernet/freescale/fec.h | 1 -
drivers/net/ethernet/freescale/fec_main.c | 5 +----
drivers/net/ethernet/freescale/fec_ptp.c | 12 ------------
3 files changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index d4ae0e7c0a44..e273129d44c9 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -690,7 +690,6 @@ struct fec_enet_private {
void fec_ptp_init(struct platform_device *pdev, int irq_idx);
void fec_ptp_stop(struct platform_device *pdev);
void fec_ptp_start_cyclecounter(struct net_device *ndev);
-void fec_ptp_disable_hwts(struct net_device *ndev);
int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config,
struct netlink_ext_ack *extack);
void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config *config);
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c0b68fc3ec8b..08744e8164e3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3878,11 +3878,8 @@ static int fec_hwtstamp_set(struct net_device *ndev,
struct fec_enet_private *fep = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
- if (phy_has_hwtstamp(phydev)) {
- fec_ptp_disable_hwts(ndev);
-
+ if (phy_has_hwtstamp(phydev))
return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
- }
if (!fep->bufdesc_ex)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 3f53b8ae57dd..55cf98557e02 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -606,18 +606,6 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp,
}
}
-/**
- * fec_ptp_disable_hwts - disable hardware time stamping
- * @ndev: pointer to net_device
- */
-void fec_ptp_disable_hwts(struct net_device *ndev)
-{
- struct fec_enet_private *fep = netdev_priv(ndev);
-
- fep->hwts_tx_en = 0;
- fep->hwts_rx_en = 0;
-}
-
int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config,
struct netlink_ext_ack *extack)
{
--
2.34.1
From: Maxim Georgiev <[email protected]>
Current hardware timestamping API for NICs requires implementing
.ndo_eth_ioctl() for SIOCGHWTSTAMP and SIOCSHWTSTAMP.
That API has some boilerplate such as request parameter translation
between user and kernel address spaces, handling possible translation
failures correctly, etc. Since it is the same all across the board, it
would be desirable to handle it through generic code.
Here we introduce .ndo_hwtstamp_get() and .ndo_hwtstamp_set(), which
implement that boilerplate and allow drivers to just act upon requests.
Suggested-by: Jakub Kicinski <[email protected]>
Signed-off-by: Maxim Georgiev <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Remove extack from ndo_hwtstamp_get()
- Reword commit message
Changes in v6:
- The patch title was updated. No code changes.
Changes in v4:
- Renamed hwtstamp_kernel_to_config() function to
hwtstamp_config_from_kernel().
- Added struct kernel_hwtstamp_config zero initialization
in dev_get_hwtstamp() and in dev_get_hwtstamp().
Changes in v3:
- Moved individual driver conversions to separate patches
Changes in v2:
- Introduced kernel_hwtstamp_config structure
- Added netlink_ext_ack* and kernel_hwtstamp_config* as NDO hw timestamp
function parameters
- Reodered function variable declarations in dev_hwtstamp()
- Refactored error handling logic in dev_hwtstamp()
- Split dev_hwtstamp() into GET and SET versions
- Changed net_hwtstamp_validate() to accept struct hwtstamp_config *
as a parameter
include/linux/net_tstamp.h | 8 +++++++
include/linux/netdevice.h | 16 +++++++++++++
net/core/dev_ioctl.c | 46 ++++++++++++++++++++++++++++++++++++--
3 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
index fd67f3cc0c4b..7c59824f43f5 100644
--- a/include/linux/net_tstamp.h
+++ b/include/linux/net_tstamp.h
@@ -30,4 +30,12 @@ static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kern
kernel_cfg->rx_filter = cfg->rx_filter;
}
+static inline void hwtstamp_config_from_kernel(struct hwtstamp_config *cfg,
+ const struct kernel_hwtstamp_config *kernel_cfg)
+{
+ cfg->flags = kernel_cfg->flags;
+ cfg->tx_type = kernel_cfg->tx_type;
+ cfg->rx_filter = kernel_cfg->rx_filter;
+}
+
#endif /* _LINUX_NET_TIMESTAMPING_H_ */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b828c7a75be2..17a442ed683b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -57,6 +57,7 @@
struct netpoll_info;
struct device;
struct ethtool_ops;
+struct kernel_hwtstamp_config;
struct phy_device;
struct dsa_port;
struct ip_tunnel_parm;
@@ -1418,6 +1419,16 @@ struct netdev_net_notifier {
* Get hardware timestamp based on normal/adjustable time or free running
* cycle counter. This function is required if physical clock supports a
* free running cycle counter.
+ *
+ * int (*ndo_hwtstamp_get)(struct net_device *dev,
+ * struct kernel_hwtstamp_config *kernel_config);
+ * Get the currently configured hardware timestamping parameters for the
+ * NIC device.
+ *
+ * int (*ndo_hwtstamp_set)(struct net_device *dev,
+ * struct kernel_hwtstamp_config *kernel_config,
+ * struct netlink_ext_ack *extack);
+ * Change the hardware timestamping parameters for NIC device.
*/
struct net_device_ops {
int (*ndo_init)(struct net_device *dev);
@@ -1652,6 +1663,11 @@ struct net_device_ops {
ktime_t (*ndo_get_tstamp)(struct net_device *dev,
const struct skb_shared_hwtstamps *hwtstamps,
bool cycles);
+ int (*ndo_hwtstamp_get)(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_config);
+ int (*ndo_hwtstamp_set)(struct net_device *dev,
+ struct kernel_hwtstamp_config *kernel_config,
+ struct netlink_ext_ack *extack);
};
struct xdp_metadata_ops {
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 3730945ee294..10c0e173b38b 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -254,11 +254,32 @@ static int dev_eth_ioctl(struct net_device *dev,
static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
{
- return dev_eth_ioctl(dev, ifr, SIOCGHWTSTAMP);
+ const struct net_device_ops *ops = dev->netdev_ops;
+ struct kernel_hwtstamp_config kernel_cfg = {};
+ struct hwtstamp_config cfg;
+ int err;
+
+ if (!ops->ndo_hwtstamp_get)
+ return dev_eth_ioctl(dev, ifr, SIOCGHWTSTAMP); /* legacy */
+
+ if (!netif_device_present(dev))
+ return -ENODEV;
+
+ err = ops->ndo_hwtstamp_get(dev, &kernel_cfg);
+ if (err)
+ return err;
+
+ hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
+
+ if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+ return -EFAULT;
+
+ return 0;
}
static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
{
+ const struct net_device_ops *ops = dev->netdev_ops;
struct kernel_hwtstamp_config kernel_cfg;
struct netlink_ext_ack extack = {};
struct hwtstamp_config cfg;
@@ -280,7 +301,28 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
return err;
}
- return dev_eth_ioctl(dev, ifr, SIOCSHWTSTAMP);
+ if (!ops->ndo_hwtstamp_set)
+ return dev_eth_ioctl(dev, ifr, SIOCSHWTSTAMP); /* legacy */
+
+ if (!netif_device_present(dev))
+ return -ENODEV;
+
+ err = ops->ndo_hwtstamp_set(dev, &kernel_cfg, &extack);
+ if (err) {
+ if (extack._msg)
+ netdev_err(dev, "%s\n", extack._msg);
+ return err;
+ }
+
+ /* The driver may have modified the configuration, so copy the
+ * updated version of it back to user space
+ */
+ hwtstamp_config_from_kernel(&cfg, &kernel_cfg);
+
+ if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+ return -EFAULT;
+
+ return 0;
}
static int dev_siocbond(struct net_device *dev,
--
2.34.1
From: Maxim Georgiev <[email protected]>
8021q is one of the stackable net devices which pass the hardware
timestamping ops to the real device through ndo_eth_ioctl(). This
prevents converting any device driver to the new hwtimestamping API
without regressions.
Remove that limitation in the vlan driver by using the newly introduced
helpers for timestamping through lower devices, that handle both the new
and the old driver API.
Signed-off-by: Maxim Georgiev <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Split vlan and macvlan to separate patches
- Reword commit message
Changes in v6:
- Patch title was updated. No code changes.
Changes in v5:
- Re-introduced the net namespace check which
was dropped in v4.
Changes in v4:
- Moved hw timestamp get/set request processing logic
from vlan_dev_ioctl() to .ndo_hwtstamp_get/set callbacks.
- Use the shared generic_hwtstamp_get/set_lower() functions
to handle ndo_hwtstamp_get/set requests.
- Apply the same changes to macvlan driver.
net/8021q/vlan_dev.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b90781b9ece6..2a7f1b15714a 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -354,6 +354,26 @@ static int vlan_dev_set_mac_address(struct net_device *dev, void *p)
return 0;
}
+static int vlan_hwtstamp_get(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg)
+{
+ struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
+
+ return generic_hwtstamp_get_lower(real_dev, cfg);
+}
+
+static int vlan_hwtstamp_set(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
+
+ if (!net_eq(dev_net(dev), dev_net(real_dev)))
+ return -EOPNOTSUPP;
+
+ return generic_hwtstamp_set_lower(real_dev, cfg, extack);
+}
+
static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
@@ -365,14 +385,9 @@ static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
ifrr.ifr_ifru = ifr->ifr_ifru;
switch (cmd) {
- case SIOCSHWTSTAMP:
- if (!net_eq(dev_net(dev), dev_net(real_dev)))
- break;
- fallthrough;
case SIOCGMIIPHY:
case SIOCGMIIREG:
case SIOCSMIIREG:
- case SIOCGHWTSTAMP:
if (netif_device_present(real_dev) && ops->ndo_eth_ioctl)
err = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
break;
@@ -1081,6 +1096,8 @@ static const struct net_device_ops vlan_netdev_ops = {
.ndo_fix_features = vlan_dev_fix_features,
.ndo_get_iflink = vlan_dev_get_iflink,
.ndo_fill_forward_path = vlan_dev_fill_forward_path,
+ .ndo_hwtstamp_get = vlan_hwtstamp_get,
+ .ndo_hwtstamp_set = vlan_hwtstamp_set,
};
static void vlan_dev_free(struct net_device *dev)
--
2.34.1
The hardware timestamping through ndo_eth_ioctl() is going away.
Convert the FEC driver to the new API before that can be removed.
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Patch is new
drivers/net/ethernet/freescale/fec.h | 5 ++-
drivers/net/ethernet/freescale/fec_main.c | 52 +++++++++++++++++------
drivers/net/ethernet/freescale/fec_ptp.c | 31 +++++---------
3 files changed, 52 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 9939ccafb556..d4ae0e7c0a44 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -691,8 +691,9 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx);
void fec_ptp_stop(struct platform_device *pdev);
void fec_ptp_start_cyclecounter(struct net_device *ndev);
void fec_ptp_disable_hwts(struct net_device *ndev);
-int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
-int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
+int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack);
+void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config *config);
/****************************************************************************/
#endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 1280da699fa3..c0b68fc3ec8b 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3196,7 +3196,6 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
{
- struct fec_enet_private *fep = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
if (!netif_running(ndev))
@@ -3205,19 +3204,6 @@ static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
if (!phydev)
return -ENODEV;
- if (fep->bufdesc_ex) {
- bool use_fec_hwts = !phy_has_hwtstamp(phydev);
-
- if (cmd == SIOCSHWTSTAMP) {
- if (use_fec_hwts)
- return fec_ptp_set(ndev, rq);
- fec_ptp_disable_hwts(ndev);
- } else if (cmd == SIOCGHWTSTAMP) {
- if (use_fec_hwts)
- return fec_ptp_get(ndev, rq);
- }
- }
-
return phy_mii_ioctl(phydev, rq, cmd);
}
@@ -3868,6 +3854,42 @@ static int fec_enet_xdp_xmit(struct net_device *dev,
return sent_frames;
}
+static int fec_hwtstamp_get(struct net_device *ndev,
+ struct kernel_hwtstamp_config *config)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ struct phy_device *phydev = ndev->phydev;
+
+ if (phy_has_hwtstamp(phydev))
+ return phy_mii_ioctl(phydev, config->ifr, SIOCGHWTSTAMP);
+
+ if (!fep->bufdesc_ex)
+ return -EOPNOTSUPP;
+
+ fec_ptp_get(ndev, config);
+
+ return 0;
+}
+
+static int fec_hwtstamp_set(struct net_device *ndev,
+ struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ struct phy_device *phydev = ndev->phydev;
+
+ if (phy_has_hwtstamp(phydev)) {
+ fec_ptp_disable_hwts(ndev);
+
+ return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
+ }
+
+ if (!fep->bufdesc_ex)
+ return -EOPNOTSUPP;
+
+ return fec_ptp_set(ndev, config, extack);
+}
+
static const struct net_device_ops fec_netdev_ops = {
.ndo_open = fec_enet_open,
.ndo_stop = fec_enet_close,
@@ -3884,6 +3906,8 @@ static const struct net_device_ops fec_netdev_ops = {
.ndo_set_features = fec_set_features,
.ndo_bpf = fec_enet_bpf,
.ndo_xdp_xmit = fec_enet_xdp_xmit,
+ .ndo_hwtstamp_get = fec_hwtstamp_get,
+ .ndo_hwtstamp_set = fec_hwtstamp_set,
};
static const unsigned short offset_des_active_rxq[] = {
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index ab86bb8562ef..3f53b8ae57dd 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -618,16 +618,12 @@ void fec_ptp_disable_hwts(struct net_device *ndev)
fep->hwts_rx_en = 0;
}
-int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr)
+int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config,
+ struct netlink_ext_ack *extack)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct hwtstamp_config config;
-
- if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
- return -EFAULT;
-
- switch (config.tx_type) {
+ switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
fep->hwts_tx_en = 0;
break;
@@ -638,33 +634,28 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr)
return -ERANGE;
}
- switch (config.rx_filter) {
+ switch (config->rx_filter) {
case HWTSTAMP_FILTER_NONE:
fep->hwts_rx_en = 0;
break;
default:
fep->hwts_rx_en = 1;
- config.rx_filter = HWTSTAMP_FILTER_ALL;
+ config->rx_filter = HWTSTAMP_FILTER_ALL;
break;
}
- return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
- -EFAULT : 0;
+ return 0;
}
-int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr)
+void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config *config)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct hwtstamp_config config;
-
- config.flags = 0;
- config.tx_type = fep->hwts_tx_en ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
- config.rx_filter = (fep->hwts_rx_en ?
- HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE);
- return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
- -EFAULT : 0;
+ config->flags = 0;
+ config->tx_type = fep->hwts_tx_en ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
+ config->rx_filter = (fep->hwts_rx_en ?
+ HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE);
}
/*
--
2.34.1
From: Maxim Georgiev <[email protected]>
bonding is one of the stackable net devices which pass the hardware
timestamping ops to the real device through ndo_eth_ioctl(). This
prevents converting any device driver to the new hwtimestamping API
without regressions.
Remove that limitation in bonding by using the newly introduced helpers
for timestamping through lower devices, that handle both the new and the
old driver API.
Signed-off-by: Maxim Georgiev <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Use copied_to_user instead of KERNEL_HWTSTAMP_FLAG_IFR_RESULT
- Reword commit message
Changes in v6:
- Patch title was updated. No code changes.
drivers/net/bonding/bond_main.c | 105 ++++++++++++++++++++------------
1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7a0f25301f7e..d591992e3eda 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4441,11 +4441,6 @@ static int bond_eth_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cm
{
struct bonding *bond = netdev_priv(bond_dev);
struct mii_ioctl_data *mii = NULL;
- const struct net_device_ops *ops;
- struct net_device *real_dev;
- struct hwtstamp_config cfg;
- struct ifreq ifrr;
- int res = 0;
netdev_dbg(bond_dev, "bond_eth_ioctl: cmd=%d\n", cmd);
@@ -4472,44 +4467,11 @@ static int bond_eth_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cm
}
break;
- case SIOCSHWTSTAMP:
- if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
- return -EFAULT;
-
- if (!(cfg.flags & HWTSTAMP_FLAG_BONDED_PHC_INDEX))
- return -EOPNOTSUPP;
-
- fallthrough;
- case SIOCGHWTSTAMP:
- real_dev = bond_option_active_slave_get_rcu(bond);
- if (!real_dev)
- return -EOPNOTSUPP;
-
- strscpy_pad(ifrr.ifr_name, real_dev->name, IFNAMSIZ);
- ifrr.ifr_ifru = ifr->ifr_ifru;
-
- ops = real_dev->netdev_ops;
- if (netif_device_present(real_dev) && ops->ndo_eth_ioctl) {
- res = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
- if (res)
- return res;
-
- ifr->ifr_ifru = ifrr.ifr_ifru;
- if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
- return -EFAULT;
-
- /* Set the BOND_PHC_INDEX flag to notify user space */
- cfg.flags |= HWTSTAMP_FLAG_BONDED_PHC_INDEX;
-
- return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ?
- -EFAULT : 0;
- }
- fallthrough;
default:
- res = -EOPNOTSUPP;
+ return -EOPNOTSUPP;
}
- return res;
+ return 0;
}
static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd)
@@ -5683,6 +5645,67 @@ static u32 bond_mode_bcast_speed(struct slave *slave, u32 speed)
return speed;
}
+/* Set the BOND_PHC_INDEX flag to notify user space */
+static int bond_set_phc_index_flag(struct kernel_hwtstamp_config *kernel_cfg)
+{
+ struct ifreq *ifr = kernel_cfg->ifr;
+ struct hwtstamp_config cfg;
+
+ if (kernel_cfg->copied_to_user) {
+ /* Lower device has a legacy implementation */
+ if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
+ return -EFAULT;
+
+ cfg.flags |= HWTSTAMP_FLAG_BONDED_PHC_INDEX;
+ if (copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)))
+ return -EFAULT;
+ } else {
+ kernel_cfg->flags |= HWTSTAMP_FLAG_BONDED_PHC_INDEX;
+ }
+
+ return 0;
+}
+
+static int bond_hwtstamp_get(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct net_device *real_dev;
+ int err;
+
+ real_dev = bond_option_active_slave_get_rcu(bond);
+ if (!real_dev)
+ return -EOPNOTSUPP;
+
+ err = generic_hwtstamp_get_lower(real_dev, cfg);
+ if (err)
+ return err;
+
+ return bond_set_phc_index_flag(cfg);
+}
+
+static int bond_hwtstamp_set(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct net_device *real_dev;
+ int err;
+
+ if (!(cfg->flags & HWTSTAMP_FLAG_BONDED_PHC_INDEX))
+ return -EOPNOTSUPP;
+
+ real_dev = bond_option_active_slave_get_rcu(bond);
+ if (!real_dev)
+ return -EOPNOTSUPP;
+
+ err = generic_hwtstamp_set_lower(real_dev, cfg, extack);
+ if (err)
+ return err;
+
+ return bond_set_phc_index_flag(cfg);
+}
+
static int bond_ethtool_get_link_ksettings(struct net_device *bond_dev,
struct ethtool_link_ksettings *cmd)
{
@@ -5831,6 +5854,8 @@ static const struct net_device_ops bond_netdev_ops = {
.ndo_bpf = bond_xdp,
.ndo_xdp_xmit = bond_xdp_xmit,
.ndo_xdp_get_xmit_slave = bond_xdp_get_xmit_slave,
+ .ndo_hwtstamp_get = bond_hwtstamp_get,
+ .ndo_hwtstamp_set = bond_hwtstamp_set,
};
static const struct device_type bond_type = {
--
2.34.1
The hardware timestamping through ndo_eth_ioctl() is going away.
Convert the sparx5 driver to the new API before that can be removed.
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Patch is new
.../ethernet/microchip/sparx5/sparx5_main.h | 9 ++--
.../ethernet/microchip/sparx5/sparx5_netdev.c | 41 +++++++++++++++----
.../ethernet/microchip/sparx5/sparx5_ptp.c | 24 +++++------
3 files changed, 49 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 62c85463b634..89a9a7afa32c 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -205,7 +205,7 @@ enum sparx5_core_clockfreq {
struct sparx5_phc {
struct ptp_clock *clock;
struct ptp_clock_info info;
- struct hwtstamp_config hwtstamp_config;
+ struct kernel_hwtstamp_config hwtstamp_config;
struct sparx5 *sparx5;
u8 index;
};
@@ -388,8 +388,11 @@ void sparx5_unregister_netdevs(struct sparx5 *sparx5);
/* sparx5_ptp.c */
int sparx5_ptp_init(struct sparx5 *sparx5);
void sparx5_ptp_deinit(struct sparx5 *sparx5);
-int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr);
-int sparx5_ptp_hwtstamp_get(struct sparx5_port *port, struct ifreq *ifr);
+int sparx5_ptp_hwtstamp_set(struct sparx5_port *port,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack);
+void sparx5_ptp_hwtstamp_get(struct sparx5_port *port,
+ struct kernel_hwtstamp_config *cfg);
void sparx5_ptp_rxtstamp(struct sparx5 *sparx5, struct sk_buff *skb,
u64 timestamp);
int sparx5_ptp_txtstamp_request(struct sparx5_port *port,
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index d078156581d5..573662d2e01a 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -212,20 +212,41 @@ static int sparx5_get_port_parent_id(struct net_device *dev,
static int sparx5_port_ioctl(struct net_device *dev, struct ifreq *ifr,
int cmd)
+{
+ return phy_mii_ioctl(dev->phydev, ifr, cmd);
+}
+
+static int sparx5_port_hwtstamp_get(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg)
{
struct sparx5_port *sparx5_port = netdev_priv(dev);
struct sparx5 *sparx5 = sparx5_port->sparx5;
- if (!phy_has_hwtstamp(dev->phydev) && sparx5->ptp) {
- switch (cmd) {
- case SIOCSHWTSTAMP:
- return sparx5_ptp_hwtstamp_set(sparx5_port, ifr);
- case SIOCGHWTSTAMP:
- return sparx5_ptp_hwtstamp_get(sparx5_port, ifr);
- }
- }
+ if (phy_has_hwtstamp(dev->phydev))
+ return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCGHWTSTAMP);
- return phy_mii_ioctl(dev->phydev, ifr, cmd);
+ if (!sparx5->ptp)
+ return -EOPNOTSUPP;
+
+ sparx5_ptp_hwtstamp_get(sparx5_port, cfg);
+
+ return 0;
+}
+
+static int sparx5_port_hwtstamp_set(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct sparx5_port *sparx5_port = netdev_priv(dev);
+ struct sparx5 *sparx5 = sparx5_port->sparx5;
+
+ if (phy_has_hwtstamp(dev->phydev))
+ return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCSHWTSTAMP);
+
+ if (!sparx5->ptp)
+ return -EOPNOTSUPP;
+
+ return sparx5_ptp_hwtstamp_set(sparx5_port, cfg, extack);
}
static const struct net_device_ops sparx5_port_netdev_ops = {
@@ -240,6 +261,8 @@ static const struct net_device_ops sparx5_port_netdev_ops = {
.ndo_get_port_parent_id = sparx5_get_port_parent_id,
.ndo_eth_ioctl = sparx5_port_ioctl,
.ndo_setup_tc = sparx5_port_setup_tc,
+ .ndo_hwtstamp_get = sparx5_port_hwtstamp_get,
+ .ndo_hwtstamp_set = sparx5_port_hwtstamp_set,
};
bool sparx5_netdevice_check(const struct net_device *dev)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
index 0edb98cef7e4..5a932460db58 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c
@@ -74,10 +74,11 @@ static u64 sparx5_ptp_get_nominal_value(struct sparx5 *sparx5)
return res;
}
-int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr)
+int sparx5_ptp_hwtstamp_set(struct sparx5_port *port,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
{
struct sparx5 *sparx5 = port->sparx5;
- struct hwtstamp_config cfg;
struct sparx5_phc *phc;
/* For now don't allow to run ptp on ports that are part of a bridge,
@@ -88,10 +89,7 @@ int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr)
if (test_bit(port->portno, sparx5->bridge_mask))
return -EINVAL;
- if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
- return -EFAULT;
-
- switch (cfg.tx_type) {
+ switch (cfg->tx_type) {
case HWTSTAMP_TX_ON:
port->ptp_cmd = IFH_REW_OP_TWO_STEP_PTP;
break;
@@ -105,7 +103,7 @@ int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr)
return -ERANGE;
}
- switch (cfg.rx_filter) {
+ switch (cfg->rx_filter) {
case HWTSTAMP_FILTER_NONE:
break;
case HWTSTAMP_FILTER_ALL:
@@ -122,7 +120,7 @@ int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr)
case HWTSTAMP_FILTER_PTP_V2_SYNC:
case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
case HWTSTAMP_FILTER_NTP_ALL:
- cfg.rx_filter = HWTSTAMP_FILTER_ALL;
+ cfg->rx_filter = HWTSTAMP_FILTER_ALL;
break;
default:
return -ERANGE;
@@ -131,20 +129,20 @@ int sparx5_ptp_hwtstamp_set(struct sparx5_port *port, struct ifreq *ifr)
/* Commit back the result & save it */
mutex_lock(&sparx5->ptp_lock);
phc = &sparx5->phc[SPARX5_PHC_PORT];
- memcpy(&phc->hwtstamp_config, &cfg, sizeof(cfg));
+ phc->hwtstamp_config = *cfg;
mutex_unlock(&sparx5->ptp_lock);
- return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+ return 0;
}
-int sparx5_ptp_hwtstamp_get(struct sparx5_port *port, struct ifreq *ifr)
+void sparx5_ptp_hwtstamp_get(struct sparx5_port *port,
+ struct kernel_hwtstamp_config *cfg)
{
struct sparx5 *sparx5 = port->sparx5;
struct sparx5_phc *phc;
phc = &sparx5->phc[SPARX5_PHC_PORT];
- return copy_to_user(ifr->ifr_data, &phc->hwtstamp_config,
- sizeof(phc->hwtstamp_config)) ? -EFAULT : 0;
+ *cfg = phc->hwtstamp_config;
}
static void sparx5_ptp_classify(struct sparx5_port *port, struct sk_buff *skb,
--
2.34.1
It is desirable that the new .ndo_hwtstamp_set() API gives more
uniformity, less overhead and future flexibility w.r.t. the PHY
timestamping behavior.
Currently there are some drivers which allow PHY timestamping through
the procedure mentioned in Documentation/networking/timestamping.rst.
They don't do anything locally if phy_has_hwtstamp() is set, except for
lan966x which installs PTP packet traps.
Centralize that behavior in a new dev_set_hwtstamp_phylib() code
function, which calls either phy_mii_ioctl() for the phylib PHY,
or .ndo_hwtstamp_set() of the netdev, based on a single policy
(currently simplistic: phy_has_hwtstamp()).
Any driver converted to .ndo_hwtstamp_set() will automatically opt into
the centralized phylib timestamping policy. Unconverted drivers still
get to choose whether they let the PHY handle timestamping or not.
Netdev drivers with integrated PHY drivers that don't use phylib
presumably don't set dev->phydev, and those will always see
HWTSTAMP_SOURCE_NETDEV requests even when converted. The timestamping
policy will remain 100% up to them.
Signed-off-by: Vladimir Oltean <[email protected]>
---
Changes in v7:
- Patch is new
drivers/net/ethernet/freescale/fec_main.c | 8 --
.../ethernet/microchip/lan966x/lan966x_main.c | 25 +++---
.../ethernet/microchip/sparx5/sparx5_netdev.c | 6 --
include/linux/net_tstamp.h | 14 +++
include/linux/netdevice.h | 4 +
net/core/dev_ioctl.c | 90 +++++++++++++++++--
6 files changed, 114 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 08744e8164e3..fa81e70eb2ab 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3858,10 +3858,6 @@ static int fec_hwtstamp_get(struct net_device *ndev,
struct kernel_hwtstamp_config *config)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phydev = ndev->phydev;
-
- if (phy_has_hwtstamp(phydev))
- return phy_mii_ioctl(phydev, config->ifr, SIOCGHWTSTAMP);
if (!fep->bufdesc_ex)
return -EOPNOTSUPP;
@@ -3876,10 +3872,6 @@ static int fec_hwtstamp_set(struct net_device *ndev,
struct netlink_ext_ack *extack)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- struct phy_device *phydev = ndev->phydev;
-
- if (phy_has_hwtstamp(phydev))
- return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
if (!fep->bufdesc_ex)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index c8fce4e79c63..5185691e10c1 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -463,9 +463,6 @@ static int lan966x_port_hwtstamp_get(struct net_device *dev,
{
struct lan966x_port *port = netdev_priv(dev);
- if (phy_has_hwtstamp(dev->phydev))
- return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCGHWTSTAMP);
-
if (!port->lan966x->ptp)
return -EOPNOTSUPP;
@@ -481,21 +478,26 @@ static int lan966x_port_hwtstamp_set(struct net_device *dev,
struct lan966x_port *port = netdev_priv(dev);
int err;
+ if (cfg->source != HWTSTAMP_SOURCE_NETDEV &&
+ cfg->source != HWTSTAMP_SOURCE_PHYLIB)
+ return -EOPNOTSUPP;
+
err = lan966x_ptp_setup_traps(port, cfg);
if (err)
return err;
- if (phy_has_hwtstamp(dev->phydev)) {
- err = phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCSHWTSTAMP);
- if (err)
+ if (cfg->source == HWTSTAMP_SOURCE_NETDEV) {
+ if (!port->lan966x->ptp)
+ return -EOPNOTSUPP;
+
+ err = lan966x_ptp_hwtstamp_set(port, cfg, extack);
+ if (err) {
lan966x_ptp_del_traps(port);
- return err;
+ return err;
+ }
}
- if (!port->lan966x->ptp)
- return -EOPNOTSUPP;
-
- return lan966x_ptp_hwtstamp_set(port, cfg, extack);
+ return 0;
}
static const struct net_device_ops lan966x_port_netdev_ops = {
@@ -823,6 +825,7 @@ static int lan966x_probe_port(struct lan966x *lan966x, u32 p,
NETIF_F_HW_VLAN_STAG_TX |
NETIF_F_HW_TC;
dev->hw_features |= NETIF_F_HW_TC;
+ dev->priv_flags |= IFF_SEE_ALL_HWTSTAMP_REQUESTS;
dev->needed_headroom = IFH_LEN_BYTES;
eth_hw_addr_gen(dev, lan966x->base_mac, p + 1);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
index 573662d2e01a..48f7022d3fb7 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
@@ -222,9 +222,6 @@ static int sparx5_port_hwtstamp_get(struct net_device *dev,
struct sparx5_port *sparx5_port = netdev_priv(dev);
struct sparx5 *sparx5 = sparx5_port->sparx5;
- if (phy_has_hwtstamp(dev->phydev))
- return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCGHWTSTAMP);
-
if (!sparx5->ptp)
return -EOPNOTSUPP;
@@ -240,9 +237,6 @@ static int sparx5_port_hwtstamp_set(struct net_device *dev,
struct sparx5_port *sparx5_port = netdev_priv(dev);
struct sparx5 *sparx5 = sparx5_port->sparx5;
- if (phy_has_hwtstamp(dev->phydev))
- return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCSHWTSTAMP);
-
if (!sparx5->ptp)
return -EOPNOTSUPP;
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
index 03e922814851..f4aff9137724 100644
--- a/include/linux/net_tstamp.h
+++ b/include/linux/net_tstamp.h
@@ -5,6 +5,11 @@
#include <uapi/linux/net_tstamp.h>
+enum hwtstamp_source {
+ HWTSTAMP_SOURCE_NETDEV,
+ HWTSTAMP_SOURCE_PHYLIB,
+};
+
/**
* struct kernel_hwtstamp_config - Kernel copy of struct hwtstamp_config
*
@@ -26,6 +31,7 @@ struct kernel_hwtstamp_config {
int rx_filter;
struct ifreq *ifr;
bool copied_to_user;
+ enum hwtstamp_source source;
};
static inline void hwtstamp_config_to_kernel(struct kernel_hwtstamp_config *kernel_cfg,
@@ -44,4 +50,12 @@ static inline void hwtstamp_config_from_kernel(struct hwtstamp_config *cfg,
cfg->rx_filter = kernel_cfg->rx_filter;
}
+static inline bool kernel_hwtstamp_config_changed(const struct kernel_hwtstamp_config *a,
+ const struct kernel_hwtstamp_config *b)
+{
+ return a->flags != b->flags ||
+ a->tx_type != b->tx_type ||
+ a->rx_filter != b->rx_filter;
+}
+
#endif /* _LINUX_NET_TIMESTAMPING_H_ */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ca3bcf2257c0..0d8a7ac67cf1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1724,6 +1724,9 @@ struct xdp_metadata_ops {
* @IFF_TX_SKB_NO_LINEAR: device/driver is capable of xmitting frames with
* skb_headlen(skb) == 0 (data starts from frag0)
* @IFF_CHANGE_PROTO_DOWN: device supports setting carrier via IFLA_PROTO_DOWN
+ * @IFF_SEE_ALL_HWTSTAMP_REQUESTS: device wants to see calls to
+ * ndo_hwtstamp_set() for all timestamp requests regardless of source,
+ * even if those aren't HWTSTAMP_SOURCE_NETDEV.
*/
enum netdev_priv_flags {
IFF_802_1Q_VLAN = 1<<0,
@@ -1759,6 +1762,7 @@ enum netdev_priv_flags {
IFF_NO_ADDRCONF = BIT_ULL(30),
IFF_TX_SKB_NO_LINEAR = BIT_ULL(31),
IFF_CHANGE_PROTO_DOWN = BIT_ULL(32),
+ IFF_SEE_ALL_HWTSTAMP_REQUESTS = BIT_ULL(33),
};
#define IFF_802_1Q_VLAN IFF_802_1Q_VLAN
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index d0223ecd6f6f..caaba0db8cb3 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -252,6 +252,30 @@ static int dev_eth_ioctl(struct net_device *dev,
return ops->ndo_eth_ioctl(dev, ifr, cmd);
}
+/**
+ * dev_get_hwtstamp_phylib() - Get hardware timestamping settings of NIC
+ * or of attached phylib PHY
+ * @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.
+ *
+ * Note: phy_mii_ioctl() only handles SIOCSHWTSTAMP (not SIOCGHWTSTAMP), and
+ * there only exists a phydev->mii_ts->hwtstamp() method. So this will return
+ * -EOPNOTSUPP for phylib for now, which is still more accurate than letting
+ * the netdev handle the GET request.
+ */
+static int dev_get_hwtstamp_phylib(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg)
+{
+ if (phy_has_hwtstamp(dev->phydev))
+ return phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCGHWTSTAMP);
+
+ return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
+}
+
static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
{
const struct net_device_ops *ops = dev->netdev_ops;
@@ -266,7 +290,7 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
return -ENODEV;
kernel_cfg.ifr = ifr;
- err = ops->ndo_hwtstamp_get(dev, &kernel_cfg);
+ err = dev_get_hwtstamp_phylib(dev, &kernel_cfg);
if (err)
return err;
@@ -283,6 +307,59 @@ static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
return 0;
}
+/**
+ * dev_set_hwtstamp_phylib() - Change hardware timestamping of NIC
+ * or of attached phylib PHY
+ * @dev: Network device
+ * @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
+ * 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.
+ */
+static int dev_set_hwtstamp_phylib(struct net_device *dev,
+ struct kernel_hwtstamp_config *cfg,
+ struct netlink_ext_ack *extack)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+ bool phy_ts = phy_has_hwtstamp(dev->phydev);
+ struct kernel_hwtstamp_config old_cfg = {};
+ bool changed = false;
+ int err;
+
+ cfg->source = phy_ts ? HWTSTAMP_SOURCE_PHYLIB : HWTSTAMP_SOURCE_NETDEV;
+
+ if (!phy_ts || (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
+ err = ops->ndo_hwtstamp_get(dev, &old_cfg);
+ if (err)
+ return err;
+
+ err = ops->ndo_hwtstamp_set(dev, cfg, extack);
+ if (err) {
+ if (extack->_msg)
+ netdev_err(dev, "%s\n", extack->_msg);
+ return err;
+ }
+
+ changed = kernel_hwtstamp_config_changed(&old_cfg, cfg);
+ }
+
+ if (phy_ts) {
+ err = phy_mii_ioctl(dev->phydev, cfg->ifr, SIOCSHWTSTAMP);
+ if (err) {
+ if (changed)
+ ops->ndo_hwtstamp_set(dev, &old_cfg, NULL);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
{
const struct net_device_ops *ops = dev->netdev_ops;
@@ -314,12 +391,9 @@ static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
if (!netif_device_present(dev))
return -ENODEV;
- err = ops->ndo_hwtstamp_set(dev, &kernel_cfg, &extack);
- if (err) {
- if (extack._msg)
- netdev_err(dev, "%s\n", extack._msg);
+ err = dev_set_hwtstamp_phylib(dev, &kernel_cfg, &extack);
+ if (err)
return err;
- }
/* The driver may have modified the configuration, so copy the
* updated version of it back to user space
@@ -362,7 +436,7 @@ int generic_hwtstamp_get_lower(struct net_device *dev,
return -ENODEV;
if (ops->ndo_hwtstamp_get)
- return ops->ndo_hwtstamp_get(dev, kernel_cfg);
+ return dev_get_hwtstamp_phylib(dev, kernel_cfg);
/* Legacy path: unconverted lower driver */
return generic_hwtstamp_ioctl_lower(dev, SIOCGHWTSTAMP, kernel_cfg);
@@ -379,7 +453,7 @@ int generic_hwtstamp_set_lower(struct net_device *dev,
return -ENODEV;
if (ops->ndo_hwtstamp_set)
- return ops->ndo_hwtstamp_set(dev, kernel_cfg, extack);
+ return dev_set_hwtstamp_phylib(dev, kernel_cfg, extack);
/* Legacy path: unconverted lower driver */
return generic_hwtstamp_ioctl_lower(dev, SIOCSHWTSTAMP, kernel_cfg);
--
2.34.1
On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote:
> On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
> > Based on previous RFCs from Maxim Georgiev:
> > https://lore.kernel.org/netdev/[email protected]/
> >
> > this series attempts to introduce new API for the hardware timestamping
> > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> >
> > I don't have any board with phylib hardware timestamping, so I would
> > appreciate testing (especially on lan966x, the most intricate
> > conversion). I was, however, able to test netdev level timestamping,
> > because I also have some more unsubmitted conversions in progress:
> >
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
> >
> > I hope that the concerns expressed in the comments of previous series
> > were addressed, and that K?ry Maincent's series:
> > https://lore.kernel.org/netdev/[email protected]/
> > can make progress in parallel with the conversion of the rest of drivers.
> >
>
> This series looks good to me, nice cleanup and reducing some boiler
> plate code is excellent.
>
> I'd like to convert the Intel drivers too, but I am not sure when I can
> commit to doing that as I have a lot on my plate presently.
>
> Reviewed-by: Jacob Keller <[email protected]>
Thanks for the review. The conversion of Intel drivers is in the Github
link I had posted.
On 7/13/2023 3:33 PM, Vladimir Oltean wrote:
> On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote:
>> On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
>>> Based on previous RFCs from Maxim Georgiev:
>>> https://lore.kernel.org/netdev/[email protected]/
>>>
>>> this series attempts to introduce new API for the hardware timestamping
>>> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>>>
>>> I don't have any board with phylib hardware timestamping, so I would
>>> appreciate testing (especially on lan966x, the most intricate
>>> conversion). I was, however, able to test netdev level timestamping,
>>> because I also have some more unsubmitted conversions in progress:
>>>
>>> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>>>
>>> I hope that the concerns expressed in the comments of previous series
>>> were addressed, and that Köry Maincent's series:
>>> https://lore.kernel.org/netdev/[email protected]/
>>> can make progress in parallel with the conversion of the rest of drivers.
>>>
>>
>> This series looks good to me, nice cleanup and reducing some boiler
>> plate code is excellent.
>>
>> I'd like to convert the Intel drivers too, but I am not sure when I can
>> commit to doing that as I have a lot on my plate presently.
>>
>> Reviewed-by: Jacob Keller <[email protected]>
>
> Thanks for the review. The conversion of Intel drivers is in the Github
> link I had posted.
Oh nice!
On 7/13/2023 5:18 AM, Vladimir Oltean wrote:
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/[email protected]/
>
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
>
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/[email protected]/
> can make progress in parallel with the conversion of the rest of drivers.
>
This series looks good to me, nice cleanup and reducing some boiler
plate code is excellent.
I'd like to convert the Intel drivers too, but I am not sure when I can
commit to doing that as I have a lot on my plate presently.
Reviewed-by: Jacob Keller <[email protected]>
> Maxim Georgiev (5):
> net: add NDOs for configuring hardware timestamping
> net: add hwtstamping helpers for stackable net devices
> net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>
> Vladimir Oltean (5):
> net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: fec: delete fec_ptp_disable_hwts()
> net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
> converted drivers
>
> drivers/net/bonding/bond_main.c | 105 ++++++----
> drivers/net/ethernet/freescale/fec.h | 6 +-
> drivers/net/ethernet/freescale/fec_main.c | 41 ++--
> drivers/net/ethernet/freescale/fec_ptp.c | 43 ++--
> .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++--
> .../ethernet/microchip/lan966x/lan966x_main.h | 12 +-
> .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++--
> .../ethernet/microchip/sparx5/sparx5_main.h | 9 +-
> .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++-
> .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++-
> drivers/net/macvlan.c | 34 ++--
> include/linux/net_tstamp.h | 28 +++
> include/linux/netdevice.h | 25 +++
> net/8021q/vlan_dev.c | 27 ++-
> net/core/dev_ioctl.c | 183 +++++++++++++++++-
> 15 files changed, 480 insertions(+), 187 deletions(-)
>
On Thu, 13 Jul 2023 15:19:07 +0300 Vladimir Oltean wrote:
> /**
> * struct kernel_hwtstamp_config - Kernel copy of struct hwtstamp_config
> *
> @@ -26,6 +31,7 @@ struct kernel_hwtstamp_config {
> int rx_filter;
> struct ifreq *ifr;
> bool copied_to_user;
> + enum hwtstamp_source source;
> };
source is missing from the kdoc
phy_mii_ioctl() can be in a module so we need a stub / indirection
--
pw-bot: cr
> -----Original Message-----
> From: Vladimir Oltean <[email protected]>
> Sent: 2023年7月13日 20:19
> To: [email protected]
> Cc: David S. Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> <[email protected]>; Maxim Georgiev <[email protected]>; Horatiu Vultur
> <[email protected]>; Köry Maincent
> <[email protected]>; Maxime Chevallier
> <[email protected]>; Richard Cochran
> <[email protected]>; Vadim Fedorenko
> <[email protected]>; Gerhard Engleder
> <[email protected]>; Hangbin Liu <[email protected]>;
> Russell King <[email protected]>; Heiner Kallweit
> <[email protected]>; Jacob Keller <[email protected]>; Jay
> Vosburgh <[email protected]>; Andy Gospodarek <[email protected]>;
> Wei Fang <[email protected]>; Shenwei Wang <[email protected]>;
> Clark Wang <[email protected]>; dl-linux-imx <[email protected]>;
> [email protected]; Lars Povlsen <[email protected]>;
> Steen Hegelund <[email protected]>; Daniel Machon
> <[email protected]>; Simon Horman
> <[email protected]>; Casper Andersson
> <[email protected]>; Sergey Organov <[email protected]>;
> [email protected]; [email protected]
> Subject: [PATCH v7 net-next 07/10] net: fec: delete fec_ptp_disable_hwts()
>
> Commit 340746398b67 ("net: fec: fix hardware time stamping by external
> devices") was overly cautious with calling fec_ptp_disable_hwts() when
> cmd == SIOCSHWTSTAMP and use_fec_hwts == false, because use_fec_hwts is
> based on a runtime invariant (phy_has_hwtstamp()). Thus, if use_fec_hwts
> is false, then fep->hwts_tx_en and fep->hwts_rx_en cannot be changed at
> runtime; their values depend on the initial memory allocation, which
> already sets them to zeroes.
>
> If the core will ever gain support for switching timestamping layers,
> it will arrange for a more organized calling convention and disable
> timestamping in the previous layer as a first step. This means that the
> code in the FEC driver is not necessary in any case.
>
> The purpose of this change is to arrange the phy_has_hwtstamp() code in
> a way in which it can be refactored away into generic logic.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> Changes in v7:
> - Patch is new
>
> drivers/net/ethernet/freescale/fec.h | 1 -
> drivers/net/ethernet/freescale/fec_main.c | 5 +----
> drivers/net/ethernet/freescale/fec_ptp.c | 12 ------------
> 3 files changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index d4ae0e7c0a44..e273129d44c9 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -690,7 +690,6 @@ struct fec_enet_private {
> void fec_ptp_init(struct platform_device *pdev, int irq_idx);
> void fec_ptp_stop(struct platform_device *pdev);
> void fec_ptp_start_cyclecounter(struct net_device *ndev);
> -void fec_ptp_disable_hwts(struct net_device *ndev);
> int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
> struct netlink_ext_ack *extack);
> void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config
> *config);
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index c0b68fc3ec8b..08744e8164e3 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3878,11 +3878,8 @@ static int fec_hwtstamp_set(struct net_device
> *ndev,
> struct fec_enet_private *fep = netdev_priv(ndev);
> struct phy_device *phydev = ndev->phydev;
>
> - if (phy_has_hwtstamp(phydev)) {
> - fec_ptp_disable_hwts(ndev);
> -
> + if (phy_has_hwtstamp(phydev))
> return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
> - }
>
> if (!fep->bufdesc_ex)
> return -EOPNOTSUPP;
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index 3f53b8ae57dd..55cf98557e02 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -606,18 +606,6 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp,
> }
> }
>
> -/**
> - * fec_ptp_disable_hwts - disable hardware time stamping
> - * @ndev: pointer to net_device
> - */
> -void fec_ptp_disable_hwts(struct net_device *ndev)
> -{
> - struct fec_enet_private *fep = netdev_priv(ndev);
> -
> - fep->hwts_tx_en = 0;
> - fep->hwts_rx_en = 0;
> -}
> -
> int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
> struct netlink_ext_ack *extack)
> {
> --
> 2.34.1
Thanks!
Reviewed-by: Wei Fang < [email protected] >
> -----Original Message-----
> From: Vladimir Oltean <[email protected]>
> Sent: 2023年7月13日 20:19
> To: [email protected]
> Cc: David S. Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> <[email protected]>; Maxim Georgiev <[email protected]>; Horatiu Vultur
> <[email protected]>; Köry Maincent
> <[email protected]>; Maxime Chevallier
> <[email protected]>; Richard Cochran
> <[email protected]>; Vadim Fedorenko
> <[email protected]>; Gerhard Engleder
> <[email protected]>; Hangbin Liu <[email protected]>;
> Russell King <[email protected]>; Heiner Kallweit
> <[email protected]>; Jacob Keller <[email protected]>; Jay
> Vosburgh <[email protected]>; Andy Gospodarek <[email protected]>;
> Wei Fang <[email protected]>; Shenwei Wang <[email protected]>;
> Clark Wang <[email protected]>; dl-linux-imx <[email protected]>;
> [email protected]; Lars Povlsen <[email protected]>;
> Steen Hegelund <[email protected]>; Daniel Machon
> <[email protected]>; Simon Horman
> <[email protected]>; Casper Andersson
> <[email protected]>; Sergey Organov <[email protected]>;
> [email protected]; [email protected]
> Subject: [PATCH v7 net-next 06/10] net: fec: convert to ndo_hwtstamp_get()
> and ndo_hwtstamp_set()
>
> The hardware timestamping through ndo_eth_ioctl() is going away.
> Convert the FEC driver to the new API before that can be removed.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> Changes in v7:
> - Patch is new
>
> drivers/net/ethernet/freescale/fec.h | 5 ++-
> drivers/net/ethernet/freescale/fec_main.c | 52 +++++++++++++++++------
> drivers/net/ethernet/freescale/fec_ptp.c | 31 +++++---------
> 3 files changed, 52 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 9939ccafb556..d4ae0e7c0a44 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -691,8 +691,9 @@ void fec_ptp_init(struct platform_device *pdev, int
> irq_idx); void fec_ptp_stop(struct platform_device *pdev); void
> fec_ptp_start_cyclecounter(struct net_device *ndev); void
> fec_ptp_disable_hwts(struct net_device *ndev); -int fec_ptp_set(struct
> net_device *ndev, struct ifreq *ifr); -int fec_ptp_get(struct net_device *ndev,
> struct ifreq *ifr);
> +int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
> + struct netlink_ext_ack *extack);
> +void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config
> +*config);
>
>
> /******************************************************************
> **********/
> #endif /* FEC_H */
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 1280da699fa3..c0b68fc3ec8b 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3196,7 +3196,6 @@ static const struct ethtool_ops
> fec_enet_ethtool_ops = {
>
> static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd) {
> - struct fec_enet_private *fep = netdev_priv(ndev);
> struct phy_device *phydev = ndev->phydev;
>
> if (!netif_running(ndev))
> @@ -3205,19 +3204,6 @@ static int fec_enet_ioctl(struct net_device *ndev,
> struct ifreq *rq, int cmd)
> if (!phydev)
> return -ENODEV;
>
> - if (fep->bufdesc_ex) {
> - bool use_fec_hwts = !phy_has_hwtstamp(phydev);
> -
> - if (cmd == SIOCSHWTSTAMP) {
> - if (use_fec_hwts)
> - return fec_ptp_set(ndev, rq);
> - fec_ptp_disable_hwts(ndev);
> - } else if (cmd == SIOCGHWTSTAMP) {
> - if (use_fec_hwts)
> - return fec_ptp_get(ndev, rq);
> - }
> - }
> -
> return phy_mii_ioctl(phydev, rq, cmd); }
>
> @@ -3868,6 +3854,42 @@ static int fec_enet_xdp_xmit(struct net_device
> *dev,
> return sent_frames;
> }
>
> +static int fec_hwtstamp_get(struct net_device *ndev,
> + struct kernel_hwtstamp_config *config) {
> + struct fec_enet_private *fep = netdev_priv(ndev);
> + struct phy_device *phydev = ndev->phydev;
> +
> + if (phy_has_hwtstamp(phydev))
> + return phy_mii_ioctl(phydev, config->ifr, SIOCGHWTSTAMP);
> +
> + if (!fep->bufdesc_ex)
> + return -EOPNOTSUPP;
> +
> + fec_ptp_get(ndev, config);
> +
> + return 0;
> +}
> +
> +static int fec_hwtstamp_set(struct net_device *ndev,
> + struct kernel_hwtstamp_config *config,
> + struct netlink_ext_ack *extack)
> +{
> + struct fec_enet_private *fep = netdev_priv(ndev);
> + struct phy_device *phydev = ndev->phydev;
> +
> + if (phy_has_hwtstamp(phydev)) {
> + fec_ptp_disable_hwts(ndev);
> +
> + return phy_mii_ioctl(phydev, config->ifr, SIOCSHWTSTAMP);
> + }
> +
> + if (!fep->bufdesc_ex)
> + return -EOPNOTSUPP;
> +
> + return fec_ptp_set(ndev, config, extack); }
> +
> static const struct net_device_ops fec_netdev_ops = {
> .ndo_open = fec_enet_open,
> .ndo_stop = fec_enet_close,
> @@ -3884,6 +3906,8 @@ static const struct net_device_ops fec_netdev_ops =
> {
> .ndo_set_features = fec_set_features,
> .ndo_bpf = fec_enet_bpf,
> .ndo_xdp_xmit = fec_enet_xdp_xmit,
> + .ndo_hwtstamp_get = fec_hwtstamp_get,
> + .ndo_hwtstamp_set = fec_hwtstamp_set,
> };
>
> static const unsigned short offset_des_active_rxq[] = { diff --git
> a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index ab86bb8562ef..3f53b8ae57dd 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -618,16 +618,12 @@ void fec_ptp_disable_hwts(struct net_device *ndev)
> fep->hwts_rx_en = 0;
> }
>
> -int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr)
> +int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config
> *config,
> + struct netlink_ext_ack *extack)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
>
> - struct hwtstamp_config config;
> -
> - if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
> - return -EFAULT;
> -
> - switch (config.tx_type) {
> + switch (config->tx_type) {
> case HWTSTAMP_TX_OFF:
> fep->hwts_tx_en = 0;
> break;
> @@ -638,33 +634,28 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq
> *ifr)
> return -ERANGE;
> }
>
> - switch (config.rx_filter) {
> + switch (config->rx_filter) {
> case HWTSTAMP_FILTER_NONE:
> fep->hwts_rx_en = 0;
> break;
>
> default:
> fep->hwts_rx_en = 1;
> - config.rx_filter = HWTSTAMP_FILTER_ALL;
> + config->rx_filter = HWTSTAMP_FILTER_ALL;
> break;
> }
>
> - return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
> - -EFAULT : 0;
> + return 0;
> }
>
> -int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr)
> +void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config
> +*config)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> - struct hwtstamp_config config;
> -
> - config.flags = 0;
> - config.tx_type = fep->hwts_tx_en ? HWTSTAMP_TX_ON :
> HWTSTAMP_TX_OFF;
> - config.rx_filter = (fep->hwts_rx_en ?
> - HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE);
>
> - return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ?
> - -EFAULT : 0;
> + config->flags = 0;
> + config->tx_type = fep->hwts_tx_en ? HWTSTAMP_TX_ON :
> HWTSTAMP_TX_OFF;
> + config->rx_filter = (fep->hwts_rx_en ?
> + HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE);
> }
>
> /*
> --
> 2.34.1
Thanks!
Reviewed-by: Wei Fang < [email protected] >
The 07/13/2023 15:18, Vladimir Oltean wrote:
Hi Vladimir,
>
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/[email protected]/
>
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
>
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
I have tested this patch series on lan966x. In both cases, when there is
a PHY that supports HW timestamping and when the isn't a PHY that
supports HW timestamping. In both cases it behaves as expected. Thanks!
Tested-by: Horatiu Vultur <[email protected]>
>
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/[email protected]/
> can make progress in parallel with the conversion of the rest of drivers.
>
> Maxim Georgiev (5):
> net: add NDOs for configuring hardware timestamping
> net: add hwtstamping helpers for stackable net devices
> net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>
> Vladimir Oltean (5):
> net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: fec: delete fec_ptp_disable_hwts()
> net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
> converted drivers
>
> drivers/net/bonding/bond_main.c | 105 ++++++----
> drivers/net/ethernet/freescale/fec.h | 6 +-
> drivers/net/ethernet/freescale/fec_main.c | 41 ++--
> drivers/net/ethernet/freescale/fec_ptp.c | 43 ++--
> .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++--
> .../ethernet/microchip/lan966x/lan966x_main.h | 12 +-
> .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++--
> .../ethernet/microchip/sparx5/sparx5_main.h | 9 +-
> .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++-
> .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++-
> drivers/net/macvlan.c | 34 ++--
> include/linux/net_tstamp.h | 28 +++
> include/linux/netdevice.h | 25 +++
> net/8021q/vlan_dev.c | 27 ++-
> net/core/dev_ioctl.c | 183 +++++++++++++++++-
> 15 files changed, 480 insertions(+), 187 deletions(-)
>
> --
> 2.34.1
>
--
/Horatiu
On Fri, Jul 14, 2023 at 03:05:36AM +0000, Wei Fang wrote:
> Thanks!
> Reviewed-by: Wei Fang < [email protected] >
Please note that correct formatting is:
Reviewed-by: Wei Fang <[email protected]>
No spaces between the <> and the email address itself. It's exactly the
same format used in email headers on the Internet.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> -----Original Message-----
> From: Russell King <[email protected]>
> Sent: 2023年7月14日 16:48
> To: Wei Fang <[email protected]>
> Cc: Vladimir Oltean <[email protected]>; [email protected];
> David S. Miller <[email protected]>; Eric Dumazet
> <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
> <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli
> <[email protected]>; Maxim Georgiev <[email protected]>; Horatiu Vultur
> <[email protected]>; Köry Maincent
> <[email protected]>; Maxime Chevallier
> <[email protected]>; Richard Cochran
> <[email protected]>; Vadim Fedorenko
> <[email protected]>; Gerhard Engleder
> <[email protected]>; Hangbin Liu <[email protected]>;
> Heiner Kallweit <[email protected]>; Jacob Keller
> <[email protected]>; Jay Vosburgh <[email protected]>; Andy
> Gospodarek <[email protected]>; Shenwei Wang
> <[email protected]>; Clark Wang <[email protected]>;
> dl-linux-imx <[email protected]>; [email protected]; Lars
> Povlsen <[email protected]>; Steen Hegelund
> <[email protected]>; Daniel Machon
> <[email protected]>; Simon Horman
> <[email protected]>; Casper Andersson
> <[email protected]>; Sergey Organov <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH v7 net-next 06/10] net: fec: convert to
> ndo_hwtstamp_get() and ndo_hwtstamp_set()
>
> On Fri, Jul 14, 2023 at 03:05:36AM +0000, Wei Fang wrote:
> > Thanks!
> > Reviewed-by: Wei Fang < [email protected] >
>
> Please note that correct formatting is:
>
> Reviewed-by: Wei Fang <[email protected]>
>
> No spaces between the <> and the email address itself. It's exactly the same
> format used in email headers on the Internet.
>
Sorry for the typo, I should check before replying. :(
> --
> RMK's Patch system:
> https://www.ar/
> mlinux.org.uk%2Fdeveloper%2Fpatches%2F&data=05%7C01%7Cwei.fang%40
> nxp.com%7C60545ab70c1248b65d7608db84470e9a%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C0%7C638249212939844945%7CUnknown%7CT
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
> XVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=422jsG%2Bd31ymJvT%2FBkntso
> Q8BI%2B7dzd0Fczwb2Qm4B0%3D&reserved=0
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Jul 13, 2023 at 6:19 AM Vladimir Oltean <[email protected]> wrote:
>
> Based on previous RFCs from Maxim Georgiev:
> https://lore.kernel.org/netdev/[email protected]/
>
> this series attempts to introduce new API for the hardware timestamping
> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
>
> I don't have any board with phylib hardware timestamping, so I would
> appreciate testing (especially on lan966x, the most intricate
> conversion). I was, however, able to test netdev level timestamping,
> because I also have some more unsubmitted conversions in progress:
>
> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>
> I hope that the concerns expressed in the comments of previous series
> were addressed, and that Köry Maincent's series:
> https://lore.kernel.org/netdev/[email protected]/
> can make progress in parallel with the conversion of the rest of drivers.
>
> Maxim Georgiev (5):
> net: add NDOs for configuring hardware timestamping
> net: add hwtstamping helpers for stackable net devices
> net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
> net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set()
>
> Vladimir Oltean (5):
> net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: fec: delete fec_ptp_disable_hwts()
> net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
> net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from
> converted drivers
>
> drivers/net/bonding/bond_main.c | 105 ++++++----
> drivers/net/ethernet/freescale/fec.h | 6 +-
> drivers/net/ethernet/freescale/fec_main.c | 41 ++--
> drivers/net/ethernet/freescale/fec_ptp.c | 43 ++--
> .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++--
> .../ethernet/microchip/lan966x/lan966x_main.h | 12 +-
> .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++--
> .../ethernet/microchip/sparx5/sparx5_main.h | 9 +-
> .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++-
> .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++-
> drivers/net/macvlan.c | 34 ++--
> include/linux/net_tstamp.h | 28 +++
> include/linux/netdevice.h | 25 +++
> net/8021q/vlan_dev.c | 27 ++-
> net/core/dev_ioctl.c | 183 +++++++++++++++++-
> 15 files changed, 480 insertions(+), 187 deletions(-)
>
> --
> 2.34.1
>
Vladimir, thank you for taking over and improving this patch stack!
I see you dropped the netdevsim patch:
https://www.spinics.net/lists/netdev/msg901378.html
Do you believe it's not useful any more since the rest of the
patches in the stack were tested through other means?
Hi Maxim,
On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote:
> Vladimir, thank you for taking over and improving this patch stack!
>
> I see you dropped the netdevsim patch:
> https://www.spinics.net/lists/netdev/msg901378.html
> Do you believe it's not useful any more since the rest of the
> patches in the stack were tested through other means?
I just didn't consider that adding mock hardware timestamping support to
netdevsim was necessary or useful, considering the number of other driver
conversions that will have to be submitted. Just an extra, avoidable effort
for me.
On Fri, Jul 14, 2023 at 10:00:59AM +0200, Horatiu Vultur wrote:
> The 07/13/2023 15:18, Vladimir Oltean wrote:
>
> Hi Vladimir,
>
> >
> > Based on previous RFCs from Maxim Georgiev:
> > https://lore.kernel.org/netdev/[email protected]/
> >
> > this series attempts to introduce new API for the hardware timestamping
> > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling).
> >
> > I don't have any board with phylib hardware timestamping, so I would
> > appreciate testing (especially on lan966x, the most intricate
> > conversion). I was, however, able to test netdev level timestamping,
> > because I also have some more unsubmitted conversions in progress:
> >
> > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7
>
> I have tested this patch series on lan966x. In both cases, when there is
> a PHY that supports HW timestamping and when the isn't a PHY that
> supports HW timestamping. In both cases it behaves as expected. Thanks!
> Tested-by: Horatiu Vultur <[email protected]>
Thanks for testing! I'll apply this tag to patches:
net: add NDOs for configuring hardware timestamping
net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()
net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from converted drivers
On 7/17/2023 4:25 AM, Vladimir Oltean wrote:
> Hi Maxim,
>
> On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote:
>> Vladimir, thank you for taking over and improving this patch stack!
>>
>> I see you dropped the netdevsim patch:
>> https://www.spinics.net/lists/netdev/msg901378.html
>> Do you believe it's not useful any more since the rest of the
>> patches in the stack were tested through other means?
>
> I just didn't consider that adding mock hardware timestamping support to
> netdevsim was necessary or useful, considering the number of other driver
> conversions that will have to be submitted. Just an extra, avoidable effort
> for me.
FWIW I think its unnecessary as well.
I read through the implementation and noticed that it also used the
.get_ts_info callback by directly reporting whatever type and filter was
set via SIOCSHWTSTAMP, rather than reporting some device capability.
Obviously as a mock device there is no real capability, and that was
likely done for testing purposes. However, it would still leave the
kernel with an implementation that does not follow the expected rules
for these ioctls.
For a mock device thats not really an issue. However, I'd prefer to
avoid such in the kernel so that its not available for copying when
someone without such knowledge comes along to write a new driver.
On Mon, Jul 17, 2023 at 01:23:02PM -0700, Jacob Keller wrote:
> For a mock device thats not really an issue. However, I'd prefer to
> avoid such in the kernel so that its not available for copying when
> someone without such knowledge comes along to write a new driver.
+1