[libvirt] [PATCH 0/2] Add MAC filtering to qemu
The following series of patches is a prototype implementation of a ebtables based MAC address filter. I hope to have addressed all the previous comments. At the moment, some defaults are set when libvirtd starts and when a domain is started or destroyed. The plan is to extend this filter capability to the API level and virsh command level. The plan is also to add more filtering features like VLAN filtering and QoS filtering. Thanks, Gerhard --- Gerhard Stenzel (2): add MAC address based port filtering to qemu add ebtables wrapper configure.in | 3 po/POTFILES.in | 1 src/Makefile.am | 5 src/libvirt_private.syms | 7 + src/qemu/qemu.conf | 2 src/qemu/qemu_bridge_filter.c | 108 ++++++++++ src/qemu/qemu_bridge_filter.h | 39 ++++ src/qemu/qemu_conf.c | 25 ++ src/qemu/qemu_conf.h | 4 src/qemu/qemu_driver.c | 16 + src/util/ebtables.c | 441 +++++++++++++++++++++++++++++++++++++++++ src/util/ebtables.h | 65 ++++++ 12 files changed, 715 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_bridge_filter.c create mode 100644 src/qemu/qemu_bridge_filter.h create mode 100644 src/util/ebtables.c create mode 100644 src/util/ebtables.h -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
This patch adds the files which implement the ebtables wrapper. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> --- configure.in | 3 src/Makefile.am | 1 src/libvirt_private.syms | 7 + src/util/ebtables.c | 441 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/ebtables.h | 65 +++++++ 5 files changed, 517 insertions(+), 0 deletions(-) create mode 100644 src/util/ebtables.c create mode 100644 src/util/ebtables.h diff --git a/configure.in b/configure.in index 2f9db72..d8abf9f 100644 --- a/configure.in +++ b/configure.in @@ -287,6 +287,9 @@ fi AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary]) +AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH]) +AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables binary]) + if test "$with_openvz" = "yes"; then AC_DEFINE_UNQUOTED([WITH_OPENVZ], 1, [whether OpenVZ driver is enabled]) fi diff --git a/src/Makefile.am b/src/Makefile.am index 8e27ea7..055573a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,6 +51,7 @@ UTIL_SOURCES = \ util/event.c util/event.h \ util/hash.c util/hash.h \ util/iptables.c util/iptables.h \ + util/ebtables.c util/ebtables.h \ util/logging.c util/logging.h \ util/memory.c util/memory.h \ util/pci.c util/pci.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 98ea7f8..42fb654 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -234,6 +234,13 @@ iptablesRemoveUdpInput; iptablesSaveRules; +# ebtables.h +ebtablesRemoveForwardAllowIn; +ebtablesAddForwardAllowIn; +ebtablesAddForwardPolicyReject; +ebtablesContextNew; +ebtablesSaveRules; + # libvirt_internal.h virStateInitialize; virStateCleanup; diff --git a/src/util/ebtables.c b/src/util/ebtables.c new file mode 100644 index 0000000..78f273a --- /dev/null +++ b/src/util/ebtables.c @@ -0,0 +1,441 @@ +/* + * Copyright (C) 2009 IBM Corp. + * Copyright (C) 2007-2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * based on iptables.c + * Authors: + * Gerhard Stenzel <gerhard.stenzel@de.ibm.com> + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdarg.h> +#include <string.h> +#include <errno.h> +#include <limits.h> +#include <unistd.h> +#include <fcntl.h> +#include <sys/types.h> +#include <sys/stat.h> + +#ifdef HAVE_SYS_WAIT_H +#include <sys/wait.h> +#endif + +#ifdef HAVE_PATHS_H +#include <paths.h> +#endif + +#include "internal.h" +#include "ebtables.h" +#include "util.h" +#include "memory.h" +#include "virterror_internal.h" +#include "logging.h" + +struct _ebtablesContext +{ + ebtRules *input_filter; + ebtRules *forward_filter; + ebtRules *nat_postrouting; +}; + +enum { + ADD = 0, + REMOVE, + CREATE, + POLICY, + INSERT +}; + +static void +ebtRulesSave(ebtRules *rules) +{ + (void) rules; +} + +static void +ebtRuleFree(ebtRule *rule) +{ + VIR_FREE(rule->rule); + + if (rule->argv) { + int i = 0; + while (rule->argv[i]) + VIR_FREE(rule->argv[i++]); + VIR_FREE(rule->argv); + } +} + +static int +ebtRulesAppend(ebtRules *rules, + char *rule, + const char **argv, + int command_idx) +{ + if (VIR_REALLOC_N(rules->rules, rules->nrules+1) < 0) { + int i = 0; + while (argv[i]) + VIR_FREE(argv[i++]); + VIR_FREE(argv); + return ENOMEM; + } + + rules->rules[rules->nrules].rule = rule; + rules->rules[rules->nrules].argv = argv; + rules->rules[rules->nrules].command_idx = command_idx; + + rules->nrules++; + + return 0; +} + +static int +ebtRulesRemove(ebtRules *rules, + char *rule) +{ + int i; + + for (i = 0; i < rules->nrules; i++) + if (STREQ(rules->rules[i].rule, rule)) + break; + + if (i >= rules->nrules) + return EINVAL; + + ebtRuleFree(&rules->rules[i]); + + memmove(&rules->rules[i], + &rules->rules[i+1], + (rules->nrules - i - 1) * sizeof (ebtRule)); + + rules->nrules--; + + return 0; +} + +static void +ebtRulesFree(ebtRules *rules) +{ + int i; + + VIR_FREE(rules->table); + VIR_FREE(rules->chain); + + if (rules->rules) { + for (i = 0; i < rules->nrules; i++) + ebtRuleFree(&rules->rules[i]); + + VIR_FREE(rules->rules); + + rules->nrules = 0; + } + + VIR_FREE(rules); +} + +static ebtRules * +ebtRulesNew(const char *table, + const char *chain) +{ + ebtRules *rules; + + if (VIR_ALLOC(rules) < 0) + return NULL; + + if (!(rules->table = strdup(table))) + goto error; + + if (!(rules->chain = strdup(chain))) + goto error; + + rules->rules = NULL; + rules->nrules = 0; + + return rules; + + error: + ebtRulesFree(rules); + return NULL; +} + +static int +ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...) +{ + va_list args; + int retval = ENOMEM; + const char **argv; + char *rule = NULL; + const char *s; + int n, command_idx; + + n = 1 + /* /sbin/ebtables */ + 2 + /* --table foo */ + 2 + /* --insert bar */ + 1; /* arg */ + + va_start(args, arg); + while ((s = va_arg(args, const char *))) + n++; + + va_end(args); + + if (VIR_ALLOC_N(argv, n + 1) < 0) + goto error; + + n = 0; + + if (!(argv[n++] = strdup(EBTABLES_PATH))) + goto error; + + command_idx = n; + + if(action == ADD || action == REMOVE) { + if (!(argv[n++] = strdup("--insert"))) + goto error; + + if (!(argv[n++] = strdup(rules->chain))) + goto error; + } + + if (!(argv[n++] = strdup(arg))) + goto error; + + va_start(args, arg); + + while ((s = va_arg(args, const char *))) + if (!(argv[n++] = strdup(s))) + goto error; + + va_end(args); + + if (!(rule = virArgvToString(&argv[command_idx]))) + goto error; + + if (action == REMOVE) { + VIR_FREE(argv[command_idx]); + if (!(argv[command_idx] = strdup("--delete"))) + goto error; + } + + if (virRun(NULL, argv, NULL) < 0) { + retval = errno; + goto error; + } + + if (action == ADD || action == CREATE || action == POLICY || action == INSERT) { + retval = ebtRulesAppend(rules, rule, argv, command_idx); + rule = NULL; + argv = NULL; + } else { + retval = ebtRulesRemove(rules, rule); + } + + error: + VIR_FREE(rule); + + if (argv) { + n = 0; + while (argv[n]) + VIR_FREE(argv[n++]); + VIR_FREE(argv); + } + + return retval; +} + + +/** + * ebtablesContextNew: + * + * Create a new ebtable context + * + * Returns a pointer to the new structure or NULL in case of error + */ +ebtablesContext * +ebtablesContextNew(const char *driver) +{ + ebtablesContext *ctx; + char chain[PATH_MAX]; + + if (VIR_ALLOC(ctx) < 0) + return NULL; + + snprintf(chain, sizeof(chain), "libvirt_%s_INPUT", driver); + if (!(ctx->input_filter = ebtRulesNew("filter", chain))) + goto error; + + snprintf(chain, sizeof(chain), "libvirt_%s_FORWARD", driver); + if (!(ctx->forward_filter = ebtRulesNew("filter", chain))) + goto error; + + snprintf(chain, sizeof(chain), "libvirt_%s_POSTROUTING", driver); + if (!(ctx->nat_postrouting = ebtRulesNew("nat", chain))) + goto error; + + return ctx; + + error: + ebtablesContextFree(ctx); + return NULL; +} + +/** + * ebtablesContextFree: + * @ctx: pointer to the EB table context + * + * Free the resources associated with an EB table context + */ +void +ebtablesContextFree(ebtablesContext *ctx) +{ + if (ctx->input_filter) + ebtRulesFree(ctx->input_filter); + if (ctx->forward_filter) + ebtRulesFree(ctx->forward_filter); + if (ctx->nat_postrouting) + ebtRulesFree(ctx->nat_postrouting); + VIR_FREE(ctx); +} + +/** + * ebtablesSaveRules: + * @ctx: pointer to the EB table context + * + * Saves all the EB table rules associated with a context + * to disk so that if ebtables is restarted, the rules + * will automatically be reload. + */ +void +ebtablesSaveRules(ebtablesContext *ctx) +{ + ebtRulesSave(ctx->input_filter); + ebtRulesSave(ctx->forward_filter); + ebtRulesSave(ctx->nat_postrouting); +} + +int +ebtablesAddForwardPolicyReject(ebtablesContext *ctx) +{ + return ebtablesForwardPolicyReject(ctx, ADD); +} + + +int +ebtablesRemoveForwardPolicyReject(ebtablesContext *ctx) +{ + return ebtablesForwardPolicyReject(ctx, REMOVE); +} + +int +ebtablesForwardPolicyReject(ebtablesContext *ctx, + int action) +{ + /* create it, if it does not exist */ + if (action == ADD) { + ebtablesAddRemoveRule(ctx->forward_filter, + CREATE, + "--new-chain", ctx->forward_filter->chain, NULL, + NULL); + ebtablesAddRemoveRule(ctx->forward_filter, + INSERT, + "--insert", "FORWARD", "--jump", + ctx->forward_filter->chain, NULL); + } + + return ebtablesAddRemoveRule(ctx->forward_filter, + POLICY, + "-P", ctx->forward_filter->chain, "DROP", + NULL); +} + +/* + * Allow all traffic destined to the bridge, with a valid network address + */ +static int +ebtablesForwardAllowIn(ebtablesContext *ctx, + const char *iface, + const char *macaddr, + int action) +{ + return ebtablesAddRemoveRule(ctx->forward_filter, + action, + "--in-interface", iface, + "--source", macaddr, + "--jump", "ACCEPT", + NULL); +} + +/** + * ebtablesAddForwardAllowIn: + * @ctx: pointer to the EB table context + * @iface: the output interface name + * @physdev: the physical input device or NULL + * + * Add rules to the EB table context to allow the traffic on + * @physdev device to be forwarded to interface @iface. This allows + * the inbound traffic on a bridge. + * + * Returns 0 in case of success or an error code otherwise + */ +int +ebtablesAddForwardAllowIn(ebtablesContext *ctx, + const char *iface, + const unsigned char *mac) +{ + char *macaddr; + + if (virAsprintf(&macaddr, + "%02x:%02x:%02x:%02x:%02x:%02x", + mac[0], mac[1], + mac[2], mac[3], + mac[4], mac[5]) < 0) { + return -1; + } + return ebtablesForwardAllowIn(ctx, iface, macaddr, ADD); +} + +/** + * ebtablesRemoveForwardAllowIn: + * @ctx: pointer to the EB table context + * @iface: the output interface name + * @physdev: the physical input device or NULL + * + * Remove rules from the EB table context hence forbidding the traffic + * on the @physdev device to be forwarded to interface @iface. This + * stops the inbound traffic on a bridge. + * + * Returns 0 in case of success or an error code otherwise + */ +int +ebtablesRemoveForwardAllowIn(ebtablesContext *ctx, + const char *iface, + const unsigned char *mac) +{ + char *macaddr; + + if (virAsprintf(&macaddr, + "%02x:%02x:%02x:%02x:%02x:%02x", + mac[0], mac[1], + mac[2], mac[3], + mac[4], mac[5]) < 0) { + return -1; + } + return ebtablesForwardAllowIn(ctx, iface, macaddr, REMOVE); +} diff --git a/src/util/ebtables.h b/src/util/ebtables.h new file mode 100644 index 0000000..cafe418 --- /dev/null +++ b/src/util/ebtables.h @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2009 IBM Corp. + * Copyright (C) 2007, 2008 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * based on iptables.h + * Authors: + * Gerhard Stenzel <gerhard.stenzel@de.ibm.com> + */ + +#ifndef __QEMUD_EBTABLES_H__ +#define __QEMUD_EBTABLES_H__ + +typedef struct +{ + char *rule; + const char **argv; + int command_idx; +} ebtRule; + +typedef struct +{ + char *table; + char *chain; + + int nrules; + ebtRule *rules; + +} ebtRules; + +typedef struct _ebtablesContext ebtablesContext; + +ebtablesContext *ebtablesContextNew (const char *driver); +void ebtablesContextFree (ebtablesContext *ctx); + +void ebtablesSaveRules (ebtablesContext *ctx); + +int ebtablesAddForwardAllowIn (ebtablesContext *ctx, + const char *iface, + const unsigned char *mac); +int ebtablesRemoveForwardAllowIn (ebtablesContext *ctx, + const char *iface, + const unsigned char *mac); + +int ebtablesAddForwardPolicyReject(ebtablesContext *ctx); + +int ebtablesRemoveForwardPolicyReject(ebtablesContext *ctx); + +int ebtablesForwardPolicyReject(ebtablesContext *ctx, + int action); + +#endif /* __QEMUD_ebtabLES_H__ */
On Tue, Oct 27, 2009 at 12:36:09PM +0100, Gerhard Stenzel wrote:
This patch adds the files which implement the ebtables wrapper.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> ---
configure.in | 3 src/Makefile.am | 1 src/libvirt_private.syms | 7 + src/util/ebtables.c | 441 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/ebtables.h | 65 +++++++ 5 files changed, 517 insertions(+), 0 deletions(-) create mode 100644 src/util/ebtables.c create mode 100644 src/util/ebtables.h
ACK, this looks good now. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Oct 27, 2009 at 12:36:09PM +0100, Gerhard Stenzel wrote:
This patch adds the files which implement the ebtables wrapper.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com>
+++ b/src/libvirt_private.syms @@ -234,6 +234,13 @@ iptablesRemoveUdpInput; iptablesSaveRules;
+# ebtables.h +ebtablesRemoveForwardAllowIn; +ebtablesAddForwardAllowIn; +ebtablesAddForwardPolicyReject; +ebtablesContextNew; +ebtablesSaveRules; +
Okay I just moved it before events.h to keep module sorting and also sorted the entry points.
+enum { + ADD = 0, + REMOVE, + CREATE, + POLICY, + INSERT +};
Somehow an enum without a name/type is all the problems of enums without the usefulness (compared to #define) but I must be biased :-)
+static void +ebtRulesSave(ebtRules *rules) +{ + (void) rules;
I assume this means /* TODO */
+} + [...] + + if (virRun(NULL, argv, NULL) < 0) { + retval = errno; + goto error; + } + + if (action == ADD || action == CREATE || action == POLICY || action == INSERT) {
need a long line break
+ retval = ebtRulesAppend(rules, rule, argv, command_idx); + rule = NULL; + argv = NULL; + } else { + retval = ebtRulesRemove(rules, rule); + } + [...] +/** + * ebtablesSaveRules: + * @ctx: pointer to the EB table context + * + * Saves all the EB table rules associated with a context + * to disk so that if ebtables is restarted, the rules + * will automatically be reload. + */ +void +ebtablesSaveRules(ebtablesContext *ctx) +{ + ebtRulesSave(ctx->input_filter); + ebtRulesSave(ctx->forward_filter); + ebtRulesSave(ctx->nat_postrouting); +}
Hum, and where ? Under /etc/libvirt/ebtables/.... ? Are the table and chain names provided in ebtRulesNew() sufficient to uniquely name the set ? I hope so otherwise we're gonna have trouble with persistance. It would be good to have ebtRulesSave() documented if not fully finished before next release. I'm gonna commit this, but I think we need to double-check that the current APIs won't be a problem when we want to implement saving (didn't checked the second patch yet). I also think the spec file should add a Requires to ebtables as this is not installed systematically (it wasn't present on my workstation by default). I will push this tonight, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, 2009-11-03 at 23:17 +0100, Daniel Veillard wrote:
On Tue, Oct 27, 2009 at 12:36:09PM +0100, Gerhard Stenzel wrote:
This patch adds the files which implement the ebtables wrapper.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> ... +/** + * ebtablesSaveRules: + * @ctx: pointer to the EB table context + * + * Saves all the EB table rules associated with a context + * to disk so that if ebtables is restarted, the rules + * will automatically be reload. + */ +void +ebtablesSaveRules(ebtablesContext *ctx) +{ + ebtRulesSave(ctx->input_filter); + ebtRulesSave(ctx->forward_filter); + ebtRulesSave(ctx->nat_postrouting); +}
Hum, and where ? Under /etc/libvirt/ebtables/.... ?
Are the table and chain names provided in ebtRulesNew() sufficient to uniquely name the set ? I hope so otherwise we're gonna have trouble with persistance. It would be good to have ebtRulesSave() documented if not fully finished before next release.
I'm gonna commit this, but I think we need to double-check that the current APIs won't be a problem when we want to implement saving (didn't checked the second patch yet).
I also think the spec file should add a Requires to ebtables as this is not installed systematically (it wasn't present on my workstation by default).
I will push this tonight,
thanks !
Daniel
This patch removes the ebtablesSaveRules() function as it more confusing than useful at the moment. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -179,7 +179,6 @@ ebtablesAddForwardAllowIn; ebtablesAddForwardPolicyReject; ebtablesContextNew; ebtablesRemoveForwardAllowIn; -ebtablesSaveRules; # event.h Index: libvirt/src/qemu/qemu_bridge_filter.c =================================================================== --- libvirt.orig/src/qemu/qemu_bridge_filter.c +++ libvirt/src/qemu/qemu_bridge_filter.c @@ -44,7 +44,6 @@ networkAddEbtablesRules(struct qemud_dri __FILE__); return err; } - ebtablesSaveRules(driver->ebtables); return 0; } Index: libvirt/src/util/ebtables.c =================================================================== --- libvirt.orig/src/util/ebtables.c +++ libvirt/src/util/ebtables.c @@ -65,12 +65,6 @@ enum { }; static void -ebtRulesSave(ebtRules *rules) -{ - (void) rules; -} - -static void ebtRuleFree(ebtRule *rule) { VIR_FREE(rule->rule); @@ -315,22 +309,6 @@ ebtablesContextFree(ebtablesContext *ctx VIR_FREE(ctx); } -/** - * ebtablesSaveRules: - * @ctx: pointer to the EB table context - * - * Saves all the EB table rules associated with a context - * to disk so that if ebtables is restarted, the rules - * will automatically be reload. - */ -void -ebtablesSaveRules(ebtablesContext *ctx) -{ - ebtRulesSave(ctx->input_filter); - ebtRulesSave(ctx->forward_filter); - ebtRulesSave(ctx->nat_postrouting); -} - int ebtablesAddForwardPolicyReject(ebtablesContext *ctx) { -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, Nov 04, 2009 at 05:12:30PM +0100, Gerhard Stenzel wrote:
On Tue, 2009-11-03 at 23:17 +0100, Daniel Veillard wrote:
On Tue, Oct 27, 2009 at 12:36:09PM +0100, Gerhard Stenzel wrote:
This patch adds the files which implement the ebtables wrapper.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> ... +/** + * ebtablesSaveRules: + * @ctx: pointer to the EB table context + * + * Saves all the EB table rules associated with a context + * to disk so that if ebtables is restarted, the rules + * will automatically be reload. + */ +void +ebtablesSaveRules(ebtablesContext *ctx) +{ + ebtRulesSave(ctx->input_filter); + ebtRulesSave(ctx->forward_filter); + ebtRulesSave(ctx->nat_postrouting); +}
Hum, and where ? Under /etc/libvirt/ebtables/.... ?
Are the table and chain names provided in ebtRulesNew() sufficient to uniquely name the set ? I hope so otherwise we're gonna have trouble with persistance. It would be good to have ebtRulesSave() documented if not fully finished before next release.
I'm gonna commit this, but I think we need to double-check that the current APIs won't be a problem when we want to implement saving (didn't checked the second patch yet).
I also think the spec file should add a Requires to ebtables as this is not installed systematically (it wasn't present on my workstation by default).
I will push this tonight,
thanks !
Daniel
This patch removes the ebtablesSaveRules() function as it more confusing than useful at the moment.
Okay, pushed, thanks ! Any solution on the broadcast/multicast issue raised ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
This patch adds MAC address based port filtering to the qemu driver. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> --- po/POTFILES.in | 1 src/Makefile.am | 4 +- src/qemu/qemu.conf | 2 + src/qemu/qemu_bridge_filter.c | 108 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_bridge_filter.h | 39 +++++++++++++++ src/qemu/qemu_conf.c | 25 +++++++++ src/qemu/qemu_conf.h | 4 ++ src/qemu/qemu_driver.c | 16 ++++++ 8 files changed, 198 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_bridge_filter.c create mode 100644 src/qemu/qemu_bridge_filter.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 1a12a39..e090f58 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -24,6 +24,7 @@ src/opennebula/one_driver.c src/openvz/openvz_conf.c src/openvz/openvz_driver.c src/phyp/phyp_driver.c +src/qemu/qemu_bridge_filter.c src/qemu/qemu_conf.c src/qemu/qemu_driver.c src/qemu/qemu_monitor_text.c diff --git a/src/Makefile.am b/src/Makefile.am index 055573a..8b02828 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -184,7 +184,9 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_conf.c qemu/qemu_conf.h \ qemu/qemu_monitor_text.c \ qemu/qemu_monitor_text.h \ - qemu/qemu_driver.c qemu/qemu_driver.h + qemu/qemu_driver.c qemu/qemu_driver.h \ + qemu/qemu_bridge_filter.c \ + qemu/qemu_bridge_filter.h UML_DRIVER_SOURCES = \ uml/uml_conf.c uml/uml_conf.h \ diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 6d6b86a..2af8ffe 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -152,3 +152,5 @@ # in a location of $MOUNTPOINT/libvirt/qemu # hugetlbfs_mount = "/dev/hugepages" + +mac_filter = 1 diff --git a/src/qemu/qemu_bridge_filter.c b/src/qemu/qemu_bridge_filter.c new file mode 100644 index 0000000..36dd01c --- /dev/null +++ b/src/qemu/qemu_bridge_filter.c @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2009 IBM Corp. + * Copyright (C) 2007-2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Gerhard Stenzel <gerhard.stenzel@de.ibm.com> + */ + +#include <config.h> + +#include "ebtables.h" +#include "qemu_conf.h" +#include "qemu_driver.h" +#include "util.h" +#include "virterror_internal.h" +#include "logging.h" + +#include "qemu_bridge_filter.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +int +networkAddEbtablesRules(struct qemud_driver *driver) { + int err; + + /* Set forward policy to DROP */ + if ((err = ebtablesAddForwardPolicyReject(driver->ebtables))) { + virReportSystemError(NULL, err, + _("failed to add ebtables rule to set default policy to drop on '%s'"), + __FILE__); + return err; + } + ebtablesSaveRules(driver->ebtables); + + return 0; +} + + +int +networkDisableAllFrames(struct qemud_driver *driver) { + int err; + + /* add default rules */ + if ((err = networkAddEbtablesRules(driver))) { + virReportSystemError(NULL, err, + _("cannot filter mac addresses on bridge '%s'"), + __FILE__); + return err; + } + return 0; +} + +int +networkAllowMacOnPort(virConnectPtr conn, + struct qemud_driver *driver, + const char * ifname, + const unsigned char * mac) { + + int err; + + /* allow this combination of macaddr and ifname */ + ebtablesContext * ebtablescontext = driver->ebtables; + if ((err = ebtablesAddForwardAllowIn(ebtablescontext, + ifname, + mac))) { + virReportSystemError(conn, err, + _("failed to add ebtables rule to allow routing to '%s'"), + ifname); + } + + return 0; +} + + +int +networkDisallowMacOnPort(virConnectPtr conn, + struct qemud_driver *driver, + const char * ifname, + const unsigned char * mac) { + + int err; + + /* disallow this combination of macaddr and ifname */ + ebtablesContext * ebtablescontext = driver->ebtables; + if ((err = ebtablesRemoveForwardAllowIn(ebtablescontext, + ifname, + mac))) { + virReportSystemError(conn, err, + _("failed to add ebtables rule to allow routing to '%s'"), + ifname); + } + + return 0; +} diff --git a/src/qemu/qemu_bridge_filter.h b/src/qemu/qemu_bridge_filter.h new file mode 100644 index 0000000..ccb4710 --- /dev/null +++ b/src/qemu/qemu_bridge_filter.h @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2009 IBM Corp. + * Copyright (C) 2007-2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Gerhard Stenzel <gerhard.stenzel@de.ibm.com> + */ + +#ifndef __QEMUD_BRIDGE_FILTER_H__ +#define __QEMUD_BRIDGE_FILTER_H__ + + +int networkAllowMacOnPort(virConnectPtr conn, + struct qemud_driver *driver, + const char * ifname, + const unsigned char * mac); +int networkDisallowMacOnPort(virConnectPtr conn, + struct qemud_driver *driver, + const char * ifname, + const unsigned char * mac); +int networkDisableAllFrames(struct qemud_driver *driver); +int networkAddEbtablesRules(struct qemud_driver *driver); + + +#endif /* __QEMUD_BRIDGE_FILTER_H__ */ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a095cb7..6559058 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -40,6 +40,7 @@ #include "c-ctype.h" #include "virterror_internal.h" #include "qemu_conf.h" +#include "qemu_bridge_filter.h" #include "uuid.h" #include "buf.h" #include "conf.h" @@ -318,6 +319,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "mac_filter"); + CHECK_TYPE ("mac_filter", VIR_CONF_LONG); + if (p) { + driver->macFilter = p->l; + if (!(driver->ebtables = ebtablesContextNew("qemu"))) { + driver->macFilter = 0; + virReportSystemError(NULL, errno, + _("failed to enable mac filter in in '%s'"), __FILE__); + } + + if ((errno = networkDisableAllFrames(driver))) { + virReportSystemError(NULL, errno, + _("failed to add rule to drop all frames in '%s'"), __FILE__); + } + } + virConfFree (conf); return 0; } @@ -1196,6 +1213,14 @@ qemudNetworkIfaceConnect(virConnectPtr conn, tapfd = -1; } + if (driver->macFilter) { + if ((err = networkAllowMacOnPort(conn, driver, net->ifname, net->mac))) { + virReportSystemError(conn, err, + _("failed to add ebtables rule to allow MAC address on '%s'"), + net->ifname); + } + } + cleanup: VIR_FREE(brname); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index f9a970f..42b8f56 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -26,6 +26,7 @@ #include <config.h> +#include "ebtables.h" #include "internal.h" #include "bridge.h" #include "capabilities.h" @@ -112,6 +113,9 @@ struct qemud_driver { char *hugetlbfs_mount; char *hugepage_path; + unsigned int macFilter : 1; + ebtablesContext *ebtables; + virCapsPtr caps; /* An array of callbacks */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 082cb04..aa16b4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -56,6 +56,7 @@ #include "qemu_driver.h" #include "qemu_conf.h" #include "qemu_monitor_text.h" +#include "qemu_bridge_filter.h" #include "c-ctype.h" #include "event.h" #include "buf.h" @@ -2176,6 +2177,21 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name); + if (driver->macFilter) { + int i; + virDomainDefPtr def = vm->def; + for (i = 0 ; i < def->nnets ; i++) { + virDomainNetDefPtr net = def->nets[i]; + if (net->ifname == NULL) + continue; + if ((errno = networkDisallowMacOnPort(conn, driver, net->ifname, net->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule to allow MAC address on '%s'"), + net->ifname); + } + } + } + if (virKillProcess(vm->pid, 0) == 0 && virKillProcess(vm->pid, SIGTERM) < 0) virReportSystemError(conn, errno,
On Tue, Oct 27, 2009 at 12:36:14PM +0100, Gerhard Stenzel wrote:
This patch adds MAC address based port filtering to the qemu driver.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> ---
po/POTFILES.in | 1 src/Makefile.am | 4 +- src/qemu/qemu.conf | 2 + src/qemu/qemu_bridge_filter.c | 108 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_bridge_filter.h | 39 +++++++++++++++ src/qemu/qemu_conf.c | 25 +++++++++ src/qemu/qemu_conf.h | 4 ++ src/qemu/qemu_driver.c | 16 ++++++ 8 files changed, 198 insertions(+), 1 deletions(-) create mode 100644 src/qemu/qemu_bridge_filter.c create mode 100644 src/qemu/qemu_bridge_filter.h
ACK, this looks good too. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Oct 27, 2009 at 12:36:14PM +0100, Gerhard Stenzel wrote:
This patch adds MAC address based port filtering to the qemu driver.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> [...] +networkAddEbtablesRules(struct qemud_driver *driver) { + int err; + + /* Set forward policy to DROP */ + if ((err = ebtablesAddForwardPolicyReject(driver->ebtables))) { + virReportSystemError(NULL, err, + _("failed to add ebtables rule to set default policy to drop on '%s'"), + __FILE__); + return err;
I changed the indentation a bit to fit into 80 columns [...]
@@ -318,6 +319,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } }
+ p = virConfGetValue (conf, "mac_filter"); + CHECK_TYPE ("mac_filter", VIR_CONF_LONG); + if (p) { + driver->macFilter = p->l; + if (!(driver->ebtables = ebtablesContextNew("qemu"))) { + driver->macFilter = 0; + virReportSystemError(NULL, errno, + _("failed to enable mac filter in in '%s'"), __FILE__);
same in a couple of place in that module too
@@ -2176,6 +2177,21 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name);
+ if (driver->macFilter) { + int i; + virDomainDefPtr def = vm->def; + for (i = 0 ; i < def->nnets ; i++) { + virDomainNetDefPtr net = def->nets[i]; + if (net->ifname == NULL) + continue; + if ((errno = networkDisallowMacOnPort(conn, driver, net->ifname, net->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule to allow MAC address on '%s'"), + net->ifname); + } + } + } + if (virKillProcess(vm->pid, 0) == 0 && virKillProcess(vm->pid, SIGTERM) < 0) virReportSystemError(conn, errno,
again a bit of formatting btut nothing to add otherwise. I have applied this patch too and I pushed both, thanks ! We just need to look at the spec file dependancy (should be added I think) and clear up potential issues in saving :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Nov 03, 2009 at 11:50:52PM +0100, Daniel Veillard wrote:
On Tue, Oct 27, 2009 at 12:36:14PM +0100, Gerhard Stenzel wrote:
This patch adds MAC address based port filtering to the qemu driver.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> [...] +networkAddEbtablesRules(struct qemud_driver *driver) { + int err; + + /* Set forward policy to DROP */ + if ((err = ebtablesAddForwardPolicyReject(driver->ebtables))) { + virReportSystemError(NULL, err, + _("failed to add ebtables rule to set default policy to drop on '%s'"), + __FILE__); + return err;
I changed the indentation a bit to fit into 80 columns [...]
@@ -318,6 +319,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } }
+ p = virConfGetValue (conf, "mac_filter"); + CHECK_TYPE ("mac_filter", VIR_CONF_LONG); + if (p) { + driver->macFilter = p->l; + if (!(driver->ebtables = ebtablesContextNew("qemu"))) { + driver->macFilter = 0; + virReportSystemError(NULL, errno, + _("failed to enable mac filter in in '%s'"), __FILE__);
same in a couple of place in that module too
@@ -2176,6 +2177,21 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name);
+ if (driver->macFilter) { + int i; + virDomainDefPtr def = vm->def; + for (i = 0 ; i < def->nnets ; i++) { + virDomainNetDefPtr net = def->nets[i]; + if (net->ifname == NULL) + continue; + if ((errno = networkDisallowMacOnPort(conn, driver, net->ifname, net->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule to allow MAC address on '%s'"), + net->ifname); + } + } + } + if (virKillProcess(vm->pid, 0) == 0 && virKillProcess(vm->pid, SIGTERM) < 0) virReportSystemError(conn, errno,
again a bit of formatting btut nothing to add otherwise.
I have applied this patch too and I pushed both, thanks !
We just need to look at the spec file dependancy (should be added I think) and clear up potential issues in saving :-)
Mark pointed out to me offlist, that this filtering is a little too restrictive because it also blocks multicast + broadcast packets. We can fix that easily enough with an extra patch though, and a single catch-all rule for multi/broad-cast packets. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, 2009-11-04 at 12:55 +0000, Daniel P. Berrange wrote:
On Tue, Nov 03, 2009 at 11:50:52PM +0100, Daniel Veillard wrote: ... Mark pointed out to me offlist, that this filtering is a little too restrictive because it also blocks multicast + broadcast packets. We can fix that easily enough with an extra patch though, and a single catch-all rule for multi/broad-cast packets.
Daniel something like the following?
ebtables -A libvirt_qemu_FORWARD -p ARP -j ACCEPT any other multi/broad-cast packets you/Mark had in mind? -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, 2009-11-04 at 12:55 +0000, Daniel P. Berrange wrote: ...
Mark pointed out to me offlist, that this filtering is a little too restrictive because it also blocks multicast + broadcast packets. We can fix that easily enough with an extra patch though, and a single catch-all rule for multi/broad-cast packets.
Daniel
Hi, I have revisited this subject and was trying to find a scenario, where multi/broad-cast packets would be affected by this patch and failed so far. Since only the source mac address of a guest is filtered, I don't see how a multicast or broadcast destination mac address could be a problem. What am I missing? -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, 2009-11-18 at 17:10 +0100, Gerhard Stenzel wrote:
On Wed, 2009-11-04 at 12:55 +0000, Daniel P. Berrange wrote: ...
Mark pointed out to me offlist, that this filtering is a little too restrictive because it also blocks multicast + broadcast packets. We can fix that easily enough with an extra patch though, and a single catch-all rule for multi/broad-cast packets.
Daniel
Hi, I have revisited this subject and was trying to find a scenario, where multi/broad-cast packets would be affected by this patch and failed so far. Since only the source mac address of a guest is filtered, I don't see how a multicast or broadcast destination mac address could be a problem.
What am I missing?
Ah, that makes sense, thanks. I hadn't looked closely enough to notice that you're only doing source mac address filtering. Cheers, Mark.
On Wed, Nov 18, 2009 at 05:10:38PM +0100, Gerhard Stenzel wrote:
On Wed, 2009-11-04 at 12:55 +0000, Daniel P. Berrange wrote: ...
Mark pointed out to me offlist, that this filtering is a little too restrictive because it also blocks multicast + broadcast packets. We can fix that easily enough with an extra patch though, and a single catch-all rule for multi/broad-cast packets.
Daniel
Hi, I have revisited this subject and was trying to find a scenario, where multi/broad-cast packets would be affected by this patch and failed so far. Since only the source mac address of a guest is filtered, I don't see how a multicast or broadcast destination mac address could be a problem.
That is sufficient, I mis-read how the rules were being added. That said I believe this is an issue in here with guests with a NIC configured with type=network instead of type=bridge. with the former, no traffic seems to go over the FORWARD chain - only the INPUT chain, so our rules are not matched. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
This patch removes the port filter if the network device is detached via virDomainDetachDevice. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -4829,6 +4829,7 @@ qemudDomainDetachNetDevice(virConnectPtr { int i, ret = -1; virDomainNetDefPtr detach = NULL; + struct qemud_driver *driver = qemu_driver; for (i = 0 ; i < vm->def->nnets ; i++) { virDomainNetDefPtr net = vm->def->nets[i]; @@ -4863,6 +4864,15 @@ qemudDomainDetachNetDevice(virConnectPtr if (qemuMonitorRemoveHostNetwork(vm, detach->vlan, detach->hostnet_name) < 0) goto cleanup; + if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(conn, driver, detach->ifname, + detach->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + if (vm->def->nnets > 1) { memmove(vm->def->nets + i, vm->def->nets + i + 1, -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Tue, 2009-11-10 at 13:28 +0100, Gerhard Stenzel wrote:
This patch removes the port filter if the network device is detached via virDomainDetachDevice.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -4829,6 +4829,7 @@ qemudDomainDetachNetDevice(virConnectPtr { int i, ret = -1; virDomainNetDefPtr detach = NULL; + struct qemud_driver *driver = qemu_driver;
for (i = 0 ; i < vm->def->nnets ; i++) { virDomainNetDefPtr net = vm->def->nets[i]; @@ -4863,6 +4864,15 @@ qemudDomainDetachNetDevice(virConnectPtr if (qemuMonitorRemoveHostNetwork(vm, detach->vlan, detach->hostnet_name) < 0) goto cleanup;
+ if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(conn, driver, detach->ifname, + detach->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + if (vm->def->nnets > 1) { memmove(vm->def->nets + i, vm->def->nets + i + 1,
This was probably overlooked. Resending for 0.7.5. This patch removes the port filter if the network device is detached via virDomainDetachDevice Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -5284,6 +5284,17 @@ qemudDomainDetachNetDevice(virConnectPtr } qemuDomainObjExitMonitorWithDriver(driver, vm); + if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(conn, + driver, + detach->ifname, + detach->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + if (vm->def->nnets > 1) { memmove(vm->def->nets + i, vm->def->nets + i + 1, -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Tue, Nov 24, 2009 at 10:38:48AM +0100, Gerhard Stenzel wrote:
On Tue, 2009-11-10 at 13:28 +0100, Gerhard Stenzel wrote:
This patch removes the port filter if the network device is detached via virDomainDetachDevice.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -4829,6 +4829,7 @@ qemudDomainDetachNetDevice(virConnectPtr { int i, ret = -1; virDomainNetDefPtr detach = NULL; + struct qemud_driver *driver = qemu_driver;
for (i = 0 ; i < vm->def->nnets ; i++) { virDomainNetDefPtr net = vm->def->nets[i]; @@ -4863,6 +4864,15 @@ qemudDomainDetachNetDevice(virConnectPtr if (qemuMonitorRemoveHostNetwork(vm, detach->vlan, detach->hostnet_name) < 0) goto cleanup;
+ if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(conn, driver, detach->ifname, + detach->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + if (vm->def->nnets > 1) { memmove(vm->def->nets + i, vm->def->nets + i + 1,
This was probably overlooked. Resending for 0.7.5.
This patch removes the port filter if the network device is detached via virDomainDetachDevice
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -5284,6 +5284,17 @@ qemudDomainDetachNetDevice(virConnectPtr } qemuDomainObjExitMonitorWithDriver(driver, vm);
+ if ((driver->macFilter) && (detach->ifname != NULL)) { + if ((errno = networkDisallowMacOnPort(conn, + driver, + detach->ifname, + detach->mac))) { + virReportSystemError(conn, errno, + _("failed to remove ebtables rule on '%s'"), + detach->ifname); + } + } + if (vm->def->nnets > 1) { memmove(vm->def->nets + i, vm->def->nets + i + 1,
Oops, I though I had pushed it ... it's in now, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Gerhard Stenzel -
Mark McLoughlin