Skip to content

Commit eb522ae

Browse files
authored
feat: complete the Kea hook's DHCP drop census and count replies (#3320)
The Kea hook's drop counter under-counts: two hard-drop paths and six allocation/renewal refusals log without incrementing, and there is no reply counter at all -- so grant rate and the full drop taxonomy are both blind spots on the primary DHCP serving path. This closes both. - `carbide_dhcp_replies_sent_total{message_type}` -- new, counts every reply Kea actually sends (`offer`/`ack`/`nak`/`other`), incremented at the end of `pkt4_send` and suppressed when the exchange was marked dropped. Lease grants are the `offer`/`ack` series, matching the DPU dhcp-server's counter of the same name. - `carbide_dhcp_dropped_requests_total{reason}` -- the exposed name, the `reason` label key, and every pre-existing reason string stay byte-identical for existing dashboards, while eight previously-uncounted paths now increment it: option-encoding failures, missing machine context, and the six allocation/renewal refusals. A refused REQUEST counts both its refusal and the `nak` reply that follows -- both really happened. Notable details: - The raw-string reason plumbing hardens into a bounded `DropReason` enum with a manual `LabelValue` impl pinning the legacy strings verbatim; unrecognized FFI strings bucket to `Unknown` instead of minting new label values. - All events are metric-only (`log = off`): the Kea process has no tracing subscriber, and every existing C++ LOG line stays untouched -- the C++ diff is pure one-line increments beside the drops they count. - An option-encoding failure that throws on several options of one reply counts that packet's drop once (status-guarded). - The in-tree real-Kea integration tests -- the hook loaded into an actual multi-threaded Kea -- pass with the new counters live. Tests added! This supports #3177 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
1 parent 2ab3c5c commit eb522ae

6 files changed

Lines changed: 580 additions & 37 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/dhcp/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ tokio-util = { workspace = true }
3030
tonic = { workspace = true }
3131

3232
# [local-dependencies]
33+
carbide-instrument = { path = "../instrument" }
3334
carbide-rpc = { path = "../rpc" }
3435
carbide-tls = { path = "../tls" }
3536
metrics-endpoint = { path = "../metrics-endpoint" }
@@ -40,6 +41,7 @@ dhcproto = { workspace = true }
4041
serde_json = { workspace = true }
4142
test-cdylib = { workspace = true }
4243
tempfile = { workspace = true }
44+
carbide-instrument = { path = "../instrument", features = ["test-support"] }
4345
carbide-rpc = { path = "../rpc" }
4446
carbide-test-support = { path = "../test-support" }
4547
prometheus = { workspace = true }

crates/dhcp/src/kea/callouts.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ void update_option(CalloutHandle &handle, Pkt4Ptr response4_ptr,
133133
"[%1]. Exception: %2")
134134
.arg(option)
135135
.arg(e.what());
136+
// Several options are updated per reply and each failing update throws;
137+
// count the packet's drop once, on the throw that marks it dropped.
138+
if (handle.getStatus() != CalloutHandle::NEXT_STEP_DROP) {
139+
carbide_increment_dropped_requests("OptionEncodingFailed");
140+
}
136141
handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
137142
}
138143
}
@@ -519,6 +524,7 @@ int pkt4_send(CalloutHandle &handle) {
519524
LOG_ERROR(logger, isc::log::LOG_CARBIDE_PKT4_SEND)
520525
.arg("Missing machine object from handle context");
521526
handle.setStatus(CalloutHandle::NEXT_STEP_DROP);
527+
carbide_increment_dropped_requests("MissingMachineContext");
522528
return 1;
523529
}
524530

@@ -543,6 +549,14 @@ int pkt4_send(CalloutHandle &handle) {
543549
LOG_INFO(logger, isc::log::LOG_CARBIDE_PKT4_SEND)
544550
.arg(response4_ptr->toText());
545551

552+
/*
553+
* The reply is fully assembled; count it by DHCP message type unless an
554+
* option failure above already marked the exchange dropped (and counted).
555+
*/
556+
if (handle.getStatus() != CalloutHandle::NEXT_STEP_DROP) {
557+
carbide_increment_reply_sent(response4_ptr->getType());
558+
}
559+
546560
return 0;
547561
}
548562

@@ -603,6 +617,7 @@ int lease4_select(CalloutHandle &handle) {
603617
.arg("Missing lease4 argument");
604618
// At lease4_select, SKIP means Kea will not assign its selected lease.
605619
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
620+
carbide_increment_dropped_requests("AllocationRefused");
606621
return 1;
607622
}
608623

@@ -620,6 +635,7 @@ int lease4_select(CalloutHandle &handle) {
620635
LOG_ERROR(logger, isc::log::LOG_CARBIDE_LEASE4_SELECT)
621636
.arg("Missing machine object from handle context");
622637
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
638+
carbide_increment_dropped_requests("AllocationRefused");
623639
return 1;
624640
}
625641

@@ -631,6 +647,7 @@ int lease4_select(CalloutHandle &handle) {
631647
LOG_ERROR(logger, isc::log::LOG_CARBIDE_LEASE4_SELECT)
632648
.arg("Carbide returned no usable IPv4 address; refusing to allocate");
633649
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
650+
carbide_increment_dropped_requests("AllocationRefused");
634651
return 1;
635652
}
636653

@@ -683,6 +700,7 @@ int lease4_renew(CalloutHandle &handle) {
683700
.arg("Missing lease4 argument");
684701
// At lease4_renew, SKIP means Kea will not update the lease database.
685702
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
703+
carbide_increment_dropped_requests("RenewalRefused");
686704
return 1;
687705
}
688706

@@ -692,6 +710,7 @@ int lease4_renew(CalloutHandle &handle) {
692710
LOG_ERROR(logger, isc::log::LOG_CARBIDE_LEASE4_RENEW)
693711
.arg("Missing machine object from handle context");
694712
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
713+
carbide_increment_dropped_requests("RenewalRefused");
695714
return 1;
696715
}
697716

@@ -700,6 +719,7 @@ int lease4_renew(CalloutHandle &handle) {
700719
LOG_ERROR(logger, isc::log::LOG_CARBIDE_LEASE4_RENEW)
701720
.arg("Carbide returned no usable IPv4 address; refusing to renew");
702721
handle.setStatus(CalloutHandle::NEXT_STEP_SKIP);
722+
carbide_increment_dropped_requests("RenewalRefused");
703723
return 1;
704724
}
705725

crates/dhcp/src/lib.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ use forge_tls::default as tls_default;
2424
use libc::c_char;
2525
use metrics_endpoint::HealthController;
2626
use once_cell::sync::Lazy;
27-
use opentelemetry::metrics::Counter;
2827
use rpc::forge_tls_client::ForgeClientConfig;
2928
use tokio::runtime::{Builder, Runtime};
3029

@@ -62,10 +61,11 @@ pub struct CarbideDhcpContext {
6261
startup_time: chrono::DateTime<chrono::Utc>,
6362
}
6463

64+
// The request/drop/reply counters are `carbide-instrument` events declared in
65+
// `metrics.rs` and resolve from the global meter; this struct holds only the
66+
// state the certificate-expiry gauge reports.
6567
#[derive(Debug, Clone)]
6668
pub struct CarbideDhcpMetrics {
67-
total_requests_counter: Counter<u64>,
68-
dropped_requests_counter: Counter<u64>,
6969
forge_client_config: ForgeClientConfig,
7070
certificate_expiration_value: Arc<AtomicI64>,
7171
}
@@ -238,17 +238,37 @@ pub unsafe extern "C" fn carbide_increment_total_requests() {
238238
metrics::increment_total_requests();
239239
}
240240

241-
/// Increments counter for number of dropped requests
241+
/// Increments counter for number of dropped or refused requests. The reason
242+
/// string is mapped onto the bounded [`metrics::DropReason`] taxonomy; a
243+
/// string outside the taxonomy (or a null / non-UTF-8 input) is bucketed as
244+
/// `Unknown` so the metric's label domain stays closed.
245+
///
246+
/// # Safety
247+
/// Function is unsafe as it dereferences a raw pointer given to it. Caller is responsible
248+
/// to validate that the pointer passed to it meets the necessary conditions to be dereferenced.
249+
#[unsafe(no_mangle)]
250+
pub unsafe extern "C" fn carbide_increment_dropped_requests(reason: *const c_char) {
251+
let reason = if reason.is_null() {
252+
metrics::DropReason::Unknown
253+
} else {
254+
unsafe { CStr::from_ptr(reason) }
255+
.to_str()
256+
.map_or(metrics::DropReason::Unknown, metrics::DropReason::from)
257+
};
258+
metrics::increment_dropped_requests(reason);
259+
}
260+
261+
/// Increments counter for number of DHCP replies sent, labelled by the
262+
/// reply's message type. `message_type` is the raw RFC 2131 message-type code
263+
/// from the response packet (`Pkt4::getType()`); the mapping onto the bounded
264+
/// label lives in [`metrics::ReplyMessageType`].
242265
///
243266
/// # Safety
244267
///
245268
/// None
246269
#[unsafe(no_mangle)]
247-
pub unsafe extern "C" fn carbide_increment_dropped_requests(reason: *const c_char) {
248-
unsafe {
249-
let reason_value = CStr::from_ptr(reason).to_str().unwrap().to_owned();
250-
metrics::increment_dropped_requests(reason_value);
251-
}
270+
pub extern "C" fn carbide_increment_reply_sent(message_type: u8) {
271+
metrics::increment_reply_sent(metrics::ReplyMessageType::from(message_type));
252272
}
253273

254274
#[cfg(test)]

0 commit comments

Comments
 (0)