rawdev: add private data size to config inputs

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>
This commit is contained in:
Bruce Richardson 2020-09-10 15:36:05 +01:00 committed by Thomas Monjalon
parent f150dd8839
commit 8db9dce72d
20 changed files with 50 additions and 29 deletions

View File

@ -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;
}

View File

@ -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
------------------------

View File

@ -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
------------------------

View File

@ -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()``

View File

@ -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");
}

View File

@ -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();

View File

@ -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 ||

View File

@ -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;
}

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;
}

View File

@ -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);

View File

@ -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;
}

View File

@ -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;

View File

@ -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");
}

View File

@ -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);

View File

@ -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

View File

@ -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);
/**

View File

@ -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.