Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Keller, Jacob E
> -Original Message-
> From: Richard Cochran 
> Sent: Wednesday, July 14, 2021 6:12 AM
> To: Keller, Jacob E 
> Cc: Miroslav Lichvar ; linuxptp-
> de...@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> On Wed, Jul 14, 2021 at 11:20:00AM +, Keller, Jacob E wrote:
> 
> > I think for Tx the challenges are higher: the timestamp is taken
> > after we've filled in the descriptor and sent the frame. The only
> > place it could reasonably be stored again is the descriptor
> > writeback (since we don't get completion messages).
> 
> Right, the would be the place to do it.
> 
> > If I remember correctly, the challenge here is that in a traditional
> > ring model the writeback is completed much earlier than the
> > timestamp so we potentially delay cleanup of other packets by
> > waiting to insert the timestamp into the writeback.
> 
> If *every* frame gets a time stamp, then their write-backs would all
> be delayed by the same amount.  Hence no clean up operations would be
> "delayed".  They would all take the same amount of time.
> 
> The only cost would be in space to keep the data for the write-back
> around until the time stamp becomes available.  Paying the price of
> the little extra memory is well worth it, as it simplifies the time
> stamping logic and removes every class of problem related to time
> stamp delivery.
> 
> IOW, KISS!
> 
> Thanks,
> Richard

Yea, if you timestamp every frame regardless of whether kernel requested it or 
not. Makes sense to me.

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Keller, Jacob E



> -Original Message-
> From: Richard Cochran 
> Sent: Wednesday, July 14, 2021 6:03 AM
> To: Keller, Jacob E 
> Cc: linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> On Wed, Jul 14, 2021 at 11:20:23AM +, Keller, Jacob E wrote:
> > What about at least checking for the case where a timestamp was never
> > started? Drivers are supposed to set a flag in the SKB when they start a
> > timestamp (and not set it if they can't start it).
> 
> How could that happen?
> 
> Putting aside egregious driver bugs, it is hard for me to imagine a
> use case that would cause this failure mode.
> 
> Thanks,
> Richard

It only matters for hardware which can only handle 1 timestamp at a time, and 
only in the case where either an application isn't waiting for timestamps, or 
for when multiple applications try using it at once I guess.

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Richard Cochran
On Wed, Jul 14, 2021 at 11:20:00AM +, Keller, Jacob E wrote:

> I think for Tx the challenges are higher: the timestamp is taken
> after we've filled in the descriptor and sent the frame. The only
> place it could reasonably be stored again is the descriptor
> writeback (since we don't get completion messages).

Right, the would be the place to do it.

> If I remember correctly, the challenge here is that in a traditional
> ring model the writeback is completed much earlier than the
> timestamp so we potentially delay cleanup of other packets by
> waiting to insert the timestamp into the writeback.

If *every* frame gets a time stamp, then their write-backs would all
be delayed by the same amount.  Hence no clean up operations would be
"delayed".  They would all take the same amount of time.

The only cost would be in space to keep the data for the write-back
around until the time stamp becomes available.  Paying the price of
the little extra memory is well worth it, as it simplifies the time
stamping logic and removes every class of problem related to time
stamp delivery.

IOW, KISS!

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Richard Cochran
On Wed, Jul 14, 2021 at 11:20:23AM +, Keller, Jacob E wrote:
> What about at least checking for the case where a timestamp was never
> started? Drivers are supposed to set a flag in the SKB when they start a
> timestamp (and not set it if they can't start it).

How could that happen?

Putting aside egregious driver bugs, it is hard for me to imagine a
use case that would cause this failure mode.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Keller, Jacob E



> -Original Message-
> From: Richard Cochran 
> Sent: Monday, July 12, 2021 6:44 PM
> To: Keller, Jacob E 
> Cc: Miroslav Lichvar ; linuxptp-
> de...@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> On Mon, Jul 12, 2021 at 03:02:50PM +, Keller, Jacob E wrote:
> 
> > Right. Though.. running something like ptp4l on the same connection
> > could be problematic if the applications aren't working together
> > because most hardware supports a single request at once,
> 
> I wouldn't say "most".  Surely some HW is like that, but I never
> counted.  I always hoped that HW designers would get a clue a simply
> provide a time stamp for each and every frame, Tx and Rx, in the
> buffer descriptors.  No filters, no parsers, just a big on/off switch.
> It would be way easier to implement in HW, and it would solve all of
> these sorts of problems.
> 

The hardware I've seen at least. (Ofcourse, I suppose I am Intel biased here...)

I think there's two issues here:

For receive, I think the issue was a belief that the cost in bytes to store the 
timestamp would be too high. This turns out to either be not true, or not 
important because the demand for useful timestamps is high enough to account 
for it. (Newer hardware has opted to simply add timestamping for all frames).

I think for Tx the challenges are higher: the timestamp is taken after we've 
filled in the descriptor and sent the frame. The only place it could reasonably 
be stored again is the descriptor writeback (since we don't get completion 
messages). If I remember correctly, the challenge here is that in a traditional 
ring model the writeback is completed much earlier than the timestamp so we 
potentially delay cleanup of other packets by waiting to insert the timestamp 
into the writeback.

I'm not sure entirely what all the complexity is here, but I know it's not as 
simple as the receive side where we already have the timestamp data when 
filling in the receive descriptor.

I imagine if we used a completion model where Tx completions have their own 
queue it wouldn't be as much of a problem. I don't know for sure though.

> > so if both
> > applications send a request at the same time one of them will
> > fail. They would need to either ensure they're off-sync or be
> > communicating to each other about when its ok to timestamp request
> > somehow.
> 
> Oh, that will make the users of the new PHC vclock thing happy!  I can
> already hear the complaints and bug reports here on our lists...
> 
> Thanks,
> Richard

At least with ice we support up to 64 timestamps outstanding, so it's less of 
an issue there

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-14 Thread Keller, Jacob E
On 7/12/2021 6:36 PM, Richard Cochran wrote:
> On Mon, Jul 12, 2021 at 05:02:58PM -0700, Vinicius Costa Gomes wrote:
>> Speaking of future improvements, wouldn't it be easier if the
>> kernel/driver was able to notify userspace that a timestamping request
>> wasn't able to be serviced?
> 
> It would fall to the drivers to implement that correctly.  I doubt the
> situation would improve.  We'd only end up chasing another class of
> bugs.
> 
> Thanks,
> Richard
> 

What about at least checking for the case where a timestamp was never
started? Drivers are supposed to set a flag in the SKB when they start a
timestamp (and not set it if they can't start it).

This could be done primarily in the core stack to send back an error of
a packet had a timestamp request but the request didn't get started?

This isn't going to solve the case where a timestamp went missing, of
course, but it would solve the case of "I can't start a timestamp while one is 
already in progress"

___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Richard Cochran
On Mon, Jul 12, 2021 at 03:02:50PM +, Keller, Jacob E wrote:

> Right. Though.. running something like ptp4l on the same connection
> could be problematic if the applications aren't working together
> because most hardware supports a single request at once,

I wouldn't say "most".  Surely some HW is like that, but I never
counted.  I always hoped that HW designers would get a clue a simply
provide a time stamp for each and every frame, Tx and Rx, in the
buffer descriptors.  No filters, no parsers, just a big on/off switch.
It would be way easier to implement in HW, and it would solve all of
these sorts of problems.

> so if both
> applications send a request at the same time one of them will
> fail. They would need to either ensure they're off-sync or be
> communicating to each other about when its ok to timestamp request
> somehow.

Oh, that will make the users of the new PHC vclock thing happy!  I can
already hear the complaints and bug reports here on our lists...

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Richard Cochran
On Mon, Jul 12, 2021 at 05:02:58PM -0700, Vinicius Costa Gomes wrote:
> Speaking of future improvements, wouldn't it be easier if the
> kernel/driver was able to notify userspace that a timestamping request
> wasn't able to be serviced?

It would fall to the drivers to implement that correctly.  I doubt the
situation would improve.  We'd only end up chasing another class of
bugs.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Vinicius Costa Gomes
Hi,

Miroslav Lichvar  writes:

> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
>> If the timestamp is available in less than the timeout (5ms) will it still 
>> wait for the timeout, or continue processing after the timestamp is received?
>
> The poll() call is waiting for the descriptor, so it should return as
> soon as the timestamp is ready. The option sets the maximum time it
> waits.
>
> I'm ok with increasing the default timeout.
>
> As a future improvement, maybe it could be adaptive, e.g. once in a
> while try waiting much longer and if that doesn't give a timestamp
> stick to a shorter interval. That is, try to detect when the hardware
> is not able to timestamp all packets.

Speaking of future improvements, wouldn't it be easier if the
kernel/driver was able to notify userspace that a timestamping request
wasn't able to be serviced?

I am thinking of sending an error via the socket error queue.

I know this won't improve the situation for current kernels, but
something like this might be worth thinking about for the future.

>
> -- 
> Miroslav Lichvar
>
>
>
> ___
> Linuxptp-devel mailing list
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Cheers,
-- 
Vinicius


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Keller, Jacob E



> -Original Message-
> From: Miroslav Lichvar 
> Sent: Monday, July 12, 2021 12:35 AM
> To: Keller, Jacob E 
> Cc: Eric Decker ; linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> On Thu, Jul 08, 2021 at 07:35:25PM +, Keller, Jacob E wrote:
> > > As a future improvement, maybe it could be adaptive, e.g. once in a
> > > while try waiting much longer and if that doesn't give a timestamp
> > > stick to a shorter interval. That is, try to detect when the hardware
> > > is not able to timestamp all packets.
> > >
> >
> > Not sure I follow here. I guess we'd default to a long timeout and 
> > periodically
> try shorter ones? I'm not sure this would be effective. I think the 
> complexity isn't
> really worth it.
> 
> That's another way to look at it. The idea is to estimate something
> like the 99th percentile of the delay to maximize the performance
> instead of wasting time waiting for a timestamp that is unlikely to
> come. The main use case where it could help is multiple applications
> doing TX timestamping on the same interface, e.g. a PTP server and
> client running in different domains.
> 
> Just an idea for future improvement.
> 
> --
> Miroslav Lichvar

Right. Though.. running something like ptp4l on the same connection could be 
problematic if the applications aren't working together because most hardware 
supports a single request at once, so if both applications send a request at 
the same time one of them will fail. They would need to either ensure they're 
off-sync or be communicating to each other about when its ok to timestamp 
request somehow.

I do like the idea of estimating the 99% percentile over time and adjusting 
delay

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-12 Thread Miroslav Lichvar
On Thu, Jul 08, 2021 at 07:35:25PM +, Keller, Jacob E wrote:
> > As a future improvement, maybe it could be adaptive, e.g. once in a
> > while try waiting much longer and if that doesn't give a timestamp
> > stick to a shorter interval. That is, try to detect when the hardware
> > is not able to timestamp all packets.
> > 
> 
> Not sure I follow here. I guess we'd default to a long timeout and 
> periodically try shorter ones? I'm not sure this would be effective. I think 
> the complexity isn't really worth it.

That's another way to look at it. The idea is to estimate something
like the 99th percentile of the delay to maximize the performance
instead of wasting time waiting for a timestamp that is unlikely to
come. The main use case where it could help is multiple applications
doing TX timestamping on the same interface, e.g. a PTP server and
client running in different domains.

Just an idea for future improvement.

-- 
Miroslav Lichvar



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-09 Thread Geva, Erez
I do not see any connection of pmc poll time-out to ptp4l waiting for TX time 
stamp.

On my libpmc https://sf.net/p/libpmc I do not use any time out.
The libpmc pmc tools works better this way.

Erez

-Original Message-
From: Eric Decker  
Sent: Friday, 9 July 2021 00:28
To: Richard Cochran ; Miroslav Lichvar 

Cc: linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

If I recall correctly there is an unconditional 150ms delay in PMC which also 
uses poll().  That I why I asked the question.  The delay in PMC may be related 
to how the firmware is structured, not poll().  I am using Linux 4.xx.

Eric Decker

-Original Message-
From: Richard Cochran 
Sent: Thursday, July 8, 2021 1:42 PM
To: Miroslav Lichvar 
Cc: Eric Decker ; linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for the timeout, or continue processing after the timestamp is 
> > received?
> 
> The poll() call is waiting for the descriptor, so it should return as 
> soon as the timestamp is ready. The option sets the maximum time it 
> waits.

But on kernels older than (mumble) (3.5?) the code will sleep the entire 
period, then wake to read the time stamp.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Flinuxptp-devel&data=04%7C01%7Cerez.geva.ext%40siemens.com%7C1d784133383c4cd4e2b408d9425fe84b%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637613801951566481%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9D4HZAuH9zExpzT86tfZjXtO4%2FQ%2B6ednYqty4SnbVAQ%3D&reserved=0


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-09 Thread Eric Decker
I agree that there is not "any connection of pmc poll time-out to ptp4l waiting 
for TX time stamp."  

I was just stating that I observed pmc unconditionally waiting for a timeout 
when using poll() and wondered if ptp4l was also unconditionally waiting on a 
timeout when using poll().  Based on the feedback from the group it is not 
waiting on the timeout.

Eric Decker

-Original Message-
From: Geva, Erez  
Sent: Friday, July 9, 2021 11:50 AM
To: Eric Decker 
Cc: linuxptp-devel@lists.sourceforge.net; Richard Cochran 
; Miroslav Lichvar 
Subject: RE: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

I do not see any connection of pmc poll time-out to ptp4l waiting for TX time 
stamp.

On my libpmc https://sf.net/p/libpmc I do not use any time out.
The libpmc pmc tools works better this way.

Erez

-Original Message-
From: Eric Decker 
Sent: Friday, 9 July 2021 00:28
To: Richard Cochran ; Miroslav Lichvar 

Cc: linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

If I recall correctly there is an unconditional 150ms delay in PMC which also 
uses poll().  That I why I asked the question.  The delay in PMC may be related 
to how the firmware is structured, not poll().  I am using Linux 4.xx.

Eric Decker

-Original Message-
From: Richard Cochran 
Sent: Thursday, July 8, 2021 1:42 PM
To: Miroslav Lichvar 
Cc: Eric Decker ; linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for the timeout, or continue processing after the timestamp is 
> > received?
> 
> The poll() call is waiting for the descriptor, so it should return as 
> soon as the timestamp is ready. The option sets the maximum time it 
> waits.

But on kernels older than (mumble) (3.5?) the code will sleep the entire 
period, then wake to read the time stamp.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Flinuxptp-devel&data=04%7C01%7Cerez.geva.ext%40siemens.com%7C1d784133383c4cd4e2b408d9425fe84b%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637613801951566481%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9D4HZAuH9zExpzT86tfZjXtO4%2FQ%2B6ednYqty4SnbVAQ%3D&reserved=0


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Eric Decker
If I recall correctly there is an unconditional 150ms delay in PMC which also 
uses poll().  That I why I asked the question.  The delay in PMC may be related 
to how the firmware is structured, not poll().  I am using Linux 4.xx.

Eric Decker

-Original Message-
From: Richard Cochran  
Sent: Thursday, July 8, 2021 1:42 PM
To: Miroslav Lichvar 
Cc: Eric Decker ; linuxptp-devel@lists.sourceforge.net
Subject: Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout 
to 5

On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for the timeout, or continue processing after the timestamp is 
> > received?
> 
> The poll() call is waiting for the descriptor, so it should return as 
> soon as the timestamp is ready. The option sets the maximum time it 
> waits.

But on kernels older than (mumble) (3.5?) the code will sleep the entire 
period, then wake to read the time stamp.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Richard Cochran
On Thu, Jul 08, 2021 at 07:15:17PM +, Machnikowski, Maciej wrote:
> Can it be a half of the packet rate?

No!

> Or there is any reason to make a specific tighter
> limit to it?

See the discussion of the effect of computational delay on stability
in John Eidson's "Measurement, Control, and Communication Using IEEE
1588" section "5.2 Clock Servo Design", especially the analysis
starting on page 153.

   https://www.springer.com/gp/book/9781846282508

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Keller, Jacob E



> -Original Message-
> From: Miroslav Lichvar 
> Sent: Thursday, July 08, 2021 4:10 AM
> To: Eric Decker 
> Cc: Keller, Jacob E ; linuxptp-
> de...@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for
> the timeout, or continue processing after the timestamp is received?
> 
> The poll() call is waiting for the descriptor, so it should return as
> soon as the timestamp is ready. The option sets the maximum time it
> waits.
> 
> I'm ok with increasing the default timeout.
> 
> As a future improvement, maybe it could be adaptive, e.g. once in a
> while try waiting much longer and if that doesn't give a timestamp
> stick to a shorter interval. That is, try to detect when the hardware
> is not able to timestamp all packets.
> 

Not sure I follow here. I guess we'd default to a long timeout and periodically 
try shorter ones? I'm not sure this would be effective. I think the complexity 
isn't really worth it.



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Keller, Jacob E



> -Original Message-
> From: Eric Decker 
> Sent: Wednesday, July 07, 2021 6:38 PM
> To: Keller, Jacob E ; linuxptp-
> de...@lists.sourceforge.net
> Subject: RE: [Linuxptp-devel] [PATCH] Increase the default 
> tx_timestamp_timeout
> to 5
> 
> If the timestamp is available in less than the timeout (5ms) will it still 
> wait for the
> timeout, or continue processing after the timestamp is received?
> 
> Eric
> 
> 

It stops waiting as soon as we get a timestamp, (using select/poll).


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Machnikowski, Maciej


> -Original Message-
> From: Richard Cochran 
> Sent: Thursday, July 8, 2021 7:42 PM

> On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> > On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > > If the timestamp is available in less than the timeout (5ms) will it 
> > > still wait
> for the timeout, or continue processing after the timestamp is received?
> >
> > The poll() call is waiting for the descriptor, so it should return as
> > soon as the timestamp is ready. The option sets the maximum time it
> > waits.
> 
> But on kernels older than (mumble) (3.5?) the code will sleep the entire
> period, then wake to read the time stamp.
> 

Can we reverse the thinking and define what's the maximum time we can allow
the SW to poll for the timestamp?

Can it be a half of the packet rate? Or there is any reason to make a specific 
tighter
limit to it?

Regards
Maciek


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Richard Cochran
On Thu, Jul 08, 2021 at 01:10:08PM +0200, Miroslav Lichvar wrote:
> On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> > If the timestamp is available in less than the timeout (5ms) will it still 
> > wait for the timeout, or continue processing after the timestamp is 
> > received?
> 
> The poll() call is waiting for the descriptor, so it should return as
> soon as the timestamp is ready. The option sets the maximum time it
> waits.

But on kernels older than (mumble) (3.5?) the code will sleep the
entire period, then wake to read the time stamp.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-08 Thread Miroslav Lichvar
On Thu, Jul 08, 2021 at 01:37:38AM +, Eric Decker wrote:
> If the timestamp is available in less than the timeout (5ms) will it still 
> wait for the timeout, or continue processing after the timestamp is received?

The poll() call is waiting for the descriptor, so it should return as
soon as the timestamp is ready. The option sets the maximum time it
waits.

I'm ok with increasing the default timeout.

As a future improvement, maybe it could be adaptive, e.g. once in a
while try waiting much longer and if that doesn't give a timestamp
stick to a shorter interval. That is, try to detect when the hardware
is not able to timestamp all packets.

-- 
Miroslav Lichvar



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-07 Thread Eric Decker
If the timestamp is available in less than the timeout (5ms) will it still wait 
for the timeout, or continue processing after the timestamp is received?

Eric


-Original Message-
From: Jacob Keller  
Sent: Wednesday, July 7, 2021 8:02 PM
To: linuxptp-devel@lists.sourceforge.net
Subject: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

The tx_timestamp_timeout configuration defines the number of milliseconds to 
wait for a Tx timestamp from the kernel stack. This delay is necessary as Tx 
timestamps are captured after a packet is sent and reported back via the socket 
error queue.

The current default is to poll for up to 1 millisecond. In practice, it turns 
out that this is not always enough time for hardware and software to capture 
the timestamp and report it back. Some hardware designs require reading 
timestamps over registers or other slow mechanisms.

This extra delay results in the timestamp not being sent back to userspace 
within the default 1 millisecond polling time. If that occurs the following can 
be seen from ptp4l:

  ptp4l[4756.840]: timed out while polling for tx timestamp
  ptp4l[4756.840]: increasing tx_timestamp_timeout may correct this issue,
   but it is likely caused by a driver bug
  ptp4l[4756.840]: port 1 (p2p1): send sync failed
  ptp4l[4756.840]: port 1 (p2p1): MASTER to FAULTY on FAULT_DETECTED
   (FT_UNSPECIFIED)

This can confuse users because it implies this is a bug, when the correct 
solution in many cases is to just increase the timeout to a slightly higher 
value.

Since we know this is a problem for many drivers and hardware designs, lets 
increase the default timeout. I chose 5 since it is a large enough increase to 
avoid the issues on test systems I have. We do want to keep this timeout small 
because it prevents ptp4l from doing any other processing while we wait for the 
timestamp.

An alternative solution would be to refactor ptp4l so that it does not stop and 
wait for a Tx timestamp, but instead handles the timestamps asynchronously. 
While this could be done, it adds significant complexity to the application 
with minimal or no gain. In most cases, hardware is only capable of a single 
outstanding timestamp at a time, so we cannot send another packet anyways until 
the first one has completed.

Signed-off-by: Jacob Keller 
---
 config.c| 2 +-
 configs/default.cfg | 2 +-
 ptp4l.8 | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 4472d3d9d6f9..f33f177c696a 100644
--- a/config.c
+++ b/config.c
@@ -323,7 +323,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("ts2phc.pulsewidth", 5, 100, 99900),
PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
GLOB_ITEM_INT("twoStepFlag", 1, 0, 1),
-   GLOB_ITEM_INT("tx_timestamp_timeout", 1, 1, INT_MAX),
+   GLOB_ITEM_INT("tx_timestamp_timeout", 5, 1, INT_MAX),
PORT_ITEM_INT("udp_ttl", 1, 1, 255),
PORT_ITEM_INT("udp6_scope", 0x0E, 0x00, 0x0F),
GLOB_ITEM_STR("uds_address", "/var/run/ptp4l"), diff --git 
a/configs/default.cfg b/configs/default.cfg index 64ef3bd7c81d..5e9444da57ee 
100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -51,7 +51,7 @@ hybrid_e2e0
 inhibit_multicast_service  0
 net_sync_monitor   0
 tc_spanning_tree   0
-tx_timestamp_timeout   1
+tx_timestamp_timeout   5
 unicast_listen 0
 unicast_master_table   0
 unicast_req_duration   3600
diff --git a/ptp4l.8 b/ptp4l.8
index fe9e1502231c..024fad3d19b2 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -496,7 +496,7 @@ switches all implement this option together with the BMCA.
 .B tx_timestamp_timeout
 The number of milliseconds to poll waiting for the tx time stamp from the 
kernel  when a message has recently been sent.
-The default is 1.
+The default is 5.
 .TP
 .B check_fup_sync
 Because of packet reordering that can occur in the network, in the
--
2.31.1.331.gb0c09ab8796f



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-07 Thread Richard Cochran
On Wed, Jul 07, 2021 at 05:02:21PM -0700, Jacob Keller wrote:
> diff --git a/config.c b/config.c
> index 4472d3d9d6f9..f33f177c696a 100644
> --- a/config.c
> +++ b/config.c
> @@ -323,7 +323,7 @@ struct config_item config_tab[] = {
>   GLOB_ITEM_INT("ts2phc.pulsewidth", 5, 100, 99900),
>   PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
>   GLOB_ITEM_INT("twoStepFlag", 1, 0, 1),
> - GLOB_ITEM_INT("tx_timestamp_timeout", 1, 1, INT_MAX),
> + GLOB_ITEM_INT("tx_timestamp_timeout", 5, 1, INT_MAX),

Let's make it 10 ms.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH] Increase the default tx_timestamp_timeout to 5

2021-07-07 Thread Jacob Keller
The tx_timestamp_timeout configuration defines the number of
milliseconds to wait for a Tx timestamp from the kernel stack. This
delay is necessary as Tx timestamps are captured after a packet is sent
and reported back via the socket error queue.

The current default is to poll for up to 1 millisecond. In practice, it
turns out that this is not always enough time for hardware and software
to capture the timestamp and report it back. Some hardware designs
require reading timestamps over registers or other slow mechanisms.

This extra delay results in the timestamp not being sent back to
userspace within the default 1 millisecond polling time. If that occurs
the following can be seen from ptp4l:

  ptp4l[4756.840]: timed out while polling for tx timestamp
  ptp4l[4756.840]: increasing tx_timestamp_timeout may correct this issue,
   but it is likely caused by a driver bug
  ptp4l[4756.840]: port 1 (p2p1): send sync failed
  ptp4l[4756.840]: port 1 (p2p1): MASTER to FAULTY on FAULT_DETECTED
   (FT_UNSPECIFIED)

This can confuse users because it implies this is a bug, when the
correct solution in many cases is to just increase the timeout to
a slightly higher value.

Since we know this is a problem for many drivers and hardware designs,
lets increase the default timeout. I chose 5 since it is a large enough
increase to avoid the issues on test systems I have. We do want to keep
this timeout small because it prevents ptp4l from doing any other
processing while we wait for the timestamp.

An alternative solution would be to refactor ptp4l so that it does not
stop and wait for a Tx timestamp, but instead handles the timestamps
asynchronously. While this could be done, it adds significant complexity
to the application with minimal or no gain. In most cases, hardware is
only capable of a single outstanding timestamp at a time, so we cannot
send another packet anyways until the first one has completed.

Signed-off-by: Jacob Keller 
---
 config.c| 2 +-
 configs/default.cfg | 2 +-
 ptp4l.8 | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/config.c b/config.c
index 4472d3d9d6f9..f33f177c696a 100644
--- a/config.c
+++ b/config.c
@@ -323,7 +323,7 @@ struct config_item config_tab[] = {
GLOB_ITEM_INT("ts2phc.pulsewidth", 5, 100, 99900),
PORT_ITEM_ENU("tsproc_mode", TSPROC_FILTER, tsproc_enu),
GLOB_ITEM_INT("twoStepFlag", 1, 0, 1),
-   GLOB_ITEM_INT("tx_timestamp_timeout", 1, 1, INT_MAX),
+   GLOB_ITEM_INT("tx_timestamp_timeout", 5, 1, INT_MAX),
PORT_ITEM_INT("udp_ttl", 1, 1, 255),
PORT_ITEM_INT("udp6_scope", 0x0E, 0x00, 0x0F),
GLOB_ITEM_STR("uds_address", "/var/run/ptp4l"),
diff --git a/configs/default.cfg b/configs/default.cfg
index 64ef3bd7c81d..5e9444da57ee 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -51,7 +51,7 @@ hybrid_e2e0
 inhibit_multicast_service  0
 net_sync_monitor   0
 tc_spanning_tree   0
-tx_timestamp_timeout   1
+tx_timestamp_timeout   5
 unicast_listen 0
 unicast_master_table   0
 unicast_req_duration   3600
diff --git a/ptp4l.8 b/ptp4l.8
index fe9e1502231c..024fad3d19b2 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -496,7 +496,7 @@ switches all implement this option together with the BMCA.
 .B tx_timestamp_timeout
 The number of milliseconds to poll waiting for the tx time stamp from the 
kernel
 when a message has recently been sent.
-The default is 1.
+The default is 5.
 .TP
 .B check_fup_sync
 Because of packet reordering that can occur in the network, in the
-- 
2.31.1.331.gb0c09ab8796f



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel