Re: [libvirt] [PATCH 0/4 v3] Support mac and port profile for
On 3/6/12 3:05 AM, "Laine Stump" wrote: > On 03/05/2012 08:12 PM, Roopa Prabhu wrote: >> This series implements the below : >> 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and >> pciConfigAddressToSysfsFile >> 02/4 virtnetdev: Add support functions for mac and portprofile associations >> on a hostdev >> 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs >> 04/4 qemu_hostdev: Add support to install port profile and mac address on >> hostdev > > Okay, I've pushed this series, with the few small nits I pointed out > squashed in. Thanks for the contribution! Thanks Laine. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4 v3] pci: Add two new pci utils pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile
From: Roopa Prabhu
pciDeviceGetVirtualFunctionInfo returns pf netdevice name and virtual
function index for a given vf. This is just a wrapper around existing functions
to return vf's pf and vf_index with one api call
pciConfigAddressToSysfsfile returns the sysfile pci device link
from a 'struct pci_config_address'
Signed-off-by: Roopa Prabhu
---
src/util/pci.c | 55 +++
src/util/pci.h |7 +++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index c660e8d..c8a5287 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2081,6 +2081,20 @@ pciSysfsFile(char *pciDeviceName, char
**pci_sysfs_device_link)
return 0;
}
+int
+pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link)
+{
+if (virAsprintf(pci_sysfs_device_link,
+PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain,
+dev->bus, dev->slot, dev->function) < 0) {
+virReportOOMError();
+return -1;
+}
+
+return 0;
+}
+
/*
* Returns the network device name of a pci device
*/
@@ -2123,6 +2137,38 @@ out:
return ret;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index)
+{
+struct pci_config_address *pf_config_address = NULL;
+char *pf_sysfs_device_path = NULL;
+int ret = -1;
+
+if (pciGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address) < 0)
+return ret;
+
+if (pciConfigAddressToSysfsFile(pf_config_address,
+&pf_sysfs_device_path) < 0) {
+
+VIR_FREE(pf_config_address);
+return ret;
+}
+
+if (pciGetVirtualFunctionIndex(pf_sysfs_device_path, vf_sysfs_device_path,
+vf_index) < 0)
+goto cleanup;
+
+ret = pciDeviceNetName(pf_sysfs_device_path, pfname);
+
+cleanup:
+VIR_FREE(pf_config_address);
+VIR_FREE(pf_sysfs_device_path);
+
+return ret;
+}
+
#else
int
pciGetPhysicalFunction(const char *vf_sysfs_path ATTRIBUTE_UNUSED,
@@ -2170,4 +2216,13 @@ pciDeviceNetName(char *device_link_sysfs_path
ATTRIBUTE_UNUSED,
"supported on non-linux platforms"));
return -1;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path
ATTRIBUTE_UNUSED,
+char **pfname, int *vf_index ATTRIBUTE_UNUSED)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceGetVirtualFunctionInfo "
+ "is not supported on non-linux platforms"));
+return -1;
+}
#endif /* __linux__ */
diff --git a/src/util/pci.h b/src/util/pci.h
index 25b5b66..b71bb12 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -111,6 +111,9 @@ int pciGetVirtualFunctionIndex(const char
*pf_sysfs_device_link,
const char *vf_sysfs_device_link,
int *vf_index);
+int pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link);
+
int pciDeviceNetName(char *device_link_sysfs_path, char **netname);
int pciSysfsFile(char *pciDeviceName, char **pci_sysfs_device_link)
@@ -122,4 +125,8 @@ int pciGetDeviceAddrString(unsigned domain,
unsigned function,
char **pciConfigAddr)
ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
+
+int pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index);
+
#endif /* __VIR_PCI_H__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4 v3] Support mac and port profile for
v3: Changes include: - Review comments from Laine - rebased with latest upstream v2: changes include: - feedback from stefan for 802.1Qbg. Code now prints an error if virtualport is specified for 802.1Qbg on an interface of type hostdev - feedback from laine for non-sriov devices. Interface type hostdev for non-sriov devices is not supported. v1: https://www.redhat.com/archives/libvir-list/2012-March/msg00015.html This patch series is based on laines patches to support . https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This series implements the below : 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile 02/4 virtnetdev: Add support functions for mac and portprofile associations on a hostdev 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs 04/4 qemu_hostdev: Add support to install port profile and mac address on hostdev Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a macvtap ifname does not exist. This patch just adds a null check for ifname in 802.1Qbg port profile handling code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4 v3] qemu_hostdev: Add support to install port profile and mac address on hostdevs
From: Roopa Prabhu
These changes are applied only if the hostdev has a parent net device.
If the parent netdevice has virtual port information, the original virtualport
associate functions are called (these set and restore both mac and port profile
on an interface). Else, only mac address is set on the device
using other methods depending on if its a sriov device or not.
Changes also include hotplug pci devices
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_hostdev.c | 241 +--
src/qemu/qemu_hostdev.h |8 +-
src/qemu/qemu_hotplug.c | 10 ++
3 files changed, 246 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b3cad8e..ebcdc52 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -29,6 +29,13 @@
#include "memory.h"
#include "pci.h"
#include "hostusb.h"
+#include "virnetdev.h"
+
+VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST,
+ "none",
+ "802.1Qbg",
+ "802.1Qbh",
+ "openvswitch")
static pciDeviceList *
qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
@@ -156,19 +163,192 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver
*driver,
return 0;
}
+static int
+qemuDomainHostdevPciSysfsPath(virDomainHostdevDefPtr hostdev, char
**sysfs_path)
+{
+struct pci_config_address config_address;
+
+config_address.domain = hostdev->source.subsys.u.pci.domain;
+config_address.bus = hostdev->source.subsys.u.pci.bus;
+config_address.slot = hostdev->source.subsys.u.pci.slot;
+config_address.function = hostdev->source.subsys.u.pci.function;
+
+return pciConfigAddressToSysfsFile(&config_address, sysfs_path);
+}
+
+int
+qemuDomainHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev)
+{
+char *sysfs_path = NULL;
+int ret = -1;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+ret = pciDeviceIsVirtualFunction(sysfs_path);
+
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+static int
+qemuDomainHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev,
+ int *vf)
+{
+int ret = -1;
+char *sysfs_path = NULL;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+if (pciDeviceIsVirtualFunction(sysfs_path) == 1) {
+if (pciDeviceGetVirtualFunctionInfo(sysfs_path, linkdev,
+vf) < 0)
+goto cleanup;
+} else {
+if (pciDeviceNetName(sysfs_path, linkdev) < 0)
+goto cleanup;
+*vf = -1;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+static int
+qemuDomainHostdevNetConfigVirtPortProfile(const char *linkdev, int vf,
+ virNetDevVPortProfilePtr virtPort,
+ const unsigned char *macaddr,
+ const unsigned char *uuid,
+ int associate)
+{
+int ret = -1;
+
+if (!virtPort)
+return ret;
+
+switch(virtPort->virtPortType) {
+case VIR_NETDEV_VPORT_PROFILE_NONE:
+case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
+case VIR_NETDEV_VPORT_PROFILE_8021QBG:
+case VIR_NETDEV_VPORT_PROFILE_LAST:
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("virtualport type %s is "
+"currently not supported on interfaces of type "
+"hostdev"),
+virNetDevVPortTypeToString(virtPort->virtPortType));
+break;
+
+case VIR_NETDEV_VPORT_PROFILE_8021QBH:
+if (associate)
+ret = virNetDevVPortProfileAssociate(NULL, virtPort, macaddr,
+ linkdev, vf, uuid,
+
VIR_NETDEV_VPORT_PROFILE_OP_CREATE, false);
+else
+ret = virNetDevVPortProfileDisassociate(NULL, virtPort,
+macaddr, linkdev, vf,
+
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY);
+break;
+}
+
+return ret;
+}
+
+int
+qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
+ const unsigned char *uuid,
+ char *stateDir)
+{
+char *linkdev = NULL;
+virNetDevVPortProfilePtr virtPort;
+int ret = -1;
+int vf = -1;
+int vlanid = -1;
+int port_profile_associate = 1;
+int isvf;
+
+isvf = qemuDomainHostdevIsVirtualFunction(hostdev);
+if (isvf <= 0) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+
[libvirt] [PATCH 2/4 v3] virtnetdev: Add support functions for mac and portprofile associations on a hostdev
From: Roopa Prabhu
This patch adds the following:
- functions to set and get vf configs
- Functions to replace and store vf configs (Only mac address is handled today.
But the functions can be easily extended for vlans and other vf configs)
- function to dump link dev info (This is moved from virnetdevvportprofile.c)
Signed-off-by: Roopa Prabhu
---
src/util/virnetdev.c | 535 ++
src/util/virnetdev.h | 19 ++
2 files changed, 553 insertions(+), 1 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 4aa7639..9f93fda 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1127,8 +1127,47 @@ virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
return ret;
}
-#else /* !__linux__ */
+/**
+ * virNetDevGetVirtualFunctionInfo:
+ * @vfname: name of the virtual function interface
+ * @pfname: name of the physical function
+ * @vf: vf index
+ *
+ * Returns 0 on success, -errno on failure.
+ *
+ */
+int
+virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
+int *vf)
+{
+char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
+int ret = -1;
+
+*pfname = NULL;
+
+if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
+return ret;
+
+if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
+goto cleanup;
+
+if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0)
+goto cleanup;
+
+ret = pciGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf);
+
+cleanup:
+if (ret < 0)
+VIR_FREE(*pfname);
+
+VIR_FREE(vf_sysfs_path);
+VIR_FREE(pf_sysfs_path);
+
+return ret;
+}
+
+#else /* !__linux__ */
int
virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED,
char ***vfname ATTRIBUTE_UNUSED,
@@ -1165,4 +1204,498 @@ virNetDevGetPhysicalFunction(const char *ifname
ATTRIBUTE_UNUSED,
_("Unable to get physical function status on this
platform"));
return -1;
}
+
#endif /* !__linux__ */
+#if defined(__linux__) && defined(HAVE_LIBNL)
+
+static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
+[IFLA_VF_MAC] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_mac) },
+[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_vlan) },
+};
+
+/**
+ * virNetDevLinkDump:
+ *
+ * @ifname: The name of the interface; only use if ifindex < 0
+ * @ifindex: The interface index; may be < 0 if ifname is given
+ * @nltarget_kernel: whether to send the message to the kernel or another
+ * process
+ * @nlattr: pointer to a pointer of netlink attributes that will contain
+ * the results
+ * @recvbuf: Pointer to the buffer holding the returned netlink response
+ * message; free it, once not needed anymore
+ * @getPidFunc: Pointer to a function that will be invoked if the kernel
+ * is not the target of the netlink message but it is to be
+ * sent to another process.
+ *
+ * Get information about an interface given its name or index.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetDevLinkDump(const char *ifname, int ifindex,
+ bool nltarget_kernel, struct nlattr **tb,
+ unsigned char **recvbuf,
+ uint32_t (*getPidFunc)(void))
+{
+int rc = 0;
+struct nlmsghdr *resp;
+struct nlmsgerr *err;
+struct ifinfomsg ifinfo = {
+.ifi_family = AF_UNSPEC,
+.ifi_index = ifindex
+};
+unsigned int recvbuflen;
+uint32_t pid = 0;
+struct nl_msg *nl_msg;
+
+*recvbuf = NULL;
+
+if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0)
+return -1;
+
+ifinfo.ifi_index = ifindex;
+
+nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST);
+if (!nl_msg) {
+virReportOOMError();
+return -1;
+}
+
+if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+goto buffer_too_small;
+
+if (ifname) {
+if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
+goto buffer_too_small;
+}
+
+if (!nltarget_kernel) {
+pid = getPidFunc();
+if (pid == 0) {
+rc = -1;
+goto cleanup;
+}
+}
+
+if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
+rc = -1;
+goto cleanup;
+}
+
+if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
+goto malformed_resp;
+
+resp = (struct nlmsghdr *)*recvbuf;
+
+switch (resp->nlmsg_type) {
+case NLMSG_ERROR:
+err = (struct nlmsgerr *)NLMSG_DATA(resp);
+if (resp->nlmsg_len < NLMSG_LENGTH(size
[libvirt] [PATCH 3/4 v3] virnetdevvportprofile: Changes to support portprofiles for hostdevs
From: Roopa Prabhu
This patch includes the following changes
- removes some netlink functions which are now available in virnetdev.c
- Adds a vf argument to all port profile functions
For 802.1Qbh devices, the port profile calls can use a vf argument if
passed by the caller. If the vf argument is -1 it will try to derive the vf
if the device passed is a virtual function.
For 802.1Qbg devices, This patch introduces a null check for the device
argument because during port profile assignment on a hostdev, this argument
can be null. Stefan CC'ed for comments
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_migration.c|2
src/util/virnetdevmacvlan.c |8 +
src/util/virnetdevvportprofile.c | 221 --
src/util/virnetdevvportprofile.h |8 +
4 files changed, 59 insertions(+), 180 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5c4297c..77d40c0 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2650,6 +2650,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def)
{
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
def->uuid,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) < 0)
goto err_exit;
@@ -2667,6 +2668,7 @@ err_exit:
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
}
}
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index f38a98c..647679f 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -452,6 +452,7 @@ struct virNetlinkCallbackData {
virNetDevVPortProfilePtr virtPortProfile;
unsigned char *macaddress;
char *linkdev;
+int vf;
unsigned char *vmuuid;
enum virNetDevVPortProfileOp vmOp;
unsigned int linkState;
@@ -719,6 +720,7 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg,
calld->virtPortProfile,
calld->macaddress,
calld->linkdev,
+calld->vf,
calld->vmuuid,
calld->vmOp, true));
*handled = true;
@@ -810,6 +812,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
const char *cr_ifname;
virNetlinkCallbackDataPtr calld = NULL;
int ret;
+int vf = -1;
macvtapMode = modeMap[mode];
@@ -871,6 +874,7 @@ create_name:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmuuid, vmOp, false) < 0) {
rc = -1;
goto link_del_exit;
@@ -948,6 +952,7 @@ disassociate_exit:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmOp));
link_del_exit:
@@ -975,6 +980,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
char *stateDir)
{
int ret = 0;
+int vf = -1;
+
if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir));
}
@@ -984,6 +991,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
virtPortProfile,
macaddr,
linkdev,
+ vf,
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0)
ret = -1;
if (virNetDevMacVLanDelete(ifname) < 0)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index f6db292..bd356d8 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@
Re: [libvirt] [PATCH 2/4] virtnetdev: Add support functions for mac and portprofile associations on a hostdev
On 3/5/12 11:50 AM, "Roopa Prabhu" wrote: > > > > On 3/5/12 10:23 AM, "Laine Stump" wrote: > >> On 03/04/2012 10:15 PM, Roopa Prabhu wrote: >>> From: Roopa Prabhu >>> >>> This patch adds the following: >>> - functions to set and get vf configs >>> - Functions to replace and store vf configs (Only mac address is handled >>> today. >>> But the functions can be easily extended for vlans and other vf configs) >>> - function to dump link dev info (This is moved from >>> virnetdevvportprofile.c) >>> >>> Signed-off-by: Roopa Prabhu >>> --- >>> src/util/virnetdev.c | 531 >>> ++ >>> src/util/virnetdev.h | 19 ++ >>> 2 files changed, 549 insertions(+), 1 deletions(-) >> >> (BTW, I never thought about doing it this way before, but I'm glad you >> added the function here in a separate patch from the patch that removes >> it from virnetdevvportprofile.c - that makes it easy to open the two >> patches side-by-side and verify that it really is moving the same code >> (well, mostly).) >> >>> >>> >>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >>> index 9d76d47..25f2155 100644 >>> --- a/src/util/virnetdev.c >>> +++ b/src/util/virnetdev.c >>> @@ -1127,8 +1127,497 @@ virNetDevGetPhysicalFunction(const char *ifname, >>> char >>> **pfname) >>> >>> return ret; >>> } >>> -#else /* !__linux__ */ >> >> The functions here that use libnl need to be inside of >> >> #if defined(__linux__) && defined(HAVE_LIBNL) >> >> since there are linux platforms that don't have libnl, or don't have the >> proper LIBNL (RHEL5, in particular, still has libnl-1.0) >> > > I was hoping someone will point out what #defines to use here. So thanks. I > will add HAVE_LIBNL. We also need it for IFLA_VF_MAC and VLAN. They were > under HAVE_VIRTPORT before. Can you suggest what I can do here ? > > Correction: They were under WITH_VIRUALPORT (which has a configure option and a check for IFLA_VF_PORT_MAX). We will need a check for IFLA_VF_MAX here. I think I will try putting this under defined(IFLA_VF_MAX). This should cover IFLA_VF_MAC and IFLA_VF_VLAN I think. If you have any other suggestions let me know. Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virtnetdev: Add support functions for mac and portprofile associations on a hostdev
On 3/5/12 10:23 AM, "Laine Stump" wrote:
> On 03/04/2012 10:15 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> This patch adds the following:
>> - functions to set and get vf configs
>> - Functions to replace and store vf configs (Only mac address is handled
>> today.
>> But the functions can be easily extended for vlans and other vf configs)
>> - function to dump link dev info (This is moved from virnetdevvportprofile.c)
>>
>> Signed-off-by: Roopa Prabhu
>> ---
>> src/util/virnetdev.c | 531
>> ++
>> src/util/virnetdev.h | 19 ++
>> 2 files changed, 549 insertions(+), 1 deletions(-)
>
> (BTW, I never thought about doing it this way before, but I'm glad you
> added the function here in a separate patch from the patch that removes
> it from virnetdevvportprofile.c - that makes it easy to open the two
> patches side-by-side and verify that it really is moving the same code
> (well, mostly).)
>
>>
>>
>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>> index 9d76d47..25f2155 100644
>> --- a/src/util/virnetdev.c
>> +++ b/src/util/virnetdev.c
>> @@ -1127,8 +1127,497 @@ virNetDevGetPhysicalFunction(const char *ifname, char
>> **pfname)
>>
>> return ret;
>> }
>> -#else /* !__linux__ */
>
> The functions here that use libnl need to be inside of
>
> #if defined(__linux__) && defined(HAVE_LIBNL)
>
> since there are linux platforms that don't have libnl, or don't have the
> proper LIBNL (RHEL5, in particular, still has libnl-1.0)
>
I was hoping someone will point out what #defines to use here. So thanks. I
will add HAVE_LIBNL. We also need it for IFLA_VF_MAC and VLAN. They were
under HAVE_VIRTPORT before. Can you suggest what I can do here ?
>>
>> +static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
>> +[IFLA_VF_MAC] = { .type = NLA_UNSPEC,
>> +.maxlen = sizeof(struct ifla_vf_mac) },
>> +[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
>> +.maxlen = sizeof(struct ifla_vf_vlan) },
>> +};
>> +
>> +/**
>> + * virNetDevLinkDump:
>> + *
>> + * @ifname: The name of the interface; only use if ifindex < 0
>> + * @ifindex: The interface index; may be < 0 if ifname is given
>> + * @nltarget_kernel: whether to send the message to the kernel or another
>> + * process
>> + * @nlattr: pointer to a pointer of netlink attributes that will contain
>> + * the results
>> + * @recvbuf: Pointer to the buffer holding the returned netlink response
>> + * message; free it, once not needed anymore
>> + * @getPidFunc: Pointer to a function that will be invoked if the kernel
>> + * is not the target of the netlink message but it is to be
>> + * sent to another process.
>> + *
>> + * Get information about an interface given its name or index.
>> + *
>> + * Returns 0 on success, -1 on fatal error.
>> + */
>> +int
>> +virNetDevLinkDump(const char *ifname, int ifindex,
>> + bool nltarget_kernel, struct nlattr **tb,
>> + unsigned char **recvbuf,
>> + uint32_t (*getPidFunc)(void))
>> +{
>> +int rc = 0;
>> +struct nlmsghdr *resp;
>> +struct nlmsgerr *err;
>> +struct ifinfomsg ifinfo = {
>> +.ifi_family = AF_UNSPEC,
>> +.ifi_index = ifindex
>> +};
>> +unsigned int recvbuflen;
>> +uint32_t pid = 0;
>> +struct nl_msg *nl_msg;
>> +
>> +*recvbuf = NULL;
>> +
>> +if (ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0)
>> +return -1;
>> +
>> +ifinfo.ifi_index = ifindex;
>> +
>> +nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST);
>> +if (!nl_msg) {
>> +virReportOOMError();
>> +return -1;
>> +}
>> +
>> +if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
>> +goto buffer_too_small;
>> +
>> +if (ifindex < 0 && ifname) {
>> +if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
>> +goto buffer_too_small;
>> +}
>
>
> Is this bit necessary any more? You've added code above that converts
> the ifname into an ifindex, and we've already returned if it wasn't
> successful.
>
>
Ok yes I will remove it. I added
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/5/12 11:16 AM, "Laine Stump" wrote:
> I encountered two conflicts when I rebased this patch to upstream. Noted
> in the comments.
>
> On 03/04/2012 10:15 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> This patch includes the following changes
>> - removes some netlink functions which are now available in virnetdev.c
>> - Adds a vf argument to all port profile functions
>>
>> For 802.1Qbh devices, the port profile calls can use a vf argument if
>> passed by the caller. If the vf argument is -1 it will try to derive the vf
>> if the device passed is a virtual function.
>>
>> For 802.1Qbg devices, This patch introduces a null check for the device
>> argument because during port profile assignment on a hostdev, this argument
>> can be null. Stefan CC'ed for comments
>>
>> Signed-off-by: Roopa Prabhu
>> ---
>> src/qemu/qemu_migration.c|2
>> src/util/virnetdevmacvlan.c |6 +
>> src/util/virnetdevvportprofile.c | 203
>> +-
>> src/util/virnetdevvportprofile.h |8 +
>> 4 files changed, 42 insertions(+), 177 deletions(-)
>>
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index 7df2d4f..b80ed69 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -2605,6 +2605,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr
>> def) {
>>
>> virDomainNetGetActualVirtPortProfile(net),
>> net->mac,
>>
>> virDomainNetGetActualDirectDev(net),
>> + -1,
>> def->uuid,
>>
>> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0)
>> goto err_exit;
>> @@ -2622,6 +2623,7 @@ err_exit:
>>
>> virDomainNetGetActualVirtPortProfile(net),
>> net->mac,
>>
>> virDomainNetGetActualDirectDev(net),
>> + -1,
>>
>> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
>> }
>> }
>> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>> index 25d0846..498a2a0 100644
>> --- a/src/util/virnetdevmacvlan.c
>> +++ b/src/util/virnetdevmacvlan.c
>> @@ -486,6 +486,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
>> *tgifname,
>> uint32_t macvtapMode;
>> const char *cr_ifname;
>> int ret;
>> +int vf = -1;
>>
>> macvtapMode = modeMap[mode];
>>
>> @@ -547,6 +548,7 @@ create_name:
>> virtPortProfile,
>> macaddress,
>> linkdev,
>> + vf,
>> vmuuid, vmOp) < 0) {
>> rc = -1;
>> goto link_del_exit;
>> @@ -597,6 +599,7 @@ disassociate_exit:
>> virtPortProfile,
>> macaddress,
>> linkdev,
>> + vf,
>> vmOp));
>>
>> link_del_exit:
>> @@ -624,6 +627,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
>> *ifname,
>> char *stateDir)
>> {
>> int ret = 0;
>> +int vf = -1;
>> +
>> if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
>> ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir));
>> }
>> @@ -633,6 +638,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
>> *ifname,
>>virtPortProfile,
>>macaddr,
>>linkdev,
>> + vf,
>>
>> VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0)
>> ret = -1;
>> if (virNetDevMacVLanDelete(ifname) < 0)
>> diff --git a/src/util/virnetdevvportprofile.c
>> b/src/util/virnetdevvportprofile.c
>> index 67fd7bc..8d9e906 100644
>> --- a/src/util
[libvirt] [PATCH 2/4] virtnetdev: Add support functions for mac and portprofile associations on a hostdev
From: Roopa Prabhu
This patch adds the following:
- functions to set and get vf configs
- Functions to replace and store vf configs (Only mac address is handled today.
But the functions can be easily extended for vlans and other vf configs)
- function to dump link dev info (This is moved from virnetdevvportprofile.c)
Signed-off-by: Roopa Prabhu
---
src/util/virnetdev.c | 531 ++
src/util/virnetdev.h | 19 ++
2 files changed, 549 insertions(+), 1 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 9d76d47..25f2155 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1127,8 +1127,497 @@ virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
return ret;
}
-#else /* !__linux__ */
+static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
+[IFLA_VF_MAC] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_mac) },
+[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_vlan) },
+};
+
+/**
+ * virNetDevLinkDump:
+ *
+ * @ifname: The name of the interface; only use if ifindex < 0
+ * @ifindex: The interface index; may be < 0 if ifname is given
+ * @nltarget_kernel: whether to send the message to the kernel or another
+ * process
+ * @nlattr: pointer to a pointer of netlink attributes that will contain
+ * the results
+ * @recvbuf: Pointer to the buffer holding the returned netlink response
+ * message; free it, once not needed anymore
+ * @getPidFunc: Pointer to a function that will be invoked if the kernel
+ * is not the target of the netlink message but it is to be
+ * sent to another process.
+ *
+ * Get information about an interface given its name or index.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetDevLinkDump(const char *ifname, int ifindex,
+ bool nltarget_kernel, struct nlattr **tb,
+ unsigned char **recvbuf,
+ uint32_t (*getPidFunc)(void))
+{
+int rc = 0;
+struct nlmsghdr *resp;
+struct nlmsgerr *err;
+struct ifinfomsg ifinfo = {
+.ifi_family = AF_UNSPEC,
+.ifi_index = ifindex
+};
+unsigned int recvbuflen;
+uint32_t pid = 0;
+struct nl_msg *nl_msg;
+
+*recvbuf = NULL;
+
+if (ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0)
+return -1;
+
+ifinfo.ifi_index = ifindex;
+
+nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST);
+if (!nl_msg) {
+virReportOOMError();
+return -1;
+}
+
+if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+goto buffer_too_small;
+
+if (ifindex < 0 && ifname) {
+if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
+goto buffer_too_small;
+}
+
+if (!nltarget_kernel) {
+pid = getPidFunc();
+if (pid == 0) {
+rc = -1;
+goto cleanup;
+}
+}
+
+if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
+rc = -1;
+goto cleanup;
+}
+
+if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
+goto malformed_resp;
+
+resp = (struct nlmsghdr *)*recvbuf;
+
+switch (resp->nlmsg_type) {
+case NLMSG_ERROR:
+err = (struct nlmsgerr *)NLMSG_DATA(resp);
+if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+goto malformed_resp;
+
+if (err->error) {
+virReportSystemError(-err->error,
+ _("error dumping %s (%d) interface"),
+ ifname, ifindex);
+rc = -1;
+}
+break;
+
+case GENL_ID_CTRL:
+case NLMSG_DONE:
+rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
+ tb, IFLA_MAX, NULL);
+if (rc < 0)
+goto malformed_resp;
+break;
+
+default:
+goto malformed_resp;
+}
+
+if (rc != 0)
+VIR_FREE(*recvbuf);
+
+cleanup:
+nlmsg_free(nl_msg);
+
+return rc;
+
+malformed_resp:
+nlmsg_free(nl_msg);
+
+virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed netlink response message"));
+VIR_FREE(*recvbuf);
+return -1;
+
+buffer_too_small:
+nlmsg_free(nl_msg);
+
+virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+return -1;
+}
+
+static int
+virNetDevSetVfConfig(const char *ifname, int ifindex, int vf,
+ bool nltarget_kernel, const unsigned char *macaddr,
+ int vlanid, uint32_t (*getPidFunc)(void))
+{
+int rc = -1;
+struct nlmsghdr *resp;
+struct nlmsgerr *err;
+unsigned char *recvbu
[libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
From: Roopa Prabhu
This patch includes the following changes
- removes some netlink functions which are now available in virnetdev.c
- Adds a vf argument to all port profile functions
For 802.1Qbh devices, the port profile calls can use a vf argument if
passed by the caller. If the vf argument is -1 it will try to derive the vf
if the device passed is a virtual function.
For 802.1Qbg devices, This patch introduces a null check for the device
argument because during port profile assignment on a hostdev, this argument
can be null. Stefan CC'ed for comments
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_migration.c|2
src/util/virnetdevmacvlan.c |6 +
src/util/virnetdevvportprofile.c | 203 +-
src/util/virnetdevvportprofile.h |8 +
4 files changed, 42 insertions(+), 177 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7df2d4f..b80ed69 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2605,6 +2605,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def)
{
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
def->uuid,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0)
goto err_exit;
@@ -2622,6 +2623,7 @@ err_exit:
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
}
}
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 25d0846..498a2a0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -486,6 +486,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
uint32_t macvtapMode;
const char *cr_ifname;
int ret;
+int vf = -1;
macvtapMode = modeMap[mode];
@@ -547,6 +548,7 @@ create_name:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmuuid, vmOp) < 0) {
rc = -1;
goto link_del_exit;
@@ -597,6 +599,7 @@ disassociate_exit:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmOp));
link_del_exit:
@@ -624,6 +627,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
char *stateDir)
{
int ret = 0;
+int vf = -1;
+
if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir));
}
@@ -633,6 +638,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
virtPortProfile,
macaddr,
linkdev,
+ vf,
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0)
ret = -1;
if (virNetDevMacVLanDelete(ifname) < 0)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 67fd7bc..8d9e906 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -126,11 +126,6 @@ static struct nla_policy ifla_port_policy[IFLA_PORT_MAX +
1] =
{
[IFLA_PORT_RESPONSE] = { .type = NLA_U16 },
};
-static struct nla_policy ifla_policy[IFLA_MAX + 1] =
-{
- [IFLA_VF_PORTS] = { .type = NLA_NESTED },
-};
-
static uint32_t
virNetDevVPortProfileGetLldpadPid(void) {
@@ -164,126 +159,6 @@ virNetDevVPortProfileGetLldpadPid(void) {
return pid;
}
-
-/**
- * virNetDevVPortProfileLinkDump:
- *
- * @ifname: The name of the interface; only use if ifindex < 0
- * @ifindex: The interface index; may be < 0 if ifname is given
- * @nltarget_kernel: whether to send the message to the kernel or another
- * process
- * @nlattr: pointer to a pointer of netlink attributes that will contain
- * the results
- * @recvbuf: Pointer to the buf
[libvirt] [PATCH 1/4] pci: Add two new pci utils pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile
From: Roopa Prabhu
pciDeviceGetVirtualFunctionInfo returns pf netdevice name and virtual
function index for a given vf. This is just a wrapper around existing functions
to return vf's pf and vf_index with one api call
pciConfigAddressToSysfsfile returns the sysfile pci device link
from a 'struct pci_config_address'
Signed-off-by: Roopa Prabhu
---
src/util/pci.c | 55 +++
src/util/pci.h |7 +++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index c660e8d..d8c136e 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2081,6 +2081,20 @@ pciSysfsFile(char *pciDeviceName, char
**pci_sysfs_device_link)
return 0;
}
+int
+pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link)
+{
+if (virAsprintf(pci_sysfs_device_link,
+PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain,
+dev->bus, dev->slot, dev->function) < 0) {
+virReportOOMError();
+return -1;
+}
+
+return 0;
+}
+
/*
* Returns the network device name of a pci device
*/
@@ -2123,6 +2137,38 @@ out:
return ret;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index)
+{
+struct pci_config_address *pf_config_address = NULL;
+char *pf_sysfs_device_path = NULL;
+int ret = -1;
+
+if (pciGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address) < 0)
+return ret;
+
+if (pciConfigAddressToSysfsFile(pf_config_address,
+&pf_sysfs_device_path) < 0) {
+
+VIR_FREE(pf_config_address);
+return ret;
+}
+
+if (pciGetVirtualFunctionIndex(pf_sysfs_device_path, vf_sysfs_device_path,
+vf_index) < 0)
+goto cleanup;
+
+ret = pciDeviceNetName(pf_sysfs_device_path, pfname);
+
+cleanup:
+VIR_FREE(pf_config_address);
+VIR_FREE(pf_sysfs_device_path);
+
+return ret;
+}
+
#else
int
pciGetPhysicalFunction(const char *vf_sysfs_path ATTRIBUTE_UNUSED,
@@ -2170,4 +2216,13 @@ pciDeviceNetName(char *device_link_sysfs_path
ATTRIBUTE_UNUSED,
"supported on non-linux platforms"));
return -1;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceGetVirtualFunctionInfo "
+ "is not supported on non-linux platforms"));
+return -1;
+}
#endif /* __linux__ */
diff --git a/src/util/pci.h b/src/util/pci.h
index 25b5b66..b71bb12 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -111,6 +111,9 @@ int pciGetVirtualFunctionIndex(const char
*pf_sysfs_device_link,
const char *vf_sysfs_device_link,
int *vf_index);
+int pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link);
+
int pciDeviceNetName(char *device_link_sysfs_path, char **netname);
int pciSysfsFile(char *pciDeviceName, char **pci_sysfs_device_link)
@@ -122,4 +125,8 @@ int pciGetDeviceAddrString(unsigned domain,
unsigned function,
char **pciConfigAddr)
ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
+
+int pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index);
+
#endif /* __VIR_PCI_H__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Support mac and port profile for
v2: changes include: - feedback from stefan for 802.1Qbg. Code now prints an error if virtualport is specified for 802.1Qbg on an interface of type hostdev - feedback from laine for non-sriov devices. Interface type hostdev for non-sriov devices is not supported. v1: https://www.redhat.com/archives/libvir-list/2012-March/msg00015.html This patch series is based on laines patches to support . https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This series implements the below : 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile 02/4 virtnetdev: Add support functions for mac and portprofile associations on a hostdev 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs 04/4 qemu_hostdev: Add support to install port profile and mac address on hostdev Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a macvtap ifname does not exist. This patch just adds a null check for ifname in 802.1Qbg port profile handling code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] qemu_hostdev: Add support to install port profile and mac address on hostdevs
From: Roopa Prabhu
These changes are applied only if the hostdev has a parent net device.
If the parent netdevice has virtual port information, the original virtualport
associate functions are called (these set and restore both mac and port profile
on an interface). Else, only mac address is set on the device
using other methods depending on if its a sriov device or not.
Changes also include hotplug pci devices
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_hostdev.c | 237 +--
src/qemu/qemu_hostdev.h |8 +-
src/qemu/qemu_hotplug.c | 11 ++
3 files changed, 243 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b3cad8e..62fe467 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -29,6 +29,13 @@
#include "memory.h"
#include "pci.h"
#include "hostusb.h"
+#include "virnetdev.h"
+
+VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST,
+ "none",
+ "802.1Qbg",
+ "802.1Qbh",
+ "openvswitch")
static pciDeviceList *
qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
@@ -156,19 +163,179 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver
*driver,
return 0;
}
+static int
+qemuDomainHostdevPciSysfsPath(virDomainHostdevDefPtr hostdev, char
**sysfs_path)
+{
+struct pci_config_address config_address;
+
+config_address.domain = hostdev->source.subsys.u.pci.domain;
+config_address.bus = hostdev->source.subsys.u.pci.bus;
+config_address.slot = hostdev->source.subsys.u.pci.slot;
+config_address.function = hostdev->source.subsys.u.pci.function;
+
+return pciConfigAddressToSysfsFile(&config_address, sysfs_path);
+}
+
+int
+qemuDomainHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev)
+{
+char *sysfs_path = NULL;
+int ret = -1;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+ret = pciDeviceIsVirtualFunction(sysfs_path);
+
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+static int
+qemuDomainHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev,
+ int *vf)
+{
+int ret = -1;
+char *sysfs_path = NULL;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+if (pciDeviceIsVirtualFunction(sysfs_path) == 1) {
+if (pciDeviceGetVirtualFunctionInfo(sysfs_path, linkdev,
+vf) < 0)
+goto cleanup;
+} else {
+if (pciDeviceNetName(sysfs_path, linkdev) < 0)
+goto cleanup;
+*vf = -1;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+static int
+qemuDomainHostdevNetConfigVirtPortProfile(const char *linkdev, int vf,
+ virNetDevVPortProfilePtr virtPort,
+ const unsigned char *macaddr,
+ const unsigned char *uuid,
+ int associate)
+{
+int ret = -1;
+
+if (!virtPort)
+return ret;
+
+switch(virtPort->virtPortType) {
+case VIR_NETDEV_VPORT_PROFILE_NONE:
+case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
+case VIR_NETDEV_VPORT_PROFILE_8021QBG:
+case VIR_NETDEV_VPORT_PROFILE_LAST:
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("virtualport type %s is "
+"currently not supported on interfaces of type "
+"hostdev"),
+virNetDevVPortTypeToString(virtPort->virtPortType));
+break;
+
+case VIR_NETDEV_VPORT_PROFILE_8021QBH:
+if (associate)
+ret = virNetDevVPortProfileAssociate(NULL, virtPort, macaddr,
+ linkdev, vf, uuid,
+
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+else
+ret = virNetDevVPortProfileDisassociate(NULL, virtPort,
+macaddr, linkdev, vf,
+
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY);
+break;
+}
+
+return ret;
+}
+
+int
+qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
+ const unsigned char *uuid,
+ char *stateDir)
+{
+char *linkdev = NULL;
+virNetDevVPortProfilePtr virtPort;
+int ret = -1;
+int vf = -1;
+int vlanid = -1;
+int port_profile_associate = 1;
+
+if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0)
+return ret;
+
+virtPort = virDomainNetGetActualVi
Re: [libvirt] [PATCH 0/4] Support mac and port profile for
On 3/2/12 12:54 PM, "Laine Stump" wrote: > On 03/02/2012 03:03 PM, Roopa Prabhu wrote: >> On 3/2/12 11:04 AM, "Laine Stump" wrote: >>> On 03/01/2012 04:02 AM, Roopa Prabhu wrote: >>>> This patch series is based on laines patches to support >>> type='hostdev'>. >>>> https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html >>>> >>>> It support to set mac and port profile on an interface of type hostdev. >>>> * If virtualport is specified, the existing virtual port functions are >>>> called to set mac, vlan and port profile. >>> I'm unable to test that part, as I don't have any 802.1QbX capable >>> switches (and it sounds like the design is problematic anyway.) >>> >> The design is still fine for 802.1Qbh. > > > Yes, my apologies for misinterpreting all the exchanges. > > No problem atall. >> I have tested it. 802.1Qbh does not >> need a macvtap device. For 802.1Qbg in v2 (was planning on pushing it the >> next hr), > > > I'll try to review it as quickly as possible. > > >> I have put a check for 802.1Qbg and hostdevs and fail as suggested >> by stefan. > > > I'm quickly learning that I understood much less about 802.1QbX (and in > particular, how it's been implemented) than I thought! (Fortunately, as > a result of all this, I think I now understand it a bit better). > > And I am understanding 802.1Qbg a bit better now :) >>>> * If virtualport is not specified and device is a sriov virtual function, >>>> - mac is set using IFLA_VF_MAC >>> Success!! I tried this for VFs that have a netdev driver attached, and >>> VFs that don't, and it behaved properly in both cases - when the guest >>> is started, the MAC address is set properly for the guest to use, and >>> when the guest is stopped, the MAC address of that VF is restored to its >>> original value (implying that your code to save the old MAC address >>> works properly). >>> >>> >> Nice. Thanks for trying it out. >> >>>> * If virtualport is not specified and device is a non-sriov virtual >>>> function, >>>> - mac is set using existing SIOCGIFHWADDR (This requires that the >>>> netdev be present on the host before starting the VM) >>> This one has a problem, at least with my non-sriov hardware (which >>> happens to be the onboard NetXtreme device of a Thinkstation, using the >>> tg3 driver) it appears the MAC address gets reset to its original >>> setting at some point after libvirt changes it. To help understand what >>> happens - assume the device's original MAC address is o:o:o:o:o:o, and >>> my xml looks like this: >>> >>> >>> >>> ... >>> >>> >>> When the guest boots up, ifconfig shows there is an interface with mac >>> address o:o:o:o:o:o. >>> >>> Additionally, if I manually change the mac address to p:p:p:p:p:p on the >>> host before starting the guest, when the guest boots, ifconfig shows the >>> mac address as... o:o:o:o:o:o. So, whether or not libvirt is >>> successfully setting the mac address, it's getting reset (probably by >>> the card's firmware). >> Yes this I kind of expected. It depends on the driver. I thought there are >> some drivers that remember the mac address set by SIOCGIFHWADDR. But my >> assumption was totally based on the fact that we wanted to add support for >> all devices. Having the code there just means we try to set the device mac. >> It takes effect only if the hw supports it. > > If there was just a way to determine that at runtime, but it seems that > by the time the MAC address has been reset, we are no longer able to > call the ioctl to check the address :-( > > >>> So perhaps this is another case of wanting to do something that just >>> isn't possible, and the way out is to simply generate an error on domain >>> startup if the netdev being passed through isn't a VF? >>> >>> >> We can do this too. Only support it for sriov vf's. > > Yes, if it's going to silently do the wrong thing, maybe we should leave > the SIOCGIFHWADDR code in there for reference, but also add a failure > condition if the card isn't SRIOV. Ok. Heres what I will do (If I understand you correctly): - I will call the mac/portprofile set functions for sriov devices only. - Throw an error for non-sriov devices - Keep the mac set code for non-sriov devices around for reference > > (I guess this means my effort to make sure USB ethernets were also > supported was kind of pointless, since they're sure to have the same > problems :-P Mostly I only included that support to promote code sharing > and consistency, though, so I'm not really disappointed.) > :) Thanks Laine. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 12:45 PM, "Laine Stump" wrote: > On 03/02/2012 03:16 PM, Roopa Prabhu wrote: >> On 3/2/12 11:27 AM, "Laine Stump" wrote: >>> On 03/02/2012 11:58 AM, Stefan Berger wrote: >>>> On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: >>>>> Letting the guest do the association is an option, which should work >>>>> already (even if noone probably tested it yet), but the question is >>>>> really how much control should the host have vs the guest. There are >>>>> definitely scenarios thinkable where the host should do the association. >>>> I think an applicable scenario is where the infrastructure provider >>>> doesn't really know the guest user and needs to have 'mandatory access >>>> control' over the configuration of the infrastructure and yet wants to >>>> use the pass-through mode for better throughput. >>> And/or when the guest OS doesn't have the necessary smarts to do the >>> association (or maybe even vlan tagging) itself. >>> >>> So, at the end of this, is there *any* 802.1QbX mode that can work using >>> PCI passthrough without at least one of the following things: >>> >>> 1) a macvtap interface on the host. (what about my idea of attaching a >>> macvtap interface to the PF? does that have any hint of practicality?) >>> >>> 2) extending the protocol for talking with lldpad to support using a raw >>> PCI device rather than a macvtap device. >>> >>> 3) the guest doing vlan tagging >>> >>> 4) the guest doing the full 802.1QbX associate/de-associate protocol >>> exchange itself? >>> >>> Nobody has said it explicitly yet (I think), but I have the impression >>> that this problem unfortunately can't be solved by libvirt alone. If >>> that's the case, we should state that as soon as possible so that we can >>> table the part of these patches for the short term, and >>> separate the mac address part to get it pushed upstream (along with the >>> new low-level PCI utility functions), as that is very useful on its own. >>> >> Laine, The patches I submitted for virtualport 802.1Qbh work just fine. > > Yeah, I'm not sure how I lost sight of the fact that you said your > testing had gone okay - I guess too little sleep. So I read too much > into the discussion, and it's just Qbg that has these restrictions. > Good! Pay no attention to my alarmism :-) > > No problem :) >> I submitted these patches mainly to get the virtualport working for >> 802.1Qbh. Because we pass the port profile via the PF to fw and then to the >> switch. The driver in the guest just comes up on the VF and uses the >> already associated VF. > > Right. That's pretty much how I've always been assuming it worked for > all of these modes. I guess I should spend more time at lower levels. > >> We do the port profile association from the host because of security >> reasons. Instead of giving control to the guest OS. >> >> And as you can see in the patches, for 802.1Qbh port profile association on >> the hostdev is not much different from port profile association in the >> macvtap case. >> > > Okay, then in the end these patches will support 802.1Qbh > setting, as well as setting the MAC address (but only for SRIOV-capable > devices). And any future support for 802.1Qbg would require both some > extra support outside libvirt, > as well as specifying the vlanid in the > config, and requiring the guest to setup VLAN tagging. Did I get it > right now? > Yes that seems right. I think we don't need to call out the setup in the guest. Its common for all modes. Host provisions the vlans (ie configures the switch port with that vlan etc). This is the step that libvirt does. And guest does his own setup for vlan devices if needed. Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 9:21 AM, "Gerhard Stenzel" wrote: > On Fri, 2012-03-02 at 14:27 -0500, Laine Stump wrote: >> So, at the end of this, is there *any* 802.1QbX mode that can work >> using >> PCI passthrough without at least one of the following things: >> >> 1) a macvtap interface on the host. (what about my idea of attaching a >> macvtap interface to the PF? does that have any hint of practicality?) >> >> 2) extending the protocol for talking with lldpad to support using a >> raw >> PCI device rather than a macvtap device. > >> 3) the guest doing vlan tagging >> >> 4) the guest doing the full 802.1QbX associate/de-associate protocol >> exchange itself? >> >> Nobody has said it explicitly yet (I think), but I have the impression >> that this problem unfortunately can't be solved by libvirt alone. If >> that's the case, we should state that as soon as possible so that we >> can >> table the part of these patches for the short term, and >> separate the mac address part to get it pushed upstream (along with >> the >> new low-level PCI utility functions), as that is very useful on its >> own. > > I am not sure I can follow the conclusion that this can not be solved in > libvirt alone. > Qbg: > For the macvtap case, the macvtap device is "attached" to the underlying > physical interface and this is where the association request is sent to, > via lldpad. > For the PCI passthrough case, the same must be possible, assuming the > physical interface can be concluded from the PCI device and the VLAN > information is provided. > > Or do I miss something? > Wondering if we simplify this to only support sriov devices and for 802.1Qbg all config can be done via the PF netdevice. Am assuming the vlan info has to only reach lldpad daemon and you don't really need a vlan device on host. In which case, your new xml syntax with vlanid should work I think. - we send the mac, vlan and port profile info to lldpad via the PF with vf index using the current IFLA_VF_MAC and IFLA_VF_PORT. No ? Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/2/12 11:27 AM, "Laine Stump" wrote: > On 03/02/2012 11:58 AM, Stefan Berger wrote: >> On 03/02/2012 11:37 AM, Gerhard Stenzel wrote: >>> >>> Letting the guest do the association is an option, which should work >>> already (even if noone probably tested it yet), but the question is >>> really how much control should the host have vs the guest. There are >>> definitely scenarios thinkable where the host should do the association. >> >> I think an applicable scenario is where the infrastructure provider >> doesn't really know the guest user and needs to have 'mandatory access >> control' over the configuration of the infrastructure and yet wants to >> use the pass-through mode for better throughput. > > And/or when the guest OS doesn't have the necessary smarts to do the > association (or maybe even vlan tagging) itself. > > So, at the end of this, is there *any* 802.1QbX mode that can work using > PCI passthrough without at least one of the following things: > > 1) a macvtap interface on the host. (what about my idea of attaching a > macvtap interface to the PF? does that have any hint of practicality?) > > 2) extending the protocol for talking with lldpad to support using a raw > PCI device rather than a macvtap device. > > 3) the guest doing vlan tagging > > 4) the guest doing the full 802.1QbX associate/de-associate protocol > exchange itself? > > Nobody has said it explicitly yet (I think), but I have the impression > that this problem unfortunately can't be solved by libvirt alone. If > that's the case, we should state that as soon as possible so that we can > table the part of these patches for the short term, and > separate the mac address part to get it pushed upstream (along with the > new low-level PCI utility functions), as that is very useful on its own. > Laine, The patches I submitted for virtualport 802.1Qbh work just fine. I submitted these patches mainly to get the virtualport working for 802.1Qbh. Because we pass the port profile via the PF to fw and then to the switch. The driver in the guest just comes up on the VF and uses the already associated VF. We do the port profile association from the host because of security reasons. Instead of giving control to the guest OS. And as you can see in the patches, for 802.1Qbh port profile association on the hostdev is not much different from port profile association in the macvtap case. Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Support mac and port profile for
On 3/2/12 11:04 AM, "Laine Stump" wrote: > On 03/01/2012 04:02 AM, Roopa Prabhu wrote: >> This patch series is based on laines patches to support > type='hostdev'>. >> https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html >> >> It support to set mac and port profile on an interface of type hostdev. >> * If virtualport is specified, the existing virtual port functions are >> called to set mac, vlan and port profile. > > I'm unable to test that part, as I don't have any 802.1QbX capable > switches (and it sounds like the design is problematic anyway.) > The design is still fine for 802.1Qbh. I have tested it. 802.1Qbh does not need a macvtap device. For 802.1Qbg in v2 (was planning on pushing it the next hr), I have put a check for 802.1Qbg and hostdevs and fail as suggested by stefan. >> * If virtualport is not specified and device is a sriov virtual function, >> - mac is set using IFLA_VF_MAC > > Success!! I tried this for VFs that have a netdev driver attached, and > VFs that don't, and it behaved properly in both cases - when the guest > is started, the MAC address is set properly for the guest to use, and > when the guest is stopped, the MAC address of that VF is restored to its > original value (implying that your code to save the old MAC address > works properly). > > Nice. Thanks for trying it out. >> * If virtualport is not specified and device is a non-sriov virtual function, >> - mac is set using existing SIOCGIFHWADDR (This requires that the >> netdev be present on the host before starting the VM) > > This one has a problem, at least with my non-sriov hardware (which > happens to be the onboard NetXtreme device of a Thinkstation, using the > tg3 driver) it appears the MAC address gets reset to its original > setting at some point after libvirt changes it. To help understand what > happens - assume the device's original MAC address is o:o:o:o:o:o, and > my xml looks like this: > > > > ... > > > When the guest boots up, ifconfig shows there is an interface with mac > address o:o:o:o:o:o. > > Additionally, if I manually change the mac address to p:p:p:p:p:p on the > host before starting the guest, when the guest boots, ifconfig shows the > mac address as... o:o:o:o:o:o. So, whether or not libvirt is > successfully setting the mac address, it's getting reset (probably by > the card's firmware). Yes this I kind of expected. It depends on the driver. I thought there are some drivers that remember the mac address set by SIOCGIFHWADDR. But my assumption was totally based on the fact that we wanted to add support for all devices. Having the code there just means we try to set the device mac. It takes effect only if the hw supports it. > > So perhaps this is another case of wanting to do something that just > isn't possible, and the way out is to simply generate an error on domain > startup if the netdev being passed through isn't a VF? > > We can do this too. Only support it for sriov vf's. Thanks, Roopa >> >> This series implements the below : >> 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and >> pciConfigAddressToSysfsFile >> 02/4 virtnetdev: Add support functions for mac and portprofile associations >> on a hostdev >> 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs >> 04/4 qemu_hostdev: Add support to install port profile and mac address on >> hostdev >> >> Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for >> 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a >> macvtap ifname does not exist. This patch just adds a null check for ifname >> in >> 802.1Qbg port profile handling code. >> > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/1/12 7:52 AM, "Stefan Berger" wrote: > On 03/01/2012 10:32 AM, Roopa Prabhu wrote: >> >> >> On 3/1/12 4:39 AM, "Stefan Berger" wrote: >> >>> On 03/01/2012 04:02 AM, Roopa Prabhu wrote: >>>> From: Roopa Prabhu >>>> >>>> This patch includes the following changes >>>> - removes some netlink functions which are now available in virnetdev.c >>>> - Adds a vf argument to all port profile functions >>>> >>>> For 802.1Qbh devices, the port profile calls can use a vf argument if >>>> passed by the caller. If the vf argument is -1 it will try to derive the vf >>>> if the device passed is a virtual function. >>>> >>>> For 802.1Qbg devices, This patch introduces a null check for the device >>>> argument because during port profile assignment on a hostdev, this argument >>>> can be null. Stefan CC'ed for comments >>> I agree to the change per se since entering the function with a NULL >>> parameter would be fatal, though I am wondering under what condition >>> this can occur. I don't see this happening in the Associate call path >>> for example. >> It happens in patch 4/4 where we call associate for a hostdev and there is >> no macvtap available there. >> > > So this hostdev related code can only be used by 802.1Qbh because the > type of device does not exist for 802.1Qbg? Not really. The hostdev device is just a pci device. But looking at 802.1Qbg port profile related code..i was not sure how it can be done when the device is a hostdev. > I think that at least there > should be an error message logging the fact that the function was called > without an interface name. Also the return error code should probably > be -1 and not be overloaded with -E*. Ok This can be done. >If possible (unless already done) > the combination of hostdev and 802.1Qbg should probably even be > prevented on the XML level. This is not done yet. I think I will do it. And then if someone knows how to do it for 802.1Qbg they can open this up. Thanks stefan. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 3/1/12 4:39 AM, "Stefan Berger" wrote: > On 03/01/2012 04:02 AM, Roopa Prabhu wrote: >> From: Roopa Prabhu >> >> This patch includes the following changes >> - removes some netlink functions which are now available in virnetdev.c >> - Adds a vf argument to all port profile functions >> >> For 802.1Qbh devices, the port profile calls can use a vf argument if >> passed by the caller. If the vf argument is -1 it will try to derive the vf >> if the device passed is a virtual function. >> >> For 802.1Qbg devices, This patch introduces a null check for the device >> argument because during port profile assignment on a hostdev, this argument >> can be null. Stefan CC'ed for comments > > I agree to the change per se since entering the function with a NULL > parameter would be fatal, though I am wondering under what condition > this can occur. I don't see this happening in the Associate call path > for example. It happens in patch 4/4 where we call associate for a hostdev and there is no macvtap available there. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Support mac and port profile for
This patch series is based on laines patches to support . https://www.redhat.com/archives/libvir-list/2012-February/msg01126.html It support to set mac and port profile on an interface of type hostdev. * If virtualport is specified, the existing virtual port functions are called to set mac, vlan and port profile. * If virtualport is not specified and device is a sriov virtual function, - mac is set using IFLA_VF_MAC * If virtualport is not specified and device is a non-sriov virtual function, - mac is set using existing SIOCGIFHWADDR (This requires that the netdev be present on the host before starting the VM) This series implements the below : 01/4 pci: Add two new pci util pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile 02/4 virtnetdev: Add support functions for mac and portprofile associations on a hostdev 03/4 virnetdevvportprofile: Changes to support portprofiles for hostdevs 04/4 qemu_hostdev: Add support to install port profile and mac address on hostdev Stefan Berger is CC'ed for 802.1Qbg changes in patch 03/4. Current code for 802.1Qbg uses macvtap ifname. And for network interfaces with type=hostdev a macvtap ifname does not exist. This patch just adds a null check for ifname in 802.1Qbg port profile handling code. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] virtnetdev: Add support functions for mac and portprofile associations on a hostdev
From: Roopa Prabhu
This patch adds the following:
- functions to set and get vf configs
- Functions to replace and store vf configs (Only mac address is handled today.
But the functions can be easily extended for vlans and other vf configs)
- function to dump link dev info (This is moved from virnetdevvportprofile.c)
Signed-off-by: Roopa Prabhu
---
src/util/virnetdev.c | 531 ++
src/util/virnetdev.h | 19 ++
2 files changed, 549 insertions(+), 1 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 9d76d47..25f2155 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1127,8 +1127,497 @@ virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
return ret;
}
-#else /* !__linux__ */
+static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
+[IFLA_VF_MAC] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_mac) },
+[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
+.maxlen = sizeof(struct ifla_vf_vlan) },
+};
+
+/**
+ * virNetDevLinkDump:
+ *
+ * @ifname: The name of the interface; only use if ifindex < 0
+ * @ifindex: The interface index; may be < 0 if ifname is given
+ * @nltarget_kernel: whether to send the message to the kernel or another
+ * process
+ * @nlattr: pointer to a pointer of netlink attributes that will contain
+ * the results
+ * @recvbuf: Pointer to the buffer holding the returned netlink response
+ * message; free it, once not needed anymore
+ * @getPidFunc: Pointer to a function that will be invoked if the kernel
+ * is not the target of the netlink message but it is to be
+ * sent to another process.
+ *
+ * Get information about an interface given its name or index.
+ *
+ * Returns 0 on success, -1 on fatal error.
+ */
+int
+virNetDevLinkDump(const char *ifname, int ifindex,
+ bool nltarget_kernel, struct nlattr **tb,
+ unsigned char **recvbuf,
+ uint32_t (*getPidFunc)(void))
+{
+int rc = 0;
+struct nlmsghdr *resp;
+struct nlmsgerr *err;
+struct ifinfomsg ifinfo = {
+.ifi_family = AF_UNSPEC,
+.ifi_index = ifindex
+};
+unsigned int recvbuflen;
+uint32_t pid = 0;
+struct nl_msg *nl_msg;
+
+*recvbuf = NULL;
+
+if (ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0)
+return -1;
+
+ifinfo.ifi_index = ifindex;
+
+nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST);
+if (!nl_msg) {
+virReportOOMError();
+return -1;
+}
+
+if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+goto buffer_too_small;
+
+if (ifindex < 0 && ifname) {
+if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
+goto buffer_too_small;
+}
+
+if (!nltarget_kernel) {
+pid = getPidFunc();
+if (pid == 0) {
+rc = -1;
+goto cleanup;
+}
+}
+
+if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
+rc = -1;
+goto cleanup;
+}
+
+if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
+goto malformed_resp;
+
+resp = (struct nlmsghdr *)*recvbuf;
+
+switch (resp->nlmsg_type) {
+case NLMSG_ERROR:
+err = (struct nlmsgerr *)NLMSG_DATA(resp);
+if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+goto malformed_resp;
+
+if (err->error) {
+virReportSystemError(-err->error,
+ _("error dumping %s (%d) interface"),
+ ifname, ifindex);
+rc = -1;
+}
+break;
+
+case GENL_ID_CTRL:
+case NLMSG_DONE:
+rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
+ tb, IFLA_MAX, NULL);
+if (rc < 0)
+goto malformed_resp;
+break;
+
+default:
+goto malformed_resp;
+}
+
+if (rc != 0)
+VIR_FREE(*recvbuf);
+
+cleanup:
+nlmsg_free(nl_msg);
+
+return rc;
+
+malformed_resp:
+nlmsg_free(nl_msg);
+
+virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed netlink response message"));
+VIR_FREE(*recvbuf);
+return -1;
+
+buffer_too_small:
+nlmsg_free(nl_msg);
+
+virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("allocated netlink buffer is too small"));
+return -1;
+}
+
+static int
+virNetDevSetVfConfig(const char *ifname, int ifindex, int vf,
+ bool nltarget_kernel, const unsigned char *macaddr,
+ int vlanid, uint32_t (*getPidFunc)(void))
+{
+int rc = -1;
+struct nlmsghdr *resp;
+struct nlmsgerr *err;
+unsigned char *recvbu
[libvirt] [PATCH 4/4] qemu_hostdev: Add support to install port profile and mac address on hostdevs
From: Roopa Prabhu
These changes are applied only if the hostdev has a parent net device.
If the parent netdevice has virtual port information, the original virtualport
associate functions are called (these set and restore both mac and port profile
on an interface). Else, only mac address is set on the device
using other methods depending on if its a sriov device or not.
Changes also include hotplug pci devices
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_hostdev.c | 229 +--
src/qemu/qemu_hostdev.h |8 +-
src/qemu/qemu_hotplug.c | 19
3 files changed, 242 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b3cad8e..43d9dd7 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -29,6 +29,7 @@
#include "memory.h"
#include "pci.h"
#include "hostusb.h"
+#include "virnetdev.h"
static pciDeviceList *
qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
@@ -156,12 +157,131 @@ int qemuUpdateActivePciHostdevs(struct qemud_driver
*driver,
return 0;
}
+static int
+qemuDomainHostdevPciSysfsPath(virDomainHostdevDefPtr hostdev, char
**sysfs_path)
+{
+struct pci_config_address config_address;
+
+config_address.domain = hostdev->source.subsys.u.pci.domain;
+config_address.bus = hostdev->source.subsys.u.pci.bus;
+config_address.slot = hostdev->source.subsys.u.pci.slot;
+config_address.function = hostdev->source.subsys.u.pci.function;
+
+return pciConfigAddressToSysfsFile(&config_address, sysfs_path);
+}
+
+int
+qemuDomainHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev)
+{
+char *sysfs_path = NULL;
+int ret = -1;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+ret = pciDeviceIsVirtualFunction(sysfs_path);
+
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+static int
+qemuDomainHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev,
+ int *vf)
+{
+int ret = -1;
+char *sysfs_path = NULL;
+
+if (qemuDomainHostdevPciSysfsPath(hostdev, &sysfs_path) < 0)
+return ret;
+
+if (pciDeviceIsVirtualFunction(sysfs_path) == 1) {
+if (pciDeviceGetVirtualFunctionInfo(sysfs_path, linkdev,
+vf) < 0)
+goto cleanup;
+}
+else {
+if (pciDeviceNetName(sysfs_path, linkdev) < 0)
+goto cleanup;
+*vf = -1;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(sysfs_path);
+
+return ret;
+}
+
+int
+qemuDomainHostdevNetConfigReplace(virDomainHostdevDefPtr hostdev,
+ const unsigned char *uuid,
+ char *stateDir)
+{
+char *linkdev = NULL;
+virNetDevVPortProfilePtr virtPortProfile;
+int ret = -1;
+int vf = -1;
+int vlanid = -1;
+
+if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0)
+return ret;
+
+virtPortProfile = virDomainNetGetActualVirtPortProfile(
+ hostdev->parent.data.net);
+if (virtPortProfile)
+ret = virNetDevVPortProfileAssociate(NULL, virtPortProfile,
+ hostdev->parent.data.net->mac,
+ linkdev, vf, uuid,
+
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+else
+/* Set only mac */
+ret = virNetDevReplaceNetConfig(linkdev, vf,
+hostdev->parent.data.net->mac, vlanid,
+stateDir);
+
+VIR_FREE(linkdev);
+
+return ret;
+}
+
+int
+qemuDomainHostdevNetConfigRestore(virDomainHostdevDefPtr hostdev,
+ char *stateDir)
+{
+char *linkdev = NULL;
+virNetDevVPortProfilePtr virtPortProfile;
+int ret = -1;
+int vf = -1;
+
+if (qemuDomainHostdevNetDevice(hostdev, &linkdev, &vf) < 0)
+return ret;
+
+virtPortProfile = virDomainNetGetActualVirtPortProfile(
+ hostdev->parent.data.net);
+if (virtPortProfile)
+ret = virNetDevVPortProfileDisassociate(NULL, virtPortProfile,
+hostdev->parent.data.net->mac,
+linkdev, vf,
+
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY);
+else
+ret = virNetDevRestoreNetConfig(linkdev, vf, stateDir);
+
+VIR_FREE(linkdev);
+
+return ret;
+}
+
int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
const char *name,
+ const unsigned char *uuid,
virDomainHostdevDefPtr *hostd
[libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
From: Roopa Prabhu
This patch includes the following changes
- removes some netlink functions which are now available in virnetdev.c
- Adds a vf argument to all port profile functions
For 802.1Qbh devices, the port profile calls can use a vf argument if
passed by the caller. If the vf argument is -1 it will try to derive the vf
if the device passed is a virtual function.
For 802.1Qbg devices, This patch introduces a null check for the device
argument because during port profile assignment on a hostdev, this argument
can be null. Stefan CC'ed for comments
Signed-off-by: Roopa Prabhu
---
src/qemu/qemu_migration.c|2
src/util/virnetdevmacvlan.c |6 +
src/util/virnetdevvportprofile.c | 203 +-
src/util/virnetdevvportprofile.h |8 +
4 files changed, 42 insertions(+), 177 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7df2d4f..b80ed69 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2605,6 +2605,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def)
{
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
def->uuid,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0)
goto err_exit;
@@ -2622,6 +2623,7 @@ err_exit:
virDomainNetGetActualVirtPortProfile(net),
net->mac,
virDomainNetGetActualDirectDev(net),
+ -1,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
}
}
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 25d0846..498a2a0 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -486,6 +486,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
uint32_t macvtapMode;
const char *cr_ifname;
int ret;
+int vf = -1;
macvtapMode = modeMap[mode];
@@ -547,6 +548,7 @@ create_name:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmuuid, vmOp) < 0) {
rc = -1;
goto link_del_exit;
@@ -597,6 +599,7 @@ disassociate_exit:
virtPortProfile,
macaddress,
linkdev,
+ vf,
vmOp));
link_del_exit:
@@ -624,6 +627,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
char *stateDir)
{
int ret = 0;
+int vf = -1;
+
if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir));
}
@@ -633,6 +638,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
virtPortProfile,
macaddr,
linkdev,
+ vf,
VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0)
ret = -1;
if (virNetDevMacVLanDelete(ifname) < 0)
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index 67fd7bc..03c85dd 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -126,11 +126,6 @@ static struct nla_policy ifla_port_policy[IFLA_PORT_MAX +
1] =
{
[IFLA_PORT_RESPONSE] = { .type = NLA_U16 },
};
-static struct nla_policy ifla_policy[IFLA_MAX + 1] =
-{
- [IFLA_VF_PORTS] = { .type = NLA_NESTED },
-};
-
static uint32_t
virNetDevVPortProfileGetLldpadPid(void) {
@@ -164,126 +159,6 @@ virNetDevVPortProfileGetLldpadPid(void) {
return pid;
}
-
-/**
- * virNetDevVPortProfileLinkDump:
- *
- * @ifname: The name of the interface; only use if ifindex < 0
- * @ifindex: The interface index; may be < 0 if ifname is given
- * @nltarget_kernel: whether to send the message to the kernel or another
- * process
- * @nlattr: pointer to a pointer of netlink attributes that will contain
- * the results
- * @recvbuf: Pointer to the buf
[libvirt] [PATCH 1/4] pci: Add two new pci utils pciDeviceGetVirtualFunctionInfo and pciConfigAddressToSysfsFile
From: Roopa Prabhu
pciDeviceGetVirtualFunctionInfo returns pf netdevice name and virtual
function index for a given vf. This is just a wrapper around existing functions
to return vf's pf and vf_index with one api call
pciConfigAddressToSysfsfile returns the sysfile pci device link
from a 'struct pci_config_address'
Signed-off-by: Roopa Prabhu
---
src/util/pci.c | 55 +++
src/util/pci.h |7 +++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index c660e8d..d8c136e 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2081,6 +2081,20 @@ pciSysfsFile(char *pciDeviceName, char
**pci_sysfs_device_link)
return 0;
}
+int
+pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link)
+{
+if (virAsprintf(pci_sysfs_device_link,
+PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain,
+dev->bus, dev->slot, dev->function) < 0) {
+virReportOOMError();
+return -1;
+}
+
+return 0;
+}
+
/*
* Returns the network device name of a pci device
*/
@@ -2123,6 +2137,38 @@ out:
return ret;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index)
+{
+struct pci_config_address *pf_config_address = NULL;
+char *pf_sysfs_device_path = NULL;
+int ret = -1;
+
+if (pciGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address) < 0)
+return ret;
+
+if (pciConfigAddressToSysfsFile(pf_config_address,
+&pf_sysfs_device_path) < 0) {
+
+VIR_FREE(pf_config_address);
+return ret;
+}
+
+if (pciGetVirtualFunctionIndex(pf_sysfs_device_path, vf_sysfs_device_path,
+vf_index) < 0)
+goto cleanup;
+
+ret = pciDeviceNetName(pf_sysfs_device_path, pfname);
+
+cleanup:
+VIR_FREE(pf_config_address);
+VIR_FREE(pf_sysfs_device_path);
+
+return ret;
+}
+
#else
int
pciGetPhysicalFunction(const char *vf_sysfs_path ATTRIBUTE_UNUSED,
@@ -2170,4 +2216,13 @@ pciDeviceNetName(char *device_link_sysfs_path
ATTRIBUTE_UNUSED,
"supported on non-linux platforms"));
return -1;
}
+
+int
+pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceGetVirtualFunctionInfo "
+ "is not supported on non-linux platforms"));
+return -1;
+}
#endif /* __linux__ */
diff --git a/src/util/pci.h b/src/util/pci.h
index 25b5b66..b71bb12 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -111,6 +111,9 @@ int pciGetVirtualFunctionIndex(const char
*pf_sysfs_device_link,
const char *vf_sysfs_device_link,
int *vf_index);
+int pciConfigAddressToSysfsFile(struct pci_config_address *dev,
+char **pci_sysfs_device_link);
+
int pciDeviceNetName(char *device_link_sysfs_path, char **netname);
int pciSysfsFile(char *pciDeviceName, char **pci_sysfs_device_link)
@@ -122,4 +125,8 @@ int pciGetDeviceAddrString(unsigned domain,
unsigned function,
char **pciConfigAddr)
ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
+
+int pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
+char **pfname, int *vf_index);
+
#endif /* __VIR_PCI_H__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough ()
On 1/31/12 1:16 AM, "Laine Stump" wrote: > On 01/30/2012 08:16 PM, Roopa Prabhu wrote: >> Laine, I haven't gone through your whole email yet. Was just curious about >> one quick thing, >> >> For sriov VF's, are we expecting that a net device (eth interface) be >> present on the host if its being used as a hostdev ?. > > > Either should be possible. If the VF is bound to a net dev, it can be > specified either with dev='ethxx' or using its pci address, and will be > unbound before assigning to the guest. If it's not bound to a net dev, > then a pci address must be used to describe it in the config. > > >> If yes, then libvirt >> will need to do an unbind of the driver on the VF before assigning it to the >> VM. Which today it does not do (correct me if I am wrong). > > > That may have been the case in the past, but with libvirt-0.9.9 (the > first version that I've tested with sriov and PCI passthrough - I'm a > newbie to both), the net driver is unbound prior to assigning to the > guest, and when the the device is detached from the guest, the net > driver is once again bound to the device. > > >> Which is still >> ok. Just wanted to call that out. >> Plus ideally it would be nice to not have an expectation that a vf netdevice >> be present on the host. Because for sriov vf's, it would mean that the vf >> driver has to be loaded on the host. Which is really not required for vfs >> because mac and port profile can be set via the pf with the vf index as >> argument. > > Right. For sriov VFs, I intend that the MAC address will always be set > via the PF. I hadn't previously thought through the details for > non-sriov devices, but your event sequence list below made me realize > that, at least for non-sriov, the MAC address and virtual port setup > will need to be done *before* unbinding the driver (my intent, for sriov > at least, was that this would happen *after* unbinding the driver). I > guess that will take some experimentation. > > Anyway, at the moment I'm slogging around in the data structures. > ok. Thanks Laine. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough ()
On 1/30/12 11:14 AM, "Laine Stump" wrote:
> On 01/23/2012 09:08 AM, Paolo Bonzini wrote:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
> This is the model that I'm now following.
>
> Looking further into it, I've found that there are lots of places in the
> libvirt code that scan through all the entries, and call
> functions that expect a virDomainHostdevDef as an argument. Of course
> all those same places will need to be visited with devices that are
> assigned via (virDomainNetDef) as well (and this will also
> apparently be needed for devices in the near future).
>
> What I'm thinking of doing now, is changing virDomainHostdevDef in the
> following way:
>
> typedef virDomainDeviceSourceInfo *virDomainDeviceSourceInfoPtr;
> struct _virDomainDeviceSourceInfo {
> int mode; /* enum virDomainHostdevMode */
> bool managed;
> union {
> virDomainDeviceSubsysAddress subsys; /* USB or PCI */
> struct {
> int dummy;
> } caps;
> };
> virDomainHostdevOrigStates origstates;
> };
>
> typedef struct _virDomainHostdevDef virDomainHostdevDef;
> typedef virDomainHostdevDef *virDomainHostdevDefPtr;
> struct _virDomainHostdevDef {
> virDomainDeviceDefPtr parent; /* specific device containing
> this def */
> virDomainDeviceSourceInfo source; /* host address info */
> virDomainDeviceInfoPtrinfo; /* guest address info */
> };
>
>
> (note that "info" is now a separate object, rather than simply being
> contained in the HostdevDef!)
>
> This new HostdevDef can then be included directly in higher level device
> types, e.g:
>
> struct _virDomainNetDef {
> enum virDomainNetType type;
> unsigned char mac[VIR_MAC_BUFLEN];
> ...
> union {
> ...
> struct {
> char *linkdev;
> int mode; /* enum virMacvtapMode from util/macvtap.h */
> virNetDevVPortProfilePtr virtPortProfile;
> } direct;
> ** struct {
> ** virDomainHostdevDef def;
> } hostdev;
> } data;
> struct {
> bool sndbuf_specified;
> unsigned long sndbuf;
> } tune;
> ...
> char *ifname;
> virDomainDeviceInfo info;
> ...
> };
>
>
> for , the hostdev would be populated like this:
>
> (interface->data.hostdev.def.source will already be filled in from
> Parse)
>
> interface->data.hostdev.def.info = &interface->info;
> interface->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET;
> interface->data.hostdev.def.parent.data.net = interface;
>
> At this point, &interface->data.hostdev.def can be sent as a parameter
> to any function that's expecting a virDomainHostdevDef. Beyond that, I'm
> thinking it can *even be added to the hostdevs list in the DomainDef*.
> This would work in the following way:
>
> 0) If a parent device (in our example a virDomainNetDef) is
> type='hostdev', in addition to be included in its normal higher level
> device list (e.g. domain->nets), parent->data.hostdev will be filled in
> as indicated above, and &parent->data.hostdev will be added to the
> domain's hostdevs list.
>
> 1) When a function is scanning through all the hostdevs to do device
> management, it will act on this higher level device just as any generic
> device, except that there may be callouts to setup functions based on
> the value of hostdevs[n]->parent.type (e.g. to setup a MAC address or
> virtual port profile).
>
> 2) When an entry from the hostdevs list is being freed, any hostdev that
> has a non-NULL parent will simply be removed from the list (and a
> callout made to the equivalent function to remove the hostdev's parent
> from its own list).
>
> 3) When an entry from the higher level device list is being freed, it
> will also be removed from the hostdev list.
>
> 4) when one of these "intelligent hostdevs" is attached/detached,
> depending on hostdev->parent.type, it may callout to a device-specific
> function,
>
> By doing things this way, we assure that these new higher level devices
> will always be included in any scans of hostdevs, while avoiding the
> necessity to add a new loop to every one of the functions that scans
> them each time we add support for PCI passthrough of another
> higher-level device type.
>
>
> Does this sound reasonable? (I'm making a proof-of-concept now, but
> figured I'd solicit input in the meantime).
>
> ---
>
> The next problem: We will need to be able to configure everything that's
> in a from within an , but there are a few things we
> haven't discussed yet:
>
> 1) "type='pci'" vs. "type='usb'"
>
> has one of these directly as an attribute of the toplevel
> element, so it isn't given in the source element. In the case
> of , type is already used for something else in the toplevel
> element, but it can instead be given as part of the . So which
> do you think is better:
>
>
>
>
>
> ...
>
> or:
>
>
>
>
>
> ??
Re: [libvirt] [RFC Incomplete Patch] Libvirt + Openvswitch
On 1/27/12 11:22 AM, "kmestery" wrote: > Hi Dan: > > We at Cisco have been looking at this as well, and in fact were going to > propose some things in this area as well. Our proposal (which frankly just > builds on top of what you have): > > The proposal at hand is to add some libvirt configuration as follows: > > >ovs-network > > > > > > This would setup the OVS network entity, and sets up a forwarding mode of > "ovs", which indicates Open vSwitch is used to forward traffic. > > > > > > > > > This model of exposing parameters to virtualport types allows for further > expansion as new interface types and parameters are added. > > Thoughts? The existing forward mode bridge could also work perhaps. If there are no special cases for ovs. ovs-network Or maybe the real syntax is, ovs-network Thanks, Roopa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough ()
On 1/20/12 1:50 PM, "Laine Stump" wrote: > To refresh everyone's memory, the origin of the problem I'm trying to > solve here is that the VFs of an SRIOV-capable ethernet card are given > new random MAC addresses each time the card is initialized. If those VFs > are then passed-through to a guest using the existing config, > the guest will see a new MAC address each time the host is restarted, > and will thus believe that a new ethernet card has been installed. This > can result in anything from a dialog claiming that the guest has > connected to a new network (MS products) to a new network device name > showing up (Linux - "hmm, eth0 was unplugged, but here's this new > device. Let's call it "eth1"!) > > Several months ago I sent out some mail proposing a scheme for > automatically allocating network devices from a pool to be assigned to > guests via PCI passthrough: > > https://www.redhat.com/archives/libvir-list/2011-August/msg00937.html > > > My idea was to have a new forward mode combined with guest > definitions that would end up auto-generating a transient > entry in the guest's config (and setting the VF's mac address > in the process). Dan Berrange pointed out in that thread that we really > do need to have a persistent entry for these devices in the > domain xml, if for no other reason than to guarantee that the same > guest-side PCI address is always used (thus preventing surprises in the > guest, such as re-activation demands from Microsoft OSes). (There were > other reasons, but that one was the real "hard stop" for me.) > > I've come back to this problem, and have decided that, while having the > actual host device auto-allocated at runtime would be nice, first > implementing a less ambitious solution that uses a hand-picked device > would not preclude adding the more complicated/useful functionality > later. So, here's a new simpler proposal. > > > Step 1 > -- > > In the end, the solution will be that the VF's auto-generated random MAC > address should be replaced with a fixed MAC address supplied by libvirt > prior to assigning the VF to the guest. As a first step to satisfy this > basic requirement, I'm figuring to just extend the xml in this > way: > > | > | > | > | > | association (i.e. vepa / 802.1Qb[gh]). Would it be feasible to do that > association with the device after setting MAC address and before > assigning it to the guest? (and likewise for the inverse) Or would the > act of PCI assignment screw that up somehow? (one of the messages in the > earlier thread says something about the device initialization by the > guest un-doing necessary setup) (if it would work, a could > just be added along with ). Sorry for the late comment on this one. I have read the rest of the emails on this thread and I like where the discussions are going. I can speak for 802.1Qbh, and the virtual port association after setting the mac address and before assigning the device to the guest is the right direction to go. It will be similar to what we do for macvtap today. We will set mac (IFLA_VF_MAC) and set port profile (IFLA_VF_PORT) for the VF before the VM comes up. And the device initialization by the guest will not undo the mac and port profile configured by the hypervisor. Thanks for all the work on this and I would be happy to contribute in any way I can. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Bug Fix: Do not release network actual device in qemuBuildCommandLine on error
From: Roopa Prabhu For direct attach devices, in qemuBuildCommandLine, we seem to be freeing actual device on error path (with networkReleaseActualDevice). But the actual device is not deleted. qemuProcessStop eventually deletes the direct attach device and releases actual device. But by the time qemuProcessStop is called qemuBuildCommandLine has already freed actual device. Leaving stray macvtap devices behind on error. So the simplest fix is to remove the networkReleaseActualDevice in qemuBuildCommandLine. This patch does just that. Does this look right ?. I have only verified this with direct and bridge mode. The other option is to do both delMacvtap and networkReleaseActualDevice in qemuBuildCommandLine instead of doing only networkReleaseActualDevice. I do have a patch for this too. Signed-off-by: Roopa Prabhu --- src/qemu/qemu_command.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bb12016..ba33a4a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5378,8 +5378,6 @@ qemuBuildCommandLine(virConnectPtr conn, virReportOOMError(); error: /* free up any resources in the network driver */ -for (i = 0 ; i < def->nnets ; i++) -networkReleaseActualDevice(def->nets[i]); for (i = 0; i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); virCommandFree(cmd); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] macvtap: Fix error return value convention/inconsistencies
From: Roopa Prabhu
- changed some return 1's to return -1
- changed if (rc) error checks to if (rc < 0)
- fixed some other minor convention violations
I might have missed some. Can fix in another patch or can respin
Signed-off-by: Roopa Prabhu
Reported-by: Eric Blake
Reported-by: Laine Stump
---
src/util/interface.c |8 +++---
src/util/macvtap.c | 64 +++---
src/util/pci.c |6 ++---
3 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/src/util/interface.c b/src/util/interface.c
index 5d473b7..4ab74b5 100644
--- a/src/util/interface.c
+++ b/src/util/interface.c
@@ -1244,7 +1244,7 @@ ifaceIsVirtualFunction(const char *ifname)
char *if_sysfs_device_link = NULL;
int ret = -1;
-if (ifaceSysfsFile(&if_sysfs_device_link, ifname, "device"))
+if (ifaceSysfsFile(&if_sysfs_device_link, ifname, "device") < 0)
return ret;
ret = pciDeviceIsVirtualFunction(if_sysfs_device_link);
@@ -1272,10 +1272,10 @@ ifaceGetVirtualFunctionIndex(const char *pfname, const
char *vfname,
char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL;
int ret = -1;
-if (ifaceSysfsFile(&pf_sysfs_device_link, pfname, "device"))
+if (ifaceSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
return ret;
-if (ifaceSysfsFile(&vf_sysfs_device_link, vfname, "device")) {
+if (ifaceSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0) {
VIR_FREE(pf_sysfs_device_link);
return ret;
}
@@ -1306,7 +1306,7 @@ ifaceGetPhysicalFunction(const char *ifname, char
**pfname)
char *physfn_sysfs_path = NULL;
int ret = -1;
-if (ifaceSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn"))
+if (ifaceSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0)
return ret;
ret = pciDeviceNetName(physfn_sysfs_path, pfname);
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index f8b9d55..78e30f8 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -210,8 +210,11 @@ configMacvtapTap(int tapfd, int vnet_hdr)
rc_on_fail = -1;
errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap");
} else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) {
-if (ioctl(tapfd, TUNGETFEATURES, &features) != 0)
-return errno;
+if (ioctl(tapfd, TUNGETFEATURES, &features) < 0) {
+virReportSystemError(errno, "%s",
+ _("cannot get feature flags on macvtap tap"));
+return -1;
+ }
if ((features & IFF_VNET_HDR)) {
new_flags = ifreq.ifr_flags | IFF_VNET_HDR;
errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap");
@@ -335,7 +338,7 @@ create_name:
macaddress,
linkdev,
virtPortProfile,
- vmuuid, vmOp) != 0) {
+ vmuuid, vmOp) < 0) {
rc = -1;
goto link_del_exit;
}
@@ -352,7 +355,6 @@ create_name:
}
rc = openTap(cr_ifname, 10);
-
if (rc >= 0) {
if (configMacvtapTap(rc, vnet_hdr) < 0) {
VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
@@ -552,6 +554,8 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
}
}
+return rc;
+
err_exit:
if (msg)
macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", msg);
@@ -572,7 +576,7 @@ doPortProfileOpSetLink(bool nltarget_kernel,
int32_t vf,
uint8_t op)
{
-int rc = 0;
+int rc = -1;
struct nlmsghdr *resp;
struct nlmsgerr *err;
struct ifinfomsg ifinfo = {
@@ -588,7 +592,7 @@ doPortProfileOpSetLink(bool nltarget_kernel,
nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST);
if (!nl_msg) {
virReportOOMError();
-return -1;
+return rc;
}
if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
@@ -690,16 +694,12 @@ doPortProfileOpSetLink(bool nltarget_kernel,
if (!nltarget_kernel) {
pid = getLldpadPid();
-if (pid == 0) {
-rc = -1;
+if (pid == 0)
goto err_exit;
-}
}
-if (nlComm(nl_msg, &recvbuf, &recvbuflen, pid) < 0) {
-rc = -1;
+if (nlComm(nl_msg, &recvbuf, &recvbuflen, pid) < 0)
goto err_exit;
-}
if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL)
goto malformed_resp;
@@ -716,7 +716,7 @@ doPortProfileOpSetLink(bool nltarget_kernel,
virReportSystemError(-err->error,
_("error during virtual port configuration of ifindex %d&quo
Re: [libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
On 10/27/11 7:01 PM, "Eric Blake" wrote: > On 10/27/2011 05:57 PM, Roopa Prabhu wrote: >>> >> Also, looks like no where else in libvirt code we return errno. We only >> print errno but always return -1. And the -ETIMEDOUT case in macvtap is an >> exception, cause the upper layer requires the cause of this particular >> error. > > We have a few places that return -1 on most errors, 0 on normal success, > and -2 on timeout; that is, the < 0 check can filter out all > non-success, but callers that care about the particular failure can > compare against -1. > > If all errors can map to errno, we do have some examples of API that > return -errno; but those tend to be lower level API (lots in > util/util.c, not so many elsewhere); it tends to be easier to return -1 > and document if errno is set to a sane value on failure. > >> I don't mind changing everything to errno. But that seems to be not the >> convention. And I cant find a clean way to not return -ETIMEDOUT. I can >> however return status of the command in an argument and return -1 for the >> ETIMEDOUT case. I will do that unless you have other suggestions. Thanks. > > Returning -1 and -errno in the same function doesn't work (since -1 > would be ambiguous with EACCES, on some systems), but returning -1 vs. > -2 to distinguish timeout from normal errors is fine. Ok thanks eric. This helps. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
On 10/26/11 8:01 PM, "Roopa Prabhu" wrote:
>
>
>
> On 10/24/11 11:14 AM, "Stefan Berger" wrote:
>
>> On 10/20/2011 01:46 PM, Roopa Prabhu wrote:
>>> From: Roopa Prabhu
>>>
>>> Fixes some cases where 1 was being returned instead of -1.
>>> There are still some inconsistencies in the file with respect
>>> to what the return variable is initialized to. Can be fixed
>>> as a separate patch if needed. The scope of this patch is just
>>> to fix the return value 1. Did some basic sanity test.
>>>
>>> Signed-off-by: Roopa Prabhu
>>> Reported-by: Eric Blake
>>> ---
>>> src/util/macvtap.c | 22 --
>>> 1 files changed, 8 insertions(+), 14 deletions(-)
>>>
>>>
>>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>>> index 7fd6eb5..f8b9d55 100644
>>> --- a/src/util/macvtap.c
>>> +++ b/src/util/macvtap.c
>>> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
>>>bool is8021Qbg,
>>>uint16_t *status)
>>> {
>>> -int rc = 1;
>>> +int rc = -1;
>>> const char *msg = NULL;
>>> struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>>>
>>> @@ -806,7 +806,7 @@ sassm...@redhat.com(bool nltarget_kernel,
>>> _("error %d during port-profile setlink on "
>>> "interface %s (%d)"),
>>> status, ifname, ifindex);
>>> -rc = 1;
>>> +rc = -1;
>>> break;
>>> }
>>>
>> In this function we later on return a -ETIMEDOUT. The -1 doesn't overlap
>> with it, but I am wondering whether we should return -EFAULT in the
>> places of -1 now ?
>>
> Instead of defaulting to -EFAULT, shall I just add a function to map the
> port profile/VDP status codes to closest errno (with default being unknown
> error) and use that instead ?.
> Also, for some reason we are reporting EINVAL in the virReportSystemError
> just above it. EINVAL also does not seem right there.
>
> I can fix it.
> Thanks,
> Roopa
>
Also, looks like no where else in libvirt code we return errno. We only
print errno but always return -1. And the -ETIMEDOUT case in macvtap is an
exception, cause the upper layer requires the cause of this particular
error.
I don't mind changing everything to errno. But that seems to be not the
convention. And I cant find a clean way to not return -ETIMEDOUT. I can
however return status of the command in an argument and return -1 for the
ETIMEDOUT case. I will do that unless you have other suggestions. Thanks.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
On 10/24/11 11:14 AM, "Stefan Berger" wrote:
> On 10/20/2011 01:46 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> Fixes some cases where 1 was being returned instead of -1.
>> There are still some inconsistencies in the file with respect
>> to what the return variable is initialized to. Can be fixed
>> as a separate patch if needed. The scope of this patch is just
>> to fix the return value 1. Did some basic sanity test.
>>
>> Signed-off-by: Roopa Prabhu
>> Reported-by: Eric Blake
>> ---
>> src/util/macvtap.c | 22 --
>> 1 files changed, 8 insertions(+), 14 deletions(-)
>>
>>
>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>> index 7fd6eb5..f8b9d55 100644
>> --- a/src/util/macvtap.c
>> +++ b/src/util/macvtap.c
>> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
>>bool is8021Qbg,
>>uint16_t *status)
>> {
>> -int rc = 1;
>> +int rc = -1;
>> const char *msg = NULL;
>> struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>>
>> @@ -806,7 +806,7 @@ sassm...@redhat.com(bool nltarget_kernel,
>> _("error %d during port-profile setlink on "
>> "interface %s (%d)"),
>> status, ifname, ifindex);
>> -rc = 1;
>> +rc = -1;
>> break;
>> }
>>
> In this function we later on return a -ETIMEDOUT. The -1 doesn't overlap
> with it, but I am wondering whether we should return -EFAULT in the
> places of -1 now ?
>
Instead of defaulting to -EFAULT, shall I just add a function to map the
port profile/VDP status codes to closest errno (with default being unknown
error) and use that instead ?.
Also, for some reason we are reporting EINVAL in the virReportSystemError
just above it. EINVAL also does not seem right there.
I can fix it.
Thanks,
Roopa
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
On 10/20/11 11:57 AM, "Laine Stump" wrote:
> On 10/20/2011 01:46 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> Fixes some cases where 1 was being returned instead of -1.
>> There are still some inconsistencies in the file with respect
>> to what the return variable is initialized to. Can be fixed
>> as a separate patch if needed. The scope of this patch is just
>> to fix the return value 1. Did some basic sanity test.
>
> This patch hasn't changed the checks of the return code made by callers
> to these functions - a spot check showed several that still say "if (rc)
> { " rather than "if (rc < 0) { ". While that is technically
> correct, it is inconsistent with the preferred style in libvirt, and I'm
> guessing that style is at least partly the reason for making this patch
> in the first place :-).
> As long as you're going to make this change, you
> may as well trace back up the call chain for all changed functions and
> fix the callers to be consistent.
Actually I did trace the call chain for every change and I thought if (rc)
for negative values was just fine. Dint realize that libvirt preferred style
was if (rc < 0)
>
> (I also noticed at least one place that uses "xxx() != 0" instead of
> "xxx() < 0". Making all of these comparisons consistent will make it
> much easier for someone auditing the code in the future to understand
> that the "errors are < 0" convention has been followed for these functions.)
>
Yes I do agree that this file has some inconsistency in the checks. I
noticed that and also listed one of them in the log comment for this patch.
And only removed the return 1.
Agree that its good to fix all inconsistencies, I will fix them all and
respin.
Thanks!
Roopa
>> Signed-off-by: Roopa Prabhu
>> Reported-by: Eric Blake
>> ---
>> src/util/macvtap.c | 22 --
>> 1 files changed, 8 insertions(+), 14 deletions(-)
>>
>>
>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>> index 7fd6eb5..f8b9d55 100644
>> --- a/src/util/macvtap.c
>> +++ b/src/util/macvtap.c
>> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
>>bool is8021Qbg,
>>uint16_t *status)
>> {
>> -int rc = 1;
>> +int rc = -1;
>> const char *msg = NULL;
>> struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>>
>> @@ -806,7 +806,7 @@ doPortProfileOpCommon(bool nltarget_kernel,
>> _("error %d during port-profile setlink on "
>> "interface %s (%d)"),
>> status, ifname, ifindex);
>> -rc = 1;
>> +rc = -1;
>> break;
>> }
>>
>> @@ -867,7 +867,7 @@ doPortProfileOp8021Qbg(const char *ifname,
>> const virVirtualPortProfileParamsPtr virtPort,
>> enum virVirtualPortOp virtPortOp)
>> {
>> -int rc;
>> +int rc = -1;
>>
>> # ifndef IFLA_VF_PORT_MAX
>>
>> @@ -877,7 +877,6 @@ doPortProfileOp8021Qbg(const char *ifname,
>> (void)virtPortOp;
>> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>>_("Kernel VF Port support was missing at compile time."));
>> -rc = 1;
>>
>> # else /* IFLA_VF_PORT_MAX */
>>
>> @@ -893,10 +892,8 @@ doPortProfileOp8021Qbg(const char *ifname,
>> int vf = PORT_SELF_VF;
>>
>> if (getPhysdevAndVlan(ifname,&physdev_ifindex, physdev_ifname,
>> -&vlanid) != 0) {
>> -rc = 1;
>> +&vlanid) != 0)
>> goto err_exit;
>> -}
>>
>> if (vlanid< 0)
>> vlanid = 0;
>> @@ -918,7 +915,6 @@ doPortProfileOp8021Qbg(const char *ifname,
>> default:
>> macvtapError(VIR_ERR_INTERNAL_ERROR,
>>_("operation type %d not supported"), virtPortOp);
>> -rc = 1;
>> goto err_exit;
>> }
>>
>> @@ -982,7 +978,7 @@ doPortProfileOp8021Qbh(const char *ifname,
>> const unsigned char *vm_uuid,
>> enum virVirtualPortOp virtPortOp)
>> {
>> -int rc;
>> +int rc = -1;
>>
>> # ifndef IFLA_VF_PORT_MAX
>>
>> @@ -993,7 +989,6 @@ doPortProfileOp8021Qbh(const char *ifname,
>> (void)virtPortOp;
>> macvtapError(VIR_ERR_INTERNAL_E
Re: [libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
On 10/20/11 10:22 AM, "Srivatsa S. Bhat"
wrote:
> On 10/20/2011 10:42 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> Fixes some cases where 1 was being returned instead of -1.
>> There are still some inconsistencies in the file with respect
>> to what the return variable is initialized to. Can be fixed
>> as a separate patch if needed. The scope of this patch is just
>> to fix the return value 1. Did some basic sanity test.
>>
>> Signed-off-by: Roopa Prabhu
>> Reported-by: Eric Blake
>
> Are you sure about Eric's email id?
> I thought Eric Blake worked for Redhat :-)
>
Oops... Sorry cut-copy-paste error. Thanks ;)
Pls ignore this patch. I will respin. :)
>> ---
>> src/util/macvtap.c | 22 --
>> 1 files changed, 8 insertions(+), 14 deletions(-)
>>
>>
>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>> index 7fd6eb5..f8b9d55 100644
>> --- a/src/util/macvtap.c
>> +++ b/src/util/macvtap.c
>> @@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
>> bool is8021Qbg,
>> uint16_t *status)
>> {
>> -int rc = 1;
>> +int rc = -1;
>> const char *msg = NULL;
>> struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
>>
>> @@ -806,7 +806,7 @@ doPortProfileOpCommon(bool nltarget_kernel,
>> _("error %d during port-profile setlink on "
>>"interface %s (%d)"),
>> status, ifname, ifindex);
>> -rc = 1;
>> +rc = -1;
>> break;
>> }
>>
>> @@ -867,7 +867,7 @@ doPortProfileOp8021Qbg(const char *ifname,
>> const virVirtualPortProfileParamsPtr virtPort,
>> enum virVirtualPortOp virtPortOp)
>> {
>> -int rc;
>> +int rc = -1;
>>
>> # ifndef IFLA_VF_PORT_MAX
>>
>> @@ -877,7 +877,6 @@ doPortProfileOp8021Qbg(const char *ifname,
>> (void)virtPortOp;
>> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Kernel VF Port support was missing at compile time."));
>> -rc = 1;
>>
>> # else /* IFLA_VF_PORT_MAX */
>>
>> @@ -893,10 +892,8 @@ doPortProfileOp8021Qbg(const char *ifname,
>> int vf = PORT_SELF_VF;
>>
>> if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname,
>> - &vlanid) != 0) {
>> -rc = 1;
>> + &vlanid) != 0)
>> goto err_exit;
>> -}
>>
>> if (vlanid < 0)
>> vlanid = 0;
>> @@ -918,7 +915,6 @@ doPortProfileOp8021Qbg(const char *ifname,
>> default:
>> macvtapError(VIR_ERR_INTERNAL_ERROR,
>> _("operation type %d not supported"), virtPortOp);
>> -rc = 1;
>> goto err_exit;
>> }
>>
>> @@ -982,7 +978,7 @@ doPortProfileOp8021Qbh(const char *ifname,
>> const unsigned char *vm_uuid,
>> enum virVirtualPortOp virtPortOp)
>> {
>> -int rc;
>> +int rc = -1;
>>
>> # ifndef IFLA_VF_PORT_MAX
>>
>> @@ -993,7 +989,6 @@ doPortProfileOp8021Qbh(const char *ifname,
>> (void)virtPortOp;
>> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Kernel VF Port support was missing at compile time."));
>> -rc = 1;
>>
>> # else /* IFLA_VF_PORT_MAX */
>>
>> @@ -1008,10 +1003,9 @@ doPortProfileOp8021Qbh(const char *ifname,
>> if (rc)
>> goto err_exit;
>>
>> -if (ifaceGetIndex(true, physfndev, &ifindex) < 0) {
>> -rc = 1;
>> +rc = ifaceGetIndex(true, physfndev, &ifindex);
>> +if (rc < 0)
>> goto err_exit;
>> -}
>>
>> switch (virtPortOp) {
>> case PREASSOCIATE_RR:
>> @@ -1059,7 +1053,7 @@ doPortProfileOp8021Qbh(const char *ifname,
>> default:
>> macvtapError(VIR_ERR_INTERNAL_ERROR,
>> _("operation type %d not supported"), virtPortOp);
>> -rc = 1;
>> +rc = -1;
>> }
>>
>> err_exit:
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
From: Roopa Prabhu
Fixes some cases where 1 was being returned instead of -1.
There are still some inconsistencies in the file with respect
to what the return variable is initialized to. Can be fixed
as a separate patch if needed. The scope of this patch is just
to fix the return value 1. Did some basic sanity test.
Signed-off-by: Roopa Prabhu
Reported-by: Eric Blake
---
src/util/macvtap.c | 22 --
1 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 7fd6eb5..f8b9d55 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
bool is8021Qbg,
uint16_t *status)
{
-int rc = 1;
+int rc = -1;
const char *msg = NULL;
struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
@@ -806,7 +806,7 @@ doPortProfileOpCommon(bool nltarget_kernel,
_("error %d during port-profile setlink on "
"interface %s (%d)"),
status, ifname, ifindex);
-rc = 1;
+rc = -1;
break;
}
@@ -867,7 +867,7 @@ doPortProfileOp8021Qbg(const char *ifname,
const virVirtualPortProfileParamsPtr virtPort,
enum virVirtualPortOp virtPortOp)
{
-int rc;
+int rc = -1;
# ifndef IFLA_VF_PORT_MAX
@@ -877,7 +877,6 @@ doPortProfileOp8021Qbg(const char *ifname,
(void)virtPortOp;
macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Kernel VF Port support was missing at compile time."));
-rc = 1;
# else /* IFLA_VF_PORT_MAX */
@@ -893,10 +892,8 @@ doPortProfileOp8021Qbg(const char *ifname,
int vf = PORT_SELF_VF;
if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname,
- &vlanid) != 0) {
-rc = 1;
+ &vlanid) != 0)
goto err_exit;
-}
if (vlanid < 0)
vlanid = 0;
@@ -918,7 +915,6 @@ doPortProfileOp8021Qbg(const char *ifname,
default:
macvtapError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
-rc = 1;
goto err_exit;
}
@@ -982,7 +978,7 @@ doPortProfileOp8021Qbh(const char *ifname,
const unsigned char *vm_uuid,
enum virVirtualPortOp virtPortOp)
{
-int rc;
+int rc = -1;
# ifndef IFLA_VF_PORT_MAX
@@ -993,7 +989,6 @@ doPortProfileOp8021Qbh(const char *ifname,
(void)virtPortOp;
macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Kernel VF Port support was missing at compile time."));
-rc = 1;
# else /* IFLA_VF_PORT_MAX */
@@ -1008,10 +1003,9 @@ doPortProfileOp8021Qbh(const char *ifname,
if (rc)
goto err_exit;
-if (ifaceGetIndex(true, physfndev, &ifindex) < 0) {
-rc = 1;
+rc = ifaceGetIndex(true, physfndev, &ifindex);
+if (rc < 0)
goto err_exit;
-}
switch (virtPortOp) {
case PREASSOCIATE_RR:
@@ -1059,7 +1053,7 @@ doPortProfileOp8021Qbh(const char *ifname,
default:
macvtapError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
-rc = 1;
+rc = -1;
}
err_exit:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] macvtap: Fix error return values to -1 instead of 1
From: Roopa Prabhu
Fixes some cases where 1 was being returned instead of -1.
There are still some inconsistencies in the file with respect
to what the return variable is initialized to. Can be fixed
as a separate patch if needed. The scope of this patch is just
to fix the return value 1. Did some basic sanity test.
Signed-off-by: Roopa Prabhu
Reported-by: Eric Blake
---
src/util/macvtap.c | 22 --
1 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 7fd6eb5..f8b9d55 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -480,7 +480,7 @@ getPortProfileStatus(struct nlattr **tb, int32_t vf,
bool is8021Qbg,
uint16_t *status)
{
-int rc = 1;
+int rc = -1;
const char *msg = NULL;
struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, };
@@ -806,7 +806,7 @@ doPortProfileOpCommon(bool nltarget_kernel,
_("error %d during port-profile setlink on "
"interface %s (%d)"),
status, ifname, ifindex);
-rc = 1;
+rc = -1;
break;
}
@@ -867,7 +867,7 @@ doPortProfileOp8021Qbg(const char *ifname,
const virVirtualPortProfileParamsPtr virtPort,
enum virVirtualPortOp virtPortOp)
{
-int rc;
+int rc = -1;
# ifndef IFLA_VF_PORT_MAX
@@ -877,7 +877,6 @@ doPortProfileOp8021Qbg(const char *ifname,
(void)virtPortOp;
macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Kernel VF Port support was missing at compile time."));
-rc = 1;
# else /* IFLA_VF_PORT_MAX */
@@ -893,10 +892,8 @@ doPortProfileOp8021Qbg(const char *ifname,
int vf = PORT_SELF_VF;
if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname,
- &vlanid) != 0) {
-rc = 1;
+ &vlanid) != 0)
goto err_exit;
-}
if (vlanid < 0)
vlanid = 0;
@@ -918,7 +915,6 @@ doPortProfileOp8021Qbg(const char *ifname,
default:
macvtapError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
-rc = 1;
goto err_exit;
}
@@ -982,7 +978,7 @@ doPortProfileOp8021Qbh(const char *ifname,
const unsigned char *vm_uuid,
enum virVirtualPortOp virtPortOp)
{
-int rc;
+int rc = -1;
# ifndef IFLA_VF_PORT_MAX
@@ -993,7 +989,6 @@ doPortProfileOp8021Qbh(const char *ifname,
(void)virtPortOp;
macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Kernel VF Port support was missing at compile time."));
-rc = 1;
# else /* IFLA_VF_PORT_MAX */
@@ -1008,10 +1003,9 @@ doPortProfileOp8021Qbh(const char *ifname,
if (rc)
goto err_exit;
-if (ifaceGetIndex(true, physfndev, &ifindex) < 0) {
-rc = 1;
+rc = ifaceGetIndex(true, physfndev, &ifindex);
+if (rc < 0)
goto err_exit;
-}
switch (virtPortOp) {
case PREASSOCIATE_RR:
@@ -1059,7 +1053,7 @@ doPortProfileOp8021Qbh(const char *ifname,
default:
macvtapError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
-rc = 1;
+rc = -1;
}
err_exit:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add missing strdup return value check
On 10/14/11 4:22 PM, "Eric Blake" wrote:
> On 10/14/2011 04:25 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> Check strdup return value and fail if error
>>
>> Signed-off-by: Roopa Prabhu
>> ---
>> src/util/pci.c |8 +++-
>> 1 files changed, 7 insertions(+), 1 deletions(-)
>>
>>
>> diff --git a/src/util/pci.c b/src/util/pci.c
>> index 2bbb90c..df4e6c8 100644
>> --- a/src/util/pci.c
>> +++ b/src/util/pci.c
>> @@ -2018,7 +2018,13 @@ pciDeviceNetName(char *device_link_sysfs_path, char
>> **netname)
>>
>> /* Assume a single directory entry */
>> *netname = strdup(entry->d_name);
>> -ret = 0;
>> +if (!*netname) {
>> +virReportOOMError();
>> +ret = -1;
>
> ret is already -1
>
>> +}
>> +else {
>
> style - "} else {" should always be on one line. But without the
> redundant ret assignment, you don't need braces in the first place.
>
>> +ret = 0;
>> +}
>
> ACK (only affects OOM corner case), and I'm pushing with this squashed in:
>
> diff --git i/src/util/pci.c w/src/util/pci.c
> index df4e6c8..33b4b0e 100644
> --- i/src/util/pci.c
> +++ w/src/util/pci.c
> @@ -2018,13 +2018,10 @@ pciDeviceNetName(char *device_link_sysfs_path,
> char **netname)
>
> /* Assume a single directory entry */
> *netname = strdup(entry->d_name);
> -if (!*netname) {
> +if (!*netname)
> virReportOOMError();
> -ret = -1;
> -}
> -else {
> +else
> ret = 0;
> -}
> break;
>}
Ah..thanks.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add missing strdup return value check
From: Roopa Prabhu
Check strdup return value and fail if error
Signed-off-by: Roopa Prabhu
---
src/util/pci.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 2bbb90c..df4e6c8 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2018,7 +2018,13 @@ pciDeviceNetName(char *device_link_sysfs_path, char
**netname)
/* Assume a single directory entry */
*netname = strdup(entry->d_name);
-ret = 0;
+if (!*netname) {
+virReportOOMError();
+ret = -1;
+}
+else {
+ret = 0;
+}
break;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: plug memory leak for 802.1Qbh
On 10/14/11 2:08 PM, "Eric Blake" wrote: > On 10/14/2011 03:05 PM, Roopa Prabhu wrote: >>> I had already pushed mine. Yours is missing a virReportOOMError() on >>> allocation failure; but ACK with that improvement. >>> >> Great. Thanks!. I will send out a patch for the bug fix and later work on >> the fixing the error return convention. > > I already pushed the bug fix in your name: > > commit 80b077ee5ea0dd899b87d370f9fa892e727832f5 > Author: Roopa Prabhu > Date: Fri Oct 14 13:41:46 2011 -0700 > > macvtap: avoid invalid free > > Commit 0472f39 plugged a leak, but introduced another bug: > > Actually looks like physfndev is conditionally allocated in > getPhysfnDev > Its better to modify getPhysfnDev to allocate physfndev every time. > > so all that remains is cleaning up the error return convention. Thanks eric. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: plug memory leak for 802.1Qbh
On 10/14/11 1:55 PM, "Eric Blake" wrote:
> On 10/14/2011 02:41 PM, Roopa Prabhu wrote:
>>
>>
>>
>> On 10/13/11 3:49 PM, "Eric Blake" wrote:
>>
>>> Detected by Coverity. Leak present since commit ca3b22b.
>>>
>>> * src/util/macvtap.c (doPortProfileOp8021Qbh): Release device name.
>>> ---
>>> getPhysfnDev allocates physfndev, but nothing freed it.
>>> Pushing under the trivial rule.
>>
>> Actually looks like physfndev is conditionally allocated in getPhysfnDev
>> Its better to modify getPhysfnDev to allocate physfndev every time.
>
> Also a good catch - otherwise we might have a double free or otherwise
> crash on freeing an invalid pointer.
>
>>
>>
>> I ACK this patch with the additional change below (looks ok ?)
>>
>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>> index a020c90..b50b7d2 100644
>> --- a/src/util/macvtap.c
>> +++ b/src/util/macvtap.c
>> @@ -964,7 +964,7 @@ getPhysfnDev(const char *linkdev,
>>*/
>>
>> *vf = PORT_SELF_VF;
>> -*physfndev = (char *)linkdev;
>> +*physfndev = strdup(linkdev);
>
> I had already pushed mine. Yours is missing a virReportOOMError() on
> allocation failure; but ACK with that improvement.
>
> Meanwhile, we need to scrub this file - it uses a convention of 1 on
> error, instead of the more typical -1 on error in the rest of the code
> base; but I want this bug fix to be separate from that subsequent cleanup.
>
> diff --git i/src/util/macvtap.c w/src/util/macvtap.c
> index b50b7d2..33f0a13 100644
> --- i/src/util/macvtap.c
> +++ w/src/util/macvtap.c
> @@ -965,6 +965,10 @@ getPhysfnDev(const char *linkdev,
>
> *vf = PORT_SELF_VF;
> *physfndev = strdup(linkdev);
> +if (!*physfndev) {
> +virReportOOMError();
> +rc = -1;
> +}
> }
>
> return rc;
>
Great. Thanks!. I will send out a patch for the bug fix and later work on
the fixing the error return convention.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap: plug memory leak for 802.1Qbh
On 10/13/11 3:49 PM, "Eric Blake" wrote:
> Detected by Coverity. Leak present since commit ca3b22b.
>
> * src/util/macvtap.c (doPortProfileOp8021Qbh): Release device name.
> ---
> getPhysfnDev allocates physfndev, but nothing freed it.
> Pushing under the trivial rule.
>
> src/util/macvtap.c |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 9bf7fa6..a020c90 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -983,27 +983,27 @@ doPortProfileOp8021Qbh(const char *ifname,
> # ifndef IFLA_VF_PORT_MAX
>
> (void)ifname;
> (void)macaddr;
> (void)virtPort;
> (void)vm_uuid;
> (void)virtPortOp;
> macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Kernel VF Port support was missing at compile time."));
> rc = 1;
>
> # else /* IFLA_VF_PORT_MAX */
>
> -char *physfndev;
> +char *physfndev = NULL;
> unsigned char hostuuid[VIR_UUID_BUFLEN];
> int32_t vf;
> bool nltarget_kernel = true;
> int ifindex;
> int vlanid = -1;
>
> rc = getPhysfnDev(ifname, &vf, &physfndev);
> if (rc)
> goto err_exit;
>
> if (ifaceGetIndex(true, physfndev, &ifindex) < 0) {
> rc = 1;
> goto err_exit;
> @@ -1049,26 +1049,27 @@ doPortProfileOp8021Qbh(const char *ifname,
> NULL,
> NULL,
> vf,
> PORT_REQUEST_DISASSOCIATE);
> break;
>
> default:
> macvtapError(VIR_ERR_INTERNAL_ERROR,
> _("operation type %d not supported"), virtPortOp);
> rc = 1;
> }
>
> err_exit:
> +VIR_FREE(physfndev);
Thanks for catching this Eric
Actually looks like physfndev is conditionally allocated in getPhysfnDev
Its better to modify getPhysfnDev to allocate physfndev every time.
I ACK this patch with the additional change below (looks ok ?)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index a020c90..b50b7d2 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -964,7 +964,7 @@ getPhysfnDev(const char *linkdev,
*/
*vf = PORT_SELF_VF;
-*physfndev = (char *)linkdev;
+*physfndev = strdup(linkdev);
}
return rc;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4 v3] macvtap: Support for sending port profile message for a SRIOV VF to its PF
On 8/16/11 9:02 AM, "Stefan Berger" wrote: > On 08/16/2011 12:28 AM, Roopa Prabhu wrote: >> This patch tries to fix getPhysFn in macvtap.c to get the physical >> function(PF) of the direct attach interface, if the interface is a SR-IOV VF. >> >> It moves some of the sriov pci device handling code from node_device_driver >> to src/util/pci.[ch]. >> >> This patch series implements the following >> 01/4 - pci: Move some pci sriov helper code out of node device driver to >> util/pci >> 02/4 - pci: Add helper functions for sriov devices >> 03/4 - interface: Add functions to get sriov PF/VF relationship of a net >> interface >> 04/4 - macvtap: Fix getPhysfn to get the PF of a direct attach network >> interface > I now checked the functions into the repository fixing the nits on the way. > Thanks stefan. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4 v3] interface: Add functions to get sriov PF/VF relationship of a net interface
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
src/util/interface.c | 148 ++
src/util/interface.h |9 +++
2 files changed, 157 insertions(+), 0 deletions(-)
diff --git a/src/util/interface.c b/src/util/interface.c
index 8b4522b..aec12f5 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,149 @@ ifaceRestoreMacAddress(const char *linkdev,
return rc;
}
+
+#ifdef __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 -1;
+}
+
+int
+ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname,
+ int *vf_index)
+{
+ifaceError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ifaceGetVirtualFunctionIndex is not supported on non-linux "
+ "platforms"));
+return -1;
+}
+
+int
+ifaceGetPhysicalFunction(const char *ifname, char **pfname)
+{
+ifaceError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ifaceGetPhysicalFunction is not supported on non-linux "
+ "platforms"));
+return -1;
+}
+#endif /* __linux__ */
diff --git a/src/util/interface.h b/src/util/interface.h
index 47c0eb0..7be1444 100644
--- a/src/util/interface.h
+++ b/src/util/interface.h
@@ -27,6 +27,8
[libvirt] [PATCH 2/4 v3] pci: Add helper functions for sriov devices
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
src/util/pci.c | 146
src/util/pci.h |8 +++
2 files changed, 154 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index c3f3bb4..099c9d3 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1685,6 +1685,19 @@ int pciDeviceIsAssignable(pciDevice *dev,
return 1;
}
+/*
+ * returns 0 if equal and 1 if not
+ */
+static int
+pciConfigAddressEqual(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));
+}
+
static int
logStrToLong_ui(char const *s,
char **end_ptr,
@@ -1889,6 +1902,112 @@ out:
return ret;
}
+
+/*
+ * Returns 1 if vf device is a virtual function, 0 if not, -1 on error
+ */
+int
+pciDeviceIsVirtualFunction(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
+pciGetVirtualFunctionIndex(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 (pciGetVirtualFunctions(pf_sysfs_device_link, &virt_fns,
+&num_virt_fns)) {
+pciReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Error getting physical function's '%s' "
+ "virtual_functions"), pf_sysfs_device_link);
+goto out;
+}
+
+for (i = 0; i < num_virt_fns; i++) {
+ if (pciConfigAddressEqual(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
+pciDeviceNetName(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 (STREQ(entry->d_name, ".") ||
+STREQ(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;
+}
#else
int
pciGetPhysicalFunction(const char *vf_sysfs_path,
@@ -1908,4 +2027,31 @@ pciGetVirtualFunctions(const char *sysfs_path,
"supported on non-linux platforms"));
return -1;
}
+
+int
+pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceIsVirtualFunction is "
+ "not supported on non-linux platforms"));
+return -1;
+}
+
+int
+pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link,
+ const char *vf_sysfs_device_link,
+ int *vf_index)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetVirtualFunctionIndex is "
+ "not supported on non-linux platforms"));
+return -1;
+
+}
+
+int
+pciDeviceNetName(char *device_link_sysfs_path, char **netname)
+{
+pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciDeviceNetName is not
[libvirt] [PATCH 4/4 v3] macvtap: Fix getPhysfn to get the PF of a direct attach network interface
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4 v3] pci: Move some pci sriov helper code out of node device driver to util/pci
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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| 230 +
src/util/pci.h| 14 ++
9 files changed, 265 insertions(+), 217 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index cf7c003..8fe7120 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -954,7 +954,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_
[libvirt] [PATCH 0/4 v3] macvtap: Support for sending port profile message for a SRIOV VF to its PF
This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It moves some of the sriov pci device handling code from node_device_driver to src/util/pci.[ch]. This patch series implements the following 01/4 - pci: Move some pci sriov helper code out of node device driver to util/pci 02/4 - pci: Add helper functions for sriov devices 03/4 - interface: Add functions to get sriov PF/VF relationship of a net interface 04/4 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Changelog: -- v1: RFC patch introduced new functions to get PF/VF relationship of SRIOV net devices by comparing PF/VF sysfs device links. Also mentioned the existance of some related code in node_device_driver. The feedback was to move and use the node_device_driver code v2: Moves sriov get_physical_function and get_virtual_functions from node_device_driver to src/util/pci*. And reworks the rest of the patches around the new code. v3: Incorporated Stefan Berger's comments. Fixed indentation errors shown by 'make syntax-check'. Used pciReportError in src/util/pci Signed-off-by: Roopa Prabhu Signed-off-by: Christian Benvenuti Signed-off-by: David Wang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4 v2] interface: Add functions to get sriov PF/VF relationship of a net interface
On 8/15/11 4:13 AM, "Stefan Berger" wrote:
> On 08/12/2011 07:14 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> 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
>> Signed-off-by: Christian Benvenuti
>> Signed-off-by: David Wang
>> ---
>> 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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4 v2] pci: Add helper functions for sriov devices
On 8/15/11 4:03 AM, "Stefan Berger" wrote:
> On 08/12/2011 07:14 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> 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
>> Signed-off-by: Christian Benvenuti
>> Signed-off-by: David Wang
>> ---
>> 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
> pciConfigAddressEqual and return '1' in case the addresses are equal?
Agree. Will implement it as pciConfigAddressEqual.
>> +
>> +/*
>> + * 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, ".") ||
>> +
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 8/15/11 3:47 AM, "Stefan Berger" wrote:
> On 08/12/2011 07:14 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> 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
>> Signed-off-by: Christian Benvenuti
>> Signed-off-by: David Wang
>> ---
>> 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_vi
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 8/15/11 3:58 AM, "Stefan Berger" 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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4 v2] macvtap: Fix getPhysfn to get the PF of a direct attach network interface
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4 v2] interface: Add functions to get sriov PF/VF relationship of a net interface
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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;
+}
+
+int
+ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname,
+ int *vf_index)
+{
+ifaceError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ifaceGetVirtualFunctionIndex is not supported on non-linux "
+ "platforms"));
+return -ENOSYS;
+}
+
+int
+ifaceGetPhysicalFunction(const char *ifname, char **pfname)
+{
+ifaceError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ifaceGetPhysicalFunction is not supported on non-linux "
+ "platforms"));
+return -ENOSYS;
+}
+
+#endif /* __linux__ */
diff --git a/src/util/interface.h b/src/util/interface.h
index 47c0eb0..bdd37e5 100644
--- a/src/util/interface.h
+++ b/src/util/inter
[libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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->
[libvirt] [PATCH 0/4 v2] macvtap: Support for sending port profile message for a SRIOV VF to its PF
This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It moves some of the sriov pci device handling code from node_device_driver to src/util/pci.[ch]. This patch series implements the following 01/3 - pci: Move some pci sriov helper code out of node device driver to util/pci 02/3 - pci: Add helper functions for sriov devices 02/3 - interface: Add functions to get sriov PF/VF relationship of a net interface 03/3 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Changelog: -- v1: RFC patch introduced new functions to get PF/VF relationship of SRIOV net devices by comparing PF/VF sysfs device links. Also mentioned the existance of some related code in node_device_driver. The feedback was to move and use the node_device_driver code v2: Moves sriov get_physical_function and get_virtual_functions from node_device_driver to src/util/pci*. And reworks the rest of the patches around the new code. Signed-off-by: Roopa Prabhu Signed-off-by: Christian Benvenuti Signed-off-by: David Wang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4 v2] pci: Add helper functions for sriov devices
From: Roopa Prabhu
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
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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));
+}
+
+/*
+ * 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)
+# define pciGetVirtualFunctionIndex(p,v,i)
+# define pciDeviceNetNameLinux(d,n)
#endif
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3 RFC] Add functions to get sriov PF/VF relationship of a network interface
On 8/2/11 6:46 AM, "Stefan Berger" wrote:
> On 08/02/2011 08:46 AM, Roopa Prabhu wrote:
>>
>>
>> On 8/1/11 6:10 PM, "Stefan Berger" wrote:
>>
>>> On 08/01/2011 07:57 PM, Roopa Prabhu wrote:
>>>> From: Roopa Prabhu
>>>>
>>>> This patch adds helper functions to derive the PF/VF relationship of an
>>>> sriov
>>>> network device
>>>>
>>>> Signed-off-by: Roopa Prabhu
>>>> Signed-off-by: Christian Benvenuti
>>>> Signed-off-by: David Wang
>>>> ---
>>>>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
>>>>#include
>>>>#include
>>>> +#include
>>>>
>>>>#ifdef __linux__
>>>># include
>>>> @@ -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;
>>>> +}
>>>> +
>>&g
Re: [libvirt] [PATCH 0/3 RFC] macvtap: Implement getPhysfn to support sending a port profile message for a VF to its PF
On 8/2/11 4:15 AM, "Daniel P. Berrange" wrote: > On Mon, Aug 01, 2011 at 09:16:09PM -0400, Dave Allan wrote: >> On Mon, Aug 01, 2011 at 04:56:58PM -0700, Roopa Prabhu wrote: >>> This patch tries to fix getPhysFn in macvtap.c to get the physical >>> function(PF) of the direct attach interface, if the interface is a SR-IOV >>> VF. >>> It derives the PF/VF relationship from sysfs. >>> >>> There is some code to do this in node device driver. But it is local >>> to the node device driver . I did not see a clean way to use some of the >>> node device stuff here. The node device driver looks at PCI capabilities to >>> get the same information. >> >> We should not have two implementations of this functionality in the >> code. Either the node device code should be made to use this version >> or vice versa. > > We already have a file src/util/pci.c that contains a bunch of helper > APIs for dealing with PCI devices. If we need some shared code between > macvtap and the node device driver, then we ought to put it in the pci.c > file > Yes, I looked at it and saw that I had to move quite a few node_device data structures and code in src/util/pci.c. Wasn't sure if there was a reason for it to not already exist in src/util/pci.c. So decided to post this patch as RFC for specific directions. >From the patches that I posted, I still need the stuff that tries to derive the net device from the pci device. I will re-evaluate moving the node device stuff to get PF-VF relationship information into src/util/pci.c and repost the patches. Thanks for the comments. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3 RFC] Add functions to get sriov PF/VF relationship of a network interface
On 8/1/11 6:10 PM, "Stefan Berger" wrote:
> On 08/01/2011 07:57 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> This patch adds helper functions to derive the PF/VF relationship of an sriov
>> network device
>>
>> Signed-off-by: Roopa Prabhu
>> Signed-off-by: Christian Benvenuti
>> Signed-off-by: David Wang
>> ---
>> 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
>> #include
>> #include
>> +#include
>>
>> #ifdef __linux__
>> # include
>> @@ -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"
pointing to the VF's pci device.
Example:
# ls -l virtfn*
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn0 -> ../:1e:00.1
lrwxrwxrwx. 1 root root 0 Jul 27 11:09 virtfn1 -> ../:1e:00.2
lrwxrwxrwx.
Re: [libvirt] [PATCH 1/3 RFC] Add function to get the network interface name of a pci device
On 8/1/11 6:00 PM, "Stefan Berger" wrote:
> On 08/01/2011 07:57 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu
>>
>> This function looks at sysfs "net" directory to get the network interface
>> name associated with the pci device
>>
>> Signed-off-by: Roopa Prabhu
>> Signed-off-by: Christian Benvenuti
>> Signed-off-by: David Wang
>> ---
>> src/util/pci.c | 42 ++
>> src/util/pci.h |2 ++
>> 2 files changed, 44 insertions(+), 0 deletions(-)
>>
>>
>> diff --git a/src/util/pci.c b/src/util/pci.c
>> index a79c164..d2deeef 100644
>> --- a/src/util/pci.c
>> +++ b/src/util/pci.c
>> @@ -1679,3 +1679,45 @@ int pciDeviceIsAssignable(pciDevice *dev,
>>
>> return 1;
>> }
>> +
>> +/*
>> + * This function returns the network device name
>> + * of a pci device
>> + */
>> +int
>> +pciDeviceGetNetName(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 (entry->d_name[0] == '.' ||
> The above check makes the following one obsolete. If all entries with
> first letter '.' can be skipped, then you could just keep the first one,
> otherwise I'd also use !strcmp(entry->d_name, ".").
Agree, yes I should use strcmp. Did not think of all cases here because the
net dir normally contains one entry. I will also revisit this check for
cases when there might be more net devices associated with a pci device.
>> + !strcmp(entry->d_name, ".."))
>> + continue;
>> +
>> + /* Assume a single directory entry */
>> + *netname = strdup(entry->d_name);
>> + ret = 0;
>> + break;
>> +}
>> +
>> +if (dir)
>> +closedir(dir);
> Check for 'dir != NULL) is not necessary due to goto above.
Yes will fix. Thanks stefan.
>> +
>> +out:
>> +VIR_FREE(pcidev_sysfs_net_path);
>> +
>> +return ret;
>> +}
>> diff --git a/src/util/pci.h b/src/util/pci.h
>> index a351baf..fa25472 100644
>> --- a/src/util/pci.h
>> +++ b/src/util/pci.h
>> @@ -74,4 +74,6 @@ int pciDeviceIsAssignable(pciDevice *dev,
>> int strict_acs_check);
>> int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);
>>
>> +int pciDeviceGetNetName(char *device_link_sysfs_path, char **netname);
>> +
>> #endif /* __VIR_PCI_H__ */
>>
> Stefan
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3 RFC] Add functions to get sriov PF/VF relationship of a network interface
From: Roopa Prabhu
This patch adds helper functions to derive the PF/VF relationship of an sriov
network device
Signed-off-by: Roopa Prabhu
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
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
#include
#include
+#include
#ifdef __linux__
# include
@@ -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"));
+ret = 0;
+}
+
+VIR_FREE(device_link);
+VIR_FREE(device_path);
+
+if ( ret == 0 )
+break;
+}
+}
+
+if (dir)
+closedir(dir);
+
+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__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3 RFC] macvtap: Fix getPhysfn to get the PF of a direct attach network interface
From: Roopa Prabhu
This patch adds code to the existing getPhysfn to get the PF device name
and VF index if the linkdev is a sriov VF.
The idea is to send the port profile message to a PF if the direct attach
interface is a VF. Eventually interface.c is probably the right place for
getPhysfn
Signed-off-by: Roopa Prabhu
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
src/util/macvtap.c | 15 ++-
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 0b00776..7fcea0d 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -948,18 +948,15 @@ getPhysfn(const char *linkdev,
char **physfndev)
{
int rc = 0;
-bool virtfn = false;
-if (virtfn) {
+if (ifaceIsVF(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 physfndev = PF device */
+rc = ifaceGetPFName(linkdev, physfndev);
+if (!rc)
+rc = ifaceGetVFIndex(*physfndev, linkdev, vf);
} else {
/* Not SR-IOV VF: physfndev is linkdev and VF index
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3 RFC] macvtap: Implement getPhysfn to support sending a port profile message for a VF to its PF
This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It derives the PF/VF relationship from sysfs. There is some code to do this in node device driver. But it is local to the node device driver . I did not see a clean way to use some of the node device stuff here. The node device driver looks at PCI capabilities to get the same information. This implementation tries to not get into PCI capability details and just looks at sysfs paths to derive the PF-VF relationship This patch series implements the following 01/3 - Add function to get the network interface name of a pci device 02/3 - Add functions to get sriov PF/VF relationship of a network interface 03/3 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Signed-off-by: Roopa Prabhu Signed-off-by: Christian Benvenuti Signed-off-by: David Wang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3 RFC] Add function to get the network interface name of a pci device
From: Roopa Prabhu
This function looks at sysfs "net" directory to get the network interface
name associated with the pci device
Signed-off-by: Roopa Prabhu
Signed-off-by: Christian Benvenuti
Signed-off-by: David Wang
---
src/util/pci.c | 42 ++
src/util/pci.h |2 ++
2 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index a79c164..d2deeef 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1679,3 +1679,45 @@ int pciDeviceIsAssignable(pciDevice *dev,
return 1;
}
+
+/*
+ * This function returns the network device name
+ * of a pci device
+ */
+int
+pciDeviceGetNetName(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 (entry->d_name[0] == '.' ||
+ !strcmp(entry->d_name, ".."))
+ continue;
+
+ /* Assume a single directory entry */
+ *netname = strdup(entry->d_name);
+ ret = 0;
+ break;
+}
+
+if (dir)
+closedir(dir);
+
+out:
+VIR_FREE(pcidev_sysfs_net_path);
+
+return ret;
+}
diff --git a/src/util/pci.h b/src/util/pci.h
index a351baf..fa25472 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -74,4 +74,6 @@ int pciDeviceIsAssignable(pciDevice *dev,
int strict_acs_check);
int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);
+int pciDeviceGetNetName(char *device_link_sysfs_path, char **netname);
+
#endif /* __VIR_PCI_H__ */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] 8021Qbh: use preassociate-rr during the migration prepare stage
From: Roopa Prabhu
This patch introduces PREASSOCIATE-RR during incoming VM migration on the
destination host. This is similar to the usage of PREASSOCIATE during
migration in 8021qbg libvirt code today. PREASSOCIATE-RR is a VDP operation.
With the latest at IEEE, 8021qbh will need to support VDP operations.
A corresponding enic driver patch to support PREASSOCIATE-RR for 8021qbh
will be posted for net-next-2.6 inclusion soon.
Signed-off-by: Roopa Prabhu
Signed-off-by: David Wang
Signed-off-by: Christian Benvenuti
---
src/util/macvtap.c | 15 ++-
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 2d3ff87..346eaf6 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -87,6 +87,7 @@ enum virVirtualPortOp {
ASSOCIATE = 0x1,
DISASSOCIATE = 0x2,
PREASSOCIATE = 0x3,
+PREASSOCIATE_RR = 0x4,
};
@@ -1452,6 +1453,7 @@ doPortProfileOp8021Qbh(const char *ifname,
}
switch (virtPortOp) {
+case PREASSOCIATE_RR:
case ASSOCIATE:
rc = virGetHostUUID(hostuuid);
if (rc)
@@ -1465,7 +1467,9 @@ doPortProfileOp8021Qbh(const char *ifname,
vm_uuid,
hostuuid,
vf,
- PORT_REQUEST_ASSOCIATE);
+ (virtPortOp == PREASSOCIATE_RR) ?
+PORT_REQUEST_PREASSOCIATE_RR
+: PORT_REQUEST_ASSOCIATE);
if (rc == -ETIMEDOUT)
/* Association timed out, disassociate */
doPortProfileOpCommon(nltarget_kernel, NULL, ifindex,
@@ -1553,10 +1557,11 @@ vpAssociatePortProfileId(const char *macvtap_ifname,
break;
case VIR_VIRTUALPORT_8021QBH:
-/* avoid associating twice */
-if (vmOp != VIR_VM_OP_MIGRATE_IN_FINISH)
-rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr,
-virtPort, vmuuid, ASSOCIATE);
+rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr,
+virtPort, vmuuid,
+(vmOp == VIR_VM_OP_MIGRATE_IN_START)
+ ? PREASSOCIATE_RR
+ : ASSOCIATE);
if (vmOp != VIR_VM_OP_MIGRATE_IN_START && !rc)
ifaceUp(linkdev);
break;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] 802.1Qbh: Delay IFF_UP'ing interface until migration final stage
From: Roopa Prabhu Current code does an IFF_UP on a 8021Qbh interface immediately after a port profile set. This is ok in most cases except when its the migration prepare stage. During migration we want to postpone IFF_UP'ing the interface on the destination host until the source host has disassociated the interface. This patch moves IFF_UP of the interface to the final stage of migration. The motivation for this change is to postpone any addr registrations on the destination host until the source host has done the addr deregistrations. While at it, for symmetry with associate move ifDown of a 8021Qbh interface to before disassociate Signed-off-by: Roopa Prabhu Signed-off-by: David Wang Signed-off-by: Christian Benvenuti --- src/util/macvtap.c | 13 ++--- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 8814400..a71db86 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -1470,8 +1470,6 @@ doPortProfileOp8021Qbh(const char *ifname, NULL, vf, PORT_REQUEST_DISASSOCIATE); -if (!rc) -ifaceUp(ifname); break; case DISASSOCIATE: @@ -1484,7 +1482,6 @@ doPortProfileOp8021Qbh(const char *ifname, NULL, vf, PORT_REQUEST_DISASSOCIATE); -ifaceDown(ifname); break; default: @@ -1550,10 +1547,11 @@ vpAssociatePortProfileId(const char *macvtap_ifname, case VIR_VIRTUALPORT_8021QBH: /* avoid associating twice */ -if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH) -break; -rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr, -virtPort, vmuuid, ASSOCIATE); +if (vmOp != VIR_VM_OP_MIGRATE_IN_FINISH) +rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr, +virtPort, vmuuid, ASSOCIATE); +if (vmOp != VIR_VM_OP_MIGRATE_IN_START && !rc) +ifaceUp(linkdev); break; } @@ -1600,6 +1598,7 @@ vpDisassociatePortProfileId(const char *macvtap_ifname, /* avoid disassociating twice */ if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH) break; +ifaceDown(linkdev); rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr, virtPort, NULL, DISASSOCIATE); break; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt PATCH] 802.1Qbh: Add support for IFLA_VF_MAC
From: Roopa Prabhu
Current code does not pass VM mac address to a 802.1Qbh direct attach
interface using IFLA_VF_MAC. This patch adds support in macvtap code to
send IFLA_VF_MAC netlink request during port profile association on a
802.1Qbh interface.
Stefan Cc'ed for comments because this patch changes a condition for
802.1Qbg
802.1Qbh support for IFLA_VF_MAC in enic driver has been posted and is
pending acceptance at http://marc.info/?l=linux-netdev&m=129185244410557&w=2
Signed-off-by: Roopa Prabhu
Signed-off-by: David Wang
Signed-off-by: Christian Benvenuti
---
src/util/macvtap.c | 59
1 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 4345d97..96df301 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -1031,19 +1031,8 @@ doPortProfileOpSetLink(bool nltarget_kernel,
nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
goto buffer_too_small;
-if (macaddr && vlanid >= 0) {
+if (macaddr || vlanid >= 0) {
struct nlattr *vfinfolist, *vfinfo;
-struct ifla_vf_mac ifla_vf_mac = {
-.vf = vf,
-.mac = { 0, },
-};
-struct ifla_vf_vlan ifla_vf_vlan = {
-.vf = vf,
-.vlan = vlanid,
-.qos = 0,
-};
-
-memcpy(ifla_vf_mac.mac, macaddr, 6);
if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST)))
goto buffer_too_small;
@@ -1051,13 +1040,30 @@ doPortProfileOpSetLink(bool nltarget_kernel,
if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO)))
goto buffer_too_small;
-if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
- &ifla_vf_mac) < 0)
-goto buffer_too_small;
+if (macaddr) {
+struct ifla_vf_mac ifla_vf_mac = {
+.vf = vf,
+.mac = { 0, },
+};
-if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
- &ifla_vf_vlan) < 0)
-goto buffer_too_small;
+memcpy(ifla_vf_mac.mac, macaddr, 6);
+
+if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac),
+&ifla_vf_mac) < 0)
+goto buffer_too_small;
+}
+
+if (vlanid >= 0) {
+struct ifla_vf_vlan ifla_vf_vlan = {
+.vf = vf,
+.vlan = vlanid,
+.qos = 0,
+};
+
+if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan),
+&ifla_vf_vlan) < 0)
+goto buffer_too_small;
+}
nla_nest_end(nl_msg, vfinfo);
nla_nest_end(nl_msg, vfinfolist);
@@ -1402,6 +1408,7 @@ getPhysfn(const char *linkdev,
static int
doPortProfileOp8021Qbh(const char *ifname,
+ const unsigned char *macaddr,
const virVirtualPortProfileParamsPtr virtPort,
const unsigned char *vm_uuid,
enum virVirtualPortOp virtPortOp)
@@ -1411,6 +1418,7 @@ doPortProfileOp8021Qbh(const char *ifname,
# ifndef IFLA_VF_PORT_MAX
(void)ifname;
+(void)macaddr;
(void)virtPort;
(void)vm_uuid;
(void)virtPortOp;
@@ -1426,7 +1434,6 @@ doPortProfileOp8021Qbh(const char *ifname,
bool nltarget_kernel = true;
int ifindex;
int vlanid = -1;
-const unsigned char *macaddr = NULL;
rc = getPhysfn(ifname, &vf, &physfndev);
if (rc)
@@ -1456,7 +1463,7 @@ doPortProfileOp8021Qbh(const char *ifname,
/* Association timed out, disassociate */
doPortProfileOpCommon(nltarget_kernel, NULL, ifindex,
NULL,
- 0,
+ vlanid,
NULL,
NULL,
NULL,
@@ -1470,7 +1477,7 @@ doPortProfileOp8021Qbh(const char *ifname,
case DISASSOCIATE:
rc = doPortProfileOpCommon(nltarget_kernel, NULL, ifindex,
NULL,
- 0,
+ vlanid,
NULL,
NULL,
NULL,
@@ -1545,9 +1552,8 @@ vpAssociatePortProfileId(const char *macvtap_ifname,
/* avoid associating twice */
if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH)
break;
-rc = doPortProfileOp8021Qbh(linkdev, virtPort,
-vmuuid,
-ASSOCIATE);
+rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr,
+virtPort, vmuuid, ASSOCIATE);
break;
}
@@ -1594,9 +1600,8 @@ vpDisassociatePortP
