Re: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req

[Keller, Jacob E]

> -Original Message-
> From: Anders Selhammer [mailto:anders.selham...@est.tech]
> Sent: Tuesday, March 20, 2018 12:14 PM
> To: Richard Cochran <richardcoch...@gmail.com>
> Cc: linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req
> 
> If I summarize this, I will make two patches instead of three
> 
> 1, port: Fix coding style (old 3/3)
> 2, port: Added TAILQ for sent delay_req (merge 1/3 and 2/3 + 5 sec limit in
> queue)
> 
> 
> Is this ok?
> 
> /Anders
> 

This make sense to me.

Thanks,
Jake


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req

[Anders Selhammer]

Tuesday, March 20, 2018 3:27 PM

>No, use a time out just like we do in other places, please.

Ok, great. I did not notice this. Much better than counter. I will include 
this, see summary in the end.


>> msg: Added missing network to host convertion of for portIdentity in 
>> received delay response messages.
>This should be a separate path.

I guess you mean separate patch. If I put this in a separate patch I will build 
in a bug. If I put it before my patch with the queue, the legacy code won't 
work. If I put it after my patch with the queue, the queue patch won't work.

Before queue patch: if (!pid_eq(>requestingPortIdentity, 
>hdr.sourcePortIdentity)) 
Rsp now in host order and req in network order

After queue patch: if (!pid_eq(>requestingPortIdentity, >portIdentity))
Rsp still in network order and p in host order

If all separate patches should work independently, I need to have msg fix in 
same patch as I include the queue, or include legacy fix for code I replace in 
queue patch. I find that unnecessary, better to include in same patch, or?


>Please pay attention to the order of the patches.
>This pruning needs to go in first, otherwise you have built a bug into the 
>previous patch!

As I read it, you want me to include this in previous patch, since I cannot put 
it before previous since the queue does not exist there.


If I summarize this, I will make two patches instead of three

1, port: Fix coding style (old 3/3)
2, port: Added TAILQ for sent delay_req (merge 1/3 and 2/3 + 5 sec limit in 
queue)


Is this ok?

/Anders

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req

[Richard Cochran]

On Tue, Mar 20, 2018 at 08:21:09AM +, Anders Selhammer wrote:

> One thing that I might overseen is the growing part. I covered that
> some packets might be lost but if all delay resp are lost by some
> reason it might be good to have some kind of a limit of how many
> delay req the stack should store.

This is essential.  Otherwise your memory allocation grows without
bounds.

> For that, a counter is needed. Should I add that?

No, use a time out just like we do in other places, please.

Thanks,
Richard

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req

[Anders Selhammer]

Hi
Any other comments on this patch? It needs to be rebased but I will wait with 
that until I got any other comments.

One thing that I might overseen is the growing part. I covered that some 
packets might be lost but if all delay resp are lost by some reason it might be 
good to have some kind of a limit of how many delay req the stack should store. 
For that, a counter is needed. Should I add that?

/Anders


-Ursprungligt meddelande-
Från: Anders Selhammer  
Skickat: Thursday, March 15, 2018 12:00 PM
Till: linuxptp-devel@lists.sourceforge.net
Ämne: [Linuxptp-devel] [PATCH 0/3] Add TAILQ for sent delay_req

In a ptp unaware network (like the telecom profile for frequency sync 
G.8265.1), both the RTD and the PDV can be substantially higher than in a ptp 
aware network. To achieve more accurate measurements, the rate may need to be 
configured higher to get more data and increase the chance of lucky packets.
In a combination of a high configured rate of delay_req and high RTD/PDV in 
network, the risk that the response from the previously sent delay_req have not 
been received before a new delay_req is sent also become high. In that case, 
the need of storing more than the latest sent delay_req arise.

Anders Selhammer (3):
  port: Added TAILQ for sent delay_req
  port: Remove obsolete delay_req in TAILQ
  port: Fix coding style in updated functions

 msg.c  |  1 +
 port.c | 57 ++---
 2 files changed, 35 insertions(+), 23 deletions(-)

-- 
1.8.3.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel