On 2022-02-28 8:03 PM, Mike Pattrick wrote:
Currently tc offload flow packet counters will roll over every ~4
billion packets. This is because the packet counter in struct
tc_stats provided by TCA_STATS_BASIC is a 32bit integer.
Now we check for the optional TCA_STATS_PKT64 attribute which provides
the full 64bit packet counter if the 32bit one has rolled over. This
patch also changes the non-empty check to use bytes, in case the 32bit
packet counter has rolled over for this update.
Since v1:
- Retain support for pre-TCA_STATS_PKT64 kernels
Since v2:
- Added compat header
Fixes: f98e418fbd ("tc: Add tc flower functions")
Reported-at:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D1776816&data=04%7C01%7Croid%40nvidia.com%7Cb5f8c349de6b4ef2ad9508d9fae4ad3c%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637816682810006558%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=aU3iqKoso%2FYhdN03GpmBP65tZCG%2F4zin3iYZQ8dnCiw%3D&reserved=0
Signed-off-by: Mike Pattrick <[email protected]>
---
acinclude.m4 | 7 ++++
include/linux/automake.mk | 1 +
include/linux/gen_stats.h | 82 +++++++++++++++++++++++++++++++++++++++
lib/tc.c | 12 +++++-
4 files changed, 100 insertions(+), 2 deletions(-)
create mode 100644 include/linux/gen_stats.h
diff --git a/acinclude.m4 b/acinclude.m4
index 5c971e98c..687f30d2d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -305,6 +305,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
])],
[AC_DEFINE([HAVE_TCA_SKBEDIT_FLAGS], [1],
[Define to 1 if TCA_SKBEDIT_FLAGS is available.])])
+
+ AC_COMPILE_IFELSE([
+ AC_LANG_PROGRAM([#include <linux/gen_stats.h>], [
+ int x = TCA_STATS_PKT64;
+ ])],
+ [AC_DEFINE([HAVE_TCA_STATS_PKT64], [1],
+ [Define to 1 if TCA_STATS_PKT64 is available.])])
])
dnl OVS_CHECK_LINUX_SCTP_CT
diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index 8f063f482..b8b26ec33 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automake.mk
@@ -1,4 +1,5 @@
noinst_HEADERS += \
+ include/linux/gen_stats.h \
include/linux/netlink.h \
include/linux/netfilter/nf_conntrack_sctp.h \
include/linux/pkt_cls.h \
diff --git a/include/linux/gen_stats.h b/include/linux/gen_stats.h
new file mode 100644
index 000000000..6df2ab45b
--- /dev/null
+++ b/include/linux/gen_stats.h
@@ -0,0 +1,82 @@
+#ifndef __LINUX_GEN_STATS_WRAPPER_H
+#define __LINUX_GEN_STATS_WRAPPER_H 1
+
+#if defined(__KERNEL__) || defined(HAVE_TCA_STATS_PKT64)
+#include_next <linux/gen_stats.h>
+#else
+
+#include <linux/types.h>
+
+enum {
+ TCA_STATS_UNSPEC,
+ TCA_STATS_BASIC,
+ TCA_STATS_RATE_EST,
+ TCA_STATS_QUEUE,
+ TCA_STATS_APP,
+ TCA_STATS_RATE_EST64,
+ TCA_STATS_PAD,
+ TCA_STATS_BASIC_HW,
+ TCA_STATS_PKT64,
+ __TCA_STATS_MAX,
+};
+#define TCA_STATS_MAX (__TCA_STATS_MAX - 1)
+
+/**
+ * struct gnet_stats_basic - byte/packet throughput statistics
+ * @bytes: number of seen bytes
+ * @packets: number of seen packets
+ */
+struct gnet_stats_basic {
+ __u64 bytes;
+ __u32 packets;
+};
+
+/**
+ * struct gnet_stats_rate_est - rate estimator
+ * @bps: current byte rate
+ * @pps: current packet rate
+ */
+struct gnet_stats_rate_est {
+ __u32 bps;
+ __u32 pps;
+};
+
+/**
+ * struct gnet_stats_rate_est64 - rate estimator
+ * @bps: current byte rate
+ * @pps: current packet rate
+ */
+struct gnet_stats_rate_est64 {
+ __u64 bps;
+ __u64 pps;
+};
+
+/**
+ * struct gnet_stats_queue - queuing statistics
+ * @qlen: queue length
+ * @backlog: backlog size of queue
+ * @drops: number of dropped packets
+ * @requeues: number of requeues
+ * @overlimits: number of enqueues over the limit
+ */
+struct gnet_stats_queue {
+ __u32 qlen;
+ __u32 backlog;
+ __u32 drops;
+ __u32 requeues;
+ __u32 overlimits;
+};
+
+/**
+ * struct gnet_estimator - rate estimator configuration
+ * @interval: sampling period
+ * @ewma_log: the log of measurement window weight
+ */
+struct gnet_estimator {
+ signed char interval;
+ unsigned char ewma_log;
+};
+
+#endif /* __KERNEL__ || !HAVE_TCA_POLICE_PKTRATE64 */
+
+#endif /* __LINUX_GEN_STATS_WRAPPER_H */
diff --git a/lib/tc.c b/lib/tc.c
index 38a1dfc0e..88e813088 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1702,6 +1702,9 @@ static const struct nl_policy stats_policy[] = {
[TCA_STATS_BASIC] = { .type = NL_A_UNSPEC,
.min_len = sizeof(struct gnet_stats_basic),
.optional = false, },
+ [TCA_STATS_PKT64] = { .type = NL_A_U64,
+ .min_len = sizeof(uint64_t),
+ .optional = true, },
};
static int
@@ -1772,8 +1775,13 @@ nl_parse_single_action(struct nlattr *action, struct
tc_flower *flower,
}
bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs);
- if (bs->packets) {
- put_32aligned_u64(&stats->n_packets, bs->packets);
+ if (bs->bytes) {
+ if (stats_attrs[TCA_STATS_PKT64]) {
+ uint64_t packets = nl_attr_get_u64(stats_attrs[TCA_STATS_PKT64]);
looks ok to me. can you add a space line between the definition
and the line of put?
beside that looks ok.
Acked-by: Roi Dayan <[email protected]>
+ put_32aligned_u64(&stats->n_packets, packets);
+ } else {
+ put_32aligned_u64(&stats->n_packets, bs->packets);
+ }
put_32aligned_u64(&stats->n_bytes, bs->bytes);
}
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
