Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Lasse Johnsen]

Hi Richard,

I understand your point and your concern.

Would you be amenable to an addition to the API so we can take advantage of 
hardware that offers only a subset of the options?

We could for example extend granularity of the HWTSTAMP TX API to make requests 
for different features visible to the user space applications directly. So the TX side 
would become much more granular as is already the case with the RX side. We could 
add HWTSTAMP_TX_ONESTEP_SYNC_L2_V2, HWTSTAMP_TX_ONESTEP_SYNC_L4_V2 etc. 

My worry is that if we do not do this, then the ONESTEP option will continue 
to not see much use because so many permutations (L2, UDPv4, UDPv6, V1, V2, VLAN etc.)
currently have to be supported by the hardware.

I like Intel’s cards. I want to support the features provided by the hardware… :-)

If you agree with this approach, then I will submit instead two patches: One patch that 
extends the API and another modified version of the current igc patch which will 
use the new more granular option. For the former, I will try and persuade (...beg... I will beg...) 
JL to supervise the API work so it does not go off the rails :-)

In parallel, I have reached out to Kevin and asked if the 1-step logic will ever be able to
update the UDP checksum on-the-fly in which case I shall certainly include this functionality
as well.

Let me know what you think.

All the best,

Lasse

> On 9 Sep 2022, at 15:21, Richard Cochran <richardcochran@gmail.com> wrote:
> 
> On Fri, Sep 09, 2022 at 09:51:23AM +0100, Lasse Johnsen wrote:
> 
>> So where the driver received an ioctl with tx config option HWTSTAMP_TX_ONESTEP_SYNC it will process 
>> skbs not matching the above criteria (including  PTP_CLASS_IPV4) as it would have had the tx config option 
>> been HWTSTAMP_TX_ON. This patch does not change the behaviour of the latter in any way.
>> 
>> Therefore a user space application which has used the HWTSTAMP_TX_ONESTEP_SYNC tx config option
>> and is sending UDP messages will as usual receive those messages back in the error queue with 
>> hardware timestamps in the normal fashion. (provided of course in the case of this specific driver that
>> another tx timestamping operation is not in progress.)
> 
> Okay, then I must NAK this patch.  If driver offers one-step and user
> enables it, then it should work.
> 
> The option, HWTSTAMP_TX_ONESTEP_SYNC, means to apply one-step to any
> and all Sync frames, not just layer 2.  The API does not offer a way
> to advertise or configure one-step selectively based on network layer.
> 
> Thanks,
> Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Richard Cochran]

On Sat, Sep 10, 2022 at 04:07:48PM +0100, Lasse Johnsen wrote:
> Would you be amenable to an addition to the API so we can take advantage of 
> hardware that offers only a subset of the options?

I think that is only user friendly option.  However, the question is
whether this would find broad use, or would it remain an isolated hack
for one borken hardware design?

> We could for example extend granularity of the HWTSTAMP TX API to make requests 
> for different features visible to the user space applications directly. So the TX side 
> would become much more granular as is already the case with the RX side. We could 
> add HWTSTAMP_TX_ONESTEP_SYNC_L2_V2, HWTSTAMP_TX_ONESTEP_SYNC_L4_V2 etc. 
> 
> My worry is that if we do not do this, then the ONESTEP option will continue 
> to not see much use because so many permutations (L2, UDPv4, UDPv6, V1, V2, VLAN etc.)
> currently have to be supported by the hardware.

Actually IMO the opposite is true.  If the API nickel and dimes every
last combination then:

- that will only encourage even more broken hardware designs
- no user space software will implement the combos

Case in point:

	/* PTP v1, UDP, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V1_L4_EVENT,
	/* PTP v1, UDP, Sync packet */
	HWTSTAMP_FILTER_PTP_V1_L4_SYNC,
	/* PTP v1, UDP, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ,
	/* PTP v2, UDP, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V2_L4_EVENT,
	/* PTP v2, UDP, Sync packet */
	HWTSTAMP_FILTER_PTP_V2_L4_SYNC,
	/* PTP v2, UDP, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ,

	/* 802.AS1, Ethernet, any kind of event packet */
	HWTSTAMP_FILTER_PTP_V2_L2_EVENT,
	/* 802.AS1, Ethernet, Sync packet */
	HWTSTAMP_FILTER_PTP_V2_L2_SYNC,
	/* 802.AS1, Ethernet, Delay_req packet */
	HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ,

This monstrosity came about because of ONE early, brain dead HW
design.  Seriously, who would ever want to have just "PTP v1, UDP,
Delay_req packet" and not the other event messages?

This horrible API is now written in stone, but never, ever used.

If hardware claims to implement PTP one-step, then it really should do
so in a way that conforms to the published standards.

Thanks,
Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[kernel test robot]

Hi Lasse,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Lasse-Johnsen/igc-ptp-Add-1-step-functionality-to-igc-driver/20220909-062001
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9f8f1933dce555d3c246f447f54fca8de8889da9
config: openrisc-randconfig-s052-20220909 (https://download.01.org/0day-ci/archive/20220909/202209092314.pShH7HvH-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/148cdacbd4f77d88190d8fbeb4a95fedeb645f6b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lasse-Johnsen/igc-ptp-Add-1-step-functionality-to-igc-driver/20220909-062001
        git checkout 148cdacbd4f77d88190d8fbeb4a95fedeb645f6b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/net/ethernet/intel/igc/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/intel/igc/igc_main.c:1264:44: sparse: sparse: invalid assignment: |=
>> drivers/net/ethernet/intel/igc/igc_main.c:1264:44: sparse:    left side has type restricted __le32
>> drivers/net/ethernet/intel/igc/igc_main.c:1264:44: sparse:    right side has type int

vim +1264 drivers/net/ethernet/intel/igc/igc_main.c

  1182	
  1183	static int igc_tx_map(struct igc_ring *tx_ring,
  1184			      struct igc_tx_buffer *first,
  1185			      const u8 hdr_len)
  1186	{
  1187		struct sk_buff *skb = first->skb;
  1188		struct igc_tx_buffer *tx_buffer;
  1189		union igc_adv_tx_desc *tx_desc;
  1190		u32 tx_flags = first->tx_flags;
  1191		skb_frag_t *frag;
  1192		u16 i = tx_ring->next_to_use;
  1193		unsigned int data_len, size;
  1194		dma_addr_t dma;
  1195		u32 cmd_type;
  1196	
  1197		cmd_type = igc_tx_cmd_type(skb, tx_flags);
  1198		tx_desc = IGC_TX_DESC(tx_ring, i);
  1199	
  1200		igc_tx_olinfo_status(tx_ring, tx_desc, tx_flags, skb->len - hdr_len);
  1201	
  1202		size = skb_headlen(skb);
  1203		data_len = skb->data_len;
  1204	
  1205		dma = dma_map_single(tx_ring->dev, skb->data, size, DMA_TO_DEVICE);
  1206	
  1207		tx_buffer = first;
  1208	
  1209		for (frag = &skb_shinfo(skb)->frags[0];; frag++) {
  1210			if (dma_mapping_error(tx_ring->dev, dma))
  1211				goto dma_error;
  1212	
  1213			/* record length, and DMA address */
  1214			dma_unmap_len_set(tx_buffer, len, size);
  1215			dma_unmap_addr_set(tx_buffer, dma, dma);
  1216	
  1217			tx_desc->read.buffer_addr = cpu_to_le64(dma);
  1218	
  1219			while (unlikely(size > IGC_MAX_DATA_PER_TXD)) {
  1220				tx_desc->read.cmd_type_len =
  1221					cpu_to_le32(cmd_type ^ IGC_MAX_DATA_PER_TXD);
  1222	
  1223				i++;
  1224				tx_desc++;
  1225				if (i == tx_ring->count) {
  1226					tx_desc = IGC_TX_DESC(tx_ring, 0);
  1227					i = 0;
  1228				}
  1229				tx_desc->read.olinfo_status = 0;
  1230	
  1231				dma += IGC_MAX_DATA_PER_TXD;
  1232				size -= IGC_MAX_DATA_PER_TXD;
  1233	
  1234				tx_desc->read.buffer_addr = cpu_to_le64(dma);
  1235			}
  1236	
  1237			if (likely(!data_len))
  1238				break;
  1239	
  1240			tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type ^ size);
  1241	
  1242			i++;
  1243			tx_desc++;
  1244			if (i == tx_ring->count) {
  1245				tx_desc = IGC_TX_DESC(tx_ring, 0);
  1246				i = 0;
  1247			}
  1248			tx_desc->read.olinfo_status = 0;
  1249	
  1250			size = skb_frag_size(frag);
  1251			data_len -= size;
  1252	
  1253			dma = skb_frag_dma_map(tx_ring->dev, frag, 0,
  1254					       size, DMA_TO_DEVICE);
  1255	
  1256			tx_buffer = &tx_ring->tx_buffer_info[i];
  1257		}
  1258	
  1259		/* write last descriptor with RS and EOP bits */
  1260		cmd_type |= size | IGC_TXD_DCMD;
  1261		tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
  1262	
  1263		if (first->tx_flags & IGC_TX_FLAGS_ONESTEP_SYNC)
> 1264			tx_desc->read.cmd_type_len |= IGC_ADVTXD_ONESTEP;
  1265	
  1266		netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount);
  1267	
  1268		/* set the timestamp */
  1269		first->time_stamp = jiffies;
  1270	
  1271		skb_tx_timestamp(skb);
  1272	
  1273		/* Force memory writes to complete before letting h/w know there
  1274		 * are new descriptors to fetch.  (Only applicable for weak-ordered
  1275		 * memory model archs, such as IA-64).
  1276		 *
  1277		 * We also need this memory barrier to make certain all of the
  1278		 * status bits have been updated before next_to_watch is written.
  1279		 */
  1280		wmb();
  1281	
  1282		/* set next_to_watch value indicating a packet is present */
  1283		first->next_to_watch = tx_desc;
  1284	
  1285		i++;
  1286		if (i == tx_ring->count)
  1287			i = 0;
  1288	
  1289		tx_ring->next_to_use = i;
  1290	
  1291		/* Make sure there is space in the ring for the next send. */
  1292		igc_maybe_stop_tx(tx_ring, DESC_NEEDED);
  1293	
  1294		if (netif_xmit_stopped(txring_txq(tx_ring)) || !netdev_xmit_more()) {
  1295			writel(i, tx_ring->tail);
  1296		}
  1297	
  1298		return 0;
  1299	dma_error:
  1300		netdev_err(tx_ring->netdev, "TX DMA map failed\n");
  1301		tx_buffer = &tx_ring->tx_buffer_info[i];
  1302	
  1303		/* clear dma mappings for failed tx_buffer_info map */
  1304		while (tx_buffer != first) {
  1305			if (dma_unmap_len(tx_buffer, len))
  1306				igc_unmap_tx_buffer(tx_ring->dev, tx_buffer);
  1307	
  1308			if (i-- == 0)
  1309				i += tx_ring->count;
  1310			tx_buffer = &tx_ring->tx_buffer_info[i];
  1311		}
  1312	
  1313		if (dma_unmap_len(tx_buffer, len))
  1314			igc_unmap_tx_buffer(tx_ring->dev, tx_buffer);
  1315	
  1316		dev_kfree_skb_any(tx_buffer->skb);
  1317		tx_buffer->skb = NULL;
  1318	
  1319		tx_ring->next_to_use = i;
  1320	
  1321		return -1;
  1322	}
  1323	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Richard Cochran]

On Fri, Sep 09, 2022 at 09:51:23AM +0100, Lasse Johnsen wrote:

> So where the driver received an ioctl with tx config option HWTSTAMP_TX_ONESTEP_SYNC it will process 
> skbs not matching the above criteria (including  PTP_CLASS_IPV4) as it would have had the tx config option 
> been HWTSTAMP_TX_ON. This patch does not change the behaviour of the latter in any way.
> 
> Therefore a user space application which has used the HWTSTAMP_TX_ONESTEP_SYNC tx config option
> and is sending UDP messages will as usual receive those messages back in the error queue with 
> hardware timestamps in the normal fashion. (provided of course in the case of this specific driver that
> another tx timestamping operation is not in progress.)

Okay, then I must NAK this patch.  If driver offers one-step and user
enables it, then it should work.

The option, HWTSTAMP_TX_ONESTEP_SYNC, means to apply one-step to any
and all Sync frames, not just layer 2.  The API does not offer a way
to advertise or configure one-step selectively based on network layer.

Thanks,
Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Lasse Johnsen]

My apologies, I will try and be more specific.

This patch affects only skbs in the TX direction which upon being passed to ptp_classify_raw are found 
to be of class PTP_CLASS_L2 and of class PTP_CLASS_V2, and on being passed to ptp_msg_is_sync
are found to be of type SYNC as well.

So where the driver received an ioctl with tx config option HWTSTAMP_TX_ONESTEP_SYNC it will process 
skbs not matching the above criteria (including  PTP_CLASS_IPV4) as it would have had the tx config option 
been HWTSTAMP_TX_ON. This patch does not change the behaviour of the latter in any way.

Therefore a user space application which has used the HWTSTAMP_TX_ONESTEP_SYNC tx config option
and is sending UDP messages will as usual receive those messages back in the error queue with 
hardware timestamps in the normal fashion. (provided of course in the case of this specific driver that
another tx timestamping operation is not in progress.)

All the best,

Lasse

> On 9 Sep 2022, at 03:51, Richard Cochran <richardcochran@gmail.com> wrote:
> 
> On Thu, Sep 08, 2022 at 11:48:48PM +0100, Lasse Johnsen wrote:
>> PTP over UDPv4 can run as 2-step concurrently with PTP over Ethernet as 1-step.
> 
> That is not my question.
> 
> What happens when user space dials one-step and UDP?
> 
> Thanks,
> Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Richard Cochran]

On Thu, Sep 08, 2022 at 11:48:48PM +0100, Lasse Johnsen wrote:
> PTP over UDPv4 can run as 2-step concurrently with PTP over Ethernet as 1-step.

That is not my question.

What happens when user space dials one-step and UDP?

Thanks,
Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Lasse Johnsen]

Hi Richard,

PTP over UDPv4 can run as 2-step concurrently with PTP over Ethernet as 1-step.

Below tshark output from Timebeat in the lab.

All the best,

Lasse
---
[root@gm03 ~]#  tshark -i ens1 ether proto 0x88F7 or port 319 or port 320
Running as user "root" and group "root". This could be dangerous.
Capturing on 'ens1'
 ** (tshark:501799) 23:40:12.801240 [Main MESSAGE] -- Capture started.
 ** (tshark:501799) 23:40:12.803786 [Main MESSAGE] -- File: "/var/tmp/wireshark_ens1zTZBYi.pcapng"
    1 0.000000000 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
    2 0.000174188 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
    3 0.029971841 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
    4 0.030112739 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
    5 0.999391207 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
    6 0.999420338 10.101.101.33 → 224.0.1.129  PTPv2 106 Announce Message
    7 0.999469318 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
    8 0.999493068 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 78 Announce Message
    9 0.999519838 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   10 1.008318061 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   11 1.008417187 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
   12 1.999732615 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
   13 1.999774247 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
   14 1.999840628 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   15 2.008590292 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   16 2.008662394 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
   17 3.000152149 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
   18 3.000183805 10.101.101.33 → 224.0.1.129  PTPv2 106 Announce Message
   19 3.000195810 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
   20 3.000216212 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 78 Announce Message
   21 3.000288899 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   22 3.009000057 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   23 3.009075505 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
   24 4.000147951 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
   25 4.000181728 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
   26 4.000248327 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   27 4.008972302 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   28 4.009030511 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
   29 4.999682198 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
   30 4.999742635 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
   31 4.999747433 10.101.101.33 → 224.0.1.129  PTPv2 106 Announce Message
   32 4.999772787 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 78 Announce Message
   33 4.999857708 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   34 5.008507739 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   35 5.008628544 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
^C   36 5.999954160 10.101.101.33 → 224.0.1.129  PTPv2 86 Sync Message
   37 5.999995843 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 60 Sync Message
   38 6.000067764 10.101.101.33 → 224.0.1.129  PTPv2 86 Follow_Up Message
   39 6.008777977 HewlettP_1c:42:03 → IEEEI&MS_00:00:00 PTPv2 60 Delay_Req Message
   40 6.008864087 IntelCor_91:5e:9d → IEEEI&MS_00:00:00 PTPv2 68 Delay_Resp Message
40 packets captured 

> On 8 Sep 2022, at 23:28, Richard Cochran <richardcochran@gmail.com> wrote:
> 
> On Thu, Sep 08, 2022 at 11:18:35PM +0100, Lasse Johnsen wrote:
>> This patch adds 1-step functionality to the igc driver
>> 1-step is only supported in L2 PTP
>> (... as the hardware can update the FCS, but not the UDP checksum on the fly..)
> 
> What happens when user space dials one-step, but in a UDPv4 PTP network?
> 
> Thanks,
> Richard

Re: [PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Richard Cochran]

On Thu, Sep 08, 2022 at 11:18:35PM +0100, Lasse Johnsen wrote:
> This patch adds 1-step functionality to the igc driver
> 1-step is only supported in L2 PTP
> (... as the hardware can update the FCS, but not the UDP checksum on the fly..)

What happens when user space dials one-step, but in a UDPv4 PTP network?

Thanks,
Richard

[PATCH net-next 1/1] igc: ptp: Add 1-step functionality to igc driver

[Lasse Johnsen]

This patch adds 1-step functionality to the igc driver
1-step is only supported in L2 PTP
(... as the hardware can update the FCS, but not the UDP checksum on the fly..)

Signed-off-by: Lasse Johnsen <lasse@timebeat.app>
---
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 1e7e707..70d9440 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -247,6 +247,8 @@ struct igc_adapter {
 		struct timespec64 start;
 		struct timespec64 period;
 	} perout[IGC_N_PEROUT];
+
+	bool onestep_discard;
 };
 
 void igc_up(struct igc_adapter *adapter);
@@ -403,13 +405,14 @@ enum igc_state_t {
 
 enum igc_tx_flags {
 	/* cmd_type flags */
-	IGC_TX_FLAGS_VLAN	= 0x01,
-	IGC_TX_FLAGS_TSO	= 0x02,
-	IGC_TX_FLAGS_TSTAMP	= 0x04,
+	IGC_TX_FLAGS_VLAN		= 0x01,
+	IGC_TX_FLAGS_TSO		= 0x02,
+	IGC_TX_FLAGS_TSTAMP		= 0x04,
+	IGC_TX_FLAGS_ONESTEP_SYNC	= 0x08,
 
 	/* olinfo flags */
-	IGC_TX_FLAGS_IPV4	= 0x10,
-	IGC_TX_FLAGS_CSUM	= 0x20,
+	IGC_TX_FLAGS_IPV4		= 0x10,
+	IGC_TX_FLAGS_CSUM		= 0x20,
 };
 
 enum igc_boards {
@@ -631,6 +634,7 @@ 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);
 void igc_ptp_read(struct igc_adapter *adapter, struct timespec64 *ts);
+bool igc_ptp_process_onestep(struct igc_adapter *adapter, struct sk_buff *skb);
 
 #define igc_rx_pg_size(_ring) (PAGE_SIZE << igc_rx_pg_order(_ring))
 
diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h
index ce530f5..a7106b5 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.h
+++ b/drivers/net/ethernet/intel/igc/igc_base.h
@@ -31,6 +31,7 @@ struct igc_adv_tx_context_desc {
 };
 
 /* Adv Transmit Descriptor Config Masks */
+#define IGC_ADVTXD_ONESTEP	0x00040000 /* IEEE1588 perform one-step */
 #define IGC_ADVTXD_MAC_TSTAMP	0x00080000 /* IEEE1588 Timestamp packet */
 #define IGC_ADVTXD_DTYP_CTXT	0x00200000 /* Advanced Context Descriptor */
 #define IGC_ADVTXD_DTYP_DATA	0x00300000 /* Advanced Data Descriptor */
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 8cc077b..cba34ae 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1536,7 +1536,8 @@ static int igc_ethtool_get_ts_info(struct net_device *dev,
 
 		info->tx_types =
 			BIT(HWTSTAMP_TX_OFF) |
-			BIT(HWTSTAMP_TX_ON);
+			BIT(HWTSTAMP_TX_ON) |
+			BIT(HWTSTAMP_TX_ONESTEP_SYNC);
 
 		info->rx_filters = BIT(HWTSTAMP_FILTER_NONE);
 		info->rx_filters |= BIT(HWTSTAMP_FILTER_ALL);
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index a5ebee7..713da04 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1260,6 +1260,9 @@ static int igc_tx_map(struct igc_ring *tx_ring,
 	cmd_type |= size | IGC_TXD_DCMD;
 	tx_desc->read.cmd_type_len = cpu_to_le32(cmd_type);
 
+	if (first->tx_flags & IGC_TX_FLAGS_ONESTEP_SYNC)
+		tx_desc->read.cmd_type_len |= IGC_ADVTXD_ONESTEP;
+
 	netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount);
 
 	/* set the timestamp */
@@ -1452,12 +1455,19 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 		 * the other timer registers before skipping the
 		 * timestamping request.
 		 */
-		if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ON &&
+
+		if (adapter->tstamp_config.tx_type >= HWTSTAMP_TX_ON &&
 		    !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;
 
+			if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ONESTEP_SYNC &&
+			    igc_ptp_process_onestep(adapter, skb)) {
+				tx_flags |= IGC_TX_FLAGS_ONESTEP_SYNC;
+				adapter->onestep_discard = true;
+			}
+
 			adapter->ptp_tx_skb = skb_get(skb);
 			adapter->ptp_tx_start = jiffies;
 		} else {
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 653e9f1..dc57339 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -21,6 +21,9 @@
 #define IGC_PTM_STAT_SLEEP		2
 #define IGC_PTM_STAT_TIMEOUT		100
 
+#define IGC_PTP_TX_ONESTEP_L2_OFFSET		48
+#define IGC_PTP_TX_ONESTEP_L2_IN_VLAN_OFFSET	52
+
 /* SYSTIM read access for I225 */
 void igc_ptp_read(struct igc_adapter *adapter, struct timespec64 *ts)
 {
@@ -550,6 +553,35 @@ static void igc_ptp_enable_tx_timestamp(struct igc_adapter *adapter)
 	rd32(IGC_TXSTMPH);
 }
 
+static void igc_ptp_update_1588_offset_in_tsynctxctl(struct igc_adapter *adapter, int ptp_class)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 onestep_offset = (rd32(IGC_TSYNCTXCTL) & 0xFF00) >> 8;
+
+	if ((ptp_class & PTP_CLASS_VLAN) &&
+	    onestep_offset != IGC_PTP_TX_ONESTEP_L2_IN_VLAN_OFFSET) {
+		wr32(IGC_TSYNCTXCTL, IGC_TSYNCTXCTL_ENABLED | IGC_TSYNCTXCTL_TXSYNSIG |
+				(IGC_PTP_TX_ONESTEP_L2_IN_VLAN_OFFSET << 8));
+	} else if (!(ptp_class & PTP_CLASS_VLAN) &&
+		   onestep_offset != IGC_PTP_TX_ONESTEP_L2_OFFSET) {
+		wr32(IGC_TSYNCTXCTL, IGC_TSYNCTXCTL_ENABLED | IGC_TSYNCTXCTL_TXSYNSIG |
+				(IGC_PTP_TX_ONESTEP_L2_OFFSET << 8));
+	}
+}
+
+bool igc_ptp_process_onestep(struct igc_adapter *adapter, struct sk_buff *skb)
+{
+	unsigned int ptp_class = ptp_classify_raw(skb);
+
+	if ((ptp_class & PTP_CLASS_L2) && (ptp_class & PTP_CLASS_V2)) {
+		if (ptp_msg_is_sync(skb, ptp_class)) {
+			igc_ptp_update_1588_offset_in_tsynctxctl(adapter, ptp_class);
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * igc_ptp_set_timestamp_mode - setup hardware for timestamping
  * @adapter: networking device structure
@@ -564,6 +596,7 @@ static int igc_ptp_set_timestamp_mode(struct igc_adapter *adapter,
 	case HWTSTAMP_TX_OFF:
 		igc_ptp_disable_tx_timestamp(adapter);
 		break;
+	case HWTSTAMP_TX_ONESTEP_SYNC:
 	case HWTSTAMP_TX_ON:
 		igc_ptp_enable_tx_timestamp(adapter);
 		break;
@@ -603,6 +636,9 @@ static void igc_ptp_tx_timeout(struct igc_adapter *adapter)
 {
 	struct igc_hw *hw = &adapter->hw;
 
+	if (adapter->onestep_discard)
+		adapter->onestep_discard = false;
+
 	dev_kfree_skb_any(adapter->ptp_tx_skb);
 	adapter->ptp_tx_skb = NULL;
 	adapter->tx_hwtstamp_timeouts++;
@@ -671,16 +707,14 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
 	shhwtstamps.hwtstamp =
 		ktime_add_ns(shhwtstamps.hwtstamp, adjust);
 
-	/* Clear the lock early before calling skb_tstamp_tx so that
-	 * applications are not woken up before the lock bit is clear. We use
-	 * a copy of the skb pointer to ensure other threads can't change it
-	 * while we're notifying the stack.
-	 */
+	if (adapter->onestep_discard)
+		adapter->onestep_discard = false;
+	else
+		skb_tstamp_tx(skb, &shhwtstamps);
+
 	adapter->ptp_tx_skb = NULL;
 	clear_bit_unlock(__IGC_PTP_TX_IN_PROGRESS, &adapter->state);
 
-	/* Notify the stack and free the skb after we've unlocked */
-	skb_tstamp_tx(skb, &shhwtstamps);
 	dev_kfree_skb_any(skb);
 }
 
@@ -959,6 +993,7 @@ void igc_ptp_init(struct igc_adapter *adapter)
 
 	adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
 	adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
+	adapter->onestep_discard = false;
 
 	adapter->prev_ptp_time = ktime_to_timespec64(ktime_get_real());
 	adapter->ptp_reset_start = ktime_get();
-- 
2.37.3