From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id CBFA61B6FE for ; Fri, 10 Nov 2017 16:02:54 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id p75so3317083wmg.3 for ; Fri, 10 Nov 2017 07:02:54 -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=F7yQx+O6X6J6qIe9e6FyWKBvmlr/yB+fsMJj7ibh7e4=; b=EmuxdF/LrK/FefJR6XnWKURlqWx0JYAHR7nHMgbhu9Iom/YJEo1bn9m08MRAZDoT+G qbE/T2XSi/0hbbmHwvGzLVZY9LaDg217TKh9/UHyGEKrL8V6QW0Ajha7XrjwieD36Gwy p7euwns/SaRnODVRLrvRE87LwfejbfsgWCRiym6Pu4SXgGGS2iOqPZGkuWvCBbruLh7f a7loXXL1CJPpIhbELgBvBFO7A8+ziYyzRlfZPUFEOSxkqwdE27BHYmxWqwD0kS48UAHj AXj6JAiGMSBMILHCl17JnIjJ2TPxnhHJPEx402gSC+GOL/b15vRfUjpW2oOu8wGoI1kR KAxw== 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=F7yQx+O6X6J6qIe9e6FyWKBvmlr/yB+fsMJj7ibh7e4=; b=Gip0232DUDKmfg2gHicHjewlJ1zwgyMBtbetxVxpyXwZ8Jb8gnmLNuYPvW7b3xmun1 /iPxU1CjR/G/KLrMIcho/iE+RZXfeYoZ5uXUiy2UcKKcC2yvsXy4zFGNni1JmIPkxeGx EcWrWwBx5zDjyl7Xjcnc56TjgiKG77zoFp82blP3TiqXc4t4N8f9sbVeIKUsyvmCiMYG G2GP4yz1AhwHWGSBu4Vqz4bBe46J2zPkGlflfUckYwcbkJOhr8BpK23b+66Xu7WaY6Qx fKiT5mPVmH7+ncTHhIZp8AYvtG1bcJ0YV2D0P7gB8OehQBkySyxELgW2VGdj0UcIzVqE GDbQ== X-Gm-Message-State: AJaThX5EYuPbV3LRd/ZOz7d1RkAf/ctldKaP0oTqmHm/YYl0I+rueNlU 7HZcxCOgLHtzihRFEI9m2kUeiw== X-Google-Smtp-Source: AGs4zMYnyATznrOALuEZprlUYkwkqdfKBFPCAo/29dY1UhsXGM1pO8prkjhfp6VhdUHuBIf8Ue20Hw== X-Received: by 10.28.238.221 with SMTP id j90mr495267wmi.44.1510326174360; Fri, 10 Nov 2017 07:02:54 -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 r14sm18948945wrb.43.2017.11.10.07.02.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Nov 2017 07:02:53 -0800 (PST) Date: Fri, 10 Nov 2017 16:02:42 +0100 From: Adrien Mazarguil To: Matan Azrad Cc: dev@dpdk.org, Ophir Munk , Moti Haimovsky , Thomas Monjalon , Olga Shern Message-ID: <20171110150241.GI24849@6wind.com> References: <1510302438-29138-1-git-send-email-matan@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1510302438-29138-1-git-send-email-matan@mellanox.com> Subject: Re: [dpdk-dev] [PATCH] net/mlx4: fix last Tx wqe stamping lack 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: Fri, 10 Nov 2017 15:02:54 -0000 On Fri, Nov 10, 2017 at 08:27:18AM +0000, Matan Azrad wrote: > When Tx pakcet HW processing is done, SW should stamp all the completion > burst WQEs. > > Stamp missed last completion burst WQE. > > Fixes: c3c977bbecbd ("net/mlx4: add Tx bypassing Verbs") > > Signed-off-by: Matan Azrad This reads like you were in a hurry :) Took me a while to understand the problem and how you addressed it. So in short, wqe_index is consumed but its TXBBs aren't stamped because the loop stops at its index without processing it. Patch looks good but could have been simpler by directly initializing nr_txbbs to sq->tail, not use sq->tail as an offset afterward and get rid of sq_tail. It's OK as this wouldn't have resulted in a smaller patch anyway. Commit log rewording suggestion: net/mlx4: fix missing stamp during Tx completion After processing completed packets, the owner bit of each TXBB comprised in its WQEs must be invalidated. The loop stops short of processing the last WQE. Other than that, Acked-by: Adrien Mazarguil > --- > drivers/net/mlx4/mlx4_rxtx.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > I think this is a critical bug fix that should be added to 17.11 version. > No performance impact was seen. > > diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c > index 3985e06..44edeac 100644 > --- a/drivers/net/mlx4/mlx4_rxtx.c > +++ b/drivers/net/mlx4/mlx4_rxtx.c > @@ -336,6 +336,7 @@ struct pv { > { > unsigned int elts_comp = txq->elts_comp; > unsigned int elts_tail = txq->elts_tail; > + unsigned int sq_tail = sq->tail; > struct mlx4_cq *cq = &txq->mcq; > volatile struct mlx4_cqe *cqe; > uint32_t cons_index = cq->cons_index; > @@ -372,13 +373,13 @@ struct pv { > rte_be_to_cpu_16(cqe->wqe_index) & sq->txbb_cnt_mask; > do { > /* Free next descriptor. */ > - nr_txbbs += > + sq_tail += nr_txbbs; > + nr_txbbs = > mlx4_txq_stamp_freed_wqe(sq, > - (sq->tail + nr_txbbs) & sq->txbb_cnt_mask, > - !!((sq->tail + nr_txbbs) & sq->txbb_cnt)); > + sq_tail & sq->txbb_cnt_mask, > + !!(sq_tail & sq->txbb_cnt)); > pkts++; > - } while (((sq->tail + nr_txbbs) & sq->txbb_cnt_mask) != > - new_index); > + } while ((sq_tail & sq->txbb_cnt_mask) != new_index); > cons_index++; > } while (1); > if (unlikely(pkts == 0)) > @@ -386,7 +387,7 @@ struct pv { > /* Update CQ. */ > cq->cons_index = cons_index; > *cq->set_ci_db = rte_cpu_to_be_32(cq->cons_index & MLX4_CQ_DB_CI_MASK); > - sq->tail = sq->tail + nr_txbbs; > + sq->tail = sq_tail + nr_txbbs; > /* Update the list of packets posted for transmission. */ > elts_comp -= pkts; > assert(elts_comp <= txq->elts_comp); -- Adrien Mazarguil 6WIND