From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id A0FDF2B97 for ; Tue, 18 Oct 2016 09:57:23 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP; 18 Oct 2016 00:57:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,361,1473145200"; d="scan'208";a="21123874" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.26]) ([10.237.221.26]) by orsmga004.jf.intel.com with ESMTP; 18 Oct 2016 00:57:21 -0700 To: Marcin Kerlin , dev@dpdk.org References: <1474974783-4861-2-git-send-email-marcinx.kerlin@intel.com> <1475244055-6309-1-git-send-email-marcinx.kerlin@intel.com> Cc: pablo.de.lara.guarch@intel.com, thomas.monjalon@6wind.com From: Sergio Gonzalez Monroy Message-ID: <3ae82ec8-03bf-9a37-b81c-b8fc1fc30624@intel.com> Date: Tue, 18 Oct 2016 08:57:20 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <1475244055-6309-1-git-send-email-marcinx.kerlin@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 0/2] app/testpmd: improve multiprocess support 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, 18 Oct 2016 07:57:24 -0000 On 30/09/2016 15:00, Marcin Kerlin wrote: > This patch ensure not overwrite device data in the multiprocess application. > > 1)Changes in the library introduces continuity in array rte_eth_dev_data[] > shared between all processes. Secondary process adds new entries in free > space instead of overwriting existing entries. > > 2)Changes in application testpmd allow secondary process to attach the > mempool created by primary process rather than create new and in the case of > quit or force quit to free devices data from shared array rte_eth_dev_data[]. > > ------------------------- > How to reproduce the bug: > > 1) Run primary process: > ./testpmd -c 0xf -n 4 --socket-mem='512,0' -w 03:00.1 -w 03:00.0 > --proc-type=primary --file-prefix=xz1 -- -i > > (gdb) print rte_eth_devices[0].data.name > $52 = "3:0.1" > (gdb) print rte_eth_devices[1].data.name > $53 = "3:0.0" > > 2) Run secondary process: > ./testpmd -c 0xf0 --socket-mem='512,0' -n 4 -v -b 03:00.1 -b 03:00.0 > --vdev 'eth_pcap0,rx_pcap=/var/log/device1.pcap, tx_pcap=/var/log/device2.pcap' > --proc-type=secondary --file-prefix=xz1 -- -i > > (gdb) print rte_eth_devices[0].data.name > $52 = "eth_pcap0" > (gdb) print rte_eth_devices[1].data.name > $53 = "eth_pcap1" > > 3) Go back to the primary and re-check: > (gdb) print rte_eth_devices[0].data.name > $54 = "eth_pcap0" > (gdb) print rte_eth_devices[1].data.name > $55 = "eth_pcap1" > > It means that secondary process overwrite data of primary process. > > This patch fix it and now if we go back to the primary and re-check then > everything is fine: > (gdb) print rte_eth_devices[0].data.name > $56 = "3:0.1" > (gdb) print rte_eth_devices[1].data.name > $57 = "3:0.0" > > So after this fix structure rte_eth_dev_data[] will keep all data one after > the other instead of overwriting: > (gdb) print rte_eth_dev_data[0].name > $52 = "3:0.1" > (gdb) print rte_eth_dev_data[1].name > $53 = "3:0.0" > (gdb) print rte_eth_dev_data[2].name > $54 = "eth_pcap0" > (gdb) print rte_eth_dev_data[3].name > $55 = "eth_pcap1" > and so on will be append in the next indexes > > If secondary process will be turned off then also will be deleted from array: > (gdb) print rte_eth_dev_data[0].name > $52 = "3:0.1" > (gdb) print rte_eth_dev_data[1].name > $53 = "3:0.0" > (gdb) print rte_eth_dev_data[2].name > $54 = "" > (gdb) print rte_eth_dev_data[3].name > $55 = "" > this also allows re-use index 2 and 3 for next another process > ------------------------- > > Breaking ABI: > Changes in the library librte_ether causes extending existing structure > rte_eth_dev_data with a new field lock. The reason is that this structure > is sharing between all the processes so it should be protected against > attempting to write from two different processes. > > Tomasz Kulasek sent announce ABI change in librte_ether on 21 July 2016. > I would like to join to this breaking ABI, if it is possible. > > v2: > * fix syntax error in version script > v3: > * changed scope of function > * improved description > v4: > * fix syntax error in version script > v5: > * fix header file > > Marcin Kerlin (2): > librte_ether: add protection against overwrite device data > app/testpmd: improve handling of multiprocess > > app/test-pmd/testpmd.c | 37 +++++++++++++- > app/test-pmd/testpmd.h | 1 + > lib/librte_ether/rte_ethdev.c | 90 +++++++++++++++++++++++++++++++--- > lib/librte_ether/rte_ethdev.h | 12 +++++ > lib/librte_ether/rte_ether_version.map | 6 +++ > 5 files changed, 136 insertions(+), 10 deletions(-) > NACK series for 16.11 The patch would break the use case where primary and secondary share same PCI device. Overall, I think Thomas has already mentioned it, we need further discussion on the use cases and scope of DPDK multi-process. This could be a good topic for the upcoming DPDK Userspace event. Sergio