From 72fb7256d72eb38dfb1230f5a5eedc15a7de05de Mon Sep 17 00:00:00 2001 From: Naredula Janardhana Reddy Date: Wed, 1 Feb 2012 12:39:10 +0530 Subject: [PATCH] Bug 13297,13375,12705 : Summary of changes : - Added a new flag -s to ipassoc command to carry if the ip address is used for SNAT or not. - SNAT is completly decoupled from the first flag. first flag is used to decide if the ip address is first ip address of the interface. - -s and -f are independent, SNAT can be enabled on the non-first ip also. --- .../VirtualRoutingResource.java | 6 +- .../vmware/resource/VmwareResource.java | 7 +- .../xen/resource/CitrixResourceBase.java | 7 +- .../config/etc/iptables/iptables-router | 2 - .../systemvm/debian/config/root/ipassoc.sh | 69 ++++++++++++++----- .../VirtualNetworkApplianceManagerImpl.java | 5 +- 6 files changed, 60 insertions(+), 36 deletions(-) diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java index 66c333648a5..846af3c473c 100755 --- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java @@ -624,9 +624,9 @@ public class VirtualRoutingResource implements Manager { } String cidrSize = Long.toString(NetUtils.getCidrSize(vlanNetmask)); if (sourceNat) { - command.add("-f"); - command.add("-l", publicIpAddress + "/" + cidrSize); - } else if (firstIP) { + command.add("-s"); + } + if (firstIP) { command.add( "-f"); command.add( "-l", publicIpAddress + "/" + cidrSize); } else { diff --git a/core/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/core/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 427478ba98d..aa1b8c0f4b0 100755 --- a/core/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/core/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -741,10 +741,9 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } String cidrSize = Long.toString(NetUtils.getCidrSize(vlanNetmask)); if (sourceNat) { - args += " -f "; - args += " -l "; - args += publicIpAddress + "/" + cidrSize; - } else if (firstIP) { + args += " -s "; + } + if (firstIP) { args += " -f "; args += " -l "; args += publicIpAddress + "/" + cidrSize; diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 520120f5c6a..8acb75b5fa2 100755 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -1625,10 +1625,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } String cidrSize = Long.toString(NetUtils.getCidrSize(vlanNetmask)); if (sourceNat) { - args += " -f"; - args += " -l "; - args += publicIpAddress + "/" + cidrSize; - } else if (firstIP) { + args += " -s"; + } + if (firstIP) { args += " -f"; args += " -l "; args += publicIpAddress + "/" + cidrSize; diff --git a/patches/systemvm/debian/config/etc/iptables/iptables-router b/patches/systemvm/debian/config/etc/iptables/iptables-router index ed0688c6e4f..5ac162a7b85 100644 --- a/patches/systemvm/debian/config/etc/iptables/iptables-router +++ b/patches/systemvm/debian/config/etc/iptables/iptables-router @@ -30,7 +30,5 @@ COMMIT :OUTPUT ACCEPT [0:0] :POSTROUTING ACCEPT [0:0] -A PREROUTING -m state --state ESTABLISHED,RELATED -j CONNMARK --restore-mark -COMMIT -*mangle -A POSTROUTING -p udp --dport bootpc -j CHECKSUM --checksum-fill COMMIT diff --git a/patches/systemvm/debian/config/root/ipassoc.sh b/patches/systemvm/debian/config/root/ipassoc.sh index 8617a9cb0fd..c5bcbf5211e 100644 --- a/patches/systemvm/debian/config/root/ipassoc.sh +++ b/patches/systemvm/debian/config/root/ipassoc.sh @@ -204,10 +204,33 @@ add_routing() { fi return 0; } +add_snat() { + if [ "$sflag" == "0" ] + then + return 0; + fi -add_nat_entry() { local pubIp=$1 - logger -t cloud "$(basename $0):Adding nat entry for ip $pubIp on interface $ethDev" + local ipNoMask=$(echo $1 | awk -F'/' '{print $1}') + logger -t cloud "$(basename $0):Added SourceNAT $pubIp on interface $ethDev" + sudo iptables -t nat -D POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask ; + sudo iptables -t nat -I POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask ; + return $? +} +remove_snat() { + if [ "$sflag" == "0" ] + then + return 0; + fi + + local pubIp=$1 + logger -t cloud "$(basename $0):Removing SourceNAT $pubIp on interface $ethDev" + sudo iptables -t nat -D POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask; + return $? +} +add_first_ip() { + local pubIp=$1 + logger -t cloud "$(basename $0):Adding first ip $pubIp on interface $ethDev" local ipNoMask=$(echo $1 | awk -F'/' '{print $1}') local mask=$(echo $1 | awk -F'/' '{print $2}') sudo ip link show $ethDev | grep "state DOWN" > /dev/null @@ -220,18 +243,20 @@ add_nat_entry() { # remove if duplicat ip with 32 mask, this happens when we are promting the ip to primary sudo ip addr del dev $ethDev $ipNoMask/32 > /dev/null fi + sudo iptables -D FORWARD -i $ethDev -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT sudo iptables -D FORWARD -i eth0 -o $ethDev -j ACCEPT - sudo iptables -t nat -D POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask ; sudo iptables -A FORWARD -i $ethDev -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT sudo iptables -A FORWARD -i eth0 -o $ethDev -j ACCEPT - sudo iptables -t nat -I POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask ; + + add_snat $1 if [ $? -gt 0 -a $? -ne 2 ] then - logger -t cloud "$(basename $0):Failed adding nat entry for ip $pubIp on interface $ethDev" + logger -t cloud "$(basename $0):Failed adding source nat entry for ip $pubIp on interface $ethDev" return 1 fi - logger -t cloud "$(basename $0):Added nat entry for ip $pubIp on interface $ethDev" + + logger -t cloud "$(basename $0):Added first ip $pubIp on interface $ethDev" if [ $if_keep_state -ne 1 -o $old_state -ne 0 ] then sudo ip link set $ethDev up @@ -242,22 +267,24 @@ add_nat_entry() { return 0 } -del_nat_entry() { +remove_first_ip() { local pubIp=$1 - logger -t cloud "$(basename $0):Deleting nat entry for ip $pubIp on interface $ethDev" + logger -t cloud "$(basename $0):Removing first ip $pubIp on interface $ethDev" local ipNoMask=$(echo $1 | awk -F'/' '{print $1}') local mask=$(echo $1 | awk -F'/' '{print $2}') [ "$mask" == "" ] && mask="32" + sudo iptables -D FORWARD -i $ethDev -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT sudo iptables -D FORWARD -i eth0 -o $ethDev -j ACCEPT - sudo iptables -t nat -D POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask; + remove_snat $1 + sudo ip addr del dev $ethDev "$ipNoMask/$mask" - - remove_routing $1 if [ $? -gt 0 -a $? -ne 2 ] then + remove_routing $1 return 1 fi + remove_routing $1 return $? } @@ -271,7 +298,7 @@ add_an_ip () { local old_state=$? sudo ip addr add dev $ethDev $pubIp ; - + add_snat $1 if [ $if_keep_state -ne 1 -o $old_state -ne 0 ] then sudo ip link set $ethDev up @@ -289,12 +316,14 @@ remove_an_ip () { local mask=$(echo $1 | awk -F'/' '{print $2}') local existingIpMask=$(sudo ip addr show dev $ethDev | grep inet | awk '{print $2}' | grep -w $ipNoMask) [ "$existingIpMask" == "" ] && return 0 + remove_snat $1 local existingMask=$(echo $existingIpMask | awk -F'/' '{print $2}') if [ "$existingMask" == "32" ] then sudo ip addr del dev $ethDev $existingIpMask result=$? fi + if [ "$existingMask" != "32" ] then replaceIpMask=`sudo ip addr show dev $ethDev | grep inet | grep -v $existingIpMask | awk '{print $2}' | sort -t/ -k2 -n|tail -1` @@ -303,21 +332,21 @@ remove_an_ip () { sudo ip addr del dev $ethDev $replaceIpMask; replaceIp=`echo $replaceIpMask | awk -F/ '{print $1}'`; sudo ip addr add dev $ethDev $replaceIp/$existingMask; - sudo iptables -t nat -D POSTROUTING -j SNAT -o $ethDev --to-source $ipNoMask ; - sudo iptables -t nat -A POSTROUTING -j SNAT -o $ethDev --to-source $replaceIp ; fi result=$? fi - remove_routing $1 + if [ $result -gt 0 -a $result -ne 2 ] then + remove_routing $1 return 1 fi + remove_routing $1 return 0 } #set -x - +sflag=0 lflag= fflag= cflag= @@ -341,7 +370,7 @@ then if_keep_state=1 fi -while getopts 'fADa:l:c:g:' OPTION +while getopts 'sfADa:l:c:g:' OPTION do case $OPTION in A) Aflag=1 @@ -352,6 +381,8 @@ do ;; f) fflag=1 ;; + s) sflag=1 + ;; l) lflag=1 publicIp="$OPTARG" ;; @@ -383,7 +414,7 @@ fi if [ "$fflag" == "1" ] && [ "$Aflag" == "1" ] then - add_nat_entry $publicIp && + add_first_ip $publicIp && add_vpn_chain_for_ip $publicIp && add_fw_chain_for_ip $publicIp unlock_exit $? $lock $locked @@ -398,7 +429,7 @@ fi if [ "$fflag" == "1" ] && [ "$Dflag" == "1" ] then - del_nat_entry $publicIp && + remove_first_ip $publicIp && del_fw_chain_for_ip $publicIp && del_vpn_chain_for_ip $publicIp unlock_exit $? $lock $locked diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index a85dcdfb6eb..8e14ee29e1f 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -2502,10 +2502,7 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian IpAddressTO[] ipsToSend = new IpAddressTO[ipAddrList.size()]; int i = 0; boolean firstIP = true; - /* If the first IP is not source NAT, then don't treat it as first IP. It would happen if it's the first IP for public nic other than first one */ - if (!ipAddrList.get(0).isSourceNat()) { - firstIP = false; - } + for (final PublicIpAddress ipAddr : ipAddrList) { boolean add = (ipAddr.getState() == IpAddress.State.Releasing ? false : true);