From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B7D2AA04B5; Thu, 10 Sep 2020 16:36:57 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 960E31C0CC; Thu, 10 Sep 2020 16:36:57 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 9842B1BF8D for ; Thu, 10 Sep 2020 16:36:55 +0200 (CEST) IronPort-SDR: ApMWtN3lZM11jWSUaiK4h/rgd70gGe2hl0NQSsiZLvPxzG8CL2CltBkZ8Z4QRt+2X2xrA3STvn sUhI/G+KaDcg== X-IronPort-AV: E=McAfee;i="6000,8403,9739"; a="138573545" X-IronPort-AV: E=Sophos;i="5.76,413,1592895600"; d="scan'208";a="138573545" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Sep 2020 07:36:54 -0700 IronPort-SDR: dtnG3qKNNqx918njRqz6D7r7xljTZQTt1c6YM7sZus0DXijxmjxH0boKDj+64Waq3ilZ0+Ddzs MkN0Mvyt/D4A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.76,413,1592895600"; d="scan'208";a="304900308" Received: from silpixa00399126.ir.intel.com ([10.237.222.27]) by orsmga006.jf.intel.com with ESMTP; 10 Sep 2020 07:36:50 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: thomas@monjalon.net, Bruce Richardson , Rosen Xu , Nipun Gupta , Hemant Agrawal , John McNamara , Marko Kovacevic , Tianfei zhang , Xiaoyun Li , Jingjing Wu Date: Thu, 10 Sep 2020 15:36:03 +0100 Message-Id: <20200910143609.986696-2-bruce.richardson@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200910143609.986696-1-bruce.richardson@intel.com> References: <20200709152047.167730-1-bruce.richardson@intel.com> <20200910143609.986696-1-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v3 1/7] rawdev: add private data length parameter to info fn X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Reviewed-by: Rosen Xu Acked-by: Rosen Xu Acked-by: Nipun Gupta --- 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