Hi Ales, Thanks for the review and caching the race condition with non x86 architectures. I have sent out a V5 patch with Coverage counters instead, please check.
Thanks, Ketan On Thu, Jan 15, 2026 at 11:38 PM Ales Musil <[email protected]> wrote: > > > On Thu, Jan 15, 2026 at 8:02 PM Ketan Supanekar via dev < > [email protected]> wrote: > >> Add comprehensive DNS query statistics tracking to ovn-controller >> for observability into DNS query processing. >> >> The statistics track query types (A, AAAA, PTR, ANY, Other), cache >> performance (hits and misses), processing errors, and responses sent. >> >> A new unixctl command 'dns/show-stats' displays the statistics. >> >> Signed-off-by: Ketan Supanekar <[email protected]> >> --- >> > > Hello Ketan, > > Thank you for the patch. I have a general question, did you consider using > coverage counters instead? They provide better statistics IMO as you could > also see rates of requests. It's a bit more memory demanding. > > controller/ovn-controller.c | 15 ++++++++ >> controller/pinctrl.c | 75 +++++++++++++++++++++++++++++++++++++ >> controller/pinctrl.h | 31 +++++++++++++++ >> 3 files changed, 121 insertions(+) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 2d9b3e033..f5ff85ffa 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -121,6 +121,7 @@ static unixctl_cb_func debug_dump_peer_ports; >> static unixctl_cb_func debug_dump_lflow_conj_ids; >> static unixctl_cb_func lflow_cache_flush_cmd; >> static unixctl_cb_func lflow_cache_show_stats_cmd; >> +static unixctl_cb_func dns_show_stats_cmd; >> static unixctl_cb_func debug_delay_nb_cfg_report; >> >> #define DEFAULT_BRIDGE_NAME "br-int" >> @@ -7377,6 +7378,9 @@ main(int argc, char *argv[]) >> lflow_cache_show_stats_cmd, >> &lflow_output_data->pd); >> >> + unixctl_command_register("dns/show-stats", "", 0, 0, >> + dns_show_stats_cmd, NULL); >> + >> bool reset_ovnsb_idl_min_index = false; >> unixctl_command_register("sb-cluster-state-reset", "", 0, 0, >> cluster_state_reset_cmd, >> @@ -8401,6 +8405,17 @@ lflow_cache_show_stats_cmd(struct unixctl_conn >> *conn, int argc OVS_UNUSED, >> ds_destroy(&ds); >> } >> >> +static void >> +dns_show_stats_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED, >> + const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED) >> +{ >> + struct ds ds = DS_EMPTY_INITIALIZER; >> + >> + pinctrl_get_dns_stats(&ds); >> > > I'm afraid there is a race condition, as it is written right now the > counters are incremented in pinctrl thread, but will be read > from the main thread. One possible way to avoid that would > be to lock in the pinctrl_get_dns_stats. Other way might > be to use atomics for all stats or as mentioned above the > coverage counters take care of that too. > > + unixctl_command_reply(conn, ds_cstr(&ds)); >> + ds_destroy(&ds); >> +} >> + >> static void >> cluster_state_reset_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED, >> const char *argv[] OVS_UNUSED, void *idl_reset_) >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 6f7ae4037..fb5539e66 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -184,6 +184,9 @@ struct pinctrl { >> >> static struct pinctrl pinctrl; >> >> +/* DNS query statistics */ >> +static struct dns_stats dns_statistics = {0}; >> + >> static bool pinctrl_is_sb_commited(int64_t commit_cfg, int64_t cur_cfg); >> static void init_buffered_packets_ctx(void); >> static void destroy_buffered_packets_ctx(void); >> @@ -3366,6 +3369,9 @@ pinctrl_handle_dns_lookup( >> uint32_t success = 0; >> bool send_refuse = false; >> >> + /* Track total DNS queries received */ >> + dns_statistics.total_queries++; >> + >> /* Parse result field. */ >> const struct mf_field *f; >> enum ofperr ofperr = nx_pull_header(userdata, NULL, &f, NULL); >> @@ -3392,6 +3398,7 @@ pinctrl_handle_dns_lookup( >> /* Check that the packet stores at least the minimal headers. */ >> if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN + DNS_HEADER_LEN)) { >> VLOG_WARN_RL(&rl, "truncated dns packet"); >> + dns_statistics.error_truncated++; >> goto exit; >> } >> >> @@ -3399,17 +3406,20 @@ pinctrl_handle_dns_lookup( >> struct dns_header const *in_dns_header = >> dp_packet_get_udp_payload(pkt_in); >> if (!in_dns_header) { >> VLOG_WARN_RL(&rl, "truncated dns packet"); >> + dns_statistics.error_truncated++; >> goto exit; >> } >> >> /* Check if it is DNS request or not */ >> if (in_dns_header->lo_flag & 0x80) { >> /* It's a DNS response packet which we are not interested in */ >> + dns_statistics.skipped_not_request++; >> goto exit; >> } >> >> /* Check if at least one query request is present */ >> if (!in_dns_header->qdcount) { >> + dns_statistics.error_no_query++; >> goto exit; >> } >> >> @@ -3431,6 +3441,7 @@ pinctrl_handle_dns_lookup( >> uint8_t label_len = in_dns_data[idx++]; >> if (in_dns_data + idx + label_len > end) { >> ds_destroy(&query_name); >> + dns_statistics.error_parse_failure++; >> goto exit; >> } >> ds_put_buffer(&query_name, (const char *) in_dns_data + idx, >> label_len); >> @@ -3449,6 +3460,20 @@ pinctrl_handle_dns_lookup( >> } >> >> uint16_t query_type = ntohs(get_unaligned_be16((void *) >> in_dns_data)); >> + >> + /* Track query type statistics */ >> + if (query_type == DNS_QUERY_TYPE_A) { >> + dns_statistics.query_type_a++; >> + } else if (query_type == DNS_QUERY_TYPE_AAAA) { >> + dns_statistics.query_type_aaaa++; >> + } else if (query_type == DNS_QUERY_TYPE_PTR) { >> + dns_statistics.query_type_ptr++; >> + } else if (query_type == DNS_QUERY_TYPE_ANY) { >> + dns_statistics.query_type_any++; >> + } else { >> + dns_statistics.query_type_other++; >> + } >> + >> /* Supported query types - A, AAAA, ANY and PTR */ >> if (!(query_type == DNS_QUERY_TYPE_A || query_type == >> DNS_QUERY_TYPE_AAAA >> || query_type == DNS_QUERY_TYPE_ANY >> @@ -3467,8 +3492,10 @@ pinctrl_handle_dns_lookup( >> &ovn_owned); >> ds_destroy(&query_name); >> if (!answer_data) { >> + dns_statistics.cache_misses++; >> goto exit; >> } >> + dns_statistics.cache_hits++; >> >> >> uint16_t ancount = 0; >> @@ -3511,6 +3538,7 @@ pinctrl_handle_dns_lookup( >> if (ovn_owned && (query_type == DNS_QUERY_TYPE_AAAA || >> query_type == DNS_QUERY_TYPE_A) && !ancount) { >> send_refuse = true; >> + dns_statistics.unsupported_ovn_owned++; >> } >> >> destroy_lport_addresses(&ip_addrs); >> @@ -3595,6 +3623,7 @@ pinctrl_handle_dns_lookup( >> pin->packet_len = dp_packet_size(&pkt_out); >> >> success = 1; >> + dns_statistics.responses_sent++; >> exit: >> if (!ofperr) { >> union mf_subvalue sv; >> @@ -8863,3 +8892,49 @@ set_from_ctrl_flag_in_pkt_metadata(struct >> ofputil_packet_in *pin) >> sv.u8_val = 1; >> mf_write_subfield(&dst, &sv, &pin->flow_metadata); >> } >> + >> +/* DNS Statistics functions */ >> +void >> +pinctrl_get_dns_stats(struct ds *output) >> +{ >> + ds_put_format(output, "DNS Query Statistics\n"); >> + ds_put_format(output, "====================\n\n"); >> + >> + ds_put_format(output, "Total queries received: %"PRIu64"\n\n", >> + dns_statistics.total_queries); >> + >> + ds_put_format(output, "Query Types:\n"); >> + ds_put_format(output, " A (IPv4): %"PRIu64"\n", >> + dns_statistics.query_type_a); >> + ds_put_format(output, " AAAA (IPv6): %"PRIu64"\n", >> + dns_statistics.query_type_aaaa); >> + ds_put_format(output, " PTR: %"PRIu64"\n", >> + dns_statistics.query_type_ptr); >> + ds_put_format(output, " ANY: %"PRIu64"\n", >> + dns_statistics.query_type_any); >> + ds_put_format(output, " Other: %"PRIu64"\n\n", >> + dns_statistics.query_type_other); >> + >> + ds_put_format(output, "Cache Performance:\n"); >> + ds_put_format(output, " Cache hits: %"PRIu64"\n", >> + dns_statistics.cache_hits); >> + ds_put_format(output, " Cache misses: %"PRIu64"\n\n", >> + dns_statistics.cache_misses); >> + >> + ds_put_format(output, >> + "Processing Errors (packets reinjected to >> pipeline):\n"); >> + ds_put_format(output, " Truncated packets: %"PRIu64"\n", >> + dns_statistics.error_truncated); >> + ds_put_format(output, " Skipped (not request): %"PRIu64"\n", >> + dns_statistics.skipped_not_request); >> + ds_put_format(output, " No query section: %"PRIu64"\n", >> + dns_statistics.error_no_query); >> + ds_put_format(output, " Parse failures: %"PRIu64"\n", >> + dns_statistics.error_parse_failure); >> + ds_put_format(output, " Unsupported OVN-owned: %"PRIu64"\n\n", >> + dns_statistics.unsupported_ovn_owned); >> + >> + ds_put_format(output, "Responses:\n"); >> + ds_put_format(output, " Responses sent: %"PRIu64"\n", >> + dns_statistics.responses_sent); >> +} >> diff --git a/controller/pinctrl.h b/controller/pinctrl.h >> index 80384ac9b..7df676669 100644 >> --- a/controller/pinctrl.h >> +++ b/controller/pinctrl.h >> @@ -83,4 +83,35 @@ void send_self_originated_neigh_packet(struct rconn >> *swconn, >> struct in6_addr *local, >> struct in6_addr *target, >> uint8_t table_id); >> + >> +/* DNS Statistics */ >> +struct dns_stats { >> + /* Total queries received */ >> + uint64_t total_queries; >> + >> + /* Queries by type */ >> + uint64_t query_type_a; /* IPv4 address lookups */ >> + uint64_t query_type_aaaa; /* IPv6 address lookups */ >> + uint64_t query_type_ptr; /* Reverse DNS lookups */ >> + uint64_t query_type_any; /* ANY type queries */ >> + uint64_t query_type_other; /* Other/unsupported types */ >> + >> + /* Cache performance */ >> + uint64_t cache_hits; /* Queries with answers found */ >> + uint64_t cache_misses; /* Queries without answers */ >> + >> + /* Processing errors (all packets reinjected to pipeline) */ >> + uint64_t error_truncated; /* Malformed/truncated packets */ >> + uint64_t skipped_not_request; /* DNS responses (not queries) */ >> + uint64_t error_no_query; /* No query section present */ >> + uint64_t error_parse_failure; /* Query name parsing failure */ >> + uint64_t unsupported_ovn_owned; /* Unsupported query on OVN-owned >> record */ >> + >> + /* Responses sent */ >> + uint64_t responses_sent; /* Successfully generated responses */ >> +}; >> + >> +struct ds; >> +void pinctrl_get_dns_stats(struct ds *output); >> + >> #endif /* controller/pinctrl.h */ >> -- >> 2.52.0 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Regards, > Ales > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
