Commit ac1df712 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-ptp-driver-opt-in-for-supported-ptp-ioctl-flags' 

Jacob Keller says:

====================
net: ptp: driver opt-in for supported PTP ioctl flags

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)

v1: https://lore.kernel.org/20250408-jk-supported-perout-flags-v1-0-d2f8e3df64f3@intel.com
====================

Link: https://patch.msgid.link/20250414-jk-supported-perout-flags-v2-0-f6b17d15475c@intel.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents a496d2f0 d9f3e9ec

drivers/net/dsa/mv88e6xxx/ptp.c

+4 −7
Original line number Diff line number Diff line
@@ -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;

drivers/net/dsa/sja1105/sja1105_ptp.c

+3 −11
Original line number Diff line number Diff line
@@ -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);
@@ -820,13 +816,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 +901,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 */

drivers/net/ethernet/intel/ice/ice_ptp.c

+5 −11
Original line number Diff line number Diff line
@@ -1627,14 +1627,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;
@@ -1805,9 +1797,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;
@@ -2740,6 +2729,11 @@ 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;

	info->supported_perout_flags = PTP_PEROUT_PHASE;



	switch (pf->hw.mac_type) {

	case ICE_MAC_E810:

		ice_ptp_set_funcs_e810(pf);

drivers/net/ethernet/intel/igb/igb_ptp.c

+6 −14
Original line number Diff line number Diff line
@@ -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;

drivers/net/ethernet/intel/igc/igc_ptp.c

+3 −11
Original line number Diff line number Diff line
@@ -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) &&
@@ -300,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);
@@ -1146,6 +1135,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;