[libvirt] [PATCH] nwfilter: fix adding std MAC and IP values to filter binding
Daniel P. Berrangé
berrange at redhat.com
Thu Mar 21 10:41:25 UTC 2019
On Thu, Mar 21, 2019 at 10:32:07AM +0300, Nikolay Shirokovskiy wrote:
> Commit d1a7c08eb changed filter instantiation code to ignore MAC and IP
> variables explicitly specified for filter binding. It just replaces
> explicit values with values associated with the binding. Before the
> commit virNWFilterCreateVarsFrom was used so that explicit value
> take precedence. Let's bring old behavior back.
>
> This is useful. For example if domain has two interfaces it makes
> sense to list both mac adresses in MAC var of every interface
> filterref. So that if guest make a bond of these interfaces
> and start sending frames with one of the mac adresses from
> both interfaces we can pass outgress traffic from both
> interfaces too.
I'm not seeing what is supposed to be broken by d1a7c08eb. I have a
guest which has a filter defined
<interface type='network'>
<mac address='52:54:00:7b:35:93'/>
<source network='default' bridge='virbr0'/>
<target dev='vnet0'/>
<model type='rtl8139'/>
<filterref filter='clean-traffic'>
<parameter name='IP' value='104.207.129.11'/>
</filterref>
<alias name='net0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</interface>
This IP parameter is reflected in the binding
# virsh nwfilter-binding-dumpxml vnet0
<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='IP' value='104.207.129.11'/>
<parameter name='MAC' value='52:54:00:7b:35:93'/>
</filterref>
</filterbinding>
and in the ebtables rules
Bridge chain: I-vnet0-ipv4-ip, entries: 3, policy: ACCEPT
-p IPv4 --ip-src 0.0.0.0 --ip-proto udp -j RETURN
-p IPv4 --ip-src 104.207.129.11 -j RETURN
-j DROP
So what's the bug ?
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
> src/nwfilter/nwfilter_gentech_driver.c | 92 ++++++++++++----------------------
> 1 file changed, 32 insertions(+), 60 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index 655f088..6d68189 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -127,60 +127,6 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
>
>
> /**
> - * virNWFilterVarHashmapAddStdValues:
> - * @tables: pointer to hash tabel to add values to
> - * @macaddr: The string of the MAC address to add to the hash table,
> - * may be NULL
> - * @ipaddr: The string of the IP address to add to the hash table;
> - * may be NULL
> - *
> - * Returns 0 in case of success, -1 in case an error happened with
> - * error having been reported.
> - *
> - * Adds a couple of standard keys (MAC, IP) to the hash table.
> - */
> -static int
> -virNWFilterVarHashmapAddStdValues(virHashTablePtr table,
> - const char *macaddr,
> - const virNWFilterVarValue *ipaddr)
> -{
> - virNWFilterVarValue *val;
> -
> - if (macaddr) {
> - val = virNWFilterVarValueCreateSimpleCopyValue(macaddr);
> - if (!val)
> - return -1;
> -
> - if (virHashUpdateEntry(table,
> - NWFILTER_STD_VAR_MAC,
> - val) < 0) {
> - virNWFilterVarValueFree(val);
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Could not add variable 'MAC' to hashmap"));
> - return -1;
> - }
> - }
> -
> - if (ipaddr) {
> - val = virNWFilterVarValueCopy(ipaddr);
> - if (!val)
> - return -1;
> -
> - if (virHashUpdateEntry(table,
> - NWFILTER_STD_VAR_IP,
> - val) < 0) {
> - virNWFilterVarValueFree(val);
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("Could not add variable 'IP' to hashmap"));
> - return -1;
> - }
> - }
> -
> - return 0;
> -}
> -
> -
> -/**
> * Convert a virHashTable into a string of comma-separated
> * variable names.
> */
> @@ -705,6 +651,28 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
> }
>
>
> +static int
> +virNWFilterVarHashmapAddStdValue(virHashTablePtr table,
> + const char *var,
> + const char *value)
> +{
> + virNWFilterVarValue *val;
> +
> + if (virHashLookup(table, var))
> + return 0;
> +
> + if (!(val = virNWFilterVarValueCreateSimpleCopyValue(value)))
> + return -1;
> +
> + if (virHashAddEntry(table, var, val) < 0) {
> + virNWFilterVarValueFree(val);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> /*
> * Call this function while holding the NWFilter filter update lock
> */
> @@ -717,7 +685,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
> bool forceWithPendingReq,
> bool *foundNewFilter)
> {
> - int rc;
> + int rc = -1;
> const char *drvname = EBIPTABLES_DRIVER_ID;
> virNWFilterTechDriverPtr techdriver;
> virNWFilterObjPtr obj;
> @@ -743,14 +711,18 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
> return -1;
>
> virMacAddrFormat(&binding->mac, vmmacaddr);
> + if (virNWFilterVarHashmapAddStdValue(binding->filterparams,
> + NWFILTER_STD_VAR_MAC,
> + vmmacaddr) < 0)
> + goto err_exit;
>
> ipaddr = virNWFilterIPAddrMapGetIPAddr(binding->portdevname);
> -
> - if (virNWFilterVarHashmapAddStdValues(binding->filterparams,
> - vmmacaddr, ipaddr) < 0) {
> - rc = -1;
> + if (ipaddr &&
> + virNWFilterVarHashmapAddStdValue(binding->filterparams,
> + NWFILTER_STD_VAR_IP,
> + virNWFilterVarValueGetSimple(ipaddr)) < 0)
> goto err_exit;
> - }
> +
>
> filter = virNWFilterObjGetDef(obj);
>
> --
> 1.8.3.1
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list