On 05/15/2018 01:43 PM, Daniel P. Berrangé wrote:
> A typical XML representation of the virNWFilterBindingDefPtr struct
> looks like this:
>
> <filterbinding>
> <owner>
> <name>f25arm7</name>
> <uuid>12ac8b8c-4f23-4248-ae42-fdcd50c400fd</uuid>
> </owner>
> <portdev name='vnet1'/>
> <mac address='52:54:00:9d:81:b1'/>
> <filterref filter='clean-traffic'>
> <parameter name='MAC' value='52:54:00:9d:81:b1'/>
> </filterref>
> </filterbinding>
I haven't had time to look at this in detail yet, or to really think
about the comment I'm going to make, but I wanted to be sure I said it
right away in case there are any public API implications
By now we all know the horror by which OpenStack's networking creates a
separate bridge device, and connects the guest's tap device to that
bridge so that (in addition to other reasons) there is a place for
libvirt's nwfilter rules to attach (what they *really* want to connect
to is an Open vSwitch, but ipfilter rules aren't in the data path when a
tap device is connected to OVS). This atrocity could be avoided if
nwfilter supported OVN (OVS's ipfilter analog) directly. In order to
support it though, nwfilter would need to know more details about the
network device that it's adding rules for. (because some guests may be
connected to OVS and others may be connected to a standard host bridge
(or nothing at all) we can't just have a system-wide config to decide).
I can't recall if there is a reasonable API to figure out what a tap
device is connected to, but I think such a thing may not exist and, if
so, we'll likely need to send some sort of indicator in the
NWFilterBinding XML. It *might* be simpler / more futureproof if we
included the <virtualport> element that is already in the domain XML
<interface> information - that's currently what we use to decide how to
connect the tap device; hopefully in the future it will continue to
conain everything that's needed (if we think that's inadequate, we could
just go for broke and include the entire <interface>, but that's
probably overkill. (although..... - thinking about the potential case
where some SRIOV network card supported some sort of filtering, if we
sent the entire <interface>, we would know that it was a hostdev...)
I started typing this wondering if the C API might need any change, but
now that I've typed this, I realize it would only require additions to
the XML, which can always be done later, so
nevermind (kind of) </EmilyLittela>
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/conf/virnwfilterbindingdef.c | 196 ++++++++++++++++++
> src/conf/virnwfilterbindingdef.h | 18 ++
> src/libvirt_private.syms | 5 +
> tests/Makefile.am | 7 +
> .../filter-vars.xml | 11 +
> .../virnwfilterbindingxml2xmldata/simple.xml | 9 +
> tests/virnwfilterbindingxml2xmltest.c | 113 ++++++++++
> 7 files changed, 359 insertions(+)
> create mode 100644 tests/virnwfilterbindingxml2xmldata/filter-vars.xml
> create mode 100644 tests/virnwfilterbindingxml2xmldata/simple.xml
> create mode 100644 tests/virnwfilterbindingxml2xmltest.c
>
> diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c
> index c7533d4063..23c040ab05 100644
> --- a/src/conf/virnwfilterbindingdef.c
> +++ b/src/conf/virnwfilterbindingdef.c
> @@ -25,6 +25,7 @@
> #include "virstring.h"
> #include "nwfilter_params.h"
> #include "virnwfilterbindingdef.h"
> +#include "viruuid.h"
>
>
> #define VIR_FROM_THIS VIR_FROM_NWFILTER
> @@ -81,3 +82,198 @@ virNWFilterBindingDefCopy(virNWFilterBindingDefPtr src)
> virNWFilterBindingDefFree(ret);
> return NULL;
> }
> +
> +
> +static virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt)
> +{
> + virNWFilterBindingDefPtr ret;
> + char *uuid = NULL;
> + char *mac = NULL;
> + xmlNodePtr node;
> +
> + if (VIR_ALLOC(ret) < 0)
> + return NULL;
> +
> + ret->portdevname = virXPathString("string(./portdev/@name)", ctxt);
> + if (!ret->portdevname) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no port dev name"));
> + goto cleanup;
> + }
> +
> + if (virXPathNode("./linkdev", ctxt)) {
> + ret->linkdevname = virXPathString("string(./linkdev/@name)", ctxt);
> + if (!ret->linkdevname) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no link dev name"));
> + goto cleanup;
> + }
> + }
> +
> + ret->ownername = virXPathString("string(./owner/name)", ctxt);
> + if (!ret->ownername) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no owner name"));
> + goto cleanup;
> + }
> +
> + uuid = virXPathString("string(./owner/uuid)", ctxt);
> + if (!uuid) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no owner UUID"));
> + goto cleanup;
> + }
> +
> + if (virUUIDParse(uuid, ret->owneruuid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse UUID '%s'"), uuid);
> + VIR_FREE(uuid);
> + goto cleanup;
> + }
> + VIR_FREE(uuid);
> +
> + mac = virXPathString("string(./mac/@address)", ctxt);
> + if (!mac) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no MAC address"));
> + goto cleanup;
> + }
> +
> + if (virMacAddrParse(mac, &ret->mac) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse MAC '%s'"), mac);
> + VIR_FREE(mac);
> + goto cleanup;
> + }
> + VIR_FREE(mac);
> +
> + ret->filter = virXPathString("string(./filterref/@filter)", ctxt);
> + if (!ret->filter) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("filter binding has no filter reference"));
> + goto cleanup;
> + }
> +
> + node = virXPathNode("./filterref", ctxt);
> + if (node &&
> + !(ret->filterparams = virNWFilterParseParamAttributes(node)))
> + goto cleanup;
> +
> + return ret;
> +
> + cleanup:
> + virNWFilterBindingDefFree(ret);
> + return NULL;
> +}
> +
> +
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseNode(xmlDocPtr xml,
> + xmlNodePtr root)
> +{
> + xmlXPathContextPtr ctxt = NULL;
> + virNWFilterBindingDefPtr def = NULL;
> +
> + if (STRNEQ((const char *)root->name, "filterbinding")) {
> + virReportError(VIR_ERR_XML_ERROR,
> + "%s",
> + _("unknown root element for nwfilter binding"));
> + goto cleanup;
> + }
> +
> + ctxt = xmlXPathNewContext(xml);
> + if (ctxt == NULL) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + ctxt->node = root;
> + def = virNWFilterBindingDefParseXML(ctxt);
> +
> + cleanup:
> + xmlXPathFreeContext(ctxt);
> + return def;
> +}
> +
> +
> +static virNWFilterBindingDefPtr
> +virNWFilterBindingDefParse(const char *xmlStr,
> + const char *filename)
> +{
> + virNWFilterBindingDefPtr def = NULL;
> + xmlDocPtr xml;
> +
> + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)")))) {
> + def = virNWFilterBindingDefParseNode(xml, xmlDocGetRootElement(xml));
> + xmlFreeDoc(xml);
> + }
> +
> + return def;
> +}
> +
> +
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseString(const char *xmlStr)
> +{
> + return virNWFilterBindingDefParse(xmlStr, NULL);
> +}
> +
> +
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseFile(const char *filename)
> +{
> + return virNWFilterBindingDefParse(NULL, filename);
> +}
> +
> +char *
> +virNWFilterBindingDefFormat(const virNWFilterBindingDef *def)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (virNWFilterBindingDefFormatBuf(&buf, def) < 0) {
> + virBufferFreeAndReset(&buf);
> + return NULL;
> + }
> +
> + if (virBufferCheckError(&buf) < 0)
> + return NULL;
> +
> + return virBufferContentAndReset(&buf);
> +}
> +
> +
> +int
> +virNWFilterBindingDefFormatBuf(virBufferPtr buf,
> + const virNWFilterBindingDef *def)
> +{
> + char uuid[VIR_UUID_STRING_BUFLEN];
> + char mac[VIR_MAC_STRING_BUFLEN];
> +
> + virBufferAddLit(buf, "<filterbinding>\n");
> +
> + virBufferAdjustIndent(buf, 2);
> +
> + virBufferAddLit(buf, "<owner>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferEscapeString(buf, "<name>%s</name>\n", def->ownername);
> + virUUIDFormat(def->owneruuid, uuid);
> + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</owner>\n");
> +
> + virBufferEscapeString(buf, "<portdev name='%s'/>\n", def->portdevname);
> + if (def->linkdevname)
> + virBufferEscapeString(buf, "<linkdev name='%s'/>\n", def->linkdevname);
> +
> + virMacAddrFormat(&def->mac, mac);
> + virBufferAsprintf(buf, "<mac address='%s'/>\n", mac);
> +
> + if (virNWFilterFormatParamAttributes(buf, def->filterparams, def->filter) < 0)
> + return -1;
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</filterbinding>\n");
> +
> + return 0;
> +}
> diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h
> index e3b18af151..af7fab6064 100644
> --- a/src/conf/virnwfilterbindingdef.h
> +++ b/src/conf/virnwfilterbindingdef.h
> @@ -24,6 +24,7 @@
> # include "internal.h"
> # include "virmacaddr.h"
> # include "virhash.h"
> +# include "virbuffer.h"
>
> typedef struct _virNWFilterBindingDef virNWFilterBindingDef;
> typedef virNWFilterBindingDef *virNWFilterBindingDefPtr;
> @@ -44,4 +45,21 @@ virNWFilterBindingDefFree(virNWFilterBindingDefPtr binding);
> virNWFilterBindingDefPtr
> virNWFilterBindingDefCopy(virNWFilterBindingDefPtr src);
>
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseNode(xmlDocPtr xml,
> + xmlNodePtr root);
> +
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseString(const char *xml);
> +
> +virNWFilterBindingDefPtr
> +virNWFilterBindingDefParseFile(const char *filename);
> +
> +char *
> +virNWFilterBindingDefFormat(const virNWFilterBindingDef *def);
> +
> +int
> +virNWFilterBindingDefFormatBuf(virBufferPtr buf,
> + const virNWFilterBindingDef *def);
> +
> #endif /* VIR_NWFILTER_BINDING_DEF_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fb754fbfea..03145c70d5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1043,7 +1043,12 @@ virNodeDeviceObjListRemove;
>
> # conf/virnwfilterbindingdef.h
> virNWFilterBindingDefCopy;
> +virNWFilterBindingDefFormat;
> +virNWFilterBindingDefFormatBuf;
> virNWFilterBindingDefFree;
> +virNWFilterBindingDefParseFile;
> +virNWFilterBindingDefParseNode;
> +virNWFilterBindingDefParseString;
>
>
> # conf/virnwfilterobj.h
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 621480dd0c..036335e770 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -156,6 +156,7 @@ EXTRA_DIST = \
> virmock.h \
> virnetdaemondata \
> virnetdevtestdata \
> + virnwfilterbindingxml2xmldata \
> virpcitestdata \
> virscsidata \
> virsh-uriprecedence \
> @@ -352,6 +353,7 @@ test_programs += storagebackendsheepdogtest
> endif WITH_STORAGE_SHEEPDOG
>
> test_programs += nwfilterxml2xmltest
> +test_programs += virnwfilterbindingxml2xmltest
>
> if WITH_NWFILTER
> test_programs += nwfilterebiptablestest
> @@ -855,6 +857,11 @@ nwfilterxml2xmltest_SOURCES = \
> testutils.c testutils.h
> nwfilterxml2xmltest_LDADD = $(LDADDS)
>
> +virnwfilterbindingxml2xmltest_SOURCES = \
> + virnwfilterbindingxml2xmltest.c \
> + testutils.c testutils.h
> +virnwfilterbindingxml2xmltest_LDADD = $(LDADDS)
> +
> if WITH_NWFILTER
> nwfilterebiptablestest_SOURCES = \
> nwfilterebiptablestest.c \
> diff --git a/tests/virnwfilterbindingxml2xmldata/filter-vars.xml b/tests/virnwfilterbindingxml2xmldata/filter-vars.xml
> new file mode 100644
> index 0000000000..dcff9640ce
> --- /dev/null
> +++ b/tests/virnwfilterbindingxml2xmldata/filter-vars.xml
> @@ -0,0 +1,11 @@
> +<filterbinding>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <portdev name='vnet0'/>
> + <mac address='52:54:00:7b:35:93'/>
> + <filterref filter='clean-traffic'>
> + <parameter name='MAC' value='52:54:00:7b:35:93'/>
> + </filterref>
> +</filterbinding>
> diff --git a/tests/virnwfilterbindingxml2xmldata/simple.xml b/tests/virnwfilterbindingxml2xmldata/simple.xml
> new file mode 100644
> index 0000000000..4577729a3c
> --- /dev/null
> +++ b/tests/virnwfilterbindingxml2xmldata/simple.xml
> @@ -0,0 +1,9 @@
> +<filterbinding>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <portdev name='vnet0'/>
> + <mac address='52:54:00:7b:35:93'/>
> + <filterref filter='clean-traffic'/>
> +</filterbinding>
> diff --git a/tests/virnwfilterbindingxml2xmltest.c b/tests/virnwfilterbindingxml2xmltest.c
> new file mode 100644
> index 0000000000..96edbdcf59
> --- /dev/null
> +++ b/tests/virnwfilterbindingxml2xmltest.c
> @@ -0,0 +1,113 @@
> +/*
> + * virnwfilterbindingxml2xmltest.h: network filter binding XML testing
> + *
> + * Copyright (C) 2018 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
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <fcntl.h>
> +
> +#include "internal.h"
> +#include "testutils.h"
> +#include "virxml.h"
> +#include "virnwfilterbindingdef.h"
> +#include "testutilsqemu.h"
> +#include "virstring.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +static int
> +testCompareXMLToXMLFiles(const char *xml)
> +{
> + char *actual = NULL;
> + int ret = -1;
> + virNWFilterBindingDefPtr dev = NULL;
> +
> + virResetLastError();
> +
> + if (!(dev = virNWFilterBindingDefParseFile(xml))) {
> + goto fail;
> + }
> +
> + if (!(actual = virNWFilterBindingDefFormat(dev)))
> + goto fail;
> +
> + if (virTestCompareToFile(actual, xml) < 0)
> + goto fail;
> +
> + ret = 0;
> +
> + fail:
> + VIR_FREE(actual);
> + virNWFilterBindingDefFree(dev);
> + return ret;
> +}
> +
> +typedef struct test_parms {
> + const char *name;
> +} test_parms;
> +
> +static int
> +testCompareXMLToXMLHelper(const void *data)
> +{
> + int result = -1;
> + const test_parms *tp = data;
> + char *xml = NULL;
> +
> + if (virAsprintf(&xml, "%s/virnwfilterbindingxml2xmldata/%s.xml",
> + abs_srcdir, tp->name) < 0) {
> + goto cleanup;
> + }
> +
> + result = testCompareXMLToXMLFiles(xml);
> +
> + cleanup:
> + VIR_FREE(xml);
> +
> + return result;
> +}
> +
> +static int
> +mymain(void)
> +{
> + int ret = 0;
> +
> +#define DO_TEST(NAME) \
> + do { \
> + test_parms tp = { \
> + .name = NAME, \
> + }; \
> + if (virTestRun("NWFilter XML-2-XML " NAME, \
> + testCompareXMLToXMLHelper, (&tp)) < 0) \
> + ret = -1; \
> + } while (0)
> +
> + DO_TEST("simple");
> + DO_TEST("filter-vars");
> +
> + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIR_TEST_MAIN(mymain)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
