Re: [libvirt] nwfilter: Enable detection of multiple IP addresses
On 11/22/2011 03:08 PM, Stefan Berger wrote:
In preparation of DHCP Snooping and the detection of multiple IP
addresses per interface:
The hash table that is used to collect the detected IP address of an
interface can so far only handle one IP address per interface. With
this patch we extend this to allow it to handle a list of IP addresses.
Above changes the returned variable type of virNWFilterGetIpAddrForIfname()
from char * to virNWFilterVarValuePtr; adapt all existing functions calling
this function.
---
src/conf/nwfilter_params.c | 62 ++++++++++++++++++--
src/conf/nwfilter_params.h | 7 +-
src/libvirt_private.syms | 5 +
src/nwfilter/nwfilter_gentech_driver.c | 21 ++-----
src/nwfilter/nwfilter_gentech_driver.h | 2
src/nwfilter/nwfilter_learnipaddr.c | 98 +++++++++++++++++++++++++--------
src/nwfilter/nwfilter_learnipaddr.h | 6 +-
7 files changed, 155 insertions(+), 46 deletions(-)
-
-void
-virNWFilterDelIpAddrForIfname(const char *ifname) {
+/* Delete all or a specific IP address from an interface.
+ *
+ * @ifname: The name of the (tap) interface
+ * @addr: An IPv4 address in dotted decimal format that the (tap)
+ * interface is not using anymore; provide NULL to remove all IP
+ * addresses associated with the given interface
+ *
+ * This function returns the number of IP addresses that are still
+ * known to be associated with this interface, in case of an error
+ * -1 is returned. Error conditions are:
+ * - no IP addresses is known to be associated with an interface
When should we return 0 vs. -1? There are several situations, with this
being my guess for the most useful semantics:
ifname ipaddr return
in table non-NULL, and associated with ifname length - 1
in table non-NULL, but not found in ifname's list length
in table NULL 0
not in table non-NULL -1
not in table NULL 0
+ */
+int
+virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr)
+{
+ int ret = -1;
+ virNWFilterVarValuePtr val = NULL;
virMutexLock(&ipAddressMapLock);
- if (virHashLookup(ipAddressMap->hashTable, ifname))
- virNWFilterHashTableRemoveEntry(ipAddressMap, ifname);
+ if (ipaddr != NULL) {
+ val = virHashLookup(ipAddressMap->hashTable, ifname);
+ if (val) {
+ if (virNWFilterVarValueGetCardinality(val) == 1)
+ goto remove_entry;
This says that if ifname has exactly one ipaddr associated, then remove
that address, even if it does not match the ipaddr...
+ virNWFilterVarValueDelValue(val, ipaddr);
that we had intended to be filtering on. This logic needs to be fixed.
+ ret = virNWFilterVarValueGetCardinality(val);
+ }
+ } else {
+remove_entry:
+ /* remove whole entry */
+ val = virNWFilterHashTableRemoveEntry(ipAddressMap, ifname);
+ if (val) {
+ ret = 0;
+ virNWFilterVarValueFree(val);
+ }
+ }
+++ libvirt-acl/src/libvirt_private.syms
@@ -846,9 +846,14 @@ virNWFilterVarCombIterCreate;
virNWFilterVarCombIterFree;
virNWFilterVarCombIterGetVarValue;
virNWFilterVarCombIterNext;
+virNWFilterVarValueAddValue;
+virNWFilterVarValueCopy;
virNWFilterVarValueCreateSimple;
virNWFilterVarValueCreateSimpleCopyValue;
+virNWFilterVarValueDelValue;
+virNWFilterVarValueFree;
virNWFilterVarValueGetSimple;
+virNWFilterVarValueGetCardinality;
Those last two lines aren't sorted :)
I think the logic bug fix warrants a v2, but the bulk of the patch looks
good.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)