From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 349C93B5 for ; Tue, 29 Nov 2016 14:06:29 +0100 (CET) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OHE00BS7N2RQI60@mailout2.w1.samsung.com> for dev@dpdk.org; Tue, 29 Nov 2016 13:06:27 +0000 (GMT) Received: from eusmges4.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20161129130627eucas1p1320644fff2e2105e2eec7ddea81e2165~LhoAh0YLU0044400444eucas1p1L; Tue, 29 Nov 2016 13:06:27 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges4.samsung.com (EUCPMTA) with SMTP id D9.92.28332.35D7D385; Tue, 29 Nov 2016 13:06:27 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20161129130626eucas1p2147cfa3ca43f39b302c5a417d23a7459~Lhn-bT0LT0973609736eucas1p2U; Tue, 29 Nov 2016 13:06:26 +0000 (GMT) X-AuditID: cbfec7f4-f791c6d000006eac-64-583d7d53ce9a Received: from eusync1.samsung.com ( [203.254.199.211]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 3C.A3.07726.26D7D385; Tue, 29 Nov 2016 13:06:42 +0000 (GMT) Received: from [106.109.129.180] by eusync1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OHE00E2SN2PNH70@eusync1.samsung.com>; Tue, 29 Nov 2016 13:06:26 +0000 (GMT) To: "Ananyev, Konstantin" , "dev@dpdk.org" , "Zhang, Helin" , "Wu, Jingjing" Cc: Dyasly Sergey , Heetae Ahn , "Richardson, Bruce" , "Yigit, Ferruh" From: Ilya Maximets Message-id: <1ba4d257-32d7-c722-e97f-ef6085682006@samsung.com> Date: Tue, 29 Nov 2016 16:06:25 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-version: 1.0 In-reply-to: <2601191342CEEE43887BDE71AB9772583F0E19CD@irsmsx105.ger.corp.intel.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02SWUwTURiFvTPTdtpYMpQiv4gSBow7IPIwYZXgA49ECVYTgSoTQGjBFohg jIQQaCESoBV13AYQFYqyyBYtUZAUAwkSRYWIWF8UDEIiRhalShlMePvuPSfn/8/NJXFFhciT TNNmszqtOoMWy4hO29LwgWMXw1WBI1ekzFhjJDP7owtjJnqGJEz1/AcJY37XhZjRkiUJc52f R8zcn1qCMd3wPCyNWebrRTF11mkspry9EcXiJ2VhyWxGWi6rC4hIkqVWNb8SZzVtPW9/wxEF yKYsRSQJVDAU3tpRiqSruAVGJpvFpUhGKqh6BJy5jBAO8wiWbi6LBFcwTD2Yx52soO4heN9B C6YpBB2zTRKn4EbFgeVTn8QpKCkLAsfd8bVcnGpG8LHGuuYSU/th0NKPnHvIqQgwmg85rwlq J6y09q9NcKdU0NtqQU6WU66waJoknCyl4mGhp3CNcSoQrplfYAJ7w+Om77hzFlCNEmgtG0FC z+3Q9hwXGhyBae4JIbAbfBtolwjsBUZDLybwBajsKJYIOUUIaioX1+tHwuD42/VhLlDVeRUX 8uVgKFYIlhgoqR9DAkeBtb1l/RlHMVh4/QurQN7chj7chg7chg48whuRks3Ra1JYfbC/Xq3R 52hT/M9katrQ6j8Zcgz87EZ1tpA+RJGI3izvDglTKUTqXH2epg8BidNK+d/8cJVCnqzOy2d1 mYm6nAxW34e2kQTtIbfyo8cVVIo6m01n2SxW91/FSKlnAYpuaImLTfRYfGbn7S5Dc77V/CUf 9gsxoL3tkZwwE2RIc0ucOfX5ckunznf3w5Xo2l0NCcxv+uj9feXDvNRxIjbCLxTbxIXeKUwv xWhHpldS0iNDIOceZTSpno6fjreZ/OxizhqQlpocFAXnfGr5PY4iXtX98qyrg5kwfo2kCX2q +uBeXKdX/wP0sfUCIwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrAIsWRmVeSWpSXmKPExsVy+t/xy7pJtbYRBt/2K1vcWGVv8e7TdiaL O3tPs1tM+3yb3WLKte2MFlfaf7JbzFzwmdHi/Z9FLBaTZ0s5cHr8WrCU1WPxnpdMHn1bVjEG MEe52WSkJqakFimk5iXnp2TmpdsqhYa46VooKeQl5qbaKkXo+oYEKSmUJeaUAnlGBmjAwTnA PVhJ3y7BLWPS+vNsBWskKx5cnsXSwHhMpIuRk0NCwETixYrPzBC2mMSFe+vZuhi5OIQEljBK bN+3lhXCecEo0X5sC1iVsECIxIPGR2AJEYHVjBJ3/j1hgqi6wiRxeN1jsAyzwEZGicMLTjGC tLAJ6EicWn0EyObg4BWwk+icYgwSZhFQlfi78QjYVFGBCIlNX+ewgNi8AoISPybfA7M5BcIk pi96xA7SyiygJ3H/ohZImFlAXmLzmrfMExgFZiHpmIVQNQtJ1QJG5lWMIqmlxbnpucWGesWJ ucWleel6yfm5mxiBsbXt2M/NOxgvbQw+xCjAwajEwytgaxMhxJpYVlyZe4hRgoNZSYT3f5Vt hBBvSmJlVWpRfnxRaU5q8SFGU6AXJjJLiSbnA+M+ryTe0MTQ3NLQyNjCwtzISEmct+TDlXAh gfTEktTs1NSC1CKYPiYOTqkGxhL+LdEqAndiOjonRwhPym6aV/D9aUDGqc+ma+dc37okI1y7 KmhWc3HlzFXx/1s/fFUyvR41wUV4lvmXUP+M/L3M5UvWGnnWtR6YOstQdXe+5/pYr7j1RbpP Xi4/xCnYs7U+/b+vo8VTnaCyeTtb0k76PZjSc3hKR8AF24Uxizmm1f39a35qphJLcUaioRZz UXEiAFFgGb/DAgAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20161129130626eucas1p2147cfa3ca43f39b302c5a417d23a7459 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?SWx5YSBNYXhpbWV0cxtTUlItVmlydHVhbGl6YXRpb24gTGFi?= =?UTF-8?B?G+yCvOyEseyghOyekBtFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?SWx5YSBNYXhpbWV0cxtTUlItVmlydHVhbGl6YXRpb24gTGFi?= =?UTF-8?B?G1NhbXN1bmcgRWxlY3Ryb25pY3MbRW5naW5lZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG0NJU0hRG0MxMEdEMDFHRDAxMDE1NA==?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20161019140730eucas1p2b1cf9daba45bdbf915bb69b24a0a850f X-RootMTR: 20161019140730eucas1p2b1cf9daba45bdbf915bb69b24a0a850f References: <1476886037-4586-1-git-send-email-i.maximets@samsung.com> <1476886037-4586-2-git-send-email-i.maximets@samsung.com> <0b31fd94-d5b2-acb8-8d55-a6fe124c9886@samsung.com> <2601191342CEEE43887BDE71AB9772583F0E19CD@irsmsx105.ger.corp.intel.com> Subject: Re: [dpdk-dev] [PATCH RFC 1/2] net/i40e: allow bulk alloc for the max size desc ring X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Nov 2016 13:06:30 -0000 On 29.11.2016 15:50, Ananyev, Konstantin wrote: > Hi Ilya, > >> Ping. >> >> Best regards, Ilya Maximets. >> >> On 19.10.2016 17:07, Ilya Maximets wrote: >>> The only reason why bulk alloc disabled for the rings with >>> more than (I40E_MAX_RING_DESC - RTE_PMD_I40E_RX_MAX_BURST) >>> descriptors is the possible out-of-bound access to the dma >>> memory. But it's the artificial limit and can be easily >>> avoided by allocating of RTE_PMD_I40E_RX_MAX_BURST more >>> descriptors in memory. This will not interfere the HW and, >>> as soon as all rings' memory zeroized, Rx functions will >>> work correctly. >>> >>> This change allows to use vectorized Rx functions with >>> 4096 descriptors in Rx ring which is important to achieve >>> zero packet drop rate in high-load installations. >>> >>> Signed-off-by: Ilya Maximets >>> --- >>> drivers/net/i40e/i40e_rxtx.c | 24 +++++++++++++----------- >>> 1 file changed, 13 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c >>> index 7ae7d9f..1f76691 100644 >>> --- a/drivers/net/i40e/i40e_rxtx.c >>> +++ b/drivers/net/i40e/i40e_rxtx.c >>> @@ -409,15 +409,6 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq) >>> "rxq->rx_free_thresh=%d", >>> rxq->nb_rx_desc, rxq->rx_free_thresh); >>> ret = -EINVAL; >>> - } else if (!(rxq->nb_rx_desc < (I40E_MAX_RING_DESC - >>> - RTE_PMD_I40E_RX_MAX_BURST))) { >>> - PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: " >>> - "rxq->nb_rx_desc=%d, " >>> - "I40E_MAX_RING_DESC=%d, " >>> - "RTE_PMD_I40E_RX_MAX_BURST=%d", >>> - rxq->nb_rx_desc, I40E_MAX_RING_DESC, >>> - RTE_PMD_I40E_RX_MAX_BURST); >>> - ret = -EINVAL; >>> } >>> #else >>> ret = -EINVAL; >>> @@ -1698,8 +1689,19 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev, >>> rxq->rx_deferred_start = rx_conf->rx_deferred_start; >>> >>> /* Allocate the maximun number of RX ring hardware descriptor. */ >>> - ring_size = sizeof(union i40e_rx_desc) * I40E_MAX_RING_DESC; >>> - ring_size = RTE_ALIGN(ring_size, I40E_DMA_MEM_ALIGN); >>> + len = I40E_MAX_RING_DESC; >>> + >>> +#ifdef RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC >>> + /** >>> + * Allocating a little more memory because vectorized/bulk_alloc Rx >>> + * functions doesn't check boundaries each time. >>> + */ >>> + len += RTE_PMD_I40E_RX_MAX_BURST; >>> +#endif >>> + > > Looks good to me. > One question, though do we really need '+#ifdef RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC' here? > Why just not remove this ifdef here and always add allocate extra descriptors. > Konstantin I put it there because all other bulk_alloc related code in i40e is under this ifdef. Just to keep the code in consistent state. Plus, it saves memory a bit. I prefer to keep ifdef here, but maintainers are free to remove it while applying. >>> + ring_size = RTE_ALIGN(len * sizeof(union i40e_rx_desc), >>> + I40E_DMA_MEM_ALIGN); >>> + >>> rz = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, >>> ring_size, I40E_RING_BASE_ALIGN, socket_id); >>> if (!rz) { >>> > > >