tc: Fix stats byte count on fragmented packets.

Fragmented packets with offset=0 are defragmented by tc act_ct, and
only when assembled pass to next action, in ovs offload case,
a goto action. Since stats are overwritten on each action dump,
only the stats for last action in the tc filter action priority
list is taken, the stats on the goto action, which count
only the assembled packets. See below for example.

Hardware updates just part of the actions (gact, ct, mirred) - those
that support stats_update() operation. Since datapath rules end
with either an output (mirred) or recirc/drop (both gact), tc rule
will at least have one action that supports it. For software packets,
the first action will have the max software packets count.
Tc dumps total packets (hw + sw) and hardware packets, then
software packets needs to be calculated from this (total - hw).

To fix the above, get hardware packets and calculate software packets
for each action, take the max of each set, then combine back
to get the total packets that went through software and hardware.

Example by running ping above MTU (ping <IP> -s 2000):
ct_state(-trk),recirc_id(0),...,ipv4(proto=1,frag=first),
  packets:14, bytes:19544,..., actions:ct(zone=1),recirc(0x1)
ct_state(-trk),recirc_id(0),...,ipv4(proto=1,frag=later),
  packets:14, bytes:28392,..., actions:ct(zone=1),recirc(0x1)

Second rule should have had bytes=14*<size of 'later' frag>, but instead
it's bytes=14*<size of assembled packets - size of 'first' + 'later'
frags>.

Fixes: 576126a931cd ("netdev-offload-tc: Add conntrack support")
Signed-off-by: Paul Blakey <paulb@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

3 files changed, 35 insertions, 12 deletions

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 9845e8d3f..3f7068c8e 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -585,8 +585,10 @@ parse_tc_flower_to_stats(struct tc_flower *flower,
     }
 
     memset(stats, 0, sizeof *stats);
-    stats->n_packets = get_32aligned_u64(&flower->stats.n_packets);
-    stats->n_bytes = get_32aligned_u64(&flower->stats.n_bytes);
+    stats->n_packets = get_32aligned_u64(&flower->stats_sw.n_packets);
+    stats->n_packets += get_32aligned_u64(&flower->stats_hw.n_packets);
+    stats->n_bytes = get_32aligned_u64(&flower->stats_sw.n_bytes);
+    stats->n_bytes += get_32aligned_u64(&flower->stats_hw.n_bytes);
     stats->used = flower->lastused;
 }
 
@@ -2015,9 +2017,7 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
     if (stats) {
         memset(stats, 0, sizeof *stats);
         if (!tc_get_flower(&id, &flower)) {
-            stats->n_packets = get_32aligned_u64(&flower.stats.n_packets);
-            stats->n_bytes = get_32aligned_u64(&flower.stats.n_bytes);
-            stats->used = flower.lastused;
+            parse_tc_flower_to_stats(&flower, stats);
         }
     }
 
diff --git a/lib/tc.c b/lib/tc.c
index adb2d3182..a52cd46d9 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_BASIC_HW] = { .type = NL_A_UNSPEC,
+                             .min_len = sizeof(struct gnet_stats_basic),
+                             .optional = true, },
 };
 
 static int
@@ -1714,8 +1717,11 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower,
     const char *act_kind;
     struct nlattr *action_attrs[ARRAY_SIZE(act_policy)];
     struct nlattr *stats_attrs[ARRAY_SIZE(stats_policy)];
-    struct ovs_flow_stats *stats = &flower->stats;
-    const struct gnet_stats_basic *bs;
+    struct ovs_flow_stats *stats_sw = &flower->stats_sw;
+    struct ovs_flow_stats *stats_hw = &flower->stats_hw;
+    const struct gnet_stats_basic *bs_all = NULL;
+    const struct gnet_stats_basic *bs_hw = NULL;
+    struct gnet_stats_basic bs_sw = { .packets = 0, .bytes = 0, };
     int err = 0;
 
     if (!nl_parse_nested(action, act_policy, action_attrs,
@@ -1771,10 +1777,26 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower,
         return EPROTO;
     }
 
-    bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs);
-    if (bs->packets) {
-        put_32aligned_u64(&stats->n_packets, bs->packets);
-        put_32aligned_u64(&stats->n_bytes, bs->bytes);
+    bs_all = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs_all);
+    if (stats_attrs[TCA_STATS_BASIC_HW]) {
+        bs_hw = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC_HW],
+                                   sizeof *bs_hw);
+
+        bs_sw.packets = bs_all->packets - bs_hw->packets;
+        bs_sw.bytes = bs_all->bytes - bs_hw->bytes;
+    } else {
+        bs_sw.packets = bs_all->packets;
+        bs_sw.bytes = bs_all->bytes;
+    }
+
+    if (bs_sw.packets > get_32aligned_u64(&stats_sw->n_packets)) {
+        put_32aligned_u64(&stats_sw->n_packets, bs_sw.packets);
+        put_32aligned_u64(&stats_sw->n_bytes, bs_sw.bytes);
+    }
+
+    if (bs_hw && bs_hw->packets > get_32aligned_u64(&stats_hw->n_packets)) {
+        put_32aligned_u64(&stats_hw->n_packets, bs_hw->packets);
+        put_32aligned_u64(&stats_hw->n_bytes, bs_hw->bytes);
     }
 
     return 0;
diff --git a/lib/tc.h b/lib/tc.h
index a147ca461..fb0534a08 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -330,7 +330,8 @@ struct tc_flower {
     int action_count;
     struct tc_action actions[TCA_ACT_MAX_NUM];
 
-    struct ovs_flow_stats stats;
+    struct ovs_flow_stats stats_sw;
+    struct ovs_flow_stats stats_hw;
     uint64_t lastused;
 
     struct {