On 25/04/2019 23:33, Pablo Neira Ayuso wrote:
> On Thu, Apr 25, 2019 at 02:23:08PM +0100, Edward Cree wrote:
>> On 24/04/2019 16:03, Edward Cree wrote:
>>> static int efx_tc_flower_replace(struct efx_nic *efx,
>>> struct net_device *net_dev,
>>> struct tc_cls_flower_offload *tc)
>>> {
>>> struct efx_tc_action_set *act;
>>>
>>> /* parse the match */
>>>
>>> tcf_exts_for_each_action(i, a, tc->exts) {
>>> if (a->ops && a->ops->stats_update) {
>>> /* act is the hw action we're building */
>>> act->count = allocate_a_counter();
>> Also, this was actually taking a->tcfa_index, allowing multiple rules to
>> share a counter. The action index doesn't seem to be available in the
>> new flow_offload API.
> Could you show a bit more code to see how you use a->tcfa_index from
> your efx_tc_flower_replace()?
>
> Thanks.
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:
> Note, your counters should also be shareable; example, count all
> the drops in one counter across multiple flows as in the following
> case where counter index 1 is used.
>
> tc flower match foo action drop index 1
> tc flower match bar action drop index 1
-Ed
[1]: https://www.spinics.net/lists/netdev/msg551448.html
