From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Roger B. Melton" Subject: Re: [PATCH 1/2] ixgbe: fix VF statistic wraparound handling macro Date: Tue, 13 Oct 2015 15:43:33 -0400 Message-ID: <561D5EE5.2000507@cisco.com> References: <1444656823-717-1-git-send-email-harry.van.haaren@intel.com> <1444656823-717-2-git-send-email-harry.van.haaren@intel.com> <561BD4AE.4010704@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit To: dev@dpdk.org Return-path: Received: from alln-iport-1.cisco.com (alln-iport-1.cisco.com [173.37.142.88]) by dpdk.org (Postfix) with ESMTP id 4AEB6E72 for ; Tue, 13 Oct 2015 21:43:27 +0200 (CEST) Received: from [10.150.217.104] ([10.150.217.104]) by rcdn-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id t9DJhPLn029064 for ; Tue, 13 Oct 2015 19:43:26 GMT In-Reply-To: <561BD4AE.4010704@gmail.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Agreed, this handles the off by one error on wrap around and should be faster. -Roger On 10/12/15 11:41 AM, Alexander Duyck wrote: > On 10/12/2015 06:33 AM, Harry van Haaren wrote: >> Fix a misinterpretation of VF stats in ixgbe >> >> Signed-off-by: Harry van Haaren >> --- >> drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c >> b/drivers/net/ixgbe/ixgbe_ethdev.c >> index ec2918c..d226e8d 100644 >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c >> @@ -329,10 +329,14 @@ static int >> ixgbe_timesync_read_tx_timestamp(struct rte_eth_dev *dev, >> /* >> * Define VF Stats MACRO for Non "cleared on read" register >> */ >> -#define UPDATE_VF_STAT(reg, last, cur) \ >> +#define UPDATE_VF_STAT(reg, last, cur) \ >> { \ >> uint32_t latest = IXGBE_READ_REG(hw, reg); \ >> - cur += latest - last; \ >> + if(likely(latest > last)) { \ >> + cur += latest - last; \ >> + } else { \ >> + cur += (UINT_MAX - last) + latest; \ >> + } \ >> last = latest; \ >> } > > From what I can tell your math is adding an off by one error. You > should probably be using UINT_MAX as a mask for the result, not as a > part of the calculation itself. > > So the correct way to compute this would be "cur += (latest - last) & > UINT_MAX". Also the mask approach should be faster as it avoids any > conditional jumps. > > - Alex > . > -- ____________________________________________________________________ |Roger B. Melton | | Cisco Systems | |CPP Software :|: :|: 7100 Kit Creek Rd | |+1.919.476.2332 phone :|||: :|||: RTP, NC 27709-4987 | |+1.919.392.1094 fax .:|||||||:..:|||||||:. rmelton@cisco.com | | | | This email may contain confidential and privileged material for the| | sole use of the intended recipient. Any review, use, distribution | | or disclosure by others is strictly prohibited. If you are not the | | intended recipient (or authorized to receive for the recipient), | | please contact the sender by reply email and delete all copies of | | this message. | | | | For corporate legal information go to: | | http://www.cisco.com/web/about/doing_business/legal/cri/index.html | |__________________________ http://www.cisco.com ____________________|