* [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags
@ 2025-04-14 21:26 Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info Jacob Keller
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Jacob Keller @ 2025-04-14 21:26 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Tony Nguyen, Przemek Kitszel,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Bryan Whitehead,
UNGLinuxDriver, Horatiu Vultur, Paul Barker,
Niklas Söderlund, Richard Cochran, Heiner Kallweit,
Russell King, Andrei Botila, Claudiu Manoil, Alexandre Belloni,
Vadim Fedorenko
Cc: netdev, linux-kernel, intel-wired-lan, linux-rdma,
linux-renesas-soc, Jacob Keller
Both the PTP_EXTTS_REQUEST(2) and PTP_PEROUT_REQUEST(2) ioctls take flags
from userspace to modify their behavior. Drivers are supposed to check
these flags, rejecting requests for flags they do not support.
Many drivers today do not check these flags, despite many attempts to
squash individual drivers as these mistakes are discovered. Additionally,
any new flags added can require updating every driver if their validation
checks are poorly implemented.
It is clear that driver authors will not reliably check for unsupported
flags. The root of the issue is that drivers must essentially opt out of
every flag, rather than opt in to the ones they support.
Instead, lets introduce .supported_perout_flags and .supported_extts_flags
to the ptp_clock_info structure. This is a pattern taken from several
ethtool ioctls which enabled validation to move out of the drivers and into
the shared ioctl handlers. This pattern has worked quite well and makes it
much more difficult for drivers to accidentally accept flags they do not
support.
With this approach, drivers which do not set the supported fields will have
the core automatically reject any request which has flags. Drivers must opt
in to each flag they support by adding it to the list, with the sole
exception being the PTP_ENABLE_FEATURE flag of the PTP_EXTTS_REQUEST ioctl
since it is entirely handled by the ptp_chardev.c file.
This change will ensure that all current and future drivers are safe for
extension when we need to extend these ioctls.
I opted to keep all the driver changes into one patch per ioctl type. The
changes are relatively small and straight forward. Splitting it per-driver
would make the series large, and also break flags between the introduction
of the supported field and setting it in each driver.
The non-Intel drivers are compile-tested only, and I would appreciate
confirmation and testing from their respective maintainers. (It is also
likely that I missed some of the driver authors especially for drivers
which didn't make any checks at all and do not set either of the supported
flags yet)
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes in v2:
- Expand PTP_EXTTS_EDGES in all .supported_extts_flags assignment
- Remove PTP_ENABLE_FEATURE from all .supported_extts_flags assignment
- Add a .supported_extts_flags assignment to ravb driver, even tho it
doesn't currently support PTP_STRICT_FLAGS. The driver did previously
check these, so I think its better to add them even if its equivalent.
- Added Vadim's Reviewed-by to patch 2/2
- Link to v1: https://lore.kernel.org/r/20250408-jk-supported-perout-flags-v1-0-d2f8e3df64f3@intel.com
---
Jacob Keller (2):
net: ptp: introduce .supported_extts_flags to ptp_clock_info
net: ptp: introduce .supported_perout_flags to ptp_clock_info
include/linux/ptp_clock_kernel.h | 18 +++++++++++++++
drivers/net/dsa/mv88e6xxx/ptp.c | 11 ++++-----
drivers/net/dsa/sja1105/sja1105_ptp.c | 14 +++---------
drivers/net/ethernet/intel/ice/ice_ptp.c | 16 +++++--------
drivers/net/ethernet/intel/igb/igb_ptp.c | 20 +++++------------
drivers/net/ethernet/intel/igc/igc_ptp.c | 14 +++---------
.../net/ethernet/mellanox/mlx5/core/lib/clock.c | 26 ++++++----------------
drivers/net/ethernet/microchip/lan743x_ptp.c | 14 ++++--------
.../net/ethernet/microchip/lan966x/lan966x_ptp.c | 14 ++++--------
drivers/net/ethernet/mscc/ocelot_ptp.c | 5 -----
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 2 ++
drivers/net/ethernet/renesas/ravb_ptp.c | 11 +--------
drivers/net/phy/dp83640.c | 13 +++--------
drivers/net/phy/micrel.c | 17 +++++---------
drivers/net/phy/microchip_rds_ptp.c | 5 +----
drivers/net/phy/nxp-c45-tja11xx.c | 13 ++++-------
drivers/ptp/ptp_chardev.c | 16 ++++++++++++-
drivers/ptp/ptp_clockmatrix.c | 14 ++----------
drivers/ptp/ptp_fc3.c | 1 +
drivers/ptp/ptp_idt82p33.c | 15 +++----------
20 files changed, 91 insertions(+), 168 deletions(-)
---
base-commit: b65999e7238e6f2a48dc77c8c2109c48318ff41b
change-id: 20250408-jk-supported-perout-flags-43219dcee093
Best regards,
--
Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v2 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info
2025-04-14 21:26 [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
@ 2025-04-14 21:26 ` Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 2/2] net: ptp: introduce .supported_perout_flags " Jacob Keller
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jacob Keller @ 2025-04-14 21:26 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Tony Nguyen, Przemek Kitszel,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Bryan Whitehead,
UNGLinuxDriver, Horatiu Vultur, Paul Barker,
Niklas Söderlund, Richard Cochran, Heiner Kallweit,
Russell King, Andrei Botila, Claudiu Manoil, Alexandre Belloni,
Vadim Fedorenko
Cc: netdev, linux-kernel, intel-wired-lan, linux-rdma,
linux-renesas-soc, Jacob Keller
The PTP_EXTTS_REQUEST(2) ioctl has a flags field which specifies how the
external timestamp request should behave. This includes which edge of the
signal to timestamp, as well as a specialized "offset" mode. It is expected
that more flags will be added in the future.
Driver authors routinely do not check the flags, often accepting requests
with flags which they do not support. Even drivers which do check flags may
not be future-proofed to reject flags not yet defined. Thus, any future
flag additions often require manually updating drivers to reject these
flags.
This approach of hoping we catch flag checks during review, or playing
whack-a-mole after the fact is the wrong approach.
Introduce the "supported_extts_flags" field to the ptp_clock_info
structure. This field defines the set of flags the device actually
supports.
Update the core character device logic to check this field and reject
unsupported requests. Getting this right is somewhat tricky. First, to
avoid unnecessary repetition and make basic functionality work when
.supported_extts_flags is 0, the core always accepts the PTP_ENABLE_FEATURE
flag. This flag is used to set the 'on' parameter to the .enable function
and is thus always 'supported' by all drivers.
For backwards compatibility, the PTP_RISING_EDGE and PTP_FALLING_EDGE flags
are merely "hints" when using the old PTP_EXTTS_REQUEST ioctl, and are not
expected to be enforced. If the user issues PTP_EXTTS_REQUEST2, the
PTP_STRICT_FLAGS flag is added which is supposed to inform the driver to
strictly validate the flags and reject unsupported requests. To handle
this, first check if the driver reports PTP_STRICT_FLAGS support. If it
does not, then always allow the PTP_RISING_EDGE and PTP_FALLING_EDGE flags.
This keeps backwards compatibility with the original PTP_EXTTS_REQUEST
ioctl where these flags are not guaranteed to be honored.
This way, drivers which do not set the supported_extts_flags will continue
to accept requests for the original PTP_EXTTS_REQUEST ioctl. The core will
automatically reject requests with new flags, and correctly reject requests
with PTP_STRICT_FLAGS, where the driver is supposed to strictly validate
the flags.
Update the various drivers, refactoring their validation logic into the
.supported_extts_flags field. For consistency and readability,
PTP_ENABLE_FEATURE is not set in the supported flags list, and
PTP_EXTTS_EDGES is expanded to PTP_RISING_EDGE | PTP_FALLING_EDGE in all
cases.
Note the following driver files set n_ext_ts to a non-zero value but did
not check flags at all:
• drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
• drivers/net/ethernet/freescale/enetc/enetc_ptp.c
• drivers/net/ethernet/intel/i40e/i40e_ptp.c
• drivers/net/ethernet/marvell/octeontx2/nic/otx2_ptp.c
• drivers/net/ethernet/renesas/ravb_ptp.c
• drivers/net/ethernet/renesas/rtsn.c
• drivers/net/ethernet/renesas/rtsn.h
• drivers/net/ethernet/ti/am65-cpts.c
• drivers/net/ethernet/ti/cpts.h
• drivers/net/ethernet/ti/icssg/icss_iep.c
• drivers/net/ethernet/xscale/ptp_ixp46x.c
• drivers/net/phy/bcm-phy-ptp.c
• drivers/ptp/ptp_ocp.c
• drivers/ptp/ptp_pch.c
• drivers/ptp/ptp_qoriq.c
These drivers behavior does change slightly: they will now reject the
PTP_EXTTS_REQUEST2 ioctl, because they do not strictly validate their
flags. This also makes them no longer incorrectly accept PTP_EXT_OFFSET.
Also note that the renesas ravb driver does not support PTP_STRICT_FLAGS.
We could leave the .supported_extts_flags as 0, but I added the
PTP_RISING_EDGE | PTP_FALLING_EDGE since the driver previously manually
validated these flags. This is equivalent to 0 because the core will allow
these flags regardless unless PTP_STRICT_FLAGS is also set.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
include/linux/ptp_clock_kernel.h | 12 ++++++++++++
drivers/net/dsa/mv88e6xxx/ptp.c | 11 ++++-------
drivers/net/dsa/sja1105/sja1105_ptp.c | 10 +++-------
drivers/net/ethernet/intel/ice/ice_ptp.c | 12 ++++--------
drivers/net/ethernet/intel/igb/igb_ptp.c | 20 ++++++--------------
drivers/net/ethernet/intel/igc/igc_ptp.c | 10 +++-------
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 11 ++++-------
drivers/net/ethernet/microchip/lan743x_ptp.c | 9 +++------
drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c | 8 ++------
drivers/net/ethernet/renesas/ravb_ptp.c | 7 +------
drivers/net/phy/dp83640.c | 10 +++-------
drivers/net/phy/micrel.c | 8 +++-----
drivers/net/phy/nxp-c45-tja11xx.c | 9 +++------
drivers/ptp/ptp_chardev.c | 14 +++++++++++++-
drivers/ptp/ptp_clockmatrix.c | 14 ++------------
drivers/ptp/ptp_fc3.c | 1 +
drivers/ptp/ptp_idt82p33.c | 15 +++------------
17 files changed, 70 insertions(+), 111 deletions(-)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 0d68d09bedd14c70a5d56b7e61ecd62c8d1571cd..25cba2e5ee69c6a52f0d8a95653988371da379a2 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -68,6 +68,17 @@ struct ptp_system_timestamp {
* @n_per_out: The number of programmable periodic signals.
* @n_pins: The number of programmable pins.
* @pps: Indicates whether the clock supports a PPS callback.
+ *
+ * @supported_extts_flags: The set of flags the driver supports for the
+ * PTP_EXTTS_REQUEST ioctl. The PTP core will use
+ * this list to reject unsupported requests.
+ * PTP_ENABLE_FEATURE is assumed and does not need to
+ * be included. If PTP_STRICT_FLAGS is *not* set,
+ * then both PTP_RISING_EDGE and PTP_FALLING_EDGE
+ * will be assumed. Note that PTP_STRICT_FLAGS must
+ * be set if the drivers wants to honor
+ * PTP_EXTTS_REQUEST2 and any future flags.
+ *
* @pin_config: Array of length 'n_pins'. If the number of
* programmable pins is nonzero, then drivers must
* allocate and initialize this array.
@@ -174,6 +185,7 @@ struct ptp_clock_info {
int n_per_out;
int n_pins;
int pps;
+ unsigned int supported_extts_flags;
struct ptp_pin_desc *pin_config;
int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm);
int (*adjphase)(struct ptp_clock_info *ptp, s32 phase);
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index aed4a4b07f34b1643a8bf51c2501d1f61ef0cf0b..1d3b2c94c53ee2b4b784fbea7244734cbb7eb4a0 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -332,13 +332,6 @@ static int mv88e6352_ptp_enable_extts(struct mv88e6xxx_chip *chip,
int pin;
int err;
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -566,6 +559,10 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
chip->ptp_clock_info.verify = ptp_ops->ptp_verify;
chip->ptp_clock_info.do_aux_work = mv88e6xxx_hwtstamp_work;
+ chip->ptp_clock_info.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
+
if (ptp_ops->set_ptp_cpu_port) {
struct dsa_port *dp;
int upstream = 0;
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 198e787e8560c76d2c0b9c44069addc69068a7dc..3b979d88ca13b554c93e3fc73c005be64b1a72c1 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -820,13 +820,6 @@ static int sja1105_extts_enable(struct sja1105_private *priv,
if (extts->index != 0)
return -EOPNOTSUPP;
- /* Reject requests with unsupported flags */
- if (extts->flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* We can only enable time stamping on both edges, sadly. */
if ((extts->flags & PTP_STRICT_FLAGS) &&
(extts->flags & PTP_ENABLE_FEATURE) &&
@@ -912,6 +905,9 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
.n_pins = 1,
.n_ext_ts = 1,
.n_per_out = 1,
+ .supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS,
};
/* Only used on SJA1105 */
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 1fd1ae03eb90960d1e3e20acb0638baecaa995f5..96f68c356fe81b6954653f8903faf433ef6018f5 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1624,14 +1624,6 @@ static int ice_ptp_cfg_extts(struct ice_pf *pf, struct ptp_extts_request *rq,
int pin_desc_idx;
u8 tmr_idx;
- /* Reject requests with unsupported flags */
-
- if (rq->flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
tmr_idx = hw->func_caps.ts_func_info.tmr_index_owned;
chan = rq->index;
@@ -2737,6 +2729,10 @@ static void ice_ptp_set_caps(struct ice_pf *pf)
info->enable = ice_ptp_gpio_enable;
info->verify = ice_verify_pin;
+ info->supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
+
switch (pf->hw.mac_type) {
case ICE_MAC_E810:
ice_ptp_set_funcs_e810(pf);
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index f323e1c1989f1bfbbf1f04043c2c0f14ae8c716f..793c96016288089844f521a36da8908ba820e3db 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -502,13 +502,6 @@ static int igb_ptp_feature_enable_82580(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Both the rising and falling edge are timestamped */
if (rq->extts.flags & PTP_STRICT_FLAGS &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -658,13 +651,6 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Reject requests failing to enable both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -1356,6 +1342,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->ptp_caps.n_per_out = IGB_N_PEROUT;
adapter->ptp_caps.n_pins = IGB_N_SDP;
adapter->ptp_caps.pps = 0;
+ adapter->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
adapter->ptp_caps.pin_config = adapter->sdp_config;
adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
adapter->ptp_caps.adjtime = igb_ptp_adjtime_82576;
@@ -1378,6 +1367,9 @@ void igb_ptp_init(struct igb_adapter *adapter)
adapter->ptp_caps.n_ext_ts = IGB_N_EXTTS;
adapter->ptp_caps.n_per_out = IGB_N_PEROUT;
adapter->ptp_caps.n_pins = IGB_N_SDP;
+ adapter->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
adapter->ptp_caps.pps = 1;
adapter->ptp_caps.pin_config = adapter->sdp_config;
adapter->ptp_caps.adjfine = igb_ptp_adjfine_82580;
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 946edbad43022c9fdb5f2196b72c0e2d07436ed5..0c6c0cc78facae697f0b96759c8e3f3a5863feaa 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -257,13 +257,6 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Reject requests failing to enable both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -1146,6 +1139,9 @@ void igc_ptp_init(struct igc_adapter *adapter)
adapter->ptp_caps.pin_config = adapter->sdp_config;
adapter->ptp_caps.n_ext_ts = IGC_N_EXTTS;
adapter->ptp_caps.n_per_out = IGC_N_PEROUT;
+ adapter->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
adapter->ptp_caps.n_pins = IGC_N_SDP;
adapter->ptp_caps.verify = igc_ptp_verify_pin;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index 65a94e46edcf167e93d8bd72f001d653dc433d8c..3eee84430ac98b7fe61469be684a0d1e92a03b39 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -643,13 +643,6 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp,
int pin = -1;
int err = 0;
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -1034,6 +1027,10 @@ static void mlx5_init_pin_config(struct mlx5_core_dev *mdev)
clock->ptp_info.verify = mlx5_ptp_verify;
clock->ptp_info.pps = 1;
+ clock->ptp_info.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
+
for (i = 0; i < clock->ptp_info.n_pins; i++) {
snprintf(clock->ptp_info.pin_config[i].name,
sizeof(clock->ptp_info.pin_config[i].name),
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 0be44dcb339387e9756f36f909f65c20870bc49b..b171c893175b3dd682f48f4adf9a724f51479332 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -942,12 +942,6 @@ static int lan743x_ptp_io_extts(struct lan743x_adapter *adapter, int on,
extts = &ptp->extts[index];
- if (extts_request->flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
if (on) {
extts_pin = ptp_find_pin(ptp->ptp_clock, PTP_PF_EXTTS, index);
if (extts_pin < 0)
@@ -1543,6 +1537,9 @@ int lan743x_ptp_open(struct lan743x_adapter *adapter)
ptp->ptp_clock_info.n_per_out = LAN743X_PTP_N_EVENT_CHAN;
ptp->ptp_clock_info.n_pins = n_pins;
ptp->ptp_clock_info.pps = LAN743X_PTP_N_PPS;
+ ptp->ptp_clock_info.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
ptp->ptp_clock_info.pin_config = ptp->pin_config;
ptp->ptp_clock_info.adjfine = lan743x_ptpci_adjfine;
ptp->ptp_clock_info.adjtime = lan743x_ptpci_adjtime;
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
index 63905bb5a63a838516f394ca051a00202c9a82a7..8cf41b0977b2ce140145ae0c293b7340c698eba6 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
@@ -917,12 +917,6 @@ static int lan966x_ptp_extts(struct ptp_clock_info *ptp,
if (lan966x->ptp_ext_irq <= 0)
return -EOPNOTSUPP;
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(phc->clock, PTP_PF_EXTTS, rq->extts.index);
if (pin == -1 || pin >= LAN966X_PHC_PINS_NUM)
return -EINVAL;
@@ -978,6 +972,8 @@ static struct ptp_clock_info lan966x_ptp_clock_info = {
.n_per_out = LAN966X_PHC_PINS_NUM,
.n_ext_ts = LAN966X_PHC_PINS_NUM,
.n_pins = LAN966X_PHC_PINS_NUM,
+ .supported_extts_flags = PTP_RISING_EDGE |
+ PTP_STRICT_FLAGS,
};
static int lan966x_ptp_phc_init(struct lan966x *lan966x,
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index b4365906669f3bd40953813e263aeaafd2e1eb70..ab1ffdc7ee4f6240ce54f8337c0b9349ff77440f 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -176,12 +176,6 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
struct net_device *ndev = priv->ndev;
unsigned long flags;
- /* Reject requests with unsupported flags */
- if (req->flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE))
- return -EOPNOTSUPP;
-
if (req->index)
return -EINVAL;
@@ -287,6 +281,7 @@ static const struct ptp_clock_info ravb_ptp_info = {
.max_adj = 50000000,
.n_ext_ts = N_EXT_TS,
.n_per_out = N_PER_OUT,
+ .supported_extts_flags = PTP_RISING_EDGE | PTP_FALLING_EDGE,
.adjfine = ravb_ptp_adjfine,
.adjtime = ravb_ptp_adjtime,
.gettime64 = ravb_ptp_gettime64,
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 85e231451093f0ed1e7cec3341889f37f8d8935a..c89c255185a6a1526bf0bd7d0b1db5ce8232f152 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -478,13 +478,6 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Reject requests to enable time stamping on both edges. */
if ((rq->extts.flags & PTP_STRICT_FLAGS) &&
(rq->extts.flags & PTP_ENABLE_FEATURE) &&
@@ -1002,6 +995,9 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
clock->caps.n_per_out = N_PER_OUT;
clock->caps.n_pins = DP83640_N_PINS;
clock->caps.pps = 0;
+ clock->caps.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
clock->caps.adjfine = ptp_dp83640_adjfine;
clock->caps.adjtime = ptp_dp83640_adjtime;
clock->caps.gettime64 = ptp_dp83640_gettime;
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 24882d30f68589b2b821d13decdb9f858cdaa609..61123ec4c8780a9442388ba30cb6ced3307bd07b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -3406,11 +3406,6 @@ static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
struct phy_device *phydev = shared->phydev;
int pin;
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_EXTTS_EDGES |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
rq->extts.index);
if (pin == -1 || pin != LAN8814_PTP_EXTTS_NUM)
@@ -3917,6 +3912,9 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
shared->ptp_clock_info.n_ext_ts = LAN8814_PTP_EXTTS_NUM;
shared->ptp_clock_info.n_pins = LAN8814_PTP_GPIO_NUM;
shared->ptp_clock_info.pps = 0;
+ shared->ptp_clock_info.supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS;
shared->ptp_clock_info.pin_config = shared->pin_config;
shared->ptp_clock_info.n_per_out = LAN8814_PTP_PEROUT_NUM;
shared->ptp_clock_info.adjfine = lan8814_ptpci_adjfine;
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 250a018d5546854b65aa118b21ac3ada673a6fd4..8634b4cb1e70840aaf6e03c52afcc68f7db79d3c 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -861,12 +861,6 @@ static int nxp_c45_extts_enable(struct nxp_c45_phy *priv,
const struct nxp_c45_phy_data *data = nxp_c45_get_data(priv->phydev);
int pin;
- if (extts->flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
/* Sampling on both edges is not supported */
if ((extts->flags & PTP_RISING_EDGE) &&
(extts->flags & PTP_FALLING_EDGE) &&
@@ -962,6 +956,9 @@ static int nxp_c45_init_ptp_clock(struct nxp_c45_phy *priv)
.n_pins = ARRAY_SIZE(nxp_c45_ptp_pins),
.n_ext_ts = 1,
.n_per_out = 1,
+ .supported_extts_flags = PTP_RISING_EDGE |
+ PTP_FALLING_EDGE |
+ PTP_STRICT_FLAGS,
};
priv->ptp_clock = ptp_clock_register(&priv->caps,
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 4380e6ddb8495cef98bb89931ce3796a9892441e..c24228c139549d14d95a1ff080e75c28420f40bd 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -162,6 +162,7 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
{
struct ptp_clock *ptp =
container_of(pccontext->clk, struct ptp_clock, clock);
+ unsigned int i, pin_index, supported_extts_flags;
struct ptp_sys_offset_extended *extoff = NULL;
struct ptp_sys_offset_precise precise_offset;
struct system_device_crosststamp xtstamp;
@@ -172,7 +173,6 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
struct ptp_clock_request req;
struct ptp_clock_caps caps;
struct ptp_clock_time *pct;
- unsigned int i, pin_index;
struct ptp_pin_desc pd;
struct timespec64 ts;
int enable, err = 0;
@@ -240,6 +240,18 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
err = -EINVAL;
break;
}
+ supported_extts_flags = ptp->info->supported_extts_flags;
+ /* The PTP_ENABLE_FEATURE flag is always supported. */
+ supported_extts_flags |= PTP_ENABLE_FEATURE;
+ /* If the driver does not support strictly checking flags, the
+ * PTP_RISING_EDGE and PTP_FALLING_EDGE flags are merely
+ * hints which are not enforced.
+ */
+ if (!(supported_extts_flags & PTP_STRICT_FLAGS))
+ supported_extts_flags |= PTP_EXTTS_EDGES;
+ /* Reject unsupported flags */
+ if (req.extts.flags & ~supported_extts_flags)
+ return -EOPNOTSUPP;
req.type = PTP_CLK_REQ_EXTTS;
enable = req.extts.flags & PTP_ENABLE_FEATURE ? 1 : 0;
if (mutex_lock_interruptible(&ptp->pincfg_mux))
diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index fbb3fa8fc60b2f6571519491620ace7d7a9f8a1a..b8d4df8c6da2c954364b9e2e4a82b1191c197071 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -283,18 +283,6 @@ static int idtcm_extts_enable(struct idtcm_channel *channel,
idtcm = channel->idtcm;
old_mask = idtcm->extts_mask;
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
- /* Reject requests to enable time stamping on falling edge */
- if ((rq->extts.flags & PTP_ENABLE_FEATURE) &&
- (rq->extts.flags & PTP_FALLING_EDGE))
- return -EOPNOTSUPP;
-
if (index >= MAX_TOD)
return -EINVAL;
@@ -2043,6 +2031,7 @@ static const struct ptp_clock_info idtcm_caps = {
.n_per_out = 12,
.n_ext_ts = MAX_TOD,
.n_pins = MAX_REF_CLK,
+ .supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS,
.adjphase = &idtcm_adjphase,
.getmaxphase = &idtcm_getmaxphase,
.adjfine = &idtcm_adjfine,
@@ -2060,6 +2049,7 @@ static const struct ptp_clock_info idtcm_caps_deprecated = {
.n_per_out = 12,
.n_ext_ts = MAX_TOD,
.n_pins = MAX_REF_CLK,
+ .supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS,
.adjphase = &idtcm_adjphase,
.getmaxphase = &idtcm_getmaxphase,
.adjfine = &idtcm_adjfine,
diff --git a/drivers/ptp/ptp_fc3.c b/drivers/ptp/ptp_fc3.c
index cfced36c70bcb2bca4bfd21b9524c32c4999dba9..70002500170eae445244cc82ec3979c3ac07d634 100644
--- a/drivers/ptp/ptp_fc3.c
+++ b/drivers/ptp/ptp_fc3.c
@@ -592,6 +592,7 @@ static const struct ptp_clock_info idtfc3_caps = {
.max_adj = MAX_FFO_PPB,
.n_per_out = 1,
.n_ext_ts = 1,
+ .supported_extts_flags = PTP_STRICT_FLAGS | PTP_EXT_OFFSET,
.adjphase = &idtfc3_adjphase,
.adjfine = &idtfc3_adjfine,
.adjtime = &idtfc3_adjtime,
diff --git a/drivers/ptp/ptp_idt82p33.c b/drivers/ptp/ptp_idt82p33.c
index b2fd94d4f863129a5fccbb1233ce0c94602de2a5..f01c50dfa44e8f398718651f35d4a03fe6ca93c8 100644
--- a/drivers/ptp/ptp_idt82p33.c
+++ b/drivers/ptp/ptp_idt82p33.c
@@ -246,18 +246,6 @@ static int idt82p33_extts_enable(struct idt82p33_channel *channel,
idt82p33 = channel->idt82p33;
old_mask = idt82p33->extts_mask;
- /* Reject requests with unsupported flags */
- if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
- PTP_RISING_EDGE |
- PTP_FALLING_EDGE |
- PTP_STRICT_FLAGS))
- return -EOPNOTSUPP;
-
- /* Reject requests to enable time stamping on falling edge */
- if ((rq->extts.flags & PTP_ENABLE_FEATURE) &&
- (rq->extts.flags & PTP_FALLING_EDGE))
- return -EOPNOTSUPP;
-
if (index >= MAX_PHC_PLL)
return -EINVAL;
@@ -1187,6 +1175,9 @@ static void idt82p33_caps_init(u32 index, struct ptp_clock_info *caps,
caps->pin_config = pin_cfg;
+ caps->supported_extts_flags = PTP_RISING_EDGE |
+ PTP_STRICT_FLAGS;
+
for (i = 0; i < max_pins; ++i) {
ppd = &pin_cfg[i];
--
2.48.1.397.gec9d649cc640
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v2 2/2] net: ptp: introduce .supported_perout_flags to ptp_clock_info
2025-04-14 21:26 [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info Jacob Keller
@ 2025-04-14 21:26 ` Jacob Keller
2025-04-15 8:29 ` [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Kory Maincent
2025-04-16 13:38 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: Jacob Keller @ 2025-04-14 21:26 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Tony Nguyen, Przemek Kitszel,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Bryan Whitehead,
UNGLinuxDriver, Horatiu Vultur, Paul Barker,
Niklas Söderlund, Richard Cochran, Heiner Kallweit,
Russell King, Andrei Botila, Claudiu Manoil, Alexandre Belloni,
Vadim Fedorenko
Cc: netdev, linux-kernel, intel-wired-lan, linux-rdma,
linux-renesas-soc, Jacob Keller
The PTP_PEROUT_REQUEST2 ioctl has gained support for flags specifying
specific output behavior including PTP_PEROUT_ONE_SHOT,
PTP_PEROUT_DUTY_CYCLE, PTP_PEROUT_PHASE.
Driver authors are notorious for not checking the flags of the request.
This results in misinterpreting the request, generating an output signal
that does not match the requested value. It is anticipated that even more
flags will be added in the future, resulting in even more broken requests.
Expecting these issues to be caught during review or playing whack-a-mole
after the fact is not a great solution.
Instead, introduce the supported_perout_flags field in the ptp_clock_info
structure. Update the core character device logic to explicitly reject any
request which has a flag not on this list.
This ensures that drivers must 'opt in' to the flags they support. Drivers
which don't set the .supported_perout_flags field will not need to check
that unsupported flags aren't passed, as the core takes care of this.
Update the drivers which do support flags to set this new field.
Note the following driver files set n_per_out to a non-zero value but did
not check the flags at all:
• drivers/ptp/ptp_clockmatrix.c
• drivers/ptp/ptp_idt82p33.c
• drivers/ptp/ptp_fc3.c
• drivers/net/ethernet/ti/am65-cpts.c
• drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
• drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
• drivers/net/dsa/sja1105/sja1105_ptp.c
• drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
• drivers/net/ethernet/mscc/ocelot_vsc7514.c
• drivers/net/ethernet/intel/i40e/i40e_ptp.c
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
include/linux/ptp_clock_kernel.h | 6 ++++++
drivers/net/dsa/sja1105/sja1105_ptp.c | 4 ----
drivers/net/ethernet/intel/ice/ice_ptp.c | 4 +---
drivers/net/ethernet/intel/igc/igc_ptp.c | 4 ----
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 15 +++------------
drivers/net/ethernet/microchip/lan743x_ptp.c | 5 +----
drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c | 6 ++----
drivers/net/ethernet/mscc/ocelot_ptp.c | 5 -----
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 2 ++
drivers/net/ethernet/renesas/ravb_ptp.c | 4 ----
drivers/net/phy/dp83640.c | 3 ---
drivers/net/phy/micrel.c | 9 ++-------
drivers/net/phy/microchip_rds_ptp.c | 5 +----
drivers/net/phy/nxp-c45-tja11xx.c | 4 +---
drivers/ptp/ptp_chardev.c | 2 ++
15 files changed, 21 insertions(+), 57 deletions(-)
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 25cba2e5ee69c6a52f0d8a95653988371da379a2..eced7e9bf69a81f9b87b5fd4ff56074647f7aef4 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -69,6 +69,11 @@ struct ptp_system_timestamp {
* @n_pins: The number of programmable pins.
* @pps: Indicates whether the clock supports a PPS callback.
*
+ * @supported_perout_flags: The set of flags the driver supports for the
+ * PTP_PEROUT_REQUEST ioctl. The PTP core will
+ * reject a request with any flag not specified
+ * here.
+ *
* @supported_extts_flags: The set of flags the driver supports for the
* PTP_EXTTS_REQUEST ioctl. The PTP core will use
* this list to reject unsupported requests.
@@ -185,6 +190,7 @@ struct ptp_clock_info {
int n_per_out;
int n_pins;
int pps;
+ unsigned int supported_perout_flags;
unsigned int supported_extts_flags;
struct ptp_pin_desc *pin_config;
int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm);
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 3b979d88ca13b554c93e3fc73c005be64b1a72c1..3abc64aec411acd95ef1ef608f931756d0e6cc98 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -737,10 +737,6 @@ static int sja1105_per_out_enable(struct sja1105_private *priv,
if (perout->index != 0)
return -EOPNOTSUPP;
- /* Reject requests with unsupported flags */
- if (perout->flags)
- return -EOPNOTSUPP;
-
mutex_lock(&ptp_data->lock);
rc = sja1105_change_ptp_clk_pin_func(priv, PTP_PF_PEROUT);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 96f68c356fe81b6954653f8903faf433ef6018f5..be691b716edb000364868cca2ad6f5e6f02aece7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1794,9 +1794,6 @@ static int ice_ptp_cfg_perout(struct ice_pf *pf, struct ptp_perout_request *rq,
struct ice_hw *hw = &pf->hw;
int pin_desc_idx;
- if (rq->flags & ~PTP_PEROUT_PHASE)
- return -EOPNOTSUPP;
-
pin_desc_idx = ice_ptp_find_pin_idx(pf, PTP_PF_PEROUT, rq->index);
if (pin_desc_idx < 0)
return -EIO;
@@ -2732,6 +2729,7 @@ static void ice_ptp_set_caps(struct ice_pf *pf)
info->supported_extts_flags = PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS;
+ info->supported_perout_flags = PTP_PEROUT_PHASE;
switch (pf->hw.mac_type) {
case ICE_MAC_E810:
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 0c6c0cc78facae697f0b96759c8e3f3a5863feaa..b6c60b3d0e3a3f14d07fec6f9c7c4e0e708d54bf 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -293,10 +293,6 @@ static int igc_ptp_feature_enable_i225(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
- /* Reject requests with unsupported flags */
- if (rq->perout.flags)
- return -EOPNOTSUPP;
-
if (on) {
pin = ptp_find_pin(igc->ptp_clock, PTP_PF_PEROUT,
rq->perout.index);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index 3eee84430ac98b7fe61469be684a0d1e92a03b39..cec18efadc7330c84ce545efc5922c359ac6b470 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -813,12 +813,6 @@ static int perout_conf_npps_real_time(struct mlx5_core_dev *mdev, struct ptp_clo
return 0;
}
-static bool mlx5_perout_verify_flags(struct mlx5_core_dev *mdev, unsigned int flags)
-{
- return ((!mlx5_npps_real_time_supported(mdev) && flags) ||
- (mlx5_npps_real_time_supported(mdev) && flags & ~PTP_PEROUT_DUTY_CYCLE));
-}
-
static int mlx5_perout_configure(struct ptp_clock_info *ptp,
struct ptp_clock_request *rq,
int on)
@@ -854,12 +848,6 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp,
goto unlock;
}
- /* Reject requests with unsupported flags */
- if (mlx5_perout_verify_flags(mdev, rq->perout.flags)) {
- err = -EOPNOTSUPP;
- goto unlock;
- }
-
if (on) {
pin_mode = MLX5_PIN_MODE_OUT;
pattern = MLX5_OUT_PATTERN_PERIODIC;
@@ -1031,6 +1019,9 @@ static void mlx5_init_pin_config(struct mlx5_core_dev *mdev)
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS;
+ if (mlx5_npps_real_time_supported(mdev))
+ clock->ptp_info.supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
+
for (i = 0; i < clock->ptp_info.n_pins; i++) {
snprintf(clock->ptp_info.pin_config[i].name,
sizeof(clock->ptp_info.pin_config[i].name),
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index b171c893175b3dd682f48f4adf9a724f51479332..b07f5b099a2b400bc47ff04573dc2ea7a0be0cee 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -463,10 +463,6 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on,
struct lan743x_ptp_perout *perout = &ptp->perout[index];
int ret = 0;
- /* Reject requests with unsupported flags */
- if (perout_request->flags & ~PTP_PEROUT_DUTY_CYCLE)
- return -EOPNOTSUPP;
-
if (on) {
perout_pin = ptp_find_pin(ptp->ptp_clock, PTP_PF_PEROUT,
perout_request->index);
@@ -1540,6 +1536,7 @@ int lan743x_ptp_open(struct lan743x_adapter *adapter)
ptp->ptp_clock_info.supported_extts_flags = PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS;
+ ptp->ptp_clock_info.supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
ptp->ptp_clock_info.pin_config = ptp->pin_config;
ptp->ptp_clock_info.adjfine = lan743x_ptpci_adjfine;
ptp->ptp_clock_info.adjtime = lan743x_ptpci_adjtime;
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
index 8cf41b0977b2ce140145ae0c293b7340c698eba6..098406e2e5bb2d07a844763d60ab45a5609e7a22 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c
@@ -815,10 +815,6 @@ static int lan966x_ptp_perout(struct ptp_clock_info *ptp,
bool pps = false;
int pin;
- if (rq->perout.flags & ~(PTP_PEROUT_DUTY_CYCLE |
- PTP_PEROUT_PHASE))
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(phc->clock, PTP_PF_PEROUT, rq->perout.index);
if (pin == -1 || pin >= LAN966X_PHC_PINS_NUM)
return -EINVAL;
@@ -974,6 +970,8 @@ static struct ptp_clock_info lan966x_ptp_clock_info = {
.n_pins = LAN966X_PHC_PINS_NUM,
.supported_extts_flags = PTP_RISING_EDGE |
PTP_STRICT_FLAGS,
+ .supported_perout_flags = PTP_PEROUT_DUTY_CYCLE |
+ PTP_PEROUT_PHASE,
};
static int lan966x_ptp_phc_init(struct lan966x *lan966x,
diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
index cc1088988da0948bd7f6212dbeace5c032383c26..d2a0a32f75ea90529641d2288fa56d3ab6d0f2e6 100644
--- a/drivers/net/ethernet/mscc/ocelot_ptp.c
+++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
@@ -211,11 +211,6 @@ int ocelot_ptp_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
- /* Reject requests with unsupported flags */
- if (rq->perout.flags & ~(PTP_PEROUT_DUTY_CYCLE |
- PTP_PEROUT_PHASE))
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(ocelot->ptp_clock, PTP_PF_PEROUT,
rq->perout.index);
if (pin == 0)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 055b55651a49fdc390acc0df22bf4258b78d6c43..498eec8ae61d83455bf9b54d685126daeb11bf6f 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -108,6 +108,8 @@ static struct ptp_clock_info ocelot_ptp_clock_info = {
.n_ext_ts = 0,
.n_per_out = OCELOT_PTP_PINS_NUM,
.n_pins = OCELOT_PTP_PINS_NUM,
+ .supported_perout_flags = PTP_PEROUT_DUTY_CYCLE |
+ PTP_PEROUT_PHASE,
.pps = 0,
.gettime64 = ocelot_ptp_gettime64,
.settime64 = ocelot_ptp_settime64,
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index ab1ffdc7ee4f6240ce54f8337c0b9349ff77440f..226c6c0ab945b652d47b2902ab6f64b742e40a3e 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -206,10 +206,6 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
unsigned long flags;
int error = 0;
- /* Reject requests with unsupported flags */
- if (req->flags)
- return -EOPNOTSUPP;
-
if (req->index)
return -EINVAL;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index c89c255185a6a1526bf0bd7d0b1db5ce8232f152..daab555721df8d1f419034b6e9a7872cb0b003c7 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -506,9 +506,6 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
- /* Reject requests with unsupported flags */
- if (rq->perout.flags)
- return -EOPNOTSUPP;
if (rq->perout.index >= N_PER_OUT)
return -EINVAL;
return periodic_output(clock, rq, on, rq->perout.index);
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 61123ec4c8780a9442388ba30cb6ced3307bd07b..71fb4410c31b170ecee7b40034416a6a0c574503 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -3236,10 +3236,6 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
int pulse_width;
int pin, event;
- /* Reject requests with unsupported flags */
- if (rq->perout.flags & ~PTP_PEROUT_DUTY_CYCLE)
- return -EOPNOTSUPP;
-
mutex_lock(&shared->shared_lock);
event = rq->perout.index;
pin = ptp_find_pin(shared->ptp_clock, PTP_PF_PEROUT, event);
@@ -3915,6 +3911,7 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
shared->ptp_clock_info.supported_extts_flags = PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS;
+ shared->ptp_clock_info.supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
shared->ptp_clock_info.pin_config = shared->pin_config;
shared->ptp_clock_info.n_per_out = LAN8814_PTP_PEROUT_NUM;
shared->ptp_clock_info.adjfine = lan8814_ptpci_adjfine;
@@ -5066,9 +5063,6 @@ static int lan8841_ptp_perout(struct ptp_clock_info *ptp,
int pin;
int ret;
- if (rq->perout.flags & ~PTP_PEROUT_DUTY_CYCLE)
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(ptp_priv->ptp_clock, PTP_PF_PEROUT, rq->perout.index);
if (pin == -1 || pin >= LAN8841_PTP_GPIO_NUM)
return -EINVAL;
@@ -5312,6 +5306,7 @@ static struct ptp_clock_info lan8841_ptp_clock_info = {
.n_per_out = LAN8841_PTP_GPIO_NUM,
.n_ext_ts = LAN8841_PTP_GPIO_NUM,
.n_pins = LAN8841_PTP_GPIO_NUM,
+ .supported_perout_flags = PTP_PEROUT_DUTY_CYCLE,
};
#define LAN8841_OPERATION_MODE_STRAP_LOW_REGISTER 3
diff --git a/drivers/net/phy/microchip_rds_ptp.c b/drivers/net/phy/microchip_rds_ptp.c
index 3e6bf10cdeed9e42a935d75be972bab4233ff1cc..e6514ce04c29fa9eefe8a89398b11578badf1256 100644
--- a/drivers/net/phy/microchip_rds_ptp.c
+++ b/drivers/net/phy/microchip_rds_ptp.c
@@ -224,10 +224,6 @@ static int mchp_rds_ptp_perout(struct ptp_clock_info *ptpci,
struct phy_device *phydev = clock->phydev;
int ret, event_pin, pulsewidth;
- /* Reject requests with unsupported flags */
- if (perout->flags & ~PTP_PEROUT_DUTY_CYCLE)
- return -EOPNOTSUPP;
-
event_pin = ptp_find_pin(clock->ptp_clock, PTP_PF_PEROUT,
perout->index);
if (event_pin != clock->event_pin)
@@ -1259,6 +1255,7 @@ struct mchp_rds_ptp_clock *mchp_rds_ptp_probe(struct phy_device *phydev, u8 mmd,
clock->caps.pps = 0;
clock->caps.n_pins = MCHP_RDS_PTP_N_PIN;
clock->caps.n_per_out = MCHP_RDS_PTP_N_PEROUT;
+ clock->caps.supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
clock->caps.pin_config = clock->pin_config;
clock->caps.adjfine = mchp_rds_ptp_ltc_adjfine;
clock->caps.adjtime = mchp_rds_ptp_ltc_adjtime;
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 8634b4cb1e70840aaf6e03c52afcc68f7db79d3c..f11dd32494c304f2a11780794e13404f17f9bd46 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -763,9 +763,6 @@ static int nxp_c45_perout_enable(struct nxp_c45_phy *priv,
struct phy_device *phydev = priv->phydev;
int pin;
- if (perout->flags & ~PTP_PEROUT_PHASE)
- return -EOPNOTSUPP;
-
pin = ptp_find_pin(priv->ptp_clock, PTP_PF_PEROUT, perout->index);
if (pin < 0)
return pin;
@@ -959,6 +956,7 @@ static int nxp_c45_init_ptp_clock(struct nxp_c45_phy *priv)
.supported_extts_flags = PTP_RISING_EDGE |
PTP_FALLING_EDGE |
PTP_STRICT_FLAGS,
+ .supported_perout_flags = PTP_PEROUT_PHASE,
};
priv->ptp_clock = ptp_clock_register(&priv->caps,
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index c24228c139549d14d95a1ff080e75c28420f40bd..4bf421765d03332234aac405fc594842760037f1 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -324,6 +324,8 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
err = -EINVAL;
break;
}
+ if (req.perout.flags & ~ptp->info->supported_perout_flags)
+ return -EOPNOTSUPP;
req.type = PTP_CLK_REQ_PEROUT;
enable = req.perout.period.sec || req.perout.period.nsec;
if (mutex_lock_interruptible(&ptp->pincfg_mux))
--
2.48.1.397.gec9d649cc640
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags
2025-04-14 21:26 [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 2/2] net: ptp: introduce .supported_perout_flags " Jacob Keller
@ 2025-04-15 8:29 ` Kory Maincent
2025-04-16 13:38 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: Kory Maincent @ 2025-04-15 8:29 UTC (permalink / raw)
To: Jacob Keller
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Tony Nguyen, Przemek Kitszel,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Bryan Whitehead,
UNGLinuxDriver, Horatiu Vultur, Paul Barker,
Niklas Söderlund, Richard Cochran, Heiner Kallweit,
Russell King, Andrei Botila, Claudiu Manoil, Alexandre Belloni,
Vadim Fedorenko, netdev, linux-kernel, intel-wired-lan,
linux-rdma, linux-renesas-soc
On Mon, 14 Apr 2025 14:26:29 -0700
Jacob Keller <jacob.e.keller@intel.com> wrote:
> Both the PTP_EXTTS_REQUEST(2) and PTP_PEROUT_REQUEST(2) ioctls take flags
> from userspace to modify their behavior. Drivers are supposed to check
> these flags, rejecting requests for flags they do not support.
>
> Many drivers today do not check these flags, despite many attempts to
> squash individual drivers as these mistakes are discovered. Additionally,
> any new flags added can require updating every driver if their validation
> checks are poorly implemented.
>
> It is clear that driver authors will not reliably check for unsupported
> flags. The root of the issue is that drivers must essentially opt out of
> every flag, rather than opt in to the ones they support.
>
> Instead, lets introduce .supported_perout_flags and .supported_extts_flags
> to the ptp_clock_info structure. This is a pattern taken from several
> ethtool ioctls which enabled validation to move out of the drivers and into
> the shared ioctl handlers. This pattern has worked quite well and makes it
> much more difficult for drivers to accidentally accept flags they do not
> support.
>
> With this approach, drivers which do not set the supported fields will have
> the core automatically reject any request which has flags. Drivers must opt
> in to each flag they support by adding it to the list, with the sole
> exception being the PTP_ENABLE_FEATURE flag of the PTP_EXTTS_REQUEST ioctl
> since it is entirely handled by the ptp_chardev.c file.
>
> This change will ensure that all current and future drivers are safe for
> extension when we need to extend these ioctls.
>
> I opted to keep all the driver changes into one patch per ioctl type. The
> changes are relatively small and straight forward. Splitting it per-driver
> would make the series large, and also break flags between the introduction
> of the supported field and setting it in each driver.
>
> The non-Intel drivers are compile-tested only, and I would appreciate
> confirmation and testing from their respective maintainers. (It is also
> likely that I missed some of the driver authors especially for drivers
> which didn't make any checks at all and do not set either of the supported
> flags yet)
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
Thank you!
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags
2025-04-14 21:26 [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
` (2 preceding siblings ...)
2025-04-15 8:29 ` [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Kory Maincent
@ 2025-04-16 13:38 ` Jakub Kicinski
3 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2025-04-16 13:38 UTC (permalink / raw)
To: Jacob Keller
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Paolo Abeni, Tony Nguyen, Przemek Kitszel, Saeed Mahameed,
Leon Romanovsky, Tariq Toukan, Bryan Whitehead, UNGLinuxDriver,
Horatiu Vultur, Paul Barker, Niklas Söderlund,
Richard Cochran, Heiner Kallweit, Russell King, Andrei Botila,
Claudiu Manoil, Alexandre Belloni, Vadim Fedorenko, netdev,
linux-kernel, intel-wired-lan, linux-rdma, linux-renesas-soc
On Mon, 14 Apr 2025 14:26:29 -0700 Jacob Keller wrote:
> Both the PTP_EXTTS_REQUEST(2) and PTP_PEROUT_REQUEST(2) ioctls take flags
> from userspace to modify their behavior. Drivers are supposed to check
> these flags, rejecting requests for flags they do not support.
Applied, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-16 13:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-14 21:26 [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 1/2] net: ptp: introduce .supported_extts_flags to ptp_clock_info Jacob Keller
2025-04-14 21:26 ` [PATCH net-next v2 2/2] net: ptp: introduce .supported_perout_flags " Jacob Keller
2025-04-15 8:29 ` [PATCH net-next v2 0/2] net: ptp: driver opt-in for supported PTP ioctl flags Kory Maincent
2025-04-16 13:38 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).