From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH v3 1/7] net/mlx4: remove error flows from Tx
	fast path
Date: Mon, 30 Oct 2017 15:23:14 +0100
Message-ID: <20171030142314.GX26782@6wind.com>
References: <1508768520-4810-1-git-send-email-ophirmu@mellanox.com>
 <1509358049-18854-1-git-send-email-matan@mellanox.com>
 <1509358049-18854-2-git-send-email-matan@mellanox.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, Ophir Munk <ophirmu@mellanox.com>
To: Matan Azrad <matan@mellanox.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65])
 by dpdk.org (Postfix) with ESMTP id 898621B2F1
 for <dev@dpdk.org>; Mon, 30 Oct 2017 15:23:26 +0100 (CET)
Received: by mail-wm0-f65.google.com with SMTP id b189so16158684wmd.4
 for <dev@dpdk.org>; Mon, 30 Oct 2017 07:23:26 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1509358049-18854-2-git-send-email-matan@mellanox.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Oct 30, 2017 at 10:07:23AM +0000, Matan Azrad wrote:
> Move unnecessary error flows to DEBUG mode.
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

I missed a couple of details while reviewing the original version, the first
one being mlx4_post_send()'s return value is still documented as updating
rte_errno in case of error, it's not the case anymore after this patch.

Please see below for the other one:

> ---
>  drivers/net/mlx4/mlx4_rxtx.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
<snip>
>  /**
> @@ -510,8 +508,6 @@ struct pv {
>  	assert(max <= elts_n);
>  	/* Always leave one free entry in the ring. */
>  	--max;
> -	if (max == 0)
> -		return 0;
>  	if (max > pkts_n)
>  		max = pkts_n;
>  	for (i = 0; (i != max); ++i) {

While minor, this change has nothing to do with this patch, right?

I think it can slightly degrade an application performance as it removes the
guarantee that subsequent code only needs to be run if there is at least one
packet to process in case the TX ring is constantly full (SW faster than
HW).

Can you remove it?

-- 
Adrien Mazarguil
6WIND