Re: [PATCH] nwfilter: fix adding std MAC and IP values to filter binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxx
> 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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux