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
