On 08/02/2011 08:46 AM, Roopa Prabhu wrote:
On 8/1/11 6:10 PM, "Stefan Berger"<stefanb(a)linux.vnet.ibm.com> wrote:
> On 08/01/2011 07:57 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu<roprabhu(a)cisco.com>
>>
>> This patch adds helper functions to derive the PF/VF relationship of an sriov
>> network device
>>
>> Signed-off-by: Roopa Prabhu<roprabhu(a)cisco.com>
>> Signed-off-by: Christian Benvenuti<benve(a)cisco.com>
>> Signed-off-by: David Wang<dwang2(a)cisco.com>
>> ---
>> src/util/interface.c | 122
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/util/interface.h | 8 +++
>> 2 files changed, 130 insertions(+), 0 deletions(-)
>>
>>
>> diff --git a/src/util/interface.c b/src/util/interface.c
>> index f5eecfb..5ee5703 100644
>> --- a/src/util/interface.c
>> +++ b/src/util/interface.c
>> @@ -30,6 +30,7 @@
>> #include<sys/ioctl.h>
>> #include<fcntl.h>
>> #include<netinet/in.h>
>> +#include<dirent.h>
>>
>> #ifdef __linux__
>> # include<linux/if.h>
>> @@ -45,6 +46,8 @@
>> #include "virfile.h"
>> #include "memory.h"
>> #include "netlink.h"
>> +#include "pci.h"
>> +#include "logging.h"
>>
>> #define VIR_FROM_THIS VIR_FROM_NET
>>
>> @@ -1197,3 +1200,122 @@ ifaceRestoreMacAddress(const char *linkdev,
>>
>> return rc;
>> }
>> +
>> +int ifaceIsVF(const char *ifname)
>> +{
>> + char *if_sysfs_device_link = NULL;
>> + int ret;
>> +
>> + if (virAsprintf(&if_sysfs_device_link, NET_SYSFS
"%s/device/physfn",
>> + ifname)< 0) {
>> + virReportOOMError();
>> + return -1;
>> + }
>> +
>> + ret = virFileExists(if_sysfs_device_link);
>> +
>> + VIR_FREE(if_sysfs_device_link);
>> +
>> + return ret;
>> +}
>> +
>> +int ifaceGetVFIndex(const char *pfname, const char *vfname,
>> + int *vf_index)
>> +{
>> + int ret = -1;
>> + DIR *dir = NULL;
>> + struct dirent *entry = NULL;
>> + char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL;
>> + char *vf_sysfs_device = NULL;
>> + char errbuf[64];
>> +
>> + if (virAsprintf(&pf_sysfs_device_link, NET_SYSFS "%s/device",
>> + pfname)< 0) {
>> + virReportOOMError();
>> + return -1;
>> + }
>> +
>> + if (virAsprintf(&vf_sysfs_device_link, NET_SYSFS "%s/device",
>> + vfname)< 0) {
>> + VIR_FREE(pf_sysfs_device_link);
>> + virReportOOMError();
>> + return -1;
>> + }
>> +
>> + vf_sysfs_device = canonicalize_file_name(vf_sysfs_device_link);
>> + if (vf_sysfs_device == NULL) {
>> + memset(errbuf, '\0', sizeof(errbuf));
>> + VIR_ERROR(_("Failed to resolve device link '%s':
'%s'"),
>> + vf_sysfs_device_link,
>> + virStrerror(errno, errbuf, sizeof(errbuf)));
>> + VIR_FREE(pf_sysfs_device_link);
>> + VIR_FREE(vf_sysfs_device_link);
>> + return -1;
>> + }
>> +
>> + dir = opendir(pf_sysfs_device_link);
>> + if (dir == NULL)
>> + goto out;
>> +
>> + while ((entry = readdir(dir))) {
>> + if (STRPREFIX(entry->d_name, "virtfn")) {
>> + char *device_link, *device_path;
>> +
>> + if (virBuildPath(&device_link, pf_sysfs_device_link,
>> + entry->d_name) == -1) {
>> + virReportOOMError();
>> + goto out;
>> + }
>> +
>> + device_path = canonicalize_file_name(device_link);
>> + if (device_path == NULL) {
>> + memset(errbuf, '\0', sizeof(errbuf));
>> + VIR_ERROR(_("Failed to resolve device link '%s':
'%s'"),
>> + device_link, virStrerror(errno, errbuf,
>> + sizeof(errbuf)));
>> + VIR_FREE(device_link);
>> + goto out;
>> + }
>> +
>> + if (!strcmp(vf_sysfs_device, device_path)) {
>> + *vf_index = atoi(entry->d_name +
strlen("virtfn"));
> This looks odd. Can you explain?
The PF device sysfs directory has links with the name "virtfn<vf_index>"
pointing to the VF's pci device.
Example:
# ls -l virtfn*
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn0 -> ../0000:1e:00.1
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn1 -> ../0000:1e:00.2
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn10 -> ../0000:1e:01.3
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn11 -> ../0000:1e:01.4
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn12 -> ../0000:1e:01.5
I see if the virt function link is pointing to the pci device of the VF I
am looking for. If it is, then I try to get the vf index from the virtual
function link name (I just need the number in "virtfn<vf_index>".
This is one way to get the vf index. The other way is to get it by reading
the pci config space of the PF, which the node device driver uses.
In that case I'd suggest to use 'if (sscanf(entry->d_name,
"virtfn%u",
&vf_index) != 1) { error }'
Stefan
>> + ret = 0;
>> + }
>> +
>> + VIR_FREE(device_link);
>> + VIR_FREE(device_path);
>> +
>> + if ( ret == 0 )
>> + break;
>> + }
>> + }
>> +
>> + if (dir)
>> + closedir(dir);
> In this case the above test should be after the 'out:' label not to
> cause a memory leak with the goto's above.
>
Thanks for catching it. Exactly similar lines of code exists in the previous
patch. There was a bug in the out label location in previous patch. When I
fixed the previous patch I accidently made the change in this patch too
introducing a leak here. :) Will fix it. Thanks stefan.
>> +
>> +out:
>> +
>> + VIR_FREE(pf_sysfs_device_link);
>> + VIR_FREE(vf_sysfs_device_link);
>> + VIR_FREE(vf_sysfs_device);
>> +
>> + return ret;
>> +}
>> +
>> +int ifaceGetPFName(const char *ifname, char **pfname)
>> +{
>> + char *physfn_sysfs_path = NULL;
>> + int ret = -1;
>> +
>> + if (virAsprintf(&physfn_sysfs_path, NET_SYSFS
"%s/device/physfn",
>> + ifname)< 0) {
>> + virReportOOMError();
>> + return -1;
>> + }
>> +
>> + ret = pciDeviceGetNetName(physfn_sysfs_path, pfname);
>> +
>> + VIR_FREE(physfn_sysfs_path);
>> +
>> + return ret;
>> +}
>> diff --git a/src/util/interface.h b/src/util/interface.h
>> index 9647653..f2c84f8 100644
>> --- a/src/util/interface.h
>> +++ b/src/util/interface.h
>> @@ -26,6 +26,8 @@ struct nlattr;
>> # include "datatypes.h"
>> # include "network.h"
>>
>> +#define NET_SYSFS "/sys/class/net/"
>> +
>> int ifaceGetFlags(const char *name, short *flags);
>> int ifaceIsUp(const char *name, bool *up);
>>
>> @@ -76,4 +78,10 @@ int ifaceReplaceMacAddress(const unsigned char
>> *macaddress,
>> int ifaceRestoreMacAddress(const char *linkdev,
>> const char *stateDir);
>>
>> +int ifaceIsVF(const char *ifname);
>> +
>> +int ifaceGetVFIndex(const char *pfname, const char *vfname, int *vf_index);
>> +
>> +int ifaceGetPFName(const char *ifname, char **pfname);
>> +
>> #endif /* __VIR_INTERFACE_H__ */
>>
> Stefan
>