* [PATCH net-next 0/5] dp83640 driver fixes
@ 2015-10-30 12:13 Stefan Sørensen
2015-10-30 12:14 ` [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching Stefan Sørensen
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Stefan Sørensen @ 2015-10-30 12:13 UTC (permalink / raw)
To: davem; +Cc: netdev, richardcochran, 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 | 60 +++++++++++++++++++++++++++-----------------
include/linux/ptp_classify.h | 7 +++---
net/core/ptp_classifier.c | 16 ++++++------
3 files changed, 49 insertions(+), 34 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen @ 2015-10-30 12:14 ` Stefan Sørensen 2015-10-30 20:32 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 2/5] dp83640: Delay scheduled work Stefan Sørensen ` (4 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Stefan Sørensen @ 2015-10-30 12:14 UTC (permalink / raw) To: davem; +Cc: netdev, richardcochran, Stefan Sørensen Only using the message type and sequence id for matching timestamps with packets is error prone, particularly if packets can be reordered. 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 | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c index 185b03c..9534478 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> @@ -789,7 +790,7 @@ static int decode_evnt(struct dp83640_private *dp83640, 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 +820,18 @@ 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(10, data + offset + 20) >> 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching 2015-10-30 12:14 ` [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching Stefan Sørensen @ 2015-10-30 20:32 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-10-30 20:32 UTC (permalink / raw) To: Stefan Sørensen; +Cc: davem, netdev On Fri, Oct 30, 2015 at 01:14:00PM +0100, Stefan Sørensen wrote: > Only using the message type and sequence id for matching timestamps with > packets is error prone, particularly if packets can be reordered. Fix by > extending the check to include the hash of bytes 20-29 (source id in PTPv2) > that is provided with the timestamps. The example of reordered packets is bogus, since the sequence numbers are not affected. The one case that benefits from the hash check is when the port is in the master role and multiple clients send Delay_Req messages all with the same sequence number by chance. > @@ -819,11 +820,18 @@ 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(10, data + offset + 20) >> 20; This could use a macro instead of magic 20. Thanks, Richard > + 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 2/5] dp83640: Delay scheduled work. 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen 2015-10-30 12:14 ` [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching Stefan Sørensen @ 2015-10-30 12:14 ` Stefan Sørensen 2015-10-30 20:37 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it Stefan Sørensen ` (3 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Stefan Sørensen @ 2015-10-30 12:14 UTC (permalink / raw) To: davem; +Cc: netdev, richardcochran, 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 9534478..f3e812b 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) @@ -1111,7 +1113,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); @@ -1158,7 +1160,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); @@ -1352,7 +1354,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. */ @@ -1369,7 +1371,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, @@ -1408,9 +1410,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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 2/5] dp83640: Delay scheduled work. 2015-10-30 12:14 ` [PATCH net-next 2/5] dp83640: Delay scheduled work Stefan Sørensen @ 2015-10-30 20:37 ` Richard Cochran 2015-11-02 7:23 ` Sørensen, Stefan 0 siblings, 1 reply; 15+ messages in thread From: Richard Cochran @ 2015-10-30 20:37 UTC (permalink / raw) To: Stefan Sørensen; +Cc: davem, netdev On Fri, Oct 30, 2015 at 01:14:01PM +0100, Stefan Sørensen wrote: > 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. NAK. We want to have the time stamp ASAP, because the delay between the transmission of the Sync and its application in the servo degrades or even spoils synchronization. Think of an ARM SoC with HZ=100. Two jiffies are 20 milliseconds. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 2/5] dp83640: Delay scheduled work. 2015-10-30 20:37 ` Richard Cochran @ 2015-11-02 7:23 ` Sørensen, Stefan 2015-11-02 9:59 ` Richard Cochran 0 siblings, 1 reply; 15+ messages in thread From: Sørensen, Stefan @ 2015-11-02 7:23 UTC (permalink / raw) To: richardcochran; +Cc: davem, netdev On Fri, 2015-10-30 at 21:37 +0100, Richard Cochran wrote: > > NAK. We want to have the time stamp ASAP, because the delay between > the transmission of the Sync and its application in the servo > degrades or even spoils synchronization. Think of an ARM SoC with > HZ=100. Two jiffies are 20 milliseconds. The workqueue is only used for delivering packets where the timestamp was lost - if a timestamp is received, the packet is delivered immediately. This patch may introduce a slight delay of the packets where the timestamp never arrives, by these packets are properly not very useful to the application anyway. Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 2/5] dp83640: Delay scheduled work. 2015-11-02 7:23 ` Sørensen, Stefan @ 2015-11-02 9:59 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-11-02 9:59 UTC (permalink / raw) To: Sørensen, Stefan; +Cc: davem, netdev On Mon, Nov 02, 2015 at 07:23:12AM +0000, Sørensen, Stefan wrote: > The workqueue is only used for delivering packets where the timestamp > was lost - if a timestamp is received, the packet is delivered > immediately. This patch may introduce a slight delay of the packets > where the timestamp never arrives, by these packets are properly not > very useful to the application anyway. Ok, thanks for the explanation. I mixed this up with another out of tree driver that I was hacking recently. Acked-by: Richard Cochran <richardcochran@gmail.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen 2015-10-30 12:14 ` [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching Stefan Sørensen 2015-10-30 12:14 ` [PATCH net-next 2/5] dp83640: Delay scheduled work Stefan Sørensen @ 2015-10-30 12:14 ` Stefan Sørensen 2015-10-30 20:38 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask Stefan Sørensen ` (2 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Stefan Sørensen @ 2015-10-30 12:14 UTC (permalink / raw) To: davem; +Cc: netdev, richardcochran, 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 f3e812b..c1f70b7 100644 --- a/drivers/net/phy/dp83640.c +++ b/drivers/net/phy/dp83640.c @@ -1394,6 +1394,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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it 2015-10-30 12:14 ` [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it Stefan Sørensen @ 2015-10-30 20:38 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-10-30 20:38 UTC (permalink / raw) To: Stefan Sørensen; +Cc: davem, netdev On Fri, Oct 30, 2015 at 01:14:02PM +0100, Stefan Sørensen wrote: > 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> Acked-by: Richard Cochran <richardcochran@gmail.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen ` (2 preceding siblings ...) 2015-10-30 12:14 ` [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it Stefan Sørensen @ 2015-10-30 12:14 ` Stefan Sørensen 2015-10-30 20:39 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled Stefan Sørensen 2015-11-02 20:13 ` [PATCH net-next 0/5] dp83640 driver fixes David Miller 5 siblings, 1 reply; 15+ messages in thread From: Stefan Sørensen @ 2015-10-30 12:14 UTC (permalink / raw) To: davem; +Cc: netdev, richardcochran, 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask 2015-10-30 12:14 ` [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask Stefan Sørensen @ 2015-10-30 20:39 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-10-30 20:39 UTC (permalink / raw) To: Stefan Sørensen; +Cc: davem, netdev On Fri, Oct 30, 2015 at 01:14:03PM +0100, Stefan Sørensen wrote: > 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> Acked-by: Richard Cochran <richardcochran@gmail.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled. 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen ` (3 preceding siblings ...) 2015-10-30 12:14 ` [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask Stefan Sørensen @ 2015-10-30 12:14 ` Stefan Sørensen 2015-10-30 20:40 ` Richard Cochran 2015-11-02 20:13 ` [PATCH net-next 0/5] dp83640 driver fixes David Miller 5 siblings, 1 reply; 15+ messages in thread From: Stefan Sørensen @ 2015-10-30 12:14 UTC (permalink / raw) To: davem; +Cc: netdev, richardcochran, 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 c1f70b7..30bfeb1 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 @@ -1292,29 +1290,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; @@ -1323,11 +1321,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; } @@ -1393,6 +1391,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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled. 2015-10-30 12:14 ` [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled Stefan Sørensen @ 2015-10-30 20:40 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-10-30 20:40 UTC (permalink / raw) To: Stefan Sørensen; +Cc: davem, netdev On Fri, Oct 30, 2015 at 01:14:04PM +0100, Stefan Sørensen wrote: > 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> Acked-by: Richard Cochran <richardcochran@gmail.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/5] dp83640 driver fixes 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen ` (4 preceding siblings ...) 2015-10-30 12:14 ` [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled Stefan Sørensen @ 2015-11-02 20:13 ` David Miller 2015-11-03 8:42 ` Richard Cochran 5 siblings, 1 reply; 15+ messages in thread From: David Miller @ 2015-11-02 20:13 UTC (permalink / raw) To: stefan.sorensen; +Cc: netdev, richardcochran From: Stefan Sørensen <stefan.sorensen@spectralink.com> Date: Fri, 30 Oct 2015 13:13:59 +0100 > This series fixes a number of minor bugs in the dp83640 driver. Looks like Richard wants changes to patch #1. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/5] dp83640 driver fixes 2015-11-02 20:13 ` [PATCH net-next 0/5] dp83640 driver fixes David Miller @ 2015-11-03 8:42 ` Richard Cochran 0 siblings, 0 replies; 15+ messages in thread From: Richard Cochran @ 2015-11-03 8:42 UTC (permalink / raw) To: David Miller; +Cc: stefan.sorensen, netdev On Mon, Nov 02, 2015 at 03:13:16PM -0500, David Miller wrote: > Looks like Richard wants changes to patch #1. Yes, please update the description and use a macro for the offset. Other than that, the series looks ok to me. Thanks, Richard ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-11-03 8:42 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-30 12:13 [PATCH net-next 0/5] dp83640 driver fixes Stefan Sørensen 2015-10-30 12:14 ` [PATCH net-next 1/5] dp83640: Include hash in timestamp/packet matching Stefan Sørensen 2015-10-30 20:32 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 2/5] dp83640: Delay scheduled work Stefan Sørensen 2015-10-30 20:37 ` Richard Cochran 2015-11-02 7:23 ` Sørensen, Stefan 2015-11-02 9:59 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 3/5] dp83640: Prune rx timestamp list before reading from it Stefan Sørensen 2015-10-30 20:38 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 4/5] ptp: Change ptp_class to a proper bitmask Stefan Sørensen 2015-10-30 20:39 ` Richard Cochran 2015-10-30 12:14 ` [PATCH net-next 5/5] dp83640: Only wait for timestamps for packets with timestamping enabled Stefan Sørensen 2015-10-30 20:40 ` Richard Cochran 2015-11-02 20:13 ` [PATCH net-next 0/5] dp83640 driver fixes David Miller 2015-11-03 8:42 ` Richard Cochran
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).