From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.radware.com (mailout1.radwarecloud.com [192.115.180.130]) by dpdk.org (Postfix) with ESMTP id 3D6EB2C23; Tue, 28 Feb 2017 08:50:49 +0100 (CET) Received: from ILMB2.corp.radware.com ([169.254.2.155]) by ILCAS1.corp.radware.com ([176.200.120.121]) with mapi id 14.03.0319.002; Tue, 28 Feb 2017 09:50:47 +0200 From: Ami Sabo To: Yuanhan Liu CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [PATCH 2/2] net/virtio-user: fix multi-process issue Thread-Index: AQHSkBaShYWLocWUDkCYNeCNZRcyiqF92VYAgAAlsJA= Date: Tue, 28 Feb 2017 07:50:47 +0000 Message-ID: <25F949B549CFD14EABC392C69312415C0106DC094F@ILMB2.corp.radware.com> References: <1487851096-32479-1-git-send-email-amis@radware.com> <1488102926-24158-1-git-send-email-amis@radware.com> <1488102926-24158-3-git-send-email-amis@radware.com> <20170228064016.GL18844@yliu-dev.sh.intel.com> In-Reply-To: <20170228064016.GL18844@yliu-dev.sh.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [176.200.121.201] x-tm-as-product-ver: SMEX-11.0.0.4179-8.100.1062-22912.005 x-tm-as-result: No--1.799500-0.000000-31 x-tm-as-user-approved-sender: Yes x-tm-as-user-blocked-sender: No Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Feb 2017 07:50:49 -0000 Hi, You are right, the commit I mentioned didn't cause the issue - it just floo= ded it. The real issue is that rte_eth_dev_allocate should be called only from the = primary process. Tomas's commit flood the issue by resseting rte_eth_dev_data, so now, when = the virtio-user secondary process comes up and calls rte_eth_dev_allocate It clears the ethdev->data struct (so fields like rx_queues, mac_addrs, etc= will be 0, plus this may cause race condition between the primary and seco= ndary processes...)=20 --ami -----Original Message----- From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]=20 Sent: Tuesday, February 28, 2017 8:40 AM To: Ami Sabo Cc: dev@dpdk.org; stable@dpdk.org Subject: Re: [PATCH 2/2] net/virtio-user: fix multi-process issue On Sun, Feb 26, 2017 at 11:55:26AM +0200, Ami Sabo wrote: > Secondary process doesn't properly attach to the rte_eth_device=20 > initialized by the primary process. >=20 > ccessing device from secondary process (e.g. via rte_eth_rx_burst),=20 > causes process to crash. because rte_eth_dev_data is not properly set. >=20 > The issue was flood by > 'commit 7f95f78a8aea ("ethdev: clear data when allocating device")' > which now clears rte_eth_dev_data entry. > For pci devices the struct is initialized by rte_eth_dev_pci_probe > ->eth_dev_attach_secondary(). > However, for virtio-user virtio_user_pmd_probe() is called instead of=20 > rte_eth_dev_pci_probe(). >=20 > The fix is to call rte_eth_dev_attach_secondary(), for secondary=20 > process, from virtio_user_pmd_probe. >=20 > Fixes: 7f95f78a8aea ("ethdev: clear data when allocating device") Are you sure that's the real culprit? As I'm aware of, virtio-user is not b= uilt with multiple process support in the beginning. That said, it's likely= that the first commit introduces virtio-user is the "culprit" commit. Besides that, the code looks good to me. If Thomas is fine with your first = patch, I could merge them to my tree. --yliu