Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On Feb 29, 2012, at 5:36 PM, Laine Stump wrote: On 02/24/2012 09:29 AM, D. Herrendoerfer wrote: The callback mechanism is not re-armed when libvirt is restarted now. The reason is: lldpad remembers who sent the associate by pid - since in theory there could be multiple agents performing associations. So if the libvirt pid changes, there is little we can do now. This seems very problematic to me - it is assumed that libvirt can be restarted at any time with no ill effects, and restarting libvirt is often suggested as a solution to clearing up problems (e.g. if some other program stomps on libvirt's iptables rules). Yes. That's the way it should be. What can be done to eliminate this problem? Perhaps there needs to be a re-associate message - it could be sent each time libvirt restarts for each association it's currently tracking. I love the idea. When libvirt starts up. It probes for running VM with vepa associations, sends one associate and re-armes the callback. This would even help situations when the switch infrastructure is out- of-sync with the host system. But how ? Put a special setup function in virnetdevmacvlan.c and call it from domain_conf once all info has been collected and the VM is in running state ? Dirk H -- 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
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On 02/24/2012 09:29 AM, D. Herrendoerfer wrote: The callback mechanism is not re-armed when libvirt is restarted now. The reason is: lldpad remembers who sent the associate by pid - since in theory there could be multiple agents performing associations. So if the libvirt pid changes, there is little we can do now. This seems very problematic to me - it is assumed that libvirt can be restarted at any time with no ill effects, and restarting libvirt is often suggested as a solution to clearing up problems (e.g. if some other program stomps on libvirt's iptables rules). What can be done to eliminate this problem? Perhaps there needs to be a re-associate message - it could be sent each time libvirt restarts for each association it's currently tracking. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On Feb 23, 2012, at 11:16 PM, Laine Stump wrote:
On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.
The main issue I see with this patch is with whitespace, but that can
easily be fixed prior to pushing it, so ACK. Is the message format
used
by this patch, the absolute final version? (i.e. can we safely push it
an know that it will be correct?)
The patch to libvirt was picked this week. So yes, we can assume that
the
message will not be changed. But as always : Never say never !
The callback mechanism is not re-armed when libvirt is restarted now.
The reason is: lldpad remembers who sent the associate by pid - since
in theory there could be multiple agents performing associations.
So if the libvirt pid changes, there is little we can do now.
And thanks !
Dirk H
Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c | 315
+-
src/util/virnetdevvportprofile.c | 33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
net-mac,
virDomainNetGetActualDirectDev(net),
def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0)
goto err_exit;
}
last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/
virnetdevmacvlan.c
index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
passthrough)
#if WITH_MACVTAP
-
spurious whitespace change.
# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include linux/if.h
# include linux/if_tun.h
+# include c-ctype.h
+
/* Older kernels lacked this enum value. */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
# define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+
# define MACVTAP_NAME_PREFIXmacvtap
# define MACVTAP_NAME_PATTERN macvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIXmacvlan
# define MACVLAN_NAME_PATTERN macvlan%d
+
Another spurious whitespace change.
/**
* virNetDevMacVLanCreate:
*
@@ -445,6 +449,293 @@ static const uint32_t
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
[VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};
+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+ char cr_ifname[64];
+ virNetDevVPortProfilePtr virtPortProfile;
+ const unsigned char *macaddress;
+ const char *linkdev;
+ const unsigned char *vmuuid;
+ enum virNetDevVPortProfileOp vmOp;
+ unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t
size)
+{
+if (dst size INSTANCE_STRLEN) {
+snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x-
+%02x%02x-%02x%02x%02x%02x%02x%02x,
+p[0], p[1], p[2], p[3],
+p[4], p[5], p[6], p[7],
+p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
+return 0;
+}
+return -1;
+}
+
+# define LLDPAD_PID_FILE /var/run/lldpad.pid
+# define VIRIP_PID_FILE /var/run/virip.pid
+
+/**
+ * virNetDevMacVLanVPortProfileCallback:
+ *
+ * @msg: The buffer containing the received netlink message
+ * @length: The length of the received netlink message.
+ * @peer: The netling sockaddr containing the peer information
+ * @handeled: Contains information if the message has been replied
to yet
+ * @opaque: Contains vital information regarding the associated vm
an interface
+ *
+ * This function is called when a netlink message is received. The
function
+ * reads the message and responds if it is pertinent to the
running VMs
+ * network interface.
+ */
+
+static void
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On Feb 24, 2012, at 3:29 PM, D. Herrendoerfer wrote:
On Feb 23, 2012, at 11:16 PM, Laine Stump wrote:
On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.
The main issue I see with this patch is with whitespace, but that can
easily be fixed prior to pushing it, so ACK. Is the message format
used
by this patch, the absolute final version? (i.e. can we safely push
it
an know that it will be correct?)
The patch to libvirt was picked this week. So yes, we can assume
that the
message will not be changed. But as always : Never say never !
The patch to lldpad (Doh!) was picked ...
The callback mechanism is not re-armed when libvirt is restarted now.
The reason is: lldpad remembers who sent the associate by pid - since
in theory there could be multiple agents performing associations.
So if the libvirt pid changes, there is little we can do now.
And thanks !
Dirk H
Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c | 315 +++
++-
src/util/virnetdevvportprofile.c | 33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@
qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
net-mac,
virDomainNetGetActualDirectDev(net),
def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0)
goto err_exit;
}
last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/
virnetdevmacvlan.c
index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
passthrough)
#if WITH_MACVTAP
-
spurious whitespace change.
# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include linux/if.h
# include linux/if_tun.h
+# include c-ctype.h
+
/* Older kernels lacked this enum value. */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
# define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+
# define MACVTAP_NAME_PREFIXmacvtap
# define MACVTAP_NAME_PATTERN macvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIXmacvlan
# define MACVLAN_NAME_PATTERN macvlan%d
+
Another spurious whitespace change.
/**
* virNetDevMacVLanCreate:
*
@@ -445,6 +449,293 @@ static const uint32_t
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
[VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};
+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+ char cr_ifname[64];
+ virNetDevVPortProfilePtr virtPortProfile;
+ const unsigned char *macaddress;
+ const char *linkdev;
+ const unsigned char *vmuuid;
+ enum virNetDevVPortProfileOp vmOp;
+ unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t
size)
+{
+if (dst size INSTANCE_STRLEN) {
+snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x-
+%02x%02x-%02x%02x%02x%02x%02x%02x,
+p[0], p[1], p[2], p[3],
+p[4], p[5], p[6], p[7],
+p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
+return 0;
+}
+return -1;
+}
+
+# define LLDPAD_PID_FILE /var/run/lldpad.pid
+# define VIRIP_PID_FILE /var/run/virip.pid
+
+/**
+ * virNetDevMacVLanVPortProfileCallback:
+ *
+ * @msg: The buffer containing the received netlink message
+ * @length: The length of the received netlink message.
+ * @peer: The netling sockaddr containing the peer information
+ * @handeled: Contains information if the message has been
replied to yet
+ * @opaque: Contains vital information regarding the associated
vm an interface
+ *
+ * This function is called when a netlink message is received.
The function
+ * reads the message and
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
While going through this code to clean up the white-space problems, I
found 3 issues that need to be addressed before I can push it. Sorry I
missed these before.
If you can base the new (and I hope final! :-) version on the version
where I've already corrected the whitespace, that would be very helpful
(otherwise I'll need to merge back with my updated version, a
mechanical, but time consuming process). Rather than post them to the
mailing list, I've pushed my updated versions of your patches to the
lldpad2 branch of my staging repo on gitorious:
git://gitorious.org/~laine/libvirt/laine-staging.git
You should be able to get that branch into your local repo like this:
git remote add laine-staging \
git://gitorious.org/~laine/libvirt/laine-staging.git
git fetch laine-staging
git checkout laine-staging/llpad2
You can then cherry-pick those commits into your own branch, or create a
local branch based on it, then just squash in changes addressing the three
problems I point out below.
Once these are fixed, I will ACK and push it.
Thanks!
On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.
Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c | 315
+-
src/util/virnetdevvportprofile.c | 33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr
def) {
net-mac,
virDomainNetGetActualDirectDev(net),
def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0)
goto err_exit;
}
last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
passthrough)
#if WITH_MACVTAP
-
# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include linux/if.h
# include linux/if_tun.h
+# include c-ctype.h
+
/* Older kernels lacked this enum value. */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
# define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+
# define MACVTAP_NAME_PREFIX macvtap
# define MACVTAP_NAME_PATTERNmacvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIX macvlan
# define MACVLAN_NAME_PATTERNmacvlan%d
+
/**
* virNetDevMacVLanCreate:
*
@@ -445,6 +449,293 @@ static const uint32_t
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
[VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};
+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+ char cr_ifname[64];
Issue 1, part 1:
This should either be IFNAMSIZ bytes long, or be a char* (pointing to
separately allocated data) instead of a fixed size. This string is
copied into strings that are sent to the kernel for interface names, and
interface names have a maximum size of IFNAMSIZE (16); if you need
consistency of the name in all places (even in log messages), then you
should limit it at the source, but otherwise there's no sense in
checking string sizes both at this level and then again at the lower level.
My preference would be a char*, I think - leave it to the lower levels
to decide if it needs to be chopped.
+ virNetDevVPortProfilePtr virtPortProfile;
+ const unsigned char *macaddress;
+ const char *linkdev;
+ const unsigned char *vmuuid;
+ enum virNetDevVPortProfileOp vmOp;
+ unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t size)
+{
+if (dst size INSTANCE_STRLEN) {
+
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On 02/22/2012 08:17 AM, D. Herrendoerfer wrote:
From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
Add de-association handling for 802.1qbg (vepa) via lldpad
netlink messages. Also adds the possibility to perform an
association request without waiting for a confirmation.
The main issue I see with this patch is with whitespace, but that can
easily be fixed prior to pushing it, so ACK. Is the message format used
by this patch, the absolute final version? (i.e. can we safely push it
an know that it will be correct?)
Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name
---
src/qemu/qemu_migration.c|2 +-
src/util/virnetdevmacvlan.c | 315
+-
src/util/virnetdevvportprofile.c | 33 +++--
src/util/virnetdevvportprofile.h |3 +-
4 files changed, 339 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 12cfbde..31428f8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr
def) {
net-mac,
virDomainNetGetActualDirectDev(net),
def-uuid,
-
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0)
+
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0)
goto err_exit;
}
last_good_net = i;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 25d0846..b3e3325 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
passthrough)
#if WITH_MACVTAP
-
spurious whitespace change.
# include stdint.h
# include stdio.h
# include errno.h
@@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include linux/if.h
# include linux/if_tun.h
+# include c-ctype.h
+
/* Older kernels lacked this enum value. */
# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU
# define MACVLAN_MODE_PASSTHRU 8
@@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# include virfile.h
# include virnetlink.h
# include virnetdev.h
+# include virpidfile.h
+
# define MACVTAP_NAME_PREFIX macvtap
# define MACVTAP_NAME_PATTERNmacvtap%d
@@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
VIR_NETDEV_MACVLAN_MODE_LAST,
# define MACVLAN_NAME_PREFIX macvlan
# define MACVLAN_NAME_PATTERNmacvlan%d
+
Another spurious whitespace change.
/**
* virNetDevMacVLanCreate:
*
@@ -445,6 +449,293 @@ static const uint32_t
modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = {
[VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU,
};
+/* Struct to hold the state and configuration of a 802.1qbg port */
+struct virNetlinkCallbackData {
+ char cr_ifname[64];
+ virNetDevVPortProfilePtr virtPortProfile;
+ const unsigned char *macaddress;
+ const char *linkdev;
+ const unsigned char *vmuuid;
+ enum virNetDevVPortProfileOp vmOp;
+ unsigned int linkState;
+};
+
+typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr;
+
+#define INSTANCE_STRLEN 36
+
+static int instance2str(const unsigned char *p, char *dst, size_t size)
+{
+if (dst size INSTANCE_STRLEN) {
+snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x-
+%02x%02x-%02x%02x%02x%02x%02x%02x,
+p[0], p[1], p[2], p[3],
+p[4], p[5], p[6], p[7],
+p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]);
+return 0;
+}
+return -1;
+}
+
+# define LLDPAD_PID_FILE /var/run/lldpad.pid
+# define VIRIP_PID_FILE /var/run/virip.pid
+
+/**
+ * virNetDevMacVLanVPortProfileCallback:
+ *
+ * @msg: The buffer containing the received netlink message
+ * @length: The length of the received netlink message.
+ * @peer: The netling sockaddr containing the peer information
+ * @handeled: Contains information if the message has been replied to yet
+ * @opaque: Contains vital information regarding the associated vm an
interface
+ *
+ * This function is called when a netlink message is received. The function
+ * reads the message and responds if it is pertinent to the running VMs
+ * network interface.
+ */
+
+static void
+virNetDevMacVLanVPortProfileCallback( unsigned char *msg,
+
int length,
+
struct sockaddr_nl *peer,
+
bool *handled,
+
