From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B7046A034F; Mon, 11 Oct 2021 12:41:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AD68410DB; Mon, 11 Oct 2021 12:41:09 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 6818340142 for ; Mon, 11 Oct 2021 12:41:08 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id E781A7F5F1; Mon, 11 Oct 2021 13:41:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru E781A7F5F1 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633948868; bh=UoIojzeOdYwpj4h4Y4AKEvqEscxUW/fPF7G+7gKpfnU=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=Nw2YsMQcDVK18sCX5pvRM4HoPIvhD59GTpU+TlyYSTiBQfXVn2riNoQoozcW1B/7G 1d4C0pC+kWZvQAsJRKpMtQc+REEMRpynpxlNSD+1SslUxHHO4sLFdJbWBck00Z19Cy OelI2FpWtXVFdHlbBf8uW/3ZsNKlXx971FaouqZc= To: Ori Kam , Xiaoyun Li , Ray Kinsella , Ajit Khaparde , Somnath Kotur , Nithin Dabilpuram , Kiran Kumar K , Sunil Kumar Kori , Satha Rao , "Min Hu (Connor)" , Yisen Zhuang , Lijun Ou , Qiming Yang , Qi Zhang , Matan Azrad , Slava Ovsiienko , Jerin Jacob , Jasvinder Singh , Cristian Dumitrescu , NBU-Contact-Thomas Monjalon , Ferruh Yigit Cc: "dev@dpdk.org" , Stephen Hemminger References: <20210928152300.989961-1-andrew.rybchenko@oktetlabs.ru> <20211008102616.2627821-1-andrew.rybchenko@oktetlabs.ru> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: Date: Mon, 11 Oct 2021 13:41:07 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] ethdev: remove deprecated shared counter attribute X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Ori, On 10/11/21 1:02 PM, Ori Kam wrote: > Hi Andrew, > > Sorry but I think I missed something. > >> -----Original Message----- >> From: dev On Behalf Of Andrew Rybchenko >> Sent: Friday, October 8, 2021 1:26 PM >> >> Indirect actions should be used to do shared counters. >> >> Signed-off-by: Andrew Rybchenko >> Acked-by: Thomas Monjalon >> Acked-by: Ajit Khaparde >> Acked-by: Somnath Kotur >> Acked-by: Ori Kam >> --- >> v2: >> - remove reserved field from count structure (review from Stephen) >> - apply mlx5 review notes from Matan >> >> app/test-pmd/cmdline_flow.c | 10 -- >> doc/guides/prog_guide/rte_flow.rst | 19 +--- >> doc/guides/rel_notes/deprecation.rst | 4 - >> doc/guides/rel_notes/release_21_11.rst | 4 + >> drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 5 - >> drivers/net/cnxk/cnxk_rte_flow.c | 8 -- >> drivers/net/hns3/hns3_flow.c | 3 +- >> drivers/net/ice/ice_fdir_filter.c | 4 +- >> drivers/net/mlx5/mlx5.c | 11 -- >> drivers/net/mlx5/mlx5.h | 9 -- >> drivers/net/mlx5/mlx5_flow_dv.c | 118 ++------------------- >> drivers/net/mlx5/mlx5_flow_verbs.c | 22 +--- >> drivers/net/octeontx2/otx2_flow_parse.c | 10 -- >> drivers/net/sfc/sfc_mae.c | 9 +- >> drivers/net/softnic/rte_eth_softnic_flow.c | 7 -- >> lib/ethdev/rte_flow.h | 16 +-- >> 16 files changed, 22 insertions(+), 237 deletions(-) >> > > [Snip] > >> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index 7b1ed7f110..9819c25d2f 100644 >> --- a/lib/ethdev/rte_flow.h >> +++ b/lib/ethdev/rte_flow.h >> @@ -75,7 +75,7 @@ extern "C" { >> * At least one direction must be specified. >> * >> * Specifying both directions at once for a given rule is not recommended >> - * but may be valid in a few cases (e.g. shared counter). >> + * but may be valid in a few cases. >> */ >> struct rte_flow_attr { >> uint32_t group; /**< Priority group. */ @@ -2498,24 +2498,10 @@ struct rte_flow_query_age >> { >> * Counters can be retrieved and reset through ``rte_flow_query()``, see >> * ``struct rte_flow_query_count``. >> * >> - * @deprecated Shared attribute is deprecated, use generic >> - * RTE_FLOW_ACTION_TYPE_INDIRECT action. >> - * >> - * The shared flag indicates whether the counter is unique to the flow rule the >> - * action is specified with, or whether it is a shared counter. >> - * >> - * For a count action with the shared flag set, then then a global device >> - * namespace is assumed for the counter id, so that any matched flow rules using >> - * a count action with the same counter id on the same port will contribute to >> - * that counter. >> - * >> * For ports within the same switch domain then the counter id namespace extends >> * to all ports within that switch domain. > > I don't think we need this anymore. I agree. I'll remove it in v3 if required, but I hope it could be removed on apply as well. > >> */ >> struct rte_flow_action_count { >> - /** @deprecated Share counter ID with other flow rules. */ >> - uint32_t shared:1; >> - uint32_t reserved:31; /**< Reserved, must be zero. */ >> uint32_t id; /**< Counter ID. */ > > Why do we need to keep the id field? It is a very good question. I thought about it and preserved it for the corner case of two COUNT actions in the same rule. If so, id is required to distinguish on query. I don't know if we really need it to have two basically duplicate counters in the same rule. However, since order of actions matter, COUNT, VXLAN_ENCAP, COUNT should produce different byte counters. I suggest to continue discussion and gather more thought on it, but do not block the patch, since strictly speaking it is a bit separate topic as noted above. Thanks, Andrew.