Restructure limiting of ASPA provider set sizes. (#996)

This PR changes how the ASPA provider ASN set is limited. It drops the
aspa-provider-limit config option again and instead hard-codes the limit
to 16380 as per the latest version of the RTR draft. Since this limit can
be exceeded when merging multiple ASPAs, the check has been moved to the
very end when the new snapshot is created which also avoid having to track
customer ASNs. A new metric was added to count the ASPA customer ASNs
ignored this way.

doc/manual/source/manual-page.rst

@@ -356,13 +356,6 @@ The available options are:
       If this option is present, ASPA assertions will be processed
       during validation and included in the produced data set.
 
-.. option:: --aspa-provider-limit
-
-      Limits the number of provider ASNs allowed in an ASPA object. If more
-      providers are given, all ASPA assertions for the customer ASN are
-      dropped to avoid false rejections. The default value if not changed
-      via configuration or this option is 10,000 provider ASNs.
-
 .. option:: --dirty
 
       If this option is present, unused files and directories will not be
@@ -1208,13 +1201,6 @@ All values can be overridden via the command line options.
             included in the published dataset. If false or missing, no ASPA
             assertions will be included.
 
-      aspa_provider_limit
-            An integer value specifying the maximum number of provider ASNs
-            allowed in an ASPA object. If more providers are given, all ASPA
-            assertions for the customer ASN are dropped to avoid false
-            rejections. If the option is missing, a default of 10,000
-            provider ASNs is used.
-
       dirty
             A boolean value which, if true, specifies that unused files and
             directories should not be deleted from the repository directory

src/config.rs

@@ -93,9 +93,6 @@ const DEFAULT_MAX_CA_DEPTH: usize = 32;
 #[cfg(unix)]
 const DEFAULT_SYSLOG_FACILITY: Facility = Facility::LOG_DAEMON;
 
-/// The default limit of provider ASNs in ASPA objects.
-const DEFAULT_ASPA_PROVIDER_LIMIT: usize = 10_000;
-
 
 //------------ Config --------------------------------------------------------  
 
@@ -272,12 +269,6 @@ pub struct Config {
     /// Whether to process ASPA objects.
     pub enable_aspa: bool,
 
-    /// The maximum number of provider ASNs accepted in ASPA objects.
-    ///
-    /// If this number is exceeded, all ASPAs for the customer ASN of the
-    /// offending object are dropped.
-    pub aspa_provider_limit: usize,
-
     /// Whether to not cleanup the repository directory after a validation run.
     ///
     /// If this is `false` and update has not been disabled otherwise, all
@@ -630,11 +621,6 @@ impl Config {
             self.enable_aspa = true
         }
 
-        // aspa_provider_limit
-        if let Some(value) = args.aspa_provider_limit {
-            self.aspa_provider_limit = value;
-        }
-
         // dirty_repository
         if args.dirty_repository {
             self.dirty_repository = true
@@ -988,11 +974,6 @@ impl Config {
 
             enable_aspa: file.take_bool("enable-aspa")?.unwrap_or(false),
 
-            aspa_provider_limit: {
-                file.take_usize("aspa-provider-limit")?
-                    .unwrap_or(DEFAULT_ASPA_PROVIDER_LIMIT)
-            },
-
             dirty_repository: file.take_bool("dirty")?.unwrap_or(false),
             validation_threads: {
                 file.take_small_usize(
@@ -1202,7 +1183,6 @@ impl Config {
             max_ca_depth: DEFAULT_MAX_CA_DEPTH,
             enable_bgpsec: false,
             enable_aspa: false,
-            aspa_provider_limit: DEFAULT_ASPA_PROVIDER_LIMIT,
             dirty_repository: DEFAULT_DIRTY_REPOSITORY,
             validation_threads: Config::default_validation_threads(),
             refresh: Duration::from_secs(DEFAULT_REFRESH),
@@ -1429,7 +1409,6 @@ impl Config {
         insert_int(&mut res, "max-ca-depth", self.max_ca_depth);
         insert(&mut res, "enable-bgpsec", self.enable_bgpsec);
         insert(&mut res, "enable-aspa", self.enable_aspa);
-        insert_int(&mut res, "aspa-provider-limit", self.aspa_provider_limit);
         insert(&mut res, "dirty", self.dirty_repository);
         insert_int(&mut res, "validation-threads", self.validation_threads);
         insert_int(&mut res, "refresh", self.refresh.as_secs());
@@ -1889,10 +1868,6 @@ struct GlobalArgs {
     #[arg(long)]
     enable_aspa: bool,
 
-    /// Maximum number of provider ASNs in ASPA objects
-    #[arg(long, value_name = "COUNT")]
-    aspa_provider_limit: Option<usize>,
-
     /// Do not clean up repository directory after validation
     #[arg(long)]
     dirty_repository: bool,

src/http/metrics.rs

@@ -152,6 +152,16 @@ async fn handle_metrics(
         metrics.local.vrps().contributed
     );
 
+    // Large ASPAs
+    target.single(
+        Metric::new(
+            "aspa_large_provider_set",
+            "ASPAs customer ASNs rejected due to large provider set",
+            MetricType::Gauge
+        ),
+        metrics.snapshot.large_aspas
+    );
+
     // Collector metrics.
     rrdp_metrics(&mut target, &metrics.rrdp);
     rsync_metrics(&mut target, &metrics.rsync);
@@ -799,7 +809,7 @@ fn deprecated_metrics(
         Metric::new(
             "vrps_final", "final number of VRPs", MetricType::Gauge
         ),
-        metrics.payload.vrps().contributed
+        metrics.snapshot.payload.vrps().contributed
     );
 
     // Overall number of stale objects

src/http/status.rs

@@ -115,7 +115,7 @@ async fn handle_status(
     writeln!(res);
 
     // vrps
-    writeln!(res, "vrps: {}", metrics.payload.vrps().valid);
+    writeln!(res, "vrps: {}", metrics.snapshot.payload.vrps().valid);
 
     // vrps-per-tal
     write!(res, "vrps-per-tal: ");
@@ -128,7 +128,7 @@ async fn handle_status(
         // unsafe-filtered-vrps
         writeln!(res,
             "unsafe-vrps: {}",
-            metrics.payload.vrps().marked_unsafe
+            metrics.snapshot.payload.vrps().marked_unsafe
         );
 
         // unsafe-vrps-per-tal
@@ -146,7 +146,7 @@ async fn handle_status(
     // locally-filtered-vrps
     writeln!(res,
         "locally-filtered-vrps: {}",
-        metrics.payload.vrps().locally_filtered
+        metrics.snapshot.payload.vrps().locally_filtered
     );
 
     // locally-filtered-vrps-per-tal
@@ -175,7 +175,7 @@ async fn handle_status(
     // final-vrps
     writeln!(res,
         "final-vrps: {}",
-        metrics.payload.vrps().contributed
+        metrics.snapshot.payload.vrps().contributed
     );
 
     // final-vrps-per-tal
@@ -373,7 +373,12 @@ async fn handle_api_status(
             target.member_raw("lastUpdateDuration", "null");
         }
 
-        json_payload_metrics(target, &metrics.payload);
+        json_payload_metrics(target, &metrics.snapshot.payload);
+
+        target.member_raw(
+            "aspasLargeProviderSet",
+            metrics.snapshot.large_aspas
+        );
 
         target.member_object("tals", |target| {
             for tal in &metrics.tals {

src/metrics.rs

@@ -47,7 +47,7 @@ pub struct Metrics {
     pub local: PayloadMetrics,
 
     /// Overall payload metrics.
-    pub payload: PayloadMetrics,
+    pub snapshot: SnapshotMetrics,
 }
 
 impl Metrics {
@@ -61,7 +61,7 @@ impl Metrics {
             repositories: Vec::new(),
             publication: Default::default(),
             local: Default::default(),
-            payload: Default::default(),
+            snapshot: Default::default(),
         }
     }
 
@@ -74,7 +74,7 @@ impl Metrics {
             metric.finalize();
         }
         self.local.finalize();
-        self.payload.finalize();
+        self.snapshot.finalize();
     }
 
     /// Returns the time the metrics were created as a Unix timestamp.
@@ -370,6 +370,31 @@ impl ops::AddAssign for PublicationMetrics {
 }
 
 
+//------------ SnapshotMetrics -----------------------------------------------
+
+/// Metrics regarding the full payload set.
+#[derive(Clone, Debug, Default)]
+pub struct SnapshotMetrics {
+    /// Payload metrics portion.
+    pub payload: PayloadMetrics,
+
+    /// The number of ASPA customer ASNs that had too large ASPAs.
+    ///
+    /// This number is subtracted from `payload.aspa.valid` through the
+    /// `finalize` method, so you don’t have to do that yourself.
+    pub large_aspas: u32,
+}
+
+impl SnapshotMetrics {
+    /// Finalizes the metrics by summing up the generated attributes.
+    pub fn finalize(&mut self) {
+        self.payload.finalize();
+        self.payload.aspas.valid =
+            self.payload.aspas.valid.saturating_sub(self.large_aspas);
+    }
+}
+
+
 //------------ PayloadMetrics ------------------------------------------------
 
 /// Metrics regarding the generated payload set.
@@ -390,6 +415,9 @@ pub struct PayloadMetrics {
     /// The metrics for ASPA payload.
     pub aspas: VrpMetrics,
 
+    /// The number of ASPA payload that was too large.
+    pub large_aspas: u32,
+
     /// The metrics for all payload items.
     pub all: VrpMetrics,
 }

src/output.rs

@@ -1419,22 +1419,22 @@ impl Summary {
         ))?;
         line(format_args!(
             "            VRPs: {:7} verified, {:7} final;",
-            metrics.payload.vrps().valid,
-            metrics.payload.vrps().contributed
+            metrics.snapshot.payload.vrps().valid,
+            metrics.snapshot.payload.vrps().contributed
         ))?;
         line(format_args!(
             "    router certs: {:7} verified;",
             metrics.publication.valid_router_certs,
         ))?;
         line(format_args!(
             "     router keys: {:7} verified, {:7} final;",
-            metrics.payload.router_keys.valid,
-            metrics.payload.router_keys.contributed
+            metrics.snapshot.payload.router_keys.valid,
+            metrics.snapshot.payload.router_keys.contributed
         ))?;
         line(format_args!(
             "           ASPAs: {:7} verified, {:7} final;",
             metrics.publication.valid_aspas,
-            metrics.payload.aspas.contributed
+            metrics.snapshot.payload.aspas.contributed
         ))?;
         Ok(())
     }