Re: [Linuxptp-devel] [PATCH v1 5/5] Implement the COMMON_P2P delay mechanism.

[Kishen M. <kis...@in...>]

Hi Richard,

Firstly, thanks so much for your reviews and efforts to get
this done.

I'll just rehash a couple of points also raised by Andrew.

On this change below:

 > +static int port_cmlds_initialize(struct port *p)
 > +{
 > +	struct config *cfg = clock_config(p->clock);
 > +	struct subscribe_events_np sen = {0};
 > +	const int zero_datalen = 1;
 > +	const UInteger8 hops = 0;
 > +
 > +	p->cmlds_port = config_get_int(cfg, p->name, "cmlds_port");
 > +	p->cmlds_pmc = pmc_create(cfg, TRANS_UDS,
 > +				  config_get_string(cfg, p->name, "cmlds_client_address"),
 > +				  config_get_string(cfg, p->name, "cmlds_server_address"),
 > +				  hops,
 > +				  config_get_int(cfg, NULL, "domainNumber"),
 > +				  config_get_int(cfg, p->name, "transportSpecific") << 4,

I haven't studied the flow here, but using the P2P_COMMON instance's
transportSpecific and domainNumber above might cause the CMLDS server's 
port_ignore() to drop this message as the
CMLDS ptp4l process would be configured with
transportSpecific/domain=0x2/0.

Perhaps just passing 0x2/0 above will fix things? This is assuming that
there are no transportSpecific/domainNumber checks on the receive path
for CMLDS events (which seemed to be the case looking at the code).

On 11/30/23 11:28 AM, Richard Cochran wrote:
...
>>> serviceMeasurementValid is implicit, because if you get a PUSH
>>> notification, it is valid.
>>
>> Right, serviceMeasurementValid == true is implicit but
>> serviceMeasurementValid == false is useful for the .port_id_valid /
>> .pdr_missing logic to work.  The timeout logic is disabled with
>> P2P_COMMON.
> 
> Right, so the next TODO is to enable FD_DELAY_TIMER for P2P_COMMON
> mode and let it 1) renew the push subscription and 2) catch the case
> when the push notification does not arrive on time.
> 

Yes, in our series CMLDS queries were performed synchronously at the 
Pdelay request cadence. If serviceMeasurementValid=false then 
port->pdr_missing was incremented and immediately followed by a call to 
port_capable(). This was necessary as I recollect there was a related 
Avnu test case.

If serviceMeasurementValid=true, then we set both peer_portid_valid and 
p->nrate.ratio_valid to true so that port_capable() could evaluate to true.