From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E775C43219 for ; Fri, 26 Apr 2019 18:49:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7F54320B7C for ; Fri, 26 Apr 2019 18:49:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726415AbfDZSts (ORCPT ); Fri, 26 Apr 2019 14:49:48 -0400 Received: from mail.us.es ([193.147.175.20]:56614 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726353AbfDZSts (ORCPT ); Fri, 26 Apr 2019 14:49:48 -0400 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id 1552611ED83 for ; Fri, 26 Apr 2019 20:49:46 +0200 (CEST) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 06E41DA70C for ; Fri, 26 Apr 2019 20:49:46 +0200 (CEST) Received: by antivirus1-rhel7.int (Postfix, from userid 99) id F0ADEDA708; Fri, 26 Apr 2019 20:49:45 +0200 (CEST) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id D4A95DA707; Fri, 26 Apr 2019 20:49:43 +0200 (CEST) Received: from 192.168.1.97 (192.168.1.97) by antivirus1-rhel7.int (F-Secure/fsigk_smtp/550/antivirus1-rhel7.int); Fri, 26 Apr 2019 20:49:43 +0200 (CEST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/antivirus1-rhel7.int) Received: from us.es (sys.soleta.eu [212.170.55.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: 1984lsi) by entrada.int (Postfix) with ESMTPSA id AE87A4265A5B; Fri, 26 Apr 2019 20:49:43 +0200 (CEST) Date: Fri, 26 Apr 2019 20:49:43 +0200 X-SMTPAUTHUS: auth mail.us.es From: Pablo Neira Ayuso To: Edward Cree Cc: Jamal Hadi Salim , netdev , Jiri Pirko , Cong Wang Subject: Re: TC stats / hw offload question Message-ID: <20190426184943.idewf2rqebvslcva@salvia> References: <26f0cfc9-3bef-8579-72cc-aa6c5ccecd43@solarflare.com> <4cb765dd-453f-3139-bce6-6e0b31167aec@mojatatu.com> <20190424141139.5c5vhihie5mryxlt@salvia> <26afcaaf-abdf-42ad-1715-5af9c6f3c2ef@solarflare.com> <58c74d0f-b92e-31f9-9828-24fb04129534@solarflare.com> <20190425223346.zqfadtphmhuj7ohp@salvia> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-Virus-Scanned: ClamAV using ClamSMTP Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Apr 26, 2019 at 01:13:41PM +0100, Edward Cree wrote: > 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 The flow_action_entry structure needs a new 'counter_index' field to store this. The tc_setup_flow_action() function needs to be updated for this for the FLOW_ACTION_{ACCEPT,DROP,REDIRECT,MIRRED} cases to set this entry->counter_index field to tcfa_index, so the driver has access to this. Thanks.