Re: [Linuxptp-devel] [PATCH v3 07/11] Subscription time limit

[Jiri Benc]

On Sat, 3 May 2014 20:32:01 +0200, Richard Cochran wrote:
 1. It is a good idea to let the client set the duration?

You wanted to get rid of the magic constant in ptp4l, this allowed me to
move it to phc2sys ;-)

Okay, seriously: I don't think this is a problem. It is limited to UDS
and the worst thing that can happen is attempting to send notifications
to a client when the client is long dead because it set too big value
and crashed.

As it is the client who knows when it will be able to renew the
subscription, it makes sense to be the client who sets the duration.

Obviously, the hyper correct way would be for the server to ack or deny
the request based on the duration value and internal policy, similarly
to e.g. bandwidth allocation in various protocols. I think it's
overkill for this case.

 2. Should we perhaps use a 32 bit field for this?
As is, it allows only 18 hours.

I even thought about limiting the maximum value to a hour or so. The
client is supposed to renew the subscription, otherwise the time limit
would be useless.

 Jiri

-- 
Jiri Benc

--
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
#149; 3 signs your SCM is hindering your productivity
#149; Requirements for releasing software faster
#149; Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Re: [Linuxptp-devel] [PATCH v3 07/11] Subscription time limit

[Richard Cochran]

On Fri, May 02, 2014 at 12:37:50PM +0200, Jiri Benc wrote:
 Add expiration time to subscriptions; they need to be renewed before they
 expiry. This way, the subscription automatically times out when phc2sys is
 killed.
 
 Signed-off-by: Jiri Benc jb...@redhat.com
 ---
  clock.c |   27 +--
  tlv.c   |8 
  tlv.h   |1 +
  3 files changed, 34 insertions(+), 2 deletions(-)
 
 diff --git a/clock.c b/clock.c
 index c66a04064ff8..29d58b0c2461 100644
 --- a/clock.c
 +++ b/clock.c
 @@ -67,6 +67,7 @@ struct clock_subscriber {
   struct PortIdentity targetPortIdentity;
   struct address addr;
   UInteger16 sequenceId;
 + time_t expiration;
  };
  
  struct clock {
 @@ -135,10 +136,11 @@ static void remove_subscriber(struct clock_subscriber 
 *s)
  }
  
  static void clock_update_subscription(struct clock *c, struct ptp_message 
 *req,
 -   uint8_t *bitmask)
 +   uint8_t *bitmask, unsigned int valid_time)

When reading this patch, I was confused by the name valid_time ...

  struct subscribe_events_np {
 + uint16_t  valid_time; /* seconds */
   uint8_t   bitmask[EVENT_BITMASK_CNT];
  } PACKED;

I think duration would be a better name. Also I have two questions:

1. It is a good idea to let the client set the duration?

2. Should we perhaps use a 32 bit field for this?
   As is, it allows only 18 hours.

Thanks,
Richard

--
Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.  Get 
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free.
http://p.sf.net/sfu/SauceLabs
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel