On 2022-01-25 2:55 AM, 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

Fixes: f98e418fbd ("tc: Add tc flower functions")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1776816
Signed-off-by: Mike Pattrick <[email protected]>
---
  lib/tc.c | 16 +++++++++++++++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/tc.c b/lib/tc.c
index 38a1dfc0e..c710e78e1 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1702,6 +1702,11 @@ static const struct nl_policy stats_policy[] = {
      [TCA_STATS_BASIC] = { .type = NL_A_UNSPEC,
                            .min_len = sizeof(struct gnet_stats_basic),
                            .optional = false, },
+#ifdef TCA_STATS_PKT64

I'm not but how does this ifdef works?
In newer header TCA_STATS_PKT64 is an enum member, so ifdef shouldn't
match ever. So I don't think you ever get inside here?

I think you should have a compat file for gen_stats.h in include/linux/
look for example the pkt_cls.h
In automate we look for police pktrate and define if it exists
and in the compat file we check if its defined and use that
header or a compat header and define what we need.

then in the C file there are no ifdefs as you will always have
the TCA_STATS_PKT64 enum.

+    [TCA_STATS_PKT64] = { .type = NL_A_U64,
+                          .min_len = sizeof(uint64_t),
+                          .optional = true, },
+#endif
  };
static int
@@ -1772,8 +1777,17 @@ 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) {
+    if (bs->bytes) {
+#ifdef TCA_STATS_PKT64
+        if (stats_attrs[TCA_STATS_PKT64]) {
+            uint64_t packets = nl_attr_get_u64(stats_attrs[TCA_STATS_PKT64]);
+            put_32aligned_u64(&stats->n_packets, packets);
+        } else {
+            put_32aligned_u64(&stats->n_packets, bs->packets);
+        }
+#else
          put_32aligned_u64(&stats->n_packets, bs->packets);
+#endif
          put_32aligned_u64(&stats->n_bytes, bs->bytes);
      }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to