On 03/30/2010 11:56 AM, Stefan Berger wrote:
Subject: Support for learning a VM's IP address
This patch implements support for learning a VM's IP address. It uses
the pcap library to listen on the VM's backend network interface (tap)
or the physical ethernet device (macvtap) and tries to capture packets
with source or destination MAC address of the VM and learn from DHCP
Offers, ARP traffic, or first-sent IPv4 packet what the IP address of
the VM's interface is. This then allows to instantiate the network
traffic filtering rules without the user having to provide the IP
parameter somewhere in the filter description or in the interface
description as a parameter. This only supports to detect the parameter
IP, which is for the assumed single IPv4 address of a VM. There is not
support for interfaces that may have multiple IP addresses (IP
aliasing) or IPv6 that may then require more than one valid IP address
to be detected. A VM can have multiple independent interfaces that each
uses a different IP address and in that case it will be attempted to
detect each one of the address independently.
Sounds interesting.
---
configure.ac | 39 ++
Should there also be an adjustment to the .rpm spec files, to mention
that we now might depend on libpcap (and building of all features
depends on libpcap-devel)?
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
...
+
+#ifdef HAVE_LIBPCAP
+#include <pcap.h>
+#endif
Fails 'make syntax-check' if you install cppi (which is now part of Fedora).
+
+#include <fcntl.h>
+#include <sys/ioctl.h>
+
+#include <arpa/inet.h>
+#include <net/ethernet.h>
+#include <netinet/ip.h>
+#include <netinet/udp.h>
+#include <net/if_arp.h>
+#include <linux/if.h>
Can we guarantee that this header always exists, or should it be
conditionally included?
+
+
+/* structure of an ARP request/reply message */
+struct f_arphdr {
+ struct arphdr arphdr;
+ uint8_t ar_sha[ETH_ALEN];
+ uint32_t ar_sip;
+ uint8_t ar_tha[ETH_ALEN];
+ uint32_t ar_tip;
+} __attribute__((packed));
This assumes GCC; do we ever intend to support other compilers?
+/**
+ * virNWFilterLearnInit
+ * Initialization of this layer
+ */
+int
+virNWFilterLearnInit(void) {
+ pendingLearnReq = virHashCreate(0);
+ if (!pendingLearnReq) {
+ virReportOOMError();
+ return 1;
+ }
+
+ if (virMutexInit(&pendingLearnReqLock)) {
+ virNWFilterLearnShutdown();
+ return 1;
+ }
Is this function leaking memory if it fails mid-way through? Or are you
requiring that the caller will call virNWFilterShutdown even if
virNWFilterLearnInit returns failure?
Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
@@ -24,6 +24,9 @@
#include <config.h>
#include <stdint.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <linux/if.h>
Another instance of using a non-portable header.
I've read through the patch, and didn't spot many glaring errors. I
have not tested it, though, and would rather defer to others to also
look through it before approving it, since it is rather large.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org