From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id BE8C6293B for ; Wed, 6 Dec 2017 13:10:05 +0100 (CET) Received: by mail-wm0-f68.google.com with SMTP id r78so6837699wme.5 for ; Wed, 06 Dec 2017 04:10:05 -0800 (PST) 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=TVlRJ/eH60ELbZ0P7xlJ9oJ/BcoXFy6FM2qaFLo4F80=; b=tPDF992yI2mXZhgKzME/Pv7s1ZcYUh/rUZNvhsfqfEdxpAr6YWJukFFr4Zlz6K41Jh rnIr3sxFDhZNYu2ABUHQ66BVxZ6/18h7uwuxE9OUkbSSKf65xmYKra0r4gLKg/7IJkNy CfgjDzIsrW/iQvaguaneN1tJs1RHhtH3NifV1LloB92VlvnwkfCwuYkwtwKbqQWGUTpH dFCT+L/IjermQUFIuzShYq/+iE1lCBQwbGIW1beOdqZwgGw+xfLInjH74gtmSU492xel ArMyEZSm4TSl3GEG3WODsgISjvIJdkjDVik1rrCwBJSx36EUJ9Fmire0zwaj9jL5C9N1 +K/w== 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=TVlRJ/eH60ELbZ0P7xlJ9oJ/BcoXFy6FM2qaFLo4F80=; b=NBziowxHOP+V+idU9mrr8PYywYuoDpPKP8rQNEzuiVo0u3k9YefPG24dz7lSk7fvS8 c1kO4ezekFYXi2yxKuyF46PUKRZ+nwuQ7vvTpOH6oPxzC7rQYNdGMXy+NXPe0EHs9O2I QImji9BZ8Da2KRTaZmLexcJSgkIH1xQzPyUvQdAUD7ATa9F/8bBVf+eyYMnri2YPm4qy q/xSoO9bPWflQt2kP/ufHoACPIVRg5/cKi4Wqmlg5LesvSCCf1xvqFVDJRGHOP8/Fw7y MeVxzNpJKCGsoBNuPTl0N74xJNqSaKftWxn0w5/itZfWqAU/rRgpn7Jkuw4DEe8YP0Kr d6yg== X-Gm-Message-State: AJaThX4Naz1Y+7UCJa54luNkIU5v2RDPCoDNsUBaj1XN8kU/Hc7/nqVW TpJFX/tOARhTFZe1aURvRVO0L75P X-Google-Smtp-Source: AGs4zMYUoMUe9aXTPYlp3yAYyycU+pNzaGp0IhkhlUWdiJrnL2SRugOf+G7eXGVmuSJYcUMKX1lv/Q== X-Received: by 10.28.137.80 with SMTP id l77mr13183053wmd.24.1512562205461; Wed, 06 Dec 2017 04:10:05 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id l3sm2934227wrg.2.2017.12.06.04.10.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 06 Dec 2017 04:10:04 -0800 (PST) Date: Wed, 6 Dec 2017 13:09:53 +0100 From: Adrien Mazarguil To: Matan Azrad Cc: "dev@dpdk.org" Message-ID: <20171206120953.GB4062@6wind.com> References: <1511871570-16826-1-git-send-email-matan@mellanox.com> <1511871570-16826-6-git-send-email-matan@mellanox.com> <20171206105850.GW4062@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH 5/8] net/mlx4: merge Tx queue rings management 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, 06 Dec 2017 12:10:05 -0000 On Wed, Dec 06, 2017 at 11:43:25AM +0000, Matan Azrad wrote: > Hi Adrien > > > -----Original Message----- > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > Sent: Wednesday, December 6, 2017 12:59 PM > > To: Matan Azrad > > Cc: dev@dpdk.org > > Subject: Re: [PATCH 5/8] net/mlx4: merge Tx queue rings management > > > > On Tue, Nov 28, 2017 at 12:19:27PM +0000, Matan Azrad wrote: > > > The Tx queue send ring was managed by Tx block head,tail,count and > > > mask management variables which were used for managing the send > > queue > > > remain space and next places of empty or completted work queue entries. > > > > completted => completed > > > > > > > > This method suffered from an actual addresses recalculation per > > > packet, an unnecessary Tx block based calculations and an expensive > > > dual management of Tx rings. > > > > > > Move send queue ring calculation to be based on actual addresses while > > > managing it by descriptors ring indexes. > > > > > > Add new work queue entry pointer to the descriptor element to hold the > > > appropriate entry in the send queue. > > > > > > Signed-off-by: Matan Azrad > > > --- > > > drivers/net/mlx4/mlx4_prm.h | 20 ++-- drivers/net/mlx4/mlx4_rxtx.c > > > | 241 +++++++++++++++++++------------------------ > > > drivers/net/mlx4/mlx4_rxtx.h | 1 + > > > drivers/net/mlx4/mlx4_txq.c | 23 +++-- > > > 4 files changed, 126 insertions(+), 159 deletions(-) > > > > > > diff --git a/drivers/net/mlx4/mlx4_prm.h b/drivers/net/mlx4/mlx4_prm.h > > > index fcc7c12..2ca303a 100644 > > > --- a/drivers/net/mlx4/mlx4_prm.h > > > +++ b/drivers/net/mlx4/mlx4_prm.h > > > { struct mlx4_sq { > > > volatile uint8_t *buf; /**< SQ buffer. */ > > > volatile uint8_t *eob; /**< End of SQ buffer */ > > > - uint32_t head; /**< SQ head counter in units of TXBBS. */ > > > - uint32_t tail; /**< SQ tail counter in units of TXBBS. */ > > > - uint32_t txbb_cnt; /**< Num of WQEBB in the Q (should be ^2). */ > > > - uint32_t txbb_cnt_mask; /**< txbbs_cnt mask (txbb_cnt is ^2). */ > > > - uint32_t headroom_txbbs; /**< Num of txbbs that should be kept > > free. */ > > > + uint32_t size; /**< SQ size includes headroom. */ > > > + int32_t remain_size; /**< Remain WQE size in SQ. */ > > > > Remain => Remaining? > > > OK > > By "size", do you mean "room" as there could be several WQEs in there? > > > Size in bytes. > remaining size | remaining space | remaining room | remaining bytes , What are you preferred? I think this should fully clarify: Remaining WQE room in SQ (bytes). > > > Note before reviewing the rest of this patch, the fact it's a signed integer > > bothers me; it's probably a mistake. > > There is place in the code where this variable can used for signed calculations. My point is these signed calculations shouldn't be signed in the first place. A buffer size cannot be negative. > > > You should standardize on unsigned values everywhere. > > Why? > Each field with the most appropriate type. Because you used the wrong type everywhere else. The root cause seems to be with: #define MLX4_MAX_WQE_SIZE 512 Which must either be cast when used or redefined like: #define MLX4_MAX_WQE_SIZE 512u Or even: #define MLX4_MAX_WQE_SIZE UINT32_C(512) > > > a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c index > > > b9cb2fc..0a8ef93 100644 > > > --- a/drivers/net/mlx4/mlx4_rxtx.c > > > +++ b/drivers/net/mlx4/mlx4_rxtx.c > > > @@ -421,41 +403,27 @@ struct pv { > > > return buf->pool; > > > } > > > > > > -static int > > > +static volatile struct mlx4_wqe_ctrl_seg * > > > mlx4_tx_burst_segs(struct rte_mbuf *buf, struct txq *txq, > > > - volatile struct mlx4_wqe_ctrl_seg **pctrl) > > > + volatile struct mlx4_wqe_ctrl_seg *ctrl) > > > > Can you use this opportunity to document this function? > > > Sure, new patch for this? You can make it part of this one if you prefer, no problem either way. -- Adrien Mazarguil 6WIND