[libvirt] [PATCHv2 0/7] handle NIC_RX_FILTER_CHANGED events from qemu

These patches set up an event handler for qemu's NIC_RX_FILTER_CHANGED event, which is sent whenever a guest makes a change to a network device's unicast/multicast filter, vlan table, or MAC address (as long as there has been no previous event of the same type sent for the same interface without a corresponding query-rx-filter sent back from qemu). The handler checks if it is appropriate to respond to the NIC_RX_FILTER_CHANGED event (based on device type and configuration) and takes appropriate action. Currently it checks if the guest interface has been configured with trustGuestRxFilters='yes' (defaults to 'no' for security reasons), and if the host side device is macvtap. If so, and the MAC address on the guest has changed, the MAC address of the macvtap device is changed to match. The result of this is that networking from the guest will continue to work if the mac address of a macvtap-connected network device is changed from within the guest, as long as trustGuestRxFilters='yes' (previously changing the MAC address in the guest would break networking). Changes from V1: Responded to review comments from John Ferlan, Amos Kong, and Tom Krowiak, updated to indicate support was added in 1.2.10 rather than 1.2.9, and added an extra patch which puts a new .txt file in the qemu directory describing the mechanics of a qemu event handler (lifted from the commit log message of 5/7) Laine Stump (7): conf: add trustGuestRxFilters attribute to network and domain interface network: set interface actual trustGuestRxFilters from network/portgroup util: define virNetDevRxFilter and basic utility functions qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED qemu: add short document on qemu event handlers docs/formatdomain.html.in | 40 +++- docs/formatnetwork.html.in | 29 ++- docs/schemas/domaincommon.rng | 5 + docs/schemas/network.rng | 10 + src/conf/domain_conf.c | 42 ++++ src/conf/domain_conf.h | 3 + src/conf/network_conf.c | 36 ++++ src/conf/network_conf.h | 2 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 10 + src/qemu/EVENTHANDLERS.txt | 77 +++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 105 ++++++++++ src/qemu/qemu_monitor.c | 39 ++++ src/qemu/qemu_monitor.h | 11 + src/qemu/qemu_monitor_json.c | 232 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 42 ++++ src/util/virnetdev.c | 31 +++ src/util/virnetdev.h | 42 +++- tests/Makefile.am | 1 + tests/networkxml2xmlin/vepa-net.xml | 4 +- tests/networkxml2xmlout/vepa-net.xml | 4 +- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +- 24 files changed, 761 insertions(+), 17 deletions(-) create mode 100644 src/qemu/EVENTHANDLERS.txt -- 1.9.3

This new attribute will control whether or not libvirt will pay attention to guest notifications about changes to network device mac addresses and receive filters. The default for this is 'no' (for security reasons). If it is set to 'yes' *and* the specified device model and connection support it (currently only macvtap+virtio) then libvirt will watch for NIC_RX_FILTER_CHANGED events, and when it receives one, it will issue a query-rx-filter command, retrieve the result, and modify the host-side macvtap interface's mac address and unicast/multicast filters accordingly. The functionality behind this attribute will be in a later patch. This patch merely adds the attribute to the top-level of a domain's <interface> as well as to <network> and <portgroup>, and adds documentation and schema/xml2xml tests. Rather than adding even more test files, I've just added the net attribute in various applicable places of existing test files. --- changes from V1: * used jferlan's documentation paragraph, as it was *much* better than mine. * changed puncuation around the "since" clauses in documentation. * moved location of attribute in xml schema * eliminate unneeded case of an int to an int :-) * don't compare to VIR_TRISTATE_BOOL_ABSENT - just see if it is non-0. * fix memory leak in virNetworkDefParseXML docs/formatdomain.html.in | 40 +++++++++++++++++---- docs/formatnetwork.html.in | 29 +++++++++++++-- docs/schemas/domaincommon.rng | 5 +++ docs/schemas/network.rng | 10 ++++++ src/conf/domain_conf.c | 42 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 ++ src/conf/network_conf.c | 36 +++++++++++++++++++ src/conf/network_conf.h | 2 ++ src/libvirt_private.syms | 1 + tests/networkxml2xmlin/vepa-net.xml | 4 +-- tests/networkxml2xmlout/vepa-net.xml | 4 +-- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +-- 12 files changed, 164 insertions(+), 16 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e7b585c..3be613e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3343,10 +3343,9 @@ <pre> ... <devices> - <interface type='bridge'> - <source bridge='xenbr0'/> - <mac address='00:16:3e:5d:c7:9e'/> - <script path='vif-bridge'/> + <interface type='direct' trustGuestRxFilters='yes'> + <source dev='eth0'/> + <mac address='52:54:00:5d:c7:9e'/> <boot order='1'/> <rom bar='off'/> </interface> @@ -3356,8 +3355,23 @@ <p> There are several possibilities for specifying a network interface visible to the guest. Each subsection below provides - more details about common setup options. Additionally, - each <code><interface></code> element has an + more details about common setup options. + </p> + <p> + <span class="since">Since 1.2.10</span>), + the <code>interface</code> element + property <code>trustGuestRxFilters</code> provides the + capability for the host to detect and trust reports from the + guest regarding changes to the interface mac address and receive + filters by setting the attribute to <code>yes</code>. The default + setting for the attribute is <code>no</code> for security + reasons and support depends on the guest network device model as + well as the type of connection on the host - currently it is + only supported for the virtio ddevice model and for macvtap + connections on the host. + </p> + <p> + Each <code><interface></code> element has an optional <code><address></code> sub-element that can tie the interface to a particular pci slot, with attribute <code>type='pci'</code> @@ -3589,6 +3603,18 @@ being the default mode. The individual modes cause the delivery of packets to behave as follows: </p> + <p> + If the model type is set to <code>virtio</code> and + interface's <code>trustGuestRxFilters</code> attribute is set + to <code>yes</code>, changes made to the interface mac address, + unicast/multicast receive filters, and vlan settings in the + guest will be monitored and propagated to the associated macvtap + device on the host (<span class="since">Since + 1.2.10</span>). If <code>trustGuestRxFilters</code> is not set, + or is not supported for the device model in use, an attempted + change to the mac address originating from the guest side will + result in a non-working network connection. + </p> <dl> <dt><code>vepa</code></dt> @@ -3621,7 +3647,7 @@ ... <devices> ... - <interface type='direct'> + <interface type='direct' trustGuestRxFilters='no'> <source dev='eth0' mode='vepa'/> </interface> </devices> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1a8ad8e..dc438ae 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -35,7 +35,7 @@ </p> <pre> - <network ipv6='yes'> + <network ipv6='yes' trustGuestRxFilters='no'> <name>default</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> ...</pre> @@ -60,6 +60,16 @@ to have guest-to-guest communications. For further information, see the example below for the example with no gateway addresses. <span class="since">Since 1.0.1</span></dd> + <dt><code>trustGuestRxFilters='yes'</code></dt> + <dd>The optional parameter <code>trustGuestRxFilters</code> can + be used to set that attribute of the same name for each domain + interface connected to this network (<span class="since">since + 1.2.10</span>). See + the <a href="formatdomain.html#elementSNICS">Network + interfaces</a> section of the domain XML documentation for + more details. Note that an explicit setting of this attribute + in a portgroup or the individual domain interface will + override the setting in the network.</dd> </dl> <h3><a name="elementsConnect">Connectivity</a></h3> @@ -606,7 +616,7 @@ <outbound average='1000' peak='5000' burst='5120'/> </bandwidth> </portgroup></b> - <b><portgroup name='sales'> + <b><portgroup name='sales' trustGuestRxFilters='no'> <virtualport type='802.1Qbh'> <parameters profileid='salestest'/> </virtualport> @@ -626,7 +636,7 @@ network can have multiple portgroup elements (and one of those can optionally be designated as the 'default' portgroup for the network), and each portgroup has a name, as well as various - subelements associated with it. The currently supported + attributes and subelements associated with it. The currently supported subelements are <code><bandwidth></code> (described <a href="formatnetwork.html#elementQoS">here</a>) and <code><virtualport></code> @@ -650,6 +660,19 @@ considered an error, and will prevent the interface from starting. </p> + <p> + portgroups also support the optional + parameter <code>trustGuestRxFilters</code> which can be used to + set that attribute of the same name for each domain interface + using this portgroup (<span class="since">since + 1.2.10</span>). See + the <a href="formatdomain.html#elementSNICS">Network + interfaces</a> section of the domain XML documentation for more + details. Note that an explicit setting of this attribute in the + portgroup overrides the network-wide setting, and an explicit + setting in the individual domain interface will override the + setting in the portgroup. + </p> <h5><a name="elementsStaticroute">Static Routes</a></h5> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1a266e5..db8ddaa 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2240,6 +2240,11 @@ </interleave> </group> </choice> + <optional> + <attribute name="trustGuestRxFilters"> + <ref name="virYesNo"/> + </attribute> + </optional> </element> </define> <!-- diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index dc732b4..4546f80 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -24,6 +24,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="trustGuestRxFilters"> + <ref name="virYesNo"/> + </attribute> + </optional> <interleave> <!-- The name of the network, used to refer to it through the API @@ -197,6 +202,11 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <attribute name="trustGuestRxFilters"> + <ref name="virYesNo"/> + </attribute> + </optional> <interleave> <optional> <ref name="virtualPortProfile"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ea25df..4570134 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6756,6 +6756,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, char *type = NULL; char *mode = NULL; char *addrtype = NULL; + char *trustGuestRxFilters = NULL; if (VIR_ALLOC(actual) < 0) return -1; @@ -6783,6 +6784,16 @@ virDomainActualNetDefParseXML(xmlNodePtr node, goto error; } + trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"); + if (trustGuestRxFilters && + ((actual->trustGuestRxFilters + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown trustGuestRxFilters value '%s'"), + trustGuestRxFilters); + goto error; + } + virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode) { if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE || @@ -6878,6 +6889,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(mode); VIR_FREE(addrtype); + VIR_FREE(trustGuestRxFilters); virDomainActualNetDefFree(actual); ctxt->node = save_ctxt; @@ -6929,6 +6941,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *vhostuser_mode = NULL; char *vhostuser_path = NULL; char *vhostuser_type = NULL; + char *trustGuestRxFilters = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6950,6 +6963,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_NET_TYPE_USER; } + trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"); + if (trustGuestRxFilters && + ((def->trustGuestRxFilters + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown trustGuestRxFilters value '%s'"), + trustGuestRxFilters); + goto error; + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { @@ -7589,6 +7612,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(mode); VIR_FREE(linkstate); VIR_FREE(addrtype); + VIR_FREE(trustGuestRxFilters); virNWFilterHashTableFree(filterparams); return def; @@ -16611,6 +16635,9 @@ virDomainActualNetDefFormat(virBufferPtr buf, if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); } + if (def->trustGuestRxFilters) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -16768,6 +16795,9 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<interface type='%s'", typeStr); if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); + if (def->trustGuestRxFilters) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -20191,6 +20221,18 @@ virDomainNetGetActualVlan(virDomainNetDefPtr iface) return NULL; } + +bool +virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) +{ + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && + iface->data.network.actual) + return (iface->data.network.actual->trustGuestRxFilters + == VIR_TRISTATE_BOOL_YES); + return iface->trustGuestRxFilters == VIR_TRISTATE_BOOL_YES; +} + + /* Return listens[i] from the appropriate union for the graphics * type, or NULL if this is an unsuitable type, or the index is out of * bounds. If force0 is TRUE, i == 0, and there is no listen array, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea201b3..21a22c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -880,6 +880,7 @@ struct _virDomainActualNetDef { virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ unsigned int class_id; /* class ID for bandwidth 'floor' */ }; @@ -969,6 +970,7 @@ struct _virDomainNetDef { virNWFilterHashTablePtr filterparams; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ int linkstate; }; @@ -2486,6 +2488,7 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface); virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); +bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 892bd8a..3bad07d 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1615,6 +1615,7 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, xmlNodePtr vlanNode; xmlNodePtr bandwidth_node; char *isDefault = NULL; + char *trustGuestRxFilters = NULL; int result = -1; @@ -1632,6 +1633,18 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, isDefault = virXPathString("string(./@default)", ctxt); def->isDefault = isDefault && STRCASEEQ(isDefault, "yes"); + trustGuestRxFilters + = virXPathString("string(./@trustGuestRxFilters)", ctxt); + if (trustGuestRxFilters) { + if ((def->trustGuestRxFilters + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid trustGuestRxFilters setting '%s' " + "in portgroup"), trustGuestRxFilters); + goto cleanup; + } + } + virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode && (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) { @@ -1654,6 +1667,7 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, virPortGroupDefClear(def); } VIR_FREE(isDefault); + VIR_FREE(trustGuestRxFilters); ctxt->node = save; return result; @@ -2013,6 +2027,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; char *ipv6nogwStr = NULL; + char *trustGuestRxFilters = NULL; xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; xmlNodePtr vlanNode; @@ -2061,6 +2076,20 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(ipv6nogwStr); } + trustGuestRxFilters + = virXPathString("string(./@trustGuestRxFilters)", ctxt); + if (trustGuestRxFilters) { + if ((def->trustGuestRxFilters + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid trustGuestRxFilters setting '%s' " + "in network '%s'"), + trustGuestRxFilters, def->name); + goto error; + } + VIR_FREE(trustGuestRxFilters); + } + /* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); @@ -2303,6 +2332,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(ipNodes); VIR_FREE(portGroupNodes); VIR_FREE(ipv6nogwStr); + VIR_FREE(trustGuestRxFilters); ctxt->node = save; return NULL; } @@ -2597,6 +2627,9 @@ virPortGroupDefFormat(virBufferPtr buf, if (def->isDefault) { virBufferAddLit(buf, " default='yes'"); } + if (def->trustGuestRxFilters) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); if (virNetDevVlanFormat(&def->vlan, buf) < 0) @@ -2675,6 +2708,9 @@ virNetworkDefFormatBuf(virBufferPtr buf, } if (def->ipv6nogw) virBufferAddLit(buf, " ipv6='yes'"); + if (def->trustGuestRxFilters) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<name>%s</name>\n", def->name); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 7ed58cd..660cd2d 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -219,6 +219,7 @@ struct _virPortGroupDef { virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ }; typedef struct _virNetworkDef virNetworkDef; @@ -256,6 +257,7 @@ struct _virNetworkDef { virPortGroupDefPtr portGroups; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7cbc35b..8d8342f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -331,6 +331,7 @@ virDomainNetGetActualBridgeName; virDomainNetGetActualDirectDev; virDomainNetGetActualDirectMode; virDomainNetGetActualHostdev; +virDomainNetGetActualTrustGuestRxFilters; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; diff --git a/tests/networkxml2xmlin/vepa-net.xml b/tests/networkxml2xmlin/vepa-net.xml index 030c1d1..07c59c5 100644 --- a/tests/networkxml2xmlin/vepa-net.xml +++ b/tests/networkxml2xmlin/vepa-net.xml @@ -1,4 +1,4 @@ -<network> +<network trustGuestRxFilters="no"> <name>vepa-net</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8b</uuid> <forward mode="vepa"> @@ -14,7 +14,7 @@ <parameters typeid="2193047" typeidversion="3"/> </virtualport> </portgroup> - <portgroup name="alice"> + <portgroup name="alice" trustGuestRxFilters="yes"> <virtualport type="802.1Qbg"> <parameters managerid="13"/> </virtualport> diff --git a/tests/networkxml2xmlout/vepa-net.xml b/tests/networkxml2xmlout/vepa-net.xml index 4d35a8a..b266620 100644 --- a/tests/networkxml2xmlout/vepa-net.xml +++ b/tests/networkxml2xmlout/vepa-net.xml @@ -1,4 +1,4 @@ -<network> +<network trustGuestRxFilters='no'> <name>vepa-net</name> <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8b</uuid> <forward dev='eth1' mode='vepa'> @@ -14,7 +14,7 @@ <parameters typeid='2193047' typeidversion='3'/> </virtualport> </portgroup> - <portgroup name='alice'> + <portgroup name='alice' trustGuestRxFilters='yes'> <virtualport type='802.1Qbg'> <parameters managerid='13'/> </virtualport> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml index 950a9db..6cba439 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml @@ -22,7 +22,7 @@ <controller type='usb' index='0'/> <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> - <interface type='network'> + <interface type='network' trustGuestRxFilters='yes'> <mac address='00:11:22:33:44:55'/> <source network='rednet' portgroup='bob'/> <vlan> @@ -33,7 +33,7 @@ </virtualport> <model type='virtio'/> </interface> - <interface type='network'> + <interface type='network' trustGuestRxFilters='no'> <mac address='10:11:22:33:44:55'/> <source network='blue' portgroup='sam'/> <virtualport> -- 1.9.3

As is done with other items such as vlan, virtualport, and bandwidth, set the actual trustGuestRxFilters value to be used by a domain interface according to a merge of the same attribute in the interface, portgroup, and network in use. the interface setting always takes precedence (if specified), followed by portgroup, and finally the setting in the network is used if it's not specified in the interface or portgroup. --- changes from V1: * don't compare to VIR_TRISTATE_BOOL_ABSENT - just see if it is non-0. src/network/bridge_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 979fb13..5f2e778 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3794,6 +3794,16 @@ networkAllocateActualDevice(virDomainDefPtr dom, if (vlan && virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) goto error; + if (iface->trustGuestRxFilters) + iface->data.network.actual->trustGuestRxFilters + = iface->trustGuestRxFilters; + else if (portgroup && portgroup->trustGuestRxFilters) + iface->data.network.actual->trustGuestRxFilters + = portgroup->trustGuestRxFilters; + else if (netdef->trustGuestRxFilters) + iface->data.network.actual->trustGuestRxFilters + = netdef->trustGuestRxFilters; + if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) || (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) || (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE)) { -- 1.9.3

This same structure will be used to retrieve RX filter info for interfaces on the host via netlink messages, and RX filter info for interfaces on the guest via the qemu "query-rx-filter" command. --- changes from V1: * use a single enum virNetDevRxFilterMode, instead of 3 separate enums for vlan, multicast, and unicast. src/libvirt_private.syms | 4 ++++ src/util/virnetdev.c | 31 +++++++++++++++++++++++++++++++ src/util/virnetdev.h | 42 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8d8342f..d6265ac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1615,6 +1615,10 @@ virNetDevReplaceMacAddress; virNetDevReplaceNetConfig; virNetDevRestoreMacAddress; virNetDevRestoreNetConfig; +virNetDevRxFilterFree; +virNetDevRxFilterModeTypeFromString; +virNetDevRxFilterModeTypeToString; +virNetDevRxFilterNew; virNetDevSetIPv4Address; virNetDevSetMAC; virNetDevSetMTU; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 8815e18..db5623a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1932,3 +1932,34 @@ virNetDevGetLinkInfo(const char *ifname, return 0; } #endif /* defined(__linux__) */ + + +VIR_ENUM_IMPL(virNetDevRxFilterMode, + VIR_NETDEV_RX_FILTER_MODE_LAST, + "none", + "normal", + "all"); + + +virNetDevRxFilterPtr +virNetDevRxFilterNew(void) +{ + virNetDevRxFilterPtr filter; + + if (VIR_ALLOC(filter) < 0) + return NULL; + return filter; +} + + +void +virNetDevRxFilterFree(virNetDevRxFilterPtr filter) +{ + if (filter) { + VIR_FREE(filter->name); + VIR_FREE(filter->unicast.table); + VIR_FREE(filter->multicast.table); + VIR_FREE(filter->vlan.table); + VIR_FREE(filter); + } +} diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 69e365e..2a6e67d 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007-2013 Red Hat, Inc. + * Copyright (C) 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,6 +37,42 @@ typedef struct ifreq virIfreq; typedef void virIfreq; # endif +typedef enum { + VIR_NETDEV_RX_FILTER_MODE_NONE = 0, + VIR_NETDEV_RX_FILTER_MODE_NORMAL, + VIR_NETDEV_RX_FILTER_MODE_ALL, + + VIR_NETDEV_RX_FILTER_MODE_LAST +} virNetDevRxFilterMode; +VIR_ENUM_DECL(virNetDevRxFilterMode) + +typedef struct _virNetDevRxFilter virNetDevRxFilter; +typedef virNetDevRxFilter *virNetDevRxFilterPtr; +struct _virNetDevRxFilter { + char *name; /* the alias used by qemu, *not* name used by guest */ + virMacAddr mac; + bool promiscuous; + bool broadcastAllowed; + + struct { + int mode; /* enum virNetDevRxFilterMode */ + bool overflow; + virMacAddrPtr table; + size_t nTable; + } unicast; + struct { + int mode; /* enum virNetDevRxFilterMode */ + bool overflow; + virMacAddrPtr table; + size_t nTable; + } multicast; + struct { + int mode; /* enum virNetDevRxFilterMode */ + unsigned int *table; + size_t nTable; + } vlan; +}; + int virNetDevSetupControl(const char *ifname, virIfreq *ifr) ATTRIBUTE_RETURN_CHECK; @@ -150,4 +186,8 @@ int virNetDevGetLinkInfo(const char *ifname, virInterfaceLinkPtr lnk) ATTRIBUTE_NONNULL(1); +virNetDevRxFilterPtr virNetDevRxFilterNew(void) + ATTRIBUTE_RETURN_CHECK; +void virNetDevRxFilterFree(virNetDevRxFilterPtr filter); + #endif /* __VIR_NETDEV_H__ */ -- 1.9.3

This function can be called at any time to get the current status of a guest's network device rx-filter. In particular it is useful to call after libvirt recieves a NIC_RX_FILTER_CHANGED event - this event only tells you that something has changed in the rx-filter, the details are retrieved with the query-rx-filter monitor command (only available in the json monitor). The command sent to the qemu monitor looks like this: {"execute":"query-rx-filter", "arguments": {"name":"net2"} }' and the results will look something like this: { "return": [ { "promiscuous": false, "name": "net2", "main-mac": "52:54:00:98:2d:e3", "unicast": "normal", "vlan": "normal", "vlan-table": [ 42, 0 ], "unicast-table": [ ], "multicast": "normal", "multicast-overflow": false, "unicast-overflow": false, "multicast-table": [ "33:33:ff:98:2d:e3", "01:80:c2:00:00:21", "01:00:5e:00:00:fb", "33:33:ff:98:2d:e2", "01:00:5e:00:00:01", "33:33:00:00:00:01" ], "broadcast-allowed": false } ], "id": "libvirt-14" } This is all parsed from JSON into a virNetDevRxFilter object for easier consumption. (unicast-table is usually empty, but is also an array of mac addresses similar to multicast-table). (NB: LIBNL_CFLAGS was added to tests/Makefile.am because virnetdev.h now includes util/virnetlink.h, which includes netlink/msg.h when appropriate. Without LIBNL_CFLAGS, gcc can't find that file (if libnl/netlink isn't available, LIBNL_CFLAGS will be empty and virnetlink.h won't try to include netlink/msg.h anyway).) --- changes from V1: * adjust for change in name of new enum * check for fil != 0 *before* starting to parse the json packet in qemuMonitorJSONQueryRxFilterParse. src/qemu/qemu_monitor.c | 26 ++++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 215 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + tests/Makefile.am | 1 + 5 files changed, 249 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 00c62f7..307d024 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2929,6 +2929,32 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, } +int +qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias, + virNetDevRxFilterPtr *filter) +{ + int ret = -1; + VIR_DEBUG("mon=%p alias=%s filter=%p", + mon, alias, filter); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + + VIR_DEBUG("mon=%p, alias=%s", mon, alias); + + if (mon->json) + ret = qemuMonitorJSONQueryRxFilter(mon, alias, filter); + else + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("query-rx-filter requires JSON monitor")); + return ret; +} + + int qemuMonitorGetPtyPaths(qemuMonitorPtr mon, virHashTablePtr paths) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 63e14cc..596468b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -31,6 +31,7 @@ # include "virbitmap.h" # include "virhash.h" # include "virjson.h" +# include "virnetdev.h" # include "device_conf.h" # include "cpu/cpu.h" @@ -627,6 +628,9 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias); +int qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias, + virNetDevRxFilterPtr *filter); + int qemuMonitorGetPtyPaths(qemuMonitorPtr mon, virHashTablePtr paths); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 90a125f..fc2f9ee 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3317,6 +3317,221 @@ int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon, } +static int +qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg, + virNetDevRxFilterPtr *filter) +{ + int ret = -1; + const char *tmp; + virJSONValuePtr returnArray, entry, table, element; + int nTable; + size_t i; + virNetDevRxFilterPtr fil = virNetDevRxFilterNew(); + + if (!fil) + goto cleanup; + + if (!(returnArray = virJSONValueObjectGet(msg, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-rx-filter reply was missing return data")); + goto cleanup; + } + if (returnArray->type != VIR_JSON_TYPE_ARRAY) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-rx-filter return data was not an array")); + goto cleanup; + } + if (!(entry = virJSONValueArrayGet(returnArray, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query -rx-filter return data missing array element")); + goto cleanup; + } + + if (!(tmp = virJSONValueObjectGetString(entry, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid name " + "in query-rx-filter response")); + goto cleanup; + } + if (VIR_STRDUP(fil->name, tmp) < 0) + goto cleanup; + if ((!(tmp = virJSONValueObjectGetString(entry, "main-mac"))) || + virMacAddrParse(tmp, &fil->mac) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'main-mac' " + "in query-rx-filter response")); + goto cleanup; + } + if (virJSONValueObjectGetBoolean(entry, "promiscuous", + &fil->promiscuous) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'promiscuous' " + "in query-rx-filter response")); + goto cleanup; + } + if (virJSONValueObjectGetBoolean(entry, "broadcast-allowed", + &fil->broadcastAllowed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'broadcast-allowed' " + "in query-rx-filter response")); + goto cleanup; + } + + if ((!(tmp = virJSONValueObjectGetString(entry, "unicast"))) || + ((fil->unicast.mode + = virNetDevRxFilterModeTypeFromString(tmp)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'unicast' " + "in query-rx-filter response")); + goto cleanup; + } + if (virJSONValueObjectGetBoolean(entry, "unicast-overflow", + &fil->unicast.overflow) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'unicast-overflow' " + "in query-rx-filter response")); + goto cleanup; + } + if ((!(table = virJSONValueObjectGet(entry, "unicast-table"))) || + ((nTable = virJSONValueArraySize(table)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'unicast-table' array " + "in query-rx-filter response")); + goto cleanup; + } + if (VIR_ALLOC_N(fil->unicast.table, nTable)) + goto cleanup; + for (i = 0; i < nTable; i++) { + if (!(element = virJSONValueArrayGet(table, i)) || + !(tmp = virJSONValueGetString(element))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid element %zu of 'unicast' " + "list in query-rx-filter response"), i); + goto cleanup; + } + if (virMacAddrParse(tmp, &fil->unicast.table[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid mac address '%s' in 'unicast-table' " + "array in query-rx-filter response"), tmp); + goto cleanup; + } + } + fil->unicast.nTable = nTable; + + if ((!(tmp = virJSONValueObjectGetString(entry, "multicast"))) || + ((fil->multicast.mode + = virNetDevRxFilterModeTypeFromString(tmp)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'multicast' " + "in query-rx-filter response")); + goto cleanup; + } + if (virJSONValueObjectGetBoolean(entry, "multicast-overflow", + &fil->multicast.overflow) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'multicast-overflow' " + "in query-rx-filter response")); + goto cleanup; + } + if ((!(table = virJSONValueObjectGet(entry, "multicast-table"))) || + ((nTable = virJSONValueArraySize(table)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'multicast-table' array " + "in query-rx-filter response")); + goto cleanup; + } + if (VIR_ALLOC_N(fil->multicast.table, nTable)) + goto cleanup; + for (i = 0; i < nTable; i++) { + if (!(element = virJSONValueArrayGet(table, i)) || + !(tmp = virJSONValueGetString(element))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid element %zu of 'multicast' " + "list in query-rx-filter response"), i); + goto cleanup; + } + if (virMacAddrParse(tmp, &fil->multicast.table[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid mac address '%s' in 'multicast-table' " + "array in query-rx-filter response"), tmp); + goto cleanup; + } + } + fil->multicast.nTable = nTable; + + if ((!(tmp = virJSONValueObjectGetString(entry, "vlan"))) || + ((fil->vlan.mode + = virNetDevRxFilterModeTypeFromString(tmp)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'vlan' " + "in query-rx-filter response")); + goto cleanup; + } + if ((!(table = virJSONValueObjectGet(entry, "vlan-table"))) || + ((nTable = virJSONValueArraySize(table)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing or invalid 'vlan-table' array " + "in query-rx-filter response")); + goto cleanup; + } + if (VIR_ALLOC_N(fil->vlan.table, nTable)) + goto cleanup; + for (i = 0; i < nTable; i++) { + if (!(element = virJSONValueArrayGet(table, i)) || + virJSONValueGetNumberUint(element, &fil->vlan.table[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid element %zu of 'vlan-table' " + "array in query-rx-filter response"), i); + goto cleanup; + } + } + fil->vlan.nTable = nTable; + + ret = 0; + cleanup: + if (ret < 0) { + virNetDevRxFilterFree(fil); + fil = NULL; + } + *filter = fil; + return ret; +} + + +int +qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char *alias, + virNetDevRxFilterPtr *filter) +{ + int ret = -1; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-rx-filter", + "s:name", alias, + NULL); + virJSONValuePtr reply = NULL; + + if (!cmd) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONQueryRxFilterParse(reply, filter) < 0) + goto cleanup; + + ret = 0; + cleanup: + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) { + virNetDevRxFilterFree(*filter); + *filter = NULL; + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + /* * Example return data * diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c7dd416..c898382 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -210,6 +210,9 @@ int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon, const char *alias); +int qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char *alias, + virNetDevRxFilterPtr *filter); + int qemuMonitorJSONGetPtyPaths(qemuMonitorPtr mon, virHashTablePtr paths); diff --git a/tests/Makefile.am b/tests/Makefile.am index 293611b..4c3d4ef 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,7 @@ AM_CFLAGS = \ -Dabs_builddir="\"$(abs_builddir)\"" \ -Dabs_srcdir="\"$(abs_srcdir)\"" \ $(LIBXML_CFLAGS) \ + $(LIBNL_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ -- 1.9.3

NIC_RX_FILTER_CHANGED is sent by qemu any time a NIC driver in the guest modified the NIC's RX Filter (for example, if the MAC address of the NIC is changed by the guest). This patch doesn't do anything useful with that event; it just sets up all the plumbing to get news of the event into a worker thread with all proper locking/reference counting, and provide an easy place to add in desired functionality. For easy reference the next time a handler for a qemu event is written, here is the sequence: The handler in qemu_json_monitor.c calls qemu_json_monitor.c:qemuMonitorJSONHandleNicRxFilterChanged() which calls qemu_monitor.c:qemuMonitorEmitNicRxFilterChanged() which uses QEMU_MONITOR_CALLBACK() to call mon->cb->domainNicRxFilterChanged(), ie: qemuProcessHandleNicRxFilterChanged() which will queue an event QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED for a worker thread to handle. When the worker thread gets the event, it calls qemuProcessEventHandler() which calls processNicRxFilterChangedEvent() and *that* is where the actual work will be done (and any event-specific memory allocated during qemuProcessHandleXXX() will be freed) - it is the middle of this function where functionality behind the event will be added in the next patch; for now there is just a VIR_DEBUG() to log the event. --- changes from V1: * fix filename of qemu_monitor_json.c in commit log message src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 13 +++++++++++ src/qemu/qemu_monitor.h | 7 ++++++ src/qemu/qemu_monitor_json.c | 17 ++++++++++++++ src/qemu/qemu_process.c | 42 +++++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 845d3c7..ad45a66 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -195,6 +195,7 @@ typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, QEMU_PROCESS_EVENT_GUESTPANIC, QEMU_PROCESS_EVENT_DEVICE_DELETED, + QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5cf235b..7770edc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4140,6 +4140,58 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, } +static void +processNicRxFilterChangedEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char *devAlias) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainDeviceDef dev; + virDomainNetDefPtr def; + + VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s " + "from domain %p %s", + devAlias, vm, vm->def->name); + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain is not running"); + goto endjob; + } + + if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) { + VIR_WARN("NIC_RX_FILTER_CHANGED event received for " + "non-existent device %s in domain %s", + devAlias, vm->def->name); + goto endjob; + } + if (dev.type != VIR_DOMAIN_DEVICE_NET) { + VIR_WARN("NIC_RX_FILTER_CHANGED event received for " + "non-network device %s in domain %s", + devAlias, vm->def->name); + goto endjob; + } + def = dev.data.net; + + /* handle the event - send query-rx-filter and respond to it. */ + + VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network " + "device %s in domain %s", def->info.alias, vm->def->name); + + endjob: + /* We had an extra reference to vm before starting a new job so ending the + * job is guaranteed not to remove the last reference. + */ + ignore_value(qemuDomainObjEndJob(driver, vm)); + + cleanup: + VIR_FREE(devAlias); + virObjectUnref(cfg); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4160,6 +4212,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_DEVICE_DELETED: processDeviceDeletedEvent(driver, vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED: + processNicRxFilterChangedEvent(driver, vm, processEvent->data); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 307d024..5dff9ff 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1387,6 +1387,19 @@ qemuMonitorEmitDeviceDeleted(qemuMonitorPtr mon, } +int +qemuMonitorEmitNicRxFilterChanged(qemuMonitorPtr mon, + const char *devAlias) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainNicRxFilterChanged, mon->vm, devAlias); + + return ret; +} + + int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 596468b..fd145a7 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -171,6 +171,10 @@ typedef int (*qemuMonitorDomainDeviceDeletedCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, const char *devAlias, void *opaque); +typedef int (*qemuMonitorDomainNicRxFilterChangedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *devAlias, + void *opaque); typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; @@ -197,6 +201,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainPMSuspendDiskCallback domainPMSuspendDisk; qemuMonitorDomainGuestPanicCallback domainGuestPanic; qemuMonitorDomainDeviceDeletedCallback domainDeviceDeleted; + qemuMonitorDomainNicRxFilterChangedCallback domainNicRxFilterChanged; }; char *qemuMonitorEscapeArg(const char *in); @@ -285,6 +290,8 @@ int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); int qemuMonitorEmitGuestPanic(qemuMonitorPtr mon); int qemuMonitorEmitDeviceDeleted(qemuMonitorPtr mon, const char *devAlias); +int qemuMonitorEmitNicRxFilterChanged(qemuMonitorPtr mon, + const char *devAlias); int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index fc2f9ee..3b205db 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -81,6 +81,7 @@ static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValueP static void qemuMonitorJSONHandlePMSuspendDisk(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleGuestPanic(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleNicRxFilterChanged(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -96,6 +97,7 @@ static qemuEventHandler eventHandlers[] = { { "DEVICE_DELETED", qemuMonitorJSONHandleDeviceDeleted, }, { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, { "GUEST_PANICKED", qemuMonitorJSONHandleGuestPanic, }, + { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, { "RESET", qemuMonitorJSONHandleReset, }, { "RESUME", qemuMonitorJSONHandleResume, }, @@ -1041,6 +1043,21 @@ qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr mon, virJSONValuePtr data) qemuMonitorEmitDeviceDeleted(mon, device); } + +static void +qemuMonitorJSONHandleNicRxFilterChanged(qemuMonitorPtr mon, virJSONValuePtr data) +{ + const char *name; + + if (!(name = virJSONValueObjectGetString(data, "name"))) { + VIR_WARN("missing device in NIC_RX_FILTER_CHANGED event"); + return; + } + + qemuMonitorEmitNicRxFilterChanged(mon, name); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 712a25e..95548aa 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1489,6 +1489,47 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *devAlias, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + struct qemuProcessEvent *processEvent = NULL; + char *data; + + virObjectLock(vm); + + VIR_DEBUG("Device %s RX Filter changed in domain %p %s", + devAlias, vm, vm->def->name); + + if (VIR_ALLOC(processEvent) < 0) + goto error; + + processEvent->eventType = QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED; + if (VIR_STRDUP(data, devAlias) < 0) + goto error; + processEvent->data = data; + processEvent->vm = vm; + + virObjectRef(vm); + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + ignore_value(virObjectUnref(vm)); + goto error; + } + + cleanup: + virObjectUnlock(vm); + return 0; + error: + if (processEvent) + VIR_FREE(processEvent->data); + VIR_FREE(processEvent); + goto cleanup; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1510,6 +1551,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainPMSuspendDisk = qemuProcessHandlePMSuspendDisk, .domainGuestPanic = qemuProcessHandleGuestPanic, .domainDeviceDeleted = qemuProcessHandleDeviceDeleted, + .domainNicRxFilterChanged = qemuProcessHandleNicRxFilterChanged, }; static int -- 1.9.3

This patch fills in the functionality of processNicRxFilterChangedEvent(). It now checks if it is appropriate to respond to the NIC_RX_FILTER_CHANGED event (based on device type and configuration) and takes appropriate action. Currently it checks if the guest interface has been configured with trustGuestRxFilters='yes', and if the host side device is macvtap. If so, and the MAC address on the guest has changed, the MAC address of the macvtap device is changed to match. The result of this is that networking from the guest will continue to work if the mac address of a macvtap-connected network device is changed from within the guest, as long as trustGuestRxFilters='yes' (previously changing the MAC address in the guest would break networking). --- No change from V1. src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7770edc..378f4dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4146,8 +4146,13 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, char *devAlias) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev; virDomainNetDefPtr def; + virNetDevRxFilterPtr filter = NULL; + virMacAddr oldMAC; + char newMacStr[VIR_MAC_STRING_BUFLEN]; + int ret; VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s " "from domain %p %s", @@ -4175,11 +4180,55 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, } def = dev.data.net; + if (!virDomainNetGetActualTrustGuestRxFilters(def)) { + VIR_WARN("ignore NIC_RX_FILTER_CHANGED event for network " + "device %s in domain %s", + def->info.alias, vm->def->name); + /* not sending "query-rx-filter" will also suppress any + * further NIC_RX_FILTER_CHANGED events for this device + */ + goto endjob; + } + /* handle the event - send query-rx-filter and respond to it. */ VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network " "device %s in domain %s", def->info.alias, vm->def->name); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &filter); + qemuDomainObjExitMonitor(driver, vm); + if (ret < 0) + goto endjob; + + virMacAddrFormat(&filter->mac, newMacStr); + + if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) { + + /* For macvtap connections, set the macvtap device's MAC + * address to match that of the guest device. + */ + + if (virNetDevGetMAC(def->ifname, &oldMAC) < 0) { + VIR_WARN("Couldn't get current MAC address of device %s " + "while responding to NIC_RX_FILTER_CHANGED", + def->ifname); + goto endjob; + } + + if (virMacAddrCmp(&oldMAC, &filter->mac)) { + /* set new MAC address from guest to associated macvtap device */ + if (virNetDevSetMAC(def->ifname, &filter->mac) < 0) { + VIR_WARN("Couldn't set new MAC address %s to device %s " + "while responding to NIC_RX_FILTER_CHANGED", + newMacStr, def->ifname); + } else { + VIR_DEBUG("device %s MAC address set to %s", + def->ifname, newMacStr); + } + } + } + endjob: /* We had an extra reference to vm before starting a new job so ending the * job is guaranteed not to remove the last reference. @@ -4187,6 +4236,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver, ignore_value(qemuDomainObjEndJob(driver, vm)); cleanup: + virNetDevRxFilterFree(filter); VIR_FREE(devAlias); virObjectUnref(cfg); } -- 1.9.3

This text was in the commit log for the patch that added the event handler for NIC_RX_FILTER_CHANGED, and John Ferlan expressed a desire that the information not be "lost", so I've put it into a file in the qemu directory, hoping that it might catch the attention of future writers of handlers for qemu events. --- src/qemu/EVENTHANDLERS.txt | 77 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/qemu/EVENTHANDLERS.txt diff --git a/src/qemu/EVENTHANDLERS.txt b/src/qemu/EVENTHANDLERS.txt new file mode 100644 index 0000000..79a6730 --- /dev/null +++ b/src/qemu/EVENTHANDLERS.txt @@ -0,0 +1,77 @@ +This is a short description of how an example qemu event can be used +to trigger handler code that is called from the context of a worker +thread, rather than directly from the event thread (which should +itself never block, and can't do things like send qemu monitor +commands, etc). + +In this case (the NIC_RX_FILTER_CHANGED event) the event is handled by +calling a qemu monitor command to get the current RX filter state, +then executing ioctls/sending netlink messages on the host in response +to changes in that filter state. This event is *not* propagated to the +libvirt API (but if someone wants to add details of how to handle that +to the end of this document, please do!). + +Hopefully this narration will be helpful when adding handlers for +other qemu events in the future. + +---------------------------------------------------- + +Any event emitted by qemu is received by +qemu_monitor_json.c:qemuMonitorJSONProcessEvent(). It looks up the +event by name in the table eventHandlers (in the same file), which +should have an entry like this for each event that libvirt +understands: + + { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, }, + + NB: This table is searched with bsearch, so it *must* be + alphabetically sorted. + +qemuMonitorJSONProcessEvent calls the function listed in +eventHandlers, e.g.: + + qemu_monitor_json.c:qemuMonitorJSONHandleNicRxFilterChanged() + +which extracts any required data from the JSON ("name" in this case), +and calls: + + qemu_monitor.c:qemuMonitorEmitNicRxFilterChanged() + +which uses QEMU_MONITOR_CALLBACK() to call +mon->cb->domainNicRxFilterChanged(). domainNicRxFilterChanged is one +in a list of function pointers in qemu_process.c:monitorCallbacks. For +our example, it has been set to: + + qemuProcessHandleNicRxFilterChanged() + +This function allocates a qemuProcessEvent object, and queues an event +named QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED (you'll want to add an +enum to qemu_domain.h:qemuProcessEventType for your event) for a +worker thread to handle. + +(Everything up to this point has happened in the context of the thread +that is reading events from qemu, so it should do as little as +possible, never block, and never call back into the qemu +monitor. Everything after this is handled in the context of a worker +thread, so it has more freedom to make qemu monitor calls and blocking +system calls on the host.) + +When the worker thread gets the event, it calls + + qemuProcessEventHandler() + +which switches on the eventType (in our example, +QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED) and decides to call: + + processNicRxFilterChangedEvent() + +and *that* is where the actual work will be done (and any +event-specific memory allocated during qemuProcessHandleXXX() will be +freed). Note that this function must do proper refcounting of the +domain object, and assure that the domain is still active prior to +performing any operations - it is possible that the domain could have +been destroyed between the time the event was received and the time +that it is processed, and it is also possible that the domain could be +destroyed *during* the event processing if it doesn't get properly +referenced by the handler. + -- 1.9.3

On 10/02/2014 12:39 PM, Laine Stump wrote:
This text was in the commit log for the patch that added the event handler for NIC_RX_FILTER_CHANGED, and John Ferlan expressed a desire that the information not be "lost", so I've put it into a file in the qemu directory, hoping that it might catch the attention of future writers of handlers for qemu events. --- src/qemu/EVENTHANDLERS.txt | 77 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/qemu/EVENTHANDLERS.txt
git am complains: Applying: qemu: add short document on qemu event handlers /home/jferlan/git/libvirt.work/.git/rebase-apply/patch:29: trailing whitespace. /home/jferlan/git/libvirt.work/.git/rebase-apply/patch:88: new blank line at EOF. + warning: 2 lines add whitespace errors. The trailing whitespace is on the line after the "----------"
diff --git a/src/qemu/EVENTHANDLERS.txt b/src/qemu/EVENTHANDLERS.txt new file mode 100644 index 0000000..79a6730 --- /dev/null +++ b/src/qemu/EVENTHANDLERS.txt @@ -0,0 +1,77 @@ +This is a short description of how an example qemu event can be used +to trigger handler code that is called from the context of a worker +thread, rather than directly from the event thread (which should +itself never block, and can't do things like send qemu monitor +commands, etc). + +In this case (the NIC_RX_FILTER_CHANGED event) the event is handled by +calling a qemu monitor command to get the current RX filter state, +then executing ioctls/sending netlink messages on the host in response +to changes in that filter state. This event is *not* propagated to the +libvirt API (but if someone wants to add details of how to handle that +to the end of this document, please do!). + +Hopefully this narration will be helpful when adding handlers for +other qemu events in the future. + +---------------------------------------------------- + ^^^ Trailing whitespace
+Any event emitted by qemu is received by +qemu_monitor_json.c:qemuMonitorJSONProcessEvent(). It looks up the +event by name in the table eventHandlers (in the same file), which +should have an entry like this for each event that libvirt +understands: + + { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, }, + + NB: This table is searched with bsearch, so it *must* be + alphabetically sorted. + +qemuMonitorJSONProcessEvent calls the function listed in +eventHandlers, e.g.: + + qemu_monitor_json.c:qemuMonitorJSONHandleNicRxFilterChanged() + +which extracts any required data from the JSON ("name" in this case), +and calls: + + qemu_monitor.c:qemuMonitorEmitNicRxFilterChanged() + +which uses QEMU_MONITOR_CALLBACK() to call +mon->cb->domainNicRxFilterChanged(). domainNicRxFilterChanged is one +in a list of function pointers in qemu_process.c:monitorCallbacks. For +our example, it has been set to: + + qemuProcessHandleNicRxFilterChanged() + +This function allocates a qemuProcessEvent object, and queues an event +named QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED (you'll want to add an +enum to qemu_domain.h:qemuProcessEventType for your event) for a +worker thread to handle. + +(Everything up to this point has happened in the context of the thread +that is reading events from qemu, so it should do as little as +possible, never block, and never call back into the qemu +monitor. Everything after this is handled in the context of a worker +thread, so it has more freedom to make qemu monitor calls and blocking +system calls on the host.) + +When the worker thread gets the event, it calls + + qemuProcessEventHandler() + +which switches on the eventType (in our example, +QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED) and decides to call: + + processNicRxFilterChangedEvent() + +and *that* is where the actual work will be done (and any +event-specific memory allocated during qemuProcessHandleXXX() will be +freed). Note that this function must do proper refcounting of the +domain object, and assure that the domain is still active prior to +performing any operations - it is possible that the domain could have +been destroyed between the time the event was received and the time +that it is processed, and it is also possible that the domain could be +destroyed *during* the event processing if it doesn't get properly +referenced by the handler. +
Blank line at EOF John

On 10/02/2014 12:39 PM, Laine Stump wrote:
These patches set up an event handler for qemu's NIC_RX_FILTER_CHANGED event, which is sent whenever a guest makes a change to a network device's unicast/multicast filter, vlan table, or MAC address (as long as there has been no previous event of the same type sent for the same interface without a corresponding query-rx-filter sent back from qemu).
The handler checks if it is appropriate to respond to the NIC_RX_FILTER_CHANGED event (based on device type and configuration) and takes appropriate action. Currently it checks if the guest interface has been configured with trustGuestRxFilters='yes' (defaults to 'no' for security reasons), and if the host side device is macvtap. If so, and the MAC address on the guest has changed, the MAC address of the macvtap device is changed to match.
The result of this is that networking from the guest will continue to work if the mac address of a macvtap-connected network device is changed from within the guest, as long as trustGuestRxFilters='yes' (previously changing the MAC address in the guest would break networking).
Changes from V1:
Responded to review comments from John Ferlan, Amos Kong, and Tom Krowiak, updated to indicate support was added in 1.2.10 rather than 1.2.9, and added an extra patch which puts a new .txt file in the qemu directory describing the mechanics of a qemu event handler (lifted from the commit log message of 5/7)
Laine Stump (7): conf: add trustGuestRxFilters attribute to network and domain interface network: set interface actual trustGuestRxFilters from network/portgroup util: define virNetDevRxFilter and basic utility functions qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED qemu: add short document on qemu event handlers
docs/formatdomain.html.in | 40 +++- docs/formatnetwork.html.in | 29 ++- docs/schemas/domaincommon.rng | 5 + docs/schemas/network.rng | 10 + src/conf/domain_conf.c | 42 ++++ src/conf/domain_conf.h | 3 + src/conf/network_conf.c | 36 ++++ src/conf/network_conf.h | 2 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 10 + src/qemu/EVENTHANDLERS.txt | 77 +++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 105 ++++++++++ src/qemu/qemu_monitor.c | 39 ++++ src/qemu/qemu_monitor.h | 11 + src/qemu/qemu_monitor_json.c | 232 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 42 ++++ src/util/virnetdev.c | 31 +++ src/util/virnetdev.h | 42 +++- tests/Makefile.am | 1 + tests/networkxml2xmlin/vepa-net.xml | 4 +- tests/networkxml2xmlout/vepa-net.xml | 4 +- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +- 24 files changed, 761 insertions(+), 17 deletions(-) create mode 100644 src/qemu/EVENTHANDLERS.txt
Beyond nit noted in patch 7 from my git am... ACK series John

On 10/06/2014 10:32 AM, John Ferlan wrote:
On 10/02/2014 12:39 PM, Laine Stump wrote:
These patches set up an event handler for qemu's NIC_RX_FILTER_CHANGED event, which is sent whenever a guest makes a change to a network device's unicast/multicast filter, vlan table, or MAC address (as long as there has been no previous event of the same type sent for the same interface without a corresponding query-rx-filter sent back from qemu).
The handler checks if it is appropriate to respond to the NIC_RX_FILTER_CHANGED event (based on device type and configuration) and takes appropriate action. Currently it checks if the guest interface has been configured with trustGuestRxFilters='yes' (defaults to 'no' for security reasons), and if the host side device is macvtap. If so, and the MAC address on the guest has changed, the MAC address of the macvtap device is changed to match.
The result of this is that networking from the guest will continue to work if the mac address of a macvtap-connected network device is changed from within the guest, as long as trustGuestRxFilters='yes' (previously changing the MAC address in the guest would break networking).
Changes from V1:
Responded to review comments from John Ferlan, Amos Kong, and Tom Krowiak, updated to indicate support was added in 1.2.10 rather than 1.2.9, and added an extra patch which puts a new .txt file in the qemu directory describing the mechanics of a qemu event handler (lifted from the commit log message of 5/7)
Laine Stump (7): conf: add trustGuestRxFilters attribute to network and domain interface network: set interface actual trustGuestRxFilters from network/portgroup util: define virNetDevRxFilter and basic utility functions qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED qemu: add short document on qemu event handlers
docs/formatdomain.html.in | 40 +++- docs/formatnetwork.html.in | 29 ++- docs/schemas/domaincommon.rng | 5 + docs/schemas/network.rng | 10 + src/conf/domain_conf.c | 42 ++++ src/conf/domain_conf.h | 3 + src/conf/network_conf.c | 36 ++++ src/conf/network_conf.h | 2 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 10 + src/qemu/EVENTHANDLERS.txt | 77 +++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 105 ++++++++++ src/qemu/qemu_monitor.c | 39 ++++ src/qemu/qemu_monitor.h | 11 + src/qemu/qemu_monitor_json.c | 232 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 42 ++++ src/util/virnetdev.c | 31 +++ src/util/virnetdev.h | 42 +++- tests/Makefile.am | 1 + tests/networkxml2xmlin/vepa-net.xml | 4 +- tests/networkxml2xmlout/vepa-net.xml | 4 +- .../qemuxml2argv-net-virtio-network-portgroup.xml | 4 +- 24 files changed, 761 insertions(+), 17 deletions(-) create mode 100644 src/qemu/EVENTHANDLERS.txt
Beyond nit noted in patch 7 from my git am...
ACK series
Okay. I fixed that, re-ordered the patches to put that last patch prior to the commit that implements the event handler, and changed the commit log message for that one to point to the documentation in the file rather than repeating it. The result has been pushed. Thanks for the reviews!
participants (2)
-
John Ferlan
-
Laine Stump