On 12/11/2012 03:54 PM, Brian J. Murrell wrote:
Hi,
Per the request on
https://bugzilla.redhat.com/show_bug.cgi?id=657918
please find attached a patch that should address the issue.
Thanks!
I'm not subscribed to this list though (I know, it's pretty rude, but
my e-mail traffic is already too heavy to add another list to it), so
if you could either CC me on any follow-up or just move followups to
the BZ ticket where the patch also appears, that would be great.
Don't worry about it. List policy is to reply-to-all precisely so that
you do not have to subscribe while still following a thread you initiate
(and mail-followup-to exists for those who do subscribe but don't like
duplicates).
I'll comment on the minor coding issues, but hopefully someone more
familiar with actual filtering can chime in on the logical correctness.
Cheers,
b.
--- src/network/bridge_driver.c.orig 2012-10-27 16:56:23.000000000 -0400
+++ src/network/bridge_driver.c 2012-12-11 15:49:13.937133883 -0500
@@ -1301,9 +1301,10 @@
*
* We need to end up with 3 rules in the table in this order
Comment should now mention 4 rules.
*
- * 1. protocol=tcp with sport mapping restriction
- * 2. protocol=udp with sport mapping restriction
- * 3. generic any protocol
+ * 1. multicast is exempted
+ * 2. protocol=tcp with sport mapping restriction
+ * 3. protocol=udp with sport mapping restriction
+ * 4. generic any protocol
*
* The sport mappings are required, because default IPtables
* MASQUERADE maintain port numbers unchanged where possible.
@@ -1361,8 +1362,21 @@
goto masqerr5;
}
+ /* exempt multicast traffic */
+ if (iptablesAddForwardMasqueradeExempt(driver->iptables) < 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add iptables rule to exempt multicast
traffic from masquerading"));
Indentation is a bit off, and you need a "%s" argument to keep the
syntax-checker happy about a message with no other % operand.
+ goto masqerr6;
+ }
+
return 0;
+ masqerr6:
+ iptablesRemoveForwardMasquerade(driver->iptables,
+ &ipdef->address,
+ prefix,
+ forwardIf,
+ "tcp");
masqerr5:
iptablesRemoveForwardMasquerade(driver->iptables,
&ipdef->address,
--- src/util/iptables.c.orig 2012-10-27 16:56:23.000000000 -0400
+++ src/util/iptables.c 2012-12-11 15:53:28.715044866 -0500
@@ -858,6 +858,26 @@
}
/**
+ * iptablesAddForwardMasqueradeExempt:
+ * @ctx: pointer to the IP table context
+ *
+ * Add rules to the IP table context to exempt masquerading
+ * for multicast networks
+ *
+ * Returns 0 in case of success or an error code otherwise
+ */
+int
+iptablesAddForwardMasqueradeExempt(iptablesContext *ctx)
+{
+ return iptablesAddRemoveRule(ctx->nat_postrouting,
+ AF_INET,
+ ADD,
+ "--destination", "224.0.0.0/4",
+ "--jump", "RETURN",
+ NULL);
+}
Do we need an IPv6 counterpart? (Or am I just showing my ignorance of
what IPv6 does as a counterpart to IPv4 multicast?)
+
+/**
* iptablesAddForwardMasquerade:
* @ctx: pointer to the IP table context
* @network: the source network name
--- src/util/iptables.h.orig 2012-10-27 16:56:23.000000000 -0400
+++ src/util/iptables.h 2012-12-11 15:57:03.284144679 -0500
@@ -101,6 +101,7 @@
int family,
const char *iface);
+int iptablesAddForwardMasqueradeExempt (iptablesContext *ctx);
int iptablesAddForwardMasquerade (iptablesContext *ctx,
virSocketAddr *netaddr,
unsigned int prefix,
--- src/libvirt_private.syms.orig 2012-12-11 15:46:11.141932324 -0500
+++ src/libvirt_private.syms 2012-12-11 15:58:11.715865516 -0500
@@ -681,6 +681,7 @@
iptablesAddForwardAllowOut;
iptablesAddForwardAllowRelatedIn;
iptablesAddForwardMasquerade;
+iptablesAddForwardMasqueradeExempt;
iptablesAddForwardRejectIn;
iptablesAddForwardRejectOut;
iptablesAddOutputFixUdpChecksum;
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org