From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f178.google.com (mail-wr0-f178.google.com [209.85.128.178]) by dpdk.org (Postfix) with ESMTP id 4DD921B820 for ; Mon, 9 Apr 2018 17:23:15 +0200 (CEST) Received: by mail-wr0-f178.google.com with SMTP id m13so9988357wrj.5 for ; Mon, 09 Apr 2018 08:23:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=pFzseCDRibNm5Ili08FiwV0id62ReHL8gAtSrd2u7Wo=; b=a9AF5Qc+EYOk9/D4Nfz2gbhK7o64d8ewUBF6PswLv5Xqj3qFoL4h8LyRqKswUR+WjG OCmL1cbFYvXlxhSPCSzz3lH5aPkdqgzZboDCBy1+GfeJe8EH5n0RcEIRVTCAZM7We/Iy 0heW1ePnJmAdmvs68UAfAmbWMT/QPBykrhHRBaZW6lvnWfiypTCuxI0vgpxtDyoVyE7N 3JBLxLIRHv5t3jHXmnMLg2k2RvhuikULSsN8XuzCIyYVoYtee8hPWkyjZlLvAQONTHSz a/EPdMoHGjKhWbEKHM0ju7zNlUSD+TLolSWgLUKoPNU3UwJ2iLiG3t+UGY7WF7eCfVtM UipA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=pFzseCDRibNm5Ili08FiwV0id62ReHL8gAtSrd2u7Wo=; b=uHJRVhSJ8vuxRmBnqsf9tv9c0ays9P+may/Pd7gbzMeGGHqGOvhgTjK9r6Uhi5PoLg DfuTW0VteaIl9O4LZ/KSx5e1ubKriz3xZtpVrG6XuVQjbnKN5YoKzGyMOesho4EaFdBS zZBR3rnaTTVhThmYq0XpejfKpSCqnJl77GRXxvfyDLIQtHbLVWaunew59yBXSZOUL3hK H19QkfrYJiVIaWlQzZbUXAmV5qVY2Jh7AFIdi0VIMzvMmTJAvbJE2Z1F1e6nF88ErSJi b+Fu+nMQILdzy21lBGyPwCpraMKT2d6/9zyeX5tYxvTfIbd+XdrhS5Pr8UDW9SYkOUX1 doOA== X-Gm-Message-State: AElRT7EYC3R5/H9AlnKRbX3JQHjRJYH1K5K0IMHmkrAOHmBNscOQOpY5 4t8NqErQu+2Ujp7aPe3UJRSTZg== X-Google-Smtp-Source: AIpwx4+FqNpNE35pgVfIYDQjvGhwFQRgTU6A2SLAaR11c7LU7zKr07BUjgnI1DtLhQiLjlYhq4S7GQ== X-Received: by 10.223.141.162 with SMTP id o31mr27629717wrb.167.1523287395012; Mon, 09 Apr 2018 08:23:15 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id n47sm790807wrf.41.2018.04.09.08.23.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Apr 2018 08:23:14 -0700 (PDT) Date: Mon, 9 Apr 2018 17:23:01 +0200 From: Adrien Mazarguil To: Mohammad Abdul Awal Cc: Declan Doherty , dev@dpdk.org Message-ID: <20180409152301.GD4957@6wind.com> References: <1523017443-12414-1-git-send-email-declan.doherty@intel.com> <1523017443-12414-2-git-send-email-declan.doherty@intel.com> <20180406202625.GR4957@6wind.com> <9d87b6bf-4c9a-a7f5-38ab-a322e9eacf02@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9d87b6bf-4c9a-a7f5-38ab-a322e9eacf02@intel.com> Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add group counter support to rte_flow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Apr 2018 15:23:15 -0000 Hi Mohammad, On Mon, Apr 09, 2018 at 03:22:45PM +0100, Mohammad Abdul Awal wrote: > Hi Adrien, > > > On 06/04/2018 21:26, Adrien Mazarguil wrote: > > On Fri, Apr 06, 2018 at 01:24:00PM +0100, Declan Doherty wrote: > > > Add new RTE_FLOW_ACTION_TYPE_GROUP_COUNT action type to enable shared > > > counters across multiple flows on a single port or across multiple > > > flows on multiple ports within the same switch domain. > > > > > > Introduce new API rte_flow_query_group_count to allow querying of group > > > counters. > > > > > > Signed-off-by: Declan Doherty > > Both features are definitely needed, however I suggest to enhance the > > existing action type and query function instead, given the rte_flow ABI > > won't be maintained for the 18.05 release [1]. > > > > Counters and query support were defined as a kind of PoC in preparation for > > future requirements back in DPDK 17.02 and so far few PMDs have implemented > > the query callback (mlx5 and failsafe, and the latter isn't really a PMD). > > > > Due to the behavior change of action lists [2], providing an action type as > > a query parameter is not specific enough anymore, for instance if a list > > contains multiple COUNT, the application should be able to tell which needs > > to be queried. > > > > Therefore I suggest to redefine the query function as follows: > > > > int > > rte_flow_query(uint16_t port_id, > > struct rte_flow *flow, > > const struct rte_flow_action *action, > > void *data, > > struct rte_flow_error *error); > > > > Third argument is an action definition with the same configuration (if any) > > as previously defined in the action list originally used to create the flow > > rule (not necessarily the same pointer, only the contents matter). > > > > It means two perfectly identical actions can't be distinguished, and that's > > how group counters will work. > > Instead of adding a new action type to distinguish groups, a configuration > > structure is added to the existing RTE_FLOW_ACTION_TYPE_COUNT, with > > non-shared counters as a default behavior: > > > > struct rte_flow_action_count { > > uint32_t shared:1; /**< Share counter ID with other flow rules. */ > > uint32_t reserved:31; /**< Reserved, must be zero. */ > > uint32_t id; /**< Counter ID. */ > > }; > > > > Doing so will impact some existing code in mlx5 and librte_flow_classify, > > but that shouldn't be much of an issue. > > > > Keep in mind testpmd and its documentation must be updated as well. > > > > Thoughts? > Please correct me if I am wrong but I think we are talking two different > things here. > If I understood you correctly, you are proposing to pass a list of actions > (instead if a action type) in the third parameter to perform multiple > actions in the same query call. Lets take an example for 100 ingress flows. > So, if we want to query the counter for all the flows, we can get them by a > single query providing a list (of size 100) of action_count in the 3rd > param. Whoa no! I'm only suggesting a pointer to a single action as a replacement for the basic action type, not a *list* of actions. I hope this addresses all concerns :) The fact the action in question would refer to a shared counter (see struct above) with a given ID would be enough to make all counters with the same ID refer to the same internal PMD object. > On the other hand, we are saying that all the flows are belongs to same > tunnel end-point (we are talking only 1 TEP here), then the PMD will be > responsible to increment the counter of TEP for matching all the flows (100 > flows). So, using one group query by passing one action_count in 3rd param, > we can get the count of the TEP. > > This case is generic enough for sure for simple flows but may not be > suitable for tunnel cases, as application needs to track the counters for > all the flows, and needs to build the list of action each time the flows > added/deleted. I think we're on the same page. I'm only suggesting to define a configuration structure for the COUNT action (which currently doesn't take any) as a replacement for GROUP_COUNT, and tweak the query callback to be able to tell a specific counter to query instead of adding a new query callback and leaving the existing one broken by design. > > A few nits below for the sake of commenting. > > > > [1] "Flow API overhaul for switch offloads" > > http://dpdk.org/ml/archives/dev/2018-April/095774.html > > [2] "ethdev: alter behavior of flow API actions" > > http://dpdk.org/ml/archives/dev/2018-April/095779.html -- Adrien Mazarguil 6WIND