From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-AM5-obe.outbound.protection.outlook.com (mail-eopbgr30089.outbound.protection.outlook.com [40.107.3.89]) by dpdk.org (Postfix) with ESMTP id 940B41D8E for ; Wed, 6 Dec 2017 12:44:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=8GoCyScmZeVnQTcrqbv9jf7Jc+cLU9wRt+yZTcQhxjc=; b=yy3tP6qaK3Mld7a+dsw3yIRWYgRHL0Jhf0xDyZF8LKSZ0InhJAM5UQkBVmnf0O+Ga1WuLBqt3HSDpMoXK1czUHwstaYCxprRP5tLVFt9bx4y202h45aCkkSgvbDDzoKx4OOzt7xqrMWhsLBhCdec8SvpHpLaFhoBrzbFJj0G3bA= Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com (10.167.127.17) by HE1PR0502MB3658.eurprd05.prod.outlook.com (10.167.127.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.282.5; Wed, 6 Dec 2017 11:44:51 +0000 Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::982e:2dce:9449:6891]) by HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::982e:2dce:9449:6891%13]) with mapi id 15.20.0282.012; Wed, 6 Dec 2017 11:44:51 +0000 From: Matan Azrad To: Adrien Mazarguil CC: "dev@dpdk.org" Thread-Topic: [PATCH 7/8] net/mlx4: align Tx descriptors number Thread-Index: AQHTboFOAy5FauF2O06k66AsyM32RaM2MfMw Date: Wed, 6 Dec 2017 11:44:51 +0000 Message-ID: References: <1511871570-16826-1-git-send-email-matan@mellanox.com> <1511871570-16826-8-git-send-email-matan@mellanox.com> <20171206105922.GY4062@6wind.com> In-Reply-To: <20171206105922.GY4062@6wind.com> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; HE1PR0502MB3658; 6:ce+4qV654ka/CUcVP6NdVayf+XsavD0Kr9dUAhN97D0PmgfAkpFPZxEJ58b++jcfezccajuwYseZkTQiAy3jC4/hgdYVlLHG2pGHxMLHFaE44WojMhW2i9lIMoZcNUK//M9zsQA9P/9uA84UUo0DX3RF59FISSeL/1VZhnkHexTiQOaUkp/mmJX3iUlIFmGp7ERE2mYm1nRcOimsHjy5gOwf91wSMlh44BT8wnkACWL3fcUExyGyW0o3/p8Zey1AOO5kv/pjXrwGyAfaM6SFlzA6pxmOpFbarFXYBBHrPjDWvz0Cdt7pLgFheWyPsFu6obD4UfLLXqd8Je/ErpnmsVIxg089TujhiaYvhRC1n4o=; 5:iTaWy+SNyiB3nXP/q7YZmz4wEHsqKxgkDnVriOSXBB5LtP4joIaFg0D+01aeaCsmvuidRVHNAGQPYmIS0x8kgWrKdw1mkXYVrwWwOYqmT7G47iuMeaR/p9zuv2FGDUJ4Fv0HInNDqcUhki43Sb2ticPNn9FLfvQK5tRwH2Z9skI=; 24:pUFa+1S4/QgEuXoVzEG8gh5km7bJYtKXd0z3kfa++ZL1wPDEc5A5jiojpauNPzYqb6cpBhaepv+6r7tmt/bkMAyhTnwr7pisJXIeZjMpZOA=; 7:+vr6Kn/kDGORu45g0YDxtlIm3q9wP/mFfb/aNRYLGTdbqIsIOZHBwesw1TSYP/bhmbD5CIjxsGGFmqeygBE6YIJ8CoVewMHsPPqfoV3aVDMitOY3Txxw1DvNNWPR3JVaQ+gW8E3t0+rbmhX5qvwxTlmzyIYNcn7TE5y1R3uZxAF6RPuqH4OHWoNgkHJchjpjVisvJMM5JhyNnjM+NfJLYQ791+V+Mi7SszCzkbFgLYqiqrLm8hzorLlii+tZGSCC x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 77264098-1ec2-4158-8bed-08d53c9ec2f8 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(5600026)(4604075)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(48565401081)(2017052603286); SRVR:HE1PR0502MB3658; x-ms-traffictypediagnostic: HE1PR0502MB3658: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(2401047)(8121501046)(5005006)(10201501046)(3002001)(93006095)(93001095)(3231022)(6055026)(6041248)(20161123562025)(20161123564025)(20161123555025)(20161123558100)(20161123560025)(201703131423075)(201703011903075)(201702281528075)(201703061421075)(6072148)(201708071742011); SRVR:HE1PR0502MB3658; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:HE1PR0502MB3658; x-forefront-prvs: 05134F8B4F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(7966004)(346002)(366004)(39860400002)(376002)(24454002)(189003)(199004)(13464003)(4326008)(76176011)(55016002)(102836003)(6506006)(6116002)(53936002)(3846002)(229853002)(8936002)(53546010)(7696005)(106356001)(97736004)(68736007)(6246003)(99286004)(5250100002)(105586002)(101416001)(5660300001)(2950100002)(6916009)(508600001)(305945005)(7736002)(14454004)(74316002)(9686003)(2900100001)(2906002)(25786009)(81156014)(8676002)(316002)(86362001)(81166006)(3660700001)(6436002)(3280700002)(33656002)(66066001); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0502MB3658; H:HE1PR0502MB3659.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 77264098-1ec2-4158-8bed-08d53c9ec2f8 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Dec 2017 11:44:51.4642 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3658 Subject: Re: [dpdk-dev] [PATCH 7/8] net/mlx4: align Tx descriptors number 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 11:44:52 -0000 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 7/8] net/mlx4: align Tx descriptors number >=20 > On Tue, Nov 28, 2017 at 12:19:29PM +0000, Matan Azrad wrote: > > Using power of 2 descriptors number makes the ring management easier > > and allows to use mask operation instead of wraparound conditions. > > > > Adjust Tx descriptor number to be power of 2 and change calculation to > > use mask accordingly. > > > > Signed-off-by: Matan Azrad >=20 > A few minor comments, see below. >=20 > > --- > > drivers/net/mlx4/mlx4_rxtx.c | 22 ++++++++-------------- > > drivers/net/mlx4/mlx4_txq.c | 20 ++++++++++++-------- > > 2 files changed, 20 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/net/mlx4/mlx4_rxtx.c > > b/drivers/net/mlx4/mlx4_rxtx.c index 30f2930..b5aaf4c 100644 > > --- a/drivers/net/mlx4/mlx4_rxtx.c > > +++ b/drivers/net/mlx4/mlx4_rxtx.c > > @@ -314,7 +314,7 @@ struct pv { > > * Pointer to Tx queue structure. > > */ > > static void > > -mlx4_txq_complete(struct txq *txq, const unsigned int elts_n, > > +mlx4_txq_complete(struct txq *txq, const unsigned int elts_m, > > struct mlx4_sq *sq) >=20 > Documentation needs updating. >=20 OK > > { > > unsigned int elts_tail =3D txq->elts_tail; @@ -355,13 +355,11 @@ > > struct pv { > > if (unlikely(!completed)) > > return; > > /* First stamping address is the end of the last one. */ > > - first_txbb =3D (&(*txq->elts)[elts_tail])->eocb; > > + first_txbb =3D (&(*txq->elts)[elts_tail & elts_m])->eocb; > > elts_tail +=3D completed; > > - if (elts_tail >=3D elts_n) > > - elts_tail -=3D elts_n; > > /* The new tail element holds the end address. */ > > sq->remain_size +=3D mlx4_txq_stamp_freed_wqe(sq, first_txbb, > > - (&(*txq->elts)[elts_tail])->eocb); > > + (&(*txq->elts)[elts_tail & elts_m])->eocb); > > /* Update CQ consumer index. */ > > cq->cons_index =3D cons_index; > > *cq->set_ci_db =3D rte_cpu_to_be_32(cons_index & > MLX4_CQ_DB_CI_MASK); > > @@ -534,6 +532,7 @@ struct pv { > > struct txq *txq =3D (struct txq *)dpdk_txq; > > unsigned int elts_head =3D txq->elts_head; > > const unsigned int elts_n =3D txq->elts_n; > > + const unsigned int elts_m =3D elts_n - 1; > > unsigned int bytes_sent =3D 0; > > unsigned int i; > > unsigned int max; > > @@ -543,24 +542,20 @@ struct pv { > > > > assert(txq->elts_comp_cd !=3D 0); > > if (likely(txq->elts_comp !=3D 0)) > > - mlx4_txq_complete(txq, elts_n, sq); > > + mlx4_txq_complete(txq, elts_m, sq); > > max =3D (elts_n - (elts_head - txq->elts_tail)); > > - if (max > elts_n) > > - max -=3D elts_n; > > assert(max >=3D 1); > > assert(max <=3D elts_n); > > /* Always leave one free entry in the ring. */ > > --max; > > if (max > pkts_n) > > max =3D pkts_n; > > - elt =3D &(*txq->elts)[elts_head]; > > + elt =3D &(*txq->elts)[elts_head & elts_m]; > > /* First Tx burst element saves the next WQE control segment. */ > > ctrl =3D elt->wqe; > > for (i =3D 0; (i !=3D max); ++i) { > > struct rte_mbuf *buf =3D pkts[i]; > > - unsigned int elts_head_next =3D > > - (((elts_head + 1) =3D=3D elts_n) ? 0 : elts_head + 1); > > - struct txq_elt *elt_next =3D &(*txq->elts)[elts_head_next]; > > + struct txq_elt *elt_next =3D &(*txq->elts)[++elts_head & > elts_m]; > > uint32_t owner_opcode =3D sq->owner_opcode; > > volatile struct mlx4_wqe_data_seg *dseg =3D > > (volatile struct mlx4_wqe_data_seg *)(ctrl + > 1); @@ -678,7 +673,6 > > @@ struct pv { > > ctrl->owner_opcode =3D rte_cpu_to_be_32(owner_opcode); > > elt->buf =3D buf; > > bytes_sent +=3D buf->pkt_len; > > - elts_head =3D elts_head_next; > > ctrl =3D ctrl_next; > > elt =3D elt_next; > > } > > @@ -694,7 +688,7 @@ struct pv { > > rte_wmb(); > > /* Ring QP doorbell. */ > > rte_write32(txq->msq.doorbell_qpn, txq->msq.db); > > - txq->elts_head =3D elts_head; > > + txq->elts_head +=3D i; > > txq->elts_comp +=3D i; > > return i; > > } > > diff --git a/drivers/net/mlx4/mlx4_txq.c b/drivers/net/mlx4/mlx4_txq.c > > index 4c7b62a..253075a 100644 > > --- a/drivers/net/mlx4/mlx4_txq.c > > +++ b/drivers/net/mlx4/mlx4_txq.c > > @@ -76,17 +76,16 @@ > > unsigned int elts_head =3D txq->elts_head; > > unsigned int elts_tail =3D txq->elts_tail; > > struct txq_elt (*elts)[txq->elts_n] =3D txq->elts; > > + unsigned int elts_m =3D txq->elts_n - 1; > > > > DEBUG("%p: freeing WRs", (void *)txq); > > while (elts_tail !=3D elts_head) { > > - struct txq_elt *elt =3D &(*elts)[elts_tail]; > > + struct txq_elt *elt =3D &(*elts)[elts_tail++ & elts_m]; > > > > assert(elt->buf !=3D NULL); > > rte_pktmbuf_free(elt->buf); > > elt->buf =3D NULL; > > elt->wqe =3D NULL; > > - if (++elts_tail =3D=3D RTE_DIM(*elts)) > > - elts_tail =3D 0; > > } > > txq->elts_tail =3D txq->elts_head; > > } > > @@ -208,7 +207,9 @@ struct txq_mp2mr_mbuf_check_data { > > struct mlx4dv_obj mlxdv; > > struct mlx4dv_qp dv_qp; > > struct mlx4dv_cq dv_cq; > > - struct txq_elt (*elts)[desc]; > > + uint32_t elts_size =3D desc > 0x1000 ? 0x1000 : > > + rte_align32pow2((uint32_t)desc); >=20 > Where is that magical 0x1000 value coming from? It should at least come > through a macro definition. >=20 > > + struct txq_elt (*elts)[elts_size]; > > struct ibv_qp_init_attr qp_init_attr; > > struct txq *txq; > > uint8_t *bounce_buf; > > @@ -247,11 +248,14 @@ struct txq_mp2mr_mbuf_check_data { > > (void *)dev, idx); > > return -rte_errno; > > } > > - if (!desc) { > > - rte_errno =3D EINVAL; > > - ERROR("%p: invalid number of Tx descriptors", (void *)dev); > > - return -rte_errno; > > + if ((uint32_t)desc !=3D elts_size) { > > + desc =3D (uint16_t)elts_size; > > + WARN("%p: changed number of descriptors in TX queue %u" > > + " to be power of two (%d)", > > + (void *)dev, idx, desc); >=20 > You should display the same message as in mlx4_rxq.c for consistency (als= o > TX =3D> Tx). >=20 OK > > } > > + DEBUG("%p: configuring queue %u for %u descriptors", > > + (void *)dev, idx, desc); >=20 > Unnecessary duplicated debugging message already printed at the beginning > of this function. Yes this is a different value but WARN() made that pret= ty > clear. >=20 > > /* Allocate and initialize Tx queue. */ > > mlx4_zmallocv_socket("TXQ", vec, RTE_DIM(vec), socket); > > if (!txq) { > > -- > > 1.8.3.1 > > >=20 > -- > Adrien Mazarguil > 6WIND