Hi Lorenzo, Thanks for the review. I can update the NEWS to remove the CLI details. The error counters are really useful when monitoring a System using Prometheus and Grafana and triggering alerts for critical workloads.
Thanks, Ketan On Mon, Feb 2, 2026 at 3:15 AM Lorenzo Bianconi <[email protected]> wrote: > > Add comprehensive DHCP statistics tracking in ovn-controller using OVS > > coverage counters for observability into DHCP query processing. > > > > Tracked metrics include: > > - Total queries processed (DHCPv4 and DHCPv6) > > - DHCPv4 message types (DISCOVER, REQUEST, DECLINE, RELEASE, INFORM) > > - DHCPv6 message types (SOLICIT, REQUEST, CONFIRM, DECLINE, RELEASE, > > INFO-REQUEST) > > - Responses sent (OFFER, ACK, NAK for v4; ADVERTISE, REPLY for v6) > > - Error conditions (truncated, invalid opcode, missing message type, > etc.) > > > > Statistics can be queried using: > > ovn-appctl -t ovn-controller coverage/read-counter <counter_name> > > ovn-appctl -t ovn-controller coverage/show > > > Hi Ketan, > > > > > Test coverage validates all DHCPv4 message types (DISCOVER, REQUEST, > > DECLINE, RELEASE, INFORM) and response types (OFFER, ACK, NAK), as well > > as DHCPv6 message types (SOLICIT, RELEASE, INFO_REQUEST) and responses > > (ADVERTISE, REPLY). Counter checks are placed before controller restarts > > to ensure reliable automated testing, following the pattern used for DNS > > coverage counter validation. > > > > Signed-off-by: Ketan Supanekar <[email protected]> > > --- > > NEWS | 8 +++++ > > controller/pinctrl.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > tests/ovn.at | 54 ++++++++++++++++++++++++++++ > > 3 files changed, 146 insertions(+) > > > > diff --git a/NEWS b/NEWS > > index b7268c835..e492b4081 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -5,6 +5,14 @@ Post v25.09.0 > > coverage/read-counter <counter_name>" or "coverage/show". Tracked > metrics > > include total queries, query types (A, AAAA, PTR, ANY, Other), > cache > > performance (hits/misses), responses sent, and error conditions. > > + - Added DHCP statistics tracking in ovn-controller using OVS coverage > > + counters. Statistics can be queried using "ovn-appctl -t > ovn-controller > > + coverage/read-counter <counter_name>" or "coverage/show". Tracked > metrics > > I guess it is not important to provide these info here. > > > + include total queries, DHCPv4 message types (DISCOVER, REQUEST, > DECLINE, > > + RELEASE, INFORM), DHCPv6 message types (SOLICIT, REQUEST, CONFIRM, > > + DECLINE, RELEASE, INFO-REQUEST), responses sent (OFFER, ACK, NAK > for v4; > > + ADVERTISE, REPLY for v6), and error conditions (truncated packets, > > + invalid opcodes, missing message types, missing client IDs, etc.). > > - Added support for TLS Server Name Indication (SNI) with the new > > --ssl-server-name option in OVN utilities and daemons. This allows > > specifying the server name for SNI, which is useful when connecting > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index ab8a0a37c..cfc0ac278 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -407,6 +407,33 @@ COVERAGE_DEFINE(dns_error_parse_failure); > > COVERAGE_DEFINE(dns_unsupported_ovn_owned); > > COVERAGE_DEFINE(dns_response_sent); > > > > +/* DHCP statistics - thread-safe coverage counters */ > > +COVERAGE_DEFINE(dhcp_query_total); > > +COVERAGE_DEFINE(dhcp_v4_discover); > > +COVERAGE_DEFINE(dhcp_v4_request); > > +COVERAGE_DEFINE(dhcp_v4_decline); > > +COVERAGE_DEFINE(dhcp_v4_release); > > +COVERAGE_DEFINE(dhcp_v4_inform); > > +COVERAGE_DEFINE(dhcp_v4_offer_sent); > > +COVERAGE_DEFINE(dhcp_v4_ack_sent); > > +COVERAGE_DEFINE(dhcp_v4_nak_sent); > > +COVERAGE_DEFINE(dhcp_v6_solicit); > > +COVERAGE_DEFINE(dhcp_v6_request); > > +COVERAGE_DEFINE(dhcp_v6_confirm); > > +COVERAGE_DEFINE(dhcp_v6_decline); > > +COVERAGE_DEFINE(dhcp_v6_release); > > +COVERAGE_DEFINE(dhcp_v6_info_request); > > +COVERAGE_DEFINE(dhcp_v6_advertise_sent); > > +COVERAGE_DEFINE(dhcp_v6_reply_sent); > > +COVERAGE_DEFINE(dhcp_error_truncated); > > +COVERAGE_DEFINE(dhcp_error_invalid_opcode); > > +COVERAGE_DEFINE(dhcp_error_missing_msg_type); > > +COVERAGE_DEFINE(dhcp_error_invalid_msg_type); > > +COVERAGE_DEFINE(dhcp_error_missing_client_id); > > +COVERAGE_DEFINE(dhcp_error_missing_iaid); > > +COVERAGE_DEFINE(dhcp_error_userdata); > > +COVERAGE_DEFINE(dhcp_error_ip_mismatch); > > Do we really need to provide all these details here? In particular all > error > conditions already trigger an error log. > > Regards, > Lorenzo > > > + > > struct empty_lb_backends_event { > > struct hmap_node hmap_node; > > long long int timestamp; > > @@ -2552,6 +2579,9 @@ pinctrl_handle_put_dhcp_opts( > > struct ofpbuf *dhcp_inform_reply_buf = NULL; > > uint32_t success = 0; > > > > + /* Track total DHCP queries received */ > > + COVERAGE_INC(dhcp_query_total); > > + > > /* Parse result field. */ > > const struct mf_field *f; > > enum ofperr ofperr = nx_pull_header(userdata, NULL, &f, NULL); > > @@ -2591,6 +2621,7 @@ pinctrl_handle_put_dhcp_opts( > > dhcp_get_hdr_from_pkt(pkt_in, &in_dhcp_ptr, end); > > > > if (!in_dhcp_data) { > > + COVERAGE_INC(dhcp_error_truncated); > > goto exit; > > } > > ovs_assert(in_dhcp_ptr); > > @@ -2599,6 +2630,7 @@ pinctrl_handle_put_dhcp_opts( > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > VLOG_WARN_RL(&rl, "Invalid opcode in the DHCP packet: %d", > > in_dhcp_data->op); > > + COVERAGE_INC(dhcp_error_invalid_opcode); > > goto exit; > > } > > > > @@ -2614,6 +2646,7 @@ pinctrl_handle_put_dhcp_opts( > > if (!dhcp_opts.dhcp_msg_type) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > VLOG_WARN_RL(&rl, "Missing DHCP message type"); > > + COVERAGE_INC(dhcp_error_missing_msg_type); > > goto exit; > > } > > > > @@ -2622,9 +2655,11 @@ pinctrl_handle_put_dhcp_opts( > > > > switch (dhcp_opts.dhcp_msg_type) { > > case DHCP_MSG_DISCOVER: > > + COVERAGE_INC(dhcp_v4_discover); > > msg_type = DHCP_MSG_OFFER; > > break; > > case DHCP_MSG_REQUEST: { > > + COVERAGE_INC(dhcp_v4_request); > > msg_type = DHCP_MSG_ACK; > > if (dhcp_opts.request_ip != *offer_ip) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > 5); > > @@ -2632,11 +2667,13 @@ pinctrl_handle_put_dhcp_opts( > > "match offer "IP_FMT, > > IP_ARGS(dhcp_opts.request_ip), > > IP_ARGS(*offer_ip)); > > + COVERAGE_INC(dhcp_error_ip_mismatch); > > msg_type = DHCP_MSG_NAK; > > } > > break; > > } > > case OVN_DHCP_MSG_RELEASE: { > > + COVERAGE_INC(dhcp_v4_release); > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 40); > > const struct eth_header *l2 = dp_packet_eth(pkt_in); > > VLOG_INFO_RL(&rl, "DHCPRELEASE "ETH_ADDR_FMT " "IP_FMT"", > > @@ -2645,6 +2682,7 @@ pinctrl_handle_put_dhcp_opts( > > break; > > } > > case OVN_DHCP_MSG_INFORM: { > > + COVERAGE_INC(dhcp_v4_inform); > > /* RFC 2131 section 3.4. > > * Remove all the offer ip related dhcp options and > > * all the time related dhcp options. > > @@ -2695,6 +2733,7 @@ pinctrl_handle_put_dhcp_opts( > > break; > > } > > case OVN_DHCP_MSG_DECLINE: > > + COVERAGE_INC(dhcp_v4_decline); > > if (dhcp_opts.request_ip == *offer_ip) { > > VLOG_INFO("DHCPDECLINE from "ETH_ADDR_FMT ", "IP_FMT" > duplicated", > > ETH_ADDR_ARGS(in_flow->dl_src), > IP_ARGS(*offer_ip)); > > @@ -2704,6 +2743,7 @@ pinctrl_handle_put_dhcp_opts( > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > VLOG_WARN_RL(&rl, "Invalid DHCP message type: %d", > > dhcp_opts.dhcp_msg_type); > > + COVERAGE_INC(dhcp_error_invalid_msg_type); > > goto exit; > > } > > } > > @@ -2865,6 +2905,19 @@ pinctrl_handle_put_dhcp_opts( > > (msg_type == DHCP_MSG_ACK ? "ACK": "NAK"), > > ETH_ADDR_ARGS(l2->eth_src), IP_ARGS(*offer_ip)); > > > > + /* Track DHCP responses sent */ > > + switch (msg_type) { > > + case DHCP_MSG_OFFER: > > + COVERAGE_INC(dhcp_v4_offer_sent); > > + break; > > + case DHCP_MSG_ACK: > > + COVERAGE_INC(dhcp_v4_ack_sent); > > + break; > > + case DHCP_MSG_NAK: > > + COVERAGE_INC(dhcp_v4_nak_sent); > > + break; > > + } > > + > > success = 1; > > exit: > > if (!ofperr) { > > @@ -3083,6 +3136,9 @@ pinctrl_handle_put_dhcpv6_opts( > > struct dp_packet *pkt_out_ptr = NULL; > > uint32_t success = 0; > > > > + /* Track total DHCP queries received */ > > + COVERAGE_INC(dhcp_query_total); > > + > > /* Parse result field. */ > > const struct mf_field *f; > > enum ofperr ofperr = nx_pull_header(userdata, NULL, &f, NULL); > > @@ -3115,6 +3171,7 @@ pinctrl_handle_put_dhcpv6_opts( > > const uint8_t *in_dhcpv6_data = dp_packet_get_udp_payload(pkt_in); > > if (!in_udp || !in_dhcpv6_data) { > > VLOG_WARN_RL(&rl, "truncated dhcpv6 packet"); > > + COVERAGE_INC(dhcp_error_truncated); > > goto exit; > > } > > > > @@ -3123,23 +3180,36 @@ pinctrl_handle_put_dhcpv6_opts( > > bool status_only = false; > > switch (in_dhcpv6_msg_type) { > > case DHCPV6_MSG_TYPE_SOLICIT: > > + COVERAGE_INC(dhcp_v6_solicit); > > out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_ADVT; > > break; > > > > case DHCPV6_MSG_TYPE_REQUEST: > > + COVERAGE_INC(dhcp_v6_request); > > + out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_REPLY; > > + break; > > case DHCPV6_MSG_TYPE_CONFIRM: > > + COVERAGE_INC(dhcp_v6_confirm); > > + out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_REPLY; > > + break; > > case DHCPV6_MSG_TYPE_DECLINE: > > + COVERAGE_INC(dhcp_v6_decline); > > + out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_REPLY; > > + break; > > case DHCPV6_MSG_TYPE_INFO_REQ: > > + COVERAGE_INC(dhcp_v6_info_request); > > out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_REPLY; > > break; > > > > case DHCPV6_MSG_TYPE_RELEASE: > > + COVERAGE_INC(dhcp_v6_release); > > out_dhcpv6_msg_type = DHCPV6_MSG_TYPE_REPLY; > > status_only = true; > > break; > > > > default: > > /* Invalid or unsupported DHCPv6 message type */ > > + COVERAGE_INC(dhcp_error_invalid_msg_type); > > goto exit; > > } > > /* Skip 4 bytes (message type (1 byte) + transaction ID (3 bytes). > */ > > @@ -3194,12 +3264,14 @@ pinctrl_handle_put_dhcpv6_opts( > > if (!in_opt_client_id) { > > VLOG_WARN_RL(&rl, "DHCPv6 option - Client id not present in the > " > > "DHCPv6 packet"); > > + COVERAGE_INC(dhcp_error_missing_client_id); > > goto exit; > > } > > > > if (!iaid && in_dhcpv6_msg_type != DHCPV6_MSG_TYPE_INFO_REQ) { > > VLOG_WARN_RL(&rl, "DHCPv6 option - IA NA not present in the " > > "DHCPv6 packet"); > > + COVERAGE_INC(dhcp_error_missing_iaid); > > goto exit; > > } > > > > @@ -3217,6 +3289,7 @@ pinctrl_handle_put_dhcpv6_opts( > > > > if (!compose) { > > VLOG_WARN_RL(&rl, "Invalid userdata"); > > + COVERAGE_INC(dhcp_error_userdata); > > goto exit; > > } > > > > @@ -3274,6 +3347,17 @@ pinctrl_handle_put_dhcpv6_opts( > > pin->packet = dp_packet_data(&pkt_out); > > pin->packet_len = dp_packet_size(&pkt_out); > > ofpbuf_uninit(&out_dhcpv6_opts); > > + > > + /* Track DHCPv6 responses sent */ > > + switch (out_dhcpv6_msg_type) { > > + case DHCPV6_MSG_TYPE_ADVT: > > + COVERAGE_INC(dhcp_v6_advertise_sent); > > + break; > > + case DHCPV6_MSG_TYPE_REPLY: > > + COVERAGE_INC(dhcp_v6_reply_sent); > > + break; > > + } > > + > > success = 1; > > exit: > > if (!ofperr) { > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 4d15d4a62..d2e4ff068 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -7409,6 +7409,37 @@ > expected_dhcp_opts=${boofile}330400000e100104ffffff0003040a00000136040a000001 > > test_dhcp 19 1 f00000000001 01 0 $ciaddr $offer_ip $request_ip 1 0 > ff1000000001 $server_ip 02 $expected_dhcp_opts > > compare_dhcp_packets 1 > > > > +AS_BOX([Verify DHCP coverage counters]) > > +# The test has sent multiple DHCP packets (tests 1-19), verify counters > > +as hv1 > > +# Total queries should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_query_total > | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# DISCOVER messages (tests 1, 19) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_discover > | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# REQUEST messages (tests 2, 3, 5, 6, 7, 8, 13, 15, 16, 17) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_request | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# DECLINE messages (test 18) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_decline | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# RELEASE messages (test 11) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_release | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# INFORM messages (tests 12, 14) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_inform | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# OFFER responses (tests 1, 18, 19) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter > dhcp_v4_offer_sent | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# ACK responses (tests 2, 5, 6, 12, 13, 14, 15, 16, 17) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_ack_sent > | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# NAK responses (tests 3, 7, 8) > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v4_nak_sent > | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > + > > # Test that ovn-controller pinctrl thread handles dhcp requests when it > > # connects to a wrong version of ovn-northd at startup. > > > > @@ -7477,6 +7508,7 @@ OVN_CLEANUP_SBOX([hv1], ["/DHCP/d > > ]) > > OVN_CLEANUP_DBS > > OVN_CLEANUP_VSWITCH([main]) > > + > > AT_CLEANUP > > ]) > > > > @@ -7812,6 +7844,28 @@ reset_pcap_file hv1-vif2 hv1/vif2 > > test_dhcpv6_release 2 $src_mac $src_lla $offer_ip > > check_packets 2 > > > > +AS_BOX([Verify DHCPv6 coverage counters]) > > +# The test has sent multiple DHCPv6 packets, verify counters > > +as hv1 > > +# Total queries should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_query_total > | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# SOLICIT messages should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v6_solicit | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# RELEASE messages should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter dhcp_v6_release | > awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# INFO_REQUEST messages should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter > dhcp_v6_info_request | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# ADVERTISE responses should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter > dhcp_v6_advertise_sent | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > +# REPLY responses should be > 0 > > +OVS_WAIT_FOR_OUTPUT([ovn-appctl coverage/read-counter > dhcp_v6_reply_sent | awk '{if ($1 > 0) print "ok"}'], [0], [ok > > +]) > > + > > OVN_CLEANUP([hv1]) > > > > AT_CLEANUP > > -- > > 2.52.0 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
