| Message ID | 1561500439-30276-2-git-send-email-vedang.patel@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net/sched: Add txtime-assist support for taprio. | expand |
On Tue, 2019-06-25 at 15:07 -0700, Vedang Patel wrote: > If a packet which is utilizing the launchtime feature (via SO_TXTIME > socket > option) also requests the hardware transmit timestamp, the hardware > timestamp is not delivered to the userspace. This is because the > value in > skb->tstamp is mistaken as the software timestamp. > > Applications, like ptp4l, request a hardware timestamp by setting the > SOF_TIMESTAMPING_TX_HARDWARE socket option. Whenever a new timestamp > is > detected by the driver (this work is done in igb_ptp_tx_work() which > calls > igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp > in the > ERR_QUEUE for the userspace to read. When the userspace is ready, it > will > issue a recvmsg() call to collect this timestamp. The problem is in > this > recvmsg() call. If the skb->tstamp is not cleared out, it will be > interpreted as a software timestamp and the hardware tx timestamp > will not > be successfully sent to the userspace. Look at skb_is_swtx_tstamp() > and the > callee function __sock_recv_timestamp() in net/socket.c for more > details. > > Signed-off-by: Vedang Patel <vedang.patel@intel.com> > --- > drivers/net/ethernet/intel/igb/igb_main.c | 1 + > 1 file changed, 1 insertion(+) Since this fix is really not needed for the rest of the patch series, if you have to do another version of the series, can you drop this patch from any future version? I don't want to keep spinning my validation team on a updated version of this patch, that is not really being updated. I plan to take this version of the patch and will hold onto it for my next 1GbE updates to Dave.
Hi Jeff, > On Jun 26, 2019, at 12:44 PM, Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> wrote: > > On Tue, 2019-06-25 at 15:07 -0700, Vedang Patel wrote: >> If a packet which is utilizing the launchtime feature (via SO_TXTIME >> socket >> option) also requests the hardware transmit timestamp, the hardware >> timestamp is not delivered to the userspace. This is because the >> value in >> skb->tstamp is mistaken as the software timestamp. >> >> Applications, like ptp4l, request a hardware timestamp by setting the >> SOF_TIMESTAMPING_TX_HARDWARE socket option. Whenever a new timestamp >> is >> detected by the driver (this work is done in igb_ptp_tx_work() which >> calls >> igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp >> in the >> ERR_QUEUE for the userspace to read. When the userspace is ready, it >> will >> issue a recvmsg() call to collect this timestamp. The problem is in >> this >> recvmsg() call. If the skb->tstamp is not cleared out, it will be >> interpreted as a software timestamp and the hardware tx timestamp >> will not >> be successfully sent to the userspace. Look at skb_is_swtx_tstamp() >> and the >> callee function __sock_recv_timestamp() in net/socket.c for more >> details. >> >> Signed-off-by: Vedang Patel <vedang.patel@intel.com> >> --- >> drivers/net/ethernet/intel/igb/igb_main.c | 1 + >> 1 file changed, 1 insertion(+) > > Since this fix is really not needed for the rest of the patch series, > if you have to do another version of the series, can you drop this > patch from any future version? I don't want to keep spinning my > validation team on a updated version of this patch, that is not really > being updated. > > I plan to take this version of the patch and will hold onto it for my > next 1GbE updates to Dave. This patch is needed for ptp4l to function properly when txtime-assist mode is enabled. So, dropping this patch will break the series. When are you planning to submit the next set of updates to Dave? If a new version is needed, I can plan to send it out after you send your updates. Thanks, Vedang
> From: Patel, Vedang > Sent: Tuesday, June 25, 2019 3:07 PM > To: netdev@vger.kernel.org > Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net; > jhs@mojatatu.com; xiyou.wangcong@gmail.com; jiri@resnulli.us; intel-wired- > lan@lists.osuosl.org; Gomes, Vinicius <vinicius.gomes@intel.com>; > l@dorileo.org; jakub.kicinski@netronome.com; m-karicheri2@ti.com; > sergei.shtylyov@cogentembedded.com; eric.dumazet@gmail.com; Brown, > Aaron F <aaron.f.brown@intel.com>; Patel, Vedang <vedang.patel@intel.com> > Subject: [PATCH net-next v6 1/8] igb: clear out skb->tstamp after reading the > txtime > > If a packet which is utilizing the launchtime feature (via SO_TXTIME socket > option) also requests the hardware transmit timestamp, the hardware > timestamp is not delivered to the userspace. This is because the value in > skb->tstamp is mistaken as the software timestamp. > > Applications, like ptp4l, request a hardware timestamp by setting the > SOF_TIMESTAMPING_TX_HARDWARE socket option. Whenever a new > timestamp is > detected by the driver (this work is done in igb_ptp_tx_work() which calls > igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp in the > ERR_QUEUE for the userspace to read. When the userspace is ready, it will > issue a recvmsg() call to collect this timestamp. The problem is in this > recvmsg() call. If the skb->tstamp is not cleared out, it will be > interpreted as a software timestamp and the hardware tx timestamp will not > be successfully sent to the userspace. Look at skb_is_swtx_tstamp() and the > callee function __sock_recv_timestamp() in net/socket.c for more details. > > Signed-off-by: Vedang Patel <vedang.patel@intel.com> > --- > drivers/net/ethernet/intel/igb/igb_main.c | 1 + > 1 file changed, 1 insertion(+) > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index fc925adbd9fa..f66dae72fe37 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -5688,6 +5688,7 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring, */ if (tx_ring->launchtime_enable) { ts = ns_to_timespec64(first->skb->tstamp); + first->skb->tstamp = 0; context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32); } else { context_desc->seqnum_seed = 0;
If a packet which is utilizing the launchtime feature (via SO_TXTIME socket option) also requests the hardware transmit timestamp, the hardware timestamp is not delivered to the userspace. This is because the value in skb->tstamp is mistaken as the software timestamp. Applications, like ptp4l, request a hardware timestamp by setting the SOF_TIMESTAMPING_TX_HARDWARE socket option. Whenever a new timestamp is detected by the driver (this work is done in igb_ptp_tx_work() which calls igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp in the ERR_QUEUE for the userspace to read. When the userspace is ready, it will issue a recvmsg() call to collect this timestamp. The problem is in this recvmsg() call. If the skb->tstamp is not cleared out, it will be interpreted as a software timestamp and the hardware tx timestamp will not be successfully sent to the userspace. Look at skb_is_swtx_tstamp() and the callee function __sock_recv_timestamp() in net/socket.c for more details. Signed-off-by: Vedang Patel <vedang.patel@intel.com> --- drivers/net/ethernet/intel/igb/igb_main.c | 1 + 1 file changed, 1 insertion(+)