* [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation [not found] <cover.1588705694.git.grive@u256.net> @ 2020-05-05 19:10 ` Gaetan Rivet 2020-05-06 11:48 ` Ferruh Yigit 0 siblings, 1 reply; 8+ messages in thread From: Gaetan Rivet @ 2020-05-05 19:10 UTC (permalink / raw) To: dev; +Cc: stable, ferruh.yigit, thomas When a net_ring device is allocated, its device pointer is not set before calling rte_eth_dev_probing_finish, which is incorrect. The following: commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") commit: a6992e961050 ("net/ring: set ethernet device field") already attempted to fix this issue in 17.08, which was fine at the time. Adding the hook rte_eth_dev_probing_finish() however created this bug, as the eth_dev exposed when this hook is executed is expected to be complete. Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and write the pointer properly in do_eth_dev_ring_create(). Cc: stable@dpdk.org Fixes: fbe90cdd776c ("ethdev: add probing finish function") Cc: ferruh.yigit@intel.com Cc: thomas@monjalon.net Signed-off-by: Gaetan Rivet <grive@u256.net> --- drivers/net/ring/rte_eth_ring.c | 36 ++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c index 41acbc513..ad27f9d89 100644 --- a/drivers/net/ring/rte_eth_ring.c +++ b/drivers/net/ring/rte_eth_ring.c @@ -244,6 +244,26 @@ static const struct eth_dev_ops ops = { .mac_addr_add = eth_mac_addr_add, }; +static int +dev_name_cmp(const struct rte_device *dev, const void *_name) +{ + const char *name = _name; + + return strcmp(dev->name, name); +} + +static struct rte_device * +ring_device_from_name(const char *name) +{ + struct rte_bus *vdev_bus; + + vdev_bus = rte_bus_find_by_name("vdev"); + if (vdev_bus == NULL) + return NULL; + + return vdev_bus->find_device(NULL, dev_name_cmp, name); +} + static int do_eth_dev_ring_create(const char *name, struct rte_ring * const rx_queues[], @@ -294,6 +314,7 @@ do_eth_dev_ring_create(const char *name, * - store queue data in internals, * - store numa_node info in eth_dev_data * - point eth_dev_data to internals + * - store EAL device in eth_dev, * - and point eth_dev structure to new eth_dev_data structure */ @@ -325,10 +346,17 @@ do_eth_dev_ring_create(const char *name, data->kdrv = RTE_KDRV_NONE; data->numa_node = numa_node; - /* finally assign rx and tx ops */ + /* assign rx and tx ops */ eth_dev->rx_pkt_burst = eth_ring_rx; eth_dev->tx_pkt_burst = eth_ring_tx; + /* finally set the rte_device pointer in eth_dev. */ + eth_dev->device = ring_device_from_name(name); + if (eth_dev->device == NULL) { + rte_errno = ENODEV; + goto error; + } + rte_eth_dev_probing_finish(eth_dev); *eth_dev_p = eth_dev; @@ -584,9 +612,6 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) DEV_ATTACH, ð_dev); } - if (eth_dev) - eth_dev->device = &dev->device; - return ret; } @@ -644,9 +669,6 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) } } - if (eth_dev) - eth_dev->device = &dev->device; - out_free: rte_kvargs_free(kvlist); rte_free(info); -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation 2020-05-05 19:10 ` [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation Gaetan Rivet @ 2020-05-06 11:48 ` Ferruh Yigit 2020-05-06 12:33 ` Gaëtan Rivet 0 siblings, 1 reply; 8+ messages in thread From: Ferruh Yigit @ 2020-05-06 11:48 UTC (permalink / raw) To: Gaetan Rivet, dev; +Cc: stable, thomas On 5/5/2020 8:10 PM, Gaetan Rivet wrote: > When a net_ring device is allocated, its device pointer is not set > before calling rte_eth_dev_probing_finish, which is incorrect. > > The following: > commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") > commit: a6992e961050 ("net/ring: set ethernet device field") > > already attempted to fix this issue in 17.08, which was fine at the > time. Adding the hook rte_eth_dev_probing_finish() however created this > bug, as the eth_dev exposed when this hook is executed is expected to be > complete. > > Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and > write the pointer properly in do_eth_dev_ring_create(). > > Cc: stable@dpdk.org > Fixes: fbe90cdd776c ("ethdev: add probing finish function") > Cc: ferruh.yigit@intel.com > Cc: thomas@monjalon.net > Signed-off-by: Gaetan Rivet <grive@u256.net> <...> > @@ -325,10 +346,17 @@ do_eth_dev_ring_create(const char *name, > data->kdrv = RTE_KDRV_NONE; > data->numa_node = numa_node; > > - /* finally assign rx and tx ops */ > + /* assign rx and tx ops */ > eth_dev->rx_pkt_burst = eth_ring_rx; > eth_dev->tx_pkt_burst = eth_ring_tx; > > + /* finally set the rte_device pointer in eth_dev. */ > + eth_dev->device = ring_device_from_name(name); > + if (eth_dev->device == NULL) { > + rte_errno = ENODEV; > + goto error; > + } > + > rte_eth_dev_probing_finish(eth_dev); > *eth_dev_p = eth_dev; Why not move the 'rte_eth_dev_probing_finish()' to the 'rte_pmd_ring_probe()', below where 'eth_dev->device' set? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation 2020-05-06 11:48 ` Ferruh Yigit @ 2020-05-06 12:33 ` Gaëtan Rivet 2020-05-06 13:43 ` Ferruh Yigit 0 siblings, 1 reply; 8+ messages in thread From: Gaëtan Rivet @ 2020-05-06 12:33 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev, stable, thomas On 06/05/20 12:48 +0100, Ferruh Yigit wrote: > On 5/5/2020 8:10 PM, Gaetan Rivet wrote: > > When a net_ring device is allocated, its device pointer is not set > > before calling rte_eth_dev_probing_finish, which is incorrect. > > > > The following: > > commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") > > commit: a6992e961050 ("net/ring: set ethernet device field") > > > > already attempted to fix this issue in 17.08, which was fine at the > > time. Adding the hook rte_eth_dev_probing_finish() however created this > > bug, as the eth_dev exposed when this hook is executed is expected to be > > complete. > > > > Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and > > write the pointer properly in do_eth_dev_ring_create(). > > > > Cc: stable@dpdk.org > > Fixes: fbe90cdd776c ("ethdev: add probing finish function") > > Cc: ferruh.yigit@intel.com > > Cc: thomas@monjalon.net > > Signed-off-by: Gaetan Rivet <grive@u256.net> > > <...> > > > @@ -325,10 +346,17 @@ do_eth_dev_ring_create(const char *name, > > data->kdrv = RTE_KDRV_NONE; > > data->numa_node = numa_node; > > > > - /* finally assign rx and tx ops */ > > + /* assign rx and tx ops */ > > eth_dev->rx_pkt_burst = eth_ring_rx; > > eth_dev->tx_pkt_burst = eth_ring_tx; > > > > + /* finally set the rte_device pointer in eth_dev. */ > > + eth_dev->device = ring_device_from_name(name); > > + if (eth_dev->device == NULL) { > > + rte_errno = ENODEV; > > + goto error; > > + } > > + > > rte_eth_dev_probing_finish(eth_dev); > > *eth_dev_p = eth_dev; > > Why not move the 'rte_eth_dev_probing_finish()' to the 'rte_pmd_ring_probe()', > below where 'eth_dev->device' set? Hi Ferruh, Sure it would work. The reason I did it this way is two-fold: * I disliked having two places where eth_dev->device was conditionally set. It makes it harder to read rte_pmd_ring_probe. * I was actually thinking, doing this patch, that we should modify rte_eth_dev_allocate() to take an rte_device as parameter, as all eth_dev are meant to be backed by an rte_device. Keeping this in mind, I meant to move writing the pointer closer to the rte_eth_dev_allocate() call. But you are right that it is needlessly verbose, using vdev_bus->find_device() to do this stuff. I'm ok with changing it as you described if you prefer. -- Gaëtan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation 2020-05-06 12:33 ` Gaëtan Rivet @ 2020-05-06 13:43 ` Ferruh Yigit 2020-05-06 17:32 ` Gaëtan Rivet 0 siblings, 1 reply; 8+ messages in thread From: Ferruh Yigit @ 2020-05-06 13:43 UTC (permalink / raw) To: Gaëtan Rivet; +Cc: dev, stable, thomas On 5/6/2020 1:33 PM, Gaëtan Rivet wrote: > On 06/05/20 12:48 +0100, Ferruh Yigit wrote: >> On 5/5/2020 8:10 PM, Gaetan Rivet wrote: >>> When a net_ring device is allocated, its device pointer is not set >>> before calling rte_eth_dev_probing_finish, which is incorrect. >>> >>> The following: >>> commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") >>> commit: a6992e961050 ("net/ring: set ethernet device field") >>> >>> already attempted to fix this issue in 17.08, which was fine at the >>> time. Adding the hook rte_eth_dev_probing_finish() however created this >>> bug, as the eth_dev exposed when this hook is executed is expected to be >>> complete. >>> >>> Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and >>> write the pointer properly in do_eth_dev_ring_create(). >>> >>> Cc: stable@dpdk.org >>> Fixes: fbe90cdd776c ("ethdev: add probing finish function") >>> Cc: ferruh.yigit@intel.com >>> Cc: thomas@monjalon.net >>> Signed-off-by: Gaetan Rivet <grive@u256.net> >> >> <...> >> >>> @@ -325,10 +346,17 @@ do_eth_dev_ring_create(const char *name, >>> data->kdrv = RTE_KDRV_NONE; >>> data->numa_node = numa_node; >>> >>> - /* finally assign rx and tx ops */ >>> + /* assign rx and tx ops */ >>> eth_dev->rx_pkt_burst = eth_ring_rx; >>> eth_dev->tx_pkt_burst = eth_ring_tx; >>> >>> + /* finally set the rte_device pointer in eth_dev. */ >>> + eth_dev->device = ring_device_from_name(name); >>> + if (eth_dev->device == NULL) { >>> + rte_errno = ENODEV; >>> + goto error; >>> + } >>> + >>> rte_eth_dev_probing_finish(eth_dev); >>> *eth_dev_p = eth_dev; >> >> Why not move the 'rte_eth_dev_probing_finish()' to the 'rte_pmd_ring_probe()', >> below where 'eth_dev->device' set? > > Hi Ferruh, > > Sure it would work. The reason I did it this way is two-fold: > > * I disliked having two places where eth_dev->device was conditionally > set. It makes it harder to read rte_pmd_ring_probe. Agree, what about using a 'goto' to have the assignment and 'rte_eth_dev_probing_finish()' in a single place? But check seems needed since creation may failed at that stage, if you think better check can be done on 'ret' instead of 'eth_dev'... > > * I was actually thinking, doing this patch, that we should modify > rte_eth_dev_allocate() to take an rte_device as parameter, as all > eth_dev are meant to be backed by an rte_device. Keeping this in > mind, I meant to move writing the pointer closer to the > rte_eth_dev_allocate() call. That is a bigger change, may affect many (if not all) PMDs, so I think this can be considered when that change is available. And although that change may fix the issues that 'eth_dev->device' is not set, which we had a few times before, not sure it worth to change all PMDs and ethdev API directly couple with rte_device, instead of PMD being the glue. Can be discussed more on its own patch. > > But you are right that it is needlessly verbose, using > vdev_bus->find_device() to do this stuff. I'm ok with changing it as you > described if you prefer. > That was the concern, that is too much code to take a value which is already available a few level up the stack. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation 2020-05-06 13:43 ` Ferruh Yigit @ 2020-05-06 17:32 ` Gaëtan Rivet 2020-05-06 18:09 ` [dpdk-stable] [PATCH v2] " Gaetan Rivet 0 siblings, 1 reply; 8+ messages in thread From: Gaëtan Rivet @ 2020-05-06 17:32 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev, stable, thomas On 06/05/20 14:43 +0100, Ferruh Yigit wrote: > On 5/6/2020 1:33 PM, Gaëtan Rivet wrote: > > On 06/05/20 12:48 +0100, Ferruh Yigit wrote: > >> On 5/5/2020 8:10 PM, Gaetan Rivet wrote: > >>> When a net_ring device is allocated, its device pointer is not set > >>> before calling rte_eth_dev_probing_finish, which is incorrect. > >>> > >>> The following: > >>> commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") > >>> commit: a6992e961050 ("net/ring: set ethernet device field") > >>> > >>> already attempted to fix this issue in 17.08, which was fine at the > >>> time. Adding the hook rte_eth_dev_probing_finish() however created this > >>> bug, as the eth_dev exposed when this hook is executed is expected to be > >>> complete. > >>> > >>> Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and > >>> write the pointer properly in do_eth_dev_ring_create(). > >>> > >>> Cc: stable@dpdk.org > >>> Fixes: fbe90cdd776c ("ethdev: add probing finish function") > >>> Cc: ferruh.yigit@intel.com > >>> Cc: thomas@monjalon.net > >>> Signed-off-by: Gaetan Rivet <grive@u256.net> > >> > >> <...> > >> > >>> @@ -325,10 +346,17 @@ do_eth_dev_ring_create(const char *name, > >>> data->kdrv = RTE_KDRV_NONE; > >>> data->numa_node = numa_node; > >>> > >>> - /* finally assign rx and tx ops */ > >>> + /* assign rx and tx ops */ > >>> eth_dev->rx_pkt_burst = eth_ring_rx; > >>> eth_dev->tx_pkt_burst = eth_ring_tx; > >>> > >>> + /* finally set the rte_device pointer in eth_dev. */ > >>> + eth_dev->device = ring_device_from_name(name); > >>> + if (eth_dev->device == NULL) { > >>> + rte_errno = ENODEV; > >>> + goto error; > >>> + } > >>> + > >>> rte_eth_dev_probing_finish(eth_dev); > >>> *eth_dev_p = eth_dev; > >> > >> Why not move the 'rte_eth_dev_probing_finish()' to the 'rte_pmd_ring_probe()', > >> below where 'eth_dev->device' set? > > > > Hi Ferruh, > > > > Sure it would work. The reason I did it this way is two-fold: > > > > * I disliked having two places where eth_dev->device was conditionally > > set. It makes it harder to read rte_pmd_ring_probe. > > Agree, what about using a 'goto' to have the assignment and > 'rte_eth_dev_probing_finish()' in a single place? > But check seems needed since creation may failed at that stage, if you think > better check can be done on 'ret' instead of 'eth_dev'... > > > > > * I was actually thinking, doing this patch, that we should modify > > rte_eth_dev_allocate() to take an rte_device as parameter, as all > > eth_dev are meant to be backed by an rte_device. Keeping this in > > mind, I meant to move writing the pointer closer to the > > rte_eth_dev_allocate() call. > > That is a bigger change, may affect many (if not all) PMDs, so I think this can > be considered when that change is available. > > And although that change may fix the issues that 'eth_dev->device' is not set, > which we had a few times before, not sure it worth to change all PMDs and ethdev > API directly couple with rte_device, instead of PMD being the glue. Can be > discussed more on its own patch. > > > > > But you are right that it is needlessly verbose, using > > vdev_bus->find_device() to do this stuff. I'm ok with changing it as you > > described if you prefer. > > > > That was the concern, that is too much code to take a value which is already > available a few level up the stack. Ok, future-proofing is a bad habit so let's not do it. I'm not a fan of the goto for the 'happy path' though. Another simple way would be to bring the vdev pointer with me as we go down the stack. I will send a v2 shortly, if it's too ugly to move the pointer down I'll use the goto, or if you tell me you'd prefer it. -- Gaëtan ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v2] net/ring: fix eth_dev device pointer on allocation 2020-05-06 17:32 ` Gaëtan Rivet @ 2020-05-06 18:09 ` Gaetan Rivet 2020-05-08 11:00 ` Ferruh Yigit 0 siblings, 1 reply; 8+ messages in thread From: Gaetan Rivet @ 2020-05-06 18:09 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, stable When a net_ring device is allocated, its device pointer is not set before calling rte_eth_dev_probing_finish, which is incorrect. The following: commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") commit: a6992e961050 ("net/ring: set ethernet device field") already fixed the same issue in 17.08, which was fine at the time. Adding the hook rte_eth_dev_probing_finish() however created this bug, as the eth_dev exposed when this hook is executed is expected to be complete. Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and write the pointer properly in do_eth_dev_ring_create(). Cc: stable@dpdk.org Fixes: fbe90cdd776c ("ethdev: add probing finish function") Signed-off-by: Gaetan Rivet <grive@u256.net> --- drivers/net/ring/rte_eth_ring.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c index 41acbc513..f0fafa0c0 100644 --- a/drivers/net/ring/rte_eth_ring.c +++ b/drivers/net/ring/rte_eth_ring.c @@ -246,6 +246,7 @@ static const struct eth_dev_ops ops = { static int do_eth_dev_ring_create(const char *name, + struct rte_vdev_device *vdev, struct rte_ring * const rx_queues[], const unsigned int nb_rx_queues, struct rte_ring *const tx_queues[], @@ -291,12 +292,15 @@ do_eth_dev_ring_create(const char *name, } /* now put it all together + * - store EAL device in eth_dev, * - store queue data in internals, * - store numa_node info in eth_dev_data * - point eth_dev_data to internals * - and point eth_dev structure to new eth_dev_data structure */ + eth_dev->device = &vdev->device; + data = eth_dev->data; data->rx_queues = rx_queues_local; data->tx_queues = tx_queues_local; @@ -408,7 +412,9 @@ rte_eth_from_ring(struct rte_ring *r) } static int -eth_dev_ring_create(const char *name, const unsigned int numa_node, +eth_dev_ring_create(const char *name, + struct rte_vdev_device *vdev, + const unsigned int numa_node, enum dev_action action, struct rte_eth_dev **eth_dev) { /* rx and tx are so-called from point of view of first port. @@ -438,7 +444,7 @@ eth_dev_ring_create(const char *name, const unsigned int numa_node, return -1; } - if (do_eth_dev_ring_create(name, rxtx, num_rings, rxtx, num_rings, + if (do_eth_dev_ring_create(name, vdev, rxtx, num_rings, rxtx, num_rings, numa_node, action, eth_dev) < 0) return -1; @@ -560,12 +566,12 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) PMD_LOG(INFO, "Initializing pmd_ring for %s", name); if (params == NULL || params[0] == '\0') { - ret = eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE, + ret = eth_dev_ring_create(name, dev, rte_socket_id(), DEV_CREATE, ð_dev); if (ret == -1) { PMD_LOG(INFO, "Attach to pmd_ring for %s", name); - ret = eth_dev_ring_create(name, rte_socket_id(), + ret = eth_dev_ring_create(name, dev, rte_socket_id(), DEV_ATTACH, ð_dev); } } else { @@ -574,19 +580,16 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) if (!kvlist) { PMD_LOG(INFO, "Ignoring unsupported parameters when creatingrings-backed ethernet device"); - ret = eth_dev_ring_create(name, rte_socket_id(), + ret = eth_dev_ring_create(name, dev, rte_socket_id(), DEV_CREATE, ð_dev); if (ret == -1) { PMD_LOG(INFO, "Attach to pmd_ring for %s", name); - ret = eth_dev_ring_create(name, rte_socket_id(), + ret = eth_dev_ring_create(name, dev, rte_socket_id(), DEV_ATTACH, ð_dev); } - if (eth_dev) - eth_dev->device = &dev->device; - return ret; } @@ -597,7 +600,7 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) if (ret < 0) goto out_free; - ret = do_eth_dev_ring_create(name, + ret = do_eth_dev_ring_create(name, dev, internal_args->rx_queues, internal_args->nb_rx_queues, internal_args->tx_queues, @@ -627,6 +630,7 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) for (info->count = 0; info->count < info->total; info->count++) { ret = eth_dev_ring_create(info->list[info->count].name, + dev, info->list[info->count].node, info->list[info->count].action, ð_dev); @@ -635,7 +639,7 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) PMD_LOG(INFO, "Attach to pmd_ring for %s", name); - ret = eth_dev_ring_create(name, + ret = eth_dev_ring_create(name, dev, info->list[info->count].node, DEV_ATTACH, ð_dev); @@ -644,9 +648,6 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev) } } - if (eth_dev) - eth_dev->device = &dev->device; - out_free: rte_kvargs_free(kvlist); rte_free(info); -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v2] net/ring: fix eth_dev device pointer on allocation 2020-05-06 18:09 ` [dpdk-stable] [PATCH v2] " Gaetan Rivet @ 2020-05-08 11:00 ` Ferruh Yigit 2020-05-11 16:54 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit 0 siblings, 1 reply; 8+ messages in thread From: Ferruh Yigit @ 2020-05-08 11:00 UTC (permalink / raw) To: Gaetan Rivet; +Cc: dev, stable, Bruce Richardson On 5/6/2020 7:09 PM, Gaetan Rivet wrote: > When a net_ring device is allocated, its device pointer is not set > before calling rte_eth_dev_probing_finish, which is incorrect. > > The following: > commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") > commit: a6992e961050 ("net/ring: set ethernet device field") > > already fixed the same issue in 17.08, which was fine at the time. > Adding the hook rte_eth_dev_probing_finish() however created this bug, > as the eth_dev exposed when this hook is executed is expected to be > complete. > > Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and > write the pointer properly in do_eth_dev_ring_create(). > > Cc: stable@dpdk.org > Fixes: fbe90cdd776c ("ethdev: add probing finish function") > Signed-off-by: Gaetan Rivet <grive@u256.net> I would prefer moving the assignment up in the stack where 'device' is available, instead of moving the variable down in the stack to assign it, but both does the work ... Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/ring: fix eth_dev device pointer on allocation 2020-05-08 11:00 ` Ferruh Yigit @ 2020-05-11 16:54 ` Ferruh Yigit 0 siblings, 0 replies; 8+ messages in thread From: Ferruh Yigit @ 2020-05-11 16:54 UTC (permalink / raw) To: Gaetan Rivet; +Cc: dev, stable, Bruce Richardson On 5/8/2020 12:00 PM, Ferruh Yigit wrote: > On 5/6/2020 7:09 PM, Gaetan Rivet wrote: >> When a net_ring device is allocated, its device pointer is not set >> before calling rte_eth_dev_probing_finish, which is incorrect. >> >> The following: >> commit: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API") >> commit: a6992e961050 ("net/ring: set ethernet device field") >> >> already fixed the same issue in 17.08, which was fine at the time. >> Adding the hook rte_eth_dev_probing_finish() however created this bug, >> as the eth_dev exposed when this hook is executed is expected to be >> complete. >> >> Remove the prior attempts to fix the issue in rte_pmd_ring_probe() and >> write the pointer properly in do_eth_dev_ring_create(). >> >> Cc: stable@dpdk.org >> Fixes: fbe90cdd776c ("ethdev: add probing finish function") >> Signed-off-by: Gaetan Rivet <grive@u256.net> > > I would prefer moving the assignment up in the stack where 'device' is > available, instead of moving the variable down in the stack to assign it, but > both does the work ... > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> > Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-11 16:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1588705694.git.grive@u256.net> 2020-05-05 19:10 ` [dpdk-stable] [PATCH v1 2/3] net/ring: fix eth_dev device pointer on allocation Gaetan Rivet 2020-05-06 11:48 ` Ferruh Yigit 2020-05-06 12:33 ` Gaëtan Rivet 2020-05-06 13:43 ` Ferruh Yigit 2020-05-06 17:32 ` Gaëtan Rivet 2020-05-06 18:09 ` [dpdk-stable] [PATCH v2] " Gaetan Rivet 2020-05-08 11:00 ` Ferruh Yigit 2020-05-11 16:54 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
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).