* [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash. [not found] <1487851096-32479-1-git-send-email-amis@radware.com> @ 2017-02-26 9:55 ` Ami Sabo 2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo 2017-02-26 9:55 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo 0 siblings, 2 replies; 15+ messages in thread From: Ami Sabo @ 2017-02-26 9:55 UTC (permalink / raw) To: yuanhan.liu; +Cc: dev, stable, Ami Sabo The patchset fixes secondary process crash issue when it tries to access virtio-user pmd (e.g. via rte_eth_rx_burst). The crash happens because in virtio_user probing, eth_dev_attach_secondary is not being called, as it does from rte_eth_dev_pci_probe. Therefore, the device is not properly initialized. The patchset contains 2 patches: 1. Export rte_eth_dev_attach_secondary, so non-pci drivers will be allowed to call it. 2. Fix the actual bug by calling the function during virtio_user probe. Ami Sabo (2): lib/librte_ether: export secondary attach function net/virtio-user: fix multi-process issue drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 7 +++++++ 4 files changed, 39 insertions(+), 13 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function 2017-02-26 9:55 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo @ 2017-02-26 9:55 ` Ami Sabo 2017-02-28 6:37 ` Yuanhan Liu ` (2 more replies) 2017-02-26 9:55 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo 1 sibling, 3 replies; 15+ messages in thread From: Ami Sabo @ 2017-02-26 9:55 UTC (permalink / raw) To: yuanhan.liu; +Cc: dev, stable, Ami Sabo Today eth_dev_attach_secondary is defined as static and can only be called by pci drivers. However, the functionality is also required for non-pci drivers - so the patch export the function. Signed-off-by: Ami Sabo <amis@radware.com> --- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 7 +++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index eb0a94a..86ee5bb 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -239,8 +239,8 @@ rte_eth_dev_allocate(const char *name) * makes sure that the same device would have the same port id both * in the primary and secondary process. */ -static struct rte_eth_dev * -eth_dev_attach_secondary(const char *name) +struct rte_eth_dev * +rte_eth_dev_attach_secondary(const char *name) { uint8_t i; struct rte_eth_dev *eth_dev; @@ -302,7 +302,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, if (eth_dev->data->dev_private == NULL) rte_panic("Cannot allocate memzone for private port data\n"); } else { - eth_dev = eth_dev_attach_secondary(ethdev_name); + eth_dev = rte_eth_dev_attach_secondary(ethdev_name); if (eth_dev == NULL) { /* * if we failed to attach a device, it means the diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 97f3e2d..9d5848b 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1762,6 +1762,19 @@ struct rte_eth_dev *rte_eth_dev_allocate(const char *name); /** * @internal + * Attach to the ethdev already initialized by the primary + * process. + * + * @param name Ethernet device's name. + @return + * - Success: Slot in the rte_dev_devices array for attached + * device. + * - Error: Null pointer. + */ +struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name); + +/** + * @internal * Release the specified ethdev port. * * @param eth_dev diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map index c6c9d0d..d34c57a 100644 --- a/lib/librte_ether/rte_ether_version.map +++ b/lib/librte_ether/rte_ether_version.map @@ -154,3 +154,10 @@ DPDK_17.02 { rte_flow_validate; } DPDK_16.11; + +DPDK_17.05 { + global: + + rte_eth_dev_attach_secondary; + +} DPDK_17.02; -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function 2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo @ 2017-02-28 6:37 ` Yuanhan Liu 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo 2 siblings, 0 replies; 15+ messages in thread From: Yuanhan Liu @ 2017-02-28 6:37 UTC (permalink / raw) To: Ami Sabo; +Cc: dev, stable, Thomas Monjalon Cc Thomas, the librte_ether maintainer. On Sun, Feb 26, 2017 at 11:55:25AM +0200, Ami Sabo wrote: > /** > * @internal > + * Attach to the ethdev already initialized by the primary > + * process. > + * > + * @param name Ethernet device's name. > + @return mailformed comment: missing *. > + * - Success: Slot in the rte_dev_devices array for attached > + * device. > + * - Error: Null pointer. > + */ > +struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name); > + > +/** > + * @internal > * Release the specified ethdev port. > * > * @param eth_dev > diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map > index c6c9d0d..d34c57a 100644 > --- a/lib/librte_ether/rte_ether_version.map > +++ b/lib/librte_ether/rte_ether_version.map > @@ -154,3 +154,10 @@ DPDK_17.02 { > rte_flow_validate; > > } DPDK_16.11; > + > +DPDK_17.05 { > + global: > + > + rte_eth_dev_attach_secondary; I saw whitespace chars. Like code, it should be TABs. Other than those minor nits, it looks good to me. Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> --yliu ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash. 2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo 2017-02-28 6:37 ` Yuanhan Liu @ 2017-03-02 7:51 ` Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 1/3] lib/librte_ether: export secondary attach function Ami Sabo ` (2 more replies) 2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo 2 siblings, 3 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 7:51 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo The patchset fixes secondary process crash issue when it tries to access virtio-user pmd (e.g. via rte_eth_rx_burst). The crash happens because the secondary process calls, at virtio_user_pmd_probe() to virtio_user_eth_dev_alloc()-> rte_eth_dev_allocate() instead of eth_dev_attach_secondary(), as it's done from rte_eth_dev_pci_probe. Therefore, the device is not properly initialized + the device data maybe overridden by the secondary process. The patchset contains 3 patches: 1. Export rte_eth_dev_attach_secondary, so non-pci drivers will be allowed to call it. 2. Fix the actual bug by calling the function during virtio_user probe. 3. Code style fixes following Yuanhan Lio's comments. Ami Sabo (3): lib/librte_ether: export secondary attach function net/virtio-user: fix multi-process issue lib/librte_ether: fix code style issues drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 7 +++++++ 4 files changed, 39 insertions(+), 13 deletions(-) -- v2: * Fix code style issues following Yuanhan Liu's review. -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH v2 1/3] lib/librte_ether: export secondary attach function 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo @ 2017-03-02 7:51 ` Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 2/3] net/virtio-user: fix multi-process issue Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues Ami Sabo 2 siblings, 0 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 7:51 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo Today eth_dev_attach_secondary is defined as static and can only be called by pci drivers. However, the functionality is also required for non-pci drivers - so the patch export the function. Signed-off-by: Ami Sabo <amis@radware.com> --- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 7 +++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index eb0a94a..86ee5bb 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -239,8 +239,8 @@ rte_eth_dev_allocate(const char *name) * makes sure that the same device would have the same port id both * in the primary and secondary process. */ -static struct rte_eth_dev * -eth_dev_attach_secondary(const char *name) +struct rte_eth_dev * +rte_eth_dev_attach_secondary(const char *name) { uint8_t i; struct rte_eth_dev *eth_dev; @@ -302,7 +302,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, if (eth_dev->data->dev_private == NULL) rte_panic("Cannot allocate memzone for private port data\n"); } else { - eth_dev = eth_dev_attach_secondary(ethdev_name); + eth_dev = rte_eth_dev_attach_secondary(ethdev_name); if (eth_dev == NULL) { /* * if we failed to attach a device, it means the diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 97f3e2d..9d5848b 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1762,6 +1762,19 @@ struct rte_eth_dev *rte_eth_dev_allocate(const char *name); /** * @internal + * Attach to the ethdev already initialized by the primary + * process. + * + * @param name Ethernet device's name. + @return + * - Success: Slot in the rte_dev_devices array for attached + * device. + * - Error: Null pointer. + */ +struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name); + +/** + * @internal * Release the specified ethdev port. * * @param eth_dev diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map index c6c9d0d..d34c57a 100644 --- a/lib/librte_ether/rte_ether_version.map +++ b/lib/librte_ether/rte_ether_version.map @@ -154,3 +154,10 @@ DPDK_17.02 { rte_flow_validate; } DPDK_16.11; + +DPDK_17.05 { + global: + + rte_eth_dev_attach_secondary; + +} DPDK_17.02; -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH v2 2/3] net/virtio-user: fix multi-process issue 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 1/3] lib/librte_ether: export secondary attach function Ami Sabo @ 2017-03-02 7:51 ` Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues Ami Sabo 2 siblings, 0 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 7:51 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo Secondary process doesn't properly attach to the rte_eth_device initialized by the primary process. ccessing device from secondary process (e.g. via rte_eth_rx_burst), causes process to crash. because rte_eth_dev_data is not properly set. 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 rte_eth_dev_pci_probe(). The fix is to call rte_eth_dev_attach_secondary(), for secondary process, from virtio_user_pmd_probe. Fixes: 7f95f78a8aea ("ethdev: clear data when allocating device") Cc: stable@dpdk.org Signed-off-by: Ami Sabo <amis@radware.com> --- drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 0b226ac..6033908 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -418,18 +418,24 @@ virtio_user_pmd_probe(const char *name, const char *params) goto end; } - eth_dev = virtio_user_eth_dev_alloc(name); - if (!eth_dev) { - PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); - goto end; - } + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + eth_dev = virtio_user_eth_dev_alloc(name); + if (!eth_dev) { + PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); + goto end; + } - hw = eth_dev->data->dev_private; - if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, + hw = eth_dev->data->dev_private; + if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, queue_size, mac_addr) < 0) { - PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); - virtio_user_eth_dev_free(eth_dev); - goto end; + PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); + virtio_user_eth_dev_free(eth_dev); + goto end; + } + } else { + eth_dev = rte_eth_dev_attach_secondary(name); + if (!eth_dev) + goto end; } /* previously called by rte_eal_pci_probe() for physical dev */ -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 1/3] lib/librte_ether: export secondary attach function Ami Sabo 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 2/3] net/virtio-user: fix multi-process issue Ami Sabo @ 2017-03-02 7:51 ` Ami Sabo 2017-03-02 8:12 ` Yuanhan Liu 2 siblings, 1 reply; 15+ messages in thread From: Ami Sabo @ 2017-03-02 7:51 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo Fixed code style issues following Yuanhan Liu's review. Signed-off-by: Ami Sabo <amis@radware.com> --- lib/librte_ether/rte_ethdev.h | 2 +- lib/librte_ether/rte_ether_version.map | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 9d5848b..6c9af46 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1766,7 +1766,7 @@ struct rte_eth_dev *rte_eth_dev_allocate(const char *name); * process. * * @param name Ethernet device's name. - @return + * @return * - Success: Slot in the rte_dev_devices array for attached * device. * - Error: Null pointer. diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map index d34c57a..28fb7d9 100644 --- a/lib/librte_ether/rte_ether_version.map +++ b/lib/librte_ether/rte_ether_version.map @@ -156,8 +156,8 @@ DPDK_17.02 { } DPDK_16.11; DPDK_17.05 { - global: + global: - rte_eth_dev_attach_secondary; + rte_eth_dev_attach_secondary; } DPDK_17.02; -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues Ami Sabo @ 2017-03-02 8:12 ` Yuanhan Liu 0 siblings, 0 replies; 15+ messages in thread From: Yuanhan Liu @ 2017-03-02 8:12 UTC (permalink / raw) To: Ami Sabo; +Cc: thomas.monjalon, dev, stable On Thu, Mar 02, 2017 at 09:51:24AM +0200, Ami Sabo wrote: > Fixed code style issues following Yuanhan Liu's review. Patch 1 is not merged yet. You should squash this fix into patch 1, instead of sending another patch. Meanwhile, when you do so, you should add version log, and put it at .....> > > Signed-off-by: Ami Sabo <amis@radware.com> > --- ... here (right below the ---). It could be something like: - v2: fix coding style issues (Yuanhan Liu) If you are still unsure, just find some examples from the mailing list. --yliu ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash. 2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo 2017-02-28 6:37 ` Yuanhan Liu 2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo @ 2017-03-02 9:00 ` Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo ` (2 more replies) 2 siblings, 3 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 9:00 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo The patchset fixes secondary process crash issue when it tries to access virtio-user pmd (e.g. via rte_eth_rx_burst). The crash happens because in virtio_user probing, eth_dev_attach_secondary is not being called, as it does from rte_eth_dev_pci_probe. Therefore, the device is not properly initialized. The patchset contains 2 patches: 1. Export rte_eth_dev_attach_secondary, so non-pci drivers will be allowed to call it. 2. Fix the actual bug by calling the function during virtio_user probe. Ami Sabo (2): lib/librte_ether: export secondary attach function net/virtio-user: fix multi-process issue drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 6 ++++++ 4 files changed, 38 insertions(+), 13 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function 2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo @ 2017-03-02 9:00 ` Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo 2017-04-14 12:13 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Thomas Monjalon 2 siblings, 0 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 9:00 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo Today eth_dev_attach_secondary is defined as static and can only be called by pci drivers. However, the functionality is also required for non-pci drivers - so the patch export the function. Signed-off-by: Ami Sabo <amis@radware.com> --- v2: Fix coding style issues (Yuanhan Liu) --- lib/librte_ether/rte_ethdev.c | 6 +++--- lib/librte_ether/rte_ethdev.h | 13 +++++++++++++ lib/librte_ether/rte_ether_version.map | 6 ++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index eb0a94a..86ee5bb 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -239,8 +239,8 @@ rte_eth_dev_allocate(const char *name) * makes sure that the same device would have the same port id both * in the primary and secondary process. */ -static struct rte_eth_dev * -eth_dev_attach_secondary(const char *name) +struct rte_eth_dev * +rte_eth_dev_attach_secondary(const char *name) { uint8_t i; struct rte_eth_dev *eth_dev; @@ -302,7 +302,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, if (eth_dev->data->dev_private == NULL) rte_panic("Cannot allocate memzone for private port data\n"); } else { - eth_dev = eth_dev_attach_secondary(ethdev_name); + eth_dev = rte_eth_dev_attach_secondary(ethdev_name); if (eth_dev == NULL) { /* * if we failed to attach a device, it means the diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 97f3e2d..6c9af46 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1762,6 +1762,19 @@ struct rte_eth_dev *rte_eth_dev_allocate(const char *name); /** * @internal + * Attach to the ethdev already initialized by the primary + * process. + * + * @param name Ethernet device's name. + * @return + * - Success: Slot in the rte_dev_devices array for attached + * device. + * - Error: Null pointer. + */ +struct rte_eth_dev *rte_eth_dev_attach_secondary(const char *name); + +/** + * @internal * Release the specified ethdev port. * * @param eth_dev diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map index c6c9d0d..85b0402 100644 --- a/lib/librte_ether/rte_ether_version.map +++ b/lib/librte_ether/rte_ether_version.map @@ -154,3 +154,9 @@ DPDK_17.02 { rte_flow_validate; } DPDK_16.11; + +DPDK_17.05 { + global: + + rte_eth_dev_attach_secondary; +} DPDK_17.02; -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue 2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo @ 2017-03-02 9:00 ` Ami Sabo 2017-04-14 12:13 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Thomas Monjalon 2 siblings, 0 replies; 15+ messages in thread From: Ami Sabo @ 2017-03-02 9:00 UTC (permalink / raw) To: yuanhan.liu, thomas.monjalon; +Cc: dev, stable, Ami Sabo Secondary process doesn't properly attach to the rte_eth_device initialized by the primary process. ccessing device from secondary process (e.g. via rte_eth_rx_burst), causes process to crash. because rte_eth_dev_data is not properly set. 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 rte_eth_dev_pci_probe(). The fix is to call rte_eth_dev_attach_secondary(), for secondary process, from virtio_user_pmd_probe. Fixes: 7f95f78a8aea ("ethdev: clear data when allocating device") Cc: stable@dpdk.org Signed-off-by: Ami Sabo <amis@radware.com> --- drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 0b226ac..6033908 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -418,18 +418,24 @@ virtio_user_pmd_probe(const char *name, const char *params) goto end; } - eth_dev = virtio_user_eth_dev_alloc(name); - if (!eth_dev) { - PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); - goto end; - } + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + eth_dev = virtio_user_eth_dev_alloc(name); + if (!eth_dev) { + PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); + goto end; + } - hw = eth_dev->data->dev_private; - if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, + hw = eth_dev->data->dev_private; + if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, queue_size, mac_addr) < 0) { - PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); - virtio_user_eth_dev_free(eth_dev); - goto end; + PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); + virtio_user_eth_dev_free(eth_dev); + goto end; + } + } else { + eth_dev = rte_eth_dev_attach_secondary(name); + if (!eth_dev) + goto end; } /* previously called by rte_eal_pci_probe() for physical dev */ -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash. 2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo 2017-03-02 9:00 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo @ 2017-04-14 12:13 ` Thomas Monjalon 2 siblings, 0 replies; 15+ messages in thread From: Thomas Monjalon @ 2017-04-14 12:13 UTC (permalink / raw) To: Ami Sabo; +Cc: yuanhan.liu, dev, stable 2017-03-02 11:00, Ami Sabo: > The patchset fixes secondary process crash issue when it tries > to access virtio-user pmd (e.g. via rte_eth_rx_burst). > > The crash happens because in virtio_user probing, > eth_dev_attach_secondary is not being called, as it does from > rte_eth_dev_pci_probe. Therefore, the device is not properly > initialized. > > The patchset contains 2 patches: > 1. Export rte_eth_dev_attach_secondary, so non-pci drivers will be > allowed to call it. > 2. Fix the actual bug by calling the function during virtio_user probe. > > Ami Sabo (2): > lib/librte_ether: export secondary attach function > net/virtio-user: fix multi-process issue Applied, thanks ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue 2017-02-26 9:55 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo 2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo @ 2017-02-26 9:55 ` Ami Sabo 2017-02-28 6:40 ` Yuanhan Liu 1 sibling, 1 reply; 15+ messages in thread From: Ami Sabo @ 2017-02-26 9:55 UTC (permalink / raw) To: yuanhan.liu; +Cc: dev, stable, Ami Sabo Secondary process doesn't properly attach to the rte_eth_device initialized by the primary process. ccessing device from secondary process (e.g. via rte_eth_rx_burst), causes process to crash. because rte_eth_dev_data is not properly set. 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 rte_eth_dev_pci_probe(). The fix is to call rte_eth_dev_attach_secondary(), for secondary process, from virtio_user_pmd_probe. Fixes: 7f95f78a8aea ("ethdev: clear data when allocating device") Cc: stable@dpdk.org Signed-off-by: Ami Sabo <amis@radware.com> --- drivers/net/virtio/virtio_user_ethdev.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 0b226ac..6033908 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -418,18 +418,24 @@ virtio_user_pmd_probe(const char *name, const char *params) goto end; } - eth_dev = virtio_user_eth_dev_alloc(name); - if (!eth_dev) { - PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); - goto end; - } + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + eth_dev = virtio_user_eth_dev_alloc(name); + if (!eth_dev) { + PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); + goto end; + } - hw = eth_dev->data->dev_private; - if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, + hw = eth_dev->data->dev_private; + if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, queue_size, mac_addr) < 0) { - PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); - virtio_user_eth_dev_free(eth_dev); - goto end; + PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); + virtio_user_eth_dev_free(eth_dev); + goto end; + } + } else { + eth_dev = rte_eth_dev_attach_secondary(name); + if (!eth_dev) + goto end; } /* previously called by rte_eal_pci_probe() for physical dev */ -- 2.7.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue 2017-02-26 9:55 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo @ 2017-02-28 6:40 ` Yuanhan Liu 2017-02-28 7:50 ` Ami Sabo 0 siblings, 1 reply; 15+ messages in thread From: Yuanhan Liu @ 2017-02-28 6:40 UTC (permalink / raw) To: Ami Sabo; +Cc: dev, stable On Sun, Feb 26, 2017 at 11:55:26AM +0200, Ami Sabo wrote: > Secondary process doesn't properly attach to the rte_eth_device > initialized by the primary process. > > ccessing device from secondary process (e.g. via rte_eth_rx_burst), > causes process to crash. because rte_eth_dev_data is not properly set. > > 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 > rte_eth_dev_pci_probe(). > > The fix is to call rte_eth_dev_attach_secondary(), for secondary > process, from virtio_user_pmd_probe. > > 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 built 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue 2017-02-28 6:40 ` Yuanhan Liu @ 2017-02-28 7:50 ` Ami Sabo 0 siblings, 0 replies; 15+ messages in thread From: Ami Sabo @ 2017-02-28 7:50 UTC (permalink / raw) To: Yuanhan Liu; +Cc: dev, stable Hi, You are right, the commit I mentioned didn't cause the issue - it just flooded 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 secondary processes...) --ami -----Original Message----- From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 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 > initialized by the primary process. > > ccessing device from secondary process (e.g. via rte_eth_rx_burst), > causes process to crash. because rte_eth_dev_data is not properly set. > > 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 > rte_eth_dev_pci_probe(). > > The fix is to call rte_eth_dev_attach_secondary(), for secondary > process, from virtio_user_pmd_probe. > > 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 built 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-04-14 12:13 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1487851096-32479-1-git-send-email-amis@radware.com>
2017-02-26 9:55 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo
2017-02-26 9:55 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo
2017-02-28 6:37 ` Yuanhan Liu
2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 0/3] Fix virtio-user multi-process crash Ami Sabo
2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 1/3] lib/librte_ether: export secondary attach function Ami Sabo
2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 2/3] net/virtio-user: fix multi-process issue Ami Sabo
2017-03-02 7:51 ` [dpdk-stable] [PATCH v2 3/3] lib/librte_ether: fix code style issues Ami Sabo
2017-03-02 8:12 ` Yuanhan Liu
2017-03-02 9:00 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Ami Sabo
2017-03-02 9:00 ` [dpdk-stable] [PATCH 1/2] lib/librte_ether: export secondary attach function Ami Sabo
2017-03-02 9:00 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo
2017-04-14 12:13 ` [dpdk-stable] [PATCH 0/2] Fix virtio-user multi-process crash Thomas Monjalon
2017-02-26 9:55 ` [dpdk-stable] [PATCH 2/2] net/virtio-user: fix multi-process issue Ami Sabo
2017-02-28 6:40 ` Yuanhan Liu
2017-02-28 7:50 ` Ami Sabo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).