From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v2 5/8] net/mlx4: merge Tx queue rings management Date: Wed, 6 Dec 2017 17:22:29 +0100 Message-ID: <20171206162229.GH4062@6wind.com> References: <1511871570-16826-1-git-send-email-matan@mellanox.com> <1512571693-15338-1-git-send-email-matan@mellanox.com> <1512571693-15338-6-git-send-email-matan@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Matan Azrad Return-path: Received: from mail-wr0-f193.google.com (mail-wr0-f193.google.com [209.85.128.193]) by dpdk.org (Postfix) with ESMTP id 768E9293B for ; Wed, 6 Dec 2017 17:22:42 +0100 (CET) Received: by mail-wr0-f193.google.com with SMTP id v22so4530801wrb.0 for ; Wed, 06 Dec 2017 08:22:42 -0800 (PST) Content-Disposition: inline In-Reply-To: <1512571693-15338-6-git-send-email-matan@mellanox.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Dec 06, 2017 at 02:48:10PM +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 completed work queue entries. > > 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 A few more comments on this version below. > diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c > index adf02c0..2467d1d 100644 > --- a/drivers/net/mlx4/mlx4_rxtx.c > +++ b/drivers/net/mlx4/mlx4_rxtx.c > @@ -61,9 +61,6 @@ > #include "mlx4_rxtx.h" > #include "mlx4_utils.h" > > -#define WQE_ONE_DATA_SEG_SIZE \ > - (sizeof(struct mlx4_wqe_ctrl_seg) + sizeof(struct mlx4_wqe_data_seg)) > - > /** > * Pointer-value pair structure used in tx_post_send for saving the first > * DWORD (32 byte) of a TXBB. > @@ -268,52 +265,48 @@ struct pv { > * > * @param sq > * Pointer to the SQ structure. > - * @param index > - * Index of the freed WQE. > - * @param num_txbbs > - * Number of blocks to stamp. > - * If < 0 the routine will use the size written in the WQ entry. > - * @param owner > - * The value of the WQE owner bit to use in the stamp. > + * @param wqe > + * Pointer of WQE address to stamp. Clarification on what happens on return is primarily needed here actually: @param[in, out] wqe Pointer of WQE address to stamp. This value is modified on return to store the address of the next WQE. > * > * @return > - * The number of Tx basic blocs (TXBB) the WQE contained. > + * WQE size and updates WQE address to the next WQE. You can leave the previous comment if @param wqe is properly documented. > @@ -654,7 +639,7 @@ struct pv { > > #ifndef NDEBUG > /* Poisoning. */ > - memset(elt, 0x66, sizeof(*elt)); > + memset(elt->buf, 0x66, sizeof(struct rte_mbuf)); This likely causes a crash (did you test in debug mode?) the goal is to poison the buffer address, not the entire mbuf. This should read: memset(&elt->buf, 0x66, sizeof(elt->buf)); -- Adrien Mazarguil 6WIND