On 6/20/19 9:49 AM, Patel, Vedang wrote:
>
>
>> On Jun 20, 2019, at 3:47 AM, Eric Dumazet <eric.duma...@gmail.com> wrote:
>>
>>
>>
>> On 6/19/19 10:40 AM, Vedang Patel wrote:
>>> skb->tstamp is being used at multiple places. On the transmit side, it
>>> is used to determine the launchtime of the packet. It is also used to
>>> determine the software timestamp after the packet has been transmitted.
>>>
>>> So, clear out the tstamp value after it has been read so that we do not
>>> report false software timestamp on the receive side.
>>>
>>> Signed-off-by: Vedang Patel <vedang.pa...@intel.com>
>>> ---
>>> drivers/net/ethernet/intel/igb/igb_main.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> 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;
>>
>> Please provide more explanations.
>>
>> Why only this driver would need this ?
>>
> Currently, igb is the only driver which uses the skb->tstamp option on the
> transmit side (to set the hardware transmit timestamp). All the other drivers
> only use it on the receive side (to collect and send the hardware transmit
> timestamp to the userspace after packet has been sent).
>
> So, any driver which supports the hardware txtime in the future will have to
> clear skb->tstamp to make sure that hardware tx transmit and tx timestamping
> can be done on the same packet.
The changelog is rather confusing :
"So, clear out the tstamp value after it has been read so that we do not
report false software timestamp on the receive side."
I have hard time understanding why sending an skb through this driver
could cause a problem on receive side ?
I suggest to rephrase it to clear the confusion.
>
> Thanks,
> Vedang
>>
>>> context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32);
>>> } else {
>>> context_desc->seqnum_seed = 0;
>>>
>
