Skip to content

Cloudstack 9339: Virtual Routers do not handle Multiple Public Interfaces#1519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
quiteclose wants to merge 1 commit into from
Closed

Cloudstack 9339: Virtual Routers do not handle Multiple Public Interfaces #1519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions systemvm/patches/debian/config/opt/cloud/bin/configure.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -871,6 +871,12 @@ def processStaticNatRule(self, rule):
device = self.getDeviceByIp(rule["public_ip"])
if device is None:
raise Exception("Ip address %s has no device in the ips databag" % rule["public_ip"])
self.fw.append(["mangle", "",
"-A PREROUTING -s %s/32 -m state --state NEW -j MARK --set-xmark 0x%s/0xffffffff" % \
(rule["internal_ip"], device[len("eth"):])])
self.fw.append(["mangle", "",
"-A PREROUTING -s %s/32 -m state --state NEW -j CONNMARK --save-mark --nfmask 0xffffffff --ctmask 0xffffffff" % \
rule["internal_ip"]])
self.fw.append(["nat", "front",
"-A PREROUTING -d %s/32 -j DNAT --to-destination %s" % (rule["public_ip"], rule["internal_ip"])])
self.fw.append(["nat", "front",
Expand Down
90 changes: 60 additions & 30 deletions systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,6 +28,8 @@
from CsRule import CsRule

VRRP_TYPES = ['guest']
VPC_PUBLIC_INTERFACE = ['eth1']
NETWORK_PUBLIC_INTERFACE = ['eth2']

class CsAddress(CsDataBag):

Expand DownExpand Up@@ -288,14 +290,16 @@ def post_configure(self, address):
""" The steps that must be done after a device is configured """
route = CsRoute()
if not self.get_type() in ["control"]:
route.add_table(self.dev)

CsRule(self.dev).addMark()
if self.dev != 'eth0':
route.add_table(self.dev)
CsRule(self.dev).addMark()
self.set_mark()

interfaces = [CsInterface(address, self.config)]
CsHelper.reconfigure_interfaces(self.cl, interfaces)

@quiteclosequitecloseMay 25, 2016

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 299 and 300 were added as part of c41edc1. I've not investigated what those lines do, I've just naively overcome the merge conflicts. I'll look at the results of CI and follow from there.

self.set_mark()
self.check_is_up()
self.arpPing()

CsRpsrfs(self.dev).enable()
Expand All@@ -305,23 +309,46 @@ def post_configure(self, address):
if not self.config.is_vpc():
self.setup_router_control()

if self.config.is_vpc() or self.cl.is_redundant():
# The code looks redundant here, but we actually have to cater for routers and
# VPC routers in a different manner. Please do not remove this block otherwise
# The VPC default route will be broken.
if self.get_type() in ["public"] and address["device"] == CsHelper.PUBLIC_INTERFACES[self.cl.get_type()]:
gateway = str(address["gateway"])
route.add_defaultroute(gateway)
try:
if str(address["gateway"]) == "None":
raise ValueError
except (KeyError, ValueError):
logging.debug("IP %s was not provided with a gateway." % self.ip())
else:
# once we start processing public ip's we need to verify there
# is a default route and add if needed
if(self.cl.get_gateway()):
route.add_defaultroute(self.cl.get_gateway())
if self.get_type() in ["public"]:
if self.config.is_vpc():
main_public_nic = VPC_PUBLIC_INTERFACE
else:
main_public_nic = NETWORK_PUBLIC_INTERFACE

if self.dev in main_public_nic:
logging.debug("IP %s has the gateway %s that should be in the main routing table." % \
(self.ip(), address["gateway"]))
route.add_defaultroute(address["gateway"])
else:
logging.debug("IP %s has the gateway %s that is not intended for the main routing table." % \
(self.ip(), address["gateway"]))


def check_is_up(self):
""" Ensure device is up """
cmd = "ip link show %s | grep 'state DOWN'" % self.getDevice()
for i in CsHelper.execute(cmd):
if " DOWN " in i:
cmd2 = "ip link set %s up" % self.getDevice()
# All interfaces should be up on non-redundant or master routers
if not self.cl.is_redundant() or self.cl.is_master():
CsHelper.execute(cmd2)
# only bring up non-public interfaces on backup redundant routers
elif not self.is_public():
CsHelper.execute(cmd2)


def set_mark(self):
cmd = "-A PREROUTING -i %s -m state --state NEW -j CONNMARK --set-xmark %s/0xffffffff" % \
(self.getDevice(), self.dnum)
self.fw.append(["mangle", "", cmd])
if self.get_type() in ['public']:
cmd = "-A PREROUTING -i %s -m state --state NEW -j CONNMARK --set-xmark %s/0xffffffff" % \
(self.getDevice(), self.dnum)
self.fw.append(["mangle", "", cmd])

def get_type(self):
""" Return the type of the IP
Expand DownExpand Up@@ -357,9 +384,13 @@ def setup_router_control(self):
def fw_router(self):
if self.config.is_vpc():
return
self.fw.append(["mangle", "front", "-A PREROUTING " +

restore_mark = ["mangle", "front", "-A PREROUTING " +
"-m state --state RELATED,ESTABLISHED " +
"-j CONNMARK --restore-mark --nfmask 0xffffffff --ctmask 0xffffffff"])
"-j CONNMARK --restore-mark --nfmask 0xffffffff --ctmask 0xffffffff"]

if restore_mark not in self.fw:
self.fw.append(restore_mark)

if self.get_type() in ["public"]:
self.fw.append(["mangle", "front",
Expand DownExpand Up@@ -387,6 +418,10 @@ def fw_router(self):
"-j CONNMARK --set-xmark %s/0xffffffff" % self.dnum])
self.fw.append(

@ustcweizhouustcweizhouMay 1, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to see these lines (line 431-433) are removed.

["mangle", "", "-A FIREWALL_%s -j DROP" % self.address['public_ip']])
self.fw.append(
["filter", "", "-A FORWARD -i %s -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT" % self.dev])
self.fw.append(
["filter", "", "-A FORWARD -i eth0 -o %s -j FW_OUTBOUND" % self.dev])

self.fw.append(["filter", "", "-A INPUT -d 224.0.0.18/32 -j ACCEPT"])
self.fw.append(["filter", "", "-A INPUT -d 225.0.0.50/32 -j ACCEPT"])
Expand All@@ -410,23 +445,16 @@ def fw_router(self):
["filter", "", "-A FORWARD -i %s -o eth1 -m state --state RELATED,ESTABLISHED -j ACCEPT" % self.dev])
self.fw.append(
["filter", "", "-A FORWARD -i %s -o %s -m state --state NEW -j ACCEPT" % (self.dev, self.dev)])
self.fw.append(
["filter", "", "-A FORWARD -i eth2 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT"])
self.fw.append(
["filter", "", "-A FORWARD -i eth0 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT"])
self.fw.append(
["filter", "", "-A FORWARD -i eth0 -o eth2 -j FW_OUTBOUND"])
self.fw.append(["mangle", "",
"-A PREROUTING -i %s -m state --state NEW " % self.dev +
"-j CONNMARK --set-xmark %s/0xffffffff" % self.dnum])

self.fw.append(['', 'front', '-A FORWARD -j NETWORK_STATS'])
self.fw.append(['', 'front', '-A INPUT -j NETWORK_STATS'])
self.fw.append(['', 'front', '-A OUTPUT -j NETWORK_STATS'])
self.fw.append(['', '', '-A NETWORK_STATS -i eth0 -o eth2'])
self.fw.append(['', '', '-A NETWORK_STATS -i eth2 -o eth0'])
self.fw.append(['', '', '-A NETWORK_STATS -o eth2 ! -i eth0 -p tcp'])
self.fw.append(['', '', '-A NETWORK_STATS -i eth2 ! -o eth0 -p tcp'])
self.fw.append(['', '', '-A NETWORK_STATS -i eth0 -o %s' % self.dev])
self.fw.append(['', '', '-A NETWORK_STATS -i %s -o eth0' % self.dev])
self.fw.append(['', '', '-A NETWORK_STATS -o %s ! -i eth0 -p tcp' % self.dev])
self.fw.append(['', '', '-A NETWORK_STATS -i %s ! -o eth0 -p tcp' % self.dev])

def fw_vpcrouter(self):
if not self.config.is_vpc():
Expand DownExpand Up@@ -507,6 +535,8 @@ def post_config_change(self, method):
route = CsRoute()
if method == "add":
route.add_table(self.dev)
if "gateway" in self.address and self.address["gateway"] != "None":
route.add_route(self.dev, "default via %s" % self.address["gateway"])
route.add_route(self.dev, str(self.address["network"]))
elif method == "delete":
logging.warn("delete route not implemented")
Expand Down
90 changes: 67 additions & 23 deletions systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,7 +36,7 @@
from CsFile import CsFile
from CsProcess import CsProcess
from CsApp import CsPasswdSvc
from CsAddress import CsDevice
from CsAddress import CsDevice, VPC_PUBLIC_INTERFACE, NETWORK_PUBLIC_INTERFACE
from CsRoute import CsRoute
from CsStaticRoutes import CsStaticRoutes
import socket
Expand DownExpand Up@@ -299,28 +299,7 @@ def set_master(self):
self.set_lock()
logging.debug("Setting router to master")

dev = ''
interfaces = [interface for interface in self.address.get_interfaces() if interface.is_public()]
route = CsRoute()
for interface in interfaces:
if dev == interface.get_device():
continue
dev = interface.get_device()
logging.info("Will proceed configuring device ==> %s" % dev)
cmd = "ip link set %s up" % dev
if CsDevice(dev, self.config).waitfordevice():
CsHelper.execute(cmd)
logging.info("Bringing public interface %s up" % dev)

try:
gateway = interface.get_gateway()
logging.info("Adding gateway ==> %s to device ==> %s" % (gateway, dev))
if dev == CsHelper.PUBLIC_INTERFACES[self.cl.get_type()]:
route.add_defaultroute(gateway)
except:
logging.error("ERROR getting gateway from device %s" % dev)
else:
logging.error("Device %s was not ready could not bring it up" % dev)
self._bring_public_interfaces_up()

logging.debug("Configuring static routes")
static_routes = CsStaticRoutes("staticroutes", self.config)
Expand All@@ -346,6 +325,71 @@ def set_master(self):
CsHelper.reconfigure_interfaces(self.cl, interfaces)
logging.info("Router switched to master mode")


def _bring_public_interfaces_up(self):
'''Brings up all public interfaces and adds routes to the
relevant routing tables.
'''

up = [] # devices we've already brought up
routes = [] # routes to be added

is_link_up = "ip link show %s | grep 'state UP'"
set_link_up = "ip link set %s up"
add_route = "ip route add %s"
arping = "arping -c 1 -U %s -I %s"

if self.config.is_vpc():
default_gateway = VPC_PUBLIC_INTERFACE
else:
default_gateway = NETWORK_PUBLIC_INTERFACE

public_ips = [ip for ip in self.address.get_ips() if ip.is_public()]

for ip in public_ips:
address = ip.get_ip()
device = ip.get_device()
gateway = ip.get_gateway()

logging.debug("Configuring device %s for IP %s" % (device, address))

if device in up:
logging.debug("Device %s already configured. Skipping..." % device)
continue

if not CsDevice(device, self.config).waitfordevice():
logging.error("Device %s was not ready could not bring it up." % device)
continue

if CsHelper.execute(is_link_up % device):
logging.warn("Device %s was found already up. Assuming routes need configuring.")
up.append(device)
else:
logging.info("Bringing public interface %s up" % device)
CsHelper.execute(set_link_up % device)

logging.debug("Collecting routes for interface %s" % device)
routes.append("default via %s dev %s table Table_%s" % (gateway, device, device))

if device in default_gateway:
logging.debug("Determined that the gateway for %s should be in the main routing table." % device)
routes.insert(0, "default via %s dev %s" % (gateway, device))

up.append(device)

logging.info("Adding all collected routes.")
for route in routes:
CsHelper.execute(add_route % route)

logging.info("Sending gratuitous ARP for each Public IP...")
for ip in public_ips:
address = ip.get_ip()
device = ip.get_device()
CsHelper.execute(arping % (address, device))




def _collect_ignore_ips(self):
"""
This returns a list of ip objects that should be ignored
Expand Down