On Mon, Oct 08, 2018 at 12:54:44PM +0000, Anders Selhammer wrote:
> If the unicast negotiation is disabled, the granted messages needs to be
> cancelled towards the server.
Yes, "canceled" not "ungranted".
> +static int unicast_client_ungrant(struct port *p, uint8_t mtype,
> + struct unicast_master_address *ucma)
> +{
> + struct ptp_message *msg;
> + int err;
> + if (~ucma->granted & (1 << mtype)) {
> + return 0;
> + }
> + msg = port_signaling_construct(p, &ucma->address, &ucma->portIdentity);
> + if (!msg) {
> + return -1;
> + }
> + err = attach_cancel(msg, mtype << 4);
This function is poorly named. You are canceling a request. Only the
one who granted something may "un-grant" it again.
> + case UC_DENIED:
> + switch (ev) {
> + case UC_EV_GRANT_ANN:
> + next = UC_HAVE_ANN;
> + break;
> + case UC_EV_SELECTED:
> + case UC_EV_GRANT_SYDY:
> + case UC_EV_UNSELECTED:
> + case UC_EV_CANCEL:
> + break;
> + case UC_EV_DISABLE:
> next = UC_WAIT;
> break;
> }
> diff --git a/unicast_fsm.h b/unicast_fsm.h
> index 49fc9b2..6eb1cfa 100644
> --- a/unicast_fsm.h
> +++ b/unicast_fsm.h
> @@ -25,6 +25,7 @@ enum unicast_state {
> UC_HAVE_ANN,
> UC_NEED_SYDY,
> UC_HAVE_SYDY,
> + UC_DENIED,
You have added a whole new state without the slightest justification.
The client doesn't need a DENIED state. If a request is rejected, the
client simply goes back to the WAITing and tried again.
Adding a DENIED state makes no sense at all, since the point of this
series is to add client side cancellation.
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
