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. --- docs/formatdomain.html.in | 38 ++++++++++++++++---- docs/formatnetwork.html.in | 28 +++++++++++++-- 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 | 35 ++++++++++++++++++ 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, 160 insertions(+), 16 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index eefdd5e..17e180d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3343,10 +3343,9 @@ <pre> ... &lt;devices&gt; - &lt;interface type='bridge'&gt; - &lt;source bridge='xenbr0'/&gt; - &lt;mac address='00:16:3e:5d:c7:9e'/&gt; - &lt;script path='vif-bridge'/&gt; + &lt;interface type='direct' trustGuestRxFilters='yes'&gt; + &lt;source dev='eth0'/&gt; + &lt;mac address='52:54:00:5d:c7:9e'/&gt; &lt;boot order='1'/&gt; &lt;rom bar='off'/&gt; &lt;/interface&gt; @@ -3356,8 +3355,21 @@ <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>&lt;interface&gt;</code> element has an + more details about common setup options. + </p> + <p> + libvirt allows specifying when the host should trust reports + from the guest of changes to the interface mac address and + receive filters by setting the <code>trustGuestRxFilters</code> + attribute to <code>yes</code><span class="since">Since + 1.2.9</span>. <code>trustGuestRxFilters</code> defaults + to <code>no</code> for security reasons, and support depends on + the guest network device driver as well as the type of + connection on the host - currently it is only supported for the + virtio driver, and for macvtap connections on the host. + </p> + <p> + Each <code>&lt;interface&gt;</code> element has an optional <code>&lt;address&gt;</code> sub-element that can tie the interface to a particular pci slot, with attribute <code>type='pci'</code> @@ -3589,6 +3601,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 propogated to the associated macvtap + device on the host. <span class="since">Since + 1.2.9</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 +3645,7 @@ ... &lt;devices&gt; ... - &lt;interface type='direct'&gt; + &lt;interface type='direct' trustGuestRxFilters='no'&gt; &lt;source dev='eth0' mode='vepa'/&gt; &lt;/interface&gt; &lt;/devices&gt; diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1a8ad8e..ff73952 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -35,7 +35,7 @@ </p> <pre> - &lt;network ipv6='yes'&gt; + &lt;network ipv6='yes' trustGuestRxFilters='no'&gt; &lt;name&gt;default&lt;/name&gt; &lt;uuid&gt;3e3fce45-4f53-4fa7-bb32-11f34168b82b&lt;/uuid&gt; ...</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.9</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 @@ &lt;outbound average='1000' peak='5000' burst='5120'/&gt; &lt;/bandwidth&gt; &lt;/portgroup&gt;</b> - <b>&lt;portgroup name='sales'&gt; + <b>&lt;portgroup name='sales' trustGuestRxFilters='no'&gt; &lt;virtualport type='802.1Qbh'&gt; &lt;parameters profileid='salestest'/&gt; &lt;/virtualport&gt; @@ -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>&lt;bandwidth&gt;</code> (described <a href="formatnetwork.html#elementQoS">here</a>) and <code>&lt;virtualport&gt;</code> @@ -650,6 +660,18 @@ 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.9</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 b6b309d..74a6d8c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2007,6 +2007,11 @@ --> <define name="interface"> <element name="interface"> + <optional> + <attribute name="trustGuestRxFilters"> + <ref name="virYesNo"/> + </attribute> + </optional> <choice> <group> <attribute name="type"> 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 9cc118c..7e0d147 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 && + ((int)(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 || @@ -6869,6 +6880,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(mode); VIR_FREE(addrtype); + VIR_FREE(trustGuestRxFilters); virDomainActualNetDefFree(actual); ctxt->node = save_ctxt; @@ -6919,6 +6931,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; @@ -6940,6 +6953,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_NET_TYPE_USER; } + trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"); + if (trustGuestRxFilters && + ((int)(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) { @@ -7469,6 +7492,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(mode); VIR_FREE(linkstate); VIR_FREE(addrtype); + VIR_FREE(trustGuestRxFilters); virNWFilterHashTableFree(filterparams); return def; @@ -16491,6 +16515,9 @@ virDomainActualNetDefFormat(virBufferPtr buf, if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); } + if (def->trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -16575,6 +16602,9 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<interface type='%s'", typeStr); if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); + if (def->trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -19976,6 +20006,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 d97b1f8..2d0cb06 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' */ }; @@ -954,6 +955,7 @@ struct _virDomainNetDef { virNWFilterHashTablePtr filterparams; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ int linkstate; }; @@ -2471,6 +2473,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..63b5917 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); @@ -2597,6 +2626,9 @@ virPortGroupDefFormat(virBufferPtr buf, if (def->isDefault) { virBufferAddLit(buf, " default='yes'"); } + if (def->trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, " trustGuestRxFilters='%s'", + virTristateBoolTypeToString(def->trustGuestRxFilters)); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); if (virNetDevVlanFormat(&def->vlan, buf) < 0) @@ -2675,6 +2707,9 @@ virNetworkDefFormatBuf(virBufferPtr buf, } if (def->ipv6nogw) virBufferAddLit(buf, " ipv6='yes'"); + if (def->trustGuestRxFilters != VIR_TRISTATE_BOOL_ABSENT) + 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 a339ced..bb2b9a3 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

On 09/24/2014 07:17 PM, John Ferlan wrote: *applause* Seriously. Can you write all my documentation from now on? Well, attributes are position-indifferent, so it doesn't really matter. I had put it up at the top so that it wasn't lost down at the bottom of all the <choice> sections. But there is something to be said for consistent positioning, so I'll change it. Yeah, I think I was following the pattern of the setting of def->type for virDomainNetDef (which is in the struct as "virDomainNetType type;" rather than "int type; /* enum virDomainNetType */). If that's already commonly done and expected, then I can change my code to do the same. That's only problematic for *elements* that are in different order in RNG vs. Format, and then can be solved by putting <interleave> ... </interleave> around all of the elements involved. (there is another potential issue, if the test case datafile has elements or attributes in a different order than they are emitted in the Format, the test case will fail unless there is a separate "out" file that has everything in the correct order.) You mean have a single utility function used in all three places? The problem with that is that the error message is slightly different to point out the location of the problematic attribute in the XML. Oops. Actually you can't *move* it, because in this function, the stuff at error: isn't executed in the case of success. It could benefit from being updated to use the currently accepted "do *all* cleanup at "cleanup:" and make sure all exits from the function go through there" form. That can/should be a separate patch though, so I'll just duplicate the VIR_FREE() at error: for now. I've made all the suggested changes. Since the freeze has started, I won't be pushing until after 1.2.9 release anyway, so I'll repost the entire series later.

On 09/24/2014 05:50 AM, Laine Stump wrote: Since BOOL_ABSENT is 0 - you don't "need" the comparison - it may look cleaner too ACK either way John

On 09/24/2014 05:50 AM, Laine Stump wrote: Is there ever a chance these could be different for one of these types? Seems "excessive" to make 3 specific definitions when 1 generic one could suffice (drop the "Unicast, Multicast, Vlan") I haven't peeked ahead yet, but are these tables where the allocation is allocate space for 10 table entries, allocate entries in each array entry... If so, then the free will need to run through the tables. Reason why I ask is I see size_t's for each structure indicating to me it's a array of entries. Same here with respect to generic rather than specific Each of the mode's would just use the generic FilterMode and the comments adjusted accordingly... soft ACK with changes depending on future patches which I haven't peeked at yet. John

On Wed, Sep 24, 2014 at 05:50:53AM -0400, Laine Stump wrote: The RxFilter ModeTypes of unicast, multicast, vlan are same 'normal', 'none', 'all' Can we just use some general functions to process the string? virNetDevRxFilterModeTypeFromString; virNetDevRxFilterModeTypeToString; -- Amos.

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 commend 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).) --- 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 | 3 + 5 files changed, 251 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c25f002..48cbe3e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2918,6 +2918,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 6b91e29..c37e36f 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" @@ -622,6 +623,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 a3d7c2c..58007e6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3194,6 +3194,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 (!(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 returne data missing array element")); + goto cleanup; + } + + if (!fil) + 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 + = virNetDevRxFilterUnicastModeTypeFromString(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 + = virNetDevRxFilterMulticastModeTypeFromString(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 + = virNetDevRxFilterVlanModeTypeFromString(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 d366179..a41281b 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 d6c3cfb..bd4371b 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) \ @@ -43,6 +44,8 @@ AM_CFLAGS = \ $(COVERAGE_CFLAGS) \ $(WARN_CFLAGS) + + AM_LDFLAGS = \ -export-dynamic -- 1.9.3

On 09/24/2014 08:11 PM, John Ferlan wrote: I'm not seeing that - every other function I checked is patterned just like this one (which is a direct copy of qemuMonitorRemoveNetdev()). At one point I had added a capability bit for it, but since the command is only issued in response to a NIC_RX_FILTER_CHANGED event, and the two were implemented in qemu at the same time, we would never attempt to call this function unless it was actually available in qemu, and I didn't want to clutter up the capabilities with an extra item unless it was really necessary. Yep. Changed it as discussed in 3/6. Fell asleep with my thumb on the space bar?? I was initially concerned as well, but keep in mind that it only affects whether an include directory is added to the compile line, and that doesn't happen for platforms without libnl. So far I just removed the extra lines in the Makefile and changed the names of the enum conversion functions.

On 09/27/2014 12:41 AM, Amos Kong wrote: Derp. I added in the extra stuff to drill into the JSON structure after the initial writing of the function, and wasn't paying attention. Thanks for catching that!

On 09/24/2014 05:50 AM, Laine Stump wrote: So how often could this be emitted? Do we need some sort of "filter" to only message once per life of the domain? There's no capabilities check necessary? What if the command doesn't exist in the underlying qemu? You filled in a lot of data from the qemuMonitorQueryRxFilter(), but you're only using filter->mac (e.g. main-mac). Or am I missing something? John