[ovs-dev] [PATCH] Avoid indeterminate statistics in offload implementations.

Ben Pfaff blp at ovn.org
Fri Oct 25 18:46:24 UTC 2019


A lot of the offload implementations didn't bother to initialize the
statistics they were supposed to return.  I don't know whether any of
the callers actually use them, but it looked wrong.

Found by inspection.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/netdev-dummy.c        | 10 ++++++++--
 lib/netdev-offload-dpdk.c | 10 ++++++++--
 lib/netdev-offload-tc.c   |  5 ++++-
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 95e1a329a908..71df29184d9b 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1434,7 +1434,7 @@ netdev_dummy_flow_put(struct netdev *netdev, struct match *match,
                       struct nlattr *actions OVS_UNUSED,
                       size_t actions_len OVS_UNUSED,
                       const ovs_u128 *ufid, struct offload_info *info,
-                      struct dpif_flow_stats *stats OVS_UNUSED)
+                      struct dpif_flow_stats *stats)
 {
     struct netdev_dummy *dev = netdev_dummy_cast(netdev);
     struct offloaded_flow *off_flow;
@@ -1476,12 +1476,15 @@ netdev_dummy_flow_put(struct netdev *netdev, struct match *match,
         ds_destroy(&ds);
     }
 
+    if (stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     return 0;
 }
 
 static int
 netdev_dummy_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
-                      struct dpif_flow_stats *stats OVS_UNUSED)
+                      struct dpif_flow_stats *stats)
 {
     struct netdev_dummy *dev = netdev_dummy_cast(netdev);
     struct offloaded_flow *off_flow;
@@ -1521,6 +1524,9 @@ exit:
         ds_destroy(&ds);
     }
 
+    if (stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     return error ? -1 : 0;
 }
 
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 01e900461adf..96794dc4dc8b 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -710,7 +710,7 @@ static int
 netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match,
                              struct nlattr *actions, size_t actions_len,
                              const ovs_u128 *ufid, struct offload_info *info,
-                             struct dpif_flow_stats *stats OVS_UNUSED)
+                             struct dpif_flow_stats *stats)
 {
     struct rte_flow *rte_flow;
     int ret;
@@ -732,13 +732,16 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match,
         return ret;
     }
 
+    if (stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     return netdev_offload_dpdk_add_flow(netdev, match, actions,
                                         actions_len, ufid, info);
 }
 
 static int
 netdev_offload_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
-                             struct dpif_flow_stats *stats OVS_UNUSED)
+                             struct dpif_flow_stats *stats)
 {
     struct rte_flow *rte_flow = ufid_to_rte_flow_find(ufid);
 
@@ -746,6 +749,9 @@ netdev_offload_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
         return -1;
     }
 
+    if (stats) {
+        memset(stats, 0, sizeof *stats);
+    }
     return netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow);
 }
 
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index f6d1abb2e695..c5b9cbdb2bfe 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1143,7 +1143,7 @@ static int
 netdev_tc_flow_put(struct netdev *netdev, struct match *match,
                    struct nlattr *actions, size_t actions_len,
                    const ovs_u128 *ufid, struct offload_info *info,
-                   struct dpif_flow_stats *stats OVS_UNUSED)
+                   struct dpif_flow_stats *stats)
 {
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
     enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
@@ -1448,6 +1448,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
 
     err = tc_replace_flower(ifindex, prio, handle, &flower, block_id, hook);
     if (!err) {
+        if (stats) {
+            memset(stats, 0, sizeof *stats);
+        }
         add_ufid_tc_mapping(ufid, flower.prio, flower.handle, netdev, ifindex);
     }
 
-- 
2.21.0



More information about the dev mailing list