[libvirt] [PATCH 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> --- src/libvirt_private.syms | 6 + src/util/virnetdev.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdev.h | 14 ++ 3 files changed, 366 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83..6b49b08 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1646,6 +1646,9 @@ virNetDevGetVirtualFunctionInfo; virNetDevGetVirtualFunctions; virNetDevGetVLanID; virNetDevIsOnline; +virNetDevIsPromiscuous; +virNetDevIsRcvAllMulti; +virNetDevIsRcvMulti; 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..c610f5b 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2337,6 +2337,333 @@ int virNetDevDelMulti(const char *ifname ATTRIBUTE_UNUSED, } #endif +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + int ifflags; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + if (promiscuous) + ifflags = ifr.ifr_flags | IFF_PROMISC; + else + ifflags = ifr.ifr_flags & ~IFF_PROMISC; + + if (ifr.ifr_flags != ifflags) { + ifr.ifr_flags = ifflags; + if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot set interface flags on '%s'"), + ifname); + goto cleanup; + } + } + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} +#else +int virNetDevSetPromiscuous(const char *ifname, + bool promiscuous ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to set device flags for interfaces " + "on this platform")); + return -1; +} +#endif + +#if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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 virNetDevIsPromiscuous(const char *ifname, + bool *promiscuous) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + *promiscuous = (ifr.ifr_flags & IFF_PROMISC) ? true : false; + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} +#else +int virNetDevIsPromiscuous(const char *ifname ATTRIBUTE_UNUSED, + bool *promiscuous ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to retrieve device flags for interfaces " + "on this platform")); + return -1; +} +#endif + +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + int ifflags; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + if (receive) + ifflags = ifr.ifr_flags | IFF_MULTICAST; + else + ifflags = ifr.ifr_flags & ~IFF_MULTICAST; + + if (ifr.ifr_flags != ifflags) { + ifr.ifr_flags = ifflags; + if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot set interface flags on '%s'"), + ifname); + goto cleanup; + } + } + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} +#else +int virNetDevSetRcvMulti(const char *ifname, + bool receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to set device flags for interfaces " + "on this platform")); + return -1; +} +#endif /* defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) */ + +#if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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 virNetDevIsRcvMulti(const char *ifname, + bool *receive) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + *receive = (ifr.ifr_flags & IFF_MULTICAST) ? true : false; + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} +#else +int virNetDevIsRcvMulti(const char *ifname, + bool *receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to retrieve device flags for interfaces " + "on this platform")); + return -1; +} +#endif /* defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) */ + +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + int ifflags; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + if (receive) + ifflags = ifr.ifr_flags | IFF_ALLMULTI; + else + ifflags = ifr.ifr_flags & ~IFF_ALLMULTI; + + if (ifr.ifr_flags != ifflags) { + ifr.ifr_flags = ifflags; + if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot set interface flags on '%s'"), + ifname); + goto cleanup; + } + } + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + +#else + +int virNetDevSetRcvAllMulti(const char *ifname, + bool receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to set device flags for interfaces " + "on this platform")); + return -1; +} +#endif /*defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ)*/ + +#if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) +/** + * 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 virNetDevIsRcvAllMulti(const char *ifname, + bool *receive) +{ + int fd = -1; + int ret = -1; + struct ifreq ifr; + + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot get interface flags on '%s'"), + ifname); + goto cleanup; + } + + *receive = (ifr.ifr_flags & IFF_ALLMULTI) ? true : false; + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} +#else + +int virNetDevIsRcvAllMulti(const char *ifname, + bool *receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to retrieve device flags for interfaces " + "on this platform")); + return -1; +} +#endif + static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr mcast) { int ifindex; @@ -2549,6 +2876,7 @@ int virNetDevGetRxFilter(const char *ifname, virNetDevRxFilterPtr *filter) { int ret = -1; + bool receive; virNetDevRxFilterPtr fil = virNetDevRxFilterNew(); if (!fil) @@ -2560,6 +2888,24 @@ int virNetDevGetRxFilter(const char *ifname, if (virNetDevGetMulticastTable(ifname, fil)) goto cleanup; + if (virNetDevIsPromiscuous(ifname, &fil->promiscuous)) + goto cleanup; + + if (virNetDevIsRcvAllMulti(ifname, &receive)) + goto cleanup; + + if (receive) + fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_ALL; + else { + if (virNetDevIsRcvMulti(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..4216957 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -200,4 +200,18 @@ int virNetDevDelMulti(const char *ifname, virMacAddrPtr macaddr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevIsPromiscuous(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 virNetDevIsRcvMulti(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 virNetDevIsRcvAllMulti(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 supplies the funtionality of synchronizing 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> --- 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 5994558..141f91a 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; + + default: + 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
(I didn't try make syntax-check, but am assuming you have and that it passes) On 01/19/2015 11:18 AM, akrowiak@linux.vnet.ibm.com wrote:
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
This patch supplies the funtionality of synchronizing the host macvtap device options with the guest device's in response to the NIC_RX_FILTER_CHANGED event.
"supplies the functionality of" sounds too busy and doesn't add anything. Instead maybe say "This patch enables synchronization of the host macvtap options ...."
The following device options will be synchronized: * PROMISC * MULTICAST * ALLMULTI
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- 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 5994558..141f91a 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) {
If you typecast the above to virNetDevRxFilterMode, then replace the "default" below with VIR_NETDEV_RX_FILTER_MODE_NONE, we will get a nice reminder to add a new case if a new value is ever added to the enum.
+ 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; + + default:
I think this should use VIR_NETDEV_RX_FILTER_MODE_NONE instead of "default". See above.
+ 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:
I think this is otherwise okay.
On 01/21/2015 01:44 PM, Laine Stump wrote:
(I didn't try make syntax-check, but am assuming you have and that it passes)
On 01/19/2015 11:18 AM, akrowiak@linux.vnet.ibm.com wrote:
From: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
This patch supplies the funtionality of synchronizing the host macvtap device options with the guest device's in response to the NIC_RX_FILTER_CHANGED event. "supplies the functionality of" sounds too busy and doesn't add anything. Instead maybe say "This patch enables synchronization of the host macvtap options ...." Okie dokie.
The following device options will be synchronized: * PROMISC * MULTICAST * ALLMULTI
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- 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 5994558..141f91a 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) { If you typecast the above to virNetDevRxFilterMode, then replace the "default" below with VIR_NETDEV_RX_FILTER_MODE_NONE, we will get a nice reminder to add a new case if a new value is ever added to the enum. Okay, will do.
+ 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; + + default: I think this should use VIR_NETDEV_RX_FILTER_MODE_NONE instead of "default". See above.
+ 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: I think this is otherwise okay.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 Tony Krowiak (2): util: Functions for getting/setting device options qemu: change macvtap device options in response to NIC_RX_FILTER_CHANGED
On 01/19/2015 11:18 AM, 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> --- src/libvirt_private.syms | 6 + src/util/virnetdev.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdev.h | 14 ++ 3 files changed, 366 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83..6b49b08 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1646,6 +1646,9 @@ virNetDevGetVirtualFunctionInfo; virNetDevGetVirtualFunctions; virNetDevGetVLanID; virNetDevIsOnline; +virNetDevIsPromiscuous; +virNetDevIsRcvAllMulti; +virNetDevIsRcvMulti; 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..c610f5b 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2337,6 +2337,333 @@ int virNetDevDelMulti(const char *ifname ATTRIBUTE_UNUSED, } #endif
There's a lot of duplicated code here. Rather than completely reimplementing the contents of virNetDev(Set|Is)Online for each of these, how about making two static helper functions: int virNetDevGetIFFlag(const char *ifname, int flag, bool *val) int virNetDevSetIFFlag(const char *ifname, int flag, bool val) then the exported function for each flag would simply be, eg: int virNetDevGetPromiscuous(const char *ifname, bool *promiscuous) { return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous); } int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) { return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous); } (also reimplement virNetDev(Set|Is)Online based on the helper functions) Because IFF_blah may not exist on all platforms, you'll need to provide the stub implementation of the toplevel functions, rather than stubs for virNetDev(Get|Set)IFFlag(), but it's still much less code. Oh, and I would define *all* of these SIOC[SG]IFFLAGS-using functions inside a single #if, rather than a separate one for each. Finally, although the existing function for "Online" uses "Is" as the verb in its name, I think it is more proper to use "Get", so all the functions you named "virNetDevIsBlah" should be named "virNetDevGetBlah" instead (rename virNetDevIsOnline while you're at it - don't forget to change the name in the symfile).
On 01/21/2015 01:34 PM, Laine Stump 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> --- src/libvirt_private.syms | 6 + src/util/virnetdev.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdev.h | 14 ++ 3 files changed, 366 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83..6b49b08 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1646,6 +1646,9 @@ virNetDevGetVirtualFunctionInfo; virNetDevGetVirtualFunctions; virNetDevGetVLanID; virNetDevIsOnline; +virNetDevIsPromiscuous; +virNetDevIsRcvAllMulti; +virNetDevIsRcvMulti; 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..c610f5b 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2337,6 +2337,333 @@ int virNetDevDelMulti(const char *ifname ATTRIBUTE_UNUSED, } #endif There's a lot of duplicated code here. Rather than completely reimplementing the contents of virNetDev(Set|Is)Online for each of
On 01/19/2015 11:18 AM, akrowiak@linux.vnet.ibm.com wrote: these, how about making two static helper functions:
int virNetDevGetIFFlag(const char *ifname, int flag, bool *val) int virNetDevSetIFFlag(const char *ifname, int flag, bool val)
then the exported function for each flag would simply be, eg:
int virNetDevGetPromiscuous(const char *ifname, bool *promiscuous) { return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous); }
int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) { return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous); }
(also reimplement virNetDev(Set|Is)Online based on the helper functions)
Because IFF_blah may not exist on all platforms, you'll need to provide the stub implementation of the toplevel functions, rather than stubs for virNetDev(Get|Set)IFFlag(), but it's still much less code.
Oh, and I would define *all* of these SIOC[SG]IFFLAGS-using functions inside a single #if, rather than a separate one for each.
Finally, although the existing function for "Online" uses "Is" as the verb in its name, I think it is more proper to use "Get", so all the functions you named "virNetDevIsBlah" should be named "virNetDevGetBlah" instead (rename virNetDevIsOnline while you're at it - don't forget to change the name in the symfile). Sounds like a plan.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
akrowiak＠linux.vnet.ibm.com -
Laine Stump -
Tony Krowiak