The functions - iptablesAddForwardDontMasquerade(), - iptablesRemoveForwardDontMasquerade handle exceptions in the masquerading implemented in the POSTROUTING chain of the "nat" table. Such exceptions should be added as chronologically latest, logically top-most rules. The bridge driver will call these functions beginning with the next patch: some special destination IP addresses always refer to the local subnetwork, even though they don't match any practical subnetwork's netmask. Packets from virbrN targeting such IP addresses are never routed outwards, but the current rules treat them as non-virbrN-destined packets and masquerade them. This causes problems for some receivers on virbrN. Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- src/util/viriptables.h | 8 +++++ src/util/viriptables.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 2 ++ 3 files changed, 98 insertions(+) diff --git a/src/util/viriptables.h b/src/util/viriptables.h index 447f4a8..aee1842 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -94,6 +94,14 @@ int iptablesRemoveForwardMasquerade (virSocketAddr *netaddr, virSocketAddrRangePtr addr, virPortRangePtr port, const char *protocol); +int iptablesAddForwardDontMasquerade(virSocketAddr *netaddr, + unsigned int prefix, + const char *physdev, + const char *destaddr); +int iptablesRemoveForwardDontMasquerade(virSocketAddr *netaddr, + unsigned int prefix, + const char *physdev, + const char *destaddr); int iptablesAddOutputFixUdpChecksum (const char *iface, int port); int iptablesRemoveOutputFixUdpChecksum (const char *iface, diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 0f57d66..349734c 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -832,6 +832,94 @@ iptablesRemoveForwardMasquerade(virSocketAddr *netaddr, } +/* Don't masquerade traffic coming from the network associated with the bridge + * if said traffic targets @destaddr. + */ +static int +iptablesForwardDontMasquerade(virSocketAddr *netaddr, + unsigned int prefix, + const char *physdev, + const char *destaddr, + int action) +{ + int ret = -1; + char *networkstr = NULL; + virCommandPtr cmd = NULL; + + if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) + return -1; + + if (!VIR_SOCKET_ADDR_IS_FAMILY(netaddr, AF_INET)) { + /* Higher level code *should* guaranteee it's impossible to get here. */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Attempted to NAT '%s'. NAT is only supported for IPv4."), + networkstr); + goto cleanup; + } + + cmd = iptablesCommandNew("nat", "POSTROUTING", AF_INET, action); + + if (physdev && physdev[0]) + virCommandAddArgList(cmd, "--out-interface", physdev, NULL); + + virCommandAddArgList(cmd, "--source", networkstr, + "--destination", destaddr, "--jump", "RETURN", NULL); + ret = virCommandRun(cmd, NULL); +cleanup: + virCommandFree(cmd); + VIR_FREE(networkstr); + return ret; +} + +/** + * iptablesAddForwardDontMasquerade: + * @netaddr: the source network name + * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr + * @physdev: the physical output device or NULL + * @destaddr: the destination network not to masquerade for + * + * Add rules to the IP table context to avoid masquerading from + * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format + * directly consumable by iptables, it must not depend on user input or + * configuration. + * + * Returns 0 in case of success or an error code otherwise. + */ +int +iptablesAddForwardDontMasquerade(virSocketAddr *netaddr, + unsigned int prefix, + const char *physdev, + const char *destaddr) +{ + return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr, + ADD); +} + +/** + * iptablesRemoveForwardDontMasquerade: + * @netaddr: the source network name + * @prefix: prefix (# of 1 bits) of netmask to apply to @netaddr + * @physdev: the physical output device or NULL + * @destaddr: the destination network not to masquerade for + * + * Remove rules from the IP table context that prevent masquerading from + * @netaddr/@prefix to @destaddr on @physdev. @destaddr must be in a format + * directly consumable by iptables, it must not depend on user input or + * configuration. + * + * Returns 0 in case of success or an error code otherwise. + */ +int +iptablesRemoveForwardDontMasquerade(virSocketAddr *netaddr, + unsigned int prefix, + const char *physdev, + const char *destaddr) +{ + return iptablesForwardDontMasquerade(netaddr, prefix, physdev, destaddr, + REMOVE); +} + + static int iptablesOutputFixUdpChecksum(const char *iface, int port, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 50e2f48..49505c9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1450,6 +1450,7 @@ iptablesAddForwardAllowCross; iptablesAddForwardAllowIn; iptablesAddForwardAllowOut; iptablesAddForwardAllowRelatedIn; +iptablesAddForwardDontMasquerade; iptablesAddForwardMasquerade; iptablesAddForwardRejectIn; iptablesAddForwardRejectOut; @@ -1460,6 +1461,7 @@ iptablesRemoveForwardAllowCross; iptablesRemoveForwardAllowIn; iptablesRemoveForwardAllowOut; iptablesRemoveForwardAllowRelatedIn; +iptablesRemoveForwardDontMasquerade; iptablesRemoveForwardMasquerade; iptablesRemoveForwardRejectIn; iptablesRemoveForwardRejectOut; -- 1.8.3.1

On 13-09-23 10:05 AM, Laszlo Ersek wrote: All multicast, not just the local subnet multicast needs to be exempt from masquerading. I would tend to guess that anyone trying to do "global Internet" multicast behind NAT is signing up for a world of trouble anyway. It's not like you could put a multicast source behind a NAT so it could really only apply to sinks. Since sinks operate by "subscribing" to a multicast source by way of their multicast router (which should be local and not through a NAT) I would think a NAT device that supports multicast to it's NAT clients should support it as if it were a multicast router and not so much a NAT and so there never really should be NATting of traffic from a sink. So AFACT there never really is a use-case for actually NATting multicast traffic, so just don't NAT the whole range, 224.0.0.0/4. An example use-case if you want for multicast is creating a corosync cluster. When that works, you have this patch right. :-) NACK. 224.0.0.0/4 is the entire multicast space. 224.0.0.0/24 is just one tiny reserved "control block" of addresses for routing protocols, etc. There's a lot more to multicast than just routing protocols. Cheers, b.

On 09/23/2013 10:05 AM, Laszlo Ersek wrote: 1) After briefly looking at the references in the BZ, I agree with you that it's safest to not yet bypass NAT on the entire multicast range. Let's make sure we fully understand it, then submit a separate patch for the larger range. 2) Along with 255.255.255.255/32, I think this patch can/should also add a "networkDirectedLocalBroadcast" (which will obviously need to be a local variable and recomputed each time). This can be computed by ORing the ip address of the network with ~netmask, then appending a 32 prefix. So for example, the directed broadcast for 192.168.122.1/24 would be 192.168.122.255/32. Beyond that it looks good, so ACK with those changes (unless someone else has anything to say).