From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f175.google.com (mail-wr0-f175.google.com [209.85.128.175]) by dpdk.org (Postfix) with ESMTP id 097741BB5A for ; Wed, 11 Apr 2018 18:32:01 +0200 (CEST) Received: by mail-wr0-f175.google.com with SMTP id d19so2394150wre.1 for ; Wed, 11 Apr 2018 09:32:01 -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=4OwhMLRo4qbIw4+ge+RBiNySoyqSnjgO0gFMOqLpO80=; b=1eZhghW/U/KcUsW9jLBh31dtC0CQ2DNcL7BCUX+cCXMIPQFLawQgKbJGOy/pfgrVPy l1LO2nzIO+YcbfJoCHPne6eNe52WHoQWo2U7nLx8hap9/mV+rm37cJGjM8Afujt/rHoo 9LGKbpN4tupzS87pLw2W+XAgZnx7ln+LQ+MOq+mdE/kCqIYkGIOlDtiSE0h7MUuZ/H/O TpHiYAOyHWw+cSwSlFP67GKzpnLDx0V1JlJ9DcR+KOwGwLdy6yL2mvl6M0yzwPgRsgQm jjMRpEOWPXeAvNr1sBl5L/mKOCZplittly/KwayWfalB35OHXJFB4NFDKDUvuGGCWTol IVYQ== 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=4OwhMLRo4qbIw4+ge+RBiNySoyqSnjgO0gFMOqLpO80=; b=BmB/w80F+SSqtqWHNq1Mlp8VTIzZ3aGwH3BHeS0BEHsxgeM00WrRIbTGsZgkJs1UdQ NY65WL6sxTtwhUQQO1lRej5jTNdqFcj+FBXmh/FPcyamRNxsr+eQ5918SoPIo+fd6RXE zU3ZFnDH2loWEK8IAvyac8JzIgzRjEAWwBmumbMwuLatrQDrbUAfhcAoYknhAlisQzG5 v6l4BK8j4o1qfRn2V0tLzESz5VEAecSYZIHa52fctTVclNkB6W5eURZ/+BXI0VGvnIW3 9t9aMuac9kAtCaF5UyF68/EPTxMl7nuQOhT9DvQSMAz3uYN2hnwHidcRVGtGriBl5YuJ I+Ew== X-Gm-Message-State: ALQs6tDeRmnn33ssUTIUb14SY+wJRHtDroOiZCSGgQpNyJsBCpH82hQX vDk/sUxq+7DxXJQD9vg/O9FO1Q== X-Google-Smtp-Source: AIpwx4+tW6UKvR0g5N2fBYobDS9r+FhlXcno5ERP/jA0nK5GoFVEkbqZvuXdZs/W3OfMB9c6qPxsAw== X-Received: by 10.223.188.19 with SMTP id s19mr3835133wrg.213.1523464320314; Wed, 11 Apr 2018 09:32:00 -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 c15sm2254513wrc.64.2018.04.11.09.31.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Apr 2018 09:31:59 -0700 (PDT) Date: Wed, 11 Apr 2018 18:31:46 +0200 From: Adrien Mazarguil To: Qi Zhang Cc: dev@dpdk.org, declan.doherty@intel.com, sugesh.chandran@intel.com, michael.j.glynn@intel.com, yu.y.liu@intel.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com Message-ID: <20180411163146.GM4957@6wind.com> References: <1522279780-34842-1-git-send-email-qi.z.zhang@intel.com> <1522617562-25940-1-git-send-email-qi.z.zhang@intel.com> <1522617562-25940-3-git-send-email-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1522617562-25940-3-git-send-email-qi.z.zhang@intel.com> Subject: Re: [dpdk-dev] [PATCH v2 2/4] ether: add flow last hit query support 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: Wed, 11 Apr 2018 16:32:01 -0000 On Sun, Apr 01, 2018 at 05:19:20PM -0400, Qi Zhang wrote: > Enhanced the action RTE_FLOW_TYPE_ACTION_COUNT, number of > milliseconds since last hit can be queried. > > Signed-off-by: Qi Zhang Please confirm whether existing devices have the ability to report time elapsed since last hit, or if PMDs are supposed to take care of that entirely on their own in software? If the latter, I suggest to drop this patch and let applications check counters regularly on their own. Unlike applications, PMDs do not easily have access to a reliable time source. Otherwise, the patch looks acceptable but I can't tell if milliseconds are the right unit for such information. Same issue as mbuf timestamps [1] basically. As a 64-bit field, a precision down to the nanosecond is a possibility so perhaps like mbufs, the reference and precision should be undefined in the API in order to be processed by a PMD callback? More comments below. [1] commit 918ae9dc775e ("mbuf: add a timestamp field") > --- > lib/librte_ether/rte_flow.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index 1080086..8f75db0 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -1054,9 +1054,11 @@ struct rte_flow_query_count { > uint32_t reset:1; /**< Reset counters after query [in]. */ > uint32_t hits_set:1; /**< hits field is set [out]. */ > uint32_t bytes_set:1; /**< bytes field is set [out]. */ > + uint32_t last_hit_set:1; /**< last_hit field is set [out]. */ > uint32_t reserved:29; /**< Reserved, must be zero [in, out]. */ You need to decrement reserved bits. > uint64_t hits; /**< Number of hits for this rule [out]. */ > uint64_t bytes; /**< Number of bytes through this rule [out]. */ > + uint64_t last_hit; /**< Number of milliseconds since last hit [out]. */ > }; Doing so impacts ABI compatibility. While normally frowned upon for rte_flow, it's OK for 18.05 because we already destroyed it. You still need to mention what functions are impacted by this change as in "ethdev: add encap level to RSS flow API action" [2] and update the .map files where necessary. In this case at least rte_flow_query() is impacted. Please update doc/guides/prog_guide/rte_flow.rst as well (look for "COUNT query"). [2] http://dpdk.org/ml/archives/dev/2018-April/096531.html -- Adrien Mazarguil 6WIND