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 63F79D532; Fri, 11 Nov 2016 10:16:43 +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 <0OGH00HSY0FTOI40@mailout2.w1.samsung.com>; Fri, 11 Nov 2016 09:16:41 +0000 (GMT) Received: from eusmges1.samsung.com (unknown [203.254.199.239]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20161111091641eucas1p27af35c6651bddfd68f017fb6f08b5176~F84QIUgsw0699506995eucas1p2X; Fri, 11 Nov 2016 09:16:41 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges1.samsung.com (EUCPMTA) with SMTP id E3.43.23383.97C85285; Fri, 11 Nov 2016 09:16:41 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20161111091640eucas1p13921bdde81ba2a275bf67b09bed54d43~F84Pbo5hX2353523535eucas1p1T; Fri, 11 Nov 2016 09:16:40 +0000 (GMT) X-AuditID: cbfec7ef-f79e76d000005b57-b0-58258c79d7fe Received: from eusync2.samsung.com ( [203.254.199.212]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id FE.FB.10494.B5C85285; Fri, 11 Nov 2016 09:16:11 +0000 (GMT) Received: from [106.109.129.180] by eusync2.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OGH00FS10FQHD20@eusync2.samsung.com>; Fri, 11 Nov 2016 09:16:40 +0000 (GMT) To: Jan Blunck , Declan Doherty , bruce.richardson@intel.com Cc: dev@dpdk.org, Heetae Ahn , Yuanhan Liu , Eric Kinzie , Bernard Iremonger , stable@dpdk.org, Dyasly Sergey , Thomas Monjalon From: Ilya Maximets Message-id: <8f4e834f-69b2-cac9-2b0a-86ffc295530f@samsung.com> Date: Fri, 11 Nov 2016 12:16:38 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-version: 1.0 In-reply-to: <8507e1f4-328e-a13d-cee2-69db06fe30be@samsung.com> Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrPKsWRmVeSWpSXmKPExsWy7djPc7qVPaoRBm3rjSw2Nwpb3Fhlb/Hm QROLxbtP25ks5p3ay24x7fNtdosr7T/ZLVoftjJZTJ4tZfGv4w+7xZdN09ksrk+4wOrA43Gx /w6jx68FS1k9ds66y+6xeYWWx+I9L5k85p0M9OjbsooxgD2KyyYlNSezLLVI3y6BK+PwzINM BdvUKm7ue8nawNgp38XIwSEhYCJx6JpPFyMnkCkmceHeerYuRi4OIYFljBKTdr1hh3A+M0o0 fPzDClFlIjHrxQMmuKqrK6ewQjgvGCX69i9kBqkSFgiX2H1hISPIChGBfIlp+1RBapgF5jNJ zOnbwwJSwyagI3Fq9RFGEJtXwE6ipe82mM0ioCrx7uwtsF5RgQiJ/jPqECWCEj8m3wNr5RSw l1j6+zxYObOApsSLL5NYIGx5ic1r3jKD7JIQuMYuMW3iLSaIN2UlNh1ghnjARWLfvsnsELaw xKvjW6BsGYnLk7tZIOxqiYlb29gh5rQwSiyc+APqe3uJUzevMkEs45OYtG06M8R8XomONiGI Eg+J/3e3Qe1ylLjS+AEaPodZJH4vWM44gVF+FpJ/ZiH5YRaSHxYwMq9iFEktLc5NTy021CtO zC0uzUvXS87P3cQITEan/x1/v4PxaXPIIUYBDkYlHl6JTpUIIdbEsuLK3EOMEhzMSiK8ad2q EUK8KYmVValF+fFFpTmpxYcYpTlYlMR59y64Ei4kkJ5YkpqdmlqQWgSTZeLglGpg5Dv0rrtT W/dJQZqL4Yrz2U7hq+a2z85p8Wk/66XQs+m4b+Vk0TC1J5sSX5jzm7Btn/nJ3mRu6fKl7xe5 +Z9NfMoWm6d9cPYN2TPHX91WEd6840/DAo53U8KVO8KfPJz78OoRW0m2T/+cGDIfbU6Nvnda 8bXd8olvypfv/u5+VbG661FSeY7GAiWW4oxEQy3mouJEALleEURCAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrPIsWRmVeSWpSXmKPExsVy+t/xK7rRPaoRBtMem1psbhS2uLHK3uLN gyYWi3eftjNZzDu1l91i2ufb7BZX2n+yW7Q+bGWymDxbyuJfxx92iy+bprNZXJ9wgdWBx+Ni /x1Gj18LlrJ67Jx1l91j8wotj8V7XjJ5zDsZ6NG3ZRVjAHuUm01GamJKapFCal5yfkpmXrqt UmiIm66FkkJeYm6qrVKErm9IkJJCWWJOKZBnZIAGHJwD3IOV9O0S3DIOzzzIVLBNreLmvpes DYyd8l2MnBwSAiYSs148YIKwxSQu3FvP1sXIxSEksIRRovf+KxaQhJDAC0aJc7OKQGxhgXCJ 3RcWMoLYIgL5Eh1b1jNCNBxmkXjZ8YwZxGEWWMgk0frtITtIFZuAjsSp1UfAOngF7CRa+m6D 2SwCqhLvzt4Cs0UFIiRuPexggagRlPgx+R6YzSlgL7H093mgGg6goeoSU6bkgoSZBeQlNq95 yzyBUWAWko5ZCFWzkFQtYGRexSiSWlqcm55bbKRXnJhbXJqXrpecn7uJERiZ24793LKDsetd 8CFGAQ5GJR7eA20qEUKsiWXFlbmHGCU4mJVEeNO6VSOEeFMSK6tSi/Lji0pzUosPMZoCvTCR WUo0OR+YNPJK4g1NDM0tDY2MLSzMjYyUxHmnfrgSLiSQnliSmp2aWpBaBNPHxMEp1cDonzP5 1TOni6/XzzFibYuas8qCT0pTe1fYxnvrktWyfoburTpzNi/zqtftLXuVtgiksy1W8nv7o7y7 vfbHt1OHatX1d1RFvlAIUCiOmt9oyy37aMu/gGYHjUQG9jk6uSeO1lZ1/D+QGxTE3iJ8s/Jd BgvvdLW2hs7Ty0/sdQ5ter08eNv3ywlKLMUZiYZazEXFiQAuo4cG4gIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20161111091640eucas1p13921bdde81ba2a275bf67b09bed54d43 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 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: 20161018122856eucas1p22493fbd48031126de6093fd312e7a0fc X-RootMTR: 20161018122856eucas1p22493fbd48031126de6093fd312e7a0fc References: <1473251290-22053-1-git-send-email-i.maximets@samsung.com> <03e11be9-45b2-e05f-99f1-8be1bcc05f19@samsung.com> <8507e1f4-328e-a13d-cee2-69db06fe30be@samsung.com> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] Revert "bonding: use existing enslaved device queues" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Nov 2016 09:16:43 -0000 Ping. On 28.10.2016 09:14, Ilya Maximets wrote: > On 25.10.2016 09:26, Ilya Maximets wrote: >> On 24.10.2016 17:54, Jan Blunck wrote: >>> On Wed, Oct 19, 2016 at 5:47 AM, Ilya Maximets wrote: >>>> On 18.10.2016 18:19, Jan Blunck wrote: >>>>> On Tue, Oct 18, 2016 at 2:49 PM, Ilya Maximets wrote: >>>>>> On 18.10.2016 15:28, Jan Blunck wrote: >>>>>>> If the application already configured queues the PMD should not >>>>>>> silently claim ownership and reset them. >>>>>>> >>>>>>> What exactly is the problem when changing MTU? This works fine from >>>>>>> what I can tell. >>>>>> >>>>>> Following scenario leads to APP PANIC: >>>>>> >>>>>> 1. mempool_1 = rte_mempool_create() >>>>>> 2. rte_eth_rx_queue_setup(bond0, ..., mempool_1); >>>>>> 3. rte_eth_dev_start(bond0); >>>>>> 4. mempool_2 = rte_mempool_create(); >>>>>> 5. rte_eth_dev_stop(bond0); >>>>>> 6. rte_eth_rx_queue_setup(bond0, ..., mempool_2); >>>>>> 7. rte_eth_dev_start(bond0); >>>>>> * RX queues still use 'mempool_1' because reconfiguration doesn't affect them. * >>>>>> 8. rte_mempool_free(mempool_1); >>>>>> 9. On any rx operation we'll get PANIC because of using freed 'mempool_1': >>>>>> PANIC in rte_mempool_get_ops(): >>>>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)" failed >>>>>> >>>>>> You may just start OVS 2.6 with DPDK bonding device and attempt to change MTU via 'mtu_request'. >>>>>> Bug is easily reproducible. >>>>>> >>>>> >>>>> I see. I'm not 100% that this is expected to work without leaking the >>>>> driver's queues though. The driver is allowed to do allocations in >>>>> its rx_queue_setup() function that are being freed via >>>>> rx_queue_release() later. But rx_queue_release() is only called if you >>>>> reconfigure the >>>>> device with 0 queues. >> >> It's not true. Drivers usually calls 'rx_queue_release()' inside >> 'rx_queue_setup()' function while reallocating the already allocated >> queue. (See ixgbe driver for example). Also all HW queues are >> usually destroyed inside 'eth_dev_stop()' and reallocated in >> 'eth_dev_start()' or '{rx,tx}_queue_setup()'. >> So, there is no leaks at all. >> >>>>> From what I understand there is no other way to >>>>> reconfigure a device to use another mempool. >>>>> >>>>> But ... even that wouldn't work with the bonding driver right now: the >>>>> bonding master only configures the slaves during startup. I can put >>>>> that on my todo list. >> >> No, bonding master reconfigures new slaves in 'rte_eth_bond_slave_add()' >> if needed. >> >>>>> Coming back to your original problem: changing the MTU for the bond >>>>> does work through rte_eth_dev_set_mtu() for slaves supporting that. In >>>>> any other case you could (re-)configure rxmode.max_rx_pkt_len (and >>>>> jumbo_frame / enable_scatter accordingly). This does work without a >>>>> call to rte_eth_rx_queue_setup(). >>>> >>>> Thanks for suggestion, but using of rte_eth_dev_set_mtu() without >>>> reconfiguration will require to have mempools with huge mbufs (9KB) >>>> for all ports from the start. This is unacceptable because leads to >>>> significant performance regressions because of fast cache exhausting. >>>> Also this will require big work to rewrite OVS reconfiguration code >>>> this way. >>>> Anyway, it isn't the MTU only problem. Number of rx/tx descriptors >>>> also can't be changed in runtime. >>>> >>>> >>>> I'm not fully understand what is the use case for this 'reusing' code. >>>> Could you, please, describe situation where this behaviour is necessary? >>> >>> The device that is added to the bond was used before and therefore >>> already has allocated queues. Therefore we reuse the existing queues >>> of the devices instead of borrowing the queues of the bond device. If >>> the slave is removed from the bond again there is no need to allocate >>> the queues again. >>> >>> Hope that clarifies the usecase >> >> So, As I see, this is just an optimization that leads to differently >> configured queues of same device and possible application crash if the >> old configuration doesn't valid any more. >> >> With revert applied in your usecase while adding the device to bond >> it's queues will be automatically reconfigured according to configuration >> of the bond device. If the slave is removed from the bond all its' >> queues will remain as configured by bond. You can reconfigure them if >> needed. I guess, that in your case configuration of slave devices, >> actually, matches configuration of bond device. In that case slave >> will remain in the same state after removing from bond as it was >> before adding. > > So, Jan, Declan, what do you think about this? > > Best regards, Ilya Maximets. >