Re: TC stats / hw offload question

Jamal Hadi Salim
2019-04-26T12:42:07Z

On 2019-04-26 8:13 a.m., Edward Cree wrote:

> Sure; this block is (still slightly abridged)
>
> if (a->ops && a->ops->stats_update) {
>     struct efx_tc_counter_index *ctr;
>
>     ctr = efx_tc_flower_get_counter_by_index(efx, a->tcfa_index);
>     if (IS_ERR(ctr)) {
>         rc = PTR_ERR(ctr);
>         goto release;
>     }
>     act->count = ctr;
>     act->count_action_idx = i;
>     efx_tc_calculate_count_delta(act);
> }
>
> and we have
>
> struct efx_tc_counter_index {
>     u32 tcfa_index;
>     struct rhash_head linkage;
>     refcount_t ref;
>     u32 fw_id;
> };
>
> const static struct rhashtable_params efx_tc_counter_ht_params = {
>     .key_len        = offsetof(struct efx_tc_counter_index, linkage),
>     .key_offset     = 0,
>     .head_offset    = offsetof(struct efx_tc_counter_index, linkage),
> };
>
> static struct efx_tc_counter_index *efx_tc_flower_get_counter_by_index(
>                 struct efx_nic *efx, u32 idx)
> {
>     struct efx_tc_counter_index *ctr, *old;
>     long rc;
>
>     ctr = kzalloc(sizeof(*ctr), GFP_USER);
>     if (!ctr)
>         return ERR_PTR(-ENOMEM);
>     ctr->tcfa_index = idx;
>     old = rhashtable_lookup_get_insert_fast(&efx->tc->counter_ht,
>                                             &ctr->linkage,
>                                             efx_tc_counter_ht_params);
>     if (old) {
>         /* don't need our new entry */
>         kfree(ctr);
>         if (!refcount_inc_not_zero(&old->ref))
>             return ERR_PTR(-EAGAIN);
>         /* existing entry found */
>         ctr = old;
>     } else {
>         rc = efx_tc_flower_allocate_counter(efx);
>         if (rc < 0) {
>             rhashtable_remove_fast(&efx->tc->counter_ht,
>                                    &ctr->linkage,
>                                    efx_tc_counter_ht_params);
>             kfree(ctr);
>             return ERR_PTR(rc);
>         }
>         ctr->fw_id = rc;
>         refcount_inc(&ctr->ref);
>     }
>     return ctr;
> }
>
> Thus if (and only if) two TC actions have the same tcfa_index, they will
>  share a single counter in the HW.
> I gathered from a previous conversation with Jamal[1] that that was the
>  correct behaviour:

Yes, this is expected behavior. Meters/policers as well used indices to
indicate sharing.

cheers,
jamal