On Tue, Nov 10, 2020 at 02:21:38PM -0800, Richard Cochran wrote:
> The main method that causes the PMC agent to update its status takes a flag
> that results in different behavior when push notifications are active.
> This patch simplifies the interface by letting the agent remember whether
> or not the caller subscribed to the notifications in the first place.
>
> Signed-off-by: Richard Cochran <richardcoch...@gmail.com>
> ---
> phc2sys.c | 6 ++++--
> pmc_agent.c | 32 ++++++++++++++++++++------------
> pmc_agent.h | 2 +-
> 3 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/pmc_agent.c b/pmc_agent.c
> index 714c5c5..24587b4 100644
> --- a/pmc_agent.c
> +++ b/pmc_agent.c
> @@ -42,6 +42,7 @@ struct pmc_agent {
> int clock_identity_set;
> int leap;
> int pmc_ds_requested;
> + int subscription_active;
Bool please? In some places, bool is used already.
> int sync_offset;
> int utc_offset_traceable;
>
> @@ -188,6 +189,19 @@ static int run_pmc(struct pmc_agent *node, int timeout,
> int ds_id,
> }
> }
>
> +static int renew_subscription(struct pmc_agent *node, int timeout)
> +{
> + struct ptp_message *msg;
> + int res;
> +
> + res = run_pmc(node, timeout, TLV_SUBSCRIBE_EVENTS_NP, &msg);
> + if (is_run_pmc_error(res)) {
> + return run_pmc_err2errno(res);
> + }
> + msg_put(msg);
> + return 0;
> +}
> +
> int run_pmc_wait_sync(struct pmc_agent *node, int timeout)
> {
> struct ptp_message *msg;
> @@ -323,7 +337,7 @@ int run_pmc_clock_identity(struct pmc_agent *node, int
> timeout)
> }
>
> /* Returns: -1 in case of error, 0 otherwise */
> -int update_pmc_node(struct pmc_agent *node, int subscribe)
> +int update_pmc_node(struct pmc_agent *node)
> {
> struct timespec tp;
> uint64_t ts;
> @@ -337,8 +351,9 @@ int update_pmc_node(struct pmc_agent *node, int subscribe)
> if (node->pmc &&
> !(ts > node->pmc_last_update &&
> ts - node->pmc_last_update < PMC_UPDATE_INTERVAL)) {
> - if (subscribe)
> - pmc_agent_subscribe(node, 0);
> + if (node->subscription_active) {
> + renew_subscription(node, 0);
> + }
> if (run_pmc_get_utc_offset(node, 0) > 0)
> node->pmc_last_update = ts;
> }
> @@ -393,15 +408,8 @@ void pmc_agent_set_sync_offset(struct pmc_agent *agent,
> int offset)
>
> int pmc_agent_subscribe(struct pmc_agent *node, int timeout)
> {
> - struct ptp_message *msg;
> - int res;
> -
> - res = run_pmc(node, timeout, TLV_SUBSCRIBE_EVENTS_NP, &msg);
> - if (is_run_pmc_error(res)) {
> - return run_pmc_err2errno(res);
> - }
> - msg_put(msg);
> - return 0;
> + node->subscription_active = 1;
> + return renew_subscription(node, timeout);
> }
Hmm. We introduce a new synonym for pmc_agent_subscribe, which we call
renew_subscription, with the only difference being that the former will
set node->subscription_active = 1 while the other will assume it was
already set that way. So the net change of this patch is that we can
keep calling update_pmc_node from the common path (between subscribers
and non-subscribers), but those common code paths no longer need to know
whether they're subscribers or not. Ok. This works, but update_pmc_node
will also need serious documentation updates when its time comes.
But another question: when ptp4l calls clock_prune_subscriptions(), our
node->subscription_active will remain 1, right? That's confusing.
"Active" might not be the right word, how about "using_subscriptions"?
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
