Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [net-next v3 0/7] new PTP ioctl fixes
@ 2019-09-26 18:11 Jacob Keller
  2019-09-26 18:11 ` [net-next v3 1/7] ptp: correctly disable flags on old ioctls Jacob Keller
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller

This series contains patches to fix the various drivers which implemented
external pin input/output support. The drivers did not explicitly reject
unknown/unsupported flags.

Changes since v2:
* Split the external timestamp changes to separate patches per-driver
* Change the check for external timestamp flags to always accept all three
  current flags.
* Add cc for authors of the PTP support, hopefully receiving feedback

Jacob Keller (7):
  ptp: correctly disable flags on old ioctls
  net: reject PTP periodic output requests with unsupported flags
  mv88e6xxx: reject unsupported external timestamp flags
  dp83640: reject unsupported external timestamp flags
  igb: reject unsupported external timestamp flags
  mlx5: reject unsupported external timestamp flags
  renesas: reject unsupported external timestamp flags

 drivers/net/dsa/mv88e6xxx/ptp.c               |  6 +++++
 drivers/net/ethernet/broadcom/tg3.c           |  4 ++++
 drivers/net/ethernet/intel/igb/igb_ptp.c      | 10 +++++++++
 .../ethernet/mellanox/mlx5/core/lib/clock.c   | 10 +++++++++
 drivers/net/ethernet/microchip/lan743x_ptp.c  |  4 ++++
 drivers/net/ethernet/renesas/ravb_ptp.c       | 10 +++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_ptp.c  |  4 ++++
 drivers/net/phy/dp83640.c                     |  8 +++++++
 drivers/ptp/ptp_chardev.c                     |  4 ++--
 include/uapi/linux/ptp_clock.h                | 22 +++++++++++++++++++
 10 files changed, 80 insertions(+), 2 deletions(-)

-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 1/7] ptp: correctly disable flags on old ioctls
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
@ 2019-09-26 18:11 ` Jacob Keller
  2019-10-11  1:03   ` Brown, Aaron F
  2019-09-26 18:11 ` [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags Jacob Keller
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev
  Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
	Felipe Balbi, David S . Miller, Christopher Hall

Commit 415606588c61 ("PTP: introduce new versions of IOCTLs",
2019-09-13) introduced new versions of the PTP ioctls which actually
validate that the flags are acceptable values.

As part of this, it cleared the flags value using a bitwise
and+negation, in an attempt to prevent the old ioctl from accidentally
enabling new features.

This is incorrect for a couple of reasons. First, it results in
accidentally preventing previously working flags on the request ioctl.
By clearing the "valid" flags, we now no longer allow setting the
enable, rising edge, or falling edge flags.

Second, if we add new additional flags in the future, they must not be
set by the old ioctl. (Since the flag wasn't checked before, we could
potentially break userspace programs which sent garbage flag data.

The correct way to resolve this is to check for and clear all but the
originally valid flags.

Create defines indicating which flags are correctly checked and
interpreted by the original ioctls. Use these to clear any bits which
will not be correctly interpreted by the original ioctls.

In the future, new flags must be added to the VALID_FLAGS macros, but
*not* to the V1_VALID_FLAGS macros. In this way, new features may be
exposed over the v2 ioctls, but without breaking previous userspace
which happened to not clear the flags value properly. The old ioctl will
continue to behave the same way, while the new ioctl gains the benefit
of using the flags fields.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/ptp/ptp_chardev.c      |  4 ++--
 include/uapi/linux/ptp_clock.h | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 9c18476d8d10..67d0199840fd 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -155,7 +155,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			err = -EINVAL;
 			break;
 		} else if (cmd == PTP_EXTTS_REQUEST) {
-			req.extts.flags &= ~PTP_EXTTS_VALID_FLAGS;
+			req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
 			req.extts.rsv[0] = 0;
 			req.extts.rsv[1] = 0;
 		}
@@ -184,7 +184,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 			err = -EINVAL;
 			break;
 		} else if (cmd == PTP_PEROUT_REQUEST) {
-			req.perout.flags &= ~PTP_PEROUT_VALID_FLAGS;
+			req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS;
 			req.perout.rsv[0] = 0;
 			req.perout.rsv[1] = 0;
 			req.perout.rsv[2] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f16301015949..59e89a1bc3bb 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -31,15 +31,37 @@
 #define PTP_ENABLE_FEATURE (1<<0)
 #define PTP_RISING_EDGE    (1<<1)
 #define PTP_FALLING_EDGE   (1<<2)
+
+/*
+ * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
+ */
 #define PTP_EXTTS_VALID_FLAGS	(PTP_ENABLE_FEATURE |	\
 				 PTP_RISING_EDGE |	\
 				 PTP_FALLING_EDGE)
 
+/*
+ * flag fields valid for the original PTP_EXTTS_REQUEST ioctl.
+ * DO NOT ADD NEW FLAGS HERE.
+ */
+#define PTP_EXTTS_V1_VALID_FLAGS	(PTP_ENABLE_FEATURE |	\
+					 PTP_RISING_EDGE |	\
+					 PTP_FALLING_EDGE)
+
 /*
  * Bits of the ptp_perout_request.flags field:
  */
 #define PTP_PEROUT_ONE_SHOT (1<<0)
+
+/*
+ * flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
+ */
 #define PTP_PEROUT_VALID_FLAGS	(PTP_PEROUT_ONE_SHOT)
+
+/*
+ * No flags are valid for the original PTP_PEROUT_REQUEST ioctl
+ */
+#define PTP_PEROUT_V1_VALID_FLAGS	(0)
+
 /*
  * struct ptp_clock_time - represents a time value
  *
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
  2019-09-26 18:11 ` [net-next v3 1/7] ptp: correctly disable flags on old ioctls Jacob Keller
@ 2019-09-26 18:11 ` Jacob Keller
  2019-10-11  1:05   ` Brown, Aaron F
  2019-10-12 18:08   ` Richard Cochran
  2019-09-26 18:11 ` [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags Jacob Keller
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev
  Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
	Felipe Balbi, David S . Miller, Christopher Hall

Commit 823eb2a3c4c7 ("PTP: add support for one-shot output") introduced
a new flag for the PTP periodic output request ioctl. This flag is not
currently supported by any driver.

Fix all drivers which implement the periodic output request ioctl to
explicitly reject any request with flags they do not understand. This
ensures that the driver does not accidentally misinterpret the
PTP_PEROUT_ONE_SHOT flag, or any new flag introduced in the future.

This is important for forward compatibility: if a new flag is
introduced, the driver should reject requests to enable the flag until
the driver has actually been modified to support the flag in question.

Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/broadcom/tg3.c                 | 4 ++++
 drivers/net/ethernet/intel/igb/igb_ptp.c            | 4 ++++
 drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 4 ++++
 drivers/net/ethernet/microchip/lan743x_ptp.c        | 4 ++++
 drivers/net/ethernet/renesas/ravb_ptp.c             | 4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c    | 4 ++++
 drivers/net/phy/dp83640.c                           | 3 +++
 7 files changed, 27 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 77f3511b97de..ca3aa1250dd1 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct ptp_clock_info *ptp,
 
 	switch (rq->type) {
 	case PTP_CLK_REQ_PEROUT:
+		/* Reject requests with unsupported flags */
+		if (rq->perout.flags)
+			return -EOPNOTSUPP;
+
 		if (rq->perout.index != 0)
 			return -EINVAL;
 
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index fd3071f55bd3..4997963149f6 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -551,6 +551,10 @@ static int igb_ptp_feature_enable_i210(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(igb->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 0059b290e095..cff6b60de304 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -290,6 +290,10 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp,
 	if (!MLX5_PPS_CAP(mdev))
 		return -EOPNOTSUPP;
 
+	/* Reject requests with unsupported flags */
+	if (rq->perout.flags)
+		return -EOPNOTSUPP;
+
 	if (rq->perout.index >= clock->ptp_info.n_pins)
 		return -EINVAL;
 
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 57b26c2acf87..e8fe9a90fe4f 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on,
 	int pulse_width = 0;
 	int perout_bit = 0;
 
+	/* Reject requests with unsupported flags */
+	if (perout->flags)
+		return -EOPNOTSUPP;
+
 	if (!on) {
 		lan743x_ptp_perout_off(adapter);
 		return 0;
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 9a42580693cb..638f1fc2166f 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -211,6 +211,10 @@ 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/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 173493db038c..352dc4c68625 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
 
 	switch (rq->type) {
 	case PTP_CLK_REQ_PEROUT:
+		/* Reject requests with unsupported flags */
+		if (rq->perout.flags)
+			return -EOPNOTSUPP;
+
 		cfg = &priv->pps[rq->perout.index];
 
 		cfg->start.tv_sec = rq->perout.start.sec;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 6580094161a9..04ad77758920 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -491,6 +491,9 @@ 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);
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
  2019-09-26 18:11 ` [net-next v3 1/7] ptp: correctly disable flags on old ioctls Jacob Keller
  2019-09-26 18:11 ` [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags Jacob Keller
@ 2019-09-26 18:11 ` Jacob Keller
  2019-10-12 18:24   ` Richard Cochran
  2019-09-26 18:11 ` [net-next v3 4/7] dp83640: " Jacob Keller
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Brandon Streiff

Fix the mv88e6xxx PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.

In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.

Cc: Brandon Streiff <brandon.streiff@ni.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/dsa/mv88e6xxx/ptp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 073cbd0bb91b..076e622a64d6 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -273,6 +273,12 @@ 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))
+		return -EOPNOTSUPP;
+
 	pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index);
 
 	if (pin < 0)
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 4/7] dp83640: reject unsupported external timestamp flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
                   ` (2 preceding siblings ...)
  2019-09-26 18:11 ` [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags Jacob Keller
@ 2019-09-26 18:11 ` " Jacob Keller
  2019-10-12 18:28   ` Richard Cochran
  2019-09-26 18:11 ` [net-next v3 5/7] igb: " Jacob Keller
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev
  Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller,
	Stefan Sørensen, Richard Cochran

Fix the dp83640 PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.

In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.

Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/phy/dp83640.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 04ad77758920..2781b0e2d947 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -469,6 +469,11 @@ 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))
+			return -EOPNOTSUPP;
 		index = rq->extts.index;
 		if (index >= N_EXT_TS)
 			return -EINVAL;
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 5/7] igb: reject unsupported external timestamp flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
                   ` (3 preceding siblings ...)
  2019-09-26 18:11 ` [net-next v3 4/7] dp83640: " Jacob Keller
@ 2019-09-26 18:11 ` " Jacob Keller
  2019-10-11  0:57   ` [Intel-wired-lan] " Brown, Aaron F
  2019-10-12 18:31   ` Richard Cochran
  2019-09-26 18:11 ` [net-next v3 6/7] mlx5: " Jacob Keller
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller

Fix the igb PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.

In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 4997963149f6..0bce3e0f1af0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -521,6 +521,12 @@ 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))
+			return -EOPNOTSUPP;
+
 		if (on) {
 			pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
 					   rq->extts.index);
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 6/7] mlx5: reject unsupported external timestamp flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
                   ` (4 preceding siblings ...)
  2019-09-26 18:11 ` [net-next v3 5/7] igb: " Jacob Keller
@ 2019-09-26 18:11 ` " Jacob Keller
  2019-10-12 18:36   ` Richard Cochran
  2019-09-26 18:11 ` [net-next v3 7/7] renesas: " Jacob Keller
  2019-09-27 18:29 ` [net-next v3 0/7] new PTP ioctl fixes David Miller
  7 siblings, 1 reply; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev
  Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Feras Daoud,
	Eugenia Emantayev

Fix the mlx5 core PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.

In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.

Cc: Feras Daoud <ferasda@mellanox.com>
Cc: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index cff6b60de304..9a40f24e3193 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -236,6 +236,12 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp,
 	if (!MLX5_PPS_CAP(mdev))
 		return -EOPNOTSUPP;
 
+	/* Reject requests with unsupported flags */
+	if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+				PTP_RISING_EDGE |
+				PTP_FALLING_EDGE))
+		return -EOPNOTSUPP;
+
 	if (rq->extts.index >= clock->ptp_info.n_pins)
 		return -EINVAL;
 
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [net-next v3 7/7] renesas: reject unsupported external timestamp flags
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
                   ` (5 preceding siblings ...)
  2019-09-26 18:11 ` [net-next v3 6/7] mlx5: " Jacob Keller
@ 2019-09-26 18:11 ` " Jacob Keller
  2019-09-27 16:22   ` Sergei Shtylyov
  2019-10-12 18:38   ` Richard Cochran
  2019-09-27 18:29 ` [net-next v3 0/7] new PTP ioctl fixes David Miller
  7 siblings, 2 replies; 23+ messages in thread
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
  To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Sergei Shtylyov

Fix the renesas PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.

In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.

Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/renesas/ravb_ptp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 638f1fc2166f..666dbee48097 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -182,6 +182,12 @@ 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;
 
-- 
2.23.0.245.gf157bbb9169d


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 7/7] renesas: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 7/7] renesas: " Jacob Keller
@ 2019-09-27 16:22   ` Sergei Shtylyov
  2019-10-12 18:38   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Sergei Shtylyov @ 2019-09-27 16:22 UTC (permalink / raw)
  To: Jacob Keller, netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher

On 09/26/2019 09:11 PM, Jacob Keller wrote:

> Fix the renesas PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.
> 
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 0/7] new PTP ioctl fixes
  2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
                   ` (6 preceding siblings ...)
  2019-09-26 18:11 ` [net-next v3 7/7] renesas: " Jacob Keller
@ 2019-09-27 18:29 ` David Miller
  2019-09-27 22:16   ` Keller, Jacob E
  7 siblings, 1 reply; 23+ messages in thread
From: David Miller @ 2019-09-27 18:29 UTC (permalink / raw)
  To: jacob.e.keller; +Cc: netdev, intel-wired-lan, jeffrey.t.kirsher


Bug fixes should target 'net' not 'net-next'

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [net-next v3 0/7] new PTP ioctl fixes
  2019-09-27 18:29 ` [net-next v3 0/7] new PTP ioctl fixes David Miller
@ 2019-09-27 22:16   ` Keller, Jacob E
  0 siblings, 0 replies; 23+ messages in thread
From: Keller, Jacob E @ 2019-09-27 22:16 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, intel-wired-lan, Kirsher, Jeffrey T

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, September 27, 2019 11:30 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: Re: [net-next v3 0/7] new PTP ioctl fixes
> 
> 
> Bug fixes should target 'net' not 'net-next'

Right, that was my mistake. I think I saw the ioctl change only on the net-next tree so I built the whole series on net-next. Come to think of it, I didn't check the net tree at all. I'll aim to do that better going forward.

Thanks Dave.

Regards,
Jake
 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [Intel-wired-lan] [net-next v3 5/7] igb: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 5/7] igb: " Jacob Keller
@ 2019-10-11  0:57   ` " Brown, Aaron F
  2019-10-12 18:31   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Brown, Aaron F @ 2019-10-11  0:57 UTC (permalink / raw)
  To: Keller, Jacob E, netdev; +Cc: Intel Wired LAN

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf Of
> Jacob Keller
> Sent: Thursday, September 26, 2019 11:11 AM
> To: netdev@vger.kernel.org
> Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Subject: [Intel-wired-lan] [net-next v3 5/7] igb: reject unsupported external
> timestamp flags
> 
> Fix the igb PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [net-next v3 1/7] ptp: correctly disable flags on old ioctls
  2019-09-26 18:11 ` [net-next v3 1/7] ptp: correctly disable flags on old ioctls Jacob Keller
@ 2019-10-11  1:03   ` Brown, Aaron F
  0 siblings, 0 replies; 23+ messages in thread
From: Brown, Aaron F @ 2019-10-11  1:03 UTC (permalink / raw)
  To: Keller, Jacob E, netdev
  Cc: Intel Wired LAN, Kirsher, Jeffrey T, Keller, Jacob E,
	Richard Cochran, Felipe Balbi, David S . Miller, Hall,
	Christopher S

> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Jacob Keller
> Sent: Thursday, September 26, 2019 11:11 AM
> To: netdev@vger.kernel.org
> Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Richard Cochran <richardcochran@gmail.com>; Felipe Balbi
> <felipe.balbi@linux.intel.com>; David S . Miller <davem@davemloft.net>; Hall,
> Christopher S <christopher.s.hall@intel.com>
> Subject: [net-next v3 1/7] ptp: correctly disable flags on old ioctls
> 
> Commit 415606588c61 ("PTP: introduce new versions of IOCTLs",
> 2019-09-13) introduced new versions of the PTP ioctls which actually
> validate that the flags are acceptable values.
> 
> As part of this, it cleared the flags value using a bitwise
> and+negation, in an attempt to prevent the old ioctl from accidentally
> enabling new features.
> 
> This is incorrect for a couple of reasons. First, it results in
> accidentally preventing previously working flags on the request ioctl.
> By clearing the "valid" flags, we now no longer allow setting the
> enable, rising edge, or falling edge flags.
> 
> Second, if we add new additional flags in the future, they must not be
> set by the old ioctl. (Since the flag wasn't checked before, we could
> potentially break userspace programs which sent garbage flag data.
> 
> The correct way to resolve this is to check for and clear all but the
> originally valid flags.
> 
> Create defines indicating which flags are correctly checked and
> interpreted by the original ioctls. Use these to clear any bits which
> will not be correctly interpreted by the original ioctls.
> 
> In the future, new flags must be added to the VALID_FLAGS macros, but
> *not* to the V1_VALID_FLAGS macros. In this way, new features may be
> exposed over the v2 ioctls, but without breaking previous userspace
> which happened to not clear the flags value properly. The old ioctl will
> continue to behave the same way, while the new ioctl gains the benefit
> of using the flags fields.
> 
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/ptp/ptp_chardev.c      |  4 ++--
>  include/uapi/linux/ptp_clock.h | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown <aaron.f.brown@intel.com>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags
  2019-09-26 18:11 ` [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags Jacob Keller
@ 2019-10-11  1:05   ` Brown, Aaron F
  2019-10-12 18:08   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Brown, Aaron F @ 2019-10-11  1:05 UTC (permalink / raw)
  To: Keller, Jacob E, netdev
  Cc: Intel Wired LAN, Kirsher, Jeffrey T, Keller, Jacob E,
	Richard Cochran, Felipe Balbi, David S . Miller, Hall,
	Christopher S

> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Jacob Keller
> Sent: Thursday, September 26, 2019 11:11 AM
> To: netdev@vger.kernel.org
> Cc: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Richard Cochran <richardcochran@gmail.com>; Felipe Balbi
> <felipe.balbi@linux.intel.com>; David S . Miller <davem@davemloft.net>; Hall,
> Christopher S <christopher.s.hall@intel.com>
> Subject: [net-next v3 2/7] net: reject PTP periodic output requests with
> unsupported flags
> 
> Commit 823eb2a3c4c7 ("PTP: add support for one-shot output") introduced
> a new flag for the PTP periodic output request ioctl. This flag is not
> currently supported by any driver.
> 
> Fix all drivers which implement the periodic output request ioctl to
> explicitly reject any request with flags they do not understand. This
> ensures that the driver does not accidentally misinterpret the
> PTP_PEROUT_ONE_SHOT flag, or any new flag introduced in the future.
> 
> This is important for forward compatibility: if a new flag is
> introduced, the driver should reject requests to enable the flag until
> the driver has actually been modified to support the flag in question.
> 
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c                 | 4 ++++
>  drivers/net/ethernet/intel/igb/igb_ptp.c            | 4 ++++
>  drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 4 ++++
>  drivers/net/ethernet/microchip/lan743x_ptp.c        | 4 ++++
>  drivers/net/ethernet/renesas/ravb_ptp.c             | 4 ++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c    | 4 ++++
>  drivers/net/phy/dp83640.c                           | 3 +++
>  7 files changed, 27 insertions(+)
> 

For the igb sections...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags
  2019-09-26 18:11 ` [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags Jacob Keller
  2019-10-11  1:05   ` Brown, Aaron F
@ 2019-10-12 18:08   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:08 UTC (permalink / raw)
  To: Jacob Keller
  Cc: netdev, Intel Wired LAN, Jeffrey Kirsher, Felipe Balbi,
	David S . Miller, Christopher Hall

On Thu, Sep 26, 2019 at 11:11:04AM -0700, Jacob Keller wrote:
> Commit 823eb2a3c4c7 ("PTP: add support for one-shot output") introduced
> a new flag for the PTP periodic output request ioctl. This flag is not
> currently supported by any driver.
> 
> Fix all drivers which implement the periodic output request ioctl to
> explicitly reject any request with flags they do not understand. This
> ensures that the driver does not accidentally misinterpret the
> PTP_PEROUT_ONE_SHOT flag, or any new flag introduced in the future.
> 
> This is important for forward compatibility: if a new flag is
> introduced, the driver should reject requests to enable the flag until
> the driver has actually been modified to support the flag in question.
> 
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Christopher Hall <christopher.s.hall@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags Jacob Keller
@ 2019-10-12 18:24   ` Richard Cochran
  2019-10-12 19:36     ` Keller, Jacob E
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:24 UTC (permalink / raw)
  To: Jacob Keller; +Cc: netdev, Intel Wired LAN, Jeffrey Kirsher, Brandon Streiff

On Thu, Sep 26, 2019 at 11:11:05AM -0700, Jacob Keller wrote:
> Fix the mv88e6xxx PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics

For the record, the semantics are (or should be):

  flags                                                 Meaning                   
  ----------------------------------------------------  -------------------------- 
  PTP_ENABLE_FEATURE                                    invalid                   
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp both edges   

> and each driver seems to
> have interpreted them slightly differently.

This driver has:

  flags                                                 Meaning                   
  ----------------------------------------------------  -------------------------- 
  PTP_ENABLE_FEATURE                                    Time stamp falling edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp rising edge
 
> Cc: Brandon Streiff <brandon.streiff@ni.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 4/7] dp83640: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 4/7] dp83640: " Jacob Keller
@ 2019-10-12 18:28   ` Richard Cochran
  0 siblings, 0 replies; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:28 UTC (permalink / raw)
  To: Jacob Keller
  Cc: netdev, Intel Wired LAN, Jeffrey Kirsher, Stefan Sørensen

On Thu, Sep 26, 2019 at 11:11:06AM -0700, Jacob Keller wrote:
> Fix the dp83640 PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.

This driver has:

  flags                                                 Meaning                   
  ----------------------------------------------------  -------------------------- 
  PTP_ENABLE_FEATURE                                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp falling edge
 
> Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 5/7] igb: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 5/7] igb: " Jacob Keller
  2019-10-11  0:57   ` [Intel-wired-lan] " Brown, Aaron F
@ 2019-10-12 18:31   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:31 UTC (permalink / raw)
  To: Jacob Keller; +Cc: netdev, Intel Wired LAN, Jeffrey Kirsher

On Thu, Sep 26, 2019 at 11:11:07AM -0700, Jacob Keller wrote:
> Fix the igb PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.

This HW always time stamps both edges:

  flags                                                 Meaning                   
  ----------------------------------------------------  -------------------------- 
  PTP_ENABLE_FEATURE                                    Time stamp both edges
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp both edges
  PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp both edges
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp both edges
 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 6/7] mlx5: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 6/7] mlx5: " Jacob Keller
@ 2019-10-12 18:36   ` Richard Cochran
  0 siblings, 0 replies; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:36 UTC (permalink / raw)
  To: Jacob Keller
  Cc: netdev, Intel Wired LAN, Jeffrey Kirsher, Feras Daoud, Eugenia Emantayev

On Thu, Sep 26, 2019 at 11:11:08AM -0700, Jacob Keller wrote:
> Fix the mlx5 core PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.

I'm not 100% sure what this driver does, but if I'm not wrong it
follows the dp83640:

  flags                                                 Meaning
  ----------------------------------------------------  --------------------------
  PTP_ENABLE_FEATURE                                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
  PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
  PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp falling edge

> Cc: Feras Daoud <ferasda@mellanox.com>
> Cc: Eugenia Emantayev <eugenia@mellanox.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 7/7] renesas: reject unsupported external timestamp flags
  2019-09-26 18:11 ` [net-next v3 7/7] renesas: " Jacob Keller
  2019-09-27 16:22   ` Sergei Shtylyov
@ 2019-10-12 18:38   ` Richard Cochran
  1 sibling, 0 replies; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 18:38 UTC (permalink / raw)
  To: Jacob Keller; +Cc: netdev, Intel Wired LAN, Jeffrey Kirsher, Sergei Shtylyov

On Thu, Sep 26, 2019 at 11:11:09AM -0700, Jacob Keller wrote:
> Fix the renesas PTP support to explicitly reject any future flags that
> get added to the external timestamp request ioctl.
> 
> In order to maintain currently functioning code, this patch accepts all
> three current flags. This is because the PTP_RISING_EDGE and
> PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
> have interpreted them slightly differently.

This driver doesn't check the flags. Not knowing this HW, I can't say
which edges are time stamped.
 
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Reviewed-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
  2019-10-12 18:24   ` Richard Cochran
@ 2019-10-12 19:36     ` Keller, Jacob E
  2019-10-12 23:27       ` Richard Cochran
  0 siblings, 1 reply; 23+ messages in thread
From: Keller, Jacob E @ 2019-10-12 19:36 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, Intel Wired LAN, Kirsher, Jeffrey T, Brandon Streiff

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Saturday, October 12, 2019 11:24 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Brandon Streiff
> <brandon.streiff@ni.com>
> Subject: Re: [net-next v3 3/7] mv88e6xxx: reject unsupported external
> timestamp flags
> 
> On Thu, Sep 26, 2019 at 11:11:05AM -0700, Jacob Keller wrote:
> > Fix the mv88e6xxx PTP support to explicitly reject any future flags that
> > get added to the external timestamp request ioctl.
> >
> > In order to maintain currently functioning code, this patch accepts all
> > three current flags. This is because the PTP_RISING_EDGE and
> > PTP_FALLING_EDGE flags have unclear semantics
> 
> For the record, the semantics are (or should be):
> 
>   flags                                                 Meaning
>   ----------------------------------------------------  --------------------------
>   PTP_ENABLE_FEATURE                                    invalid
>   PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
>   PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
>   PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp
> both edges
> 
> > and each driver seems to
> > have interpreted them slightly differently.
> 
> This driver has:
> 
>   flags                                                 Meaning
>   ----------------------------------------------------  --------------------------
>   PTP_ENABLE_FEATURE                                    Time stamp falling edge
>   PTP_ENABLE_FEATURE|PTP_RISING_EDGE                    Time stamp rising edge
>   PTP_ENABLE_FEATURE|PTP_FALLING_EDGE                   Time stamp falling edge
>   PTP_ENABLE_FEATURE|PTP_RISING_EDGE|PTP_FALLING_EDGE   Time stamp
> rising edge
> 
> > Cc: Brandon Streiff <brandon.streiff@ni.com>
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> 
> Reviewed-by: Richard Cochran <richardcochran@gmail.com>

Right, so in practice, unless it supports both edges, it should reject setting both RISING and FALLING together.

Thanks,
Jake

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
  2019-10-12 19:36     ` Keller, Jacob E
@ 2019-10-12 23:27       ` Richard Cochran
  2019-10-14 17:20         ` Keller, Jacob E
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Cochran @ 2019-10-12 23:27 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: netdev, Intel Wired LAN, Kirsher, Jeffrey T, Brandon Streiff

On Sat, Oct 12, 2019 at 07:36:31PM +0000, Keller, Jacob E wrote:
> Right, so in practice, unless it supports both edges, it should reject setting both RISING and FALLING together.

Enforcing that now *could* break existing user space, but I wonder
whether any programs would actually be affected.

Maybe we can add a STRICT flag than requests strict checking.  If user
space uses the "2" ioctl, then we would add this flag before invoking
the driver callback.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
  2019-10-12 23:27       ` Richard Cochran
@ 2019-10-14 17:20         ` Keller, Jacob E
  0 siblings, 0 replies; 23+ messages in thread
From: Keller, Jacob E @ 2019-10-14 17:20 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, Intel Wired LAN, Kirsher, Jeffrey T, Brandon Streiff

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Saturday, October 12, 2019 4:27 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Brandon Streiff
> <brandon.streiff@ni.com>
> Subject: Re: [net-next v3 3/7] mv88e6xxx: reject unsupported external
> timestamp flags
> 
> On Sat, Oct 12, 2019 at 07:36:31PM +0000, Keller, Jacob E wrote:
> > Right, so in practice, unless it supports both edges, it should reject setting both
> RISING and FALLING together.
> 
> Enforcing that now *could* break existing user space, but I wonder
> whether any programs would actually be affected.
> 
> Maybe we can add a STRICT flag than requests strict checking.  If user
> space uses the "2" ioctl, then we would add this flag before invoking
> the driver callback.
> 
> Thanks,
> Richard

That could work. I don't know how much it's worth fixing it, but that would be the right way to fix it, I think.

I think the strict flag should do the following:

a) enforce that you must set at least one of the two edge flags (ensuring that a request to timestamp without one of the edges is rejected)
b) drivers *must* honor the edge flags or exit with a rejection if they can't support it. (unlike without 'strict', which would be like today and driver defined).
c) possibly: reject both flags at once since it doesn't really make sense to create a timestamp for each edge. At least, I think that makes it harder to actually use the timestamps since you need to be more careful to separate the two timestamps.

I'm not 100% sure on (c) but (b) can only be implemented in drivers, since the stack wouldn't know which modes the driver supports.

Thanks,
Jake

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, back to index

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 18:11 [net-next v3 0/7] new PTP ioctl fixes Jacob Keller
2019-09-26 18:11 ` [net-next v3 1/7] ptp: correctly disable flags on old ioctls Jacob Keller
2019-10-11  1:03   ` Brown, Aaron F
2019-09-26 18:11 ` [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags Jacob Keller
2019-10-11  1:05   ` Brown, Aaron F
2019-10-12 18:08   ` Richard Cochran
2019-09-26 18:11 ` [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags Jacob Keller
2019-10-12 18:24   ` Richard Cochran
2019-10-12 19:36     ` Keller, Jacob E
2019-10-12 23:27       ` Richard Cochran
2019-10-14 17:20         ` Keller, Jacob E
2019-09-26 18:11 ` [net-next v3 4/7] dp83640: " Jacob Keller
2019-10-12 18:28   ` Richard Cochran
2019-09-26 18:11 ` [net-next v3 5/7] igb: " Jacob Keller
2019-10-11  0:57   ` [Intel-wired-lan] " Brown, Aaron F
2019-10-12 18:31   ` Richard Cochran
2019-09-26 18:11 ` [net-next v3 6/7] mlx5: " Jacob Keller
2019-10-12 18:36   ` Richard Cochran
2019-09-26 18:11 ` [net-next v3 7/7] renesas: " Jacob Keller
2019-09-27 16:22   ` Sergei Shtylyov
2019-10-12 18:38   ` Richard Cochran
2019-09-27 18:29 ` [net-next v3 0/7] new PTP ioctl fixes David Miller
2019-09-27 22:16   ` Keller, Jacob E

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git