Re: [PATCH net-next v2 8/9] nfp: add a stats handler for flower offloads

On Thu, Jun 29, 2017 at 06:16:41PM +0300, Or Gerlitz wrote:

On Wed, Jun 28, 2017 at 11:30 PM, Simon Horman
[off-list ref] wrote:

quoted

From: Pieter Jansen van Vuuren <redacted>

Previously there was no way of updating flow rule stats after they
have been offloaded to hardware. This is solved by keeping track of
stats received from hardware and providing this to the TC handler
on request.

quoted

+static void
+nfp_flower_update_stats(struct nfp_app *app, struct nfp_fl_stats_frame *stats)
+{
+       struct nfp_fl_payload *nfp_flow;
+       unsigned long flower_cookie;
+
+       flower_cookie = be64_to_cpu(stats->stats_cookie);
+
+       rcu_read_lock();
+       nfp_flow = nfp_flower_find_in_fl_table(app, flower_cookie);
+       if (!nfp_flow)
+               goto exit_rcu_unlock;
+
+       if (nfp_flow->meta.host_ctx_id != stats->stats_con_id)
+               goto exit_rcu_unlock;
+
+       spin_lock(&nfp_flow->lock);
+       nfp_flow->stats.pkts += be32_to_cpu(stats->pkt_count);
+       nfp_flow->stats.bytes += be64_to_cpu(stats->byte_count);

you're using += with the values you get form the fw, are they incremental?

Yes, they are incremental.

quoted

+       nfp_flow->stats.used = jiffies;

if nothing was changed since your last reading, it's wrong to say that
used == NOW

This function is called on receipt of a message from the firmware.
And the firmware will only send a message if there is a change in
the counters.

quoted

+       spin_unlock(&nfp_flow->lock);

if indeed you need to keep a clone of earlier calls to correctly
compute the (last)used value,
maybe you can get rid of the locking?