Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure limiting of ASPA provider set sizes. #996

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions doc/manual/source/manual-page.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 --------------------------------------------------------

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 11 additions & 1 deletion src/http/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
15 changes: 10 additions & 5 deletions src/http/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: ");
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 31 additions & 3 deletions src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub struct Metrics {
pub local: PayloadMetrics,

/// Overall payload metrics.
pub payload: PayloadMetrics,
pub snapshot: SnapshotMetrics,
}

impl Metrics {
Expand All @@ -61,7 +61,7 @@ impl Metrics {
repositories: Vec::new(),
publication: Default::default(),
local: Default::default(),
payload: Default::default(),
snapshot: Default::default(),
}
}

Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
}
Expand Down
10 changes: 5 additions & 5 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
Loading
Loading