On 8/15/11 3:47 AM, "Stefan Berger" <stefanb@linux.vnet.ibm.com> wrote:

On 08/12/2011 07:14 PM, Roopa Prabhu wrote:

...

From: Roopa Prabhu<roprabhu@cisco.com>

This patch moves some of the sriov related pci code from node_device driver to src/util/pci.[ch]. Some functions had to go thru name and argument list change to accommodate the move.

Signed-off-by: Roopa Prabhu<roprabhu@cisco.com> Signed-off-by: Christian Benvenuti<benve@cisco.com> Signed-off-by: David Wang<dwang2@cisco.com> --- src/Makefile.am | 5 + src/conf/node_device_conf.c | 1 src/conf/node_device_conf.h | 7 - src/node_device/node_device_driver.h | 14 -- src/node_device/node_device_hal.c | 10 + src/node_device/node_device_linux_sysfs.c | 191 ---------------------------- src/node_device/node_device_udev.c | 10 + src/util/pci.c | 196 +++++++++++++++++++++++++++++ src/util/pci.h | 25 ++++ 9 files changed, 242 insertions(+), 217 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am index 009ff25..4246823 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -953,7 +953,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) libvirt_driver_nodedev_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_nodedev_la_LIBADD = +libvirt_driver_nodedev_la_LIBADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + if HAVE_HAL libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dde2921..548bbff 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -36,6 +36,7 @@ #include "util.h" #include "buf.h" #include "uuid.h" +#include "pci.h"

#define VIR_FROM_THIS VIR_FROM_NODEDEV

diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index cef86d4..17be031 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags { VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1<< 1), };

-struct pci_config_address { - unsigned int domain; - unsigned int bus; - unsigned int slot; - unsigned int function; -}; - typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 08779b1..673e95b 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -37,10 +37,6 @@ # define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create" # define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete"

-# define SRIOV_FOUND 0 -# define SRIOV_NOT_FOUND 1 -# define SRIOV_ERROR -1 - # define LINUX_NEW_DEVICE_WAIT_TIME 60

# ifdef HAVE_HAL @@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d);

-# define get_physical_function(s,d) get_physical_function_linux(s,d) -int get_physical_function_linux(const char *sysfs_path, - union _virNodeDevCapData *d); - -# define get_virtual_functions(s,d) get_virtual_functions_linux(s,d) -int get_virtual_functions_linux(const char *sysfs_path, - union _virNodeDevCapData *d); - # define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) int read_wwn_linux(int host, const char *file, char **wwn);

@@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn);

# define check_fc_host(d) (-1) # define check_vport_capable(d) (-1) -# define get_physical_function(sysfs_path, d) -# define get_virtual_functions(sysfs_path, d) # define read_wwn(host, file, wwn)

# endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 421f5ad..481be97 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -35,6 +35,7 @@ #include "datatypes.h" #include "memory.h" #include "uuid.h" +#include "pci.h" #include "logging.h" #include "node_device_driver.h"

@@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, (void)virStrToLong_ui(p+1,&p, 16,&d->pci_dev.function); }

- get_physical_function(sysfs_path, d); - get_virtual_functions(sysfs_path, d); + if (!pciGetPhysicalFunction(sysfs_path,&d->pci_dev.physical_function)) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!pciGetVirtualFunctions(sysfs_path,&d->pci_dev.virtual_functions, +&d->pci_dev.num_virtual_functions) || + d->pci_dev.num_virtual_functions> 0) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;

VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index f9ff20f..844231a 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -205,195 +205,4 @@ out: return retval; }

- -static int logStrToLong_ui(char const *s, - char **end_ptr, - int base, - unsigned int *result) -{ - int ret = 0; - - ret = virStrToLong_ui(s, end_ptr, base, result); - if (ret != 0) { - VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s); - } else { - VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result); - } - - return ret; -} - - -static int parse_pci_config_address(char *address, struct pci_config_address *bdf) -{ - char *p = NULL; - int ret = -1; - - if ((address == NULL) || (logStrToLong_ui(address,&p, 16, -&bdf->domain) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->bus) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->slot) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->function) == -1)) { - goto out; - } - - ret = 0; - -out: - return ret; -} - - - - -static int get_sriov_function(const char *device_link, - struct pci_config_address **bdf) -{ - char *config_address = NULL; - char *device_path = NULL; - char errbuf[64]; - int ret = SRIOV_ERROR; - - VIR_DEBUG("Attempting to resolve device path from device link '%s'", - device_link); - - if (!virFileExists(device_link)) { - - VIR_DEBUG("SR IOV function link '%s' does not exist", device_link); - /* Not an SR IOV device, not an error, either. */ - ret = SRIOV_NOT_FOUND; - - 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))); - goto out; - } - - VIR_DEBUG("SR IOV device path is '%s'", device_path); - config_address = basename(device_path); - if (VIR_ALLOC(*bdf) != 0) { - VIR_ERROR(_("Failed to allocate memory for PCI device name")); - goto out; - } - - if (parse_pci_config_address(config_address, *bdf) != 0) { - VIR_ERROR(_("Failed to parse PCI config address '%s'"), config_address); - goto out; - } - - VIR_DEBUG("SR IOV function %.4x:%.2x:%.2x.%.1x", - (*bdf)->domain, - (*bdf)->bus, - (*bdf)->slot, - (*bdf)->function); - - ret = SRIOV_FOUND; - -out: - VIR_FREE(device_path); - return ret; -} - - -int get_physical_function_linux(const char *sysfs_path, - union _virNodeDevCapData *d ATTRIBUTE_UNUSED) -{ - int ret = -1; - char *device_link = NULL; - - VIR_DEBUG("Attempting to get SR IOV physical function for device " - "with sysfs path '%s'", sysfs_path); - - if (virBuildPath(&device_link, sysfs_path, "physfn") == -1) { - virReportOOMError(); - } else { - ret = get_sriov_function(device_link,&d->pci_dev.physical_function); - if (ret == SRIOV_FOUND) { - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - } - } - - VIR_FREE(device_link); - return ret; -} - - -int get_virtual_functions_linux(const char *sysfs_path, - union _virNodeDevCapData *d) -{ - int ret = -1; - DIR *dir = NULL; - struct dirent *entry = NULL; - char *device_link = NULL; - - VIR_DEBUG("Attempting to get SR IOV virtual functions for device" - "with sysfs path '%s'", sysfs_path); - - dir = opendir(sysfs_path); - if (dir == NULL) { - goto out; - } - - while ((entry = readdir(dir))) { - if (STRPREFIX(entry->d_name, "virtfn")) { - /* This local is just to avoid lines of code much> 80 col. */ - unsigned int *num_funcs =&d->pci_dev.num_virtual_functions; - - if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { - virReportOOMError(); - goto out; - } - - VIR_DEBUG("Number of virtual functions: %d", *num_funcs); - if (VIR_REALLOC_N(d->pci_dev.virtual_functions, - (*num_funcs) + 1) != 0) { - virReportOOMError(); - goto out; - } - - if (get_sriov_function(device_link, -&d->pci_dev.virtual_functions[*num_funcs]) - != SRIOV_FOUND) { - - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - VIR_ERROR(_("Failed to get SR IOV function from device link '%s'"), - device_link); - goto out; - } else { - (*num_funcs)++; - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; - } - - VIR_FREE(device_link); - } - } - - ret = 0; - -out: - if (dir) - closedir(dir); - VIR_FREE(device_link); - return ret; -} - #endif /* __linux__ */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2c5d016..badf241 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -37,6 +37,7 @@ #include "uuid.h" #include "util.h" #include "buf.h" +#include "pci.h"

#define VIR_FROM_THIS VIR_FROM_NODEDEV

@@ -480,8 +481,13 @@ static int udevProcessPCI(struct udev_device *device, goto out; }

- get_physical_function(syspath, data); - get_virtual_functions(syspath, data); + if (!pciGetPhysicalFunction(syspath,&data->pci_dev.physical_function)) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!pciGetVirtualFunctions(syspath,&data->pci_dev.virtual_functions, +&data->pci_dev.num_virtual_functions) || + data->pci_dev.num_virtual_functions> 0) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;

ret = 0;

diff --git a/src/util/pci.c b/src/util/pci.c index a79c164..e017db9 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -32,6 +32,7 @@ #include<sys/types.h> #include<sys/stat.h> #include<unistd.h> +#include<stdlib.h>

#include "logging.h" #include "memory.h" @@ -48,6 +49,10 @@ #define PCI_ID_LEN 10 /* "XXXX XXXX" */ #define PCI_ADDR_LEN 13 /* "XXXX:XX:XX.X" */

+# define SRIOV_FOUND 0 +# define SRIOV_NOT_FOUND 1 +# define SRIOV_ERROR -1 + struct _pciDevice { unsigned domain; unsigned bus; @@ -1679,3 +1684,194 @@ int pciDeviceIsAssignable(pciDevice *dev,

return 1; } + +static int +logStrToLong_ui(char const *s, + char **end_ptr, + int base, + unsigned int *result) +{ + int ret = 0; + + ret = virStrToLong_ui(s, end_ptr, base, result); + if (ret != 0) { + VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s); + } else { + VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result); + } + + return ret; +} + +static int +pciParsePciConfigAddress(char *address, + struct pci_config_address *bdf) +{ + char *p = NULL; + int ret = -1; + + if ((address == NULL) || (logStrToLong_ui(address,&p, 16, +&bdf->domain) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->bus) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->slot) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->function) == -1)) { + goto out; + } + + ret = 0; + +out: + return ret; +} + +static int +pciGetPciConfigAddressFromSysfsDeviceLink(const char *device_link, + struct pci_config_address **bdf) +{ + char *config_address = NULL; + char *device_path = NULL; + char errbuf[64]; + int ret = -1; + + VIR_DEBUG("Attempting to resolve device path from device link '%s'", + device_link); + + if (!virFileExists(device_link)) { + VIR_DEBUG("sysfs_path '%s' does not exist", device_link); + 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))); + goto out; + } + + config_address = basename(device_path); + if (VIR_ALLOC(*bdf) != 0) { + VIR_ERROR(_("Failed to allocate memory for PCI device name")); + goto out; + } + + if (pciParsePciConfigAddress(config_address, *bdf) != 0) { + VIR_ERROR(_("Failed to parse PCI config address '%s'"), + config_address); + goto out; + } + + VIR_DEBUG("pci_config_address %.4x:%.2x:%.2x.%.1x", + (*bdf)->domain, + (*bdf)->bus, + (*bdf)->slot, + (*bdf)->function); + + ret = 0; + +out: + VIR_FREE(device_path);

Caller likely does not expect *bdf to have a valid value in case of an error. Free bdf ? Yeah, will fix it.

...

+ + return ret; +} + +/* + * Returns Physical function given a virtual function + */ +int +pciGetPhysicalFunctionLinux(const char *vf_sysfs_path, + struct pci_config_address **physical_function) +{ + int ret = -1; + char *device_link = NULL; + + VIR_DEBUG("Attempting to get SR IOV physical function for device " + "with sysfs path '%s'", vf_sysfs_path); + + if (virBuildPath(&device_link, vf_sysfs_path, "physfn") == -1) { + virReportOOMError(); + } else { + ret = pciGetPciConfigAddressFromSysfsDeviceLink(device_link, + physical_function); + } + + VIR_FREE(device_link); + + return ret; +} + +/* + * Returns virtual functions of a physical function + */ +int +pciGetVirtualFunctionsLinux(const char *sysfs_path, + struct pci_config_address ***virtual_functions, 3 '*' necessary ? Yes I think so, Cause I have to allocate the whole ** inside the function and the caller is the one who frees it.

...

+ unsigned int *num_virtual_functions) +{ + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + char *device_link = NULL; + + VIR_DEBUG("Attempting to get SR IOV virtual functions for device" + "with sysfs path '%s'", sysfs_path); + + dir = opendir(sysfs_path); + if (dir == NULL) + goto out; + + *virtual_functions = NULL; + *num_virtual_functions = 0; + while ((entry = readdir(dir))) { + if (STRPREFIX(entry->d_name, "virtfn")) { + + if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { + virReportOOMError(); + goto out; + } + + VIR_DEBUG("Number of virtual functions: %d", + *num_virtual_functions); + if (VIR_REALLOC_N(*virtual_functions, + (*num_virtual_functions) + 1) != 0) { + virReportOOMError(); + goto out; + } + + if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, +&((*virtual_functions)[*num_virtual_functions])) != + SRIOV_FOUND) { + /* We should not get back SRIOV_NOT_FOUND in this + * case, so if we do, it's an error. */ + VIR_ERROR(_("Failed to get SR IOV function from device " + "link '%s'"), device_link); + goto out; + } else { + (*num_virtual_functions)++; + } + VIR_FREE(device_link); + } + } + + ret = 0; + +out: + if (dir) + closedir(dir); + + VIR_FREE(device_link); + + return ret; +} diff --git a/src/util/pci.h b/src/util/pci.h index a351baf..367881e 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -27,6 +27,13 @@ typedef struct _pciDevice pciDevice; typedef struct _pciDeviceList pciDeviceList;

+struct pci_config_address { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; +}; + pciDevice *pciGetDevice (unsigned domain, unsigned bus, unsigned slot, @@ -74,4 +81,22 @@ int pciDeviceIsAssignable(pciDevice *dev, int strict_acs_check); int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);

+#ifdef __linux__ There should probably be a space between the '#' and the 'ifdef'. Try with 'make syntax-check'. Ok will fix it.

...

+ +# define pciGetPhysicalFunction(s,a) pciGetPhysicalFunctionLinux(s,a) +int pciGetPhysicalFunctionLinux(const char *sysfs_path, + struct pci_config_address **phys_fn); + +# define pciGetVirtualFunctions(s,a,n) pciGetVirtualFunctionsLinux(s,a,n) +int pciGetVirtualFunctionsLinux(const char *sysfs_path, + struct pci_config_address ***virtual_functions, + unsigned int *num_virtual_functions); + +#else /* __linux__ */ Same single space here... + +# define pciGetPhysicalFunction(s,a) +# define pciGetVirtualFunctions(s,a,n) + +#endif and here. + #endif /* __VIR_PCI_H__ */

ACK with those nits addressed.

Stefan

Thanks. will fix them and resubmit.

On 8/15/11 3:58 AM, "Stefan Berger" <stefanb@linux.vnet.ibm.com> wrote:

On 08/12/2011 07:14 PM, Roopa Prabhu wrote:

...

- get_physical_function(sysfs_path, d); - get_virtual_functions(sysfs_path, d); + if (!pciGetPhysicalFunction(sysfs_path,&d->pci_dev.physical_function)) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!pciGetVirtualFunctions(sysfs_path,&d->pci_dev.virtual_functions, +&d->pci_dev.num_virtual_functions) || + d->pci_dev.num_virtual_functions> 0) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;

VIR_FREE(sysfs_path);

[...]

...

diff --git a/src/util/pci.h b/src/util/pci.h index a351baf..367881e 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -27,6 +27,13 @@ typedef struct _pciDevice pciDevice; typedef struct _pciDeviceList pciDeviceList;

+struct pci_config_address { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; +}; + pciDevice *pciGetDevice (unsigned domain, unsigned bus, unsigned slot, @@ -74,4 +81,22 @@ int pciDeviceIsAssignable(pciDevice *dev, int strict_acs_check); int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);

+#ifdef __linux__ + +# define pciGetPhysicalFunction(s,a) pciGetPhysicalFunctionLinux(s,a) +int pciGetPhysicalFunctionLinux(const char *sysfs_path, + struct pci_config_address **phys_fn); + +# define pciGetVirtualFunctions(s,a,n) pciGetVirtualFunctionsLinux(s,a,n) +int pciGetVirtualFunctionsLinux(const char *sysfs_path, + struct pci_config_address ***virtual_functions, + unsigned int *num_virtual_functions); + +#else /* __linux__ */ + +# define pciGetPhysicalFunction(s,a) +# define pciGetVirtualFunctions(s,a,n) + I don't think these #defines will produce compilable code if they are used above. You'll probably have to implement functions for them.

Ok, I actually moved them from node_device driver as is with only name changes. I can implement them returning -1 for non __linux__ Will resubmit. Thanks.

From: Roopa Prabhu<roprabhu@cisco.com>

This patch adds the following helper functions: pciDeviceIsVirtualFunction: Function to check if a pci device is a sriov VF pciGetVirtualFunctionIndex: Function to get the VF index of a sriov VF pciDeviceNetName: Function to get the network device name of a pci device pciConfigAddressCompare: Function to compare pci config addresses

Signed-off-by: Roopa Prabhu<roprabhu@cisco.com> Signed-off-by: Christian Benvenuti<benve@cisco.com> Signed-off-by: David Wang<dwang2@cisco.com> --- src/util/pci.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/pci.h | 15 +++++++ 2 files changed, 133 insertions(+), 0 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c index e017db9..558c7ae 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1875,3 +1875,121 @@ out:

return ret; } + +/* + * returns 0 if equal and 1 if not + */ +static int +pciConfigAddressCompare(struct pci_config_address *bdf1, + struct pci_config_address *bdf2) +{ + return !((bdf1->domain == bdf2->domain)& + (bdf1->bus == bdf2->bus)& + (bdf1->slot == bdf2->slot)& + (bdf1->function == bdf2->function)); +} Would it no be more intuitive to call implement a function

On 08/12/2011 07:14 PM, Roopa Prabhu wrote: pciConfigAddressEqual and return '1' in case the addresses are equal?

+ +/* + * Returns 1 if vf device is a virtual function, 0 if not, -1 on error + */ +int +pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link) +{ + char *vf_sysfs_physfn_link = NULL; + int ret = -1; + + if (virAsprintf(&vf_sysfs_physfn_link, "%s/physfn", + vf_sysfs_device_link)< 0) { + virReportOOMError(); + return ret; + } + + ret = virFileExists(vf_sysfs_physfn_link); + + VIR_FREE(vf_sysfs_physfn_link); + + return ret; +} + +/* + * Returns the sriov virtual function index of vf given its pf + */ +int +pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, + const char *vf_sysfs_device_link, + int *vf_index) +{ + int ret = -1, i; + unsigned int num_virt_fns = 0; + struct pci_config_address *vf_bdf = NULL; + struct pci_config_address **virt_fns = NULL; + + if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, +&vf_bdf)) + return ret; + + if (pciGetVirtualFunctionsLinux(pf_sysfs_device_link,&virt_fns, +&num_virt_fns)) { + VIR_DEBUG("Error getting physical function's '%s' virtual_functions\n", + pf_sysfs_device_link); + goto out; + } + + for (i = 0; i< num_virt_fns; i++) { + if (!pciConfigAddressCompare(vf_bdf, virt_fns[i])) { + *vf_index = i; + ret = 0; + break; + } + } + +out: + + /* free virtual functions */ + for (i = 0; i< num_virt_fns; i++) + VIR_FREE(virt_fns[i]); + + VIR_FREE(vf_bdf); + + return ret; +} + +/* + * Returns the network device name of a pci device + */ +int +pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname) +{ + char *pcidev_sysfs_net_path = NULL; + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + + if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, + "net") == -1) { + virReportOOMError(); + return -1; + } + + dir = opendir(pcidev_sysfs_net_path); + if (dir == NULL) + goto out; + + while ((entry = readdir(dir))) { + if (!strcmp(entry->d_name, ".") || + !strcmp(entry->d_name, "..")) + continue; + + /* Assume a single directory entry */ + *netname = strdup(entry->d_name); + ret = 0; + break; + } + + closedir(dir); + +out: + VIR_FREE(pcidev_sysfs_net_path); + + return ret; +} diff --git a/src/util/pci.h b/src/util/pci.h index 367881e..0cdc6ec 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -92,10 +92,25 @@ int pciGetVirtualFunctionsLinux(const char *sysfs_path, struct pci_config_address ***virtual_functions, unsigned int *num_virtual_functions);

+# define pciDeviceIsVirtualFunction(v) pciDeviceIsVirtualFunctionLinux(v) +int pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link); + +# define pciGetVirtualFunctionIndex(p,v,i) \ + pciGetVirtualFunctionIndexLinux(p,v,i) +int pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, + const char *vf_sysfs_device_link, + int *vf_index); + +# define pciDeviceNetName(d,n) pciDeviceNetNameLinux(d,n) +int pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname); + #else /* __linux__ */

# define pciGetPhysicalFunction(s,a) # define pciGetVirtualFunctions(s,a,n) +# define pciDeviceIsVirtualFunction(v) Also here is there is code like 'if (!pciDeviceIsVirtualFunction())' it will be expanded to if (!) on non-Linux platforms, which is not compilable. Maybe an inline function in this case will do?

+# define pciGetVirtualFunctionIndex(p,v,i) +# define pciDeviceNetNameLinux(d,n) Same for these two also. #endif

Stefan

On 8/15/11 4:13 AM, "Stefan Berger" <stefanb@linux.vnet.ibm.com> wrote:

On 08/12/2011 07:14 PM, Roopa Prabhu wrote:

...

From: Roopa Prabhu<roprabhu@cisco.com>

This patch adds the following functions to get PF/VF relationship of an SRIOV network interface: ifaceIsVirtualFunction: Function to check if a network interface is a SRIOV VF ifaceGetVirtualFunctionIndex: Function to get VF index if a network interface is a SRIOV VF ifaceGetPhysicalFunction: Function to get the PF net interface name of a SRIOV VF net interface

Signed-off-by: Roopa Prabhu<roprabhu@cisco.com> Signed-off-by: Christian Benvenuti<benve@cisco.com> Signed-off-by: David Wang<dwang2@cisco.com> --- src/util/interface.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/interface.h | 9 +++ 2 files changed, 159 insertions(+), 0 deletions(-)

diff --git a/src/util/interface.c b/src/util/interface.c index 8b4522b..ec00606 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -45,6 +45,8 @@ #include "virfile.h" #include "memory.h" #include "netlink.h" +#include "pci.h" +#include "logging.h"

#define VIR_FROM_THIS VIR_FROM_NET

@@ -1196,3 +1198,151 @@ ifaceRestoreMacAddress(const char *linkdev,

return rc; } + +#if __linux__ +static int +ifaceSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + + if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", + ifname, file)< 0) { + virReportOOMError(); + return -1; + } + + return 0; +} + +static int +ifaceSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + + if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/device/%s", + ifname, file)< 0) { + virReportOOMError(); + return -1; + } + + return 0; +} + +/** + * ifaceIsVirtualFunction + * + * @ifname : name of the interface + * + * Checks if an interface is a SRIOV virtual function. + * + * Returns 1 if interface is SRIOV virtual function, 0 if not and -1 if error + * + */ +int +ifaceIsVirtualFunction(const char *ifname) +{ + char *if_sysfs_device_link = NULL; + int ret = -1; + + if (ifaceSysfsFile(&if_sysfs_device_link, ifname, "device")) + return ret; + + ret = pciDeviceIsVirtualFunction(if_sysfs_device_link); + + VIR_FREE(if_sysfs_device_link); + + return ret; +} + +/** + * ifaceGetVirtualFunctionIndex + * + * @pfname : name of the physical function interface name + * @vfname : name of the virtual function interface name + * @vf_index : Pointer to int. Contains vf index of interface upon successful + * return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ + char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; + int ret = -1; + + if (ifaceSysfsFile(&pf_sysfs_device_link, pfname, "device")) + return ret; + + if (ifaceSysfsFile(&vf_sysfs_device_link, vfname, "device")) { + VIR_FREE(pf_sysfs_device_link); + return ret; + } + + ret = pciGetVirtualFunctionIndex(pf_sysfs_device_link, + vf_sysfs_device_link, + vf_index); + + VIR_FREE(pf_sysfs_device_link); + VIR_FREE(vf_sysfs_device_link); + + return ret; +} + +/** + * ifaceGetPhysicalFunction + * + * @ifname : name of the physical function interface name + * @pfname : Contains sriov physical function for interface ifname + * upon successful return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ + char *physfn_sysfs_path = NULL; + int ret = -1; + + if (ifaceSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn")) + return ret; + + ret = pciDeviceNetName(physfn_sysfs_path, pfname); + + VIR_FREE(physfn_sysfs_path); + + return ret; +} +#else + +int +ifaceIsVirtualFunction(const char *ifname) +{ + ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ifaceIsVirtualFunction is not supported on non-linux " + "platforms")); + return -ENOSYS; Since above functions are described to return -1 on error, also do this in these ones here.

Will do. Thanks.

On 08/12/2011 07:14 PM, Roopa Prabhu wrote:

From: Roopa Prabhu<roprabhu@cisco.com>

This patch renames getPhysfn to getPhysfnDev and adds code to get the Physical function and Virtual Function index of the direct attach linkdev (if the direct attach interface is a SRIOV VF). The idea is to send the port profile message to a PF if the direct attach interface is a SRIOV VF.

Signed-off-by: Roopa Prabhu<roprabhu@cisco.com> Signed-off-by: Christian Benvenuti<benve@cisco.com> Signed-off-by: David Wang<dwang2@cisco.com> --- src/util/macvtap.c | 23 ++++++++++------------- 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 0b00776..9bf7fa6 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -943,23 +943,20 @@ err_exit:

# ifdef IFLA_VF_PORT_MAX static int -getPhysfn(const char *linkdev, - int32_t *vf, - char **physfndev) +getPhysfnDev(const char *linkdev, + int32_t *vf, + char **physfndev) { int rc = 0; - bool virtfn = false;

- if (virtfn) { + if (ifaceIsVirtualFunction(linkdev)) {

- /* XXX: if linkdev is SR-IOV VF, then set vf = VF index */ - /* XXX: and set linkdev = PF device */ - /* XXX: need to use get_physical_function_linux() or */ - /* XXX: something like that to get PF */ - /* XXX: device and figure out VF index */ - - rc = 1; + /* if linkdev is SR-IOV VF, then set vf = VF index */ + /* and set linkdev = PF device */

+ rc = ifaceGetPhysicalFunction(linkdev, physfndev); + if (!rc) + rc = ifaceGetVirtualFunctionIndex(*physfndev, linkdev, vf); } else {

/* Not SR-IOV VF: physfndev is linkdev and VF index @@ -1003,7 +1000,7 @@ doPortProfileOp8021Qbh(const char *ifname, int ifindex; int vlanid = -1;

- rc = getPhysfn(ifname,&vf,&physfndev); + rc = getPhysfnDev(ifname,&vf,&physfndev); if (rc) goto err_exit;

ACK. Stefan