* [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs
@ 2020-07-09 15:20 Bruce Richardson
2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn Bruce Richardson
` (6 more replies)
0 siblings, 7 replies; 30+ messages in thread
From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw)
To: Nipun Gupta, Hemant Agrawal
Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao,
Mahipal Challa, Jerin Jacob, Bruce Richardson
This patchset proposes some internal and externally-visible changes to the
rawdev API. If consensus is in favour, I will submit a deprecation notice
for the changes for the 20.08 release, so that these ABI/API-breaking
changes can be merged in 20.11
The changes are in two areas:
* For any APIs which take a void * parameter for driver-specific structs,
add an additional parameter to provide the struct length. This allows
some runtime type-checking, as well as possible ABI-compatibility support
in the future as structure change generally involve a change in the size
of the structure.
* Ensure all APIs which can return error values have int type, rather than
void. Since functions like info_get and queue_default_get can now do some
typechecking, they need to be modified to allow them to return error
codes on failure.
Bruce Richardson (5):
rawdev: add private data length parameter to info fn
rawdev: allow drivers to return error from info function
rawdev: add private data length parameter to config fn
rawdev: add private data length parameter to queue fns
rawdev: allow queue default config query to return error
drivers/bus/ifpga/ifpga_bus.c | 2 +-
drivers/raw/ifpga/ifpga_rawdev.c | 23 +++++-----
drivers/raw/ioat/ioat_rawdev.c | 17 ++++---
drivers/raw/ioat/ioat_rawdev_test.c | 6 +--
drivers/raw/ntb/ntb.c | 49 ++++++++++++++++-----
drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 +--
drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 +-
drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 +--
drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +-
drivers/raw/skeleton/skeleton_rawdev.c | 34 ++++++++------
drivers/raw/skeleton/skeleton_rawdev_test.c | 32 ++++++++------
examples/ioat/ioatfwd.c | 4 +-
examples/ntb/ntb_fwd.c | 7 +--
lib/librte_rawdev/rte_rawdev.c | 27 +++++++-----
lib/librte_rawdev/rte_rawdev.h | 27 ++++++++++--
lib/librte_rawdev/rte_rawdev_pmd.h | 22 ++++++---
16 files changed, 178 insertions(+), 91 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson @ 2020-07-09 15:20 ` Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function Bruce Richardson ` (5 subsequent siblings) 6 siblings, 1 reply; 30+ messages in thread From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao, Mahipal Challa, Jerin Jacob, Bruce Richardson Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the dev_info structure is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/bus/ifpga/ifpga_bus.c | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 4 ++-- drivers/raw/ntb/ntb.c | 8 +++++++- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 19 ++++++++++++------- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 9 ++++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 5 ++++- 12 files changed, 49 insertions(+), 23 deletions(-) diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c index 6b16a20bb..bb8b3dcfb 100644 --- a/drivers/bus/ifpga/ifpga_bus.c +++ b/drivers/bus/ifpga/ifpga_bus.c @@ -162,7 +162,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev, afu_dev->id.port = afu_pr_conf.afu_id.port; if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get) - rawdev->dev_ops->dev_info_get(rawdev, afu_dev); + rawdev->dev_ops->dev_info_get(rawdev, afu_dev, sizeof(*afu_dev)); if (rawdev->dev_ops && rawdev->dev_ops->dev_start && diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index cc25c662b..47cfa3877 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -605,7 +605,8 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, static void ifpga_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct opae_adapter *adapter; struct opae_accelerator *acc; @@ -617,7 +618,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, IFPGA_RAWDEV_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index f876ffc3f..8dd856c55 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -113,12 +113,13 @@ ioat_dev_stop(struct rte_rawdev *dev) } static void -ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL) + if (cfg != NULL && dev_info_size == sizeof(*cfg)) cfg->ring_size = ioat->ring_size; } diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index d99f1bd6b..90f5974cd 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -157,7 +157,7 @@ ioat_rawdev_test(uint16_t dev_id) return TEST_SKIPPED; } - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != expected_ring_size[dev_id]) { printf("Error, initial ring size is not as expected (Actual: %d, Expected: %d)\n", (int)p.ring_size, expected_ring_size[dev_id]); @@ -169,7 +169,7 @@ ioat_rawdev_test(uint16_t dev_id) printf("Error with rte_rawdev_configure()\n"); return -1; } - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != IOAT_TEST_RINGSIZE) { printf("Error, ring size is not %d (%d)\n", IOAT_TEST_RINGSIZE, (int)p.ring_size); diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index e40412bb7..4676c6f8f 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -801,11 +801,17 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, } static void -ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct ntb_hw *hw = dev->dev_private; struct ntb_dev_info *info = dev_info; + if (dev_info_size != sizeof(*info)){ + NTB_LOG(ERR, "Invalid size parameter to %s", __func__); + return; + } + info->mw_cnt = hw->mw_cnt; info->mw_size = hw->mw_size; diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 72ece887a..dc05f3ecf 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -42,14 +42,15 @@ static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); static void skeleton_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; SKELETON_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9ecfdee81..9b8390dfb 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -106,12 +106,12 @@ test_rawdev_info_get(void) struct rte_rawdev_info rdev_info = {0}; struct skeleton_rawdev_conf skel_conf = {0}; - ret = rte_rawdev_info_get(test_dev_id, NULL); + ret = rte_rawdev_info_get(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); rdev_info.dev_private = &skel_conf; - ret = rte_rawdev_info_get(test_dev_id, &rdev_info); + ret = rte_rawdev_info_get(test_dev_id, &rdev_info, sizeof(skel_conf)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get raw dev info"); return TEST_SUCCESS; @@ -142,7 +142,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -170,7 +171,8 @@ test_rawdev_queue_default_conf_get(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -218,7 +220,8 @@ test_rawdev_queue_setup(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -327,7 +330,8 @@ test_rawdev_start_stop(void) dummy_firmware = NULL; rte_rawdev_start(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -336,7 +340,8 @@ test_rawdev_start_stop(void) rdev_conf_get.device_state); rte_rawdev_stop(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index b66ee73bc..5c631da1b 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -757,7 +757,7 @@ assign_rawdevs(void) do { if (rdev_id == rte_rawdev_count()) goto end; - rte_rawdev_info_get(rdev_id++, &rdev_info); + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); } while (rdev_info.driver_name == NULL || strcmp(rdev_info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) != 0); diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index eba8ebf9f..11e224451 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1389,7 +1389,7 @@ main(int argc, char **argv) rte_rawdev_set_attr(dev_id, NTB_QUEUE_NUM_NAME, num_queues); printf("Set queue number as %u.\n", num_queues); ntb_rawdev_info.dev_private = (rte_rawdev_obj_t)(&ntb_info); - rte_rawdev_info_get(dev_id, &ntb_rawdev_info); + rte_rawdev_info_get(dev_id, &ntb_rawdev_info, sizeof(ntb_info)); nb_mbuf = nb_desc * num_queues * 2 * 2 + rte_lcore_count() * MEMPOOL_CACHE_SIZE; diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 8f84d0b22..a57689035 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -78,7 +78,8 @@ rte_rawdev_socket_id(uint16_t dev_id) } int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size) { struct rte_rawdev *rawdev; @@ -89,7 +90,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private); + (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + dev_private_size); } dev_info->driver_name = rawdev->driver_name; diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 32f6b8bb0..cf6acfd26 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -82,13 +82,20 @@ struct rte_rawdev_info; * will be returned. This can be used to safely query the type of a rawdev * instance without needing to know the size of the private data to return. * + * @param dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * returned, and may be checked by drivers to ensure the expected struct + * type is provided. + * * @return * - 0: Success, driver updates the contextual information of the raw device * - <0: Error code returned by the driver info get function. * */ int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info); +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size); /** * Configure a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 4395a2182..0e72a9205 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -138,12 +138,15 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * Raw device pointer * @param dev_info * Raw device information structure + * @param dev_private_size + * The size of the structure pointed to by dev_info->dev_private * * @return * Returns 0 on success */ typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info); + rte_rawdev_obj_t dev_info, + size_t dev_private_size); /** * Configure a device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn Bruce Richardson @ 2020-07-12 14:13 ` Xu, Rosen 0 siblings, 0 replies; 30+ messages in thread From: Xu, Rosen @ 2020-07-12 14:13 UTC (permalink / raw) To: Richardson, Bruce, Nipun Gupta, Hemant Agrawal Cc: dev, Zhang, Tianfei, Li, Xiaoyun, Wu, Jingjing, Satha Rao, Mahipal Challa, Jerin Jacob Hi, Reviewed-by: Rosen Xu <rosen.xu@intel.com> > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, July 09, 2020 23:21 > To: Nipun Gupta <nipun.gupta@nxp.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Cc: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei > <tianfei.zhang@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; Satha Rao <skoteshwar@marvell.com>; Mahipal > Challa <mchalla@marvell.com>; Jerin Jacob <jerinj@marvell.com>; > Richardson, Bruce <bruce.richardson@intel.com> > Subject: [PATCH 20.11 1/5] rawdev: add private data length parameter to info > fn > > Currently with the rawdev API there is no way to check that the structure > passed in via the dev_private pointer in the dev_info structure is of the > correct type - it's just checked that it is non-NULL. Adding in the length of the > expected structure provides a measure of typechecking, and can also be > used for ABI compatibility in future, since ABI changes involving structs > almost always involve a change in size. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/bus/ifpga/ifpga_bus.c | 2 +- > drivers/raw/ifpga/ifpga_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev_test.c | 4 ++-- > drivers/raw/ntb/ntb.c | 8 +++++++- > drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- > drivers/raw/skeleton/skeleton_rawdev_test.c | 19 ++++++++++++------- > examples/ioat/ioatfwd.c | 2 +- > examples/ntb/ntb_fwd.c | 2 +- > lib/librte_rawdev/rte_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev.h | 9 ++++++++- > lib/librte_rawdev/rte_rawdev_pmd.h | 5 ++++- > 12 files changed, 49 insertions(+), 23 deletions(-) > > diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c > index 6b16a20bb..bb8b3dcfb 100644 > --- a/drivers/bus/ifpga/ifpga_bus.c > +++ b/drivers/bus/ifpga/ifpga_bus.c > @@ -162,7 +162,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev, > afu_dev->id.port = afu_pr_conf.afu_id.port; > > if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get) > - rawdev->dev_ops->dev_info_get(rawdev, afu_dev); > + rawdev->dev_ops->dev_info_get(rawdev, afu_dev, > sizeof(*afu_dev)); > > if (rawdev->dev_ops && > rawdev->dev_ops->dev_start && > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > b/drivers/raw/ifpga/ifpga_rawdev.c > index cc25c662b..47cfa3877 100644 > --- a/drivers/raw/ifpga/ifpga_rawdev.c > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > @@ -605,7 +605,8 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, > > static void > ifpga_rawdev_info_get(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info) > + rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct opae_adapter *adapter; > struct opae_accelerator *acc; > @@ -617,7 +618,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > > IFPGA_RAWDEV_PMD_FUNC_TRACE(); > > - if (!dev_info) { > + if (!dev_info || dev_info_size != sizeof(*afu_dev)) { > IFPGA_RAWDEV_PMD_ERR("Invalid request"); > return; > } > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index f876ffc3f..8dd856c55 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -113,12 +113,13 @@ ioat_dev_stop(struct rte_rawdev *dev) } > > static void > -ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) > +ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct rte_ioat_rawdev_config *cfg = dev_info; > struct rte_ioat_rawdev *ioat = dev->dev_private; > > - if (cfg != NULL) > + if (cfg != NULL && dev_info_size == sizeof(*cfg)) > cfg->ring_size = ioat->ring_size; > } > > diff --git a/drivers/raw/ioat/ioat_rawdev_test.c > b/drivers/raw/ioat/ioat_rawdev_test.c > index d99f1bd6b..90f5974cd 100644 > --- a/drivers/raw/ioat/ioat_rawdev_test.c > +++ b/drivers/raw/ioat/ioat_rawdev_test.c > @@ -157,7 +157,7 @@ ioat_rawdev_test(uint16_t dev_id) > return TEST_SKIPPED; > } > > - rte_rawdev_info_get(dev_id, &info); > + rte_rawdev_info_get(dev_id, &info, sizeof(p)); > if (p.ring_size != expected_ring_size[dev_id]) { > printf("Error, initial ring size is not as expected (Actual: %d, > Expected: %d)\n", > (int)p.ring_size, expected_ring_size[dev_id]); > @@ -169,7 +169,7 @@ ioat_rawdev_test(uint16_t dev_id) > printf("Error with rte_rawdev_configure()\n"); > return -1; > } > - rte_rawdev_info_get(dev_id, &info); > + rte_rawdev_info_get(dev_id, &info, sizeof(p)); > if (p.ring_size != IOAT_TEST_RINGSIZE) { > printf("Error, ring size is not %d (%d)\n", > IOAT_TEST_RINGSIZE, (int)p.ring_size); diff -- > git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index > e40412bb7..4676c6f8f 100644 > --- a/drivers/raw/ntb/ntb.c > +++ b/drivers/raw/ntb/ntb.c > @@ -801,11 +801,17 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, } > > static void > -ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) > +ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct ntb_hw *hw = dev->dev_private; > struct ntb_dev_info *info = dev_info; > > + if (dev_info_size != sizeof(*info)){ > + NTB_LOG(ERR, "Invalid size parameter to %s", __func__); > + return; > + } > + > info->mw_cnt = hw->mw_cnt; > info->mw_size = hw->mw_size; > > diff --git a/drivers/raw/skeleton/skeleton_rawdev.c > b/drivers/raw/skeleton/skeleton_rawdev.c > index 72ece887a..dc05f3ecf 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev.c > +++ b/drivers/raw/skeleton/skeleton_rawdev.c > @@ -42,14 +42,15 @@ static struct queue_buffers > queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int > queue_id); > > static void skeleton_rawdev_info_get(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info) > + rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct skeleton_rawdev *skeldev; > struct skeleton_rawdev_conf *skeldev_conf; > > SKELETON_PMD_FUNC_TRACE(); > > - if (!dev_info) { > + if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { > SKELETON_PMD_ERR("Invalid request"); > return; > } > diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c > b/drivers/raw/skeleton/skeleton_rawdev_test.c > index 9ecfdee81..9b8390dfb 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev_test.c > +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c > @@ -106,12 +106,12 @@ test_rawdev_info_get(void) > struct rte_rawdev_info rdev_info = {0}; > struct skeleton_rawdev_conf skel_conf = {0}; > > - ret = rte_rawdev_info_get(test_dev_id, NULL); > + ret = rte_rawdev_info_get(test_dev_id, NULL, 0); > RTE_TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); > > rdev_info.dev_private = &skel_conf; > > - ret = rte_rawdev_info_get(test_dev_id, &rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, &rdev_info, > sizeof(skel_conf)); > RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get raw dev info"); > > return TEST_SUCCESS; > @@ -142,7 +142,8 @@ test_rawdev_configure(void) > > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -170,7 +171,8 @@ test_rawdev_queue_default_conf_get(void) > /* Get the current configuration */ > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev > configuration (%d)", > ret); > > @@ -218,7 +220,8 @@ test_rawdev_queue_setup(void) > /* Get the current configuration */ > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -327,7 +330,8 @@ test_rawdev_start_stop(void) > dummy_firmware = NULL; > > rte_rawdev_start(test_dev_id); > - ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -336,7 +340,8 @@ test_rawdev_start_stop(void) > rdev_conf_get.device_state); > > rte_rawdev_stop(test_dev_id); > - ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index > b66ee73bc..5c631da1b 100644 > --- a/examples/ioat/ioatfwd.c > +++ b/examples/ioat/ioatfwd.c > @@ -757,7 +757,7 @@ assign_rawdevs(void) > do { > if (rdev_id == rte_rawdev_count()) > goto end; > - rte_rawdev_info_get(rdev_id++, > &rdev_info); > + rte_rawdev_info_get(rdev_id++, &rdev_info, > 0); > } while (rdev_info.driver_name == NULL || > strcmp(rdev_info.driver_name, > > IOAT_PMD_RAWDEV_NAME_STR) != 0); > diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index > eba8ebf9f..11e224451 100644 > --- a/examples/ntb/ntb_fwd.c > +++ b/examples/ntb/ntb_fwd.c > @@ -1389,7 +1389,7 @@ main(int argc, char **argv) > rte_rawdev_set_attr(dev_id, NTB_QUEUE_NUM_NAME, > num_queues); > printf("Set queue number as %u.\n", num_queues); > ntb_rawdev_info.dev_private = (rte_rawdev_obj_t)(&ntb_info); > - rte_rawdev_info_get(dev_id, &ntb_rawdev_info); > + rte_rawdev_info_get(dev_id, &ntb_rawdev_info, sizeof(ntb_info)); > > nb_mbuf = nb_desc * num_queues * 2 * 2 + rte_lcore_count() * > MEMPOOL_CACHE_SIZE; > diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c > index 8f84d0b22..a57689035 100644 > --- a/lib/librte_rawdev/rte_rawdev.c > +++ b/lib/librte_rawdev/rte_rawdev.c > @@ -78,7 +78,8 @@ rte_rawdev_socket_id(uint16_t dev_id) } > > int > -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) > +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, > + size_t dev_private_size) > { > struct rte_rawdev *rawdev; > > @@ -89,7 +90,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info) > > if (dev_info->dev_private != NULL) { > RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops- > >dev_info_get, -ENOTSUP); > - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info- > >dev_private); > + (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info- > >dev_private, > + dev_private_size); > } > > dev_info->driver_name = rawdev->driver_name; diff --git > a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index > 32f6b8bb0..cf6acfd26 100644 > --- a/lib/librte_rawdev/rte_rawdev.h > +++ b/lib/librte_rawdev/rte_rawdev.h > @@ -82,13 +82,20 @@ struct rte_rawdev_info; > * will be returned. This can be used to safely query the type of a rawdev > * instance without needing to know the size of the private data to return. > * > + * @param dev_private_size > + * The length of the memory space pointed to by dev_private in dev_info. > + * This should be set to the size of the expected private structure to be > + * returned, and may be checked by drivers to ensure the expected struct > + * type is provided. > + * > * @return > * - 0: Success, driver updates the contextual information of the raw device > * - <0: Error code returned by the driver info get function. > * > */ > int > -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info); > +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, > + size_t dev_private_size); > > /** > * Configure a raw device. > diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h > b/lib/librte_rawdev/rte_rawdev_pmd.h > index 4395a2182..0e72a9205 100644 > --- a/lib/librte_rawdev/rte_rawdev_pmd.h > +++ b/lib/librte_rawdev/rte_rawdev_pmd.h > @@ -138,12 +138,15 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) > * Raw device pointer > * @param dev_info > * Raw device information structure > + * @param dev_private_size > + * The size of the structure pointed to by dev_info->dev_private > * > * @return > * Returns 0 on success > */ > typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info); > + rte_rawdev_obj_t dev_info, > + size_t dev_private_size); > > /** > * Configure a device. > -- > 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn Bruce Richardson @ 2020-07-09 15:20 ` Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn Bruce Richardson ` (4 subsequent siblings) 6 siblings, 1 reply; 30+ messages in thread From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao, Mahipal Challa, Jerin Jacob, Bruce Richardson Since we now allow some parameter checking inside the driver info_get() functions, it makes sense to allow error return from those functions to the caller. Therefore we change the driver callback return type from void to int. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ifpga/ifpga_rawdev.c | 15 ++++++++------- drivers/raw/ioat/ioat_rawdev.c | 9 ++++++--- drivers/raw/ntb/ntb.c | 8 +++++--- drivers/raw/skeleton/skeleton_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 ++-- 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 47cfa3877..32a2b96c9 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -603,7 +603,7 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, return 0; } -static void +static int ifpga_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) @@ -620,12 +620,12 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); - return; + return -EINVAL; } adapter = ifpga_rawdev_get_priv(dev); if (!adapter) - return; + return -ENOENT; afu_dev = dev_info; afu_dev->rawdev = dev; @@ -637,7 +637,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (ifpga_fill_afu_dev(acc, afu_dev)) { IFPGA_RAWDEV_PMD_ERR("cannot get info\n"); - return; + return -ENOENT; } } @@ -647,21 +647,21 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, /* get LineSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 0, &opae_lside_eth_info)) { - return; + return -ENOENT; } lside_bar_idx = opae_lside_eth_info.mem_idx; /* get NICSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 1, &opae_nside_eth_info)) { - return; + return -ENOENT; } nside_bar_idx = opae_nside_eth_info.mem_idx; if (lside_bar_idx >= PCI_MAX_RESOURCE || nside_bar_idx >= PCI_MAX_RESOURCE || lside_bar_idx == nside_bar_idx) - return; + return -ENOENT; /* fill LineSide BAR Index */ afu_dev->mem_resource[lside_bar_idx].phys_addr = @@ -679,6 +679,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, afu_dev->mem_resource[nside_bar_idx].addr = opae_nside_eth_info.addr; } + return 0; } static int diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 8dd856c55..6a336795d 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -112,15 +112,18 @@ ioat_dev_stop(struct rte_rawdev *dev) RTE_SET_USED(dev); } -static void +static int ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL && dev_info_size == sizeof(*cfg)) - cfg->ring_size = ioat->ring_size; + if (dev_info == NULL || dev_info_size != sizeof(*cfg)) + return -EINVAL; + + cfg->ring_size = ioat->ring_size; + return 0; } static const char * const xstat_names[] = { diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index 4676c6f8f..eaeb67b74 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -800,7 +800,7 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, return nb_rx; } -static void +static int ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -809,7 +809,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (dev_info_size != sizeof(*info)){ NTB_LOG(ERR, "Invalid size parameter to %s", __func__); - return; + return -EINVAL; } info->mw_cnt = hw->mw_cnt; @@ -824,7 +824,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (!hw->queue_size || !hw->queue_pairs) { NTB_LOG(ERR, "No queue size and queue num assigned."); - return; + return -EAGAIN; } hw->hdr_size_per_queue = RTE_ALIGN(sizeof(struct ntb_header) + @@ -832,6 +832,8 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, hw->queue_size * sizeof(struct ntb_used), RTE_CACHE_LINE_SIZE); info->ntb_hdr_size = hw->hdr_size_per_queue * hw->queue_pairs; + + return 0; } static int diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dc05f3ecf..dce300c35 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -41,7 +41,7 @@ struct queue_buffers { static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); -static void skeleton_rawdev_info_get(struct rte_rawdev *dev, +static int skeleton_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -52,7 +52,7 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); - return; + return -EINVAL; } skeldev = skeleton_rawdev_get_priv(dev); @@ -63,6 +63,8 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, skeldev_conf->capabilities = skeldev->capabilities; skeldev_conf->device_state = skeldev->device_state; skeldev_conf->firmware_state = skeldev->fw.firmware_state; + + return 0; } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index a57689035..bde33763e 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -82,6 +82,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, size_t dev_private_size) { struct rte_rawdev *rawdev; + int ret = 0; RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); RTE_FUNC_PTR_OR_ERR_RET(dev_info, -EINVAL); @@ -90,7 +91,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + ret = (*rawdev->dev_ops->dev_info_get)(rawdev, + dev_info->dev_private, dev_private_size); } @@ -98,7 +100,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, dev_info->device = rawdev->device; dev_info->socket_id = rawdev->socket_id; - return 0; + return ret; } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 0e72a9205..89e46412a 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -142,9 +142,9 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * The size of the structure pointed to by dev_info->dev_private * * @return - * Returns 0 on success + * Returns 0 on success, negative error code on failure */ -typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_private_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function Bruce Richardson @ 2020-07-12 14:13 ` Xu, Rosen 0 siblings, 0 replies; 30+ messages in thread From: Xu, Rosen @ 2020-07-12 14:13 UTC (permalink / raw) To: Richardson, Bruce, Nipun Gupta, Hemant Agrawal Cc: dev, Zhang, Tianfei, Li, Xiaoyun, Wu, Jingjing, Satha Rao, Mahipal Challa, Jerin Jacob Hi, Reviewed-by: Rosen Xu <rosen.xu@intel.com> > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, July 09, 2020 23:21 > To: Nipun Gupta <nipun.gupta@nxp.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Cc: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei > <tianfei.zhang@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; Satha Rao <skoteshwar@marvell.com>; Mahipal > Challa <mchalla@marvell.com>; Jerin Jacob <jerinj@marvell.com>; > Richardson, Bruce <bruce.richardson@intel.com> > Subject: [PATCH 20.11 2/5] rawdev: allow drivers to return error from info > function > > Since we now allow some parameter checking inside the driver info_get() > functions, it makes sense to allow error return from those functions to the > caller. Therefore we change the driver callback return type from void to int. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/raw/ifpga/ifpga_rawdev.c | 15 ++++++++------- > drivers/raw/ioat/ioat_rawdev.c | 9 ++++++--- > drivers/raw/ntb/ntb.c | 8 +++++--- > drivers/raw/skeleton/skeleton_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev_pmd.h | 4 ++-- > 6 files changed, 29 insertions(+), 19 deletions(-) > > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > b/drivers/raw/ifpga/ifpga_rawdev.c > index 47cfa3877..32a2b96c9 100644 > --- a/drivers/raw/ifpga/ifpga_rawdev.c > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > @@ -603,7 +603,7 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, > return 0; > } > > -static void > +static int > ifpga_rawdev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > size_t dev_info_size) > @@ -620,12 +620,12 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > > if (!dev_info || dev_info_size != sizeof(*afu_dev)) { > IFPGA_RAWDEV_PMD_ERR("Invalid request"); > - return; > + return -EINVAL; > } > > adapter = ifpga_rawdev_get_priv(dev); > if (!adapter) > - return; > + return -ENOENT; > > afu_dev = dev_info; > afu_dev->rawdev = dev; > @@ -637,7 +637,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > > if (ifpga_fill_afu_dev(acc, afu_dev)) { > IFPGA_RAWDEV_PMD_ERR("cannot get info\n"); > - return; > + return -ENOENT; > } > } > > @@ -647,21 +647,21 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > /* get LineSide BAR Index */ > if (opae_manager_get_eth_group_region_info(mgr, 0, > &opae_lside_eth_info)) { > - return; > + return -ENOENT; > } > lside_bar_idx = opae_lside_eth_info.mem_idx; > > /* get NICSide BAR Index */ > if (opae_manager_get_eth_group_region_info(mgr, 1, > &opae_nside_eth_info)) { > - return; > + return -ENOENT; > } > nside_bar_idx = opae_nside_eth_info.mem_idx; > > if (lside_bar_idx >= PCI_MAX_RESOURCE || > nside_bar_idx >= PCI_MAX_RESOURCE || > lside_bar_idx == nside_bar_idx) > - return; > + return -ENOENT; > > /* fill LineSide BAR Index */ > afu_dev->mem_resource[lside_bar_idx].phys_addr = @@ - > 679,6 +679,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > afu_dev->mem_resource[nside_bar_idx].addr = > opae_nside_eth_info.addr; > } > + return 0; > } > > static int > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 8dd856c55..6a336795d 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -112,15 +112,18 @@ ioat_dev_stop(struct rte_rawdev *dev) > RTE_SET_USED(dev); > } > > -static void > +static int > ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > size_t dev_info_size) > { > struct rte_ioat_rawdev_config *cfg = dev_info; > struct rte_ioat_rawdev *ioat = dev->dev_private; > > - if (cfg != NULL && dev_info_size == sizeof(*cfg)) > - cfg->ring_size = ioat->ring_size; > + if (dev_info == NULL || dev_info_size != sizeof(*cfg)) > + return -EINVAL; > + > + cfg->ring_size = ioat->ring_size; > + return 0; > } > > static const char * const xstat_names[] = { diff --git a/drivers/raw/ntb/ntb.c > b/drivers/raw/ntb/ntb.c index 4676c6f8f..eaeb67b74 100644 > --- a/drivers/raw/ntb/ntb.c > +++ b/drivers/raw/ntb/ntb.c > @@ -800,7 +800,7 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, > return nb_rx; > } > > -static void > +static int > ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > size_t dev_info_size) > { > @@ -809,7 +809,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > > if (dev_info_size != sizeof(*info)){ > NTB_LOG(ERR, "Invalid size parameter to %s", __func__); > - return; > + return -EINVAL; > } > > info->mw_cnt = hw->mw_cnt; > @@ -824,7 +824,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > > if (!hw->queue_size || !hw->queue_pairs) { > NTB_LOG(ERR, "No queue size and queue num assigned."); > - return; > + return -EAGAIN; > } > > hw->hdr_size_per_queue = RTE_ALIGN(sizeof(struct ntb_header) + > @@ -832,6 +832,8 @@ ntb_dev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > hw->queue_size * sizeof(struct ntb_used), > RTE_CACHE_LINE_SIZE); > info->ntb_hdr_size = hw->hdr_size_per_queue * hw->queue_pairs; > + > + return 0; > } > > static int > diff --git a/drivers/raw/skeleton/skeleton_rawdev.c > b/drivers/raw/skeleton/skeleton_rawdev.c > index dc05f3ecf..dce300c35 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev.c > +++ b/drivers/raw/skeleton/skeleton_rawdev.c > @@ -41,7 +41,7 @@ struct queue_buffers { static struct queue_buffers > queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int > queue_id); > > -static void skeleton_rawdev_info_get(struct rte_rawdev *dev, > +static int skeleton_rawdev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > size_t dev_info_size) > { > @@ -52,7 +52,7 @@ static void skeleton_rawdev_info_get(struct > rte_rawdev *dev, > > if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { > SKELETON_PMD_ERR("Invalid request"); > - return; > + return -EINVAL; > } > > skeldev = skeleton_rawdev_get_priv(dev); @@ -63,6 +63,8 @@ > static void skeleton_rawdev_info_get(struct rte_rawdev *dev, > skeldev_conf->capabilities = skeldev->capabilities; > skeldev_conf->device_state = skeldev->device_state; > skeldev_conf->firmware_state = skeldev->fw.firmware_state; > + > + return 0; > } > > static int skeleton_rawdev_configure(const struct rte_rawdev *dev, diff -- > git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c > index a57689035..bde33763e 100644 > --- a/lib/librte_rawdev/rte_rawdev.c > +++ b/lib/librte_rawdev/rte_rawdev.c > @@ -82,6 +82,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info, > size_t dev_private_size) > { > struct rte_rawdev *rawdev; > + int ret = 0; > > RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); > RTE_FUNC_PTR_OR_ERR_RET(dev_info, -EINVAL); @@ -90,7 +91,8 > @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info > *dev_info, > > if (dev_info->dev_private != NULL) { > RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops- > >dev_info_get, -ENOTSUP); > - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info- > >dev_private, > + ret = (*rawdev->dev_ops->dev_info_get)(rawdev, > + dev_info->dev_private, > dev_private_size); > } > > @@ -98,7 +100,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info, > dev_info->device = rawdev->device; > dev_info->socket_id = rawdev->socket_id; > > - return 0; > + return ret; > } > > int > diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h > b/lib/librte_rawdev/rte_rawdev_pmd.h > index 0e72a9205..89e46412a 100644 > --- a/lib/librte_rawdev/rte_rawdev_pmd.h > +++ b/lib/librte_rawdev/rte_rawdev_pmd.h > @@ -142,9 +142,9 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) > * The size of the structure pointed to by dev_info->dev_private > * > * @return > - * Returns 0 on success > + * Returns 0 on success, negative error code on failure > */ > -typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, > +typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, > size_t dev_private_size); > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function Bruce Richardson @ 2020-07-09 15:20 ` Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 4/5] rawdev: add private data length parameter to queue fns Bruce Richardson ` (3 subsequent siblings) 6 siblings, 1 reply; 30+ messages in thread From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao, Mahipal Challa, Jerin Jacob, Bruce Richardson Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the structure passed to configure API is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ifpga/ifpga_rawdev.c | 3 ++- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 2 +- drivers/raw/ntb/ntb.c | 6 +++++- drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 ++++--- drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 ++- drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 ++++--- drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 5 +++-- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 8 +++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 3 ++- 15 files changed, 43 insertions(+), 23 deletions(-) diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 32a2b96c9..a50173264 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -684,7 +684,8 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, static int ifpga_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size __rte_unused) { IFPGA_RAWDEV_PMD_FUNC_TRACE(); diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 6a336795d..b29ff983f 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -41,7 +41,8 @@ RTE_LOG_REGISTER(ioat_pmd_logtype, rawdev.ioat, INFO); #define COMPLETION_SZ sizeof(__m128i) static int -ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct rte_ioat_rawdev_config *params = config; struct rte_ioat_rawdev *ioat = dev->dev_private; @@ -51,7 +52,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) if (dev->started) return -EBUSY; - if (params == NULL) + if (params == NULL || config_size != sizeof(*params)) return -EINVAL; if (params->ring_size > 4096 || params->ring_size < 64 || diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index 90f5974cd..e5b50ae9f 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -165,7 +165,7 @@ ioat_rawdev_test(uint16_t dev_id) } p.ring_size = IOAT_TEST_RINGSIZE; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { printf("Error with rte_rawdev_configure()\n"); return -1; } diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index eaeb67b74..c181094d5 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -837,13 +837,17 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, } static int -ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct ntb_dev_config *conf = config; struct ntb_hw *hw = dev->dev_private; uint32_t xstats_num; int ret; + if (conf == NULL || config_size != sizeof(*conf)) + return -EINVAL; + hw->queue_pairs = conf->num_queues; hw->queue_size = conf->queue_size; hw->used_mw_num = conf->mz_num; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c index e398abb75..5b496446c 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c @@ -294,7 +294,8 @@ otx2_dpi_rawdev_reset(struct rte_rawdev *dev) } static int -otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct dpi_rawdev_conf_s *conf = config; struct dpi_vf_s *dpivf = NULL; @@ -302,8 +303,8 @@ otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) uintptr_t pool; uint32_t gaura; - if (conf == NULL) { - otx2_dpi_dbg("NULL configuration"); + if (conf == NULL || config_size != sizeof(*conf)) { + otx2_dpi_dbg("NULL or invalid configuration"); return -EINVAL; } dpivf = (struct dpi_vf_s *)dev->dev_private; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_test.c b/drivers/raw/octeontx2_dma/otx2_dpi_test.c index 276658af0..cec6ca91b 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_test.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_test.c @@ -182,7 +182,8 @@ test_otx2_dma_rawdev(uint16_t val) /* Configure rawdev ports */ conf.chunk_pool = dpi_create_mempool(); rdev_info.dev_private = &conf; - ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info, + sizeof(conf)); if (ret) { otx2_dpi_dbg("Unable to configure DPIVF %d", i); return -ENODEV; diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c index 0778603d5..2b78a7941 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c @@ -224,13 +224,14 @@ sdp_rawdev_close(struct rte_rawdev *dev) } static int -sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct sdp_rawdev_info *app_info = (struct sdp_rawdev_info *)config; struct sdp_device *sdpvf; - if (app_info == NULL) { - otx2_err("Application config info [NULL]"); + if (app_info == NULL || config_size != sizeof(*app_info)) { + otx2_err("Application config info [NULL] or incorrect size"); return -EINVAL; } diff --git a/drivers/raw/octeontx2_ep/otx2_ep_test.c b/drivers/raw/octeontx2_ep/otx2_ep_test.c index 091f1827c..b876275f7 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_test.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_test.c @@ -108,7 +108,7 @@ sdp_rawdev_selftest(uint16_t dev_id) dev_info.dev_private = &app_info; - ret = rte_rawdev_configure(dev_id, &dev_info); + ret = rte_rawdev_configure(dev_id, &dev_info, sizeof(app_info)); if (ret) { otx2_err("Unable to configure SDP_VF %d", dev_id); rte_mempool_free(ioq_mpool); diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dce300c35..531d0450c 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -68,7 +68,8 @@ static int skeleton_rawdev_info_get(struct rte_rawdev *dev, } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; @@ -77,7 +78,7 @@ static int skeleton_rawdev_configure(const struct rte_rawdev *dev, RTE_FUNC_PTR_OR_ERR_RET(dev, -EINVAL); - if (!config) { + if (config == NULL || config_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid configuration"); return -EINVAL; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9b8390dfb..7dc7c7684 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -126,7 +126,7 @@ test_rawdev_configure(void) struct skeleton_rawdev_conf rdev_conf_get = {0}; /* Check invalid configuration */ - ret = rte_rawdev_configure(test_dev_id, NULL); + ret = rte_rawdev_configure(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Null configure; Expected -EINVAL, got %d", ret); @@ -137,7 +137,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_set; ret = rte_rawdev_configure(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_set)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to configure rawdev (%d)", ret); rdev_info.dev_private = &rdev_conf_get; diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index 5c631da1b..8e9513e44 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -734,7 +734,7 @@ configure_rawdev_queue(uint32_t dev_id) struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; struct rte_rawdev_info info = { .dev_private = &dev_config }; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { rte_exit(EXIT_FAILURE, "Error with rte_rawdev_configure()\n"); } diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 11e224451..656f73659 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1401,7 +1401,7 @@ main(int argc, char **argv) ntb_conf.num_queues = num_queues; ntb_conf.queue_size = nb_desc; ntb_rawdev_conf.dev_private = (rte_rawdev_obj_t)(&ntb_conf); - ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf); + ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf, sizeof(ntb_conf)); if (ret) rte_exit(EXIT_FAILURE, "Can't config ntb dev: err=%d, " "port=%u\n", ret, dev_id); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index bde33763e..6c4d783cc 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -104,7 +104,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, } int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size) { struct rte_rawdev *dev; int diag; @@ -123,7 +124,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) } /* Configure the device */ - diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private); + diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private, + dev_private_size); if (diag != 0) RTE_RDEV_ERR("dev%d dev_configure = %d", dev_id, diag); else diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index cf6acfd26..73e3bd5ae 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -116,13 +116,19 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, * driver/implementation can use to configure the device. It is also assumed * that once the configuration is done, a `queue_id` type field can be used * to refer to some arbitrary internal representation of a queue. + * @dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * used by the driver, and may be checked by drivers to ensure the expected + * struct type is provided. * * @return * - 0: Success, device configured. * - <0: Error code returned by the driver configuration function. */ int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf); +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size); /** diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 89e46412a..050f8b029 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -160,7 +160,8 @@ typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, * Returns 0 on success */ typedef int (*rawdev_configure_t)(const struct rte_rawdev *dev, - rte_rawdev_obj_t config); + rte_rawdev_obj_t config, + size_t config_size); /** * Start a configured device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn Bruce Richardson @ 2020-07-12 14:13 ` Xu, Rosen 0 siblings, 0 replies; 30+ messages in thread From: Xu, Rosen @ 2020-07-12 14:13 UTC (permalink / raw) To: Richardson, Bruce, Nipun Gupta, Hemant Agrawal Cc: dev, Zhang, Tianfei, Li, Xiaoyun, Wu, Jingjing, Satha Rao, Mahipal Challa, Jerin Jacob Hi, Reviewed-by: Rosen Xu <rosen.xu@intel.com> > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, July 09, 2020 23:21 > To: Nipun Gupta <nipun.gupta@nxp.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Cc: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei > <tianfei.zhang@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing > <jingjing.wu@intel.com>; Satha Rao <skoteshwar@marvell.com>; Mahipal > Challa <mchalla@marvell.com>; Jerin Jacob <jerinj@marvell.com>; > Richardson, Bruce <bruce.richardson@intel.com> > Subject: [PATCH 20.11 3/5] rawdev: add private data length parameter to > config fn > > Currently with the rawdev API there is no way to check that the structure > passed in via the dev_private pointer in the structure passed to configure API > is of the correct type - it's just checked that it is non-NULL. Adding in the > length of the expected structure provides a measure of typechecking, and > can also be used for ABI compatibility in future, since ABI changes involving > structs almost always involve a change in size. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/raw/ifpga/ifpga_rawdev.c | 3 ++- > drivers/raw/ioat/ioat_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev_test.c | 2 +- > drivers/raw/ntb/ntb.c | 6 +++++- > drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 ++++--- > drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 ++- > drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 ++++--- > drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- > drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- > drivers/raw/skeleton/skeleton_rawdev_test.c | 5 +++-- > examples/ioat/ioatfwd.c | 2 +- > examples/ntb/ntb_fwd.c | 2 +- > lib/librte_rawdev/rte_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev.h | 8 +++++++- > lib/librte_rawdev/rte_rawdev_pmd.h | 3 ++- > 15 files changed, 43 insertions(+), 23 deletions(-) > > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > b/drivers/raw/ifpga/ifpga_rawdev.c > index 32a2b96c9..a50173264 100644 > --- a/drivers/raw/ifpga/ifpga_rawdev.c > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > @@ -684,7 +684,8 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > > static int > ifpga_rawdev_configure(const struct rte_rawdev *dev, > - rte_rawdev_obj_t config) > + rte_rawdev_obj_t config, > + size_t config_size __rte_unused) > { > IFPGA_RAWDEV_PMD_FUNC_TRACE(); > > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 6a336795d..b29ff983f 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -41,7 +41,8 @@ RTE_LOG_REGISTER(ioat_pmd_logtype, rawdev.ioat, > INFO); #define COMPLETION_SZ sizeof(__m128i) > > static int > -ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) > +ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t > config, > + size_t config_size) > { > struct rte_ioat_rawdev_config *params = config; > struct rte_ioat_rawdev *ioat = dev->dev_private; @@ -51,7 +52,7 > @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t > config) > if (dev->started) > return -EBUSY; > > - if (params == NULL) > + if (params == NULL || config_size != sizeof(*params)) > return -EINVAL; > > if (params->ring_size > 4096 || params->ring_size < 64 || diff --git > a/drivers/raw/ioat/ioat_rawdev_test.c > b/drivers/raw/ioat/ioat_rawdev_test.c > index 90f5974cd..e5b50ae9f 100644 > --- a/drivers/raw/ioat/ioat_rawdev_test.c > +++ b/drivers/raw/ioat/ioat_rawdev_test.c > @@ -165,7 +165,7 @@ ioat_rawdev_test(uint16_t dev_id) > } > > p.ring_size = IOAT_TEST_RINGSIZE; > - if (rte_rawdev_configure(dev_id, &info) != 0) { > + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { > printf("Error with rte_rawdev_configure()\n"); > return -1; > } > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index > eaeb67b74..c181094d5 100644 > --- a/drivers/raw/ntb/ntb.c > +++ b/drivers/raw/ntb/ntb.c > @@ -837,13 +837,17 @@ ntb_dev_info_get(struct rte_rawdev *dev, > rte_rawdev_obj_t dev_info, } > > static int > -ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) > +ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t > config, > + size_t config_size) > { > struct ntb_dev_config *conf = config; > struct ntb_hw *hw = dev->dev_private; > uint32_t xstats_num; > int ret; > > + if (conf == NULL || config_size != sizeof(*conf)) > + return -EINVAL; > + > hw->queue_pairs = conf->num_queues; > hw->queue_size = conf->queue_size; > hw->used_mw_num = conf->mz_num; > diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c > b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c > index e398abb75..5b496446c 100644 > --- a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c > +++ b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c > @@ -294,7 +294,8 @@ otx2_dpi_rawdev_reset(struct rte_rawdev *dev) } > > static int > -otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, > rte_rawdev_obj_t config) > +otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, > rte_rawdev_obj_t config, > + size_t config_size) > { > struct dpi_rawdev_conf_s *conf = config; > struct dpi_vf_s *dpivf = NULL; > @@ -302,8 +303,8 @@ otx2_dpi_rawdev_configure(const struct rte_rawdev > *dev, rte_rawdev_obj_t config) > uintptr_t pool; > uint32_t gaura; > > - if (conf == NULL) { > - otx2_dpi_dbg("NULL configuration"); > + if (conf == NULL || config_size != sizeof(*conf)) { > + otx2_dpi_dbg("NULL or invalid configuration"); > return -EINVAL; > } > dpivf = (struct dpi_vf_s *)dev->dev_private; diff --git > a/drivers/raw/octeontx2_dma/otx2_dpi_test.c > b/drivers/raw/octeontx2_dma/otx2_dpi_test.c > index 276658af0..cec6ca91b 100644 > --- a/drivers/raw/octeontx2_dma/otx2_dpi_test.c > +++ b/drivers/raw/octeontx2_dma/otx2_dpi_test.c > @@ -182,7 +182,8 @@ test_otx2_dma_rawdev(uint16_t val) > /* Configure rawdev ports */ > conf.chunk_pool = dpi_create_mempool(); > rdev_info.dev_private = &conf; > - ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info); > + ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info, > + sizeof(conf)); > if (ret) { > otx2_dpi_dbg("Unable to configure DPIVF %d", i); > return -ENODEV; > diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c > b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c > index 0778603d5..2b78a7941 100644 > --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c > +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c > @@ -224,13 +224,14 @@ sdp_rawdev_close(struct rte_rawdev *dev) } > > static int > -sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t > config) > +sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t > config, > + size_t config_size) > { > struct sdp_rawdev_info *app_info = (struct sdp_rawdev_info > *)config; > struct sdp_device *sdpvf; > > - if (app_info == NULL) { > - otx2_err("Application config info [NULL]"); > + if (app_info == NULL || config_size != sizeof(*app_info)) { > + otx2_err("Application config info [NULL] or incorrect size"); > return -EINVAL; > } > > diff --git a/drivers/raw/octeontx2_ep/otx2_ep_test.c > b/drivers/raw/octeontx2_ep/otx2_ep_test.c > index 091f1827c..b876275f7 100644 > --- a/drivers/raw/octeontx2_ep/otx2_ep_test.c > +++ b/drivers/raw/octeontx2_ep/otx2_ep_test.c > @@ -108,7 +108,7 @@ sdp_rawdev_selftest(uint16_t dev_id) > > dev_info.dev_private = &app_info; > > - ret = rte_rawdev_configure(dev_id, &dev_info); > + ret = rte_rawdev_configure(dev_id, &dev_info, sizeof(app_info)); > if (ret) { > otx2_err("Unable to configure SDP_VF %d", dev_id); > rte_mempool_free(ioq_mpool); > diff --git a/drivers/raw/skeleton/skeleton_rawdev.c > b/drivers/raw/skeleton/skeleton_rawdev.c > index dce300c35..531d0450c 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev.c > +++ b/drivers/raw/skeleton/skeleton_rawdev.c > @@ -68,7 +68,8 @@ static int skeleton_rawdev_info_get(struct rte_rawdev > *dev, } > > static int skeleton_rawdev_configure(const struct rte_rawdev *dev, > - rte_rawdev_obj_t config) > + rte_rawdev_obj_t config, > + size_t config_size) > { > struct skeleton_rawdev *skeldev; > struct skeleton_rawdev_conf *skeldev_conf; @@ -77,7 +78,7 @@ > static int skeleton_rawdev_configure(const struct rte_rawdev *dev, > > RTE_FUNC_PTR_OR_ERR_RET(dev, -EINVAL); > > - if (!config) { > + if (config == NULL || config_size != sizeof(*skeldev_conf)) { > SKELETON_PMD_ERR("Invalid configuration"); > return -EINVAL; > } > diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c > b/drivers/raw/skeleton/skeleton_rawdev_test.c > index 9b8390dfb..7dc7c7684 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev_test.c > +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c > @@ -126,7 +126,7 @@ test_rawdev_configure(void) > struct skeleton_rawdev_conf rdev_conf_get = {0}; > > /* Check invalid configuration */ > - ret = rte_rawdev_configure(test_dev_id, NULL); > + ret = rte_rawdev_configure(test_dev_id, NULL, 0); > RTE_TEST_ASSERT(ret == -EINVAL, > "Null configure; Expected -EINVAL, got %d", ret); > > @@ -137,7 +137,8 @@ test_rawdev_configure(void) > > rdev_info.dev_private = &rdev_conf_set; > ret = rte_rawdev_configure(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_set)); > RTE_TEST_ASSERT_SUCCESS(ret, "Failed to configure rawdev (%d)", > ret); > > rdev_info.dev_private = &rdev_conf_get; diff --git > a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index > 5c631da1b..8e9513e44 100644 > --- a/examples/ioat/ioatfwd.c > +++ b/examples/ioat/ioatfwd.c > @@ -734,7 +734,7 @@ configure_rawdev_queue(uint32_t dev_id) > struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; > struct rte_rawdev_info info = { .dev_private = &dev_config }; > > - if (rte_rawdev_configure(dev_id, &info) != 0) { > + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { > rte_exit(EXIT_FAILURE, > "Error with rte_rawdev_configure()\n"); > } > diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index > 11e224451..656f73659 100644 > --- a/examples/ntb/ntb_fwd.c > +++ b/examples/ntb/ntb_fwd.c > @@ -1401,7 +1401,7 @@ main(int argc, char **argv) > ntb_conf.num_queues = num_queues; > ntb_conf.queue_size = nb_desc; > ntb_rawdev_conf.dev_private = (rte_rawdev_obj_t)(&ntb_conf); > - ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf); > + ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf, > +sizeof(ntb_conf)); > if (ret) > rte_exit(EXIT_FAILURE, "Can't config ntb dev: err=%d, " > "port=%u\n", ret, dev_id); > diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c > index bde33763e..6c4d783cc 100644 > --- a/lib/librte_rawdev/rte_rawdev.c > +++ b/lib/librte_rawdev/rte_rawdev.c > @@ -104,7 +104,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info, } > > int > -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) > +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, > + size_t dev_private_size) > { > struct rte_rawdev *dev; > int diag; > @@ -123,7 +124,8 @@ rte_rawdev_configure(uint16_t dev_id, struct > rte_rawdev_info *dev_conf) > } > > /* Configure the device */ > - diag = (*dev->dev_ops->dev_configure)(dev, dev_conf- > >dev_private); > + diag = (*dev->dev_ops->dev_configure)(dev, dev_conf- > >dev_private, > + dev_private_size); > if (diag != 0) > RTE_RDEV_ERR("dev%d dev_configure = %d", dev_id, diag); > else > diff --git a/lib/librte_rawdev/rte_rawdev.h > b/lib/librte_rawdev/rte_rawdev.h index cf6acfd26..73e3bd5ae 100644 > --- a/lib/librte_rawdev/rte_rawdev.h > +++ b/lib/librte_rawdev/rte_rawdev.h > @@ -116,13 +116,19 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info, > * driver/implementation can use to configure the device. It is also assumed > * that once the configuration is done, a `queue_id` type field can be used > * to refer to some arbitrary internal representation of a queue. > + * @dev_private_size > + * The length of the memory space pointed to by dev_private in dev_info. > + * This should be set to the size of the expected private structure to be > + * used by the driver, and may be checked by drivers to ensure the > expected > + * struct type is provided. > * > * @return > * - 0: Success, device configured. > * - <0: Error code returned by the driver configuration function. > */ > int > -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf); > +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, > + size_t dev_private_size); > > > /** > diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h > b/lib/librte_rawdev/rte_rawdev_pmd.h > index 89e46412a..050f8b029 100644 > --- a/lib/librte_rawdev/rte_rawdev_pmd.h > +++ b/lib/librte_rawdev/rte_rawdev_pmd.h > @@ -160,7 +160,8 @@ typedef int (*rawdev_info_get_t)(struct rte_rawdev > *dev, > * Returns 0 on success > */ > typedef int (*rawdev_configure_t)(const struct rte_rawdev *dev, > - rte_rawdev_obj_t config); > + rte_rawdev_obj_t config, > + size_t config_size); > > /** > * Start a configured device. > -- > 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 20.11 4/5] rawdev: add private data length parameter to queue fns 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson ` (2 preceding siblings ...) 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn Bruce Richardson @ 2020-07-09 15:20 ` Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 5/5] rawdev: allow queue default config query to return error Bruce Richardson ` (2 subsequent siblings) 6 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao, Mahipal Challa, Jerin Jacob, Bruce Richardson The queue setup and queue defaults query functions take a void * parameter as configuration data, preventing any compile-time checking of the parameters and limiting runtime checks. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ntb/ntb.c | 25 ++++++++++++++++----- drivers/raw/skeleton/skeleton_rawdev.c | 10 +++++---- drivers/raw/skeleton/skeleton_rawdev_test.c | 8 +++---- examples/ntb/ntb_fwd.c | 3 ++- lib/librte_rawdev/rte_rawdev.c | 10 +++++---- lib/librte_rawdev/rte_rawdev.h | 10 +++++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 6 +++-- 7 files changed, 49 insertions(+), 23 deletions(-) diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index c181094d5..7c15e204c 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -249,11 +249,15 @@ ntb_dev_intr_handler(void *param) static void ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *q_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; + if (conf_size != sizeof(*q_conf)) + return; + q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; @@ -294,12 +298,16 @@ ntb_rxq_release(struct ntb_rx_queue *rxq) static int ntb_rxq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *rxq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_rx_queue *rxq; + if (conf_size != sizeof(*rxq_conf)) + return -EINVAL; + /* Allocate the rx queue data structure */ rxq = rte_zmalloc_socket("ntb rx queue", sizeof(struct ntb_rx_queue), @@ -375,13 +383,17 @@ ntb_txq_release(struct ntb_tx_queue *txq) static int ntb_txq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *txq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_tx_queue *txq; uint16_t i, prev; + if (conf_size != sizeof(*txq_conf)) + return -EINVAL; + /* Allocate the TX queue data structure. */ txq = rte_zmalloc_socket("ntb tx queue", sizeof(struct ntb_tx_queue), @@ -439,7 +451,8 @@ ntb_txq_setup(struct rte_rawdev *dev, static int ntb_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_hw *hw = dev->dev_private; int ret; @@ -447,11 +460,11 @@ ntb_queue_setup(struct rte_rawdev *dev, if (queue_id >= hw->queue_pairs) return -EINVAL; - ret = ntb_txq_setup(dev, queue_id, queue_conf); + ret = ntb_txq_setup(dev, queue_id, queue_conf, conf_size); if (ret < 0) return ret; - ret = ntb_rxq_setup(dev, queue_id, queue_conf); + ret = ntb_rxq_setup(dev, queue_id, queue_conf, conf_size); return ret; } diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 531d0450c..f109e4d2c 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -222,14 +222,15 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || conf_size != sizeof(struct skeleton_rawdev_queue)) return; skeldev = skeleton_rawdev_get_priv(dev); @@ -252,7 +253,8 @@ clear_queue_bufs(int queue_id) static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { int ret = 0; struct skeleton_rawdev *skeldev; @@ -260,7 +262,7 @@ static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || conf_size != sizeof(struct skeleton_rawdev_queue)) return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 7dc7c7684..bb4b6efe4 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -185,7 +185,7 @@ test_rawdev_queue_default_conf_get(void) * depth = DEF_DEPTH */ for (i = 0; i < rdev_conf_get.num_queues; i++) { - rte_rawdev_queue_conf_get(test_dev_id, i, &q); + rte_rawdev_queue_conf_get(test_dev_id, i, &q, sizeof(q)); RTE_TEST_ASSERT_EQUAL(q.depth, SKELETON_QUEUE_DEF_DEPTH, "Invalid default depth of queue (%d)", q.depth); @@ -235,11 +235,11 @@ test_rawdev_queue_setup(void) /* Modify the queue depth for Queue 0 and attach it */ qset.depth = 15; qset.state = SKELETON_QUEUE_ATTACH; - ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset); + ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset, sizeof(qset)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to setup queue (%d)", ret); /* Now, fetching the queue 0 should show depth as 15 */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qset.depth, qget.depth, @@ -263,7 +263,7 @@ test_rawdev_queue_release(void) RTE_TEST_ASSERT_SUCCESS(ret, "Failed to release queue 0; (%d)", ret); /* Now, fetching the queue 0 should show depth as default */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qget.depth, SKELETON_QUEUE_DEF_DEPTH, diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 656f73659..5a8439b8d 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1411,7 +1411,8 @@ main(int argc, char **argv) ntb_q_conf.rx_mp = mbuf_pool; for (i = 0; i < num_queues; i++) { /* Setup rawdev queue */ - ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf); + ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf, + sizeof(ntb_q_conf)); if (ret < 0) rte_exit(EXIT_FAILURE, "Failed to setup ntb queue %u.\n", i); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 6c4d783cc..8965f2ce3 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -137,7 +137,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -145,14 +146,15 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf); + (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, queue_conf_size); return 0; } int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -160,7 +162,7 @@ rte_rawdev_queue_setup(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_setup, -ENOTSUP); - return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf); + return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf, queue_conf_size); } int diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 73e3bd5ae..bbd63913a 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -146,6 +146,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, * previously supplied to rte_rawdev_configure(). * @param[out] queue_conf * The pointer to the default raw queue configuration data. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * @return * - 0: Success, driver updates the default raw queue configuration data. * - <0: Error code returned by the driver info get function. @@ -156,7 +158,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Allocate and set up a raw queue for a raw device. @@ -169,6 +172,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, * @param queue_conf * The pointer to the configuration data to be used for the raw queue. * NULL value is allowed, in which case default configuration used. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * * @see rte_rawdev_queue_conf_get() * @@ -179,7 +184,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release and deallocate a raw queue from a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 050f8b029..34eb667f6 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -218,7 +218,8 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); */ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Setup an raw queue. @@ -235,7 +236,8 @@ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, */ typedef int (*rawdev_queue_setup_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release resources allocated by given raw queue. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH 20.11 5/5] rawdev: allow queue default config query to return error 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson ` (3 preceding siblings ...) 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 4/5] rawdev: add private data length parameter to queue fns Bruce Richardson @ 2020-07-09 15:20 ` Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson 6 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-07-09 15:20 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Satha Rao, Mahipal Challa, Jerin Jacob, Bruce Richardson The driver APIs for returning the queue default config can fail if the parameters are invalid, or other reasons, so allow them to return error codes to the rawdev layer and from hence to the app. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ntb/ntb.c | 6 ++++-- drivers/raw/skeleton/skeleton_rawdev.c | 12 +++++++----- lib/librte_rawdev/rte_rawdev.c | 3 +-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 +++- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index 7c15e204c..72489c521 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -246,7 +246,7 @@ ntb_dev_intr_handler(void *param) } } -static void +static int ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, @@ -256,11 +256,13 @@ ntb_queue_conf_get(struct rte_rawdev *dev, struct ntb_hw *hw = dev->dev_private; if (conf_size != sizeof(*q_conf)) - return; + return -EINVAL; q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; + + return 0; } static void diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index f109e4d2c..9c4082b9a 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -220,10 +220,10 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) return 0; } -static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, - uint16_t queue_id, - rte_rawdev_obj_t queue_conf, - size_t conf_size) +static int skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, + uint16_t queue_id, + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; @@ -231,7 +231,7 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, SKELETON_PMD_FUNC_TRACE(); if (!dev || !queue_conf || conf_size != sizeof(struct skeleton_rawdev_queue)) - return; + return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); skelq = &skeldev->queues[queue_id]; @@ -239,6 +239,8 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, if (queue_id < SKELETON_MAX_QUEUES) rte_memcpy(queue_conf, skelq, sizeof(struct skeleton_rawdev_queue)); + + return 0; } static void diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 8965f2ce3..69872d450 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -146,8 +146,7 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, queue_conf_size); - return 0; + return (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, queue_conf_size); } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 34eb667f6..34dd7181b 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -215,8 +215,10 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); * @param[out] queue_conf * Raw device queue configuration structure * + * @return + * Returns 0 on success, negative errno on failure */ -typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, size_t queue_conf_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson ` (4 preceding siblings ...) 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 5/5] rawdev: allow queue default config query to return error Bruce Richardson @ 2020-08-13 11:27 ` Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn Bruce Richardson ` (8 more replies) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson 6 siblings, 9 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:27 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal; +Cc: dev, Bruce Richardson This patchset proposes some internal and externally-visible changes to the rawdev API, the ABI change of which were previously announced. The changes are in two main areas: * For any APIs which take a void * parameter for driver-specific structs, add an additional parameter to provide the struct length. This allows some runtime type-checking, as well as possible ABI-compatibility support in the future as structure change generally involve a change in the size of the structure. * Ensure all APIs which can return error values have int type, rather than void. Since functions like info_get and queue_default_get can now do some typechecking, they need to be modified to allow them to return error codes on failure. V2: - add additional patch to make start/stop functions optional - remove deprecation notice once changes applied Bruce Richardson (7): rawdev: add private data length parameter to info fn rawdev: allow drivers to return error from info function rawdev: add private data length parameter to config fn rawdev: add private data length parameter to queue fns rawdev: allow queue default config query to return error rawdev: mark start and stop functions optional doc: remove rawdev deprecation notice app/test/test_rawdev.c | 2 +- doc/guides/rawdevs/ioat.rst | 4 +- doc/guides/rawdevs/octeontx2_dma.rst | 2 +- doc/guides/rawdevs/octeontx2_ep.rst | 3 +- doc/guides/rel_notes/deprecation.rst | 7 --- doc/guides/sample_app_ug/ioat.rst | 4 +- drivers/bus/ifpga/ifpga_bus.c | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 23 +++++----- drivers/raw/ioat/ioat_rawdev.c | 17 ++++--- drivers/raw/ioat/ioat_rawdev_test.c | 6 +-- drivers/raw/ntb/ntb.c | 49 ++++++++++++++++----- drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 +-- drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 +- drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 +-- drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- drivers/raw/skeleton/skeleton_rawdev.c | 36 +++++++++------ drivers/raw/skeleton/skeleton_rawdev_test.c | 32 ++++++++------ examples/ioat/ioatfwd.c | 4 +- examples/ntb/ntb_fwd.c | 7 +-- lib/librte_rawdev/rte_rawdev.c | 47 +++++++++++++------- lib/librte_rawdev/rte_rawdev.h | 27 ++++++++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 22 ++++++--- 22 files changed, 201 insertions(+), 112 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson @ 2020-08-13 11:27 ` Bruce Richardson 2020-08-17 5:26 ` Xu, Rosen 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 2/7] rawdev: allow drivers to return error from info function Bruce Richardson ` (7 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:27 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Rosen Xu, John McNamara, Marko Kovacevic, Tianfei zhang, Xiaoyun Li, Jingjing Wu Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the dev_info structure is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> --- app/test/test_rawdev.c | 2 +- doc/guides/rawdevs/ioat.rst | 2 +- doc/guides/sample_app_ug/ioat.rst | 2 +- drivers/bus/ifpga/ifpga_bus.c | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 4 ++-- drivers/raw/ntb/ntb.c | 8 +++++++- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 19 ++++++++++++------- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 9 ++++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 5 ++++- 15 files changed, 52 insertions(+), 26 deletions(-) diff --git a/app/test/test_rawdev.c b/app/test/test_rawdev.c index 524a9d5f3b..d8d9595be1 100644 --- a/app/test/test_rawdev.c +++ b/app/test/test_rawdev.c @@ -34,7 +34,7 @@ test_rawdev_selftest_ioat(void) for (i = 0; i < count; i++) { struct rte_rawdev_info info = { .dev_private = NULL }; - if (rte_rawdev_info_get(i, &info) == 0 && + if (rte_rawdev_info_get(i, &info, 0) == 0 && strstr(info.driver_name, "ioat") != NULL) return rte_rawdev_selftest(i) == 0 ? TEST_SUCCESS : TEST_FAILED; diff --git a/doc/guides/rawdevs/ioat.rst b/doc/guides/rawdevs/ioat.rst index d0eee5e237..dac52fabf1 100644 --- a/doc/guides/rawdevs/ioat.rst +++ b/doc/guides/rawdevs/ioat.rst @@ -107,7 +107,7 @@ rawdev device for use by an application: for (i = 0; i < count && !found; i++) { struct rte_rawdev_info info = { .dev_private = NULL }; - found = (rte_rawdev_info_get(i, &info) == 0 && + found = (rte_rawdev_info_get(i, &info, 0) == 0 && strcmp(info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) == 0); } diff --git a/doc/guides/sample_app_ug/ioat.rst b/doc/guides/sample_app_ug/ioat.rst index bab7654b8d..b5188220ba 100644 --- a/doc/guides/sample_app_ug/ioat.rst +++ b/doc/guides/sample_app_ug/ioat.rst @@ -265,7 +265,7 @@ functions: do { if (rdev_id == rte_rawdev_count()) goto end; - rte_rawdev_info_get(rdev_id++, &rdev_info); + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); } while (strcmp(rdev_info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) != 0); diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c index 6b16a20bb6..bb8b3dcfb9 100644 --- a/drivers/bus/ifpga/ifpga_bus.c +++ b/drivers/bus/ifpga/ifpga_bus.c @@ -162,7 +162,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev, afu_dev->id.port = afu_pr_conf.afu_id.port; if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get) - rawdev->dev_ops->dev_info_get(rawdev, afu_dev); + rawdev->dev_ops->dev_info_get(rawdev, afu_dev, sizeof(*afu_dev)); if (rawdev->dev_ops && rawdev->dev_ops->dev_start && diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index cc25c662bc..47cfa38778 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -605,7 +605,8 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, static void ifpga_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct opae_adapter *adapter; struct opae_accelerator *acc; @@ -617,7 +618,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, IFPGA_RAWDEV_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 87fd088aac..a5c0452d7e 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -111,12 +111,13 @@ ioat_dev_stop(struct rte_rawdev *dev) } static void -ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL) + if (cfg != NULL && dev_info_size == sizeof(*cfg)) cfg->ring_size = ioat->ring_size; } diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index c37351af2d..2b40222eb4 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -148,7 +148,7 @@ ioat_rawdev_test(uint16_t dev_id) unsigned int nb_xstats; unsigned int i; - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != expected_ring_size) { printf("Error, initial ring size is not as expected (Actual: %d, Expected: %d)\n", (int)p.ring_size, expected_ring_size); @@ -160,7 +160,7 @@ ioat_rawdev_test(uint16_t dev_id) printf("Error with rte_rawdev_configure()\n"); return -1; } - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != IOAT_TEST_RINGSIZE) { printf("Error, ring size is not %d (%d)\n", IOAT_TEST_RINGSIZE, (int)p.ring_size); diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index e40412bb7e..c890c43a32 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -801,11 +801,17 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, } static void -ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct ntb_hw *hw = dev->dev_private; struct ntb_dev_info *info = dev_info; + if (dev_info_size != sizeof(*info)) { + NTB_LOG(ERR, "Invalid size parameter to %s", __func__); + return; + } + info->mw_cnt = hw->mw_cnt; info->mw_size = hw->mw_size; diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 72ece887af..dc05f3ecf8 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -42,14 +42,15 @@ static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); static void skeleton_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; SKELETON_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9ecfdee818..9b8390dfb7 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -106,12 +106,12 @@ test_rawdev_info_get(void) struct rte_rawdev_info rdev_info = {0}; struct skeleton_rawdev_conf skel_conf = {0}; - ret = rte_rawdev_info_get(test_dev_id, NULL); + ret = rte_rawdev_info_get(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); rdev_info.dev_private = &skel_conf; - ret = rte_rawdev_info_get(test_dev_id, &rdev_info); + ret = rte_rawdev_info_get(test_dev_id, &rdev_info, sizeof(skel_conf)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get raw dev info"); return TEST_SUCCESS; @@ -142,7 +142,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -170,7 +171,8 @@ test_rawdev_queue_default_conf_get(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -218,7 +220,8 @@ test_rawdev_queue_setup(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -327,7 +330,8 @@ test_rawdev_start_stop(void) dummy_firmware = NULL; rte_rawdev_start(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -336,7 +340,8 @@ test_rawdev_start_stop(void) rdev_conf_get.device_state); rte_rawdev_stop(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index 75d8d5b9fe..76932d3dd2 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -757,7 +757,7 @@ assign_rawdevs(void) do { if (rdev_id == rte_rawdev_count()) goto end; - rte_rawdev_info_get(rdev_id++, &rdev_info); + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); } while (rdev_info.driver_name == NULL || strcmp(rdev_info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) != 0); diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index eba8ebf9fa..11e224451c 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1389,7 +1389,7 @@ main(int argc, char **argv) rte_rawdev_set_attr(dev_id, NTB_QUEUE_NUM_NAME, num_queues); printf("Set queue number as %u.\n", num_queues); ntb_rawdev_info.dev_private = (rte_rawdev_obj_t)(&ntb_info); - rte_rawdev_info_get(dev_id, &ntb_rawdev_info); + rte_rawdev_info_get(dev_id, &ntb_rawdev_info, sizeof(ntb_info)); nb_mbuf = nb_desc * num_queues * 2 * 2 + rte_lcore_count() * MEMPOOL_CACHE_SIZE; diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 8f84d0b228..a576890356 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -78,7 +78,8 @@ rte_rawdev_socket_id(uint16_t dev_id) } int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size) { struct rte_rawdev *rawdev; @@ -89,7 +90,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private); + (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + dev_private_size); } dev_info->driver_name = rawdev->driver_name; diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 32f6b8bb03..cf6acfd261 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -82,13 +82,20 @@ struct rte_rawdev_info; * will be returned. This can be used to safely query the type of a rawdev * instance without needing to know the size of the private data to return. * + * @param dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * returned, and may be checked by drivers to ensure the expected struct + * type is provided. + * * @return * - 0: Success, driver updates the contextual information of the raw device * - <0: Error code returned by the driver info get function. * */ int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info); +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size); /** * Configure a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 4395a2182d..0e72a92058 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -138,12 +138,15 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * Raw device pointer * @param dev_info * Raw device information structure + * @param dev_private_size + * The size of the structure pointed to by dev_info->dev_private * * @return * Returns 0 on success */ typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info); + rte_rawdev_obj_t dev_info, + size_t dev_private_size); /** * Configure a device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn Bruce Richardson @ 2020-08-17 5:26 ` Xu, Rosen 0 siblings, 0 replies; 30+ messages in thread From: Xu, Rosen @ 2020-08-17 5:26 UTC (permalink / raw) To: Richardson, Bruce, Nipun Gupta, Hemant Agrawal Cc: dev, Mcnamara, John, Kovacevic, Marko, Zhang, Tianfei, Li, Xiaoyun, Wu, Jingjing Hi, > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, August 13, 2020 19:28 > To: Nipun Gupta <nipun.gupta@nxp.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Xu, > Rosen <rosen.xu@intel.com>; Mcnamara, John > <john.mcnamara@intel.com>; Kovacevic, Marko > <marko.kovacevic@intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com>; Li, > Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com> > Subject: [PATCH v2 1/7] rawdev: add private data length parameter to info fn > > Currently with the rawdev API there is no way to check that the structure > passed in via the dev_private pointer in the dev_info structure is of the > correct type - it's just checked that it is non-NULL. Adding in the length of the > expected structure provides a measure of typechecking, and can also be used > for ABI compatibility in future, since ABI changes involving structs almost > always involve a change in size. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > Reviewed-by: Rosen Xu <rosen.xu@intel.com> > --- > app/test/test_rawdev.c | 2 +- > doc/guides/rawdevs/ioat.rst | 2 +- > doc/guides/sample_app_ug/ioat.rst | 2 +- > drivers/bus/ifpga/ifpga_bus.c | 2 +- > drivers/raw/ifpga/ifpga_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev_test.c | 4 ++-- > drivers/raw/ntb/ntb.c | 8 +++++++- > drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- > drivers/raw/skeleton/skeleton_rawdev_test.c | 19 ++++++++++++------- > examples/ioat/ioatfwd.c | 2 +- > examples/ntb/ntb_fwd.c | 2 +- > lib/librte_rawdev/rte_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev.h | 9 ++++++++- > lib/librte_rawdev/rte_rawdev_pmd.h | 5 ++++- > 15 files changed, 52 insertions(+), 26 deletions(-) > > diff --git a/app/test/test_rawdev.c b/app/test/test_rawdev.c index > 524a9d5f3b..d8d9595be1 100644 > --- a/app/test/test_rawdev.c > +++ b/app/test/test_rawdev.c > @@ -34,7 +34,7 @@ test_rawdev_selftest_ioat(void) > > for (i = 0; i < count; i++) { > struct rte_rawdev_info info = { .dev_private = NULL }; > - if (rte_rawdev_info_get(i, &info) == 0 && > + if (rte_rawdev_info_get(i, &info, 0) == 0 && > strstr(info.driver_name, "ioat") != NULL) > return rte_rawdev_selftest(i) == 0 ? > TEST_SUCCESS : TEST_FAILED; > diff --git a/doc/guides/rawdevs/ioat.rst b/doc/guides/rawdevs/ioat.rst index > d0eee5e237..dac52fabf1 100644 > --- a/doc/guides/rawdevs/ioat.rst > +++ b/doc/guides/rawdevs/ioat.rst > @@ -107,7 +107,7 @@ rawdev device for use by an application: > > for (i = 0; i < count && !found; i++) { > struct rte_rawdev_info info = { .dev_private = NULL }; > - found = (rte_rawdev_info_get(i, &info) == 0 && > + found = (rte_rawdev_info_get(i, &info, 0) == 0 && > strcmp(info.driver_name, > IOAT_PMD_RAWDEV_NAME_STR) == 0); > } > diff --git a/doc/guides/sample_app_ug/ioat.rst > b/doc/guides/sample_app_ug/ioat.rst > index bab7654b8d..b5188220ba 100644 > --- a/doc/guides/sample_app_ug/ioat.rst > +++ b/doc/guides/sample_app_ug/ioat.rst > @@ -265,7 +265,7 @@ functions: > do { > if (rdev_id == rte_rawdev_count()) > goto end; > - rte_rawdev_info_get(rdev_id++, &rdev_info); > + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); > } while (strcmp(rdev_info.driver_name, > IOAT_PMD_RAWDEV_NAME_STR) != 0); > > diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c > index 6b16a20bb6..bb8b3dcfb9 100644 > --- a/drivers/bus/ifpga/ifpga_bus.c > +++ b/drivers/bus/ifpga/ifpga_bus.c > @@ -162,7 +162,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev, > afu_dev->id.port = afu_pr_conf.afu_id.port; > > if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get) > - rawdev->dev_ops->dev_info_get(rawdev, afu_dev); > + rawdev->dev_ops->dev_info_get(rawdev, afu_dev, > sizeof(*afu_dev)); > > if (rawdev->dev_ops && > rawdev->dev_ops->dev_start && > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c > b/drivers/raw/ifpga/ifpga_rawdev.c > index cc25c662bc..47cfa38778 100644 > --- a/drivers/raw/ifpga/ifpga_rawdev.c > +++ b/drivers/raw/ifpga/ifpga_rawdev.c > @@ -605,7 +605,8 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, > > static void > ifpga_rawdev_info_get(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info) > + rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct opae_adapter *adapter; > struct opae_accelerator *acc; > @@ -617,7 +618,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, > > IFPGA_RAWDEV_PMD_FUNC_TRACE(); > > - if (!dev_info) { > + if (!dev_info || dev_info_size != sizeof(*afu_dev)) { > IFPGA_RAWDEV_PMD_ERR("Invalid request"); > return; > } > diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c > index 87fd088aac..a5c0452d7e 100644 > --- a/drivers/raw/ioat/ioat_rawdev.c > +++ b/drivers/raw/ioat/ioat_rawdev.c > @@ -111,12 +111,13 @@ ioat_dev_stop(struct rte_rawdev *dev) } > > static void > -ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) > +ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct rte_ioat_rawdev_config *cfg = dev_info; > struct rte_ioat_rawdev *ioat = dev->dev_private; > > - if (cfg != NULL) > + if (cfg != NULL && dev_info_size == sizeof(*cfg)) > cfg->ring_size = ioat->ring_size; > } > > diff --git a/drivers/raw/ioat/ioat_rawdev_test.c > b/drivers/raw/ioat/ioat_rawdev_test.c > index c37351af2d..2b40222eb4 100644 > --- a/drivers/raw/ioat/ioat_rawdev_test.c > +++ b/drivers/raw/ioat/ioat_rawdev_test.c > @@ -148,7 +148,7 @@ ioat_rawdev_test(uint16_t dev_id) > unsigned int nb_xstats; > unsigned int i; > > - rte_rawdev_info_get(dev_id, &info); > + rte_rawdev_info_get(dev_id, &info, sizeof(p)); > if (p.ring_size != expected_ring_size) { > printf("Error, initial ring size is not as expected (Actual: %d, > Expected: %d)\n", > (int)p.ring_size, expected_ring_size); @@ - > 160,7 +160,7 @@ ioat_rawdev_test(uint16_t dev_id) > printf("Error with rte_rawdev_configure()\n"); > return -1; > } > - rte_rawdev_info_get(dev_id, &info); > + rte_rawdev_info_get(dev_id, &info, sizeof(p)); > if (p.ring_size != IOAT_TEST_RINGSIZE) { > printf("Error, ring size is not %d (%d)\n", > IOAT_TEST_RINGSIZE, (int)p.ring_size); diff -- > git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index > e40412bb7e..c890c43a32 100644 > --- a/drivers/raw/ntb/ntb.c > +++ b/drivers/raw/ntb/ntb.c > @@ -801,11 +801,17 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, } > > static void > -ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) > +ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct ntb_hw *hw = dev->dev_private; > struct ntb_dev_info *info = dev_info; > > + if (dev_info_size != sizeof(*info)) { > + NTB_LOG(ERR, "Invalid size parameter to %s", __func__); > + return; > + } > + > info->mw_cnt = hw->mw_cnt; > info->mw_size = hw->mw_size; > > diff --git a/drivers/raw/skeleton/skeleton_rawdev.c > b/drivers/raw/skeleton/skeleton_rawdev.c > index 72ece887af..dc05f3ecf8 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev.c > +++ b/drivers/raw/skeleton/skeleton_rawdev.c > @@ -42,14 +42,15 @@ static struct queue_buffers > queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int > queue_id); > > static void skeleton_rawdev_info_get(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info) > + rte_rawdev_obj_t dev_info, > + size_t dev_info_size) > { > struct skeleton_rawdev *skeldev; > struct skeleton_rawdev_conf *skeldev_conf; > > SKELETON_PMD_FUNC_TRACE(); > > - if (!dev_info) { > + if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { > SKELETON_PMD_ERR("Invalid request"); > return; > } > diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c > b/drivers/raw/skeleton/skeleton_rawdev_test.c > index 9ecfdee818..9b8390dfb7 100644 > --- a/drivers/raw/skeleton/skeleton_rawdev_test.c > +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c > @@ -106,12 +106,12 @@ test_rawdev_info_get(void) > struct rte_rawdev_info rdev_info = {0}; > struct skeleton_rawdev_conf skel_conf = {0}; > > - ret = rte_rawdev_info_get(test_dev_id, NULL); > + ret = rte_rawdev_info_get(test_dev_id, NULL, 0); > RTE_TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); > > rdev_info.dev_private = &skel_conf; > > - ret = rte_rawdev_info_get(test_dev_id, &rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, &rdev_info, > sizeof(skel_conf)); > RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get raw dev info"); > > return TEST_SUCCESS; > @@ -142,7 +142,8 @@ test_rawdev_configure(void) > > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -170,7 +171,8 @@ test_rawdev_queue_default_conf_get(void) > /* Get the current configuration */ > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev > configuration (%d)", > ret); > > @@ -218,7 +220,8 @@ test_rawdev_queue_setup(void) > /* Get the current configuration */ > rdev_info.dev_private = &rdev_conf_get; > ret = rte_rawdev_info_get(test_dev_id, > - (rte_rawdev_obj_t)&rdev_info); > + (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -327,7 +330,8 @@ test_rawdev_start_stop(void) > dummy_firmware = NULL; > > rte_rawdev_start(test_dev_id); > - ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > @@ -336,7 +340,8 @@ test_rawdev_start_stop(void) > rdev_conf_get.device_state); > > rte_rawdev_stop(test_dev_id); > - ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info); > + ret = rte_rawdev_info_get(test_dev_id, > (rte_rawdev_obj_t)&rdev_info, > + sizeof(rdev_conf_get)); > RTE_TEST_ASSERT_SUCCESS(ret, > "Failed to obtain rawdev configuration (%d)", > ret); > diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index > 75d8d5b9fe..76932d3dd2 100644 > --- a/examples/ioat/ioatfwd.c > +++ b/examples/ioat/ioatfwd.c > @@ -757,7 +757,7 @@ assign_rawdevs(void) > do { > if (rdev_id == rte_rawdev_count()) > goto end; > - rte_rawdev_info_get(rdev_id++, &rdev_info); > + rte_rawdev_info_get(rdev_id++, &rdev_info, > 0); > } while (rdev_info.driver_name == NULL || > strcmp(rdev_info.driver_name, > > IOAT_PMD_RAWDEV_NAME_STR) != 0); > diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index > eba8ebf9fa..11e224451c 100644 > --- a/examples/ntb/ntb_fwd.c > +++ b/examples/ntb/ntb_fwd.c > @@ -1389,7 +1389,7 @@ main(int argc, char **argv) > rte_rawdev_set_attr(dev_id, NTB_QUEUE_NUM_NAME, > num_queues); > printf("Set queue number as %u.\n", num_queues); > ntb_rawdev_info.dev_private = (rte_rawdev_obj_t)(&ntb_info); > - rte_rawdev_info_get(dev_id, &ntb_rawdev_info); > + rte_rawdev_info_get(dev_id, &ntb_rawdev_info, sizeof(ntb_info)); > > nb_mbuf = nb_desc * num_queues * 2 * 2 + rte_lcore_count() * > MEMPOOL_CACHE_SIZE; > diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c > index 8f84d0b228..a576890356 100644 > --- a/lib/librte_rawdev/rte_rawdev.c > +++ b/lib/librte_rawdev/rte_rawdev.c > @@ -78,7 +78,8 @@ rte_rawdev_socket_id(uint16_t dev_id) } > > int > -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) > +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, > + size_t dev_private_size) > { > struct rte_rawdev *rawdev; > > @@ -89,7 +90,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct > rte_rawdev_info *dev_info) > > if (dev_info->dev_private != NULL) { > RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops- > >dev_info_get, -ENOTSUP); > - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info- > >dev_private); > + (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info- > >dev_private, > + dev_private_size); > } > > dev_info->driver_name = rawdev->driver_name; diff --git > a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index > 32f6b8bb03..cf6acfd261 100644 > --- a/lib/librte_rawdev/rte_rawdev.h > +++ b/lib/librte_rawdev/rte_rawdev.h > @@ -82,13 +82,20 @@ struct rte_rawdev_info; > * will be returned. This can be used to safely query the type of a rawdev > * instance without needing to know the size of the private data to return. > * > + * @param dev_private_size > + * The length of the memory space pointed to by dev_private in dev_info. > + * This should be set to the size of the expected private structure to be > + * returned, and may be checked by drivers to ensure the expected struct > + * type is provided. > + * > * @return > * - 0: Success, driver updates the contextual information of the raw device > * - <0: Error code returned by the driver info get function. > * > */ > int > -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info); > +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, > + size_t dev_private_size); > > /** > * Configure a raw device. > diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h > b/lib/librte_rawdev/rte_rawdev_pmd.h > index 4395a2182d..0e72a92058 100644 > --- a/lib/librte_rawdev/rte_rawdev_pmd.h > +++ b/lib/librte_rawdev/rte_rawdev_pmd.h > @@ -138,12 +138,15 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) > * Raw device pointer > * @param dev_info > * Raw device information structure > + * @param dev_private_size > + * The size of the structure pointed to by dev_info->dev_private > * > * @return > * Returns 0 on success > */ > typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, > - rte_rawdev_obj_t dev_info); > + rte_rawdev_obj_t dev_info, > + size_t dev_private_size); > > /** > * Configure a device. > -- > 2.25.1 Acked-by: Rosen Xu <rosen.xu@intel.com> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 2/7] rawdev: allow drivers to return error from info function 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn Bruce Richardson ` (6 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Rosen Xu, Tianfei zhang, Xiaoyun Li, Jingjing Wu Since we now allow some parameter checking inside the driver info_get() functions, it makes sense to allow error return from those functions to the caller. Therefore we change the driver callback return type from void to int. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> --- drivers/raw/ifpga/ifpga_rawdev.c | 15 ++++++++------- drivers/raw/ioat/ioat_rawdev.c | 9 ++++++--- drivers/raw/ntb/ntb.c | 8 +++++--- drivers/raw/skeleton/skeleton_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 ++-- 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 47cfa38778..32a2b96c90 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -603,7 +603,7 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, return 0; } -static void +static int ifpga_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) @@ -620,12 +620,12 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); - return; + return -EINVAL; } adapter = ifpga_rawdev_get_priv(dev); if (!adapter) - return; + return -ENOENT; afu_dev = dev_info; afu_dev->rawdev = dev; @@ -637,7 +637,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (ifpga_fill_afu_dev(acc, afu_dev)) { IFPGA_RAWDEV_PMD_ERR("cannot get info\n"); - return; + return -ENOENT; } } @@ -647,21 +647,21 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, /* get LineSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 0, &opae_lside_eth_info)) { - return; + return -ENOENT; } lside_bar_idx = opae_lside_eth_info.mem_idx; /* get NICSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 1, &opae_nside_eth_info)) { - return; + return -ENOENT; } nside_bar_idx = opae_nside_eth_info.mem_idx; if (lside_bar_idx >= PCI_MAX_RESOURCE || nside_bar_idx >= PCI_MAX_RESOURCE || lside_bar_idx == nside_bar_idx) - return; + return -ENOENT; /* fill LineSide BAR Index */ afu_dev->mem_resource[lside_bar_idx].phys_addr = @@ -679,6 +679,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, afu_dev->mem_resource[nside_bar_idx].addr = opae_nside_eth_info.addr; } + return 0; } static int diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index a5c0452d7e..e2fc787263 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -110,15 +110,18 @@ ioat_dev_stop(struct rte_rawdev *dev) RTE_SET_USED(dev); } -static void +static int ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL && dev_info_size == sizeof(*cfg)) - cfg->ring_size = ioat->ring_size; + if (dev_info == NULL || dev_info_size != sizeof(*cfg)) + return -EINVAL; + + cfg->ring_size = ioat->ring_size; + return 0; } static const char * const xstat_names[] = { diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index c890c43a32..ced5b6d624 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -800,7 +800,7 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, return nb_rx; } -static void +static int ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -809,7 +809,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (dev_info_size != sizeof(*info)) { NTB_LOG(ERR, "Invalid size parameter to %s", __func__); - return; + return -EINVAL; } info->mw_cnt = hw->mw_cnt; @@ -824,7 +824,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (!hw->queue_size || !hw->queue_pairs) { NTB_LOG(ERR, "No queue size and queue num assigned."); - return; + return -EAGAIN; } hw->hdr_size_per_queue = RTE_ALIGN(sizeof(struct ntb_header) + @@ -832,6 +832,8 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, hw->queue_size * sizeof(struct ntb_used), RTE_CACHE_LINE_SIZE); info->ntb_hdr_size = hw->hdr_size_per_queue * hw->queue_pairs; + + return 0; } static int diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dc05f3ecf8..dce300c358 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -41,7 +41,7 @@ struct queue_buffers { static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); -static void skeleton_rawdev_info_get(struct rte_rawdev *dev, +static int skeleton_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -52,7 +52,7 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); - return; + return -EINVAL; } skeldev = skeleton_rawdev_get_priv(dev); @@ -63,6 +63,8 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, skeldev_conf->capabilities = skeldev->capabilities; skeldev_conf->device_state = skeldev->device_state; skeldev_conf->firmware_state = skeldev->fw.firmware_state; + + return 0; } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index a576890356..bde33763ee 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -82,6 +82,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, size_t dev_private_size) { struct rte_rawdev *rawdev; + int ret = 0; RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); RTE_FUNC_PTR_OR_ERR_RET(dev_info, -EINVAL); @@ -90,7 +91,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + ret = (*rawdev->dev_ops->dev_info_get)(rawdev, + dev_info->dev_private, dev_private_size); } @@ -98,7 +100,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, dev_info->device = rawdev->device; dev_info->socket_id = rawdev->socket_id; - return 0; + return ret; } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 0e72a92058..89e46412a3 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -142,9 +142,9 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * The size of the structure pointed to by dev_info->dev_private * * @return - * Returns 0 on success + * Returns 0 on success, negative error code on failure */ -typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_private_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 2/7] rawdev: allow drivers to return error from info function Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-09-04 14:16 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson ` (5 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Rosen Xu, John McNamara, Marko Kovacevic, Satha Rao, Mahipal Challa, Tianfei zhang, Xiaoyun Li, Jingjing Wu Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the structure passed to configure API is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> --- doc/guides/rawdevs/ioat.rst | 2 +- doc/guides/rawdevs/octeontx2_dma.rst | 2 +- doc/guides/rawdevs/octeontx2_ep.rst | 3 ++- doc/guides/sample_app_ug/ioat.rst | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 3 ++- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 2 +- drivers/raw/ntb/ntb.c | 6 +++++- drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 ++++--- drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 ++- drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 ++++--- drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 5 +++-- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 8 +++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 3 ++- 19 files changed, 48 insertions(+), 27 deletions(-) diff --git a/doc/guides/rawdevs/ioat.rst b/doc/guides/rawdevs/ioat.rst index dac52fabf1..c46460ff45 100644 --- a/doc/guides/rawdevs/ioat.rst +++ b/doc/guides/rawdevs/ioat.rst @@ -142,7 +142,7 @@ The following code shows how the device is configured in /* ... */ p.ring_size = IOAT_TEST_RINGSIZE; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { printf("Error with rte_rawdev_configure()\n"); return -1; } diff --git a/doc/guides/rawdevs/octeontx2_dma.rst b/doc/guides/rawdevs/octeontx2_dma.rst index 1e1dfbe931..4a9794489e 100644 --- a/doc/guides/rawdevs/octeontx2_dma.rst +++ b/doc/guides/rawdevs/octeontx2_dma.rst @@ -92,7 +92,7 @@ The following code shows how the device is configured rte_mempool_set_ops_byname(conf.chunk_pool, rte_mbuf_platform_mempool_ops(), NULL); rte_mempool_populate_default(conf.chunk_pool); - rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info); + rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info, sizeof(conf)); Performing Data Transfer ------------------------ diff --git a/doc/guides/rawdevs/octeontx2_ep.rst b/doc/guides/rawdevs/octeontx2_ep.rst index bbcf530a45..b03fcf789a 100644 --- a/doc/guides/rawdevs/octeontx2_ep.rst +++ b/doc/guides/rawdevs/octeontx2_ep.rst @@ -66,7 +66,8 @@ The following code shows how the device is configured struct rte_rawdev_info rdev_info = {.dev_private = &config}; config.enqdeq_mpool = (void *)rte_mempool_create(...); - rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info); + rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(config)); Performing Data Transfer ------------------------ diff --git a/doc/guides/sample_app_ug/ioat.rst b/doc/guides/sample_app_ug/ioat.rst index b5188220ba..3f7d5c34a6 100644 --- a/doc/guides/sample_app_ug/ioat.rst +++ b/doc/guides/sample_app_ug/ioat.rst @@ -296,7 +296,7 @@ is done in ``configure_rawdev_queue()``. struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; struct rte_rawdev_info info = { .dev_private = &dev_config }; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { rte_exit(EXIT_FAILURE, "Error with rte_rawdev_configure()\n"); } diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 32a2b96c90..a50173264c 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -684,7 +684,8 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, static int ifpga_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size __rte_unused) { IFPGA_RAWDEV_PMD_FUNC_TRACE(); diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index e2fc787263..7f1a154360 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -39,7 +39,8 @@ RTE_LOG_REGISTER(ioat_pmd_logtype, rawdev.ioat, INFO); #define COMPLETION_SZ sizeof(__m128i) static int -ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct rte_ioat_rawdev_config *params = config; struct rte_ioat_rawdev *ioat = dev->dev_private; @@ -49,7 +50,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) if (dev->started) return -EBUSY; - if (params == NULL) + if (params == NULL || config_size != sizeof(*params)) return -EINVAL; if (params->ring_size > 4096 || params->ring_size < 64 || diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index 2b40222eb4..c463a82ad6 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -156,7 +156,7 @@ ioat_rawdev_test(uint16_t dev_id) } p.ring_size = IOAT_TEST_RINGSIZE; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { printf("Error with rte_rawdev_configure()\n"); return -1; } diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index ced5b6d624..7f6b835a4b 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -837,13 +837,17 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, } static int -ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct ntb_dev_config *conf = config; struct ntb_hw *hw = dev->dev_private; uint32_t xstats_num; int ret; + if (conf == NULL || config_size != sizeof(*conf)) + return -EINVAL; + hw->queue_pairs = conf->num_queues; hw->queue_size = conf->queue_size; hw->used_mw_num = conf->mz_num; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c index e398abb75b..5b496446ca 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c @@ -294,7 +294,8 @@ otx2_dpi_rawdev_reset(struct rte_rawdev *dev) } static int -otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct dpi_rawdev_conf_s *conf = config; struct dpi_vf_s *dpivf = NULL; @@ -302,8 +303,8 @@ otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) uintptr_t pool; uint32_t gaura; - if (conf == NULL) { - otx2_dpi_dbg("NULL configuration"); + if (conf == NULL || config_size != sizeof(*conf)) { + otx2_dpi_dbg("NULL or invalid configuration"); return -EINVAL; } dpivf = (struct dpi_vf_s *)dev->dev_private; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_test.c b/drivers/raw/octeontx2_dma/otx2_dpi_test.c index 276658af07..cec6ca91b0 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_test.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_test.c @@ -182,7 +182,8 @@ test_otx2_dma_rawdev(uint16_t val) /* Configure rawdev ports */ conf.chunk_pool = dpi_create_mempool(); rdev_info.dev_private = &conf; - ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info, + sizeof(conf)); if (ret) { otx2_dpi_dbg("Unable to configure DPIVF %d", i); return -ENODEV; diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c index 0778603d5b..2b78a7941d 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c @@ -224,13 +224,14 @@ sdp_rawdev_close(struct rte_rawdev *dev) } static int -sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct sdp_rawdev_info *app_info = (struct sdp_rawdev_info *)config; struct sdp_device *sdpvf; - if (app_info == NULL) { - otx2_err("Application config info [NULL]"); + if (app_info == NULL || config_size != sizeof(*app_info)) { + otx2_err("Application config info [NULL] or incorrect size"); return -EINVAL; } diff --git a/drivers/raw/octeontx2_ep/otx2_ep_test.c b/drivers/raw/octeontx2_ep/otx2_ep_test.c index 091f1827c7..b876275f7a 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_test.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_test.c @@ -108,7 +108,7 @@ sdp_rawdev_selftest(uint16_t dev_id) dev_info.dev_private = &app_info; - ret = rte_rawdev_configure(dev_id, &dev_info); + ret = rte_rawdev_configure(dev_id, &dev_info, sizeof(app_info)); if (ret) { otx2_err("Unable to configure SDP_VF %d", dev_id); rte_mempool_free(ioq_mpool); diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dce300c358..531d0450c8 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -68,7 +68,8 @@ static int skeleton_rawdev_info_get(struct rte_rawdev *dev, } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; @@ -77,7 +78,7 @@ static int skeleton_rawdev_configure(const struct rte_rawdev *dev, RTE_FUNC_PTR_OR_ERR_RET(dev, -EINVAL); - if (!config) { + if (config == NULL || config_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid configuration"); return -EINVAL; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9b8390dfb7..7dc7c76847 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -126,7 +126,7 @@ test_rawdev_configure(void) struct skeleton_rawdev_conf rdev_conf_get = {0}; /* Check invalid configuration */ - ret = rte_rawdev_configure(test_dev_id, NULL); + ret = rte_rawdev_configure(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Null configure; Expected -EINVAL, got %d", ret); @@ -137,7 +137,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_set; ret = rte_rawdev_configure(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_set)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to configure rawdev (%d)", ret); rdev_info.dev_private = &rdev_conf_get; diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index 76932d3dd2..43a19843ee 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -734,7 +734,7 @@ configure_rawdev_queue(uint32_t dev_id) struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; struct rte_rawdev_info info = { .dev_private = &dev_config }; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { rte_exit(EXIT_FAILURE, "Error with rte_rawdev_configure()\n"); } diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 11e224451c..656f736594 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1401,7 +1401,7 @@ main(int argc, char **argv) ntb_conf.num_queues = num_queues; ntb_conf.queue_size = nb_desc; ntb_rawdev_conf.dev_private = (rte_rawdev_obj_t)(&ntb_conf); - ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf); + ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf, sizeof(ntb_conf)); if (ret) rte_exit(EXIT_FAILURE, "Can't config ntb dev: err=%d, " "port=%u\n", ret, dev_id); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index bde33763ee..6c4d783cc5 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -104,7 +104,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, } int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size) { struct rte_rawdev *dev; int diag; @@ -123,7 +124,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) } /* Configure the device */ - diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private); + diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private, + dev_private_size); if (diag != 0) RTE_RDEV_ERR("dev%d dev_configure = %d", dev_id, diag); else diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index cf6acfd261..73e3bd5aea 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -116,13 +116,19 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, * driver/implementation can use to configure the device. It is also assumed * that once the configuration is done, a `queue_id` type field can be used * to refer to some arbitrary internal representation of a queue. + * @dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * used by the driver, and may be checked by drivers to ensure the expected + * struct type is provided. * * @return * - 0: Success, device configured. * - <0: Error code returned by the driver configuration function. */ int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf); +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size); /** diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 89e46412a3..050f8b0292 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -160,7 +160,8 @@ typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, * Returns 0 on success */ typedef int (*rawdev_configure_t)(const struct rte_rawdev *dev, - rte_rawdev_obj_t config); + rte_rawdev_obj_t config, + size_t config_size); /** * Start a configured device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn Bruce Richardson @ 2020-09-04 14:16 ` Bruce Richardson 0 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-04 14:16 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Rosen Xu, John McNamara, Marko Kovacevic, Satha Rao, Mahipal Challa, Tianfei zhang, Xiaoyun Li, Jingjing Wu On Thu, Aug 13, 2020 at 12:28:01PM +0100, Bruce Richardson wrote: > Currently with the rawdev API there is no way to check that the structure > passed in via the dev_private pointer in the structure passed to configure > API is of the correct type - it's just checked that it is non-NULL. Adding > in the length of the expected structure provides a measure of typechecking, > and can also be used for ABI compatibility in future, since ABI changes > involving structs almost always involve a change in size. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > Reviewed-by: Rosen Xu <rosen.xu@intel.com> > --- > doc/guides/rawdevs/ioat.rst | 2 +- > doc/guides/rawdevs/octeontx2_dma.rst | 2 +- > doc/guides/rawdevs/octeontx2_ep.rst | 3 ++- > doc/guides/sample_app_ug/ioat.rst | 2 +- > drivers/raw/ifpga/ifpga_rawdev.c | 3 ++- > drivers/raw/ioat/ioat_rawdev.c | 5 +++-- > drivers/raw/ioat/ioat_rawdev_test.c | 2 +- > drivers/raw/ntb/ntb.c | 6 +++++- > drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 ++++--- > drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 ++- > drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 ++++--- > drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- > drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- > drivers/raw/skeleton/skeleton_rawdev_test.c | 5 +++-- > examples/ioat/ioatfwd.c | 2 +- > examples/ntb/ntb_fwd.c | 2 +- > lib/librte_rawdev/rte_rawdev.c | 6 ++++-- > lib/librte_rawdev/rte_rawdev.h | 8 +++++++- > lib/librte_rawdev/rte_rawdev_pmd.h | 3 ++- > 19 files changed, 48 insertions(+), 27 deletions(-) > <snip> > diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h > index cf6acfd261..73e3bd5aea 100644 > --- a/lib/librte_rawdev/rte_rawdev.h > +++ b/lib/librte_rawdev/rte_rawdev.h > @@ -116,13 +116,19 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, > * driver/implementation can use to configure the device. It is also assumed > * that once the configuration is done, a `queue_id` type field can be used > * to refer to some arbitrary internal representation of a queue. > + * @dev_private_size Small mistake here I just spotted. This should be "@param dev_private_size", otherwise we get doc build warnings. Will fix in V3 if one is needed, otherwise I'd ask that this just be fixed on apply. Thanks, /Bruce ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 4/7] rawdev: add private data length parameter to queue fns 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (2 preceding siblings ...) 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 5/7] rawdev: allow queue default config query to return error Bruce Richardson ` (4 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Xiaoyun Li, Jingjing Wu The queue setup and queue defaults query functions take a void * parameter as configuration data, preventing any compile-time checking of the parameters and limiting runtime checks. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ntb/ntb.c | 25 ++++++++++++++++----- drivers/raw/skeleton/skeleton_rawdev.c | 12 ++++++---- drivers/raw/skeleton/skeleton_rawdev_test.c | 8 +++---- examples/ntb/ntb_fwd.c | 3 ++- lib/librte_rawdev/rte_rawdev.c | 12 ++++++---- lib/librte_rawdev/rte_rawdev.h | 10 +++++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 6 +++-- 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index 7f6b835a4b..ad10857991 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -249,11 +249,15 @@ ntb_dev_intr_handler(void *param) static void ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *q_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; + if (conf_size != sizeof(*q_conf)) + return; + q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; @@ -294,12 +298,16 @@ ntb_rxq_release(struct ntb_rx_queue *rxq) static int ntb_rxq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *rxq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_rx_queue *rxq; + if (conf_size != sizeof(*rxq_conf)) + return -EINVAL; + /* Allocate the rx queue data structure */ rxq = rte_zmalloc_socket("ntb rx queue", sizeof(struct ntb_rx_queue), @@ -375,13 +383,17 @@ ntb_txq_release(struct ntb_tx_queue *txq) static int ntb_txq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *txq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_tx_queue *txq; uint16_t i, prev; + if (conf_size != sizeof(*txq_conf)) + return -EINVAL; + /* Allocate the TX queue data structure. */ txq = rte_zmalloc_socket("ntb tx queue", sizeof(struct ntb_tx_queue), @@ -439,7 +451,8 @@ ntb_txq_setup(struct rte_rawdev *dev, static int ntb_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_hw *hw = dev->dev_private; int ret; @@ -447,11 +460,11 @@ ntb_queue_setup(struct rte_rawdev *dev, if (queue_id >= hw->queue_pairs) return -EINVAL; - ret = ntb_txq_setup(dev, queue_id, queue_conf); + ret = ntb_txq_setup(dev, queue_id, queue_conf, conf_size); if (ret < 0) return ret; - ret = ntb_rxq_setup(dev, queue_id, queue_conf); + ret = ntb_rxq_setup(dev, queue_id, queue_conf, conf_size); return ret; } diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 531d0450c8..801c707fb9 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -222,14 +222,16 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || + conf_size != sizeof(struct skeleton_rawdev_queue)) return; skeldev = skeleton_rawdev_get_priv(dev); @@ -252,7 +254,8 @@ clear_queue_bufs(int queue_id) static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { int ret = 0; struct skeleton_rawdev *skeldev; @@ -260,7 +263,8 @@ static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || + conf_size != sizeof(struct skeleton_rawdev_queue)) return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 7dc7c76847..bb4b6efe40 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -185,7 +185,7 @@ test_rawdev_queue_default_conf_get(void) * depth = DEF_DEPTH */ for (i = 0; i < rdev_conf_get.num_queues; i++) { - rte_rawdev_queue_conf_get(test_dev_id, i, &q); + rte_rawdev_queue_conf_get(test_dev_id, i, &q, sizeof(q)); RTE_TEST_ASSERT_EQUAL(q.depth, SKELETON_QUEUE_DEF_DEPTH, "Invalid default depth of queue (%d)", q.depth); @@ -235,11 +235,11 @@ test_rawdev_queue_setup(void) /* Modify the queue depth for Queue 0 and attach it */ qset.depth = 15; qset.state = SKELETON_QUEUE_ATTACH; - ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset); + ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset, sizeof(qset)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to setup queue (%d)", ret); /* Now, fetching the queue 0 should show depth as 15 */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qset.depth, qget.depth, @@ -263,7 +263,7 @@ test_rawdev_queue_release(void) RTE_TEST_ASSERT_SUCCESS(ret, "Failed to release queue 0; (%d)", ret); /* Now, fetching the queue 0 should show depth as default */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qget.depth, SKELETON_QUEUE_DEF_DEPTH, diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 656f736594..5a8439b8d3 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1411,7 +1411,8 @@ main(int argc, char **argv) ntb_q_conf.rx_mp = mbuf_pool; for (i = 0; i < num_queues; i++) { /* Setup rawdev queue */ - ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf); + ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf, + sizeof(ntb_q_conf)); if (ret < 0) rte_exit(EXIT_FAILURE, "Failed to setup ntb queue %u.\n", i); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 6c4d783cc5..f48d8cf10b 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -137,7 +137,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -145,14 +146,16 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf); + (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, + queue_conf_size); return 0; } int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -160,7 +163,8 @@ rte_rawdev_queue_setup(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_setup, -ENOTSUP); - return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf); + return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf, + queue_conf_size); } int diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 73e3bd5aea..bbd63913a0 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -146,6 +146,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, * previously supplied to rte_rawdev_configure(). * @param[out] queue_conf * The pointer to the default raw queue configuration data. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * @return * - 0: Success, driver updates the default raw queue configuration data. * - <0: Error code returned by the driver info get function. @@ -156,7 +158,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Allocate and set up a raw queue for a raw device. @@ -169,6 +172,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, * @param queue_conf * The pointer to the configuration data to be used for the raw queue. * NULL value is allowed, in which case default configuration used. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * * @see rte_rawdev_queue_conf_get() * @@ -179,7 +184,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release and deallocate a raw queue from a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 050f8b0292..34eb667f62 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -218,7 +218,8 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); */ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Setup an raw queue. @@ -235,7 +236,8 @@ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, */ typedef int (*rawdev_queue_setup_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release resources allocated by given raw queue. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 5/7] rawdev: allow queue default config query to return error 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (3 preceding siblings ...) 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 6/7] rawdev: mark start and stop functions optional Bruce Richardson ` (3 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Xiaoyun Li, Jingjing Wu The driver APIs for returning the queue default config can fail if the parameters are invalid, or other reasons, so allow them to return error codes to the rawdev layer and from hence to the app. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/raw/ntb/ntb.c | 6 ++++-- drivers/raw/skeleton/skeleton_rawdev.c | 12 +++++++----- lib/librte_rawdev/rte_rawdev.c | 3 +-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 +++- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index ad10857991..bddb1f5916 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -246,7 +246,7 @@ ntb_dev_intr_handler(void *param) } } -static void +static int ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, @@ -256,11 +256,13 @@ ntb_queue_conf_get(struct rte_rawdev *dev, struct ntb_hw *hw = dev->dev_private; if (conf_size != sizeof(*q_conf)) - return; + return -EINVAL; q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; + + return 0; } static void diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 801c707fb9..f8b47a391a 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -220,10 +220,10 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) return 0; } -static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, - uint16_t queue_id, - rte_rawdev_obj_t queue_conf, - size_t conf_size) +static int skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, + uint16_t queue_id, + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; @@ -232,7 +232,7 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, if (!dev || !queue_conf || conf_size != sizeof(struct skeleton_rawdev_queue)) - return; + return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); skelq = &skeldev->queues[queue_id]; @@ -240,6 +240,8 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, if (queue_id < SKELETON_MAX_QUEUES) rte_memcpy(queue_conf, skelq, sizeof(struct skeleton_rawdev_queue)); + + return 0; } static void diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index f48d8cf10b..fd123bb848 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -146,9 +146,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, + return (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, queue_conf_size); - return 0; } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 34eb667f62..34dd7181b4 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -215,8 +215,10 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); * @param[out] queue_conf * Raw device queue configuration structure * + * @return + * Returns 0 on success, negative errno on failure */ -typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, size_t queue_conf_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 6/7] rawdev: mark start and stop functions optional 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (4 preceding siblings ...) 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 5/7] rawdev: allow queue default config query to return error Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 7/7] doc: remove rawdev deprecation notice Bruce Richardson ` (2 subsequent siblings) 8 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal; +Cc: dev, Bruce Richardson, Kevin Laatz Not all rawdevs will require a device start/stop function, so rather than requiring such drivers to provide dummy functions, just set the started/stopped rawdev flag from the rawdev layer and return success. Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- lib/librte_rawdev/rte_rawdev.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index fd123bb848..36f3acf2b7 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -398,20 +398,21 @@ rte_rawdev_start(uint16_t dev_id) RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); dev = &rte_rawdevs[dev_id]; - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_start, -ENOTSUP); - if (dev->started != 0) { RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already started", dev_id); return 0; } + if (dev->dev_ops->dev_start == NULL) + goto mark_started; + diag = (*dev->dev_ops->dev_start)(dev); - if (diag == 0) - dev->started = 1; - else + if (diag != 0) return diag; +mark_started: + dev->started = 1; return 0; } @@ -425,15 +426,18 @@ rte_rawdev_stop(uint16_t dev_id) RTE_RAWDEV_VALID_DEVID_OR_RET(dev_id); dev = &rte_rawdevs[dev_id]; - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop); - if (dev->started == 0) { RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already stopped", dev_id); return; } + if (dev->dev_ops->dev_stop == NULL) + goto mark_stopped; + (*dev->dev_ops->dev_stop)(dev); + +mark_stopped: dev->started = 0; } -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v2 7/7] doc: remove rawdev deprecation notice 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (5 preceding siblings ...) 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 6/7] rawdev: mark start and stop functions optional Bruce Richardson @ 2020-08-13 11:28 ` Bruce Richardson 2020-09-02 11:21 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Nipun Gupta 2020-09-09 9:47 ` Thomas Monjalon 8 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-08-13 11:28 UTC (permalink / raw) To: Nipun Gupta, Hemant Agrawal Cc: dev, Bruce Richardson, Ray Kinsella, Neil Horman, John McNamara, Marko Kovacevic Since all changes covered by the deprecation notice have been applied, the notice can be removed. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- doc/guides/rel_notes/deprecation.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index 345c38d5b6..c0b3b7a605 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -301,13 +301,6 @@ Deprecation Notices - https://patches.dpdk.org/patch/71457/ - https://patches.dpdk.org/patch/71456/ -* rawdev: The rawdev APIs which take a device-specific structure as - parameter directly, or indirectly via a "private" pointer inside another - structure, will be modified to take an additional parameter of the - structure size. The affected APIs will include ``rte_rawdev_info_get``, - ``rte_rawdev_configure``, ``rte_rawdev_queue_conf_get`` and - ``rte_rawdev_queue_setup``. - * acl: ``RTE_ACL_CLASSIFY_NUM`` enum value will be removed. This enum value is not used inside DPDK, while it prevents to add new classify algorithms without causing an ABI breakage. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (6 preceding siblings ...) 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 7/7] doc: remove rawdev deprecation notice Bruce Richardson @ 2020-09-02 11:21 ` Nipun Gupta 2020-09-09 9:47 ` Thomas Monjalon 8 siblings, 0 replies; 30+ messages in thread From: Nipun Gupta @ 2020-09-02 11:21 UTC (permalink / raw) To: Bruce Richardson, Hemant Agrawal; +Cc: dev Series Acked-by: Nipun Gupta <nipun.gupta@nxp.com> > -----Original Message----- > From: Bruce Richardson <bruce.richardson@intel.com> > Sent: Thursday, August 13, 2020 4:58 PM > To: Nipun Gupta <nipun.gupta@nxp.com>; Hemant Agrawal > <hemant.agrawal@nxp.com> > Cc: dev@dpdk.org; Bruce Richardson <bruce.richardson@intel.com> > Subject: [PATCH v2 0/7] Enhance rawdev APIs > > This patchset proposes some internal and externally-visible changes to the > rawdev API, the ABI change of which were previously announced. > > The changes are in two main areas: > * For any APIs which take a void * parameter for driver-specific structs, > add an additional parameter to provide the struct length. This allows > some runtime type-checking, as well as possible ABI-compatibility support > in the future as structure change generally involve a change in the size > of the structure. > * Ensure all APIs which can return error values have int type, rather than > void. Since functions like info_get and queue_default_get can now do some > typechecking, they need to be modified to allow them to return error > codes on failure. > > V2: > - add additional patch to make start/stop functions optional > - remove deprecation notice once changes applied > > Bruce Richardson (7): > rawdev: add private data length parameter to info fn > rawdev: allow drivers to return error from info function > rawdev: add private data length parameter to config fn > rawdev: add private data length parameter to queue fns > rawdev: allow queue default config query to return error > rawdev: mark start and stop functions optional > doc: remove rawdev deprecation notice > > app/test/test_rawdev.c | 2 +- > doc/guides/rawdevs/ioat.rst | 4 +- > doc/guides/rawdevs/octeontx2_dma.rst | 2 +- > doc/guides/rawdevs/octeontx2_ep.rst | 3 +- > doc/guides/rel_notes/deprecation.rst | 7 --- > doc/guides/sample_app_ug/ioat.rst | 4 +- > drivers/bus/ifpga/ifpga_bus.c | 2 +- > drivers/raw/ifpga/ifpga_rawdev.c | 23 +++++----- > drivers/raw/ioat/ioat_rawdev.c | 17 ++++--- > drivers/raw/ioat/ioat_rawdev_test.c | 6 +-- > drivers/raw/ntb/ntb.c | 49 ++++++++++++++++----- > drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 +-- > drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 +- > drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 +-- > drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- > drivers/raw/skeleton/skeleton_rawdev.c | 36 +++++++++------ > drivers/raw/skeleton/skeleton_rawdev_test.c | 32 ++++++++------ > examples/ioat/ioatfwd.c | 4 +- > examples/ntb/ntb_fwd.c | 7 +-- > lib/librte_rawdev/rte_rawdev.c | 47 +++++++++++++------- > lib/librte_rawdev/rte_rawdev.h | 27 ++++++++++-- > lib/librte_rawdev/rte_rawdev_pmd.h | 22 ++++++--- > 22 files changed, 201 insertions(+), 112 deletions(-) > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson ` (7 preceding siblings ...) 2020-09-02 11:21 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Nipun Gupta @ 2020-09-09 9:47 ` Thomas Monjalon 8 siblings, 0 replies; 30+ messages in thread From: Thomas Monjalon @ 2020-09-09 9:47 UTC (permalink / raw) To: Bruce Richardson; +Cc: Nipun Gupta, Hemant Agrawal, dev > app/test/test_rawdev.c | 2 +- > doc/guides/rawdevs/ioat.rst | 4 +- > doc/guides/rawdevs/octeontx2_dma.rst | 2 +- > doc/guides/rawdevs/octeontx2_ep.rst | 3 +- > doc/guides/rel_notes/deprecation.rst | 7 --- > doc/guides/sample_app_ug/ioat.rst | 4 +- > drivers/bus/ifpga/ifpga_bus.c | 2 +- > drivers/raw/ifpga/ifpga_rawdev.c | 23 +++++----- > drivers/raw/ioat/ioat_rawdev.c | 17 ++++--- > drivers/raw/ioat/ioat_rawdev_test.c | 6 +-- > drivers/raw/ntb/ntb.c | 49 ++++++++++++++++----- > drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 +-- > drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 +- > drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 +-- > drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- > drivers/raw/skeleton/skeleton_rawdev.c | 36 +++++++++------ > drivers/raw/skeleton/skeleton_rawdev_test.c | 32 ++++++++------ > examples/ioat/ioatfwd.c | 4 +- > examples/ntb/ntb_fwd.c | 7 +-- > lib/librte_rawdev/rte_rawdev.c | 47 +++++++++++++------- > lib/librte_rawdev/rte_rawdev.h | 27 ++++++++++-- > lib/librte_rawdev/rte_rawdev_pmd.h | 22 ++++++--- > 22 files changed, 201 insertions(+), 112 deletions(-) It is missing updates of the release notes in each patch changing the API. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 0/7] Enhance rawdev APIs 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson ` (5 preceding siblings ...) 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn Bruce Richardson ` (7 more replies) 6 siblings, 8 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev; +Cc: thomas, Bruce Richardson This patchset proposes some internal and externally-visible changes to the rawdev API, the ABI change of which were previously announced. The changes are in two main areas: * For any APIs which take a void * parameter for driver-specific structs, add an additional parameter to provide the struct length. This allows some runtime type-checking, as well as possible ABI-compatibility support in the future as structure change generally involve a change in the size of the structure. * Ensure all APIs which can return error values have int type, rather than void. Since functions like info_get and queue_default_get can now do some typechecking, they need to be modified to allow them to return error codes on failure. V3: - fix doxygen error - add release note update for changes to public APIs V2: - add additional patch to make start/stop functions optional - remove deprecation notice once changes applied Bruce Richardson (7): rawdev: add private data length parameter to info fn rawdev: allow drivers to return error from info function rawdev: add private data length parameter to config fn rawdev: add private data length parameter to queue fns rawdev: allow queue config query to return error rawdev: mark start and stop functions optional doc: remove rawdev deprecation notice app/test/test_rawdev.c | 2 +- doc/guides/rawdevs/ioat.rst | 4 +- doc/guides/rawdevs/octeontx2_dma.rst | 2 +- doc/guides/rawdevs/octeontx2_ep.rst | 3 +- doc/guides/rel_notes/deprecation.rst | 7 --- doc/guides/rel_notes/release_20_11.rst | 9 ++++ doc/guides/sample_app_ug/ioat.rst | 4 +- drivers/bus/ifpga/ifpga_bus.c | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 23 +++++----- drivers/raw/ioat/ioat_rawdev.c | 17 ++++--- drivers/raw/ioat/ioat_rawdev_test.c | 6 +-- drivers/raw/ntb/ntb.c | 49 ++++++++++++++++----- drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 +-- drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 +- drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 +-- drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- drivers/raw/skeleton/skeleton_rawdev.c | 36 +++++++++------ drivers/raw/skeleton/skeleton_rawdev_test.c | 32 ++++++++------ examples/ioat/ioatfwd.c | 4 +- examples/ntb/ntb_fwd.c | 7 +-- lib/librte_rawdev/rte_rawdev.c | 47 +++++++++++++------- lib/librte_rawdev/rte_rawdev.h | 27 ++++++++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 22 ++++++--- 23 files changed, 210 insertions(+), 112 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 2/7] rawdev: allow drivers to return error from info function Bruce Richardson ` (6 subsequent siblings) 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Rosen Xu, Nipun Gupta, Hemant Agrawal, John McNamara, Marko Kovacevic, Tianfei zhang, Xiaoyun Li, Jingjing Wu Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the dev_info structure is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> Acked-by: Rosen Xu <rosen.xu@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- V3: Added short release-note item describing the change. --- app/test/test_rawdev.c | 2 +- doc/guides/rawdevs/ioat.rst | 2 +- doc/guides/rel_notes/release_20_11.rst | 4 ++++ doc/guides/sample_app_ug/ioat.rst | 2 +- drivers/bus/ifpga/ifpga_bus.c | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 4 ++-- drivers/raw/ntb/ntb.c | 8 +++++++- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 19 ++++++++++++------- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 9 ++++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 5 ++++- 16 files changed, 56 insertions(+), 26 deletions(-) diff --git a/app/test/test_rawdev.c b/app/test/test_rawdev.c index 524a9d5f3b..d8d9595be1 100644 --- a/app/test/test_rawdev.c +++ b/app/test/test_rawdev.c @@ -34,7 +34,7 @@ test_rawdev_selftest_ioat(void) for (i = 0; i < count; i++) { struct rte_rawdev_info info = { .dev_private = NULL }; - if (rte_rawdev_info_get(i, &info) == 0 && + if (rte_rawdev_info_get(i, &info, 0) == 0 && strstr(info.driver_name, "ioat") != NULL) return rte_rawdev_selftest(i) == 0 ? TEST_SUCCESS : TEST_FAILED; diff --git a/doc/guides/rawdevs/ioat.rst b/doc/guides/rawdevs/ioat.rst index d0eee5e237..dac52fabf1 100644 --- a/doc/guides/rawdevs/ioat.rst +++ b/doc/guides/rawdevs/ioat.rst @@ -107,7 +107,7 @@ rawdev device for use by an application: for (i = 0; i < count && !found; i++) { struct rte_rawdev_info info = { .dev_private = NULL }; - found = (rte_rawdev_info_get(i, &info) == 0 && + found = (rte_rawdev_info_get(i, &info, 0) == 0 && strcmp(info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) == 0); } diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index df227a1773..a931dc0bff 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -84,6 +84,10 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= +* rawdev: Added a structure size parameter to the function + ``rte_rawdev_info_get()``, + allowing limited driver type-checking and ABI compatibility. + ABI Changes ----------- diff --git a/doc/guides/sample_app_ug/ioat.rst b/doc/guides/sample_app_ug/ioat.rst index bab7654b8d..b5188220ba 100644 --- a/doc/guides/sample_app_ug/ioat.rst +++ b/doc/guides/sample_app_ug/ioat.rst @@ -265,7 +265,7 @@ functions: do { if (rdev_id == rte_rawdev_count()) goto end; - rte_rawdev_info_get(rdev_id++, &rdev_info); + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); } while (strcmp(rdev_info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) != 0); diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c index 6b16a20bb6..bb8b3dcfb9 100644 --- a/drivers/bus/ifpga/ifpga_bus.c +++ b/drivers/bus/ifpga/ifpga_bus.c @@ -162,7 +162,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev, afu_dev->id.port = afu_pr_conf.afu_id.port; if (rawdev->dev_ops && rawdev->dev_ops->dev_info_get) - rawdev->dev_ops->dev_info_get(rawdev, afu_dev); + rawdev->dev_ops->dev_info_get(rawdev, afu_dev, sizeof(*afu_dev)); if (rawdev->dev_ops && rawdev->dev_ops->dev_start && diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index cc25c662bc..47cfa38778 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -605,7 +605,8 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, static void ifpga_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct opae_adapter *adapter; struct opae_accelerator *acc; @@ -617,7 +618,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, IFPGA_RAWDEV_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index 87fd088aac..a5c0452d7e 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -111,12 +111,13 @@ ioat_dev_stop(struct rte_rawdev *dev) } static void -ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL) + if (cfg != NULL && dev_info_size == sizeof(*cfg)) cfg->ring_size = ioat->ring_size; } diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index c37351af2d..2b40222eb4 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -148,7 +148,7 @@ ioat_rawdev_test(uint16_t dev_id) unsigned int nb_xstats; unsigned int i; - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != expected_ring_size) { printf("Error, initial ring size is not as expected (Actual: %d, Expected: %d)\n", (int)p.ring_size, expected_ring_size); @@ -160,7 +160,7 @@ ioat_rawdev_test(uint16_t dev_id) printf("Error with rte_rawdev_configure()\n"); return -1; } - rte_rawdev_info_get(dev_id, &info); + rte_rawdev_info_get(dev_id, &info, sizeof(p)); if (p.ring_size != IOAT_TEST_RINGSIZE) { printf("Error, ring size is not %d (%d)\n", IOAT_TEST_RINGSIZE, (int)p.ring_size); diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index e40412bb7e..c890c43a32 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -801,11 +801,17 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, } static void -ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info) +ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct ntb_hw *hw = dev->dev_private; struct ntb_dev_info *info = dev_info; + if (dev_info_size != sizeof(*info)) { + NTB_LOG(ERR, "Invalid size parameter to %s", __func__); + return; + } + info->mw_cnt = hw->mw_cnt; info->mw_size = hw->mw_size; diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 72ece887af..dc05f3ecf8 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -42,14 +42,15 @@ static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); static void skeleton_rawdev_info_get(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info) + rte_rawdev_obj_t dev_info, + size_t dev_info_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; SKELETON_PMD_FUNC_TRACE(); - if (!dev_info) { + if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); return; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9ecfdee818..9b8390dfb7 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -106,12 +106,12 @@ test_rawdev_info_get(void) struct rte_rawdev_info rdev_info = {0}; struct skeleton_rawdev_conf skel_conf = {0}; - ret = rte_rawdev_info_get(test_dev_id, NULL); + ret = rte_rawdev_info_get(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); rdev_info.dev_private = &skel_conf; - ret = rte_rawdev_info_get(test_dev_id, &rdev_info); + ret = rte_rawdev_info_get(test_dev_id, &rdev_info, sizeof(skel_conf)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get raw dev info"); return TEST_SUCCESS; @@ -142,7 +142,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -170,7 +171,8 @@ test_rawdev_queue_default_conf_get(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -218,7 +220,8 @@ test_rawdev_queue_setup(void) /* Get the current configuration */ rdev_info.dev_private = &rdev_conf_get; ret = rte_rawdev_info_get(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -327,7 +330,8 @@ test_rawdev_start_stop(void) dummy_firmware = NULL; rte_rawdev_start(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); @@ -336,7 +340,8 @@ test_rawdev_start_stop(void) rdev_conf_get.device_state); rte_rawdev_stop(test_dev_id); - ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_info_get(test_dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_get)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain rawdev configuration (%d)", ret); diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index 75d8d5b9fe..76932d3dd2 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -757,7 +757,7 @@ assign_rawdevs(void) do { if (rdev_id == rte_rawdev_count()) goto end; - rte_rawdev_info_get(rdev_id++, &rdev_info); + rte_rawdev_info_get(rdev_id++, &rdev_info, 0); } while (rdev_info.driver_name == NULL || strcmp(rdev_info.driver_name, IOAT_PMD_RAWDEV_NAME_STR) != 0); diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index eba8ebf9fa..11e224451c 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1389,7 +1389,7 @@ main(int argc, char **argv) rte_rawdev_set_attr(dev_id, NTB_QUEUE_NUM_NAME, num_queues); printf("Set queue number as %u.\n", num_queues); ntb_rawdev_info.dev_private = (rte_rawdev_obj_t)(&ntb_info); - rte_rawdev_info_get(dev_id, &ntb_rawdev_info); + rte_rawdev_info_get(dev_id, &ntb_rawdev_info, sizeof(ntb_info)); nb_mbuf = nb_desc * num_queues * 2 * 2 + rte_lcore_count() * MEMPOOL_CACHE_SIZE; diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 8f84d0b228..a576890356 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -78,7 +78,8 @@ rte_rawdev_socket_id(uint16_t dev_id) } int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size) { struct rte_rawdev *rawdev; @@ -89,7 +90,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info) if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private); + (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + dev_private_size); } dev_info->driver_name = rawdev->driver_name; diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 32f6b8bb03..cf6acfd261 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -82,13 +82,20 @@ struct rte_rawdev_info; * will be returned. This can be used to safely query the type of a rawdev * instance without needing to know the size of the private data to return. * + * @param dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * returned, and may be checked by drivers to ensure the expected struct + * type is provided. + * * @return * - 0: Success, driver updates the contextual information of the raw device * - <0: Error code returned by the driver info get function. * */ int -rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info); +rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, + size_t dev_private_size); /** * Configure a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 4395a2182d..0e72a92058 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -138,12 +138,15 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * Raw device pointer * @param dev_info * Raw device information structure + * @param dev_private_size + * The size of the structure pointed to by dev_info->dev_private * * @return * Returns 0 on success */ typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, - rte_rawdev_obj_t dev_info); + rte_rawdev_obj_t dev_info, + size_t dev_private_size); /** * Configure a device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 2/7] rawdev: allow drivers to return error from info function 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 3/7] rawdev: add private data length parameter to config fn Bruce Richardson ` (5 subsequent siblings) 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Rosen Xu, Nipun Gupta, John McNamara, Marko Kovacevic, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Hemant Agrawal Since we now allow some parameter checking inside the driver info_get() functions, it makes sense to allow error return from those functions to the caller. Therefore we change the driver callback return type from void to int. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- v3: Add release note entry for change --- doc/guides/rel_notes/release_20_11.rst | 3 +++ drivers/raw/ifpga/ifpga_rawdev.c | 15 ++++++++------- drivers/raw/ioat/ioat_rawdev.c | 9 ++++++--- drivers/raw/ntb/ntb.c | 8 +++++--- drivers/raw/skeleton/skeleton_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 ++-- 7 files changed, 32 insertions(+), 19 deletions(-) diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index a931dc0bff..6431efe56b 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -88,6 +88,9 @@ API Changes ``rte_rawdev_info_get()``, allowing limited driver type-checking and ABI compatibility. +* rawdev: Changed the return type of the function ``rte_dev_info_get()`` + from ``void`` to ``int`` allowing the return of error codes from drivers. + ABI Changes ----------- diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 47cfa38778..32a2b96c90 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -603,7 +603,7 @@ ifpga_fill_afu_dev(struct opae_accelerator *acc, return 0; } -static void +static int ifpga_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) @@ -620,12 +620,12 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*afu_dev)) { IFPGA_RAWDEV_PMD_ERR("Invalid request"); - return; + return -EINVAL; } adapter = ifpga_rawdev_get_priv(dev); if (!adapter) - return; + return -ENOENT; afu_dev = dev_info; afu_dev->rawdev = dev; @@ -637,7 +637,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, if (ifpga_fill_afu_dev(acc, afu_dev)) { IFPGA_RAWDEV_PMD_ERR("cannot get info\n"); - return; + return -ENOENT; } } @@ -647,21 +647,21 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, /* get LineSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 0, &opae_lside_eth_info)) { - return; + return -ENOENT; } lside_bar_idx = opae_lside_eth_info.mem_idx; /* get NICSide BAR Index */ if (opae_manager_get_eth_group_region_info(mgr, 1, &opae_nside_eth_info)) { - return; + return -ENOENT; } nside_bar_idx = opae_nside_eth_info.mem_idx; if (lside_bar_idx >= PCI_MAX_RESOURCE || nside_bar_idx >= PCI_MAX_RESOURCE || lside_bar_idx == nside_bar_idx) - return; + return -ENOENT; /* fill LineSide BAR Index */ afu_dev->mem_resource[lside_bar_idx].phys_addr = @@ -679,6 +679,7 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, afu_dev->mem_resource[nside_bar_idx].addr = opae_nside_eth_info.addr; } + return 0; } static int diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index a5c0452d7e..e2fc787263 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -110,15 +110,18 @@ ioat_dev_stop(struct rte_rawdev *dev) RTE_SET_USED(dev); } -static void +static int ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { struct rte_ioat_rawdev_config *cfg = dev_info; struct rte_ioat_rawdev *ioat = dev->dev_private; - if (cfg != NULL && dev_info_size == sizeof(*cfg)) - cfg->ring_size = ioat->ring_size; + if (dev_info == NULL || dev_info_size != sizeof(*cfg)) + return -EINVAL; + + cfg->ring_size = ioat->ring_size; + return 0; } static const char * const xstat_names[] = { diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index c890c43a32..ced5b6d624 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -800,7 +800,7 @@ ntb_dequeue_bufs(struct rte_rawdev *dev, return nb_rx; } -static void +static int ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -809,7 +809,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (dev_info_size != sizeof(*info)) { NTB_LOG(ERR, "Invalid size parameter to %s", __func__); - return; + return -EINVAL; } info->mw_cnt = hw->mw_cnt; @@ -824,7 +824,7 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, if (!hw->queue_size || !hw->queue_pairs) { NTB_LOG(ERR, "No queue size and queue num assigned."); - return; + return -EAGAIN; } hw->hdr_size_per_queue = RTE_ALIGN(sizeof(struct ntb_header) + @@ -832,6 +832,8 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, hw->queue_size * sizeof(struct ntb_used), RTE_CACHE_LINE_SIZE); info->ntb_hdr_size = hw->hdr_size_per_queue * hw->queue_pairs; + + return 0; } static int diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dc05f3ecf8..dce300c358 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -41,7 +41,7 @@ struct queue_buffers { static struct queue_buffers queue_buf[SKELETON_MAX_QUEUES] = {}; static void clear_queue_bufs(int queue_id); -static void skeleton_rawdev_info_get(struct rte_rawdev *dev, +static int skeleton_rawdev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_info_size) { @@ -52,7 +52,7 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, if (!dev_info || dev_info_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid request"); - return; + return -EINVAL; } skeldev = skeleton_rawdev_get_priv(dev); @@ -63,6 +63,8 @@ static void skeleton_rawdev_info_get(struct rte_rawdev *dev, skeldev_conf->capabilities = skeldev->capabilities; skeldev_conf->device_state = skeldev->device_state; skeldev_conf->firmware_state = skeldev->fw.firmware_state; + + return 0; } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index a576890356..bde33763ee 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -82,6 +82,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, size_t dev_private_size) { struct rte_rawdev *rawdev; + int ret = 0; RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); RTE_FUNC_PTR_OR_ERR_RET(dev_info, -EINVAL); @@ -90,7 +91,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, if (dev_info->dev_private != NULL) { RTE_FUNC_PTR_OR_ERR_RET(*rawdev->dev_ops->dev_info_get, -ENOTSUP); - (*rawdev->dev_ops->dev_info_get)(rawdev, dev_info->dev_private, + ret = (*rawdev->dev_ops->dev_info_get)(rawdev, + dev_info->dev_private, dev_private_size); } @@ -98,7 +100,7 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, dev_info->device = rawdev->device; dev_info->socket_id = rawdev->socket_id; - return 0; + return ret; } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 0e72a92058..89e46412a3 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -142,9 +142,9 @@ rte_rawdev_pmd_is_valid_dev(uint8_t dev_id) * The size of the structure pointed to by dev_info->dev_private * * @return - * Returns 0 on success + * Returns 0 on success, negative error code on failure */ -typedef void (*rawdev_info_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, size_t dev_private_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 3/7] rawdev: add private data length parameter to config fn 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 2/7] rawdev: allow drivers to return error from info function Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson ` (4 subsequent siblings) 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Rosen Xu, Nipun Gupta, John McNamara, Marko Kovacevic, Satha Rao, Mahipal Challa, Tianfei zhang, Xiaoyun Li, Jingjing Wu, Hemant Agrawal Currently with the rawdev API there is no way to check that the structure passed in via the dev_private pointer in the structure passed to configure API is of the correct type - it's just checked that it is non-NULL. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Reviewed-by: Rosen Xu <rosen.xu@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- V3: fixed doxygen comment. add release note update --- doc/guides/rawdevs/ioat.rst | 2 +- doc/guides/rawdevs/octeontx2_dma.rst | 2 +- doc/guides/rawdevs/octeontx2_ep.rst | 3 ++- doc/guides/rel_notes/release_20_11.rst | 4 ++-- doc/guides/sample_app_ug/ioat.rst | 2 +- drivers/raw/ifpga/ifpga_rawdev.c | 3 ++- drivers/raw/ioat/ioat_rawdev.c | 5 +++-- drivers/raw/ioat/ioat_rawdev_test.c | 2 +- drivers/raw/ntb/ntb.c | 6 +++++- drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c | 7 ++++--- drivers/raw/octeontx2_dma/otx2_dpi_test.c | 3 ++- drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 7 ++++--- drivers/raw/octeontx2_ep/otx2_ep_test.c | 2 +- drivers/raw/skeleton/skeleton_rawdev.c | 5 +++-- drivers/raw/skeleton/skeleton_rawdev_test.c | 5 +++-- examples/ioat/ioatfwd.c | 2 +- examples/ntb/ntb_fwd.c | 2 +- lib/librte_rawdev/rte_rawdev.c | 6 ++++-- lib/librte_rawdev/rte_rawdev.h | 8 +++++++- lib/librte_rawdev/rte_rawdev_pmd.h | 3 ++- 20 files changed, 50 insertions(+), 29 deletions(-) diff --git a/doc/guides/rawdevs/ioat.rst b/doc/guides/rawdevs/ioat.rst index dac52fabf1..c46460ff45 100644 --- a/doc/guides/rawdevs/ioat.rst +++ b/doc/guides/rawdevs/ioat.rst @@ -142,7 +142,7 @@ The following code shows how the device is configured in /* ... */ p.ring_size = IOAT_TEST_RINGSIZE; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { printf("Error with rte_rawdev_configure()\n"); return -1; } diff --git a/doc/guides/rawdevs/octeontx2_dma.rst b/doc/guides/rawdevs/octeontx2_dma.rst index 1e1dfbe931..4a9794489e 100644 --- a/doc/guides/rawdevs/octeontx2_dma.rst +++ b/doc/guides/rawdevs/octeontx2_dma.rst @@ -92,7 +92,7 @@ The following code shows how the device is configured rte_mempool_set_ops_byname(conf.chunk_pool, rte_mbuf_platform_mempool_ops(), NULL); rte_mempool_populate_default(conf.chunk_pool); - rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info); + rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info, sizeof(conf)); Performing Data Transfer ------------------------ diff --git a/doc/guides/rawdevs/octeontx2_ep.rst b/doc/guides/rawdevs/octeontx2_ep.rst index bbcf530a45..b03fcf789a 100644 --- a/doc/guides/rawdevs/octeontx2_ep.rst +++ b/doc/guides/rawdevs/octeontx2_ep.rst @@ -66,7 +66,8 @@ The following code shows how the device is configured struct rte_rawdev_info rdev_info = {.dev_private = &config}; config.enqdeq_mpool = (void *)rte_mempool_create(...); - rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info); + rte_rawdev_configure(dev_id, (rte_rawdev_obj_t)&rdev_info, + sizeof(config)); Performing Data Transfer ------------------------ diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index 6431efe56b..9543e7e98b 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -84,8 +84,8 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= -* rawdev: Added a structure size parameter to the function - ``rte_rawdev_info_get()``, +* rawdev: Added a structure size parameter to the functions + ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, allowing limited driver type-checking and ABI compatibility. * rawdev: Changed the return type of the function ``rte_dev_info_get()`` diff --git a/doc/guides/sample_app_ug/ioat.rst b/doc/guides/sample_app_ug/ioat.rst index b5188220ba..3f7d5c34a6 100644 --- a/doc/guides/sample_app_ug/ioat.rst +++ b/doc/guides/sample_app_ug/ioat.rst @@ -296,7 +296,7 @@ is done in ``configure_rawdev_queue()``. struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; struct rte_rawdev_info info = { .dev_private = &dev_config }; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { rte_exit(EXIT_FAILURE, "Error with rte_rawdev_configure()\n"); } diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c index 32a2b96c90..a50173264c 100644 --- a/drivers/raw/ifpga/ifpga_rawdev.c +++ b/drivers/raw/ifpga/ifpga_rawdev.c @@ -684,7 +684,8 @@ ifpga_rawdev_info_get(struct rte_rawdev *dev, static int ifpga_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size __rte_unused) { IFPGA_RAWDEV_PMD_FUNC_TRACE(); diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c index e2fc787263..7f1a154360 100644 --- a/drivers/raw/ioat/ioat_rawdev.c +++ b/drivers/raw/ioat/ioat_rawdev.c @@ -39,7 +39,8 @@ RTE_LOG_REGISTER(ioat_pmd_logtype, rawdev.ioat, INFO); #define COMPLETION_SZ sizeof(__m128i) static int -ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct rte_ioat_rawdev_config *params = config; struct rte_ioat_rawdev *ioat = dev->dev_private; @@ -49,7 +50,7 @@ ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) if (dev->started) return -EBUSY; - if (params == NULL) + if (params == NULL || config_size != sizeof(*params)) return -EINVAL; if (params->ring_size > 4096 || params->ring_size < 64 || diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c index 2b40222eb4..c463a82ad6 100644 --- a/drivers/raw/ioat/ioat_rawdev_test.c +++ b/drivers/raw/ioat/ioat_rawdev_test.c @@ -156,7 +156,7 @@ ioat_rawdev_test(uint16_t dev_id) } p.ring_size = IOAT_TEST_RINGSIZE; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(p)) != 0) { printf("Error with rte_rawdev_configure()\n"); return -1; } diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index ced5b6d624..7f6b835a4b 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -837,13 +837,17 @@ ntb_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info, } static int -ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +ntb_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct ntb_dev_config *conf = config; struct ntb_hw *hw = dev->dev_private; uint32_t xstats_num; int ret; + if (conf == NULL || config_size != sizeof(*conf)) + return -EINVAL; + hw->queue_pairs = conf->num_queues; hw->queue_size = conf->queue_size; hw->used_mw_num = conf->mz_num; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c index e398abb75b..5b496446ca 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_rawdev.c @@ -294,7 +294,8 @@ otx2_dpi_rawdev_reset(struct rte_rawdev *dev) } static int -otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct dpi_rawdev_conf_s *conf = config; struct dpi_vf_s *dpivf = NULL; @@ -302,8 +303,8 @@ otx2_dpi_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) uintptr_t pool; uint32_t gaura; - if (conf == NULL) { - otx2_dpi_dbg("NULL configuration"); + if (conf == NULL || config_size != sizeof(*conf)) { + otx2_dpi_dbg("NULL or invalid configuration"); return -EINVAL; } dpivf = (struct dpi_vf_s *)dev->dev_private; diff --git a/drivers/raw/octeontx2_dma/otx2_dpi_test.c b/drivers/raw/octeontx2_dma/otx2_dpi_test.c index 276658af07..cec6ca91b0 100644 --- a/drivers/raw/octeontx2_dma/otx2_dpi_test.c +++ b/drivers/raw/octeontx2_dma/otx2_dpi_test.c @@ -182,7 +182,8 @@ test_otx2_dma_rawdev(uint16_t val) /* Configure rawdev ports */ conf.chunk_pool = dpi_create_mempool(); rdev_info.dev_private = &conf; - ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info); + ret = rte_rawdev_configure(i, (rte_rawdev_obj_t)&rdev_info, + sizeof(conf)); if (ret) { otx2_dpi_dbg("Unable to configure DPIVF %d", i); return -ENODEV; diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c index 0778603d5b..2b78a7941d 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c @@ -224,13 +224,14 @@ sdp_rawdev_close(struct rte_rawdev *dev) } static int -sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config) +sdp_rawdev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config, + size_t config_size) { struct sdp_rawdev_info *app_info = (struct sdp_rawdev_info *)config; struct sdp_device *sdpvf; - if (app_info == NULL) { - otx2_err("Application config info [NULL]"); + if (app_info == NULL || config_size != sizeof(*app_info)) { + otx2_err("Application config info [NULL] or incorrect size"); return -EINVAL; } diff --git a/drivers/raw/octeontx2_ep/otx2_ep_test.c b/drivers/raw/octeontx2_ep/otx2_ep_test.c index 091f1827c7..b876275f7a 100644 --- a/drivers/raw/octeontx2_ep/otx2_ep_test.c +++ b/drivers/raw/octeontx2_ep/otx2_ep_test.c @@ -108,7 +108,7 @@ sdp_rawdev_selftest(uint16_t dev_id) dev_info.dev_private = &app_info; - ret = rte_rawdev_configure(dev_id, &dev_info); + ret = rte_rawdev_configure(dev_id, &dev_info, sizeof(app_info)); if (ret) { otx2_err("Unable to configure SDP_VF %d", dev_id); rte_mempool_free(ioq_mpool); diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index dce300c358..531d0450c8 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -68,7 +68,8 @@ static int skeleton_rawdev_info_get(struct rte_rawdev *dev, } static int skeleton_rawdev_configure(const struct rte_rawdev *dev, - rte_rawdev_obj_t config) + rte_rawdev_obj_t config, + size_t config_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_conf *skeldev_conf; @@ -77,7 +78,7 @@ static int skeleton_rawdev_configure(const struct rte_rawdev *dev, RTE_FUNC_PTR_OR_ERR_RET(dev, -EINVAL); - if (!config) { + if (config == NULL || config_size != sizeof(*skeldev_conf)) { SKELETON_PMD_ERR("Invalid configuration"); return -EINVAL; } diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 9b8390dfb7..7dc7c76847 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -126,7 +126,7 @@ test_rawdev_configure(void) struct skeleton_rawdev_conf rdev_conf_get = {0}; /* Check invalid configuration */ - ret = rte_rawdev_configure(test_dev_id, NULL); + ret = rte_rawdev_configure(test_dev_id, NULL, 0); RTE_TEST_ASSERT(ret == -EINVAL, "Null configure; Expected -EINVAL, got %d", ret); @@ -137,7 +137,8 @@ test_rawdev_configure(void) rdev_info.dev_private = &rdev_conf_set; ret = rte_rawdev_configure(test_dev_id, - (rte_rawdev_obj_t)&rdev_info); + (rte_rawdev_obj_t)&rdev_info, + sizeof(rdev_conf_set)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to configure rawdev (%d)", ret); rdev_info.dev_private = &rdev_conf_get; diff --git a/examples/ioat/ioatfwd.c b/examples/ioat/ioatfwd.c index 76932d3dd2..43a19843ee 100644 --- a/examples/ioat/ioatfwd.c +++ b/examples/ioat/ioatfwd.c @@ -734,7 +734,7 @@ configure_rawdev_queue(uint32_t dev_id) struct rte_ioat_rawdev_config dev_config = { .ring_size = ring_size }; struct rte_rawdev_info info = { .dev_private = &dev_config }; - if (rte_rawdev_configure(dev_id, &info) != 0) { + if (rte_rawdev_configure(dev_id, &info, sizeof(dev_config)) != 0) { rte_exit(EXIT_FAILURE, "Error with rte_rawdev_configure()\n"); } diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 11e224451c..656f736594 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1401,7 +1401,7 @@ main(int argc, char **argv) ntb_conf.num_queues = num_queues; ntb_conf.queue_size = nb_desc; ntb_rawdev_conf.dev_private = (rte_rawdev_obj_t)(&ntb_conf); - ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf); + ret = rte_rawdev_configure(dev_id, &ntb_rawdev_conf, sizeof(ntb_conf)); if (ret) rte_exit(EXIT_FAILURE, "Can't config ntb dev: err=%d, " "port=%u\n", ret, dev_id); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index bde33763ee..6c4d783cc5 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -104,7 +104,8 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, } int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size) { struct rte_rawdev *dev; int diag; @@ -123,7 +124,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf) } /* Configure the device */ - diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private); + diag = (*dev->dev_ops->dev_configure)(dev, dev_conf->dev_private, + dev_private_size); if (diag != 0) RTE_RDEV_ERR("dev%d dev_configure = %d", dev_id, diag); else diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index cf6acfd261..1720f1dd6b 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -116,13 +116,19 @@ rte_rawdev_info_get(uint16_t dev_id, struct rte_rawdev_info *dev_info, * driver/implementation can use to configure the device. It is also assumed * that once the configuration is done, a `queue_id` type field can be used * to refer to some arbitrary internal representation of a queue. + * @param dev_private_size + * The length of the memory space pointed to by dev_private in dev_info. + * This should be set to the size of the expected private structure to be + * used by the driver, and may be checked by drivers to ensure the expected + * struct type is provided. * * @return * - 0: Success, device configured. * - <0: Error code returned by the driver configuration function. */ int -rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf); +rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, + size_t dev_private_size); /** diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 89e46412a3..050f8b0292 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -160,7 +160,8 @@ typedef int (*rawdev_info_get_t)(struct rte_rawdev *dev, * Returns 0 on success */ typedef int (*rawdev_configure_t)(const struct rte_rawdev *dev, - rte_rawdev_obj_t config); + rte_rawdev_obj_t config, + size_t config_size); /** * Start a configured device. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 4/7] rawdev: add private data length parameter to queue fns 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson ` (2 preceding siblings ...) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 3/7] rawdev: add private data length parameter to config fn Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 5/7] rawdev: allow queue config query to return error Bruce Richardson ` (3 subsequent siblings) 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Nipun Gupta, John McNamara, Marko Kovacevic, Xiaoyun Li, Jingjing Wu, Hemant Agrawal The queue setup and queue defaults query functions take a void * parameter as configuration data, preventing any compile-time checking of the parameters and limiting runtime checks. Adding in the length of the expected structure provides a measure of typechecking, and can also be used for ABI compatibility in future, since ABI changes involving structs almost always involve a change in size. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- V3: Added release note entry --- doc/guides/rel_notes/release_20_11.rst | 1 + drivers/raw/ntb/ntb.c | 25 ++++++++++++++++----- drivers/raw/skeleton/skeleton_rawdev.c | 12 ++++++---- drivers/raw/skeleton/skeleton_rawdev_test.c | 8 +++---- examples/ntb/ntb_fwd.c | 3 ++- lib/librte_rawdev/rte_rawdev.c | 12 ++++++---- lib/librte_rawdev/rte_rawdev.h | 10 +++++++-- lib/librte_rawdev/rte_rawdev_pmd.h | 6 +++-- 8 files changed, 54 insertions(+), 23 deletions(-) diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index 9543e7e98b..e79b88641b 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -85,6 +85,7 @@ API Changes ======================================================= * rawdev: Added a structure size parameter to the functions + ``rte_rawdev_queue_setup()``, ``rte_rawdev_queue_conf_get()``, ``rte_rawdev_info_get()`` and ``rte_rawdev_configure()``, allowing limited driver type-checking and ABI compatibility. diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index 7f6b835a4b..ad10857991 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -249,11 +249,15 @@ ntb_dev_intr_handler(void *param) static void ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *q_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; + if (conf_size != sizeof(*q_conf)) + return; + q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; @@ -294,12 +298,16 @@ ntb_rxq_release(struct ntb_rx_queue *rxq) static int ntb_rxq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *rxq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_rx_queue *rxq; + if (conf_size != sizeof(*rxq_conf)) + return -EINVAL; + /* Allocate the rx queue data structure */ rxq = rte_zmalloc_socket("ntb rx queue", sizeof(struct ntb_rx_queue), @@ -375,13 +383,17 @@ ntb_txq_release(struct ntb_tx_queue *txq) static int ntb_txq_setup(struct rte_rawdev *dev, uint16_t qp_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_queue_conf *txq_conf = queue_conf; struct ntb_hw *hw = dev->dev_private; struct ntb_tx_queue *txq; uint16_t i, prev; + if (conf_size != sizeof(*txq_conf)) + return -EINVAL; + /* Allocate the TX queue data structure. */ txq = rte_zmalloc_socket("ntb tx queue", sizeof(struct ntb_tx_queue), @@ -439,7 +451,8 @@ ntb_txq_setup(struct rte_rawdev *dev, static int ntb_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct ntb_hw *hw = dev->dev_private; int ret; @@ -447,11 +460,11 @@ ntb_queue_setup(struct rte_rawdev *dev, if (queue_id >= hw->queue_pairs) return -EINVAL; - ret = ntb_txq_setup(dev, queue_id, queue_conf); + ret = ntb_txq_setup(dev, queue_id, queue_conf, conf_size); if (ret < 0) return ret; - ret = ntb_rxq_setup(dev, queue_id, queue_conf); + ret = ntb_rxq_setup(dev, queue_id, queue_conf, conf_size); return ret; } diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 531d0450c8..801c707fb9 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -222,14 +222,16 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || + conf_size != sizeof(struct skeleton_rawdev_queue)) return; skeldev = skeleton_rawdev_get_priv(dev); @@ -252,7 +254,8 @@ clear_queue_bufs(int queue_id) static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t conf_size) { int ret = 0; struct skeleton_rawdev *skeldev; @@ -260,7 +263,8 @@ static int skeleton_rawdev_queue_setup(struct rte_rawdev *dev, SKELETON_PMD_FUNC_TRACE(); - if (!dev || !queue_conf) + if (!dev || !queue_conf || + conf_size != sizeof(struct skeleton_rawdev_queue)) return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c index 7dc7c76847..bb4b6efe40 100644 --- a/drivers/raw/skeleton/skeleton_rawdev_test.c +++ b/drivers/raw/skeleton/skeleton_rawdev_test.c @@ -185,7 +185,7 @@ test_rawdev_queue_default_conf_get(void) * depth = DEF_DEPTH */ for (i = 0; i < rdev_conf_get.num_queues; i++) { - rte_rawdev_queue_conf_get(test_dev_id, i, &q); + rte_rawdev_queue_conf_get(test_dev_id, i, &q, sizeof(q)); RTE_TEST_ASSERT_EQUAL(q.depth, SKELETON_QUEUE_DEF_DEPTH, "Invalid default depth of queue (%d)", q.depth); @@ -235,11 +235,11 @@ test_rawdev_queue_setup(void) /* Modify the queue depth for Queue 0 and attach it */ qset.depth = 15; qset.state = SKELETON_QUEUE_ATTACH; - ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset); + ret = rte_rawdev_queue_setup(test_dev_id, 0, &qset, sizeof(qset)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to setup queue (%d)", ret); /* Now, fetching the queue 0 should show depth as 15 */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qset.depth, qget.depth, @@ -263,7 +263,7 @@ test_rawdev_queue_release(void) RTE_TEST_ASSERT_SUCCESS(ret, "Failed to release queue 0; (%d)", ret); /* Now, fetching the queue 0 should show depth as default */ - ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget); + ret = rte_rawdev_queue_conf_get(test_dev_id, 0, &qget, sizeof(qget)); RTE_TEST_ASSERT_SUCCESS(ret, "Failed to get queue config (%d)", ret); RTE_TEST_ASSERT_EQUAL(qget.depth, SKELETON_QUEUE_DEF_DEPTH, diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c index 656f736594..5a8439b8d3 100644 --- a/examples/ntb/ntb_fwd.c +++ b/examples/ntb/ntb_fwd.c @@ -1411,7 +1411,8 @@ main(int argc, char **argv) ntb_q_conf.rx_mp = mbuf_pool; for (i = 0; i < num_queues; i++) { /* Setup rawdev queue */ - ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf); + ret = rte_rawdev_queue_setup(dev_id, i, &ntb_q_conf, + sizeof(ntb_q_conf)); if (ret < 0) rte_exit(EXIT_FAILURE, "Failed to setup ntb queue %u.\n", i); diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index 6c4d783cc5..f48d8cf10b 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -137,7 +137,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -145,14 +146,16 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf); + (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, + queue_conf_size); return 0; } int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf) + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size) { struct rte_rawdev *dev; @@ -160,7 +163,8 @@ rte_rawdev_queue_setup(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_setup, -ENOTSUP); - return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf); + return (*dev->dev_ops->queue_setup)(dev, queue_id, queue_conf, + queue_conf_size); } int diff --git a/lib/librte_rawdev/rte_rawdev.h b/lib/librte_rawdev/rte_rawdev.h index 1720f1dd6b..14e8eb972f 100644 --- a/lib/librte_rawdev/rte_rawdev.h +++ b/lib/librte_rawdev/rte_rawdev.h @@ -146,6 +146,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, * previously supplied to rte_rawdev_configure(). * @param[out] queue_conf * The pointer to the default raw queue configuration data. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * @return * - 0: Success, driver updates the default raw queue configuration data. * - <0: Error code returned by the driver info get function. @@ -156,7 +158,8 @@ rte_rawdev_configure(uint16_t dev_id, struct rte_rawdev_info *dev_conf, int rte_rawdev_queue_conf_get(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Allocate and set up a raw queue for a raw device. @@ -169,6 +172,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, * @param queue_conf * The pointer to the configuration data to be used for the raw queue. * NULL value is allowed, in which case default configuration used. + * @param queue_conf_size + * The size of the structure pointed to by queue_conf * * @see rte_rawdev_queue_conf_get() * @@ -179,7 +184,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, int rte_rawdev_queue_setup(uint16_t dev_id, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release and deallocate a raw queue from a raw device. diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 050f8b0292..34eb667f62 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -218,7 +218,8 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); */ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Setup an raw queue. @@ -235,7 +236,8 @@ typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, */ typedef int (*rawdev_queue_setup_t)(struct rte_rawdev *dev, uint16_t queue_id, - rte_rawdev_obj_t queue_conf); + rte_rawdev_obj_t queue_conf, + size_t queue_conf_size); /** * Release resources allocated by given raw queue. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 5/7] rawdev: allow queue config query to return error 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson ` (3 preceding siblings ...) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 6/7] rawdev: mark start and stop functions optional Bruce Richardson ` (2 subsequent siblings) 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Nipun Gupta, John McNamara, Marko Kovacevic, Xiaoyun Li, Jingjing Wu, Hemant Agrawal The driver APIs for returning the queue default config can fail if the parameters are invalid, or other reasons, so allow them to return error codes to the rawdev layer and from hence to the app. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- v3: add release note update --- doc/guides/rel_notes/release_20_11.rst | 1 + drivers/raw/ntb/ntb.c | 6 ++++-- drivers/raw/skeleton/skeleton_rawdev.c | 12 +++++++----- lib/librte_rawdev/rte_rawdev.c | 3 +-- lib/librte_rawdev/rte_rawdev_pmd.h | 4 +++- 5 files changed, 16 insertions(+), 10 deletions(-) diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst index e79b88641b..667e3d54ad 100644 --- a/doc/guides/rel_notes/release_20_11.rst +++ b/doc/guides/rel_notes/release_20_11.rst @@ -90,6 +90,7 @@ API Changes allowing limited driver type-checking and ABI compatibility. * rawdev: Changed the return type of the function ``rte_dev_info_get()`` + and the function ``rte_rawdev_queue_conf_get()`` from ``void`` to ``int`` allowing the return of error codes from drivers. diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index ad10857991..bddb1f5916 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -246,7 +246,7 @@ ntb_dev_intr_handler(void *param) } } -static void +static int ntb_queue_conf_get(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, @@ -256,11 +256,13 @@ ntb_queue_conf_get(struct rte_rawdev *dev, struct ntb_hw *hw = dev->dev_private; if (conf_size != sizeof(*q_conf)) - return; + return -EINVAL; q_conf->tx_free_thresh = hw->tx_queues[queue_id]->tx_free_thresh; q_conf->nb_desc = hw->rx_queues[queue_id]->nb_rx_desc; q_conf->rx_mp = hw->rx_queues[queue_id]->mpool; + + return 0; } static void diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c index 801c707fb9..f8b47a391a 100644 --- a/drivers/raw/skeleton/skeleton_rawdev.c +++ b/drivers/raw/skeleton/skeleton_rawdev.c @@ -220,10 +220,10 @@ static int skeleton_rawdev_reset(struct rte_rawdev *dev) return 0; } -static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, - uint16_t queue_id, - rte_rawdev_obj_t queue_conf, - size_t conf_size) +static int skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, + uint16_t queue_id, + rte_rawdev_obj_t queue_conf, + size_t conf_size) { struct skeleton_rawdev *skeldev; struct skeleton_rawdev_queue *skelq; @@ -232,7 +232,7 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, if (!dev || !queue_conf || conf_size != sizeof(struct skeleton_rawdev_queue)) - return; + return -EINVAL; skeldev = skeleton_rawdev_get_priv(dev); skelq = &skeldev->queues[queue_id]; @@ -240,6 +240,8 @@ static void skeleton_rawdev_queue_def_conf(struct rte_rawdev *dev, if (queue_id < SKELETON_MAX_QUEUES) rte_memcpy(queue_conf, skelq, sizeof(struct skeleton_rawdev_queue)); + + return 0; } static void diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index f48d8cf10b..fd123bb848 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -146,9 +146,8 @@ rte_rawdev_queue_conf_get(uint16_t dev_id, dev = &rte_rawdevs[dev_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf, -ENOTSUP); - (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, + return (*dev->dev_ops->queue_def_conf)(dev, queue_id, queue_conf, queue_conf_size); - return 0; } int diff --git a/lib/librte_rawdev/rte_rawdev_pmd.h b/lib/librte_rawdev/rte_rawdev_pmd.h index 34eb667f62..34dd7181b4 100644 --- a/lib/librte_rawdev/rte_rawdev_pmd.h +++ b/lib/librte_rawdev/rte_rawdev_pmd.h @@ -215,8 +215,10 @@ typedef int (*rawdev_reset_t)(struct rte_rawdev *dev); * @param[out] queue_conf * Raw device queue configuration structure * + * @return + * Returns 0 on success, negative errno on failure */ -typedef void (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, +typedef int (*rawdev_queue_conf_get_t)(struct rte_rawdev *dev, uint16_t queue_id, rte_rawdev_obj_t queue_conf, size_t queue_conf_size); -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 6/7] rawdev: mark start and stop functions optional 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson ` (4 preceding siblings ...) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 5/7] rawdev: allow queue config query to return error Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 7/7] doc: remove rawdev deprecation notice Bruce Richardson 2020-09-11 9:56 ` [dpdk-dev] [PATCH v3 0/7] Enhance rawdev APIs Thomas Monjalon 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev; +Cc: thomas, Bruce Richardson, Kevin Laatz, Nipun Gupta, Hemant Agrawal Not all rawdevs will require a device start/stop function, so rather than requiring such drivers to provide dummy functions, just set the started/stopped rawdev flag from the rawdev layer and return success. Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- lib/librte_rawdev/rte_rawdev.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/librte_rawdev/rte_rawdev.c b/lib/librte_rawdev/rte_rawdev.c index fd123bb848..36f3acf2b7 100644 --- a/lib/librte_rawdev/rte_rawdev.c +++ b/lib/librte_rawdev/rte_rawdev.c @@ -398,20 +398,21 @@ rte_rawdev_start(uint16_t dev_id) RTE_RAWDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); dev = &rte_rawdevs[dev_id]; - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_start, -ENOTSUP); - if (dev->started != 0) { RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already started", dev_id); return 0; } + if (dev->dev_ops->dev_start == NULL) + goto mark_started; + diag = (*dev->dev_ops->dev_start)(dev); - if (diag == 0) - dev->started = 1; - else + if (diag != 0) return diag; +mark_started: + dev->started = 1; return 0; } @@ -425,15 +426,18 @@ rte_rawdev_stop(uint16_t dev_id) RTE_RAWDEV_VALID_DEVID_OR_RET(dev_id); dev = &rte_rawdevs[dev_id]; - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_stop); - if (dev->started == 0) { RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already stopped", dev_id); return; } + if (dev->dev_ops->dev_stop == NULL) + goto mark_stopped; + (*dev->dev_ops->dev_stop)(dev); + +mark_stopped: dev->started = 0; } -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [dpdk-dev] [PATCH v3 7/7] doc: remove rawdev deprecation notice 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson ` (5 preceding siblings ...) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 6/7] rawdev: mark start and stop functions optional Bruce Richardson @ 2020-09-10 14:36 ` Bruce Richardson 2020-09-11 9:56 ` [dpdk-dev] [PATCH v3 0/7] Enhance rawdev APIs Thomas Monjalon 7 siblings, 0 replies; 30+ messages in thread From: Bruce Richardson @ 2020-09-10 14:36 UTC (permalink / raw) To: dev Cc: thomas, Bruce Richardson, Nipun Gupta, Ray Kinsella, Neil Horman, John McNamara, Marko Kovacevic Since all changes covered by the deprecation notice have been applied, the notice can be removed. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Nipun Gupta <nipun.gupta@nxp.com> --- doc/guides/rel_notes/deprecation.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index 279eccb04a..88d7d07613 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -294,13 +294,6 @@ Deprecation Notices - https://patches.dpdk.org/patch/71457/ - https://patches.dpdk.org/patch/71456/ -* rawdev: The rawdev APIs which take a device-specific structure as - parameter directly, or indirectly via a "private" pointer inside another - structure, will be modified to take an additional parameter of the - structure size. The affected APIs will include ``rte_rawdev_info_get``, - ``rte_rawdev_configure``, ``rte_rawdev_queue_conf_get`` and - ``rte_rawdev_queue_setup``. - * acl: ``RTE_ACL_CLASSIFY_NUM`` enum value will be removed. This enum value is not used inside DPDK, while it prevents to add new classify algorithms without causing an ABI breakage. -- 2.25.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [dpdk-dev] [PATCH v3 0/7] Enhance rawdev APIs 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson ` (6 preceding siblings ...) 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 7/7] doc: remove rawdev deprecation notice Bruce Richardson @ 2020-09-11 9:56 ` Thomas Monjalon 7 siblings, 0 replies; 30+ messages in thread From: Thomas Monjalon @ 2020-09-11 9:56 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev Bruce Richardson <bruce.richardson@intel.com> wrote: > This patchset proposes some internal and externally-visible changes to the > rawdev API, the ABI change of which were previously announced. > > The changes are in two main areas: > * For any APIs which take a void * parameter for driver-specific structs, > > add an additional parameter to provide the struct length. This allows > some runtime type-checking, as well as possible ABI-compatibility support > in the future as structure change generally involve a change in the size > of the structure. > > * Ensure all APIs which can return error values have int type, rather than > > void. Since functions like info_get and queue_default_get can now do some > typechecking, they need to be modified to allow them to return error > codes on failure. > > V3: > - fix doxygen error > - add release note update for changes to public APIs > > V2: > - add additional patch to make start/stop functions optional > - remove deprecation notice once changes applied Applied, thanks ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2020-09-11 9:56 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-09 15:20 [dpdk-dev] [PATCH 20.11 0/5] Enhance rawdev APIs Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 1/5] rawdev: add private data length parameter to info fn Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 2/5] rawdev: allow drivers to return error from info function Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 3/5] rawdev: add private data length parameter to config fn Bruce Richardson 2020-07-12 14:13 ` Xu, Rosen 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 4/5] rawdev: add private data length parameter to queue fns Bruce Richardson 2020-07-09 15:20 ` [dpdk-dev] [PATCH 20.11 5/5] rawdev: allow queue default config query to return error Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Bruce Richardson 2020-08-13 11:27 ` [dpdk-dev] [PATCH v2 1/7] rawdev: add private data length parameter to info fn Bruce Richardson 2020-08-17 5:26 ` Xu, Rosen 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 2/7] rawdev: allow drivers to return error from info function Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 3/7] rawdev: add private data length parameter to config fn Bruce Richardson 2020-09-04 14:16 ` Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 5/7] rawdev: allow queue default config query to return error Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 6/7] rawdev: mark start and stop functions optional Bruce Richardson 2020-08-13 11:28 ` [dpdk-dev] [PATCH v2 7/7] doc: remove rawdev deprecation notice Bruce Richardson 2020-09-02 11:21 ` [dpdk-dev] [PATCH v2 0/7] Enhance rawdev APIs Nipun Gupta 2020-09-09 9:47 ` Thomas Monjalon 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 " Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 2/7] rawdev: allow drivers to return error from info function Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 3/7] rawdev: add private data length parameter to config fn Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 4/7] rawdev: add private data length parameter to queue fns Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 5/7] rawdev: allow queue config query to return error Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 6/7] rawdev: mark start and stop functions optional Bruce Richardson 2020-09-10 14:36 ` [dpdk-dev] [PATCH v3 7/7] doc: remove rawdev deprecation notice Bruce Richardson 2020-09-11 9:56 ` [dpdk-dev] [PATCH v3 0/7] Enhance rawdev APIs Thomas Monjalon
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).