Git SHA: [email protected]
Author: Gaetan Rivet <[email protected]>
Subject: [RFC PATCH 05/12] ofproto: Add ofproto packet stats function
This commit adds a function to read current ofproto packet stats by
iterating over ofproto rules and reading their counters. Removed rules
sum their counters within ofproto upon being marked for removal.
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 7df3f52469..3ca87748bf 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -57,6 +57,7 @@
> #include "simap.h"
> #include "timeval.h"
> #include "tun-metadata.h"
> +#include "util.h"
> #include "versions.h"
> #include "vl-mff-map.h"
>
[ ... ]
> +struct pkt_stats {
> + uint64_t n_packets;
> + uint64_t n_bytes;
> + uint64_t n_offload_packets; /* n_offload_packets are a subset n_packets
> */
> + uint64_t n_offload_bytes; /* n_offload_bytes are a subset of n_bytes */
> +};
> +
> +static inline void
> +pkt_stats_add(struct pkt_stats *dst, struct pkt_stats src)
> +{
> + dst->n_packets = ovs_u64_safeadd(dst->n_packets, src.n_packets);
> + dst->n_bytes = ovs_u64_safeadd(dst->n_bytes, src.n_bytes);
> + dst->n_offload_packets = ovs_u64_safeadd(dst->n_offload_packets,
> + src.n_offload_packets);
> + dst->n_offload_bytes = ovs_u64_safeadd(dst->n_offload_bytes,
> + src.n_offload_bytes);
Is the ovs_u64_safeadd function available? I noticed it was mentioned in
recent commits but want to confirm it exists before this patch is applied.
> @@ -119,6 +138,7 @@ struct ofproto {
>
> /* List of flows to remove from flow tables. */
> struct rule_collection *to_remove OVS_GUARDED_BY(ofproto_mutex);
> + struct pkt_stats removed_stats OVS_GUARDED_BY(ofproto_mutex);
[ ... ]
> @@ -608,13 +634,6 @@ struct ofgroup {
> struct rule_collection rules OVS_GUARDED; /* Referring rules. */
> };
>
> -struct pkt_stats {
> - uint64_t n_packets;
> - uint64_t n_bytes;
> - uint64_t n_offload_packets; /* n_offload_packets are a subset n_packets
> */
> - uint64_t n_offload_bytes; /* n_offload_bytes are a subset of n_bytes */
> -};
[ ... ]
> @@ -4589,12 +4589,20 @@ rule_criteria_destroy(struct rule_criteria *criteria)
> /* Adds rules to the 'to_remove' collection, so they can be destroyed
> * later all together. Destroys 'rules'. */
> static void
> -rules_mark_for_removal(struct ofproto *ofproto, struct rule_collection
> *rules)
> +rules_mark_for_removal(struct ofproto *ofproto, struct rule_collection
> *rules,
> + bool keep_counts)
> OVS_REQUIRES(ofproto_mutex)
> {
> struct rule *rule;
>
> RULE_COLLECTION_FOR_EACH (rule, rules) {
> + if (keep_counts) {
> + struct pkt_stats stats;
> + long long int used;
> +
> + ofproto->ofproto_class->rule_get_stats(rule, &stats, &used);
> + pkt_stats_add(&ofproto->removed_stats, stats);
> + }
> rule_collection_add(ofproto->to_remove, rule);
> }
> rule_collection_destroy(rules);
[ ... ]
> +enum ofperr
> +ofproto_get_pkt_stats(struct ofproto *ofproto, struct pkt_stats *stats)
> + OVS_EXCLUDED(ofproto_mutex)
> +{
> + struct pkt_stats removed_stats;
> + struct rule_criteria criteria;
> + struct rule_collection rules;
> + struct minimatch match;
> + struct rule *rule;
> + enum ofperr error;
> +
> + /* Match *any* rule. */
> + minimatch_init_catchall(&match);
> + rule_criteria_init(&criteria, OFPTT_ALL, &match, 0,
> + OVS_VERSION_MAX, 0, 0,
> + OFPP_ANY, OFPG_ANY);
> + minimatch_destroy(&match);
Could minimatch_destroy be called before using criteria in collect_rules_loose?
The criteria structure might reference the match internally, making early
destruction potentially unsafe.
> +
> + ovs_mutex_lock(&ofproto_mutex);
> + error = collect_rules_loose(ofproto, &criteria, &rules);
> + rule_criteria_destroy(&criteria);
> + if (!error) {
> + rule_collection_ref(&rules);
> + }
> + removed_stats = ofproto->removed_stats;
> + ovs_mutex_unlock(&ofproto_mutex);
> +
> + if (error) {
> + return error;
> + }
> +
> + *stats = removed_stats;
> +
> + RULE_COLLECTION_FOR_EACH (rule, &rules) {
> + struct pkt_stats rule_stats;
> + long long int used;
> +
> + ofproto->ofproto_class->rule_get_stats(rule, &rule_stats, &used);
> + pkt_stats_add(stats, rule_stats);
> + }
> +
> + rule_collection_unref(&rules);
> + rule_collection_destroy(&rules);
> +
> + return error;
Should ofproto_get_pkt_stats initialize the removed_stats struct to zero
in the removed_stats field of ofproto somewhere during ofproto initialization?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
