[PATCH v2 net-next 0/5] dp83640 driver fixes

[PATCH v2 net-next 0/5] dp83640 driver fixes

[Stefan Sørensen]

This series fixes a number of minor bugs in the dp83640 driver.

Stefan Sørensen (5):
  dp83640: Include hash in timestamp/packet matching
  dp83640: Delay scheduled work.
  dp83640: Prune rx timestamp list before reading from it
  ptp: Change ptp_class to a proper bitmask
  dp83640: Only wait for timestamps for packets with timestamping
    enabled.

 drivers/net/phy/dp83640.c    | 64 ++++++++++++++++++++++++++++----------------
 include/linux/ptp_classify.h |  7 ++---
 net/core/ptp_classifier.c    | 16 +++++------
 3 files changed, 53 insertions(+), 34 deletions(-)

v2 changes: Use macro for hash data offset and length and reword commit
description.

-- 
2.5.0

[PATCH v2 net-next 1/5] dp83640: Include hash in timestamp/packet matching

[Stefan Sørensen]

Only using the message type and sequence id for matching timestamps
with packets is error prone, as multiple clients may very well be
sending packets with the same messagetype and timestamp at the same
time. Fix by extending the check to include the hash of bytes 20-29
(source id in PTPv2) that is provided with the timestamps.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 185b03c..cae3b3b 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -20,6 +20,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/crc32.h>
 #include <linux/ethtool.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -787,9 +788,12 @@ static int decode_evnt(struct dp83640_private *dp83640,
 	return parsed;
 }
 
+#define DP83640_PACKET_HASH_OFFSET	20
+#define DP83640_PACKET_HASH_LEN		10
+
 static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
 {
-	u16 *seqid;
+	u16 *seqid, hash;
 	unsigned int offset = 0;
 	u8 *msgtype, *data = skb_mac_header(skb);
 
@@ -819,11 +823,19 @@ static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
 		msgtype = data + offset + OFF_PTP_CONTROL;
 	else
 		msgtype = data + offset;
+	if (rxts->msgtype != (*msgtype & 0xf))
+		return 0;
 
 	seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);
+	if (rxts->seqid != ntohs(*seqid))
+		return 0;
+
+	hash = ether_crc(DP83640_PACKET_HASH_LEN,
+			 data + offset + DP83640_PACKET_HASH_OFFSET) >> 20;
+	if (rxts->hash != hash)
+		return 0;
 
-	return rxts->msgtype == (*msgtype & 0xf) &&
-		rxts->seqid   == ntohs(*seqid);
+	return 1;
 }
 
 static void decode_rxts(struct dp83640_private *dp83640,
-- 
2.5.0

[PATCH v2 net-next 2/5] dp83640: Delay scheduled work.

[Stefan Sørensen]

Currently rx_timestamp_work reschedules itself as a regular workqueue item,
effectively causing it run constantly as long as there are packets left in
the queue. Fix by using delayed workqueue items, limiting it to run only
every two jiffies.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index cae3b3b..69a2f56 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -69,6 +69,8 @@
 /* phyter seems to miss the mark by 16 ns */
 #define ADJTIME_FIX	16
 
+#define SKB_TIMESTAMP_TIMEOUT	2 /* jiffies */
+
 #if defined(__BIG_ENDIAN)
 #define ENDIAN_FLAG	0
 #elif defined(__LITTLE_ENDIAN)
@@ -111,7 +113,7 @@ struct dp83640_private {
 	struct list_head list;
 	struct dp83640_clock *clock;
 	struct phy_device *phydev;
-	struct work_struct ts_work;
+	struct delayed_work ts_work;
 	int hwts_tx_en;
 	int hwts_rx_en;
 	int layer;
@@ -285,7 +287,7 @@ static void phy2rxts(struct phy_rxts *p, struct rxts *rxts)
 	rxts->seqid = p->seqid;
 	rxts->msgtype = (p->msgtype >> 12) & 0xf;
 	rxts->hash = p->msgtype & 0x0fff;
-	rxts->tmo = jiffies + 2;
+	rxts->tmo = jiffies + SKB_TIMESTAMP_TIMEOUT;
 }
 
 static u64 phy2txts(struct phy_txts *p)
@@ -1115,7 +1117,7 @@ static int dp83640_probe(struct phy_device *phydev)
 		goto no_memory;
 
 	dp83640->phydev = phydev;
-	INIT_WORK(&dp83640->ts_work, rx_timestamp_work);
+	INIT_DELAYED_WORK(&dp83640->ts_work, rx_timestamp_work);
 
 	INIT_LIST_HEAD(&dp83640->rxts);
 	INIT_LIST_HEAD(&dp83640->rxpool);
@@ -1162,7 +1164,7 @@ static void dp83640_remove(struct phy_device *phydev)
 		return;
 
 	enable_status_frames(phydev, false);
-	cancel_work_sync(&dp83640->ts_work);
+	cancel_delayed_work_sync(&dp83640->ts_work);
 
 	skb_queue_purge(&dp83640->rx_queue);
 	skb_queue_purge(&dp83640->tx_queue);
@@ -1356,7 +1358,7 @@ static int dp83640_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
 static void rx_timestamp_work(struct work_struct *work)
 {
 	struct dp83640_private *dp83640 =
-		container_of(work, struct dp83640_private, ts_work);
+		container_of(work, struct dp83640_private, ts_work.work);
 	struct sk_buff *skb;
 
 	/* Deliver expired packets. */
@@ -1373,7 +1375,7 @@ static void rx_timestamp_work(struct work_struct *work)
 	}
 
 	if (!skb_queue_empty(&dp83640->rx_queue))
-		schedule_work(&dp83640->ts_work);
+		schedule_delayed_work(&dp83640->ts_work, SKB_TIMESTAMP_TIMEOUT);
 }
 
 static bool dp83640_rxtstamp(struct phy_device *phydev,
@@ -1412,9 +1414,11 @@ static bool dp83640_rxtstamp(struct phy_device *phydev,
 
 	if (!shhwtstamps) {
 		skb_info->ptp_type = type;
-		skb_info->tmo = jiffies + 2;
+		skb_info->tmo = jiffies + SKB_TIMESTAMP_TIMEOUT;
 		skb_queue_tail(&dp83640->rx_queue, skb);
-		schedule_work(&dp83640->ts_work);
+		schedule_delayed_work(&dp83640->ts_work, SKB_TIMESTAMP_TIMEOUT);
+	} else {
+		netif_rx_ni(skb);
 	}
 
 	return true;
-- 
2.5.0

[PATCH v2 net-next 3/5] dp83640: Prune rx timestamp list before reading from it

[Stefan Sørensen]

The list of rx timestamps are currently only pruned of old entries when a
new entry is inserted. If no new entries are added, old timestamps may
survive beyond their lifetime, possible causing them to be attached to
packets with the same sequence number after a rollover.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 69a2f56..39ac633 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1398,6 +1398,7 @@ static bool dp83640_rxtstamp(struct phy_device *phydev,
 		return false;
 
 	spin_lock_irqsave(&dp83640->rx_lock, flags);
+	prune_rx_ts(dp83640);
 	list_for_each_safe(this, next, &dp83640->rxts) {
 		rxts = list_entry(this, struct rxts, list);
 		if (match(skb, type, rxts)) {
-- 
2.5.0

[PATCH v2 net-next 4/5] ptp: Change ptp_class to a proper bitmask

[Stefan Sørensen]

Change the definition of PTP_CLASS_L2 to not have any bits overlapping with
the other defined protocol values, allowing the PTP_CLASS_* definitions to
be for simple filtering on packet type.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 include/linux/ptp_classify.h |  7 ++++---
 net/core/ptp_classifier.c    | 16 ++++++++--------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
index 159c987..a079656 100644
--- a/include/linux/ptp_classify.h
+++ b/include/linux/ptp_classify.h
@@ -32,9 +32,9 @@
 #define PTP_CLASS_VMASK 0x0f /* max protocol version is 15 */
 #define PTP_CLASS_IPV4  0x10 /* event in an IPV4 UDP packet */
 #define PTP_CLASS_IPV6  0x20 /* event in an IPV6 UDP packet */
-#define PTP_CLASS_L2    0x30 /* event in a L2 packet */
-#define PTP_CLASS_PMASK 0x30 /* mask for the packet type field */
-#define PTP_CLASS_VLAN  0x40 /* event in a VLAN tagged packet */
+#define PTP_CLASS_L2    0x40 /* event in a L2 packet */
+#define PTP_CLASS_PMASK	0x70 /* mask for the packet type field */
+#define PTP_CLASS_VLAN	0x80 /* event in a VLAN tagged packet */
 
 #define PTP_CLASS_V1_IPV4 (PTP_CLASS_V1 | PTP_CLASS_IPV4)
 #define PTP_CLASS_V1_IPV6 (PTP_CLASS_V1 | PTP_CLASS_IPV6) /* probably DNE */
@@ -42,6 +42,7 @@
 #define PTP_CLASS_V2_IPV6 (PTP_CLASS_V2 | PTP_CLASS_IPV6)
 #define PTP_CLASS_V2_L2   (PTP_CLASS_V2 | PTP_CLASS_L2)
 #define PTP_CLASS_V2_VLAN (PTP_CLASS_V2 | PTP_CLASS_VLAN)
+#define PTP_CLASS_L4      (PTP_CLASS_IPV4 | PTP_CLASS_IPV6)
 
 #define PTP_EV_PORT 319
 #define PTP_GEN_BIT 0x08 /* indicates general message, if set in message type */
diff --git a/net/core/ptp_classifier.c b/net/core/ptp_classifier.c
index 4eab4a9..703cf76 100644
--- a/net/core/ptp_classifier.c
+++ b/net/core/ptp_classifier.c
@@ -58,7 +58,7 @@
  *   jneq #0x0, drop_ieee1588      ; for PTP_GEN_BIT and drop these
  *   ldh [18]                      ; reload payload
  *   and #0xf                      ; mask PTP_CLASS_VMASK
- *   or #0x70                      ; PTP_CLASS_VLAN|PTP_CLASS_L2
+ *   or #0xc0                      ; PTP_CLASS_VLAN|PTP_CLASS_L2
  *   ret a                         ; return PTP class
  *
  * ; PTP over UDP over IPv4 over 802.1Q over Ethernet
@@ -73,7 +73,7 @@
  *   jneq #319, drop_8021q_ipv4    ; is port PTP_EV_PORT ?
  *   ldh [x + 26]                  ; load payload
  *   and #0xf                      ; mask PTP_CLASS_VMASK
- *   or #0x50                      ; PTP_CLASS_VLAN|PTP_CLASS_IPV4
+ *   or #0x90                      ; PTP_CLASS_VLAN|PTP_CLASS_IPV4
  *   ret a                         ; return PTP class
  *   drop_8021q_ipv4: ret #0x0     ; PTP_CLASS_NONE
  *
@@ -86,7 +86,7 @@
  *   jneq #319, drop_8021q_ipv6          ; is port PTP_EV_PORT ?
  *   ldh [66]                      ; load payload
  *   and #0xf                      ; mask PTP_CLASS_VMASK
- *   or #0x60                      ; PTP_CLASS_VLAN|PTP_CLASS_IPV6
+ *   or #0xa0                      ; PTP_CLASS_VLAN|PTP_CLASS_IPV6
  *   ret a                         ; return PTP class
  *   drop_8021q_ipv6: ret #0x0     ; PTP_CLASS_NONE
  *
@@ -98,7 +98,7 @@
  *   jneq #0x0, drop_ieee1588      ; for PTP_GEN_BIT and drop these
  *   ldh [14]                      ; reload payload
  *   and #0xf                      ; mask PTP_CLASS_VMASK
- *   or #0x30                      ; PTP_CLASS_L2
+ *   or #0x40                      ; PTP_CLASS_L2
  *   ret a                         ; return PTP class
  *   drop_ieee1588: ret #0x0       ; PTP_CLASS_NONE
  */
@@ -150,7 +150,7 @@ void __init ptp_classifier_init(void)
 		{ 0x15,  0, 35, 0x00000000 },
 		{ 0x28,  0,  0, 0x00000012 },
 		{ 0x54,  0,  0, 0x0000000f },
-		{ 0x44,  0,  0, 0x00000070 },
+		{ 0x44,  0,  0, 0x000000c0 },
 		{ 0x16,  0,  0, 0x00000000 },
 		{ 0x15,  0, 12, 0x00000800 },
 		{ 0x30,  0,  0, 0x0000001b },
@@ -162,7 +162,7 @@ void __init ptp_classifier_init(void)
 		{ 0x15,  0,  4, 0x0000013f },
 		{ 0x48,  0,  0, 0x0000001a },
 		{ 0x54,  0,  0, 0x0000000f },
-		{ 0x44,  0,  0, 0x00000050 },
+		{ 0x44,  0,  0, 0x00000090 },
 		{ 0x16,  0,  0, 0x00000000 },
 		{ 0x06,  0,  0, 0x00000000 },
 		{ 0x15,  0,  8, 0x000086dd },
@@ -172,7 +172,7 @@ void __init ptp_classifier_init(void)
 		{ 0x15,  0,  4, 0x0000013f },
 		{ 0x28,  0,  0, 0x00000042 },
 		{ 0x54,  0,  0, 0x0000000f },
-		{ 0x44,  0,  0, 0x00000060 },
+		{ 0x44,  0,  0, 0x000000a0 },
 		{ 0x16,  0,  0, 0x00000000 },
 		{ 0x06,  0,  0, 0x00000000 },
 		{ 0x15,  0,  7, 0x000088f7 },
@@ -181,7 +181,7 @@ void __init ptp_classifier_init(void)
 		{ 0x15,  0,  4, 0x00000000 },
 		{ 0x28,  0,  0, 0x0000000e },
 		{ 0x54,  0,  0, 0x0000000f },
-		{ 0x44,  0,  0, 0x00000030 },
+		{ 0x44,  0,  0, 0x00000040 },
 		{ 0x16,  0,  0, 0x00000000 },
 		{ 0x06,  0,  0, 0x00000000 },
 	};
-- 
2.5.0

[PATCH v2 net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled.

[Stefan Sørensen]

In the packet timestamping function, check that the ptp version and
protocol of the packet matches what we have configured the hardware to
actually generate timestamps for, before looking/waiting for a timestamp.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 drivers/net/phy/dp83640.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 39ac633..47b7117 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -37,8 +37,6 @@
 
 #define DP83640_PHY_ID	0x20005ce1
 #define PAGESEL		0x13
-#define LAYER4		0x02
-#define LAYER2		0x01
 #define MAX_RXTS	64
 #define N_EXT_TS	6
 #define N_PER_OUT	7
@@ -1296,29 +1294,29 @@ static int dp83640_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
 	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
 		dp83640->hwts_rx_en = 1;
-		dp83640->layer = LAYER4;
-		dp83640->version = 1;
+		dp83640->layer = PTP_CLASS_L4;
+		dp83640->version = PTP_CLASS_V1;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
 		dp83640->hwts_rx_en = 1;
-		dp83640->layer = LAYER4;
-		dp83640->version = 2;
+		dp83640->layer = PTP_CLASS_L4;
+		dp83640->version = PTP_CLASS_V2;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
 		dp83640->hwts_rx_en = 1;
-		dp83640->layer = LAYER2;
-		dp83640->version = 2;
+		dp83640->layer = PTP_CLASS_L2;
+		dp83640->version = PTP_CLASS_V2;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
 		dp83640->hwts_rx_en = 1;
-		dp83640->layer = LAYER4|LAYER2;
-		dp83640->version = 2;
+		dp83640->layer = PTP_CLASS_L4 | PTP_CLASS_L2;
+		dp83640->version = PTP_CLASS_V2;
 		break;
 	default:
 		return -ERANGE;
@@ -1327,11 +1325,11 @@ static int dp83640_hwtstamp(struct phy_device *phydev, struct ifreq *ifr)
 	txcfg0 = (dp83640->version & TX_PTP_VER_MASK) << TX_PTP_VER_SHIFT;
 	rxcfg0 = (dp83640->version & TX_PTP_VER_MASK) << TX_PTP_VER_SHIFT;
 
-	if (dp83640->layer & LAYER2) {
+	if (dp83640->layer & PTP_CLASS_L2) {
 		txcfg0 |= TX_L2_EN;
 		rxcfg0 |= RX_L2_EN;
 	}
-	if (dp83640->layer & LAYER4) {
+	if (dp83640->layer & PTP_CLASS_L4) {
 		txcfg0 |= TX_IPV6_EN | TX_IPV4_EN;
 		rxcfg0 |= RX_IPV6_EN | RX_IPV4_EN;
 	}
@@ -1397,6 +1395,9 @@ static bool dp83640_rxtstamp(struct phy_device *phydev,
 	if (!dp83640->hwts_rx_en)
 		return false;
 
+	if ((type & dp83640->version) == 0 || (type & dp83640->layer) == 0)
+		return false;
+
 	spin_lock_irqsave(&dp83640->rx_lock, flags);
 	prune_rx_ts(dp83640);
 	list_for_each_safe(this, next, &dp83640->rxts) {
-- 
2.5.0

Re: [PATCH v2 net-next 0/5] dp83640 driver fixes

[Richard Cochran]

On Tue, Nov 03, 2015 at 09:34:03AM +0100, Stefan Sørensen wrote:
> This series fixes a number of minor bugs in the dp83640 driver.

For the series,

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

Re: [PATCH v2 net-next 0/5] dp83640 driver fixes

[David Miller]

From: Stefan Sørensen <stefan.sorensen@spectralink.com>
Date: Tue, 3 Nov 2015 09:34:03 +0100

> This series fixes a number of minor bugs in the dp83640 driver.

Series applied, thanks.