Skip to content

Commit 1ff6cd5

Browse files
authored
Use a single router process in attachment points (#381)
Use a single router process in attachment points, vs previously distributing the interfaces over many routers with max 12 interfaces each. This was a workaround for some issue in an old version of the router. Now, this is no longer necessary and running many instances is harmful for the system's performance. Remove the automatic splitting of interfaces over many routers. Once this change is deployed, the script `merge_ap_routers` should be run to merge and clean up the left router instances. One notable caveat: the router, as it is implemented now, uses a blocking write, which can in this configuration potentially slow down processing of the downstream traffic to the slowest User AS link speed. This should be fixed in the router implementation, hopefully soon.
1 parent 5e93c71 commit 1ff6cd5

File tree

6 files changed

+56
-119
lines changed

6 files changed

+56
-119
lines changed

scionlab/fixtures/testdata.yaml

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4219,7 +4219,7 @@
42194219
AS: 4
42204220
interface_id: 2
42214221
host: 4
4222-
border_router: 17
4222+
border_router: 4
42234223
public_ip: 10.0.8.1
42244224
public_port: 50001
42254225
bind_ip: null
@@ -4239,7 +4239,7 @@
42394239
AS: 7
42404240
interface_id: 3
42414241
host: 7
4242-
border_router: 19
4242+
border_router: 7
42434243
public_ip: null
42444244
public_port: 50002
42454245
bind_ip: null
@@ -4259,7 +4259,7 @@
42594259
AS: 13
42604260
interface_id: 3
42614261
host: 13
4262-
border_router: 21
4262+
border_router: 13
42634263
public_ip: null
42644264
public_port: 50002
42654265
bind_ip: null
@@ -4279,7 +4279,7 @@
42794279
AS: 14
42804280
interface_id: 3
42814281
host: 14
4282-
border_router: 23
4282+
border_router: 14
42834283
public_ip: 10.8.0.1
42844284
public_port: 50002
42854285
bind_ip: null
@@ -4299,7 +4299,7 @@
42994299
AS: 4
43004300
interface_id: 3
43014301
host: 4
4302-
border_router: 17
4302+
border_router: 4
43034303
public_ip: null
43044304
public_port: 50002
43054305
bind_ip: null
@@ -4600,41 +4600,21 @@
46004600
fields:
46014601
AS: 16
46024602
host: 16
4603-
- model: scionlab.borderrouter
4604-
pk: 17
4605-
fields:
4606-
AS: 4
4607-
host: 4
46084603
- model: scionlab.borderrouter
46094604
pk: 18
46104605
fields:
46114606
AS: 17
46124607
host: 17
4613-
- model: scionlab.borderrouter
4614-
pk: 19
4615-
fields:
4616-
AS: 7
4617-
host: 7
46184608
- model: scionlab.borderrouter
46194609
pk: 20
46204610
fields:
46214611
AS: 18
46224612
host: 18
4623-
- model: scionlab.borderrouter
4624-
pk: 21
4625-
fields:
4626-
AS: 13
4627-
host: 13
46284613
- model: scionlab.borderrouter
46294614
pk: 22
46304615
fields:
46314616
AS: 19
46324617
host: 19
4633-
- model: scionlab.borderrouter
4634-
pk: 23
4635-
fields:
4636-
AS: 14
4637-
host: 14
46384618
- model: scionlab.borderrouter
46394619
pk: 24
46404620
fields:

scionlab/models/user_as.py

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,6 @@ def update_attachments(self,
211211
self._create_or_update_attachment(att_conf)
212212
for deleted_link in deleted_links:
213213
self._delete_attachment(deleted_link)
214-
for ap in aps_set:
215-
ap.split_border_routers()
216214
self._deactivate_unused_vpn_clients(att_confs)
217215

218216
self.save()
@@ -482,8 +480,6 @@ def get_border_router_for_useras_interface(self) -> BorderRouter:
482480
"""
483481
Selects the preferred border router on which the Interfaces to UserASes should be configured
484482
485-
Note: the border router effectively used will be be overwritten by `split_border_routers`.
486-
487483
:returns: a `BorderRouter` of the related `AS`
488484
"""
489485
host = self._get_host_for_useras_attachment()
@@ -515,51 +511,6 @@ def _get_host_for_useras_attachment(self) -> Host:
515511
host = self.AS.hosts.filter(public_ip__isnull=False)[0]
516512
return host
517513

518-
def split_border_routers(self, max_ifaces=10):
519-
"""
520-
This is a workaround for an (apparent) issue with the border router, that cannot handle more
521-
than ~12 interfaces per process; this problem seemed to be fixed but is apparently still
522-
here. This is a hopefully temporary patch.
523-
524-
Will create / remove border routers so no one of them has more than
525-
the specified limit of interfaces. The links to parent ASes will
526-
always remain in a different border router.
527-
:param int max_ifaces The maximum number of interfaces per BR
528-
"""
529-
host = self._get_host_for_useras_attachment()
530-
# find the *active* interfaces attaching for UserASes (attaching_ifaces) and the rest
531-
# (infra_ifaces)
532-
ifaces = host.interfaces.active().order_by('interface_id')
533-
attaching_ifaces = ifaces.filter(
534-
link_as_interfaceA__type=Link.PROVIDER,
535-
link_as_interfaceA__interfaceB__AS__owner__isnull=False)
536-
infra_ifaces = ifaces.exclude(pk__in=attaching_ifaces)
537-
538-
# attaching non children all to one BR:
539-
infra_br = BorderRouter.objects.first_or_create(host)
540-
brs_to_delete = list(
541-
host.border_routers.order_by('pk').exclude(pk=infra_br.pk).values_list('pk', flat=True))
542-
brs_to_delete.reverse()
543-
infra_ifaces.update(border_router=infra_br)
544-
# attaching children to several BRs:
545-
attaching_ifaces = attaching_ifaces.all()
546-
for i in range(0, len(attaching_ifaces), max_ifaces):
547-
if brs_to_delete:
548-
br = BorderRouter.objects.get(pk=brs_to_delete.pop())
549-
else:
550-
br = BorderRouter.objects.create(host=host)
551-
for j in range(i, min(len(attaching_ifaces), i + max_ifaces)):
552-
iface = attaching_ifaces[j]
553-
iface.border_router = br
554-
iface.save()
555-
br.save()
556-
# squirrel away the *inactive* interfaces somewhere...
557-
host.interfaces.inactive().update(border_router=infra_br)
558-
559-
# delete old BRs
560-
if brs_to_delete:
561-
BorderRouter.objects.filter(pk__in=brs_to_delete).delete()
562-
563514
@staticmethod
564515
def from_link(link: Link) -> 'AttachmentPoint':
565516
"""

scionlab/tests/data/test_config_tar/host_4.yml

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -139,17 +139,6 @@ etc/scion/br-1.toml: |
139139
[metrics]
140140
prometheus = "127.0.0.1:30401"
141141
142-
[log.console]
143-
level = "info"
144-
etc/scion/br-2.toml: |
145-
[general]
146-
config_dir = "/etc/scion"
147-
id = "br-2"
148-
reconnect_to_dispatcher = true
149-
150-
[metrics]
151-
prometheus = "127.0.0.1:30402"
152-
153142
[log.console]
154143
level = "info"
155144
etc/scion/certs/ISD17-B1-S1.trc: !!binary |
@@ -513,13 +502,7 @@ etc/scion/topology.json: |-
513502
"public": "192.0.2.17:50000",
514503
"remote": "192.0.2.12:50001"
515504
}
516-
}
517-
},
518-
"internal_addr": "127.0.0.1:30001"
519-
},
520-
"br-2": {
521-
"ctrl_addr": "127.0.0.1:30202",
522-
"interfaces": {
505+
},
523506
"2": {
524507
"bandwidth": 1000,
525508
"isd_as": "17-ffaa:1:1",
@@ -541,7 +524,7 @@ etc/scion/topology.json: |-
541524
}
542525
}
543526
},
544-
"internal_addr": "127.0.0.1:30002"
527+
"internal_addr": "127.0.0.1:30001"
545528
}
546529
},
547530
"control_service": {
@@ -564,7 +547,6 @@ scionlab-config.json: |-
564547
"etc/openvpn/server.conf": "4da81a8209247858c583fcda94c7b617381d64ab",
565548
"etc/scion/beacon_policy.yaml": "90c69ea879ca52546774c3007e7d29104766fc07",
566549
"etc/scion/br-1.toml": "93b34c926178d1ea7c422d7d9fa2d8d7dd770d3b",
567-
"etc/scion/br-2.toml": "3e4c3c9c65f706f6792830ce68e3ae0d6b394a1f",
568550
"etc/scion/certs/ISD17-B1-S1.trc": "6e20b26b9831223ff8557b88cf7e2a3a441619b0",
569551
"etc/scion/certs/ISD19-B1-S1.trc": "b91dfb74d7f9d24d7cd81b53fdc2ffd820b1f46c",
570552
"etc/scion/certs/ISD20-B1-S1.trc": "66a6da3efd14c19c51c88dacd3de1534caa9b92c",
@@ -573,13 +555,12 @@ scionlab-config.json: |-
573555
"etc/scion/cs-1.toml": "84467cd10682d975caeccba3a7eaaec1bd9ea858",
574556
"etc/scion/keys/master0.key": "73184546f40ab4bc57870965e4a5896105a8ca2a",
575557
"etc/scion/keys/master1.key": "73184546f40ab4bc57870965e4a5896105a8ca2a",
576-
"etc/scion/topology.json": "919e85f60aa61648a1d45d792fda85581c0537f4"
558+
"etc/scion/topology.json": "6febb6e9910b8291d46bbc2767ae6f75ee820457"
577559
},
578560
"host_id": "ab0189ab83c940f88b0e16d51136e441",
579561
"host_secret": "8a6244bc807f481092cca4b70e8be75b",
580562
"systemd_units": [
581563
582-
583564
584565
"scion-bwtestserver.service",
585566
"scion-daemon.service",

scionlab/tests/test_user_as_models.py

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -231,25 +231,8 @@ def _check_attachment_point(testcase, attachment_point: AttachmentPoint):
231231
"""
232232
Check the assignment of interfaces to border routers in the attachment point.
233233
"""
234-
235234
host = attachment_point._get_host_for_useras_attachment()
236-
border_routers = list(host.border_routers.all())
237-
238-
# The first BR is for the infrastructure links and also contains the inactive interfaces.
239-
infra_br = border_routers.pop(0)
240-
for iface in infra_br.interfaces.iterator():
241-
testcase.assertTrue(iface.remote_as().owner is None or not iface.link().active)
242-
243-
# The other BRs contain up to 10 interfaces each.
244-
MAX_IFACES = 10
245-
for br in border_routers:
246-
# Expecting only active interfaces in these BRs
247-
testcase.assertTrue(all(interface.link().active for interface in br.interfaces.iterator()))
248-
c = br.interfaces.count()
249-
if br == border_routers[-1]: # only last one can have less than max
250-
testcase.assertLessEqual(c, MAX_IFACES)
251-
else:
252-
testcase.assertEqual(c, MAX_IFACES)
235+
testcase.assertEqual(host.border_routers.count(), 1)
253236

254237

255238
def update_useras(testcase,

scripts/deactivate_dormant.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from django.db import transaction
3131
from django.db.models import Q
3232

33-
from scionlab.models.user_as import UserAS, AttachmentPoint
33+
from scionlab.models.user_as import UserAS
3434

3535
nologin_threshold = datetime.timedelta(days=365)
3636

@@ -54,7 +54,3 @@ def deactivate_dormant(as_ids):
5454
for user_as in q:
5555
print(user_as.as_id)
5656
user_as.update_active(False)
57-
58-
# Fix-up distribution of user AS interfaces to router instances.
59-
for ap in AttachmentPoint.objects.all():
60-
ap.split_border_routers()

scripts/merge_ap_routers.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Copyright 2021 ETH Zurich
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
16+
"""
17+
:mod: scripts.merge_ap_routers
18+
=========================
19+
20+
Run with python manage.py runscript merge_ap_routers
21+
22+
For each attachment point, merge all border router instances such that only a single router is used.
23+
This is a cleanup step.
24+
Previously, we've automatically distributed the interfaces for attachment points over many router
25+
instances with at most ~12 interfaces each (`AttachmentPoint.split_border_routers`). This was a
26+
workaround for some issue in an old version of the router. Now, this is no longer necessary and
27+
running many instances is harmful for the system's performance.
28+
"""
29+
30+
from django.db import transaction
31+
32+
from scionlab.models.user_as import AttachmentPoint
33+
34+
35+
@transaction.atomic
36+
def run():
37+
for ap in AttachmentPoint.objects.all():
38+
merge_router_instances(ap.AS.hosts.get())
39+
40+
41+
def merge_router_instances(host):
42+
chosen = host.border_routers.first()
43+
for iface in host.interfaces.all():
44+
iface.border_router = chosen
45+
iface.save()
46+
host.border_routers.exclude(pk=chosen.pk).delete()

0 commit comments

Comments
 (0)