On 3/4/21 5:30 PM, Xueming Li wrote:
> The NIC can have multiple PCIe links and can be attached to multiple
> hosts, for example the same single NIC can be shared for multiple server
> units in the rack. On each PCIe link NIC can provide multiple PFs and
> VFs/SFs based on these ones. The full representor identifier consists of
> three indices - controller index, PF index, and VF or SF index (if any).
>
> SR-IOV and SubFunction are created on top of PF. PF index is introduced
> because there might be multiple PFs in the bonding configuration and
> only bonding device is probed.
>
> In eth representor comparator callback, ethdev representor ID was
> compared with devarg. Since controller index and PF index not compared,
> callback returned representor from other PF or controller.
>
> This patch adds new API to get representor ID from controller, pf and
> vf/sf index. Representor comparer callback get representor ID then
> compare with device representor ID.
>
> Signed-off-by: Xueming Li <xuemi...@nvidia.com>
Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
One minor suggestions below to make it a bit more robust
against errors in PMDs.
Make thanks for careful review notes processing.
[snip]
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 3d4ec8ad5c..2dc464a7ad 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -5609,6 +5609,94 @@ rte_eth_devargs_parse(const char *dargs, struct
> rte_eth_devargs *eth_da)
> return result;
> }
>
> +int
> +rte_eth_representor_id_get(const struct rte_eth_dev *ethdev,
> + enum rte_eth_representor_type type,
> + int controller, int pf, int representor_port,
> + uint16_t *repr_id)
> +{
> + int ret, n, i, count;
> + struct rte_eth_representor_info *info = NULL;
> + size_t size;
> +
> + if (type == RTE_ETH_REPRESENTOR_NONE)
> + return 0;
> + if (repr_id == NULL)
> + return -EINVAL;
> +
> + /* Get PMD representor range info. */
> + ret = rte_eth_representor_info_get(ethdev->data->port_id, NULL);
> + if (ret < 0) {
> + if (type == RTE_ETH_REPRESENTOR_VF && controller == -1 &&
> + pf == -1) {
> + /* Direct mapping for legacy VF representor. */
> + *repr_id = representor_port;
> + return 0;
> + } else {
> + return ret;
> + }
> + }
> + n = ret;
> + size = sizeof(*info) + n * sizeof(info->ranges[0]);
> + info = calloc(1, size);
> + if (info == NULL)
> + return -ENOMEM;
> + ret = rte_eth_representor_info_get(ethdev->data->port_id, info);
> + if (ret < 0)
> + goto out;
> +
> + /* Default controller and pf to caller. */
> + if (controller == -1)
> + controller = info->controller;
> + if (pf == -1)
> + pf = info->pf;
> +
> + /* Locate representor ID. */
> + ret = -ENOENT;
> + for (i = 0; i < n; ++i) {
> + if (info->ranges[i].type != type)
> + continue;
> + if (info->ranges[i].controller != controller)
> + continue;
> + count = info->ranges[i].id_end - info->ranges[i].id_base + 1;
I think it would be useful to sanity test these values to
ensure that we have no overflow here.
Let's check above, log error and skip the range if id_end is
smaller than id_base.
> + switch (info->ranges[i].type) {
> + case RTE_ETH_REPRESENTOR_PF:
> + if (pf < info->ranges[i].pf ||
> + pf >= info->ranges[i].pf + count)
> + continue;
> + *repr_id = info->ranges[i].id_base +
> + (pf - info->ranges[i].pf);
> + ret = 0;
> + goto out;
> + case RTE_ETH_REPRESENTOR_VF:
> + if (info->ranges[i].pf != pf)
> + continue;
> + if (representor_port < info->ranges[i].vf ||
> + representor_port >= info->ranges[i].vf + count)
> + continue;
> + *repr_id = info->ranges[i].id_base +
> + (representor_port - info->ranges[i].vf);
> + ret = 0;
> + goto out;
> + case RTE_ETH_REPRESENTOR_SF:
> + if (info->ranges[i].pf != pf)
> + continue;
> + if (representor_port < info->ranges[i].sf ||
> + representor_port >= info->ranges[i].sf + count)
> + continue;
> + *repr_id = info->ranges[i].id_base +
> + (representor_port - info->ranges[i].sf);
> + ret = 0;
> + goto out;
> + default:
> + break;
> + }
> + }
> +out:
> + free(info);
> + return ret;
> +}
> +
> static int
> eth_dev_handle_port_list(const char *cmd __rte_unused,
> const char *params __rte_unused,
[snip]
