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

On Mon, Jun 22, 2026 at 07:30:04AM +0000, Job Snijders wrote:
> 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 :)

Why do you talk about iterators? There is no iteration here.

There is insertion and removal from the tree matching the definition
that a CA is non-functional as long as there is no subordinate object,
and the accounting followed that. I don't see how that is complicated or
unwieldy and it certainly isn't more copmlicated than your logic.
Perhaps the naming wasn't great and one of them should have included a
DEC. The pattern exists for several other counters.