* Background on rte_eth_representor_info_get()
rte_eth_representor_info_get() experimental function was added
to ethdev public API in 21.05 DPDK release [1].
Its purpose is to provide representor IDs in form of ID ranges.
Driver can return the following example info:
info = (struct rte_eth_representor_info){
.controller = 0, /* Controller ID of caller device. */
.pf = 0, /* Physical function ID of caller device. */
.nb_ranges = 2,
.ranges = {
{
.type = RTE_ETH_REPRESENTOR_VF,
.controller = 0,
.pf = 0,
.vf = 0,
.id_base = 100,
.id_end = 199,
.name = "pfvf"
},
{
.type = RTE_ETH_REPRESENTOR_SF,
.controller = 0,
.pf = 0,
.sf = 0,
.id_base = 200,
.id_end = 299,
.name = "pfsf"
},
}
};
This roughly means that, representors on PF 0 will have IDs:
- PF0VF0 -> ID == 100
- PF0VF1 -> ID == 101
- ...
- PF0VF99 -> ID == 199
- PF0SF0 -> ID == 200
- PF0SF1 -> ID == 201
- ...
- PF0SF99 -> ID == 299
All representors are related to controller with ID equal to 0.
Since the introduction, this API was implemented by 3 drivers:
- mlx5
- sfc
- cnxk
and resulting implementations suggest that some clean up is needed
to address the following issues.
** Issue #1: All drivers return degenerate ranges
All 3 drivers implementing rte_eth_representor_info_get() return
degenerate ranges i.e.:
- one struct rte_eth_representor_range is returned per representor
- each has id_base == id_end
Due to this, current range format seems to be an overkill.
** Issue #2: Representor ID is not directly usable by DPDK users
rte_eth_representor_info_get() is part of the public ethdev API.
Applications can, for example, use it to answer a question:
What representor ID will have a representor of VF 3 on PF 0?
The problem is that applications have no clear way to correlate
an ethdev port with a representor ID.
Representor ID itself is stored in rte_eth_dev_data.representor_id,
but this field is not exposed to the user.
This field is used internally in ethdev to implement
iteration over matching representors. This is exposed to applications
through rte_eth_iterator and related macros.
Drivers implementing rte_eth_representor_info_get()
reuse the same representor ID in rte_eth_switch_info.port_id.
However whether these values should be treated the same way,
is not part of the API contract.
Regardless of that, representor ID alone does not bring any useful
information to the user.
* Proposal
To address the 2 issues above, I would like to propose simplification of
rte_eth_representor_info_get():
- Function should return information about
what a given ethdev port represents:
- representor type
- PCI controller index
- PF index
- VF/SF index (for VF/SF type)
- Function will return information only for provided port.
- If driver does not provide representor_info_get() in its dev_ops,
then ethdev library will handle it for the driver in the following way:
# for representors
type = RTE_ETH_REPRESENTOR_VF
controller_valid = false
PF_valid = false
VF = dev_info.switch_info.port_id
# for non-representors
type = RTE_ETH_REPRESENTOR_NONE
controller_valid = false
PF_valid = false
This behavior would be consistent with how
rte_eth_representor_id_get() calculates representor_id for each driver.
- Otherwise, driver is responsible for filling representor info.
As a result rte_eth_representor_info_get() would be implemented
for all drivers.
Iteration over representors can be implemented
in terms of this new implementation by:
- iterating over ethdev ports with matching rte_device
- for ethdev ports which are representors
- convert devargs to rte_eth_representor_info output
- compare converted info with rte_eth_representor_info_get() output for port
Comparison mentioned above is provided by internal
rte_eth_representor_info_match() function provided in this RFC.
Changes in eth_representor_cmp() showcase rough algorithm of that new logic.
This change would also deprecate rte_eth_representor_id_get() internal function.
[1]:
https://inbox.dpdk.org/dev/[email protected]/
Signed-off-by: Dariusz Sosnowski <[email protected]>
---
lib/ethdev/ethdev_driver.c | 89 ++++++++++++++++++++++++++++++++++++++
lib/ethdev/ethdev_driver.h | 4 ++
lib/ethdev/rte_class_eth.c | 35 ++++++++++-----
lib/ethdev/rte_ethdev.c | 32 ++++++++++++--
lib/ethdev/rte_ethdev.h | 47 +++++---------------
5 files changed, 157 insertions(+), 50 deletions(-)
diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index eab5c15d12..514c8fa29d 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -902,6 +902,95 @@ rte_eth_recycle_rx_descriptors_refill_dummy(void *queue
__rte_unused,
/* No action. */
}
+static bool controller_match(const struct rte_eth_representor_info *rep_info,
+ const struct rte_eth_representor_info *backer_info,
+ const struct rte_eth_representor_info *other)
+{
+ /* If device has controller, but devargs do not have one, then use
backer's controller as default. */
+ if (rep_info->controller_valid && !other->controller_valid) {
+ if (rep_info->controller != backer_info->controller)
+ return false;
+ }
+
+ /* If devargs have controller, but device does not have one, then no
way to compare. */
+ if (!rep_info->controller_valid && other->controller_valid)
+ return false;
+
+ /* If both device and devargs have controller, then they must match. */
+ if (rep_info->controller_valid && other->controller_valid &&
rep_info->controller != other->controller)
+ return false;
+
+ /* If both device and devargs do not have controller, then no need to
compare. */
+
+ return true;
+}
+
+static bool pf_match(const struct rte_eth_representor_info *rep_info,
+ const struct rte_eth_representor_info *backer_info,
+ const struct rte_eth_representor_info *other)
+{
+ /* If device has pf, but devargs do not have one, then use backer's pf
as default. */
+ if (rep_info->pf_valid && !other->pf_valid) {
+ if (rep_info->pf != backer_info->pf)
+ return false;
+ }
+
+ /* If devargs have pf, but device does not have one, then no way to
compare. */
+ if (!rep_info->pf_valid && other->pf_valid)
+ return false;
+
+ /* If both device and devargs have pf, then they must match. */
+ if (rep_info->pf_valid && other->pf_valid && rep_info->pf != other->pf)
+ return false;
+
+ /* If both device and devargs do not have pf, then no need to compare.
*/
+
+ return true;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_representor_id_get)
+bool
+rte_eth_representor_info_match(const struct rte_eth_dev *dev, const struct
rte_eth_representor_info *other)
+{
+ struct rte_eth_representor_info info;
+ struct rte_eth_representor_info backer_info;
+ int ret;
+
+ memset(&info, 0, sizeof(info));
+ memset(&backer_info, 0, sizeof(backer_info));
+
+ ret = rte_eth_representor_info_get(dev->data->port_id, &info);
+ if (ret < 0)
+ return false;
+ ret = rte_eth_representor_info_get(dev->data->backer_port_id,
&backer_info);
+ if (ret < 0)
+ return false;
+
+ if (!controller_match(&info, &backer_info, other))
+ return false;
+
+ if (!pf_match(&info, &backer_info, other))
+ return false;
+
+ if (info.type != other->type)
+ return false;
+
+ switch (info.type) {
+ case RTE_ETH_REPRESENTOR_VF:
+ if (info.vf != other->vf)
+ return false;
+ break;
+ case RTE_ETH_REPRESENTOR_SF:
+ if (info.sf != other->sf)
+ return false;
+ break;
+ default:
+ break;
+ }
+
+ return true;
+}
+
RTE_EXPORT_INTERNAL_SYMBOL(rte_eth_representor_id_get)
int
rte_eth_representor_id_get(uint16_t port_id,
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 0f336f9567..349b8bbc71 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -2045,6 +2045,10 @@ struct rte_eth_devargs {
enum rte_eth_representor_type type; /* type of representor */
};
+__rte_internal
+bool
+rte_eth_representor_info_match(const struct rte_eth_dev *dev, const struct
rte_eth_representor_info *other);
+
/**
* PMD helper function to get representor ID from location detail.
*
diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c
index a8d01e2595..8235ac1d28 100644
--- a/lib/ethdev/rte_class_eth.c
+++ b/lib/ethdev/rte_class_eth.c
@@ -93,19 +93,34 @@ eth_representor_cmp(const char *key __rte_unused,
/* Return 0 if representor ID is matching one of the values. */
for (i = 0; i < nc * np * nf; ++i) {
+ struct rte_eth_representor_info info;
+
c = i / (np * nf);
p = (i / nf) % np;
f = i % nf;
- if (rte_eth_representor_id_get(edev->data->backer_port_id,
- eth_da.type,
- eth_da.nb_mh_controllers == 0 ? -1 :
- eth_da.mh_controllers[c],
- eth_da.nb_ports == 0 ? -1 : eth_da.ports[p],
- eth_da.nb_representor_ports == 0 ? -1 :
- eth_da.representor_ports[f],
- &id) < 0)
- continue;
- if (data->representor_id == id)
+
+ memset(&info, 0, sizeof(info));
+ info.type = eth_da.type;
+ if (eth_da.nb_mh_controllers > 0) {
+ info.controller = eth_da.mh_controllers[c];
+ info.controller_valid = true;
+ }
+ if (eth_da.nb_ports > 0) {
+ info.pf = eth_da.ports[p];
+ info.pf = true;
+ }
+ switch (info.type) {
+ case RTE_ETH_REPRESENTOR_VF:
+ info.vf = eth_da.representor_ports[f];
+ break;
+ case RTE_ETH_REPRESENTOR_SF:
+ info.sf = eth_da.representor_ports[f];
+ break;
+ default:
+ break;
+ }
+
+ if (rte_eth_representor_info_match(edev, &info))
return 0;
}
return -1; /* no match */
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9efeaf77cb..5ec08810fd 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -7155,17 +7155,41 @@ rte_eth_representor_info_get(uint16_t port_id,
struct rte_eth_representor_info *info)
{
struct rte_eth_dev *dev;
- int ret;
+ int ret = 0;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
- if (dev->dev_ops->representor_info_get == NULL)
- return -ENOTSUP;
+ if (info == NULL)
+ return -EINVAL;
+
+ if (dev->dev_ops->representor_info_get == NULL) {
+ if (!rte_eth_dev_is_repr(dev)) {
+ info->type = RTE_ETH_REPRESENTOR_NONE;
+ } else {
+ struct rte_eth_dev_info dev_info = {};
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ if (ret != 0) {
+ RTE_ETHDEV_LOG_LINE(ERR,
"rte_eth_dev_info_get() failed for port %u",
+ port_id);
+ goto finish;
+ }
+
+ info->type = RTE_ETH_REPRESENTOR_VF;
+ info->vf = dev_info.switch_info.port_id;
+ }
+
+ info->controller_valid = false;
+ info->pf_valid = false;
+
+ goto finish;
+ }
+
ret = eth_err(port_id, dev->dev_ops->representor_info_get(dev, info));
+finish:
rte_eth_trace_representor_info_get(port_id, info, ret);
-
return ret;
}
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index ee400b386f..940b999c11 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5785,58 +5785,33 @@ int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
* @warning
* @b EXPERIMENTAL: this structure may change without prior notice.
*
- * Ethernet device representor ID range entry
+ * Ethernet device representor information
*/
-struct rte_eth_representor_range {
- enum rte_eth_representor_type type; /**< Representor type */
- int controller; /**< Controller index */
- int pf; /**< Physical function index */
+struct rte_eth_representor_info {
+ enum rte_eth_representor_type type;
+ uint32_t controller;
+ bool controller_valid;
+ uint32_t pf;
+ bool pf_valid;
__extension__
union {
- int vf; /**< VF start index */
- int sf; /**< SF start index */
+ uint32_t vf;
+ uint32_t sf;
};
- uint32_t id_base; /**< Representor ID start index */
- uint32_t id_end; /**< Representor ID end index */
- char name[RTE_DEV_NAME_MAX_LEN]; /**< Representor name */
-};
-
-/**
- * @warning
- * @b EXPERIMENTAL: this structure may change without prior notice.
- *
- * Ethernet device representor information
- */
-struct rte_eth_representor_info {
- uint16_t controller; /**< Controller ID of caller device. */
- uint16_t pf; /**< Physical function ID of caller device. */
- uint32_t nb_ranges_alloc; /**< Size of the ranges array. */
- uint32_t nb_ranges; /**< Number of initialized ranges. */
- struct rte_eth_representor_range ranges[];/**< Representor ID range. */
};
/**
* Retrieve the representor info of the device.
*
- * Get device representor info to be able to calculate a unique
- * representor ID. @see rte_eth_representor_id_get helper.
- *
* @param port_id
* The port identifier of the device.
* @param info
* A pointer to a representor info structure.
- * NULL to return number of range entries and allocate memory
- * for next call to store detail.
- * The number of ranges that were written into this structure
- * will be placed into its nb_ranges field. This number cannot be
- * larger than the nb_ranges_alloc that by the user before calling
- * this function. It can be smaller than the value returned by the
- * function, however.
* @return
- * - (-ENOTSUP) if operation is not supported.
+ * - (-EINVAL) if @p info is NULL.
* - (-ENODEV) if *port_id* invalid.
* - (-EIO) if device is removed.
- * - (>=0) number of available representor range entries.
+ * - (0) @p info was filled.
*/
__rte_experimental
int rte_eth_representor_info_get(uint16_t port_id,
--
2.43.0
