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 43EB8A04B5; Thu, 10 Sep 2020 16:37:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B600B1C10C; Thu, 10 Sep 2020 16:37:15 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 25D331C0B9 for ; Thu, 10 Sep 2020 16:37:11 +0200 (CEST) IronPort-SDR: 91/rekOiDmg7OWIivtc3UGcchyMUO+S5I67ozBOPilgVGi6Vox0ndmj5xZVqRbvZNlHBTgz7Ch fN9ayPIq/kqQ== X-IronPort-AV: E=McAfee;i="6000,8403,9739"; a="146263387" X-IronPort-AV: E=Sophos;i="5.76,413,1592895600"; d="scan'208";a="146263387" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Sep 2020 07:37:05 -0700 IronPort-SDR: f2GQE7WZ2ypy5o/doORRfVkyQZCGfd6KKrlyQlD0Az8BXIz+0EBoE03BsBhCW0ZW9UEmZuutiF 3gX1gffu9ubg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.76,413,1592895600"; d="scan'208";a="304900363" Received: from silpixa00399126.ir.intel.com ([10.237.222.27]) by orsmga006.jf.intel.com with ESMTP; 10 Sep 2020 07:36:59 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: thomas@monjalon.net, Bruce Richardson , Rosen Xu , Nipun Gupta , John McNamara , Marko Kovacevic , Satha Rao , Mahipal Challa , Tianfei zhang , Xiaoyun Li , Jingjing Wu , Hemant Agrawal Date: Thu, 10 Sep 2020 15:36:05 +0100 Message-Id: <20200910143609.986696-4-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 3/7] rawdev: add private data length parameter to config 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 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 Reviewed-by: Rosen Xu Acked-by: Nipun Gupta --- 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