From: Job Snijders <job@bsd.nl>
Subject: Re: rpki-client: refactor non-functional CA metrics accounting
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Mon, 22 Jun 2026 07:30:04 +0000

On Mon, Jun 22, 2026 at 07:44:44AM +0200, Theo Buehler wrote:
> On Sun, Jun 21, 2026 at 09:47:13PM +0000, Job Snijders wrote:
> > Rpki-client detects non-functional CAs by initially marking every CA as
> > non-functional, and then later on (after a valid manifest is discovered)
> > clearing that mark. Statistics were done along the same lines: first
> > increment and later on substract for all functional ones. This resulted
> > in rather unwieldy code, e.g., a repo pointer needed to be hoisted
> > around which is getting in the way of upcoming work.
> >
> > The below diff simplifies the dance described above, and also fixes a
> > subtle defect: non-functional CAs were counted towards the repository
> > those broken CAs were pointing towards, instead of being counted
> > towards the repository that contained the broken CA. Attributing the
> > non-functional CA to the issuing parent makes more sense to me, because
> > the issuing parent can actually do something about it, for example by
> > revoking the broken child.
> 
> I don't think this description is very on point. There is no unwieldy
> code being changed nor any real simplification being made. Your diff
> adds more code than it removes (the removed code is entirely trivial)
> and your code walks more trees and lists.

At the risk of splitting hairs..., I thought an iterator doing '++'
followed by an iterator doing '--' to be a complicated way of counting,
(compared to a single '++' pass). And, repo_stat_inc(.., STYPE_FUNC)
('repo stat increment') actually doing a decrement seemed unintuitive :)

> That said, I agree that the new accounting is better, so that bug fix
> is what your description in the commit message should focus on.

Thanks, will do.

> > In short, I think this diff helps better show which repository is
> > responsible for non-functional CAs. See the metrics output that follows.
> >
> > OK?
> 
> STYPE_NONFUNC and STYPE_FUNC need to be garbage collected as well.
> 
> I'm not sure repo_stat_inc_nca() is a very good name for the new
> function but I can live with it.

perhaps repo_stat_count_nca() ?

I included the two feedbacks from your follow-up email in my tree
(remove comment and garbage collect STYPE_NONFUNC + STYPE_FUNC).

Kind regards,

Job