On Mon, Mar 26, 2012 at 01:25:48PM -0700, David L Stevens wrote:
This patch adds DHCP snooping support to libvirt. The learning method for IP addresses is specified by setting the "ip_learning" variable to one of "any" [default] (existing IP learning code), "none" (static only addresses) or "dhcp" (DHCP snooping).
Active leases are saved in a lease file and reloaded on restart or HUP.
Changes since v6: - replace pthread_cancel() with synchronous cancelation method
Signed-off-by: David L Stevens <dlstevens@us.ibm.com> --- docs/formatnwfilter.html.in | 17 + src/Makefile.am | 2 + src/nwfilter/nwfilter_dhcpsnoop.c | 1139 ++++++++++++++++++++++++++++++++ src/nwfilter/nwfilter_dhcpsnoop.h | 38 ++ src/nwfilter/nwfilter_driver.c | 6 + src/nwfilter/nwfilter_gentech_driver.c | 59 ++- 6 files changed, 1248 insertions(+), 13 deletions(-) create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h
diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 9cb7644..ad10765 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c new file mode 100644 index 0000000..8e5dcc5 --- /dev/null +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
+#ifdef HAVE_LIBPCAP + +#define LEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.leases" +#define TMPLEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.ltmp" +static int lease_fd = -1; +static int nleases = 0; /* number of active leases */ +static int wleases = 0; /* number of written leases */ + +static virHashTablePtr SnoopReqs; +static virHashTablePtr IfnameToKey; +static pthread_mutex_t SnoopLock = PTHREAD_MUTEX_INITIALIZER; + +/* snooper thread management */ + +static virHashTablePtr Active; +static pthread_mutex_t ActiveLock = PTHREAD_MUTEX_INITIALIZER;
I'd prefer to see all these random variables put into a 'struct virNWFilterSnoopState', and have a single global variable, or even better, pass an instance of the struct into the methods that require it.
+SnoopActivate(pthread_t thread) +SnoopCancel(char **ThreadKey) +SnoopIsActive(char *ThreadKey)
+#define snoop_lock() { pthread_mutex_lock(&SnoopLock); } +#define snoop_unlock() { pthread_mutex_unlock(&SnoopLock); }
+struct iplease {
+static struct iplease *ipl_getbyip(struct iplease *start, uint32_t ipaddr); +static void ipl_update(struct iplease *pl, uint32_t timeout); +static struct virNWFilterSnoopReq *newreq(const char *ifkey); +static void lease_open(void); +static void lease_close(void); +static void lease_load(void); +static void lease_save(struct iplease *ipl); +static void lease_restore(struct virNWFilterSnoopReq *req); +static void lease_refresh(void);
+ipl_ladd(struct iplease *plnew, struct iplease **start, struct iplease **end)
+ipl_tadd(struct iplease *plnew)
+ipl_install(struct iplease *ipl)
+ipl_add(struct iplease *plnew, bool update_leasefile)
+ipl_ldel(struct iplease *ipl, struct iplease **start, struct iplease **end)
+ipl_tdel(struct iplease *ipl)
+ipl_del(struct virNWFilterSnoopReq *req, uint32_t ipaddr, bool update_leasefile)
+ipl_update(struct iplease *ipl, uint32_t timeout)
+ipl_getbyip(struct iplease *start, uint32_t ipaddr)
+ipl_trun(struct virNWFilterSnoopReq *req)
+struct eth {
+struct dhcp {
+dhcp_getopt(struct dhcp *pd, int len, int *pmtype, int *pleasetime)
+dhcpdecode(struct virNWFilterSnoopReq *req, struct eth *pep, int len)
+dhcpopen(const char *intf)
+snoopReqFree(struct virNWFilterSnoopReq *req)
+virNWFilterDHCPSnoop(void *req0)
+ifkeyFormat(char *ifkey, const unsigned char *vmuuid, + unsigned const char *macaddr)
+virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, + const char *ifname, + const char *linkdev, + enum virDomainNetType nettype, + const unsigned char *vmuuid, + const unsigned char *macaddr, + const char *filtername, + virNWFilterHashTablePtr filterparams, + virNWFilterDriverStatePtr driver)
+freeReq(void *req0, const void *name ATTRIBUTE_UNUSED)
+lease_close(void)
+lease_open(void)
+virNWFilterDHCPSnoopInit(void)
+virNWFilterDHCPSnoopEnd(const char *ifname)
+lease_write(int lfd, const char *ifkey, struct iplease *ipl)
+lease_save(struct iplease *ipl)
+newreq(const char *ifkey)
+SaveSnoopReqIter(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data)
+lease_refresh(void)
+lease_load(void)
+lease_restore(struct virNWFilterSnoopReq *req)
The number of different function / struct naming schemes used here is out of control. Please change everything to use 'virNWFilter' or 'virNWFilterDHCPSnoop' as a prefix, and avoid '_' in function or struct names. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|