RDKEMW-18410: Xconf thread should wait for the NTP indicator for network connectivity#367
Conversation
Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a boot-time gate in the XConf worker thread to wait for an NTP synchronization indicator file (/tmp/clock-event) before starting TLS-dependent XConf configuration fetches.
Changes:
- Introduces
waitForNTPSync()which watches/tmpvia inotify (with a timeout) for creation of/tmp/clock-event. - Adds new NTP-sync indicator constants and integrates the wait into
getUpdatedConfigurationThread()prior to the existing fetch/retry loop.
Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
source/xconf-client/xconfclient.c:29
- The new NTP sync wait code uses errno, time(), and NAME_MAX, but this file doesn’t include the headers that declare/define them. This will fail to compile with modern C compilers (implicit function declarations) and can leave NAME_MAX undefined. Add the missing includes (e.g., <errno.h>, <time.h>, and a header that defines NAME_MAX such as <limits.h>).
#include <string.h>
#include <stdlib.h>
#include <sys/time.h>
#include <sys/select.h>
#include <sys/inotify.h>
#include <net/if.h>
#include <ifaddrs.h>
#include <stdbool.h>
#include <curl/curl.h>
#include <unistd.h>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
source/xconf-client/xconfclient.c:33
- The new NTP wait logic uses
errno,NAME_MAX, andtime()but this file does not include the headers that define them (<errno.h>,<limits.h>/<linux/limits.h>, and<time.h>). Depending on toolchain flags (e.g., -Werror / C99+), this can fail to compile or rely on non-portable implicit declarations; please add the appropriate includes near the top include block.
#include <string.h>
#include <stdlib.h>
#include <sys/time.h>
#include <sys/select.h>
#include <sys/inotify.h>
#include <net/if.h>
#include <ifaddrs.h>
#include <stdbool.h>
#include <curl/curl.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <signal.h>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
source/xconf-client/xconfclient.c:913
- This fallback path returns after a single 60s sleep even if the NTP indicator still doesn’t exist, which contradicts the stated “wait indefinitely” gating and can cause xconf fetch/TLS to run before time sync. Consider looping (with stop flag checks) until
access(NTP_SYNC_INDICATOR)succeeds or shutdown is requested, even when inotify isn’t available.
{
T2Error("inotify_init1 failed (errno=%d), falling back to timed wait\n", errno);
sleep(60);
return (access(NTP_SYNC_INDICATOR, F_OK) == 0);
}
source/xconf-client/xconfclient.c:921
- If
NTP_SYNC_DIRdoesn’t exist yet (common at boot for/tmp/systimemgr),inotify_add_watchwill fail and this path also returns after a single 60s sleep. That makes the “wait for NTP indicator” unreliable. Instead, keep retrying watch setup (or pollaccess()in a loop) until the indicator appears or shutdown is requested.
{
T2Error("inotify_add_watch on %s failed (errno=%d)\n", NTP_SYNC_DIR, errno);
close(ifd);
sleep(60);
return (access(NTP_SYNC_INDICATOR, F_OK) == 0);
source/xconf-client/xconfclient.c:1019
- Proceeding when
waitForNTPSync()returns false undermines the purpose of this change: TLS/xconf fetch may still run with incorrect time and then enter existing retry loops. IfwaitForNTPSync()fails due to missing directory/inotify issues, it would be safer to keep waiting (with shutdown interrupt) or exit the thread early rather than continuing immediately.
if (!waitForNTPSync())
{
T2Warning("Proceeding without NTP sync confirmation\n");
}
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
) * Initial plan * Disable NTP sync indication flag in container build path Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/fe1b0563-e051-4df8-a022-0ae17a31725d Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
source/xconf-client/xconfclient.c:1018
- This
clock_gettime()result is ignored beforenowis read. If the call fails, the inotify wait loop compares an uninitialized timestamp against the deadline, making the timeout behavior undefined.
struct timespec now;
clock_gettime(CLOCK_MONOTONIC, &now);
if (now.tv_sec >= deadline.tv_sec)
…ork connectivity Signed-off-by: Yogeswaran K <yogeswaran_k@comcast.com>
No description provided.