From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <nelio.laranjeiro@6wind.com> Received: from mail-wr0-f174.google.com (mail-wr0-f174.google.com [209.85.128.174]) by dpdk.org (Postfix) with ESMTP id 1DB872A58 for <dev@dpdk.org>; Wed, 5 Jul 2017 10:21:35 +0200 (CEST) Received: by mail-wr0-f174.google.com with SMTP id c11so259063855wrc.3 for <dev@dpdk.org>; Wed, 05 Jul 2017 01:21:35 -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:content-transfer-encoding:in-reply-to :user-agent; bh=mAE22moxyt2LYQttXnVAnQAimLGJHxJg0eWripbozAg=; b=SLAn8cb78fme2M+oSeS2EB15rmQAyIWEy4Ch/gBk9GOGHuquLh16Tn1S2fpomt6fby q2RpVlzaOezzqVVAE/wwh9i1W6I4A53ZO9qRdcb33f2uBQ2yGSfZk3lHJ+aLC816dnmF WZgY/64cupAbxsHh4E/83nvoBQymrPaEJCtVL+LdD+o58lNRSbJ+3ouJH5S4+O3sByjc LVPMHbv3afPfv9Am6JYI0ztUqx2ooC9kmGxQVEyDevrMJMJF21k7hKfEKVy7o2ehCqMA kxw3WDfoAsMUCAfBS0iZFfvliO5JX+gDYM810NKw0iGdPARN27097PGHtZpNOqwVvUjR q3ag== 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:content-transfer-encoding :in-reply-to:user-agent; bh=mAE22moxyt2LYQttXnVAnQAimLGJHxJg0eWripbozAg=; b=godZaaCKBfPall5f8FqzcCRST9a8X/Kc/1uhA8XE/Ijw+F4hN10oDtib3tpzFP2CWB jU1Mruappvi/6YkfrSHEPoABJrfI0LPkvjWYAhEwY0Es49rZv/AMDsXpm3RlkH0JeVnc kmifyWuovCy5MsgO4bl17JKwNw74f5THRPksLG2ng9qTw7lkH1kc+Nxk8QHm0n3niWzg 9gVC4T3+e/yZX1a/SVtICByMaMCrgTDPzRFy9lEz/YetCx+Uyvn9Qa2x21VJXNj7Giqp 7ikbstnHbNcU4Qy7ZKGM9pvrlh0Ag4SXfpG8RML3nfp5927o6BGSvcIehK2KSpD1aqdn cLQQ== X-Gm-Message-State: AKS2vOym5X8kwffQfZ3dUrfFmTdFednixnMUfOQvWedh+kiqKUB+YaoB uVBej+ZTdl0Ez1d8 X-Received: by 10.223.171.83 with SMTP id r19mr38509714wrc.173.1499242894717; Wed, 05 Jul 2017 01:21:34 -0700 (PDT) Received: from autoinstall.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id s65sm13971503wmb.7.2017.07.05.01.21.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Jul 2017 01:21:34 -0700 (PDT) Date: Wed, 5 Jul 2017 10:21:26 +0200 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro <nelio.laranjeiro@6wind.com> To: Yongseok Koh <yskoh@mellanox.com> Cc: ferruh.yigit@intel.com, dev@dpdk.org, adrien.mazarguil@6wind.com Message-ID: <20170705082126.GF21379@autoinstall.dev.6wind.com> References: <20170628230403.10142-1-yskoh@mellanox.com> <cover.1498850005.git.yskoh@mellanox.com> <969ef71aa84f02c19f7fe011fe75e25049177d76.1498850005.git.yskoh@mellanox.com> <20170704085852.GD21379@autoinstall.dev.6wind.com> <20170705003842.GA3440@minint-98vp2qg> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170705003842.GA3440@minint-98vp2qg> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 5/5] net/mlx5: add vectorized Rx/Tx burst for SSE4.1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Wed, 05 Jul 2017 08:21:35 -0000 On Tue, Jul 04, 2017 at 05:38:44PM -0700, Yongseok Koh wrote: > On Tue, Jul 04, 2017 at 10:58:52AM +0200, Nélio Laranjeiro wrote: > > Yongseok, some comments in this huge and great work, > > > > On Fri, Jun 30, 2017 at 12:23:33PM -0700, Yongseok Koh wrote: > > > To make vectorized burst routines enabled, it is required to run on x86_64 > > > architecture which can support at least SSE4.1. If all the conditions are > > > met, the vectorized burst functions are enabled automatically. The decision > > > is made individually on RX and TX. There's no PMD option to make a > > > selection. > > > > > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com> > > > --- > > > drivers/net/mlx5/Makefile | 10 + > > > drivers/net/mlx5/mlx5_defs.h | 18 + > > > drivers/net/mlx5/mlx5_ethdev.c | 28 +- > > > drivers/net/mlx5/mlx5_rxq.c | 55 +- > > > drivers/net/mlx5/mlx5_rxtx.c | 339 ++------ > > > drivers/net/mlx5/mlx5_rxtx.h | 283 ++++++- > > > drivers/net/mlx5/mlx5_rxtx_vec_sse.c | 1451 ++++++++++++++++++++++++++++++++++ > > > drivers/net/mlx5/mlx5_txq.c | 2 +- > > > 8 files changed, 1909 insertions(+), 277 deletions(-) > > > create mode 100644 drivers/net/mlx5/mlx5_rxtx_vec_sse.c > > > > > > diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h > > > index 51e258a15..2d0894fcd 100644 > > > --- a/drivers/net/mlx5/mlx5_rxtx.h > > > +++ b/drivers/net/mlx5/mlx5_rxtx.h > [...] > > > + txq_complete(txq); > > > + /* A CQE slot must always be available. */ > > > + assert((1u << txq->cqe_n) - (txq->cq_pi - txq->cq_ci)); > > > > This assert should be moved to the txq_complete(), or it should not be > > an assert. > txq_complete() is a common function, so this can't force to spare at least one > slot in a completion queue. This assert is to force to allocate enough CQE slots > by accurate calculation as completion is suppressed by MLX5_TX_COMP_THRESH. If > the CQ size is well defined (e.g. size of Tx ring / MLX5_TX_COMP_THRESH), it > doesn't need to check deficiency of CQ slot but checking slots in Tx ring > (max_elts) is sufficient. If you are okay with this, please let me know, then > I'll send out v3. Just using your comment... /* A CQE slot must always be available. */ This is always true in any Tx function where this assumption should be true to avoid testing it before posting a completion request and thus avoid cycles waste and this independently of the computation for the CQ ring size. As it is an assert it is only present to help the developer in error code he developed (or for a user to point an issue in the code), this can be any-where in the Tx data path. It becomes useful for any Tx function using this txq_complete(). If this assert is only related to your code, it may means it should be an if which avoids to post a completion request when no slots are available. > And I agree on all other comments you made. I'll make changes accordingly in v3. Ok, > > Thanks for your quick review! > Yongseok Thanks, -- Nélio Laranjeiro 6WIND