[PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Muhammad Husaini Zulkifli]

The HW TX timestamps created by the NIC via socket options can be
requested using the current network timestamps generation capability of
SOF_TIMESTAMPING_TX_HARDWARE. The most common users of this socket flag
is PTP, however other packet applications that require tx timestamps might
also ask for it.

The problem is that, when there is a lot of traffic, there is a high chance
that the timestamps for a PTP packet will be lost if both PTP and Non-PTP
packets use the same SOF TIMESTAMPING TX HARDWARE causing the tx timeout.

DMA timestamps through socket options are not currently available to
the user. Because if the user wants to, they can configure the hwtstamp
config option to use the new introduced DMA Time Stamp flag through the
setsockopt().

With these additional socket options, users can continue to utilise
HW timestamps for PTP while specifying non-PTP packets to use DMA
timestamps if the NIC can support them.

This patch series also add a new HWTSTAMP_FILTER_DMA_TIMESTAMP receive
filters. This filter can be configured for devices that support/allow the
DMA timestamp retrieval on receive side.

Any socket application can be use to verify this.
TSN Ref SW application is been used for testing by changing as below:

	int timestamping_flags = SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH;

	strncpy(hwtstamp.ifr_name, opt->ifname, sizeof(hwtstamp.ifr_name)-1);
	hwtstamp.ifr_data = (void *)&hwconfig;
	hwconfig.tx_type = HWTSTAMP_TX_ON;
	hwconfig.flags = HWTSTAMP_FLAG_DMA_TIMESTAMP;
	hwconfig.rx_filter = HWTSTAMP_FILTER_ALL;

	if (ioctl(sock, SIOCSHWTSTAMP, &hwtstamp) < 0) {
		fprintf(stderr, "%s: %s\n", "ioctl", strerror(errno));
		exit(1);
	}

	if (setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPING, &timestamping_flags,
			sizeof(timestamping_flags)) < 0)
		exit_with_error("setsockopt SO_TIMESTAMPING");

v1 -> v2:
	- Move to the end for the new enum.
	- Add new HWTSTAMP_FILTER_DMA_TIMESTAMP receive filters.

Muhammad Husaini Zulkifli (4):
  ethtool: Add new hwtstamp flag
  net-timestamp: Increase the size of tsflags
  net: sock: extend SO_TIMESTAMPING for DMA Fetch
  ethtool: Add support for HWTSTAMP_FILTER_DMA_TIMESTAMP

Vinicius Costa Gomes (1):
  igc: Add support for DMA timestamp for non-PTP packets

 drivers/net/ethernet/intel/igc/igc.h         | 10 +++
 drivers/net/ethernet/intel/igc/igc_base.h    |  2 +-
 drivers/net/ethernet/intel/igc/igc_defines.h |  2 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  5 +-
 drivers/net/ethernet/intel/igc/igc_main.c    | 24 ++++--
 drivers/net/ethernet/intel/igc/igc_ptp.c     | 84 ++++++++++++++++++++
 include/linux/skbuff.h                       |  3 +
 include/net/sock.h                           | 12 +--
 include/uapi/linux/ethtool.h                 |  3 +
 include/uapi/linux/ethtool_netlink.h         |  1 +
 include/uapi/linux/net_tstamp.h              | 14 +++-
 net/core/dev_ioctl.c                         |  1 +
 net/ethtool/common.c                         |  8 ++
 net/ethtool/common.h                         |  2 +
 net/ethtool/strset.c                         |  5 ++
 net/ethtool/tsinfo.c                         | 17 ++++
 net/socket.c                                 |  5 +-
 17 files changed, 181 insertions(+), 17 deletions(-)

--
2.17.1

[PATCH v2 1/5] ethtool: Add new hwtstamp flag

[Muhammad Husaini Zulkifli]

This add patch add a new DMA Time Stamp flag. User can configure
hwtstamp_config with this flag if they want to use DMA time stamp.

Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 include/uapi/linux/ethtool.h         |  3 +++
 include/uapi/linux/ethtool_netlink.h |  1 +
 include/uapi/linux/net_tstamp.h      |  5 ++++-
 net/ethtool/common.c                 |  6 ++++++
 net/ethtool/common.h                 |  2 ++
 net/ethtool/strset.c                 |  5 +++++
 net/ethtool/tsinfo.c                 | 17 +++++++++++++++++
 7 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index dc2aa3d75b39..a3c60e2bde36 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -683,6 +683,7 @@ enum ethtool_link_ext_substate_module {
  * @ETH_SS_STATS_ETH_MAC: names of IEEE 802.3 MAC statistics
  * @ETH_SS_STATS_ETH_CTRL: names of IEEE 802.3 MAC Control statistics
  * @ETH_SS_STATS_RMON: names of RMON statistics
+ * @ETH_SS_HWTSTAMP_FLAG: timestamping flags
  *
  * @ETH_SS_COUNT: number of defined string sets
  */
@@ -708,6 +709,7 @@ enum ethtool_stringset {
 	ETH_SS_STATS_ETH_MAC,
 	ETH_SS_STATS_ETH_CTRL,
 	ETH_SS_STATS_RMON,
+	ETH_SS_HWTSTAMP_FLAG,
 
 	/* add new constants above here */
 	ETH_SS_COUNT
@@ -1416,6 +1418,7 @@ struct ethtool_ts_info {
 	__u32	tx_reserved[3];
 	__u32	rx_filters;
 	__u32	rx_reserved[3];
+	__u32	flag;
 };
 
 /*
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index bb57084ac524..4b7bd7554a3b 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -458,6 +458,7 @@ enum {
 	ETHTOOL_A_TSINFO_TX_TYPES,			/* bitset */
 	ETHTOOL_A_TSINFO_RX_FILTERS,			/* bitset */
 	ETHTOOL_A_TSINFO_PHC_INDEX,			/* u32 */
+	ETHTOOL_A_TSINFO_FLAG,				/* bitset */
 
 	/* add new constants above here */
 	__ETHTOOL_A_TSINFO_CNT,
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 55501e5e7ac8..4966d5ca521f 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -89,7 +89,10 @@ enum hwtstamp_flags {
 	HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
 #define HWTSTAMP_FLAG_BONDED_PHC_INDEX	HWTSTAMP_FLAG_BONDED_PHC_INDEX
 
-	HWTSTAMP_FLAG_LAST = HWTSTAMP_FLAG_BONDED_PHC_INDEX,
+	HWTSTAMP_FLAG_DMA_TIMESTAMP = (1<<1),
+#define HWTSTAMP_FLAG_DMA_TIMESTAMP	HWTSTAMP_FLAG_DMA_TIMESTAMP
+
+	HWTSTAMP_FLAG_LAST = HWTSTAMP_FLAG_DMA_TIMESTAMP,
 	HWTSTAMP_FLAG_MASK = (HWTSTAMP_FLAG_LAST - 1) | HWTSTAMP_FLAG_LAST
 };
 
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 566adf85e658..f2a178d162ef 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -406,6 +406,12 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = {
 };
 static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT);
 
+const char ts_flag_names[][ETH_GSTRING_LEN] = {
+	[const_ilog2(HWTSTAMP_FLAG_BONDED_PHC_INDEX)]	= "bonded-phc-index",
+	[const_ilog2(HWTSTAMP_FLAG_DMA_TIMESTAMP)]	= "dma-time-stamp",
+};
+static_assert(ARRAY_SIZE(ts_flag_names) == __HWTSTAMP_FLAG_CNT);
+
 const char ts_tx_type_names[][ETH_GSTRING_LEN] = {
 	[HWTSTAMP_TX_OFF]		= "off",
 	[HWTSTAMP_TX_ON]		= "on",
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index c1779657e074..0161e04d4de8 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -13,6 +13,7 @@
 	ETHTOOL_LINK_MODE_ ## speed ## base ## type ## _ ## duplex ## _BIT
 
 #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1)
+#define __HWTSTAMP_FLAG_CNT (const_ilog2(HWTSTAMP_FLAG_LAST) + 1)
 
 struct link_mode_info {
 	int				speed;
@@ -36,6 +37,7 @@ extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
 extern const char ts_tx_type_names[][ETH_GSTRING_LEN];
 extern const char ts_rx_filter_names[][ETH_GSTRING_LEN];
 extern const char udp_tunnel_type_names[][ETH_GSTRING_LEN];
+extern const char ts_flag_names[][ETH_GSTRING_LEN];
 
 int __ethtool_get_link(struct net_device *dev);
 
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 3f7de54d85fb..b3fb1b1c516c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -105,6 +105,11 @@ static const struct strset_info info_template[] = {
 		.count		= __ETHTOOL_A_STATS_RMON_CNT,
 		.strings	= stats_rmon_names,
 	},
+	[ETH_SS_HWTSTAMP_FLAG] = {
+		.per_dev	= false,
+		.count		= __HWTSTAMP_FLAG_CNT,
+		.strings	= ts_flag_names,
+	},
 };
 
 struct strset_req_info {
diff --git a/net/ethtool/tsinfo.c b/net/ethtool/tsinfo.c
index 63b5814bd460..af5acf7bf561 100644
--- a/net/ethtool/tsinfo.c
+++ b/net/ethtool/tsinfo.c
@@ -52,6 +52,7 @@ static int tsinfo_reply_size(const struct ethnl_req_info *req_base,
 	BUILD_BUG_ON(__SOF_TIMESTAMPING_CNT > 32);
 	BUILD_BUG_ON(__HWTSTAMP_TX_CNT > 32);
 	BUILD_BUG_ON(__HWTSTAMP_FILTER_CNT > 32);
+	BUILD_BUG_ON(__HWTSTAMP_FLAG_CNT > 32);
 
 	if (ts_info->so_timestamping) {
 		ret = ethnl_bitset32_size(&ts_info->so_timestamping, NULL,
@@ -79,6 +80,14 @@ static int tsinfo_reply_size(const struct ethnl_req_info *req_base,
 	}
 	if (ts_info->phc_index >= 0)
 		len += nla_total_size(sizeof(u32));	/* _TSINFO_PHC_INDEX */
+	if (ts_info->flag) {
+		ret = ethnl_bitset32_size(&ts_info->flag, NULL,
+					  __HWTSTAMP_FLAG_CNT,
+					  ts_flag_names, compact);
+		if (ret < 0)
+			return ret;
+		len += ret;	/* _TSINFO_FLAG */
+	}
 
 	return len;
 }
@@ -119,6 +128,14 @@ static int tsinfo_fill_reply(struct sk_buff *skb,
 	if (ts_info->phc_index >= 0 &&
 	    nla_put_u32(skb, ETHTOOL_A_TSINFO_PHC_INDEX, ts_info->phc_index))
 		return -EMSGSIZE;
+	if (ts_info->flag) {
+		ret = ethnl_put_bitset32(skb, ETHTOOL_A_TSINFO_FLAG,
+					 &ts_info->flag, NULL,
+					 __HWTSTAMP_FLAG_CNT,
+					 ts_flag_names, compact);
+		if (ret < 0)
+			return ret;
+	}
 
 	return 0;
 }
-- 
2.17.1

[PATCH v2 2/5] net-timestamp: Increase the size of tsflags

[Muhammad Husaini Zulkifli]

Increase the size of tsflags to support more SOF_TIMESTAMPING flags.
Current flag size can only support up to 16 flags only.

Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 include/net/sock.h | 12 ++++++------
 net/socket.c       |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 08038a385ef2..ad5a3d44ad25 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -503,7 +503,7 @@ struct sock {
 #if BITS_PER_LONG==32
 	seqlock_t		sk_stamp_seq;
 #endif
-	u16			sk_tsflags;
+	u32			sk_tsflags;
 	u8			sk_shutdown;
 	atomic_t		sk_tskey;
 	atomic_t		sk_zckey;
@@ -1892,7 +1892,7 @@ void sk_send_sigurg(struct sock *sk);
 struct sockcm_cookie {
 	u64 transmit_time;
 	u32 mark;
-	u16 tsflags;
+	u32 tsflags;
 };
 
 static inline void sockcm_init(struct sockcm_cookie *sockc,
@@ -2723,7 +2723,7 @@ static inline void sock_recv_cmsgs(struct msghdr *msg, struct sock *sk,
 		sock_write_timestamp(sk, 0);
 }
 
-void __sock_tx_timestamp(__u16 tsflags, __u8 *tx_flags);
+void __sock_tx_timestamp(__u32 tsflags, __u8 *tx_flags);
 
 /**
  * _sock_tx_timestamp - checks whether the outgoing packet is to be time stamped
@@ -2734,7 +2734,7 @@ void __sock_tx_timestamp(__u16 tsflags, __u8 *tx_flags);
  *
  * Note: callers should take care of initial ``*tx_flags`` value (usually 0)
  */
-static inline void _sock_tx_timestamp(struct sock *sk, __u16 tsflags,
+static inline void _sock_tx_timestamp(struct sock *sk, __u32 tsflags,
 				      __u8 *tx_flags, __u32 *tskey)
 {
 	if (unlikely(tsflags)) {
@@ -2747,13 +2747,13 @@ static inline void _sock_tx_timestamp(struct sock *sk, __u16 tsflags,
 		*tx_flags |= SKBTX_WIFI_STATUS;
 }
 
-static inline void sock_tx_timestamp(struct sock *sk, __u16 tsflags,
+static inline void sock_tx_timestamp(struct sock *sk, __u32 tsflags,
 				     __u8 *tx_flags)
 {
 	_sock_tx_timestamp(sk, tsflags, tx_flags, NULL);
 }
 
-static inline void skb_setup_tx_timestamp(struct sk_buff *skb, __u16 tsflags)
+static inline void skb_setup_tx_timestamp(struct sk_buff *skb, __u32 tsflags)
 {
 	_sock_tx_timestamp(skb->sk, tsflags, &skb_shinfo(skb)->tx_flags,
 			   &skb_shinfo(skb)->tskey);
diff --git a/net/socket.c b/net/socket.c
index 00da9ce3dba0..ab5d8973e719 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -679,7 +679,7 @@ void sock_release(struct socket *sock)
 }
 EXPORT_SYMBOL(sock_release);
 
-void __sock_tx_timestamp(__u16 tsflags, __u8 *tx_flags)
+void __sock_tx_timestamp(__u32 tsflags, __u8 *tx_flags)
 {
 	u8 flags = *tx_flags;
 
-- 
2.17.1

Re: [Intel-wired-lan] [PATCH v2 2/5] net-timestamp: Increase the size of tsflags

[Paul Menzel]

Dear Muhammad,


Thank you for the patch. For the summary/title you could be more 
specific with

 > net-timestamp: Double tsflags size to 32-bit for more flags


Kind regards,

Paul

RE: [Intel-wired-lan] [PATCH v2 2/5] net-timestamp: Increase the size of tsflags

[Zulkifli, Muhammad Husaini]

Hi Paul,

Thanks for review. 

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Tuesday, 18 October, 2022 1:58 PM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
> Cc: leon@kernel.org; netdev@vger.kernel.org; richardcochran@gmail.com;
> saeed@kernel.org; edumazet@google.com; gal@nvidia.com;
> kuba@kernel.org; michael.chan@broadcom.com; davem@davemloft.net;
> andy@greyhouse.net; intel-wired-lan@osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH v2 2/5] net-timestamp: Increase the
> size of tsflags
> 
> Dear Muhammad,
> 
> 
> Thank you for the patch. For the summary/title you could be more specific
> with
> 
>  > net-timestamp: Double tsflags size to 32-bit for more flags
> 

Sure. Will do

> 
> Kind regards,
> 
> Paul

[PATCH v2 3/5] net: sock: extend SO_TIMESTAMPING for DMA Fetch

[Muhammad Husaini Zulkifli]

This patch is to extend SO_TIMESTAMPING API to support the DMA Time
Stamp by adding a new flag SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH.
User space application can configure this flag into hwtstamp_config flag
if require to use the DMA Time Stamp for that socket application.

Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 include/linux/skbuff.h          | 3 +++
 include/uapi/linux/net_tstamp.h | 6 ++++--
 net/ethtool/common.c            | 1 +
 net/socket.c                    | 3 +++
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9fcf534f2d92..49f0ef60701a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -485,6 +485,9 @@ enum {
 
 	/* generate software time stamp when entering packet scheduling */
 	SKBTX_SCHED_TSTAMP = 1 << 6,
+
+	/* generate hardware DMA time stamp */
+	SKBTX_HW_DMA_TSTAMP = 1 << 7,
 };
 
 #define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP    | \
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 4966d5ca521f..c9e113cea883 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -31,8 +31,9 @@ enum {
 	SOF_TIMESTAMPING_OPT_PKTINFO = (1<<13),
 	SOF_TIMESTAMPING_OPT_TX_SWHW = (1<<14),
 	SOF_TIMESTAMPING_BIND_PHC = (1 << 15),
+	SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH = (1 << 16),
 
-	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_BIND_PHC,
+	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH,
 	SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
 				 SOF_TIMESTAMPING_LAST
 };
@@ -45,7 +46,8 @@ enum {
 #define SOF_TIMESTAMPING_TX_RECORD_MASK	(SOF_TIMESTAMPING_TX_HARDWARE | \
 					 SOF_TIMESTAMPING_TX_SOFTWARE | \
 					 SOF_TIMESTAMPING_TX_SCHED | \
-					 SOF_TIMESTAMPING_TX_ACK)
+					 SOF_TIMESTAMPING_TX_ACK | \
+					 SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH)
 
 /**
  * struct so_timestamping - SO_TIMESTAMPING parameter
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index f2a178d162ef..24f8e7f9b4a5 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -403,6 +403,7 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = {
 	[const_ilog2(SOF_TIMESTAMPING_OPT_PKTINFO)]  = "option-pktinfo",
 	[const_ilog2(SOF_TIMESTAMPING_OPT_TX_SWHW)]  = "option-tx-swhw",
 	[const_ilog2(SOF_TIMESTAMPING_BIND_PHC)]     = "bind-phc",
+	[const_ilog2(SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH)]	= "hardware-dma-transmit",
 };
 static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT);
 
diff --git a/net/socket.c b/net/socket.c
index ab5d8973e719..9e2175685f4e 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -695,6 +695,9 @@ void __sock_tx_timestamp(__u32 tsflags, __u8 *tx_flags)
 			flags |= SKBTX_HW_TSTAMP_USE_CYCLES;
 	}
 
+	if (tsflags & SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH)
+		flags |= SKBTX_HW_DMA_TSTAMP;
+
 	if (tsflags & SOF_TIMESTAMPING_TX_SOFTWARE)
 		flags |= SKBTX_SW_TSTAMP;
 
-- 
2.17.1

[PATCH v2 4/5] igc: Add support for DMA timestamp for non-PTP packets

[Muhammad Husaini Zulkifli]

From: Vinicius Costa Gomes <vinicius.gomes@intel.com>

For PTP traffic, timestamp is retrieved from TXSTMP register.
For all other packets, DMA time stamp field of the Transmit
Descriptor Write-back is used.

If the TXSTAMPO register is used for both PTP and non-PTP packets,
there is a significant possibility that the time stamp for a PTP packet
will be lost when there is a lot of traffic.

This patch introduce to use the DMA Time Stamp for non PTP packet to
solve the current issue. User application can add new SOF_TIMESTAMPING flag
SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH when configure the
hwtstamp_config for the socket option if require DMA Time Stamp.

Before:

ptp4l: rms    2 max    5 freq  -3404 +/-   3 delay     1 +/-   0
ptp4l: rms    3 max    6 freq  -3400 +/-   3 delay     1 +/-   0
ptp4l: rms    2 max    4 freq  -3400 +/-   3 delay     1 +/-   0
ptp4l: timed out while polling for tx timestamp
ptp4l: increasing tx_timestamp_timeout may correct this issue,
but it is likely caused by a driver bug
ptp4l: port 1 (enp170s0.vlan): send peer delay response failed
ptp4l: port 1 (enp170s0.vlan): SLAVE to FAULTY on FAULT_DETECTED
ptp4l: port 1 (enp170s0.vlan): FAULTY to LISTENING on INIT_COMPLETE
ptp4l: port 1 (enp170s0.vlan): LISTENING to MASTER on
ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l: selected local clock aa00aa.fffe.00aa00 as best master
ptp4l: port 1 (enp170s0.vlan): assuming the grand master role
ptp4l: port 1 (enp170s0.vlan): new foreign master 22bb22.fffe.bb22bb-1
ptp4l: selected best master clock 22bb22.fffe.bb22bb
ptp4l: port 1 (enp170s0.vlan): MASTER to UNCALIBRATED on RS_SLAVE
ptp4l: port 1 (enp170s0.vlan): UNCALIBRATED to SLAVE on
MASTER_CLOCK_SELECTED
ptp4l: rms   39 max   66 freq  -3355 +/-  45 delay     1 +/-   0
ptp4l: rms   20 max   36 freq  -3339 +/-  12 delay     1 +/-   0
ptp4l: rms   11 max   18 freq  -3371 +/-  11 delay     1 +/-   0
ptp4l: rms   10 max   16 freq  -3384 +/-   2 delay     1 +/-   0
ptp4l: rms    1 max    2 freq  -3375 +/-   2 delay     1 +/-   0
ptp4l: rms    3 max    6 freq  -3373 +/-   4 delay     0 +/-   0

After:

ptp4l: rms    3 max    4 freq  -3386 +/-   4 delay     0 +/-   0
ptp4l: rms    3 max    7 freq  -3380 +/-   3 delay     0 +/-   0
ptp4l: rms    3 max    6 freq  -3380 +/-   3 delay     0 +/-   0
ptp4l: rms    1 max    3 freq  -3381 +/-   2 delay     0 +/-   0
ptp4l: rms    3 max    5 freq  -3377 +/-   2 delay     0 +/-   0
ptp4l: rms    2 max    3 freq  -3377 +/-   2 delay     0 +/-   0
ptp4l: rms    3 max    6 freq  -3375 +/-   4 delay     0 +/-   0
ptp4l: rms    2 max    4 freq  -3380 +/-   2 delay     1 +/-   0
ptp4l: rms    4 max    7 freq  -3385 +/-   3 delay     0 +/-   0
ptp4l: rms    2 max    3 freq  -3384 +/-   2 delay     0 +/-   0
ptp4l: rms    4 max    7 freq  -3376 +/-   2 delay     0 +/-   0
ptp4l: rms    3 max    5 freq  -3376 +/-   4 delay     0 +/-   0
ptp4l: rms    3 max    5 freq  -3382 +/-   2 delay     0 +/-   0
ptp4l: rms    5 max    7 freq  -3389 +/-   2 delay     0 +/-   0
ptp4l: rms    3 max    4 freq  -3388 +/-   3 delay     1 +/-   0
ptp4l: rms    3 max    5 freq  -3387 +/-   4 delay     1 +/-   0
ptp4l: rms    5 max    8 freq  -3395 +/-   3 delay     1 +/-   0
ptp4l: rms    5 max    8 freq  -3399 +/-   4 delay     0 +/-   0
ptp4l: rms    2 max    5 freq  -3397 +/-   3 delay     1 +/-   0
ptp4l: rms    2 max    4 freq  -3397 +/-   3 delay     1 +/-   0
ptp4l: rms    2 max    3 freq  -3397 +/-   2 delay     1 +/-   0
ptp4l: rms    3 max    5 freq  -3391 +/-   2 delay     2 +/-   0

Test Setup:
back-to-back communication between Host and DUT. Host will act as
transmitter and DUT will become receiver. Host will generate the
packet using sample application with timestamping_flag of
SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH and hwtstamp_config flag of
HWTSTAMP_FLAG_DMA_TIMESTAMP.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Co-developed-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
Co-developed-by: Aravindhan Gunasekaran <aravindhan.gunasekaran@intel.com>
Signed-off-by: Aravindhan Gunasekaran <aravindhan.gunasekaran@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h         | 10 +++
 drivers/net/ethernet/intel/igc/igc_base.h    |  2 +-
 drivers/net/ethernet/intel/igc/igc_defines.h |  2 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  5 +-
 drivers/net/ethernet/intel/igc/igc_main.c    | 24 ++++--
 drivers/net/ethernet/intel/igc/igc_ptp.c     | 83 ++++++++++++++++++++
 6 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 1e7e7071f64d..38a24b5260d1 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -348,6 +348,12 @@ extern char igc_driver_name[];
 #define IGC_I225_RX_LATENCY_1000	300
 #define IGC_I225_RX_LATENCY_2500	1485
 
+/* Transmit latency (for DMA timestamps) in nanosecond */
+#define IGC_I225_TX_DMA_LATENCY_10	13100
+#define IGC_I225_TX_DMA_LATENCY_100	1410
+#define IGC_I225_TX_DMA_LATENCY_1000	285
+#define IGC_I225_TX_DMA_LATENCY_2500	1485
+
 /* RX and TX descriptor control thresholds.
  * PTHRESH - MAC will consider prefetch if it has fewer than this number of
  *           descriptors available in its onboard memory.
@@ -410,6 +416,8 @@ enum igc_tx_flags {
 	/* olinfo flags */
 	IGC_TX_FLAGS_IPV4	= 0x10,
 	IGC_TX_FLAGS_CSUM	= 0x20,
+
+	IGC_TX_FLAGS_DMA_TSTAMP	= 0x200,
 };
 
 enum igc_boards {
@@ -627,6 +635,8 @@ void igc_ptp_reset(struct igc_adapter *adapter);
 void igc_ptp_suspend(struct igc_adapter *adapter);
 void igc_ptp_stop(struct igc_adapter *adapter);
 ktime_t igc_ptp_rx_pktstamp(struct igc_adapter *adapter, __le32 *buf);
+void igc_ptp_tx_dma_tstamp(struct igc_adapter *adapter,
+			   struct sk_buff *skb, u64 tstamp);
 int igc_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr);
 int igc_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr);
 void igc_ptp_tx_hang(struct igc_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h
index ce530f5fd7bd..672cf2d92165 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.h
+++ b/drivers/net/ethernet/intel/igc/igc_base.h
@@ -16,7 +16,7 @@ union igc_adv_tx_desc {
 		__le32 olinfo_status;
 	} read;
 	struct {
-		__le64 rsvd;       /* Reserved */
+		__le64 dma_tstamp;
 		__le32 nxtseq_seed;
 		__le32 status;
 	} wb;
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index f7311aeb293b..baedf48b4e2e 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -312,6 +312,7 @@
 #define IGC_TXD_CMD_DEXT	0x20000000 /* Desc extension (0 = legacy) */
 #define IGC_TXD_CMD_VLE		0x40000000 /* Add VLAN tag */
 #define IGC_TXD_STAT_DD		0x00000001 /* Descriptor Done */
+#define IGC_TXD_STAT_TS_STAT    0x00000002 /* DMA Timestamp in packet */
 #define IGC_TXD_CMD_TCP		0x01000000 /* TCP packet */
 #define IGC_TXD_CMD_IP		0x02000000 /* IP packet */
 #define IGC_TXD_CMD_TSE		0x04000000 /* TCP Seg enable */
@@ -520,6 +521,7 @@
 /* Transmit Scheduling */
 #define IGC_TQAVCTRL_TRANSMIT_MODE_TSN	0x00000001
 #define IGC_TQAVCTRL_ENHANCED_QAV	0x00000008
+#define IGC_TQAVCTRL_1588_STAT_EN	0x00000004
 
 #define IGC_TXQCTL_QUEUE_MODE_LAUNCHT	0x00000001
 #define IGC_TXQCTL_STRICT_CYCLE		0x00000002
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 8cc077b712ad..7d198fb6d619 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1532,7 +1532,8 @@ static int igc_ethtool_get_ts_info(struct net_device *dev,
 			SOF_TIMESTAMPING_SOFTWARE |
 			SOF_TIMESTAMPING_TX_HARDWARE |
 			SOF_TIMESTAMPING_RX_HARDWARE |
-			SOF_TIMESTAMPING_RAW_HARDWARE;
+			SOF_TIMESTAMPING_RAW_HARDWARE |
+			SOF_TIMESTAMPING_TX_HARDWARE_DMA_FETCH;
 
 		info->tx_types =
 			BIT(HWTSTAMP_TX_OFF) |
@@ -1541,6 +1542,8 @@ static int igc_ethtool_get_ts_info(struct net_device *dev,
 		info->rx_filters = BIT(HWTSTAMP_FILTER_NONE);
 		info->rx_filters |= BIT(HWTSTAMP_FILTER_ALL);
 
+		info->flag = HWTSTAMP_FLAG_DMA_TIMESTAMP;
+
 		return 0;
 	default:
 		return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 671255edf3c2..daa6ca5acab3 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1415,6 +1415,7 @@ static int igc_tso(struct igc_ring *tx_ring,
 static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 				       struct igc_ring *tx_ring)
 {
+	struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
 	u16 count = TXD_USE_COUNT(skb_headlen(skb));
 	__be16 protocol = vlan_get_protocol(skb);
 	struct igc_tx_buffer *first;
@@ -1445,16 +1446,14 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 	first->bytecount = skb->len;
 	first->gso_segs = 1;
 
-	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
-		struct igc_adapter *adapter = netdev_priv(tx_ring->netdev);
-
+	if (unlikely((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+		     !(skb_shinfo(skb)->tx_flags & SKBTX_HW_DMA_TSTAMP))) {
 		/* FIXME: add support for retrieving timestamps from
 		 * the other timer registers before skipping the
 		 * timestamping request.
 		 */
 		if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ON &&
-		    !test_and_set_bit_lock(__IGC_PTP_TX_IN_PROGRESS,
-					   &adapter->state)) {
+		    !test_and_set_bit_lock(__IGC_PTP_TX_IN_PROGRESS, &adapter->state))	{
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 			tx_flags |= IGC_TX_FLAGS_TSTAMP;
 
@@ -1463,6 +1462,14 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 		} else {
 			adapter->tx_hwtstamp_skipped++;
 		}
+	} else if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_DMA_TSTAMP)) {
+		if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ON &&
+		    adapter->tstamp_config.flags == HWTSTAMP_FLAG_DMA_TIMESTAMP) {
+			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+			tx_flags |= IGC_TX_FLAGS_DMA_TSTAMP;
+		} else {
+			adapter->tx_hwtstamp_skipped++;
+		}
 	}
 
 	if (skb_vlan_tag_present(skb)) {
@@ -2741,6 +2748,13 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
 		if (!(eop_desc->wb.status & cpu_to_le32(IGC_TXD_STAT_DD)))
 			break;
 
+		if (eop_desc->wb.status & cpu_to_le32(IGC_TXD_STAT_TS_STAT) &&
+		    tx_buffer->tx_flags & IGC_TX_FLAGS_DMA_TSTAMP) {
+			u64 tstamp = le64_to_cpu(eop_desc->wb.dma_tstamp);
+
+			igc_ptp_tx_dma_tstamp(adapter, tx_buffer->skb, tstamp);
+		}
+
 		/* clear next_to_watch to prevent false hangs */
 		tx_buffer->next_to_watch = NULL;
 
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 8dbb9f903ca7..631972d7e97b 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -432,6 +432,29 @@ static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
 	}
 }
 
+static void igc_ptp_dma_time_to_hwtstamp(struct igc_adapter *adapter,
+					 struct skb_shared_hwtstamps *hwtstamps,
+					 u64 systim)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 sec, nsec;
+
+	nsec = rd32(IGC_SYSTIML);
+	sec = rd32(IGC_SYSTIMH);
+
+	if (unlikely(nsec < (systim & 0xFFFFFFFF)))
+		--sec;
+
+	switch (adapter->hw.mac.type) {
+	case igc_i225:
+		memset(hwtstamps, 0, sizeof(*hwtstamps));
+		hwtstamps->hwtstamp = ktime_set(sec, systim & 0xFFFFFFFF);
+		break;
+	default:
+		break;
+	}
+}
+
 /**
  * igc_ptp_rx_pktstamp - Retrieve timestamp from Rx packet buffer
  * @adapter: Pointer to adapter the packet buffer belongs to
@@ -549,6 +572,28 @@ static void igc_ptp_enable_tx_timestamp(struct igc_adapter *adapter)
 	rd32(IGC_TXSTMPH);
 }
 
+static void igc_ptp_disable_dma_timestamp(struct igc_adapter *adapter)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 tqavctrl;
+
+	tqavctrl = rd32(IGC_TQAVCTRL);
+	tqavctrl &= ~IGC_TQAVCTRL_1588_STAT_EN;
+
+	wr32(IGC_TQAVCTRL, tqavctrl);
+}
+
+static void igc_ptp_enable_dma_timestamp(struct igc_adapter *adapter)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 tqavctrl;
+
+	tqavctrl = rd32(IGC_TQAVCTRL);
+	tqavctrl |= IGC_TQAVCTRL_1588_STAT_EN;
+
+	wr32(IGC_TQAVCTRL, tqavctrl);
+}
+
 /**
  * igc_ptp_set_timestamp_mode - setup hardware for timestamping
  * @adapter: networking device structure
@@ -562,9 +607,14 @@ static int igc_ptp_set_timestamp_mode(struct igc_adapter *adapter,
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
 		igc_ptp_disable_tx_timestamp(adapter);
+		igc_ptp_disable_dma_timestamp(adapter);
 		break;
 	case HWTSTAMP_TX_ON:
 		igc_ptp_enable_tx_timestamp(adapter);
+
+		/* Ensure that flag only can be used during HWTSTAMP_TX_ON */
+		if (config->flags == HWTSTAMP_FLAG_DMA_TIMESTAMP)
+			igc_ptp_enable_dma_timestamp(adapter);
 		break;
 	default:
 		return -ERANGE;
@@ -683,6 +733,39 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
 	dev_kfree_skb_any(skb);
 }
 
+void igc_ptp_tx_dma_tstamp(struct igc_adapter *adapter,
+			   struct sk_buff *skb, u64 tstamp)
+{
+	struct skb_shared_hwtstamps shhwtstamps;
+	int adjust = 0;
+
+	if (!(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
+		return;
+
+	igc_ptp_dma_time_to_hwtstamp(adapter, &shhwtstamps, tstamp);
+
+	switch (adapter->link_speed) {
+	case SPEED_10:
+		adjust = IGC_I225_TX_DMA_LATENCY_10;
+		break;
+	case SPEED_100:
+		adjust = IGC_I225_TX_DMA_LATENCY_100;
+		break;
+	case SPEED_1000:
+		adjust = IGC_I225_TX_DMA_LATENCY_1000;
+		break;
+	case SPEED_2500:
+		adjust = IGC_I225_TX_DMA_LATENCY_2500;
+		break;
+	}
+
+	shhwtstamps.hwtstamp =
+		ktime_add_ns(shhwtstamps.hwtstamp, adjust);
+
+	/* Notify the stack and free the skb after we've unlocked */
+	skb_tstamp_tx(skb, &shhwtstamps);
+}
+
 /**
  * igc_ptp_tx_work
  * @work: pointer to work struct
-- 
2.17.1

[PATCH v2 5/5] ethtool: Add support for HWTSTAMP_FILTER_DMA_TIMESTAMP

[Muhammad Husaini Zulkifli]

Add new HWTSTAMP_FILTER_DMA_TIMESTAMP receive filters.
This filter can be configured for devices that support/allow the
DMA timestamp retrieval on receive side.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 1 +
 include/uapi/linux/net_tstamp.h          | 3 +++
 net/core/dev_ioctl.c                     | 1 +
 net/ethtool/common.c                     | 1 +
 4 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 631972d7e97b..39ed759f9057 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -637,6 +637,7 @@ static int igc_ptp_set_timestamp_mode(struct igc_adapter *adapter,
 	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
 	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
 	case HWTSTAMP_FILTER_NTP_ALL:
+	case HWTSTAMP_FILTER_DMA_TIMESTAMP:
 	case HWTSTAMP_FILTER_ALL:
 		igc_ptp_enable_rx_timestamp(adapter);
 		config->rx_filter = HWTSTAMP_FILTER_ALL;
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index c9e113cea883..e72261ed879f 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -177,6 +177,9 @@ enum hwtstamp_rx_filters {
 	/* NTP, UDP, all versions and packet modes */
 	HWTSTAMP_FILTER_NTP_ALL,
 
+	/* DMA time stamp packet */
+	HWTSTAMP_FILTER_DMA_TIMESTAMP,
+
 	/* add new constants above here */
 	__HWTSTAMP_FILTER_CNT
 };
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 7674bb9f3076..963327472c26 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -229,6 +229,7 @@ static int net_hwtstamp_validate(struct ifreq *ifr)
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
 	case HWTSTAMP_FILTER_NTP_ALL:
+	case HWTSTAMP_FILTER_DMA_TIMESTAMP:
 		rx_filter_valid = 1;
 		break;
 	case __HWTSTAMP_FILTER_CNT:
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 24f8e7f9b4a5..fe6e5dfdfcce 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -438,6 +438,7 @@ const char ts_rx_filter_names[][ETH_GSTRING_LEN] = {
 	[HWTSTAMP_FILTER_PTP_V2_SYNC]		= "ptpv2-sync",
 	[HWTSTAMP_FILTER_PTP_V2_DELAY_REQ]	= "ptpv2-delay-req",
 	[HWTSTAMP_FILTER_NTP_ALL]		= "ntp-all",
+	[HWTSTAMP_FILTER_DMA_TIMESTAMP]		= "dma-timestamp",
 };
 static_assert(ARRAY_SIZE(ts_rx_filter_names) == __HWTSTAMP_FILTER_CNT);
 
-- 
2.17.1

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Richard Cochran]

On Tue, Oct 18, 2022 at 09:07:28AM +0800, Muhammad Husaini Zulkifli wrote:

> With these additional socket options, users can continue to utilise
> HW timestamps for PTP while specifying non-PTP packets to use DMA
> timestamps if the NIC can support them.

What is the use case for reporting DMA transmit time?

How is this better than using SOF_TIMESTAMPING_TX_SOFTWARE ?


Thanks,
Richard

RE: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Zulkifli, Muhammad Husaini]

Hi Richard,

Thanks for your review. Replied inline.

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Tuesday, 18 October, 2022 7:45 PM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
> Cc: intel-wired-lan@osuosl.org; netdev@vger.kernel.org; kuba@kernel.org;
> davem@davemloft.net; edumazet@google.com; Gunasekaran, Aravindhan
> <aravindhan.gunasekaran@intel.com>; gal@nvidia.com; saeed@kernel.org;
> leon@kernel.org; michael.chan@broadcom.com; andy@greyhouse.net;
> Gomes, Vinicius <vinicius.gomes@intel.com>
> Subject: Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP
> packets
> 
> On Tue, Oct 18, 2022 at 09:07:28AM +0800, Muhammad Husaini Zulkifli
> wrote:
> 
> > With these additional socket options, users can continue to utilise HW
> > timestamps for PTP while specifying non-PTP packets to use DMA
> > timestamps if the NIC can support them.
> 
> What is the use case for reporting DMA transmit time?
> 
> How is this better than using SOF_TIMESTAMPING_TX_SOFTWARE ?

The ideal situation is to use SOF TIMESTAMPING TX SOFTWARE. 
Application side will undoubtedly want to utilize PHY timestamps as much as possible. 

But if every application requested the HW Tx timestamp, especially during periods of high load like TSN, 
in some cases when the controller only supports 1 HW timestamp, it would be a disaster.
We will observe some missing timestamp reported later.

Using the DMA transmit time for the tx timestamp, if supported, is one of the solutions that we propose here.

> 
> 
> Thanks,
> Richard

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Richard Cochran]

On Tue, Oct 18, 2022 at 02:12:37PM +0000, Zulkifli, Muhammad Husaini wrote:
> > What is the use case for reporting DMA transmit time?

So the use case (mentioned below) is TSN?

> > How is this better than using SOF_TIMESTAMPING_TX_SOFTWARE ?
> 
> The ideal situation is to use SOF TIMESTAMPING TX SOFTWARE. 
> Application side will undoubtedly want to utilize PHY timestamps as much as possible. 

I'm asking about SOFTWARE not hardware time stamps.
 
> But if every application requested the HW Tx timestamp, especially during periods of high load like TSN, 
> in some cases when the controller only supports 1 HW timestamp, it would be a disaster.

But can't you offer 1 HW time stamp with many SW time stamps?

Thanks,
Rihcard

RE: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Zulkifli, Muhammad Husaini]

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Wednesday, 19 October, 2022 10:37 AM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
> Cc: intel-wired-lan@osuosl.org; netdev@vger.kernel.org; kuba@kernel.org;
> davem@davemloft.net; edumazet@google.com; Gunasekaran, Aravindhan
> <aravindhan.gunasekaran@intel.com>; gal@nvidia.com; saeed@kernel.org;
> leon@kernel.org; michael.chan@broadcom.com; andy@greyhouse.net;
> Gomes, Vinicius <vinicius.gomes@intel.com>
> Subject: Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP
> packets
> 
> On Tue, Oct 18, 2022 at 02:12:37PM +0000, Zulkifli, Muhammad Husaini
> wrote:
> > > What is the use case for reporting DMA transmit time?
> 
> So the use case (mentioned below) is TSN?

Yes. TSN/Real Time applications. The most desirable is reporting the HW Timestamp.
But if all real time applications in a heavy traffic load trying to use same 
HW Timestamp (ex. 1 HW Timestamp only available) there will be a failure event 
Like missing a timestamp. The purpose of DMA transmit time is to give the application 
a different option for using DMA Timestamp when this event occur.

> 
> > > How is this better than using SOF_TIMESTAMPING_TX_SOFTWARE ?
> >
> > The ideal situation is to use SOF TIMESTAMPING TX SOFTWARE.
> > Application side will undoubtedly want to utilize PHY timestamps as much
> as possible.
> 
> I'm asking about SOFTWARE not hardware time stamps.

Sorry for misinterpreting SOFTWARE as HARDWARE in my previous reply.
DMA Timestamping is definitely better than SOFTWARE timestamp because 
we sample the time at the controller MAC level.

> 
> > But if every application requested the HW Tx timestamp, especially
> > during periods of high load like TSN, in some cases when the controller only
> supports 1 HW timestamp, it would be a disaster.
> 
> But can't you offer 1 HW time stamp with many SW time stamps? 

Could you please provide additional details about this? What do you meant by 
offering 1 HW Timestamp with many SW timestamps? 

> 
> Thanks,
> Rihcard

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Richard Cochran]

On Fri, Oct 21, 2022 at 12:25:13AM +0000, Zulkifli, Muhammad Husaini wrote:

> Sorry for misinterpreting SOFTWARE as HARDWARE in my previous reply.
> DMA Timestamping is definitely better than SOFTWARE timestamp because 
> we sample the time at the controller MAC level.

Do you have numbers to back up this claim?

> Could you please provide additional details about this? What do you meant by 
> offering 1 HW Timestamp with many SW timestamps? 

- Let the PTP stack use hardware time stamps.

- Let all other applications use software time stamps.

Hm?

Thanks,
Richard

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Jakub Kicinski]

On Fri, 21 Oct 2022 05:48:44 -0700 Richard Cochran wrote:
> > Could you please provide additional details about this? What do you meant by 
> > offering 1 HW Timestamp with many SW timestamps?   
> 
> - Let the PTP stack use hardware time stamps.
> 
> - Let all other applications use software time stamps.

We do need HW stamps for congestion control, the Rx ring queuing 
(as well as Tx ring scheduling jitter) is very often in 10s of msec.
Comparing the SW stamp to the HW stamp is where we derive the signal
that the system is under excessive load.

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Gal Pressman]

Hello Muhammad,

On 18/10/2022 04:07, Muhammad Husaini Zulkifli wrote:
> The problem is that, when there is a lot of traffic, there is a high chance
> that the timestamps for a PTP packet will be lost if both PTP and Non-PTP
> packets use the same SOF TIMESTAMPING TX HARDWARE causing the tx timeout.

Why would the timestamp be affected by the amount of traffic?
What tx timeout?

> DMA timestamps through socket options are not currently available to
> the user. Because if the user wants to, they can configure the hwtstamp
> config option to use the new introduced DMA Time Stamp flag through the
> setsockopt().
>
> With these additional socket options, users can continue to utilise
> HW timestamps for PTP while specifying non-PTP packets to use DMA
> timestamps if the NIC can support them.

Is it per socket?
Will there be a way to know which types of timestamps are going to be
used on queues setup time?

> This patch series also add a new HWTSTAMP_FILTER_DMA_TIMESTAMP receive
> filters. This filter can be configured for devices that support/allow the
> DMA timestamp retrieval on receive side.

So if I understand correctly, to solve the problem you described at the
beginning, you'll disable port timestamp for all incoming packets? ptp
included?

RE: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Zulkifli, Muhammad Husaini]

Hi Gal,

> -----Original Message-----
> From: Gal Pressman <gal@nvidia.com>
> Sent: Tuesday, 18 October, 2022 10:23 PM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> intel-wired-lan@osuosl.org
> Cc: netdev@vger.kernel.org; kuba@kernel.org; davem@davemloft.net;
> edumazet@google.com; Gunasekaran, Aravindhan
> <aravindhan.gunasekaran@intel.com>; richardcochran@gmail.com;
> saeed@kernel.org; leon@kernel.org; michael.chan@broadcom.com;
> andy@greyhouse.net; Gomes, Vinicius <vinicius.gomes@intel.com>
> Subject: Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP
> packets
> 
> Hello Muhammad,
> 
> On 18/10/2022 04:07, Muhammad Husaini Zulkifli wrote:
> > The problem is that, when there is a lot of traffic, there is a high
> > chance that the timestamps for a PTP packet will be lost if both PTP
> > and Non-PTP packets use the same SOF TIMESTAMPING TX HARDWARE
> causing the tx timeout.
> 
> Why would the timestamp be affected by the amount of traffic?
> What tx timeout?

Basically, the original timestamp register collect the timestamp at the PHY level 
and interrupt is triggered for the driver to read it. We observed timestamp missed 
issues when the traffic is high. DMA Timestamp guarantees the timestamp for every
packets as it is delivered through descriptor write-back mechanism.

When there is a timestamp missed, ptp4l application will complain there is a tx_timeout.

> 
> > DMA timestamps through socket options are not currently available to
> > the user. Because if the user wants to, they can configure the
> > hwtstamp config option to use the new introduced DMA Time Stamp flag
> > through the setsockopt().
> >
> > With these additional socket options, users can continue to utilise HW
> > timestamps for PTP while specifying non-PTP packets to use DMA
> > timestamps if the NIC can support them.
> 
> Is it per socket?

Yes it is per socket.

> Will there be a way to know which types of timestamps are going to be used
> on queues setup time?

We can get the which timestamp that is supported through "ethtool -T" command.
May I know why you want to know which timestamp need to configure during queue setup?

> 
> > This patch series also add a new HWTSTAMP_FILTER_DMA_TIMESTAMP
> receive
> > filters. This filter can be configured for devices that support/allow
> > the DMA timestamp retrieval on receive side.
> 
> So if I understand correctly, to solve the problem you described at the
> beginning, you'll disable port timestamp for all incoming packets? ptp
> included?

For ptp, it will always use Port Timestamp.
Other application that want to use DMA Timestamp can request for the same.

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Gal Pressman]

On 19/10/2022 17:23, Zulkifli, Muhammad Husaini wrote:
>>> DMA timestamps through socket options are not currently available to
>>> the user. Because if the user wants to, they can configure the
>>> hwtstamp config option to use the new introduced DMA Time Stamp flag
>>> through the setsockopt().
>>>
>>> With these additional socket options, users can continue to utilise HW
>>> timestamps for PTP while specifying non-PTP packets to use DMA
>>> timestamps if the NIC can support them.
>> Is it per socket?
> Yes it is per socket.
>
>> Will there be a way to know which types of timestamps are going to be used
>> on queues setup time?
> We can get the which timestamp that is supported through "ethtool -T" command.
> May I know why you want to know which timestamp need to configure during queue setup?

In mlx5 we need dedicated queues which support port timestamp. In
today's implementation we have a setup time priv-flag which dictates
whether these queues should be opened or not.

>
>>> This patch series also add a new HWTSTAMP_FILTER_DMA_TIMESTAMP
>> receive
>>> filters. This filter can be configured for devices that support/allow
>>> the DMA timestamp retrieval on receive side.
>> So if I understand correctly, to solve the problem you described at the
>> beginning, you'll disable port timestamp for all incoming packets? ptp
>> included?
> For ptp, it will always use Port Timestamp.
> Other application that want to use DMA Timestamp can request for the same.
>

How?
When a packet arrives from the wire the hardware doesn't know if it's
ptp or not. Does your hardware inspect the packet headers to decide when
to timestamp it?

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Jakub Kicinski]

On Tue, 18 Oct 2022 09:07:28 +0800 Muhammad Husaini Zulkifli wrote:
> v1 -> v2:
> 	- Move to the end for the new enum.
> 	- Add new HWTSTAMP_FILTER_DMA_TIMESTAMP receive filters.

Did you address my feedback? How do we know if existing
HWTSTAMP_FILTER_ALL is PHC quality of DMA?

RE: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Zulkifli, Muhammad Husaini]

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Wednesday, 19 October, 2022 3:03 AM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
> Cc: intel-wired-lan@osuosl.org; netdev@vger.kernel.org;
> davem@davemloft.net; edumazet@google.com; Gunasekaran, Aravindhan
> <aravindhan.gunasekaran@intel.com>; richardcochran@gmail.com;
> gal@nvidia.com; saeed@kernel.org; leon@kernel.org;
> michael.chan@broadcom.com; andy@greyhouse.net; Gomes, Vinicius
> <vinicius.gomes@intel.com>
> Subject: Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP
> packets
> 
> On Tue, 18 Oct 2022 09:07:28 +0800 Muhammad Husaini Zulkifli wrote:
> > v1 -> v2:
> > 	- Move to the end for the new enum.
> > 	- Add new HWTSTAMP_FILTER_DMA_TIMESTAMP receive filters.
> 
> Did you address my feedback? How do we know if existing
> HWTSTAMP_FILTER_ALL is PHC quality of DMA?

I apologize if I didn't respond to your feedback. If I recall properly, you agreed to only
 use the flag rather than creating a new tx type as a result of below conversation. 
https://lore.kernel.org/all/20220930074016.295cbfab@kernel.org/

Re: [PATCH v2 0/5] Add support for DMA timestamp for non-PTP packets

[Jakub Kicinski]

On Thu, 20 Oct 2022 02:16:25 +0000 Zulkifli, Muhammad Husaini wrote:
> > On Tue, 18 Oct 2022 09:07:28 +0800 Muhammad Husaini Zulkifli wrote:  
> > > v1 -> v2:
> > > 	- Move to the end for the new enum.
> > > 	- Add new HWTSTAMP_FILTER_DMA_TIMESTAMP receive filters.  
> > 
> > Did you address my feedback? How do we know if existing
> > HWTSTAMP_FILTER_ALL is PHC quality of DMA?  
> 
> I apologize if I didn't respond to your feedback. If I recall properly, you agreed to only
>  use the flag rather than creating a new tx type as a result of below conversation. 
> https://lore.kernel.org/all/20220930074016.295cbfab@kernel.org/

My bad, I wasn't very clear. I meant to agree that if you prefer we can
forgo adding a new tx_type, and depend on SOF_TIMESTAMPING_TX_* to
advertise the capabilities and request particular type. But
SOF_TIMESTAMPING_TX_* should still not assume that SOF_TIMESTAMPING_TX_HARDWARE 
is guaranteed to be measured close to the wire.

I was just looking at some mlx5 patches from last night:
https://lore.kernel.org/all/20221019063813.802772-5-saeed@kernel.org/
and if you look around you'll see that they advertise hardware
timestamps but AFAIU only packets they match to be PTP will go on 
the special queue that gets real close-to-wire time:
https://elixir.bootlin.com/linux/v6.1-rc1/source/drivers/net/ethernet/mellanox/mlx5/core/en/selq.c#L247
If application requests a HW stamp on a non-PTP packet (NTP? custom
proto?), or PTP is running on a custom UDP port, or simply the FW/HW
does not support this feature (mlx5 supports at least a decade's worth
of HW generations) - the application will get a DMA stamp.

Same for Rx - close-to-wire stamps may be more expensive for the NIC
and some modern TCP congestion control algos (BBRv2, Swift) need
HW timestamps of all packets but are okay with DMA stamps.

So we need:
 1) clear disambiguation when time stamps are *really* taken
    close-to-the-wire (CTW);
 2) ability to configure Rx to provide CTW stamps for PTP/NTP
    frames and DMA stamps for all the rest for congestion control.

I think extending the documentation in:
  Documentation/networking/timestamping.rst
to explain the API you're adding and how it will allow the above cases
to be satisfied will be a good start.