On 21.03.2019 13:41, Daniel P. Berrangé wrote:
> 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 ?
The bug is next. In case of next domain conf:
<interface type='network'>
<mac address='00:1c:42:91:b2:cd'/>
<target dev='vme4291b2cd'/>
<filterref filter='no-mac-spoofing'>
<parameter name='MAC' value='00:1C:42:91:B2:CD'/>
<parameter name='MAC' value='00:1C:42:FC:23:78'/>
</filterref>
</interface>
<interface type='network'>
<mac address='00:1c:42:fc:23:78'/>
<target dev='vme42fc2378'/>
<filterref filter='no-mac-spoofing'>
<parameter name='MAC' value='00:1C:42:91:B2:CD'/>
<parameter name='MAC' value='00:1C:42:FC:23:78'/>
</filterref>
</interface>
ebtables-save:
-A I-vme4291b2cd-mac -s 00:1c:42:91:b2:cd -j RETURN
-A I-vme4291b2cd-mac -j DROP
-A I-vme42fc2378-mac -s 00:1c:42:fc:23:78 -j RETURN
-A I-vme42fc2378-mac -j DROP
while should be:
-A I-vme4291b2cd-mac -s 00:1c:42:91:b2:cd -j RETURN
-A I-vme4291b2cd-mac -s 00:1c:42:fc:23:78 -j RETURN
-A I-vme4291b2cd-mac -j DROP
-A I-vme42fc2378-mac -s 00:1c:42:91:b2:cd -j RETURN
-A I-vme42fc2378-mac -s 00:1c:42:fc:23:78 -j RETURN
-A I-vme42fc2378-mac -j DROP
Nikolay
>
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovs...@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@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
> Regards,
> Daniel
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
