From 380d411833ba816bbe2b2188728dac31ba0b7a77 Mon Sep 17 00:00:00 2001
From: Blake Harnden <32446120+bharnden@users.noreply.github.com>
Date: Sat, 17 Oct 2020 08:18:49 -0700
Subject: [PATCH] daemon: updated linkconfig to calculate a limit when bw/delay
 are present, updated and simplified logic as well, leveraging code from
 outstanding pull request, updated code to factor in the mtu of the iface
 being configured

---
 daemon/core/api/tlv/corehandlers.py           |   8 +-
 .../configservices/frrservices/services.py    |   4 +-
 .../configservices/quaggaservices/services.py |   4 +-
 daemon/core/nodes/base.py                     |   2 -
 daemon/core/nodes/interface.py                |   6 +-
 daemon/core/nodes/network.py                  | 107 ++++++++----------
 daemon/core/nodes/physical.py                 |   4 +-
 daemon/core/services/frr.py                   |   4 +-
 daemon/core/services/quagga.py                |   4 +-
 9 files changed, 65 insertions(+), 78 deletions(-)

diff --git a/daemon/core/api/tlv/corehandlers.py b/daemon/core/api/tlv/corehandlers.py
index 73546f8e..b99b8630 100644
--- a/daemon/core/api/tlv/corehandlers.py
+++ b/daemon/core/api/tlv/corehandlers.py
@@ -788,14 +788,18 @@ class CoreHandler(socketserver.BaseRequestHandler):
         options = LinkOptions()
         options.delay = message.get_tlv(LinkTlvs.DELAY.value)
         options.bandwidth = message.get_tlv(LinkTlvs.BANDWIDTH.value)
-        options.loss = message.get_tlv(LinkTlvs.LOSS.value)
-        options.dup = message.get_tlv(LinkTlvs.DUP.value)
         options.jitter = message.get_tlv(LinkTlvs.JITTER.value)
         options.mer = message.get_tlv(LinkTlvs.MER.value)
         options.burst = message.get_tlv(LinkTlvs.BURST.value)
         options.mburst = message.get_tlv(LinkTlvs.MBURST.value)
         options.unidirectional = message.get_tlv(LinkTlvs.UNIDIRECTIONAL.value)
         options.key = message.get_tlv(LinkTlvs.KEY.value)
+        loss = message.get_tlv(LinkTlvs.LOSS.value)
+        dup = message.get_tlv(LinkTlvs.DUP.value)
+        if loss is not None:
+            options.loss = float(loss)
+        if dup is not None:
+            options.dup = int(dup)
 
         if message.flags & MessageFlags.ADD.value:
             self.session.add_link(
diff --git a/daemon/core/configservices/frrservices/services.py b/daemon/core/configservices/frrservices/services.py
index fa6f599a..d6ebacf3 100644
--- a/daemon/core/configservices/frrservices/services.py
+++ b/daemon/core/configservices/frrservices/services.py
@@ -5,7 +5,7 @@ from core.config import Configuration
 from core.configservice.base import ConfigService, ConfigServiceMode
 from core.emane.nodes import EmaneNet
 from core.nodes.base import CoreNodeBase
-from core.nodes.interface import CoreInterface
+from core.nodes.interface import DEFAULT_MTU, CoreInterface
 from core.nodes.network import WlanNode
 
 GROUP: str = "FRR"
@@ -18,7 +18,7 @@ def has_mtu_mismatch(iface: CoreInterface) -> bool:
     mtu-ignore command. This is needed when e.g. a node is linked via a
     GreTap device.
     """
-    if iface.mtu != 1500:
+    if iface.mtu != DEFAULT_MTU:
         return True
     if not iface.net:
         return False
diff --git a/daemon/core/configservices/quaggaservices/services.py b/daemon/core/configservices/quaggaservices/services.py
index bf23e00c..d3083ab6 100644
--- a/daemon/core/configservices/quaggaservices/services.py
+++ b/daemon/core/configservices/quaggaservices/services.py
@@ -6,7 +6,7 @@ from core.config import Configuration
 from core.configservice.base import ConfigService, ConfigServiceMode
 from core.emane.nodes import EmaneNet
 from core.nodes.base import CoreNodeBase
-from core.nodes.interface import CoreInterface
+from core.nodes.interface import DEFAULT_MTU, CoreInterface
 from core.nodes.network import WlanNode
 
 GROUP: str = "Quagga"
@@ -19,7 +19,7 @@ def has_mtu_mismatch(iface: CoreInterface) -> bool:
     mtu-ignore command. This is needed when e.g. a node is linked via a
     GreTap device.
     """
-    if iface.mtu != 1500:
+    if iface.mtu != DEFAULT_MTU:
         return True
     if not iface.net:
         return False
diff --git a/daemon/core/nodes/base.py b/daemon/core/nodes/base.py
index cf0c8738..9069b14c 100644
--- a/daemon/core/nodes/base.py
+++ b/daemon/core/nodes/base.py
@@ -30,8 +30,6 @@ if TYPE_CHECKING:
     CoreServices = List[Union[CoreService, Type[CoreService]]]
     ConfigServiceType = Type[ConfigService]
 
-_DEFAULT_MTU = 1500
-
 
 class NodeBase(abc.ABC):
     """
diff --git a/daemon/core/nodes/interface.py b/daemon/core/nodes/interface.py
index bc242eac..28f7f925 100644
--- a/daemon/core/nodes/interface.py
+++ b/daemon/core/nodes/interface.py
@@ -19,6 +19,8 @@ if TYPE_CHECKING:
     from core.emulator.session import Session
     from core.nodes.base import CoreNetworkBase, CoreNode
 
+DEFAULT_MTU: int = 1500
+
 
 class CoreInterface:
     """
@@ -338,7 +340,7 @@ class Veth(CoreInterface):
         node: "CoreNode",
         name: str,
         localname: str,
-        mtu: int = 1500,
+        mtu: int = DEFAULT_MTU,
         server: "DistributedServer" = None,
         start: bool = True,
     ) -> None:
@@ -403,7 +405,7 @@ class TunTap(CoreInterface):
         node: "CoreNode",
         name: str,
         localname: str,
-        mtu: int = 1500,
+        mtu: int = DEFAULT_MTU,
         server: "DistributedServer" = None,
         start: bool = True,
     ) -> None:
diff --git a/daemon/core/nodes/network.py b/daemon/core/nodes/network.py
index 58c1e195..a5510084 100644
--- a/daemon/core/nodes/network.py
+++ b/daemon/core/nodes/network.py
@@ -3,6 +3,7 @@ Defines network nodes used within core.
 """
 
 import logging
+import math
 import threading
 import time
 from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Type
@@ -447,77 +448,59 @@ class CoreNetwork(CoreNetworkBase):
         :param iface2: interface two
         :return: nothing
         """
-        devname = iface.localname
-        tc = f"{TC} qdisc replace dev {devname}"
-        parent = "root"
-        changed = False
-        bw = options.bandwidth
-        if iface.setparam("bw", bw):
-            # from tc-tbf(8): minimum value for burst is rate / kernel_hz
-            burst = max(2 * iface.mtu, int(bw / 1000))
-            # max IP payload
-            limit = 0xFFFF
-            tbf = f"tbf rate {bw} burst {burst} limit {limit}"
-            if bw > 0:
-                if self.up:
-                    cmd = f"{tc} {parent} handle 1: {tbf}"
-                    iface.host_cmd(cmd)
-                iface.setparam("has_tbf", True)
-                changed = True
-            elif iface.getparam("has_tbf") and bw <= 0:
-                if self.up:
-                    cmd = f"{TC} qdisc delete dev {devname} {parent}"
-                    iface.host_cmd(cmd)
-                iface.setparam("has_tbf", False)
-                # removing the parent removes the child
-                iface.setparam("has_netem", False)
-                changed = True
-        if iface.getparam("has_tbf"):
-            parent = "parent 1:1"
-        netem = "netem"
-        delay = options.delay
-        changed = max(changed, iface.setparam("delay", delay))
-        loss = options.loss
-        if loss is not None:
-            loss = float(loss)
-        changed = max(changed, iface.setparam("loss", loss))
-        duplicate = options.dup
-        if duplicate is not None:
-            duplicate = int(duplicate)
-        changed = max(changed, iface.setparam("duplicate", duplicate))
-        jitter = options.jitter
-        changed = max(changed, iface.setparam("jitter", jitter))
+        # determine if any settings have changed
+        changed = any(
+            [
+                iface.setparam("bw", options.bandwidth),
+                iface.setparam("delay", options.delay),
+                iface.setparam("loss", options.loss),
+                iface.setparam("duplicate", options.dup),
+                iface.setparam("jitter", options.jitter),
+            ]
+        )
         if not changed:
             return
-        # jitter and delay use the same delay statement
-        if delay is not None:
-            netem += f" delay {delay}us"
-        if jitter is not None:
-            if delay is None:
-                netem += f" delay 0us {jitter}us 25%"
-            else:
-                netem += f" {jitter}us 25%"
 
-        if loss is not None and loss > 0:
-            netem += f" loss {min(loss, 100)}%"
-        if duplicate is not None and duplicate > 0:
-            netem += f" duplicate {min(duplicate, 100)}%"
-
-        delay_check = delay is None or delay <= 0
-        jitter_check = jitter is None or jitter <= 0
-        loss_check = loss is None or loss <= 0
-        duplicate_check = duplicate is None or duplicate <= 0
-        if all([delay_check, jitter_check, loss_check, duplicate_check]):
-            # possibly remove netem if it exists and parent queue wasn't removed
+        # delete tc configuration or create and add it
+        devname = iface.localname
+        if all(
+            [
+                options.delay is None or options.delay <= 0,
+                options.jitter is None or options.jitter <= 0,
+                options.loss is None or options.loss <= 0,
+                options.dup is None or options.dup <= 0,
+                options.bandwidth is None or options.bandwidth <= 0,
+            ]
+        ):
             if not iface.getparam("has_netem"):
                 return
             if self.up:
-                cmd = f"{TC} qdisc delete dev {devname} {parent} handle 10:"
+                cmd = f"{TC} qdisc delete dev {devname} root handle 10:"
                 iface.host_cmd(cmd)
             iface.setparam("has_netem", False)
-        elif len(netem) > 1:
+        else:
+            netem = ""
+            if options.bandwidth is not None:
+                limit = 1000
+                bw = options.bandwidth / 1000
+                if options.delay and options.bandwidth:
+                    delay = options.delay / 1000
+                    limit = max(2, math.ceil((2 * bw * delay) / (8 * iface.mtu)))
+                netem += f" rate {bw}kbit"
+                netem += f" limit {limit}"
+            if options.delay is not None:
+                netem += f" delay {options.delay}us"
+            if options.jitter is not None:
+                if options.delay is None:
+                    netem += f" delay 0us {options.jitter}us 25%"
+                else:
+                    netem += f" {options.jitter}us 25%"
+            if options.loss is not None and options.loss > 0:
+                netem += f" loss {min(options.loss, 100)}%"
+            if options.dup is not None and options.dup > 0:
+                netem += f" duplicate {min(options.dup, 100)}%"
             if self.up:
-                cmd = f"{TC} qdisc replace dev {devname} {parent} handle 10: {netem}"
+                cmd = f"{TC} qdisc replace dev {devname} root handle 10: netem {netem}"
                 iface.host_cmd(cmd)
             iface.setparam("has_netem", True)
 
diff --git a/daemon/core/nodes/physical.py b/daemon/core/nodes/physical.py
index 22819f6d..e69985ef 100644
--- a/daemon/core/nodes/physical.py
+++ b/daemon/core/nodes/physical.py
@@ -13,7 +13,7 @@ from core.emulator.enumerations import NodeTypes, TransportType
 from core.errors import CoreCommandError, CoreError
 from core.executables import MOUNT, TEST, UMOUNT
 from core.nodes.base import CoreNetworkBase, CoreNodeBase
-from core.nodes.interface import CoreInterface
+from core.nodes.interface import DEFAULT_MTU, CoreInterface
 from core.nodes.network import CoreNetwork, GreTap
 
 if TYPE_CHECKING:
@@ -252,7 +252,7 @@ class Rj45Node(CoreNodeBase):
         session: "Session",
         _id: int = None,
         name: str = None,
-        mtu: int = 1500,
+        mtu: int = DEFAULT_MTU,
         server: DistributedServer = None,
     ) -> None:
         """
diff --git a/daemon/core/services/frr.py b/daemon/core/services/frr.py
index cec9d860..0677a1d8 100644
--- a/daemon/core/services/frr.py
+++ b/daemon/core/services/frr.py
@@ -8,7 +8,7 @@ import netaddr
 
 from core.emane.nodes import EmaneNet
 from core.nodes.base import CoreNode
-from core.nodes.interface import CoreInterface
+from core.nodes.interface import DEFAULT_MTU, CoreInterface
 from core.nodes.network import PtpNet, WlanNode
 from core.nodes.physical import Rj45Node
 from core.services.coreservices import CoreService
@@ -384,7 +384,7 @@ class FRROspfv2(FrrService):
         mtu-ignore command. This is needed when e.g. a node is linked via a
         GreTap device.
         """
-        if iface.mtu != 1500:
+        if iface.mtu != DEFAULT_MTU:
             # a workaround for PhysicalNode GreTap, which has no knowledge of
             # the other nodes/nets
             return "  ip ospf mtu-ignore\n"
diff --git a/daemon/core/services/quagga.py b/daemon/core/services/quagga.py
index 8c474fd8..f47da1d0 100644
--- a/daemon/core/services/quagga.py
+++ b/daemon/core/services/quagga.py
@@ -8,7 +8,7 @@ import netaddr
 from core.emane.nodes import EmaneNet
 from core.emulator.enumerations import LinkTypes
 from core.nodes.base import CoreNode
-from core.nodes.interface import CoreInterface
+from core.nodes.interface import DEFAULT_MTU, CoreInterface
 from core.nodes.network import PtpNet, WlanNode
 from core.nodes.physical import Rj45Node
 from core.services.coreservices import CoreService
@@ -301,7 +301,7 @@ class Ospfv2(QuaggaService):
         mtu-ignore command. This is needed when e.g. a node is linked via a
         GreTap device.
         """
-        if iface.mtu != 1500:
+        if iface.mtu != DEFAULT_MTU:
             # a workaround for PhysicalNode GreTap, which has no knowledge of
             # the other nodes/nets
             return "  ip ospf mtu-ignore\n"