From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7915DA0567; Tue, 9 Mar 2021 09:19:36 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 303C74069D; Tue, 9 Mar 2021 09:19:36 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 4B5304068A for ; Tue, 9 Mar 2021 09:19:35 +0100 (CET) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id A0B437F593; Tue, 9 Mar 2021 11:19:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru A0B437F593 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1615277974; bh=1wiQzNGqo1V1zK6xJJ6LgGbD7zlB1TGIcFl+4ZkwCH8=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=FX2VInVTbxEpbIn8VySVaXWxNrK0/QBeSrT77/v90ZUOGJCUTnC6+d4yZos3wln3k WQ84oCgIuXqO0I9SA0AiDy6CL+CpFV37g8CjiSRc0BUvHjbBLP0ZZCPry31ORmVB6j h6/5Ba+nt2SwezjCRj7AejWEnN5ohb4vHd9ekHKk= To: Xueming Li Cc: dev@dpdk.org, Viacheslav Ovsiienko , Asaf Penso , Thomas Monjalon , Ferruh Yigit , Ray Kinsella , Neil Horman References: <1614868228-13685-1-git-send-email-xuemingl@nvidia.com> <1614868228-13685-9-git-send-email-xuemingl@nvidia.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: Date: Tue, 9 Mar 2021 11:19:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <1614868228-13685-9-git-send-email-xuemingl@nvidia.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 8/9] ethdev: representor iterator compare complete info X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" 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 Reviewed-by: Andrew Rybchenko 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]