Re: [PATCH net-next 3/3] flow_offload: add process to update action stats from hardware

On Tue, Aug 03, 2021 at 07:24:47AM -0400, Jamal Hadi Salim wrote:

On 2021-07-22 5:19 a.m., Simon Horman wrote:

[..]

quoted

  /* offload the tc command after deleted */
  int tcf_action_offload_del_post(struct flow_offload_action *fl_act,
  				struct tc_action *actions[],

@@ -1255,6 +1293,9 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,
  	if (p == NULL)
  		goto errout;
+	/* update hw stats for this action */
+	tcf_action_update_hw_stats(p);

This is more a curiosity than a comment on the patch: Is the
driver polling for these stats synchronously and what we get here
is the last update or do we end up invoking beyond
the driver when requesting for the stats?

I would have to double check but I believe the driver will report
back stats already received from the HW, rather than going all the way
to HW when the above call is made.

Overall commentary from looking at the patch set:
I believe your patches will support the individual tc actions
add/del/get/dump command line requests.

Yes, that is the aim of this patchset.

What is missing is an example usage all the way to the driver. I am sure
you have additional patches that put this to good  use. My suggestion
is to test that cli with that pov against your overall patches and
show this in your commit logs - even if those patches are to follow
later.

Thanks, I'll see about making that so.

Just to be clear. We do have patches for the driver. And we do plan to post
them for inclusion in mainline. But I do believe that from a review
perspective its easier if one thing follows another.