Re: [PATCH net-next v1 5/6] ptp: Support late timestamp determination
From: Richard Cochran <richardcochran@gmail.com>
Date: 2022-03-24 14:01:25
On Tue, Mar 22, 2022 at 10:07:21PM +0100, Gerhard Engleder wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c index 54b9f54ac0b2..b7a8cf27c349 100644 --- a/drivers/ptp/ptp_clock.c +++ b/drivers/ptp/ptp_clock.c@@ -450,6 +450,33 @@ void ptp_cancel_worker_sync(struct ptp_clock *ptp) } EXPORT_SYMBOL(ptp_cancel_worker_sync); +ktime_t ptp_get_timestamp(int index, + const struct skb_shared_hwtstamps *hwtstamps, + bool cycles) +{ + char name[PTP_CLOCK_NAME_LEN] = ""; + struct ptp_clock *ptp; + struct device *dev; + ktime_t ts; + + snprintf(name, PTP_CLOCK_NAME_LEN, "ptp%d", index); + dev = class_find_device_by_name(ptp_class, name);
This seems expensive for every single Rx frame in a busy PTP network. Can't this be cached in the socket?
quoted hunk ↗ jump to hunk
+ if (!dev) + return 0; + + ptp = dev_get_drvdata(dev); + + if (ptp->info->gettstamp) + ts = ptp->info->gettstamp(ptp->info, hwtstamps, cycles); + else + ts = hwtstamps->hwtstamp; + + put_device(dev); + + return ts; +} +EXPORT_SYMBOL(ptp_get_timestamp); + /* module operations */ static void __exit ptp_exit(void)diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h index cc6a7b2e267d..f4f0d8a880c6 100644 --- a/include/linux/ptp_clock_kernel.h +++ b/include/linux/ptp_clock_kernel.h@@ -133,6 +133,16 @@ struct ptp_system_timestamp { * parameter cts: Contains timestamp (device,system) pair, * where system time is realtime and monotonic. * + * @gettstamp: Get hardware timestamp based on normal/adjustable time or free + * running time. If @getcycles64 or @getcyclesx64 are supported, + * then this method is required to provide timestamps based on the + * free running time. This method will be called if + * SKBTX_HW_TSTAMP_PHC is set by the driver. + * parameter hwtstamps: skb_shared_hwtstamps structure pointer. + * parameter cycles: If false, then hardware timestamp based on + * normal/adjustable time is requested. If true, then hardware + * timestamp based on free running time is requested. + * * @enable: Request driver to enable or disable an ancillary feature. * parameter request: Desired resource to enable or disable. * parameter on: Caller passes one to enable or zero to disable.@@ -185,6 +195,9 @@ struct ptp_clock_info { struct ptp_system_timestamp *sts); int (*getcrosscycles)(struct ptp_clock_info *ptp, struct system_device_crosststamp *cts); + ktime_t (*gettstamp)(struct ptp_clock_info *ptp, + const struct skb_shared_hwtstamps *hwtstamps, + bool cycles); int (*enable)(struct ptp_clock_info *ptp, struct ptp_clock_request *request, int on); int (*verify)(struct ptp_clock_info *ptp, unsigned int pin,@@ -364,6 +377,19 @@ static inline void ptp_cancel_worker_sync(struct ptp_clock *ptp) * a loadable module. */ +/** + * ptp_get_timestamp() - get timestamp of ptp clock + * + * @index: phc index of ptp pclock. + * @hwtstamps: skb_shared_hwtstamps structure pointer. + * @cycles: true for timestamp based on cycles. + * + * Returns timestamp, or 0 on error. + */ +ktime_t ptp_get_timestamp(int index, + const struct skb_shared_hwtstamps *hwtstamps, + bool cycles); + /** * ptp_get_vclocks_index() - get all vclocks index on pclock, and * caller is responsible to free memory@@ -386,6 +412,10 @@ int ptp_get_vclocks_index(int pclock_index, int **vclock_index); */ ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp, int vclock_index); #else +static inline ktime_t ptp_get_timestamp(int index, + const struct skb_shared_hwtstamps *hwtstamps, + bool cycles); +{ return 0; } static inline int ptp_get_vclocks_index(int pclock_index, int **vclock_index) { return 0; } static inline ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp,diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index f494ddbfc826..38929c113953 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h@@ -564,7 +564,10 @@ static inline bool skb_frag_must_loop(struct page *p) * &skb_shared_info. Use skb_hwtstamps() to get a pointer. */ struct skb_shared_hwtstamps { - ktime_t hwtstamp; + union { + ktime_t hwtstamp; + void *phc_data;
needs kdoc update
quoted hunk ↗ jump to hunk
+ }; }; /* Definitions for tx_flags in struct skb_shared_info */@@ -581,6 +584,9 @@ enum { /* generate hardware time stamp based on cycles if supported */ SKBTX_HW_TSTAMP_USE_CYCLES = 1 << 3, + /* call PHC to get actual hardware time stamp */ + SKBTX_HW_TSTAMP_PHC = 1 << 3, + /* generate wifi status information (where possible) */ SKBTX_WIFI_STATUS = 1 << 4,diff --git a/net/socket.c b/net/socket.c index 2e932c058002..fe765d559086 100644 --- a/net/socket.c +++ b/net/socket.c@@ -804,21 +804,17 @@ static bool skb_is_swtx_tstamp(const struct sk_buff *skb, int false_tstamp) return skb->tstamp && !false_tstamp && skb_is_err_queue(skb); } -static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb) +static void put_ts_pktinfo(struct msghdr *msg, struct sk_buff *skb, + int if_index) { struct scm_ts_pktinfo ts_pktinfo; - struct net_device *orig_dev; if (!skb_mac_header_was_set(skb)) return; memset(&ts_pktinfo, 0, sizeof(ts_pktinfo)); - rcu_read_lock(); - orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); - if (orig_dev) - ts_pktinfo.if_index = orig_dev->ifindex; - rcu_read_unlock(); + ts_pktinfo.if_index = if_index; ts_pktinfo.pkt_length = skb->len - skb_mac_offset(skb); put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_PKTINFO,@@ -838,6 +834,9 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, int empty = 1, false_tstamp = 0; struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb); + struct net_device *orig_dev; + int if_index = 0; + int phc_index = -1; ktime_t hwtstamp; /* Race occurred between timestamp enabling and packet@@ -886,18 +885,32 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk, if (shhwtstamps && (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) && !skb_is_swtx_tstamp(skb, false_tstamp)) { - if (sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC) - hwtstamp = ptp_convert_timestamp(&shhwtstamps->hwtstamp, - sk->sk_bind_phc); + rcu_read_lock(); + orig_dev = dev_get_by_napi_id(skb_napi_id(skb)); + if (orig_dev) { + if_index = orig_dev->ifindex; + if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_PHC) + phc_index = ethtool_get_phc(orig_dev);
again, this is something that can be cached, no?
+ }
+ rcu_read_unlock();
+
+ if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_PHC) &&
+ (phc_index != -1))
+ hwtstamp = ptp_get_timestamp(phc_index, shhwtstamps,
+ sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC);
else
hwtstamp = shhwtstamps->hwtstamp;
+ if (sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC)
+ hwtstamp = ptp_convert_timestamp(&hwtstamp,
+ sk->sk_bind_phc);
+
if (ktime_to_timespec64_cond(hwtstamp, tss.ts + 2)) {
empty = 0;
if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_PKTINFO) &&
!skb_is_err_queue(skb))
- put_ts_pktinfo(msg, skb);
+ put_ts_pktinfo(msg, skb, if_index);
}
}
if (!empty) {
--
2.20.1Thanks, Richard