[PATCH] net/e1000: support launchtime feature

2023-12-17 Thread Chuanyu Xue
Enable the time-based scheduled Tx of packets based on the
RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP flag. The launchtime defines the
packet transmission time based on PTP clock at MAC layer, which should
be set to the advanced transmit descriptor.

Signed-off-by: Chuanyu Xue 
---
 drivers/net/e1000/base/e1000_regs.h |  1 +
 drivers/net/e1000/e1000_ethdev.h|  3 ++
 drivers/net/e1000/igb_ethdev.c  | 28 ++
 drivers/net/e1000/igb_rxtx.c| 44 -
 4 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index d44de59c29..092d9d71e6 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -162,6 +162,7 @@
 
 /* QAV Tx mode control register */
 #define E1000_I210_TQAVCTRL0x3570
+#define E1000_I210_LAUNCH_OS0 0x3578
 
 /* QAV Tx mode control register bitfields masks */
 /* QAV enable */
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 718a9746ed..174f7aaf52 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -382,6 +382,9 @@ extern struct igb_rss_filter_list igb_filter_rss_list;
 TAILQ_HEAD(igb_flow_mem_list, igb_flow_mem);
 extern struct igb_flow_mem_list igb_flow_list;
 
+extern uint64_t igb_tx_timestamp_dynflag;
+extern int igb_tx_timestamp_dynfield_offset;
+
 extern const struct rte_flow_ops igb_flow_ops;
 
 /*
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8..4d3d8ae30a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -223,6 +223,7 @@ static int igb_timesync_read_time(struct rte_eth_dev *dev,
  struct timespec *timestamp);
 static int igb_timesync_write_time(struct rte_eth_dev *dev,
   const struct timespec *timestamp);
+static int eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t 
*clock);
 static int eth_igb_rx_queue_intr_enable(struct rte_eth_dev *dev,
uint16_t queue_id);
 static int eth_igb_rx_queue_intr_disable(struct rte_eth_dev *dev,
@@ -313,6 +314,9 @@ static const struct rte_pci_id pci_id_igbvf_map[] = {
{ .vendor_id = 0, /* sentinel */ },
 };
 
+uint64_t igb_tx_timestamp_dynflag;
+int igb_tx_timestamp_dynfield_offset = -1;
+
 static const struct rte_eth_desc_lim rx_desc_lim = {
.nb_max = E1000_MAX_RING_DESC,
.nb_min = E1000_MIN_RING_DESC,
@@ -389,6 +393,7 @@ static const struct eth_dev_ops eth_igb_ops = {
.timesync_adjust_time = igb_timesync_adjust_time,
.timesync_read_time   = igb_timesync_read_time,
.timesync_write_time  = igb_timesync_write_time,
+   .read_clock   = eth_igb_read_clock,
 };
 
 /*
@@ -1198,6 +1203,7 @@ eth_igb_start(struct rte_eth_dev *dev)
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
int ret, mask;
+   uint32_t tqavctrl;
uint32_t intr_vector = 0;
uint32_t ctrl_ext;
uint32_t *speeds;
@@ -1281,6 +1287,15 @@ eth_igb_start(struct rte_eth_dev *dev)
return ret;
}
 
+   if (igb_tx_timestamp_dynflag > 0) {
+   tqavctrl = E1000_READ_REG(hw, E1000_I210_TQAVCTRL);
+   tqavctrl |= E1000_TQAVCTRL_MODE;
+   tqavctrl |= E1000_TQAVCTRL_FETCH_ARB; /* Fetch the queue most 
empty, no Round Robin*/
+   tqavctrl |= E1000_TQAVCTRL_LAUNCH_TIMER_ENABLE; /* Enable 
launch time */
+   E1000_WRITE_REG(hw, E1000_I210_TQAVCTRL, tqavctrl);
+   E1000_WRITE_REG(hw, E1000_I210_LAUNCH_OS0, 1ULL << 31); /* Set 
launch offset to default */
+   }
+
e1000_clear_hw_cntrs_base_generic(hw);
 
/*
@@ -4882,6 +4897,19 @@ igb_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
return  0;
 }
 
+static int
+eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t *clock)
+{
+   uint64_t systime_cycles;
+   struct e1000_adapter *adapter = dev->data->dev_private;
+
+   systime_cycles = igb_read_systime_cyclecounter(dev);
+   uint64_t ns = rte_timecounter_update(&adapter->systime_tc, 
systime_cycles);
+   *clock = ns;
+
+   return 0;
+}
+
 static int
 eth_igb_get_reg_length(struct rte_eth_dev *dev __rte_unused)
 {
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 448c4b7d9d..e5da8e250d 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -212,6 +212,9 @@ struct igb_tx_queue {
 #define IGB_TSO_MAX_HDRLEN (512)
 #define IGB_TSO_MAX_MSS(9216)
 
+/* Macro to compensate latency in launch time offloading*/
+#define E1000_I210_LT_LATENCY  0x41F9
+
 /

RE: [PATCH] net/e1000: support launchtime feature

2023-12-21 Thread Chuanyu Xue
Hi Simei,
Thank you so much for your review.

>> 
>>  /* QAV Tx mode control register */
>>  #define E1000_I210_TQAVCTRL 0x3570
>> +#define E1000_I210_LAUNCH_OS0 0x3578
>
>What does this register mean?
>

"LAUNCH_OS0" is defined as LaunchOffset register, which sets the base time
for launchtime. Based on i210 datasheet V3.7 Sec 7.2.2.2.3, the actual launch
time is computed as 32 * (LaunchOffset + LaunchTime). In this context, the
register is used to set the LaunchOffset later as 0. 

>> 
>> +if (igb_tx_timestamp_dynflag > 0) {
>> +tqavctrl = E1000_READ_REG(hw, E1000_I210_TQAVCTRL);
>> +tqavctrl |= E1000_TQAVCTRL_MODE;
>> +tqavctrl |= E1000_TQAVCTRL_FETCH_ARB; /* Fetch the queue most
>> empty, no Round Robin*/
>> +tqavctrl |= E1000_TQAVCTRL_LAUNCH_TIMER_ENABLE; /* Enable
>> launch time */
>
> In kernel driver, "E1000_TQAVCTRL_DATATRANTIM (BIT(9))" and
> "E1000_TQAVCTRL_FETCHTIME_DELTA (0x << 16)" are set, does it have some
> other intention here?

"E1000_TQAVCTRL_DATATRANTIM" is same as "E1000_TQAVCTRL_LAUNCH_TIMER_ENABLE"

"E1000_TQAVCTRL_FETCHTIME_DELTA" maximizes the data fetch time.
If "E1000_TQAVCTRL_FETCH_ARB" is set, there is no need to set this field,
because the arbitrary fetching prioritizes the most empty queue, regardless
of the fetch time. (referring Sec 7.2.7.5) 

I have also tested aligning with the kernel driver settings using (0x << 
16) 
and omitting 'E1000_TQAVCTRL_FETCH_ARB', the launchtime feature also worked
as expected. However, the arbitrary fetch mode seems more suitable 
as DPDK lacks an interface to set fetch delay, unlike in the kernel which can 
be configured (e.g., through 'Delta' in ETF Qdisc). Any suggestions here?

>> +static int
>> +eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t
>> +*clock) {
>> +uint64_t systime_cycles;
>> +struct e1000_adapter *adapter = dev->data->dev_private;
>> +
>> +systime_cycles = igb_read_systime_cyclecounter(dev);
>> +uint64_t ns = rte_timecounter_update(&adapter->systime_tc,
>> systime_cycles);
>
>Do you also run "ptp timesync" when testing this launchtime feature?
>

I used `rte_eth_timesync_enable` function during the test. I am not familiar 
with the `ptp timesync` in DPDK. If you are referring to something
else, could you please guide me on how to test it?

>> 
>> +/* Macro to compensate latency in launch time offloading*/
>> +#define E1000_I210_LT_LATENCY   0x41F9
>
>What does this value depend on? 
>

Through my test, I observed a constant latency between the launchtime
and the actual Tx time measured by the `rte_eth_timesync_read_tx_timestamp` 
function.
I didn't find a description of this latency in the datasheet.

In my test, the latency appears to be relative to the data rate, and 
independent from the packet size and throughput. The latency slightly changed 
in different experiments, but in each experiment, it remained constant for all 
the Tx packets.
I also tested this latency consistently on two different NICs (I210 GE-1T-X1, 
I210 X1-V2).

Here are some measurement results (in ns):

+---+---+---+---+---+---+
| Data Rate | Measurement 1 | Measurement 2 | Measurement 3 | Measurement 4 | 
Measurement 5 |
+---+---+---+---+---+---+
| 10M   | 14400 | 14544 | 14384 | 14896 | 
14336 |
+---+---+---+---+---+---+
| 100M  | 31016 | 31016 | 31000 | 31000 | 
31048 |
+---+---+---+---+---+---+
| 1G| 16880 | 16880 | 16880 | 16880 | 
16880 |
+---+---+---+---+---+---+

Any suggestions here? Is it supposed to be embedded directly here or left to 
the 
application level to compensate? I can fix it accordingly.

- Chuanyu


RE: [PATCH] net/e1000: support launchtime feature

2023-12-29 Thread Chuanyu Xue
>> 
>> >> +static int
>> >> +eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t
>> >> +*clock) {
>> >> + uint64_t systime_cycles;
>> >> + struct e1000_adapter *adapter = dev->data->dev_private;
>> >> +
>> >> + systime_cycles = igb_read_systime_cyclecounter(dev);
>> >> + uint64_t ns = rte_timecounter_update(&adapter->systime_tc,
>> >> systime_cycles);
>> >
>> >Do you also run "ptp timesync" when testing this launchtime feature?
>> >
>> 
>> I used `rte_eth_timesync_enable` function during the test. I am not familiar
>> with the `ptp timesync` in DPDK. If you are referring to something else, 
>> could
>> you please guide me on how to test it?
>
>Do you use your own application or DPDK application to test this launchtime 
>feature,
>for example, dpdk testpmd?

Yes, I used my own application to test it. The benefit of launch time feature
in boundable delay and jitter is significant compared with when it is disabled. 

Specifically, my app periodically calls `rte_eth_tx_burst` with 
`rte_dynfield_timestamp` 
field on talker, and compares whether the receiving time in NIC hardware 
timestamp 
on listener is as expected. Talker and listener are directly connected by a 
RJ45 cable,
both installed with i210 NIC. The feature works perfect in my test.

I also tested it with testpmd with `txtimes` config. However it seems there is 
an issue 
in testpmd. Specifically the tx_only mode sends packets as fast as possible, 
results 
in an increasing gap between the current time and the scheduled transmission 
time.
Based on i210 NIC sheet Sec 7.2.2.2.3, the launch time should be within 
(current_time, current time + 0.5 Sec), thus most of tx packets are not 
scheduled. 
I got the similar test results with dpdk igc driver which already implemeted 
launch 
time feature.

Following is how I try to test with testpmd. Please let me know if I did 
something wrong.

sudo ./dpdk-testpmd -- -i --forward-mode=txonly

testpmd> port stop 0
testpmd> set burst 1
testpmd> set txtimes 1,0
testpmd> port config 0 tx_offload send_on_timestamp on
testpmd> port start 0
testpmd> start

>
>> +---+---+---+---+---+---+
>> | 1G| 16880 | 16880 | 16880 | 16880
>> | 16880 |
>> +---+---+---+---+---+---+
>> 
>> Any suggestions here? Is it supposed to be embedded directly here or left to
>> the application level to compensate? I can fix it accordingly.
>
>I think it can be put here directly just as you do.

Got it. Will keep this delay compensiation here and revise it in the next batch 
version.


[PATCH v2] net/e1000: support launchtime feature

2023-12-30 Thread Chuanyu Xue
Enable the time-based scheduled Tx of packets based on the
RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP flag. The launchtime defines the
packet transmission time based on PTP clock at MAC layer, which should
be set to the advanced transmit descriptor.

Signed-off-by: Chuanyu Xue 
---
change log:

v2:
- Add delay compensation for i210 NIC by setting tx offset register.
- Revise read_clock function.

 drivers/net/e1000/base/e1000_regs.h |  1 +
 drivers/net/e1000/e1000_ethdev.h| 14 +++
 drivers/net/e1000/igb_ethdev.c  | 63 -
 drivers/net/e1000/igb_rxtx.c| 42 +++
 4 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_regs.h 
b/drivers/net/e1000/base/e1000_regs.h
index d44de59c29..092d9d71e6 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -162,6 +162,7 @@
 
 /* QAV Tx mode control register */
 #define E1000_I210_TQAVCTRL0x3570
+#define E1000_I210_LAUNCH_OS0 0x3578
 
 /* QAV Tx mode control register bitfields masks */
 /* QAV enable */
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 718a9746ed..339ae1f4b6 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -382,6 +382,20 @@ extern struct igb_rss_filter_list igb_filter_rss_list;
 TAILQ_HEAD(igb_flow_mem_list, igb_flow_mem);
 extern struct igb_flow_mem_list igb_flow_list;
 
+/*
+ * Macros to compensate the constant latency observed in i210 for launch time
+ *
+ * launch time = (offset_speed - offset_base + txtime) * 32
+ * offset_speed is speed dependent, set in E1000_I210_LAUNCH_OS0
+ */
+#define IGB_I210_TX_OFFSET_BASE0xffe0
+#define IGB_I210_TX_OFFSET_SPEED_100xc7a0
+#define IGB_I210_TX_OFFSET_SPEED_100   0x86e0
+#define IGB_I210_TX_OFFSET_SPEED_1000  0xbe00
+
+extern uint64_t igb_tx_timestamp_dynflag;
+extern int igb_tx_timestamp_dynfield_offset;
+
 extern const struct rte_flow_ops igb_flow_ops;
 
 /*
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8..2262035710 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -223,6 +223,7 @@ static int igb_timesync_read_time(struct rte_eth_dev *dev,
  struct timespec *timestamp);
 static int igb_timesync_write_time(struct rte_eth_dev *dev,
   const struct timespec *timestamp);
+static int eth_igb_read_clock(struct rte_eth_dev *dev, uint64_t *clock);
 static int eth_igb_rx_queue_intr_enable(struct rte_eth_dev *dev,
uint16_t queue_id);
 static int eth_igb_rx_queue_intr_disable(struct rte_eth_dev *dev,
@@ -313,6 +314,9 @@ static const struct rte_pci_id pci_id_igbvf_map[] = {
{ .vendor_id = 0, /* sentinel */ },
 };
 
+uint64_t igb_tx_timestamp_dynflag;
+int igb_tx_timestamp_dynfield_offset = -1;
+
 static const struct rte_eth_desc_lim rx_desc_lim = {
.nb_max = E1000_MAX_RING_DESC,
.nb_min = E1000_MIN_RING_DESC,
@@ -389,6 +393,7 @@ static const struct eth_dev_ops eth_igb_ops = {
.timesync_adjust_time = igb_timesync_adjust_time,
.timesync_read_time   = igb_timesync_read_time,
.timesync_write_time  = igb_timesync_write_time,
+   .read_clock   = eth_igb_read_clock,
 };
 
 /*
@@ -1188,6 +1193,40 @@ eth_igb_rxtx_control(struct rte_eth_dev *dev,
E1000_WRITE_FLUSH(hw);
 }
 
+
+static uint32_t igb_tx_offset(struct rte_eth_dev *dev)
+{
+   struct e1000_hw *hw =
+   E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+   uint16_t duplex, speed;
+   hw->mac.ops.get_link_up_info(hw, &speed, &duplex);
+
+   uint32_t launch_os0 = E1000_READ_REG(hw, E1000_I210_LAUNCH_OS0);
+   if (hw->mac.type != e1000_i210) {
+   /* Set launch offset to base, no compensation */
+   launch_os0 |= IGB_I210_TX_OFFSET_BASE;
+   } else {
+   /* Set launch offset depend on link speeds */
+   switch (speed) {
+   case SPEED_10:
+   launch_os0 |= IGB_I210_TX_OFFSET_SPEED_10;
+   break;
+   case SPEED_100:
+   launch_os0 |= IGB_I210_TX_OFFSET_SPEED_100;
+   break;
+   case SPEED_1000:
+   launch_os0 |= IGB_I210_TX_OFFSET_SPEED_1000;
+   break;
+   default:
+   launch_os0 |= IGB_I210_TX_OFFSET_BASE;
+   break;
+   }
+   }
+
+   return launch_os0;
+}
+
 static int
 eth_igb_start(struct rte_eth_dev *dev)
 {
@@ -1198,6 +1237,7 @@ eth_igb_start(struct rte_eth_dev *dev)
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = pci_dev->intr_handle

RE: [PATCH] net/e1000: support launchtime feature

2024-01-03 Thread Chuanyu Xue
Hi, Simei

Thank you for your guidance on how to test this feature.

>> Following is how I try to test with testpmd. Please let me know if I did
>> something wrong.
>> 
>>   sudo ./dpdk-testpmd -- -i --forward-mode=txonly
>> 
>>   testpmd> port stop 0
>>   testpmd> set burst 1
>>   testpmd> set txtimes 1,0
>>   testpmd> port config 0 tx_offload send_on_timestamp on
>>   testpmd> port start 0
>>   testpmd> start
>
>When testing launch time feature with igc driver, firstly, some code change 
>made in txonly.c:
>pkt->ol_flags |= RTE_MBUF_F_TX_IEEE1588_TMST; (this flag should be added to 
>forward PTP packet with hardware Tx timestamp)
>
># ./build/app/dpdk-testpmd -a :81:00.0 -c f -n 4 -- -i 
>--tx-offloads=0x20
>testpmd> set burst 1
>testpmd> set fwd txonly
>testpmd> set txtimes 100,0
>testpmd> start
>
>On receiver side (with tcpdump):
># tcpdump -Q in -ttt -ni ens25f3 --time-stamp-precision=nano -j 
>adapter_unsynced -c 32

Now dpdk-testpmd works well with this patch after I add the flag in txonly.c 
as you mentioned. 

It is worth noting that I also added `rte_eth_timesync_enable(pi);` in the 
function `tx_only_begin` in txonly.c to enable the PTP clock. Otherwise, all Tx
packets scheduled are dropped.

Following are the measurement results on the listener. I use the same 
configuration
as you mentioned for dpdk-testpmd on the talker.

➜  ~ sudo tcpdump -Q in -ttt -ni enp1s0 --time-stamp-precision=nano -j 
adapter_unsynced -c 32

tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on enp1s0, link-type EN10MB (Ethernet), snapshot length 262144 
bytes


 00:00:00.0 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00108 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00108 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
 00:00:00.00100 IP 198.18.0.1.9 > 198.18.0.2.9: UDP, length 22
32 packets captured
118 packets received by filter
0 packets dropped by kernel

Above test is based on the patch v2 with Intel i210 NIC.

- Chuanyu