[libvirt] [PATCH v3 1/2] util: Functions for getting/setting device options
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com> This patch provides the utility functions needed to synchronize the rxfilter changes made to a guest domain with the corresponding macvtap devices on the host: * Get/set PROMISC flag * Get/set ALLMULTI, MULTICAST Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- Changes for v3: * Fixed a syntax-check error in virNetDevGetRxFilter function src/libvirt_private.syms | 8 ++- src/util/virnetdev.c | 186 +++++++++++++++++++++++++++++++++++++++------- src/util/virnetdev.h | 24 ++++++- 3 files changed, 190 insertions(+), 28 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83..8d76f9b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1639,13 +1639,16 @@ virNetDevGetIPv4Address; virNetDevGetLinkInfo; virNetDevGetMAC; virNetDevGetMTU; +virNetDevGetOnline; virNetDevGetPhysicalFunction; +virNetDevGetPromiscuous; +virNetDevGetRcvAllMulti; +virNetDevGetRcvMulti; virNetDevGetRxFilter; virNetDevGetVirtualFunctionIndex; virNetDevGetVirtualFunctionInfo; virNetDevGetVirtualFunctions; virNetDevGetVLanID; -virNetDevIsOnline; virNetDevIsVirtualFunction; virNetDevLinkDump; virNetDevReplaceMacAddress; @@ -1663,6 +1666,9 @@ virNetDevSetMTUFromDevice; virNetDevSetName; virNetDevSetNamespace; virNetDevSetOnline; +virNetDevSetPromiscuous; +virNetDevSetRcvAllMulti; +virNetDevSetRcvMulti; virNetDevSetupControl; virNetDevValidateConfig; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index ef96b2b..5d330ce 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -610,17 +610,7 @@ int virNetDevSetName(const char* ifname, const char *newifname) #if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) -/** - * virNetDevSetOnline: - * @ifname: the interface name - * @online: true for up, false for down - * - * Function to control if an interface is activated (up, true) or not (down, false) - * - * Returns 0 in case of success or -1 on error. - */ -int virNetDevSetOnline(const char *ifname, - bool online) +int virNetDevSetIFFlag(const char *ifname, int flag, bool val) { int fd = -1; int ret = -1; @@ -637,10 +627,10 @@ int virNetDevSetOnline(const char *ifname, goto cleanup; } - if (online) - ifflags = ifr.ifr_flags | IFF_UP; + if (val) + ifflags = ifr.ifr_flags | flag; else - ifflags = ifr.ifr_flags & ~IFF_UP; + ifflags = ifr.ifr_flags & ~flag; if (ifr.ifr_flags != ifflags) { ifr.ifr_flags = ifflags; @@ -659,8 +649,9 @@ int virNetDevSetOnline(const char *ifname, return ret; } #else -int virNetDevSetOnline(const char *ifname, - bool online ATTRIBUTE_UNUSED) +int virNetDevSetIFFlag(const char *ifname, + int flag ATTRIBUTE_UNUSED, + bool val ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, _("Cannot set interface flags on '%s'"), @@ -670,18 +661,77 @@ int virNetDevSetOnline(const char *ifname, #endif -#if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) + /** - * virNetDevIsOnline: + * virNetDevSetOnline: * @ifname: the interface name - * @online: where to store the status + * @online: true for up, false for down * - * Function to query if an interface is activated (true) or not (false) + * Function to control if an interface is activated (up, true) or not (down, false) * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 in case of success or -1 on error. */ -int virNetDevIsOnline(const char *ifname, - bool *online) +int virNetDevSetOnline(const char *ifname, + bool online) +{ + + return virNetDevSetIFFlag(ifname, IFF_UP, online); +} + +/** + * virNetDevSetPromiscuous: + * @ifname: the interface name + * @promiscuous: true for receive all packets, false for do not receive + * all packets + * + * Function to control if an interface is to receive all + * packets (receive all, true) or not (do not receive all, false) + * + * Returns 0 in case of success or -1 on error. + */ +int virNetDevSetPromiscuous(const char *ifname, + bool promiscuous) +{ + return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous); +} + +/** + * virNetDevSetRcvMulti: + * @ifname: the interface name + * @:receive true for receive multicast packets, false for do not receive + * multicast packets + * + * Function to control if an interface is to receive multicast + * packets in which it is interested (receive, true) + * or not (do not receive, false) + * + * Returns 0 in case of success or -1 on error. + */ +int virNetDevSetRcvMulti(const char *ifname, + bool receive) +{ + return virNetDevSetIFFlag(ifname, IFF_MULTICAST, receive); +} + +/** + * virNetDevSetRcvAllMulti: + * @ifname: the interface name + * @:receive true for receive all packets, false for do not receive all packets + * + * Function to control if an interface is to receive all multicast + * packets (receive, true) or not (do not receive, false) + * + * Returns 0 in case of success or -1 on error. + */ +int virNetDevSetRcvAllMulti(const char *ifname, + bool receive) +{ + return virNetDevSetIFFlag(ifname, IFF_ALLMULTI, receive); +} + + +#if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +int virNetDevGetIFFlag(const char *ifname, int flag, bool *val) { int fd = -1; int ret = -1; @@ -697,7 +747,7 @@ int virNetDevIsOnline(const char *ifname, goto cleanup; } - *online = (ifr.ifr_flags & IFF_UP) ? true : false; + *val = (ifr.ifr_flags & flag) ? true : false; ret = 0; cleanup: @@ -705,8 +755,9 @@ int virNetDevIsOnline(const char *ifname, return ret; } #else -int virNetDevIsOnline(const char *ifname, - bool *online ATTRIBUTE_UNUSED) +int virNetDevGetIFFlag(const char *ifname, + int flag ATTRIBUTE_UNUSED, + bool *val ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, _("Cannot get interface flags on '%s'"), @@ -717,6 +768,70 @@ int virNetDevIsOnline(const char *ifname, /** + * virNetDevGetOnline: + * @ifname: the interface name + * @online: where to store the status + * + * Function to query if an interface is activated (true) or not (false) + * + * Returns 0 in case of success or an errno code in case of failure. + */ +int virNetDevGetOnline(const char *ifname, + bool *online) +{ + return virNetDevGetIFFlag(ifname, IFF_UP, online); +} + +/** + * virNetDevIsPromiscuous: + * @ifname: the interface name + * @promiscuous: where to store the status + * + * Function to query if an interface is receiving all packets (true) or + * not (false) + * + * Returns 0 in case of success or an errno code in case of failure. + */ +int virNetDevGetPromiscuous(const char *ifname, + bool *promiscuous) +{ + return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous); +} + +/** + * virNetDevIsRcvMulti: + * @ifname: the interface name + * @receive where to store the status + * + * Function to query whether an interface is receiving multicast packets (true) + * in which it is interested, or not (false) + * + * Returns 0 in case of success or -1 on error. + */ +int virNetDevGetRcvMulti(const char *ifname, + bool *receive) +{ + return virNetDevGetIFFlag(ifname, IFF_MULTICAST, receive); +} + +/** + * virNetDevIsRcvAllMulti: + * @ifname: the interface name + * @:receive where to store the status + * + * Function to query whether an interface is receiving all multicast + * packets (receiving, true) or not (is not receiving, false) + * + * Returns 0 in case of success or -1 on error. + */ +int virNetDevGetRcvAllMulti(const char *ifname, + bool *receive) +{ + return virNetDevGetIFFlag(ifname, IFF_ALLMULTI, receive); +} + + +/** * virNetDevGetIndex: * @ifname : Name of the interface whose index is to be found * @ifindex: Pointer to int where the index will be written into @@ -2549,6 +2664,7 @@ int virNetDevGetRxFilter(const char *ifname, virNetDevRxFilterPtr *filter) { int ret = -1; + bool receive; virNetDevRxFilterPtr fil = virNetDevRxFilterNew(); if (!fil) @@ -2560,6 +2676,24 @@ int virNetDevGetRxFilter(const char *ifname, if (virNetDevGetMulticastTable(ifname, fil)) goto cleanup; + if (virNetDevGetPromiscuous(ifname, &fil->promiscuous)) + goto cleanup; + + if (virNetDevGetRcvAllMulti(ifname, &receive)) + goto cleanup; + + if (receive) { + fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_ALL; + } else { + if (virNetDevGetRcvMulti(ifname, &receive)) + goto cleanup; + + if (receive) + fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_NORMAL; + else + fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_NONE; + } + ret = 0; cleanup: if (ret < 0) { diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index fb7988f..8d03459 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -83,7 +83,7 @@ int virNetDevExists(const char *brname) int virNetDevSetOnline(const char *ifname, bool online) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -int virNetDevIsOnline(const char *ifname, +int virNetDevGetOnline(const char *ifname, bool *online) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; @@ -200,4 +200,26 @@ int virNetDevDelMulti(const char *ifname, virMacAddrPtr macaddr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevSetIFFlag(const char *ifname, int flag, bool val) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +int virNetDevGetIFFlag(const char *ifname, int flag, bool *val) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevGetPromiscuous(const char *ifname, bool *promiscuous) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevSetRcvMulti(const char *ifname, bool receive) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevGetRcvMulti(const char *ifname, bool *receive) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevSetRcvAllMulti(const char *ifname, bool receive) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_NETDEV_H__ */ -- 1.7.1
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com> This patch enables synchronization of the host macvtap device options with the guest device's in response to the NIC_RX_FILTER_CHANGED event. The following device options will be synchronized: * PROMISC * MULTICAST * ALLMULTI Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- No changes for v3 src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 92 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc6aae4..47c1b5e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4168,6 +4168,93 @@ syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilterPtr guestFilter, static void +syncNicRxFilterPromiscMode(char *ifname, virNetDevRxFilterPtr guestFilter, + virNetDevRxFilterPtr hostFilter) +{ + bool promisc; + bool setpromisc = false; + + /* Set macvtap promisc mode to true if the guest has vlans defined */ + /* or synchronize the macvtap promisc mode if different from guest */ + if (guestFilter->vlan.nTable > 0) { + if (!hostFilter->promiscuous) { + setpromisc = true; + promisc = true; + } + } else if (hostFilter->promiscuous != guestFilter->promiscuous) { + setpromisc = true; + promisc = guestFilter->promiscuous; + } + + if (setpromisc) { + if (virNetDevSetPromiscuous(ifname, promisc) < 0) { + VIR_WARN("Couldn't set PROMISC flag to %s for device %s " + "while responding to NIC_RX_FILTER_CHANGED", + promisc ? "true" : "false", ifname); + } + } +} + + +static void +syncNicRxFilterMultiMode(char *ifname, virNetDevRxFilterPtr guestFilter, + virNetDevRxFilterPtr hostFilter) +{ + if (hostFilter->multicast.mode != guestFilter->multicast.mode) { + switch (guestFilter->multicast.mode) { + case VIR_NETDEV_RX_FILTER_MODE_ALL: + if (virNetDevSetRcvAllMulti(ifname, true)) { + + VIR_WARN("Couldn't set allmulticast flag to 'on' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + break; + + case VIR_NETDEV_RX_FILTER_MODE_NORMAL: + if (virNetDevSetRcvMulti(ifname, true)) { + + VIR_WARN("Couldn't set multicast flag to 'on' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + + if (virNetDevSetRcvAllMulti(ifname, false)) { + VIR_WARN("Couldn't set allmulticast flag to 'off' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + break; + + case VIR_NETDEV_RX_FILTER_MODE_NONE: + if (virNetDevSetRcvAllMulti(ifname, false)) { + VIR_WARN("Couldn't set allmulticast flag to 'off' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", ifname); + } + + if (virNetDevSetRcvMulti(ifname, false)) { + VIR_WARN("Couldn't set multicast flag to 'off' for " + "device %s while responding to " + "NIC_RX_FILTER_CHANGED", + ifname); + } + break; + } + } +} + + +static void +syncNicRxFilterDeviceOptions(char *ifname, virNetDevRxFilterPtr guestFilter, + virNetDevRxFilterPtr hostFilter) +{ + syncNicRxFilterPromiscMode(ifname, guestFilter, hostFilter); + syncNicRxFilterMultiMode(ifname, guestFilter, hostFilter); +} + + +static void syncNicRxFilterMulticast(char *ifname, virNetDevRxFilterPtr guestFilter, virNetDevRxFilterPtr hostFilter) @@ -4250,9 +4337,14 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, * attributes to match those of the guest network device: * - MAC address * - Multicast MAC address table + * - Device options: + * - PROMISC + * - MULTICAST + * - ALLMULTI */ syncNicRxFilterMacAddr(def->ifname, guestFilter, hostFilter); syncNicRxFilterMulticast(def->ifname, guestFilter, hostFilter); + syncNicRxFilterDeviceOptions(def->ifname, guestFilter, hostFilter); } endjob: -- 1.7.1
On 22.01.2015 20:47, akrowiak@linux.vnet.ibm.com wrote:
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
This patch enables synchronization of the host macvtap device options with the guest device's in response to the NIC_RX_FILTER_CHANGED event.
The following device options will be synchronized: * PROMISC * MULTICAST * ALLMULTI
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- No changes for v3 src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 92 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc6aae4..47c1b5e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4168,6 +4168,93 @@ syncNicRxFilterHostMulticast(char *ifname, virNetDevRxFilterPtr guestFilter,
static void +syncNicRxFilterPromiscMode(char *ifname, virNetDevRxFilterPtr guestFilter, + virNetDevRxFilterPtr hostFilter)
Indentation's off.
+{ + bool promisc; + bool setpromisc = false; + + /* Set macvtap promisc mode to true if the guest has vlans defined */ + /* or synchronize the macvtap promisc mode if different from guest */ + if (guestFilter->vlan.nTable > 0) { + if (!hostFilter->promiscuous) { + setpromisc = true; + promisc = true; + } + } else if (hostFilter->promiscuous != guestFilter->promiscuous) { + setpromisc = true; + promisc = guestFilter->promiscuous; + } + + if (setpromisc) { + if (virNetDevSetPromiscuous(ifname, promisc) < 0) { + VIR_WARN("Couldn't set PROMISC flag to %s for device %s " + "while responding to NIC_RX_FILTER_CHANGED", + promisc ? "true" : "false", ifname); + } + } +} +
ACK with that fixed. Michal
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com> This patch set provides the code to synchonize some macvtap device modes when the values are changed on the guest's network device. The following modes will by synchronized: * PROMISC * MULTICAST * ALLMULTI I noticed something while testing this patch set that did not occur prior to installing more recent kernel and Qemu distributions. It seems that the VLAN table always has an entry for VLAN ID 0 when the rxfilter is queried during processing of the NIC_RX_FILTER_CHANGED event. That means that the PROMISC flag for macvtap0 will be set. I don't know if this will cause problems, but I thought I'd make note of it in case anybody wants to comment on that. v2 changes: * virnetdev.c * Changed names of virNetDevIs... functions to virNetDevGet... * Consolidated code for getting/setting of device option flags * qemu_driver.c * Replaced "default" case in syncNicRxFilterMultiMode function with VIR_NETDEV_RX_FILTER_MODE_NONE v3 changes: * virnetdev.c * Fixed a syntax-check error in virNetDevGetRxFilter function Tony Krowiak (2): util: Functions for getting/setting device options qemu: change macvtap device options in response to NIC_RX_FILTER_CHANGED
On 22.01.2015 20:47, akrowiak@linux.vnet.ibm.com wrote:
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
This patch set provides the code to synchonize some macvtap device modes when the values are changed on the guest's network device. The following modes will by synchronized: * PROMISC * MULTICAST * ALLMULTI
I noticed something while testing this patch set that did not occur prior to installing more recent kernel and Qemu distributions. It seems that the VLAN table always has an entry for VLAN ID 0 when the rxfilter is queried during processing of the NIC_RX_FILTER_CHANGED event. That means that the PROMISC flag for macvtap0 will be set. I don't know if this will cause problems, but I thought I'd make note of it in case anybody wants to comment on that.
v2 changes: * virnetdev.c * Changed names of virNetDevIs... functions to virNetDevGet... * Consolidated code for getting/setting of device option flags * qemu_driver.c * Replaced "default" case in syncNicRxFilterMultiMode function with VIR_NETDEV_RX_FILTER_MODE_NONE
v3 changes: * virnetdev.c * Fixed a syntax-check error in virNetDevGetRxFilter function
Tony Krowiak (2): util: Functions for getting/setting device options qemu: change macvtap device options in response to NIC_RX_FILTER_CHANGED
I've fixed the small nits I've found, and pushed. Michal
On 22.01.2015 20:47, akrowiak@linux.vnet.ibm.com wrote:
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
This patch provides the utility functions needed to synchronize the rxfilter changes made to a guest domain with the corresponding macvtap devices on the host:
* Get/set PROMISC flag * Get/set ALLMULTI, MULTICAST
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- Changes for v3: * Fixed a syntax-check error in virNetDevGetRxFilter function
src/libvirt_private.syms | 8 ++- src/util/virnetdev.c | 186 +++++++++++++++++++++++++++++++++++++++------- src/util/virnetdev.h | 24 ++++++- 3 files changed, 190 insertions(+), 28 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index ef96b2b..5d330ce 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c
@@ -717,6 +768,70 @@ int virNetDevIsOnline(const char *ifname,
/** + * virNetDevGetOnline: + * @ifname: the interface name + * @online: where to store the status + * + * Function to query if an interface is activated (true) or not (false) + * + * Returns 0 in case of success or an errno code in case of failure. + */ +int virNetDevGetOnline(const char *ifname, + bool *online)
Indentation's off.
+{ + return virNetDevGetIFFlag(ifname, IFF_UP, online); +} +
ACK with that fixed. Michal
participants (2)
-
akrowiak＠linux.vnet.ibm.com -
Michal Privoznik