From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DB692A0564; Sat, 7 Mar 2020 13:52:03 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 157281BF90; Sat, 7 Mar 2020 13:52:03 +0100 (CET) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 285B23B5 for ; Sat, 7 Mar 2020 13:52:01 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us5.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id A650740051; Sat, 7 Mar 2020 12:51:59 +0000 (UTC) Received: from [127.0.0.27] (10.17.10.39) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Sat, 7 Mar 2020 12:51:48 +0000 To: Jerin Jacob , Tonghao Zhang CC: dpdk-dev , Olivier Matz , Gage Eads , "Artem V. Andreev" , Jerin Jacob , Nithin Dabilpuram , Vamsi Attunuru , "Hemant Agrawal" References: <1583114253-15345-1-git-send-email-xiangxia.m.yue@gmail.com> <1583501776-9958-1-git-send-email-xiangxia.m.yue@gmail.com> From: Andrew Rybchenko Message-ID: <7420c590-4906-34e2-b0b8-d412df9005c8@solarflare.com> Date: Sat, 7 Mar 2020 15:51:29 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [10.17.10.39] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1020-25274.003 X-TM-AS-Result: No-15.010500-8.000000-10 X-TMASE-MatchedRID: eVEkOcJu0F7mLzc6AOD8DfHkpkyUphL9q/CNWo5Wx+O2ZO4LLn5VSuFo /YmgpQJJLKxcrLNnnsbsmswpTDj0yh1YpEPWJiyz2oIkXC6Ol0Bj8vb8BkkBZMcySOqW2ey321p BzAE1M7ry2UZqCS3UtWhKmI6nO4J3O5cso4YfEOxH+PTjR9EWkm5BhzAcWjgGy5JfHvVu9IuMbv oO9mofAassgi/t6bTQqUFzXY3N6oG6687/uEqqCVkxnoxnQfVSOHhqIXe4IzYK1SsgKmaayYgnL dHU7oiOrsRbop3C67AO6bgzXko5LODocHyqS9HwE367MfaNw8PM8zLNncnslQdC9P1Le8dQgOD5 MuJpssvi8zVgXoAltkWL4rBlm20vt7DW3B48kkHdB/CxWTRRu/558CedkGIvzP4Frv6e/e6gWFr 3itL2GBIsF0+sY4cVtcZo1AuDwWGzhtXniSXloX7cGd19dSFd X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--15.010500-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1020-25274.003 X-MDID: 1583585520-9KPW5wtaOMkL Subject: Re: [dpdk-dev] [PATCH dpdk-dev v3] mempool: sort the rte_mempool_ops by name 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 3/6/20 4:37 PM, Jerin Jacob wrote: > On Fri, Mar 6, 2020 at 7:06 PM wrote: >> From: Tonghao Zhang >> >> The order of mempool initiation affects mempool index in the >> rte_mempool_ops_table. For example, when building APPs with: >> >> $ gcc -lrte_mempool_bucket -lrte_mempool_ring ... >> >> The "bucket" mempool will be registered firstly, and its index >> in table is 0 while the index of "ring" mempool is 1. DPDK >> uses the mk/rte.app.mk to build APPs, and others, for example, >> Open vSwitch, use the libdpdk.a or libdpdk.so to build it. >> The mempool lib linked in dpdk and Open vSwitch is different. >> >> The mempool can be used between primary and secondary process, >> such as dpdk-pdump and pdump-pmd/Open vSwitch(pdump enabled). >> There will be a crash because dpdk-pdump creates the "ring_mp_mc" >> ring which index in table is 0, but the index of "bucket" ring >> is 0 in Open vSwitch. If Open vSwitch use the index 0 to get >> mempool ops and malloc memory from mempool. The crash will occur: >> >> bucket_dequeue (access null and crash) >> rte_mempool_get_ops (should get "ring_mp_mc", >> but get "bucket" mempool) >> rte_mempool_ops_dequeue_bulk >> ... >> rte_pktmbuf_alloc >> rte_pktmbuf_copy >> pdump_copy >> pdump_rx >> rte_eth_rx_burst >> >> To avoid the crash, there are some solution: >> * constructor priority: Different mempool uses different >> priority in RTE_INIT, but it's not easy to maintain. >> >> * change mk/rte.app.mk: Change the order in mk/rte.app.mk to >> be same as libdpdk.a/libdpdk.so, but when adding a new mempool >> driver in future, we must make sure the order. >> >> * register mempool orderly: Sort the mempool when registering, >> so the lib linked will not affect the index in mempool table. >> >> Signed-off-by: Tonghao Zhang >> Acked-by: Olivier Matz > Acked-by: Jerin Jacob The patch is OK, but the fact that ops index changes during mempool driver lifetime is frightening. In fact it breaks rte_mempool_register_ops() return value semantics (read as API break). The return value is not used in DPDK, but it is a public function. If I'm not mistaken it should be taken into account. Also I remember patches which warn about above behaviour in documentation. If behaviour changes, corresponding documentation must be updated.