netdev - Re: TC stats / hw offload question

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eb718924-08b3-90ce-d8f0-3656d851a18b@mojatatu.com>
Date:   Fri, 26 Apr 2019 08:42:01 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Edward Cree <ecree@...arflare.com>,
        Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netdev <netdev@...r.kernel.org>, Jiri Pirko <jiri@...nulli.us>,
        Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: TC stats / hw offload question

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