On Wed, May 24, 2023 at 04:18:30PM +0000, Job Snijders wrote:
> Dear all,
>
> Claudio made some suggestions to pass the desired modification times
> around in a different way, below is an updated patch proposal.
> I also added some instrumentation to also adjust GBRs and TAKs.
>
> RIPE & APNIC informally indicated some interest in this hack.
>
This looks good. Some feedback below.
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 parser.c
> --- parser.c 11 May 2023 20:13:30 -0000 1.94
> +++ parser.c 24 May 2023 16:11:15 -0000
> @@ -615,8 +621,11 @@ parse_entity(struct entityq *q, struct m
> struct mft *mft;
> struct roa *roa;
> struct aspa *aspa;
> + struct gbr *gbr;
> + struct tak *tak;
> struct ibuf *b;
> unsigned char *f;
> + time_t mtime = 0;
Please initialize mtime to 0 inside of the
while ((entp = TAILQ_FIRST(q)) != NULL) {
loop. e.g right were f and file are set to 0.
> size_t flen;
> char *file, *crlfile;
> int c;
...
> @@ -693,6 +709,11 @@ parse_entity(struct entityq *q, struct m
> io_simple_buffer(b2, &entp->talid,
> sizeof(entp->talid));
> io_str_buffer(b2, crlfile);
> + /*
> + * FIXME: MFT CMS signing-time is used as mtime
> + * instead of CRL lastUpdate.
> + */
> + io_simple_buffer(b2, &mtime, sizeof(mtime));
> free(crlfile);
>
> io_close_buffer(msgq, b2);
> @@ -727,13 +758,18 @@ parse_entity(struct entityq *q, struct m
> case RTYPE_TAK:
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> - proc_parser_tak(file, f, flen, entp->mftaki);
> + tak = proc_parser_tak(file, f, flen, entp->mftaki);
> + if (tak != NULL)
> + mtime = tak->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> + tak_free(tak);
> break;
Thanks for plugging this memory leak here.
> @@ -1540,6 +1542,26 @@ repo_move_valid(struct filepath_tree *tr
>
> if (repo_mkpath(AT_FDCWD, fn) == -1)
> continue;
> +
> + /*
> + * Adjust file last modification time in order to minimize
> + * RSYNC synchronization load after transport failover.
> + * While serializing RRDP datastructures to disk, set the last
> + * modified timestamp to the CMS signing-time or the X.509
> + * notBefore timestamp.
> + */
> + if (fp->mtime != 0 &&
> + strncmp(fp->file, ".rrdp/", rrdpsz) == 0) {
> + struct timespec ts[2];
> +
> + ts[0].tv_nsec = UTIME_OMIT;
> + ts[1].tv_sec = fp->mtime;
> + ts[1].tv_nsec = 0;
> + if (utimensat(AT_FDCWD, fp->file, ts, 0) == -1) {
> + warn("utimensat %s", fp->file);
> + continue;
> + }
> + }
I would move this code up into the else branch of
if (strncmp(fp->file, ".rsync/", rsyncsz) == 0) {
} else {
base = strchr(fp->file + rrdpsz, '/');
assert(base != NULL);
fn = base + 1;
/* HERE */
}
Then it is enough to just check fp->mtime != 0.
>
> if (rename(fp->file, fn) == -1) {
> warn("rename %s", fp->file);
>
--
:wq Claudio
