On Thu, Feb 22, 2024 at 05:59:09PM +0100, Theo Buehler wrote:
> > I'm happy to have something like -x to exclude / include experimental
> > bits. So we don't constantly add flags and remove them again. With ASPA we
> > got hit rather bad IIRC since we had to go back and forth a few times to
> > get it right.
> 
> Something like this? This short circuits the parsing of the DER of the
> SPL and counts the "failure" as "skipped" instead. It's a bit dumb to
> have two mutually exclusive counters, but it feels wrong to use the same
> counter.

I would just remove the extra counter and just not count at all.
This is a temporary measure to ensure we can handle draft changes or other
unexpected problems with these early code dumps.

rpki-client is normally around 1 year ahead of the other RP software when
it comes to new features and we don't need to be too aggressive.
 
I though -x would enable .spl parsing so that people like Job can play
with it but the general public is not affected. We had a few issues with
new file types. One was around handling of a new file RTYPE. So I wonder
if it is enough to "just" skip the decoder.

> I thought it makes more sense to keep -x a no-op with -f mode. After
> all, you hand such a file explicitly on the command line.
> 
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> diff -u -p -r1.208 extern.h
> --- extern.h	22 Feb 2024 12:49:42 -0000	1.208
> +++ extern.h	22 Feb 2024 16:31:37 -0000
> @@ -575,6 +575,7 @@ enum stype {
>  	STYPE_OK,
>  	STYPE_FAIL,
>  	STYPE_INVALID,
> +	STYPE_SKIPPED,
>  	STYPE_BGPSEC,
>  	STYPE_TOTAL,
>  	STYPE_UNIQUE,
> @@ -612,7 +613,8 @@ struct repotalstats {
>  	uint32_t	 vrps_uniqs; /* number of unique vrps */
>  	uint32_t	 spls; /* signed prefix list */
>  	uint32_t	 spls_fail; /* failing syntactic parse */
> -	uint32_t	 spls_invalid; /* invalid asid */
> +	uint32_t	 spls_skipped; /* skipped spls */
> +	uint32_t	 spls_invalid; /* invalid spls */
>  	uint32_t	 vsps; /* total number of Validated SPL Payloads */
>  	uint32_t	 vsps_uniqs; /* number of unique vsps */
>  };
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> diff -u -p -r1.251 main.c
> --- main.c	22 Feb 2024 12:49:42 -0000	1.251
> +++ main.c	22 Feb 2024 16:31:25 -0000
> @@ -72,6 +72,7 @@ int	filemode;
>  int	shortlistmode;
>  int	rrdpon = 1;
>  int	repo_timeout;
> +int	skip_experimental;
>  time_t	deadline;
>  
>  /* 9999-12-31 23:59:59 UTC */
> @@ -670,7 +671,10 @@ entity_process(struct ibuf *b, struct st
>  	case RTYPE_SPL:
>  		io_read_buf(b, &c, sizeof(c));
>  		if (c == 0) {
> -			repo_stat_inc(rp, talid, type, STYPE_FAIL);
> +			if (skip_experimental)
> +				repo_stat_inc(rp, talid, type, STYPE_SKIPPED);
> +			else
> +				repo_stat_inc(rp, talid, type, STYPE_FAIL);
>  			break;
>  		}
>  		spl = spl_read(b);
> @@ -772,6 +776,7 @@ sum_stats(const struct repo *rp, const s
>  	out->vaps_pas += in->vaps_pas;
>  	out->spls += in->spls;
>  	out->spls_fail += in->spls_fail;
> +	out->spls_skipped += in->spls_skipped;
>  	out->spls_invalid += in->spls_invalid;
>  	out->vsps += in->vsps;
>  	out->vsps_uniqs += in->vsps_uniqs;
> @@ -996,7 +1001,7 @@ main(int argc, char *argv[])
>  	    "proc exec unveil", NULL) == -1)
>  		err(1, "pledge");
>  
> -	while ((c = getopt(argc, argv, "Ab:Bcd:e:fH:jmnoP:rRs:S:t:T:vV")) != -1)
> +	while ((c = getopt(argc, argv, "Ab:Bcd:e:fH:jmnoP:rRs:S:t:T:vVx")) != -1)
>  		switch (c) {
>  		case 'A':
>  			excludeaspa = 1;
> @@ -1074,6 +1079,9 @@ main(int argc, char *argv[])
>  		case 'V':
>  			fprintf(stderr, "rpki-client %s\n", RPKI_VERSION);
>  			return 0;
> +		case 'x':
> +			skip_experimental = 1;
> +			break;
>  		default:
>  			goto usage;
>  		}
> @@ -1473,8 +1481,10 @@ main(int argc, char *argv[])
>  	    "invalid)\n", stats.repo_tal_stats.aspas,
>  	    stats.repo_tal_stats.aspas_fail,
>  	    stats.repo_tal_stats.aspas_invalid);
> -	printf("Signed Prefix Lists: %u (%u failed parse, %u invalid)\n",
> +	printf("Signed Prefix Lists: %u (%u failed parse, %u skipped, "
> +	    "%u invalid)\n",
>  	    stats.repo_tal_stats.spls, stats.repo_tal_stats.spls_fail,
> +	    stats.repo_tal_stats.spls_skipped,
>  	    stats.repo_tal_stats.spls_invalid);
>  	printf("BGPsec Router Certificates: %u\n", stats.repo_tal_stats.brks);
>  	printf("Certificates: %u (%u invalid)\n",
> @@ -1505,7 +1515,7 @@ main(int argc, char *argv[])
>  
>  usage:
>  	fprintf(stderr,
> -	    "usage: rpki-client [-ABcjmnoRrVv] [-b sourceaddr] [-d cachedir]"
> +	    "usage: rpki-client [-ABcjmnoRrVvx] [-b sourceaddr] [-d cachedir]"
>  	    " [-e rsync_prog]\n"
>  	    "                   [-H fqdn] [-P epoch] [-S skiplist] [-s timeout]"
>  	    " [-T table]\n"
> Index: output-ometric.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/output-ometric.c,v
> diff -u -p -r1.8 output-ometric.c
> --- output-ometric.c	22 Feb 2024 12:49:42 -0000	1.8
> +++ output-ometric.c	22 Feb 2024 16:50:34 -0000
> @@ -87,6 +87,8 @@ set_common_stats(const struct repotalsta
>  	    OKV("type", "state"), OKV("spl", "valid"), ol);
>  	ometric_set_int_with_labels(metric, in->spls_fail,
>  	    OKV("type", "state"), OKV("spl", "failed parse"), ol);
> +	ometric_set_int_with_labels(metric, in->spls_skipped,
> +	    OKV("type", "state"), OKV("spl", "skipped"), ol);
>  	ometric_set_int_with_labels(metric, in->spls_invalid,
>  	    OKV("type", "state"), OKV("spl", "invalid"), ol);
>  
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> diff -u -p -r1.129 parser.c
> --- parser.c	22 Feb 2024 12:49:42 -0000	1.129
> +++ parser.c	22 Feb 2024 15:29:45 -0000
> @@ -39,6 +39,8 @@
>  #include "extern.h"
>  
>  extern int noop;
> +extern int skip_experimental;
> +extern int verbose;
>  
>  static X509_STORE_CTX	*ctx;
>  static struct auth_tree	 auths = RB_INITIALIZER(&auths);
> @@ -861,9 +863,15 @@ parse_entity(struct entityq *q, struct m
>  		case RTYPE_SPL:
>  			file = parse_load_file(entp, &f, &flen);
>  			io_str_buffer(b, file);
> -			spl = proc_parser_spl(file, f, flen, entp);
> -			if (spl != NULL)
> -				mtime = spl->signtime;
> +			if (skip_experimental) {
> +				if (verbose)
> +					warnx("%s: skipped", file);
> +				spl = NULL;
> +			} else {
> +				spl = proc_parser_spl(file, f, flen, entp);
> +				if (spl != NULL)
> +					mtime = spl->signtime;
> +			}
>  			io_simple_buffer(b, &mtime, sizeof(mtime));
>  			c = (spl != NULL);
>  			io_simple_buffer(b, &c, sizeof(int));
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> diff -u -p -r1.53 repo.c
> --- repo.c	22 Feb 2024 12:49:42 -0000	1.53
> +++ repo.c	22 Feb 2024 15:49:33 -0000
> @@ -1488,6 +1488,9 @@ repo_stat_inc(struct repo *rp, int talid
>  		case STYPE_FAIL:
>  			rp->stats[talid].spls_fail++;
>  			break;
> +		case STYPE_SKIPPED:
> +			rp->stats[talid].spls_skipped++;
> +			break;
>  		case STYPE_INVALID:
>  			rp->stats[talid].spls_invalid++;
>  			break;
> Index: rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> diff -u -p -r1.101 rpki-client.8
> --- rpki-client.8	22 Feb 2024 12:49:42 -0000	1.101
> +++ rpki-client.8	22 Feb 2024 16:15:25 -0000
> @@ -22,7 +22,7 @@
>  .Nd RPKI validator to support BGP routing security
>  .Sh SYNOPSIS
>  .Nm
> -.Op Fl ABcjmnoRrVv
> +.Op Fl ABcjmnoRrVvx
>  .Op Fl b Ar sourceaddr
>  .Op Fl d Ar cachedir
>  .Op Fl e Ar rsync_prog
> @@ -231,6 +231,10 @@ If
>  .Fl f
>  is given, specify once to print more information about the encapsulated X.509
>  certificate, twice to print the certificate in PEM format.
> +.It Fl x
> +Skip processing of experimental file formats.
> +This option has no effect if
> +.Fl f is given .
>  .It Ar outputdir
>  The directory where
>  .Nm
> 

-- 
:wq Claudio