On 8/30/20 1:02 PM, Andy Shevchenko wrote:

On Sun, Aug 30, 2020 at 6:17 PM [off-list ref] wrote:

quoted

From: Tom Rix <trix@redhat.com>

clang static analysis flags these problems

flow_table.c:713:2: warning: The expression is an uninitialized
  value. The computed value will also be garbage
        (*n_mask_hit)++;
        ^~~~~~~~~~~~~~~
flow_table.c:748:5: warning: The expression is an uninitialized
  value. The computed value will also be garbage
                                (*n_cache_hit)++;
                                ^~~~~~~~~~~~~~~~

These are not problems because neither pararmeter is used

parameter

Too many ar's, it must be talk like a pirate day.

quoted

by the calling function.

Looking at all of the calling functions, there are many
cases where the results are unused.  Passing unused
parameters is a waste.

To avoid passing unused parameters, rework the
masked_flow_lookup() and flow_lookup() routines to check
for NULL parameters and change the unused parameters to NULL.

Signed-off-by: Tom Rix <trix@redhat.com>
---
 net/openvswitch/flow_table.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index e2235849a57e..18e7fa3aa67e 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c

@@ -710,7 +710,8 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
        ovs_flow_mask_key(&masked_key, unmasked, false, mask);
        hash = flow_hash(&masked_key, &mask->range);
        head = find_bucket(ti, hash);
-       (*n_mask_hit)++;
+       if (n_mask_hit)
+               (*n_mask_hit)++;

        hlist_for_each_entry_rcu(flow, head, flow_table.node[ti->node_ver],
                                lockdep_ovsl_is_held()) {

@@ -745,7 +746,8 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
                                u64_stats_update_begin(&ma->syncp);
                                usage_counters[*index]++;
                                u64_stats_update_end(&ma->syncp);
-                               (*n_cache_hit)++;
+                               if (n_cache_hit)
+                                       (*n_cache_hit)++;
                                return flow;
                        }
                }

@@ -798,9 +800,8 @@ struct sw_flow *ovs_flow_tbl_lookup_stats(struct flow_table *tbl,
        *n_cache_hit = 0;
        if (unlikely(!skb_hash || mc->cache_size == 0)) {
                u32 mask_index = 0;
-               u32 cache = 0;

-               return flow_lookup(tbl, ti, ma, key, n_mask_hit, &cache,
+               return flow_lookup(tbl, ti, ma, key, n_mask_hit, NULL,
                                   &mask_index);

Can it be done for mask_index as well?

Yes, it's a bit more complicated but doable.

Tom