Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.04.29 15:34:15 +0200:
> Like for rsync repos files in the RRDP repos should be delayed until after
> the validation finished. As with anything RPKI related there is little
> trust in the repositories and their abilities to not botch an update.
>
> One thing I'm not sure is what should happen if a file is supposed to be
> removed but is still referenced by some other file. For now this fact is
> logged and the file is kept in the repo. I'm unsure about keeping the
fine with me, ok benno@
> file, it feels like the right move but may result in unreferenced files
> piling up in the rrdp repo dirs.
>
> There is no way to detect stale files in RRDP repos (apart from removing
> all files and fetching a snapshot). So until RRDP grows up to a real sync
> protocol the only thing one can do is to provide a large enough partition
> and to remove the cache from time to time.
You could get a file listing at the start and then remove files from the
list that are referenced, at the end you delete the ones left.
> --
> :wq Claudio
>
>
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 repo.c
> --- repo.c 19 Apr 2021 17:04:35 -0000 1.6
> +++ repo.c 29 Apr 2021 13:12:34 -0000
> @@ -553,16 +553,19 @@ rrdp_free(void)
> }
> }
>
> -static int
> +static struct rrdprepo *
> rrdp_basedir(const char *dir)
> {
> struct rrdprepo *rr;
>
> SLIST_FOREACH(rr, &rrdprepos, entry)
> - if (strcmp(dir, rr->basedir) == 0)
> - return 1;
> + if (strcmp(dir, rr->basedir) == 0) {
> + if (rr->state == REPO_FAILED)
> + return NULL;
> + return rr;
> + }
>
> - return 0;
> + return NULL;
> }
>
> /*
> @@ -840,27 +843,6 @@ rrdp_merge_repo(struct rrdprepo *rr)
> struct filepath *fp, *nfp;
> char *fn, *rfn;
>
> - /* XXX should delay deletes */
> - RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
> - fn = rrdp_filename(rr, fp->file, 1);
> - rfn = rrdp_filename(rr, fp->file, 0);
> -
> - if (fn == NULL || rfn == NULL)
> - errx(1, "bad filepath"); /* should not happen */
> -
> - if (unlink(rfn) == -1) {
> - if (errno == ENOENT) {
> - if (unlink(fn) == -1)
> - warn("%s: unlink", fn);
> - } else
> - warn("%s: unlink", rfn);
> - }
> -
> - free(rfn);
> - free(fn);
> - filepath_put(&rr->deleted, fp);
> - }
> -
> RB_FOREACH_SAFE(fp, filepath_tree, &rr->added, nfp) {
> fn = rrdp_filename(rr, fp->file, 1);
> rfn = rrdp_filename(rr, fp->file, 0);
> @@ -1146,12 +1128,40 @@ add_to_del(char **del, size_t *dsz, char
> *dsz = i + 1;
> return del;
> }
> +
> +static char **
> +repo_rrdp_cleanup(struct filepath_tree *tree, struct rrdprepo *rr,
> + char **del, size_t *delsz)
> +{
> + struct filepath *fp, *nfp;
> + char *fn;
> +
> + RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
> + fn = rrdp_filename(rr, fp->file, 0);
> + /* temp dir will be cleaned up by repo_cleanup() */
> +
> + if (fn == NULL)
> + errx(1, "bad filepath"); /* should not happen */
> +
> + if (!filepath_exists(tree, fn))
> + del = add_to_del(del, delsz, fn);
> + else
> + warnx("%s: referenced file supposed to be deleted", fn);
> +
> + free(fn);
> + filepath_put(&rr->deleted, fp);
> + }
> +
> + return del;
> +}
> +
> void
> repo_cleanup(struct filepath_tree *tree)
> {
> - size_t i, delsz = 0, dirsz = 0;
> + size_t i, cnt, delsz = 0, dirsz = 0;
> char **del = NULL, **dir = NULL;
> char *argv[4] = { "ta", "rsync", "rrdp", NULL };
> + struct rrdprepo *rr;
> FTS *fts;
> FTSENT *e;
>
> @@ -1166,10 +1176,12 @@ repo_cleanup(struct filepath_tree *tree)
> e->fts_path);
> break;
> case FTS_D:
> - /* skip rrdp base directories during cleanup */
> - if (rrdp_basedir(e->fts_path))
> + /* special cleanup for rrdp directories */
> + if ((rr = rrdp_basedir(e->fts_path)) != NULL) {
> + del = repo_rrdp_cleanup(tree, rr, del, &delsz);
> if (fts_set(fts, e, FTS_SKIP) == -1)
> err(1, "fts_set");
> + }
> break;
> case FTS_DP:
> if (!filepath_dir_exists(tree, e->fts_path))
> @@ -1203,25 +1215,31 @@ repo_cleanup(struct filepath_tree *tree)
> if (fts_close(fts) == -1)
> err(1, "fts_close");
>
> + cnt = 0;
> for (i = 0; i < delsz; i++) {
> - if (unlink(del[i]) == -1)
> - warn("unlink %s", del[i]);
> - if (verbose > 1)
> - logx("deleted %s", del[i]);
> + if (unlink(del[i]) == -1) {
> + if (errno != ENOENT)
> + warn("unlink %s", del[i]);
> + } else {
> + if (verbose > 1)
> + logx("deleted %s", del[i]);
> + cnt++;
> + }
> free(del[i]);
> }
> free(del);
> - stats.del_files = delsz;
> + stats.del_files = cnt;
>
> + cnt = 0;
> for (i = 0; i < dirsz; i++) {
> if (rmdir(dir[i]) == -1)
> warn("rmdir %s", dir[i]);
> - if (verbose > 1)
> - logx("deleted dir %s", dir[i]);
> + else
> + cnt++;
> free(dir[i]);
> }
> free(dir);
> - stats.del_dirs = dirsz;
> + stats.del_dirs = cnt;
> }
>
> void
>
